diff --git a/crates/bevy_ecs/macros/src/fetch.rs b/crates/bevy_ecs/macros/src/fetch.rs index 8bb5597f0d..3ae328e952 100644 --- a/crates/bevy_ecs/macros/src/fetch.rs +++ b/crates/bevy_ecs/macros/src/fetch.rs @@ -1,9 +1,10 @@ +use bevy_macro_utils::ensure_no_collision; use proc_macro::TokenStream; use proc_macro2::{Ident, Span}; use quote::{quote, ToTokens}; use syn::{ parse::{Parse, ParseStream}, - parse_quote, + parse_macro_input, parse_quote, punctuated::Punctuated, Attribute, Data, DataStruct, DeriveInput, Field, Fields, }; @@ -25,7 +26,10 @@ mod field_attr_keywords { pub static WORLD_QUERY_ATTRIBUTE_NAME: &str = "world_query"; -pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { +pub fn derive_world_query_impl(input: TokenStream) -> TokenStream { + let tokens = input.clone(); + + let ast = parse_macro_input!(input as DeriveInput); let visibility = ast.vis; let mut fetch_struct_attributes = FetchStructAttributes::default(); @@ -104,13 +108,18 @@ pub fn derive_world_query_impl(ast: DeriveInput) -> TokenStream { }; let fetch_struct_name = Ident::new(&format!("{struct_name}Fetch"), Span::call_site()); + let fetch_struct_name = ensure_no_collision(fetch_struct_name, tokens.clone()); let read_only_fetch_struct_name = if fetch_struct_attributes.is_mutable { - Ident::new(&format!("{struct_name}ReadOnlyFetch"), Span::call_site()) + let new_ident = Ident::new(&format!("{struct_name}ReadOnlyFetch"), Span::call_site()); + ensure_no_collision(new_ident, tokens.clone()) } else { fetch_struct_name.clone() }; + // Generate a name for the state struct that doesn't conflict + // with the struct definition. let state_struct_name = Ident::new(&format!("{struct_name}State"), Span::call_site()); + let state_struct_name = ensure_no_collision(state_struct_name, tokens); let fields = match &ast.data { Data::Struct(DataStruct { diff --git a/crates/bevy_ecs/macros/src/lib.rs b/crates/bevy_ecs/macros/src/lib.rs index a59c54db8d..3866cea315 100644 --- a/crates/bevy_ecs/macros/src/lib.rs +++ b/crates/bevy_ecs/macros/src/lib.rs @@ -6,7 +6,9 @@ mod set; mod states; use crate::{fetch::derive_world_query_impl, set::derive_set}; -use bevy_macro_utils::{derive_boxed_label, get_named_struct_fields, BevyManifest}; +use bevy_macro_utils::{ + derive_boxed_label, ensure_no_collision, get_named_struct_fields, BevyManifest, +}; use proc_macro::TokenStream; use proc_macro2::Span; use quote::{format_ident, quote}; @@ -260,6 +262,7 @@ static SYSTEM_PARAM_ATTRIBUTE_NAME: &str = "system_param"; /// Implement `SystemParam` to use a struct as a parameter in a system #[proc_macro_derive(SystemParam, attributes(system_param))] pub fn derive_system_param(input: TokenStream) -> TokenStream { + let token_stream = input.clone(); let ast = parse_macro_input!(input as DeriveInput); let syn::Data::Struct(syn::DataStruct { fields: field_definitions, ..}) = ast.data else { return syn::Error::new(ast.span(), "Invalid `SystemParam` type: expected a `struct`") @@ -394,6 +397,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { let struct_name = &ast.ident; let state_struct_visibility = &ast.vis; + let state_struct_name = ensure_no_collision(format_ident!("FetchState"), token_stream); TokenStream::from(quote! { // We define the FetchState struct in an anonymous scope to avoid polluting the user namespace. @@ -401,7 +405,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { // as SystemParam>::State const _: () = { #[doc(hidden)] - #state_struct_visibility struct FetchState <'w, 's, #(#lifetimeless_generics,)*> + #state_struct_visibility struct #state_struct_name <'w, 's, #(#lifetimeless_generics,)*> #where_clause { state: (#(<#tuple_types as #path::system::SystemParam>::State,)*), marker: std::marker::PhantomData<( @@ -411,11 +415,11 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { } unsafe impl<'w, 's, #punctuated_generics> #path::system::SystemParam for #struct_name #ty_generics #where_clause { - type State = FetchState<'static, 'static, #punctuated_generic_idents>; + type State = #state_struct_name<'static, 'static, #punctuated_generic_idents>; type Item<'_w, '_s> = #struct_name <#(#shadowed_lifetimes,)* #punctuated_generic_idents>; fn init_state(world: &mut #path::world::World, system_meta: &mut #path::system::SystemMeta) -> Self::State { - FetchState { + #state_struct_name { state: <(#(#tuple_types,)*) as #path::system::SystemParam>::init_state(world, system_meta), marker: std::marker::PhantomData, } @@ -454,8 +458,7 @@ pub fn derive_system_param(input: TokenStream) -> TokenStream { /// Implement `WorldQuery` to use a struct as a parameter in a query #[proc_macro_derive(WorldQuery, attributes(world_query))] pub fn derive_world_query(input: TokenStream) -> TokenStream { - let ast = parse_macro_input!(input as DeriveInput); - derive_world_query_impl(ast) + derive_world_query_impl(input) } /// Derive macro generating an impl of the trait `ScheduleLabel`. diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 3b4a6a076f..17aa0203ed 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -1388,3 +1388,37 @@ unsafe impl WorldQuery for NopWorldQuery { /// SAFETY: `NopFetch` never accesses any data unsafe impl ReadOnlyWorldQuery for NopWorldQuery {} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{self as bevy_ecs, system::Query}; + + // Ensures that metadata types generated by the WorldQuery macro + // do not conflict with user-defined types. + // Regression test for https://github.com/bevyengine/bevy/issues/8010. + #[test] + fn world_query_metadata_collision() { + // The metadata types generated would be named `ClientState` and `ClientFetch`, + // but they should rename themselves to avoid conflicts. + #[derive(WorldQuery)] + pub struct Client { + pub state: &'static S, + pub fetch: &'static ClientFetch, + } + + pub trait ClientState: Component {} + + #[derive(Component)] + pub struct ClientFetch; + + #[derive(Component)] + pub struct C; + + impl ClientState for C {} + + fn client_system(_: Query>) {} + + crate::system::assert_is_system(client_system); + } +} diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 47f668d8ae..d0668025c7 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -1625,7 +1625,7 @@ mod tests { #[derive(SystemParam)] pub struct EncapsulatedParam<'w>(Res<'w, PrivateResource>); - // regression test for https://github.com/bevyengine/bevy/issues/7103. + // Regression test for https://github.com/bevyengine/bevy/issues/7103. #[derive(SystemParam)] pub struct WhereParam<'w, 's, Q> where @@ -1633,4 +1633,13 @@ mod tests { { _q: Query<'w, 's, Q, ()>, } + + // Regression test for https://github.com/bevyengine/bevy/issues/1727. + #[derive(SystemParam)] + pub struct Collide<'w> { + _x: Res<'w, FetchState>, + } + + #[derive(Resource)] + pub struct FetchState; } diff --git a/crates/bevy_macro_utils/Cargo.toml b/crates/bevy_macro_utils/Cargo.toml index 8b9149864a..76cb1d690f 100644 --- a/crates/bevy_macro_utils/Cargo.toml +++ b/crates/bevy_macro_utils/Cargo.toml @@ -12,3 +12,4 @@ keywords = ["bevy"] toml_edit = "0.19" syn = "1.0" quote = "1.0" +rustc-hash = "1.0" diff --git a/crates/bevy_macro_utils/src/lib.rs b/crates/bevy_macro_utils/src/lib.rs index 3fd275ceb9..3a72994510 100644 --- a/crates/bevy_macro_utils/src/lib.rs +++ b/crates/bevy_macro_utils/src/lib.rs @@ -8,10 +8,11 @@ pub use attrs::*; pub use shape::*; pub use symbol::*; -use proc_macro::TokenStream; +use proc_macro::{TokenStream, TokenTree}; use quote::{quote, quote_spanned}; +use rustc_hash::FxHashSet; use std::{env, path::PathBuf}; -use syn::spanned::Spanned; +use syn::{spanned::Spanned, Ident}; use toml_edit::{Document, Item}; pub struct BevyManifest { @@ -108,6 +109,48 @@ impl BevyManifest { } } +/// Finds an identifier that will not conflict with the specified set of tokens. +/// If the identifier is present in `haystack`, extra characters will be added +/// to it until it no longer conflicts with anything. +/// +/// Note that the returned identifier can still conflict in niche cases, +/// such as if an identifier in `haystack` is hidden behind an un-expanded macro. +pub fn ensure_no_collision(value: Ident, haystack: TokenStream) -> Ident { + // Collect all the identifiers in `haystack` into a set. + let idents = { + // List of token streams that will be visited in future loop iterations. + let mut unvisited = vec![haystack]; + // Identifiers we have found while searching tokens. + let mut found = FxHashSet::default(); + while let Some(tokens) = unvisited.pop() { + for t in tokens { + match t { + // Collect any identifiers we encounter. + TokenTree::Ident(ident) => { + found.insert(ident.to_string()); + } + // Queue up nested token streams to be visited in a future loop iteration. + TokenTree::Group(g) => unvisited.push(g.stream()), + TokenTree::Punct(_) | TokenTree::Literal(_) => {} + } + } + } + + found + }; + + let span = value.span(); + + // If there's a collision, add more characters to the identifier + // until it doesn't collide with anything anymore. + let mut value = value.to_string(); + while idents.contains(&value) { + value.push('X'); + } + + Ident::new(&value, span) +} + /// Derive a label trait /// /// # Args