diff --git a/crates/bevy_ecs/src/archetype.rs b/crates/bevy_ecs/src/archetype.rs index e0d85242f9206..e3b8b8dac545d 100644 --- a/crates/bevy_ecs/src/archetype.rs +++ b/crates/bevy_ecs/src/archetype.rs @@ -789,7 +789,10 @@ pub struct Archetypes { pub struct ArchetypeRecord { /// Index of the component in the archetype's [`Table`](crate::storage::Table), /// or None if the component is a sparse set component. - #[allow(dead_code)] + #[expect( + dead_code, + reason = "Currently unused, but planned to be used to implement a component index to improve performance of fragmenting relations." + )] pub(crate) column: Option, } @@ -827,7 +830,10 @@ impl Archetypes { /// Fetches the total number of [`Archetype`]s within the world. #[inline] - #[allow(clippy::len_without_is_empty)] // the internal vec is never empty. + #[expect( + clippy::len_without_is_empty, + reason = "The internal vec is never empty" + )] pub fn len(&self) -> usize { self.archetypes.len() } diff --git a/crates/bevy_ecs/src/bundle.rs b/crates/bevy_ecs/src/bundle.rs index 4dc96311bb25a..6b4d9da2811bb 100644 --- a/crates/bevy_ecs/src/bundle.rs +++ b/crates/bevy_ecs/src/bundle.rs @@ -246,6 +246,15 @@ impl DynamicBundle for C { macro_rules! tuple_impl { ($(#[$meta:meta])* $($name: ident),*) => { + #[expect( + clippy::allow_attributes, + reason = "This is a tuple-related macro; as such, the lints below may not always apply." + )] + #[allow( + unused_mut, + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] $(#[$meta])* // SAFETY: // - `Bundle::component_ids` calls `ids` for each component type in the @@ -254,43 +263,57 @@ macro_rules! tuple_impl { // - `Bundle::get_components` is called exactly once for each member. Relies on the above implementation to pass the correct // `StorageType` into the callback. unsafe impl<$($name: Bundle),*> Bundle for ($($name,)*) { - #[allow(unused_variables)] fn component_ids(components: &mut Components, storages: &mut Storages, ids: &mut impl FnMut(ComponentId)){ $(<$name as Bundle>::component_ids(components, storages, ids);)* } - #[allow(unused_variables)] fn get_component_ids(components: &Components, ids: &mut impl FnMut(Option)){ $(<$name as Bundle>::get_component_ids(components, ids);)* } - #[allow(unused_variables, unused_mut)] - #[allow(clippy::unused_unit)] + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples will generate a function body equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." + )] unsafe fn from_components(ctx: &mut T, func: &mut F) -> Self where F: FnMut(&mut T) -> OwningPtr<'_> { - #[allow(unused_unsafe)] + #[allow( + unused_unsafe, + reason = "Zero-length tuples will not run anything in the unsafe block. Additionally, rewriting this to move the () outside of the unsafe would require putting the safety comment inside the tuple, hurting readability of the code." + )] // SAFETY: Rust guarantees that tuple calls are evaluated 'left to right'. // https://doc.rust-lang.org/reference/expressions.html#evaluation-order-of-operands unsafe { ($(<$name as Bundle>::from_components(ctx, func),)*) } } fn register_required_components( - _components: &mut Components, - _storages: &mut Storages, - _required_components: &mut RequiredComponents, + components: &mut Components, + storages: &mut Storages, + required_components: &mut RequiredComponents, ) { - $(<$name as Bundle>::register_required_components(_components, _storages, _required_components);)* + $(<$name as Bundle>::register_required_components(components, storages, required_components);)* } } + #[expect( + clippy::allow_attributes, + reason = "This is a tuple-related macro; as such, the lints below may not always apply." + )] + #[allow( + unused_mut, + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] $(#[$meta])* impl<$($name: Bundle),*> DynamicBundle for ($($name,)*) { - #[allow(unused_variables, unused_mut)] #[inline(always)] fn get_components(self, func: &mut impl FnMut(StorageType, OwningPtr<'_>)) { - #[allow(non_snake_case)] + #[allow( + non_snake_case, + reason = "The names of these variables are provided by the caller, not by us." + )] let ($(mut $name,)*) = self; $( $name.get_components(&mut *func); diff --git a/crates/bevy_ecs/src/change_detection.rs b/crates/bevy_ecs/src/change_detection.rs index 6be63ba964b87..e238287a7ec3d 100644 --- a/crates/bevy_ecs/src/change_detection.rs +++ b/crates/bevy_ecs/src/change_detection.rs @@ -609,7 +609,10 @@ impl<'w, T: Resource> Res<'w, T> { /// /// Note that unless you actually need an instance of `Res`, you should /// prefer to just convert it to `&T` which can be freely copied. - #[allow(clippy::should_implement_trait)] + #[expect( + clippy::should_implement_trait, + reason = "As this struct derefs to the inner resource, a `Clone` trait implementation would interfere with the common case of cloning the inner content. (A similar case of this happening can be found with `std::cell::Ref::clone()`.)" + )] pub fn clone(this: &Self) -> Self { Self { value: this.value, diff --git a/crates/bevy_ecs/src/entity/entity_set.rs b/crates/bevy_ecs/src/entity/entity_set.rs index be8ac88d04595..e2d77a98fd46a 100644 --- a/crates/bevy_ecs/src/entity/entity_set.rs +++ b/crates/bevy_ecs/src/entity/entity_set.rs @@ -382,25 +382,24 @@ impl + Debug> Debug for UniqueEntityIter< mod tests { use alloc::{vec, vec::Vec}; - #[allow(unused_imports)] use crate::prelude::{Schedule, World}; - #[allow(unused_imports)] use crate::component::Component; + use crate::entity::Entity; use crate::query::{QueryState, With}; use crate::system::Query; use crate::world::Mut; - #[allow(unused_imports)] use crate::{self as bevy_ecs}; - #[allow(unused_imports)] - use crate::{entity::Entity, world::unsafe_world_cell}; use super::UniqueEntityIter; #[derive(Component, Clone)] pub struct Thing; - #[allow(clippy::iter_skip_zero)] + #[expect( + clippy::iter_skip_zero, + reason = "The `skip(0)` is used to ensure that the `Skip` iterator implements `EntitySet`, which is needed to pass the iterator as the `entities` parameter." + )] #[test] fn preserving_uniqueness() { let mut world = World::new(); diff --git a/crates/bevy_ecs/src/entity/mod.rs b/crates/bevy_ecs/src/entity/mod.rs index 07df26a09a78d..946b6821a4e40 100644 --- a/crates/bevy_ecs/src/entity/mod.rs +++ b/crates/bevy_ecs/src/entity/mod.rs @@ -592,7 +592,14 @@ impl Entities { /// Reserve entity IDs concurrently. /// /// Storage for entity generation and location is lazily allocated by calling [`flush`](Entities::flush). - #[allow(clippy::unnecessary_fallible_conversions)] // Because `IdCursor::try_from` may fail on 32-bit platforms. + #[expect( + clippy::allow_attributes, + reason = "`clippy::unnecessary_fallible_conversions` may not always lint." + )] + #[allow( + clippy::unnecessary_fallible_conversions, + reason = "`IdCursor::try_from` may fail on 32-bit platforms." + )] pub fn reserve_entities(&self, count: u32) -> ReserveEntitiesIterator { // Use one atomic subtract to grab a range of new IDs. The range might be // entirely nonnegative, meaning all IDs come from the freelist, or entirely @@ -786,7 +793,14 @@ impl Entities { } /// Ensure at least `n` allocations can succeed without reallocating. - #[allow(clippy::unnecessary_fallible_conversions)] // Because `IdCursor::try_from` may fail on 32-bit platforms. + #[expect( + clippy::allow_attributes, + reason = "`clippy::unnecessary_fallible_conversions` may not always lint." + )] + #[allow( + clippy::unnecessary_fallible_conversions, + reason = "`IdCursor::try_from` may fail on 32-bit platforms." + )] pub fn reserve(&mut self, additional: u32) { self.verify_flushed(); @@ -1178,7 +1192,10 @@ mod tests { } #[test] - #[allow(clippy::nonminimal_bool)] // This is intentionally testing `lt` and `ge` as separate functions. + #[expect( + clippy::nonminimal_bool, + reason = "This intentionally tests all possible comparison operators as separate functions; thus, we don't want to rewrite these comparisons to use different operators." + )] fn entity_comparison() { assert_eq!( Entity::from_raw_and_generation(123, NonZero::::new(456).unwrap()), diff --git a/crates/bevy_ecs/src/entity/visit_entities.rs b/crates/bevy_ecs/src/entity/visit_entities.rs index a9f5e8dcbd847..0896d7208100a 100644 --- a/crates/bevy_ecs/src/entity/visit_entities.rs +++ b/crates/bevy_ecs/src/entity/visit_entities.rs @@ -71,7 +71,6 @@ mod tests { ordered: Vec, unordered: HashSet, single: Entity, - #[allow(dead_code)] #[visit_entities(ignore)] not_an_entity: String, } diff --git a/crates/bevy_ecs/src/event/event_cursor.rs b/crates/bevy_ecs/src/event/event_cursor.rs index ca1be152e5caa..262d4e3636adc 100644 --- a/crates/bevy_ecs/src/event/event_cursor.rs +++ b/crates/bevy_ecs/src/event/event_cursor.rs @@ -74,7 +74,6 @@ impl Clone for EventCursor { } } -#[allow(clippy::len_without_is_empty)] // Check fails since the is_empty implementation has a signature other than `(&self) -> bool` impl EventCursor { /// See [`EventReader::read`](super::EventReader::read) pub fn read<'a>(&'a mut self, events: &'a Events) -> EventIterator<'a, E> { diff --git a/crates/bevy_ecs/src/event/mod.rs b/crates/bevy_ecs/src/event/mod.rs index 1dbcf1ba0cb14..c8bd24dcdde5f 100644 --- a/crates/bevy_ecs/src/event/mod.rs +++ b/crates/bevy_ecs/src/event/mod.rs @@ -568,7 +568,6 @@ mod tests { assert!(last.is_none(), "EventMutator should be empty"); } - #[allow(clippy::iter_nth_zero)] #[test] fn test_event_reader_iter_nth() { use bevy_ecs::prelude::*; @@ -595,7 +594,6 @@ mod tests { schedule.run(&mut world); } - #[allow(clippy::iter_nth_zero)] #[test] fn test_event_mutator_iter_nth() { use bevy_ecs::prelude::*; diff --git a/crates/bevy_ecs/src/identifier/mod.rs b/crates/bevy_ecs/src/identifier/mod.rs index 6134e472427e2..cf467da505dfc 100644 --- a/crates/bevy_ecs/src/identifier/mod.rs +++ b/crates/bevy_ecs/src/identifier/mod.rs @@ -216,7 +216,10 @@ mod tests { #[rustfmt::skip] #[test] - #[allow(clippy::nonminimal_bool)] // This is intentionally testing `lt` and `ge` as separate functions. + #[expect( + clippy::nonminimal_bool, + reason = "This intentionally tests all possible comparison operators as separate functions; thus, we don't want to rewrite these comparisons to use different operators." + )] fn id_comparison() { assert!(Identifier::new(123, 456, IdKind::Entity).unwrap() == Identifier::new(123, 456, IdKind::Entity).unwrap()); assert!(Identifier::new(123, 456, IdKind::Placeholder).unwrap() == Identifier::new(123, 456, IdKind::Placeholder).unwrap()); diff --git a/crates/bevy_ecs/src/lib.rs b/crates/bevy_ecs/src/lib.rs index 860916362fc99..5b2b2d7a891e6 100644 --- a/crates/bevy_ecs/src/lib.rs +++ b/crates/bevy_ecs/src/lib.rs @@ -1,7 +1,14 @@ -// FIXME(11590): remove this once the lint is fixed -#![allow(unsafe_op_in_unsafe_fn)] -// TODO: remove once Edition 2024 is released -#![allow(dependency_on_unit_never_type_fallback)] +#![expect( + unsafe_op_in_unsafe_fn, + reason = "See #11590. To be removed once all applicable unsafe code has an unsafe block with a safety comment." +)] +#![cfg_attr( + test, + expect( + dependency_on_unit_never_type_fallback, + reason = "See #17340. To be removed once Edition 2024 is released" + ) +)] #![doc = include_str!("../README.md")] #![cfg_attr( any(docsrs, docsrs_dep), @@ -11,7 +18,12 @@ ) )] #![cfg_attr(any(docsrs, docsrs_dep), feature(doc_auto_cfg, rustdoc_internals))] -#![allow(unsafe_code)] +#![expect(unsafe_code, reason = "Unsafe code is used to improve performance.")] +#![warn( + clippy::allow_attributes, + clippy::allow_attributes_without_reason, + reason = "See #17111; To be removed once all crates are in-line with these attributes" +)] #![doc( html_logo_url = "https://bevyengine.org/assets/icon.png", html_favicon_url = "https://bevyengine.org/assets/icon.png" @@ -55,7 +67,10 @@ pub use bevy_ptr as ptr; /// /// This includes the most common types in this crate, re-exported for your convenience. pub mod prelude { - #[expect(deprecated)] + #[expect( + deprecated, + reason = "`crate::schedule::apply_deferred` is considered deprecated; however, it may still be used by crates which consume `bevy_ecs`, so its removal here may cause confusion. It is intended to be removed in the Bevy 0.17 cycle." + )] #[doc(hidden)] pub use crate::{ bundle::Bundle, @@ -147,9 +162,8 @@ mod tests { #[derive(Component, Debug, PartialEq, Eq, Clone, Copy)] struct C; - #[allow(dead_code)] #[derive(Default)] - struct NonSendA(usize, PhantomData<*mut ()>); + struct NonSendA(PhantomData<*mut ()>); #[derive(Component, Clone, Debug)] struct DropCk(Arc); @@ -166,8 +180,10 @@ mod tests { } } - // TODO: The compiler says the Debug and Clone are removed during dead code analysis. Investigate. - #[allow(dead_code)] + #[expect( + dead_code, + reason = "This struct is used to test how `Drop` behavior works in regards to SparseSet storage, and as such is solely a wrapper around `DropCk` to make it use the SparseSet storage. Because of this, the inner field is intentionally never read." + )] #[derive(Component, Clone, Debug)] #[component(storage = "SparseSet")] struct DropCkSparse(DropCk); @@ -2641,42 +2657,49 @@ mod tests { World::new().register_component::(); } - // These structs are primarily compilation tests to test the derive macros. Because they are - // never constructed, we have to manually silence the `dead_code` lint. - #[allow(dead_code)] + #[expect( + dead_code, + reason = "This struct is used as a compilation test to test the derive macros, and as such is intentionally never constructed." + )] #[derive(Component)] struct ComponentA(u32); - #[allow(dead_code)] + #[expect( + dead_code, + reason = "This struct is used as a compilation test to test the derive macros, and as such is intentionally never constructed." + )] #[derive(Component)] struct ComponentB(u32); - #[allow(dead_code)] #[derive(Bundle)] struct Simple(ComponentA); - #[allow(dead_code)] #[derive(Bundle)] struct Tuple(Simple, ComponentB); - #[allow(dead_code)] #[derive(Bundle)] struct Record { field0: Simple, field1: ComponentB, } - #[allow(dead_code)] #[derive(Component, VisitEntities, VisitEntitiesMut)] struct MyEntities { entities: Vec, another_one: Entity, maybe_entity: Option, + #[expect( + dead_code, + reason = "This struct is used as a compilation test to test the derive macros, and as such this field is intentionally never used." + )] #[visit_entities(ignore)] something_else: String, } - #[allow(dead_code)] + #[expect( + dead_code, + reason = "This struct is used as a compilation test to test the derive macros, and as such is intentionally never constructed." + )] #[derive(Component, VisitEntities, VisitEntitiesMut)] struct MyEntitiesTuple(Vec, Entity, #[visit_entities(ignore)] usize); } diff --git a/crates/bevy_ecs/src/query/fetch.rs b/crates/bevy_ecs/src/query/fetch.rs index 6df9b8e2de745..afb56206454e4 100644 --- a/crates/bevy_ecs/src/query/fetch.rs +++ b/crates/bevy_ecs/src/query/fetch.rs @@ -2015,8 +2015,6 @@ pub struct AnyOf(PhantomData); macro_rules! impl_tuple_query_data { ($(#[$meta:meta])* $(($name: ident, $state: ident)),*) => { - #[allow(non_snake_case)] - #[allow(clippy::unused_unit)] $(#[$meta])* // SAFETY: defers to soundness `$name: WorldQuery` impl unsafe impl<$($name: QueryData),*> QueryData for ($($name,)*) { @@ -2033,8 +2031,22 @@ macro_rules! impl_tuple_query_data { macro_rules! impl_anytuple_fetch { ($(#[$meta:meta])* $(($name: ident, $state: ident)),*) => { $(#[$meta])* - #[allow(non_snake_case)] - #[allow(clippy::unused_unit)] + #[expect( + clippy::allow_attributes, + reason = "This is a tuple-related macro; as such the lints below may not always apply." + )] + #[allow( + non_snake_case, + reason = "The names of some variables are provided by the macro's caller, not by us." + )] + #[allow( + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples will generate some function bodies equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." + )] /// SAFETY: /// `fetch` accesses are a subset of the subqueries' accesses /// This is sound because `update_component_access` and `update_archetype_component_access` adds accesses according to the implementations of all the subqueries. @@ -2059,7 +2071,6 @@ macro_rules! impl_anytuple_fetch { } #[inline] - #[allow(clippy::unused_unit)] unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; // SAFETY: The invariants are uphold by the caller. @@ -2100,7 +2111,6 @@ macro_rules! impl_anytuple_fetch { } #[inline(always)] - #[allow(clippy::unused_unit)] unsafe fn fetch<'w>( _fetch: &mut Self::Fetch<'w>, _entity: Entity, @@ -2139,11 +2149,9 @@ macro_rules! impl_anytuple_fetch { <($(Option<$name>,)*)>::update_component_access(state, access); } - #[allow(unused_variables)] fn init_state(world: &mut World) -> Self::State { ($($name::init_state(world),)*) } - #[allow(unused_variables)] fn get_state(components: &Components) -> Option { Some(($($name::get_state(components)?,)*)) } @@ -2155,8 +2163,6 @@ macro_rules! impl_anytuple_fetch { } $(#[$meta])* - #[allow(non_snake_case)] - #[allow(clippy::unused_unit)] // SAFETY: defers to soundness of `$name: WorldQuery` impl unsafe impl<$($name: QueryData),*> QueryData for AnyOf<($($name,)*)> { type ReadOnly = AnyOf<($($name::ReadOnly,)*)>; diff --git a/crates/bevy_ecs/src/query/filter.rs b/crates/bevy_ecs/src/query/filter.rs index 2540e7cc7abe3..3a3d9a0162769 100644 --- a/crates/bevy_ecs/src/query/filter.rs +++ b/crates/bevy_ecs/src/query/filter.rs @@ -98,7 +98,6 @@ pub unsafe trait QueryFilter: WorldQuery { /// /// Must always be called _after_ [`WorldQuery::set_table`] or [`WorldQuery::set_archetype`]. `entity` and /// `table_row` must be in the range of the current table and archetype. - #[allow(unused_variables)] unsafe fn filter_fetch( fetch: &mut Self::Fetch<'_>, entity: Entity, @@ -381,9 +380,22 @@ impl Clone for OrFetch<'_, T> { macro_rules! impl_or_query_filter { ($(#[$meta:meta])* $(($filter: ident, $state: ident)),*) => { $(#[$meta])* - #[allow(unused_variables)] - #[allow(non_snake_case)] - #[allow(clippy::unused_unit)] + #[expect( + clippy::allow_attributes, + reason = "This is a tuple-related macro; as such the lints below may not always apply." + )] + #[allow( + non_snake_case, + reason = "The names of some variables are provided by the macro's caller, not by us." + )] + #[allow( + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples will generate some function bodies equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." + )] /// SAFETY: /// `fetch` accesses are a subset of the subqueries' accesses /// This is sound because `update_component_access` adds accesses according to the implementations of all the subqueries. @@ -454,34 +466,34 @@ macro_rules! impl_or_query_filter { #[inline(always)] unsafe fn fetch<'w>( fetch: &mut Self::Fetch<'w>, - _entity: Entity, - _table_row: TableRow + entity: Entity, + table_row: TableRow ) -> Self::Item<'w> { let ($($filter,)*) = fetch; // SAFETY: The invariants are uphold by the caller. - false $(|| ($filter.matches && unsafe { $filter::filter_fetch(&mut $filter.fetch, _entity, _table_row) }))* + false $(|| ($filter.matches && unsafe { $filter::filter_fetch(&mut $filter.fetch, entity, table_row) }))* } fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { let ($($filter,)*) = state; - let mut _new_access = FilteredAccess::matches_nothing(); + let mut new_access = FilteredAccess::matches_nothing(); $( // Create an intermediate because `access`'s value needs to be preserved // for the next filter, and `_new_access` has to be modified only by `append_or` to it. let mut intermediate = access.clone(); $filter::update_component_access($filter, &mut intermediate); - _new_access.append_or(&intermediate); + new_access.append_or(&intermediate); // Also extend the accesses required to compute the filter. This is required because // otherwise a `Query<(), Or<(Changed,)>` won't conflict with `Query<&mut Foo>`. - _new_access.extend_access(&intermediate); + new_access.extend_access(&intermediate); )* // The required components remain the same as the original `access`. - _new_access.required = core::mem::take(&mut access.required); + new_access.required = core::mem::take(&mut access.required); - *access = _new_access; + *access = new_access; } fn init_state(world: &mut World) -> Self::State { @@ -492,15 +504,15 @@ macro_rules! impl_or_query_filter { Some(($($filter::get_state(components)?,)*)) } - fn matches_component_set(_state: &Self::State, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool { - let ($($filter,)*) = _state; - false $(|| $filter::matches_component_set($filter, _set_contains_id))* + fn matches_component_set(state: &Self::State, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool { + let ($($filter,)*) = state; + false $(|| $filter::matches_component_set($filter, set_contains_id))* } } - $(#[$meta])* - // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. - unsafe impl<$($filter: QueryFilter),*> QueryFilter for Or<($($filter,)*)> { + $(#[$meta])* + // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. + unsafe impl<$($filter: QueryFilter),*> QueryFilter for Or<($($filter,)*)> { const IS_ARCHETYPAL: bool = true $(&& $filter::IS_ARCHETYPAL)*; #[inline(always)] @@ -518,9 +530,18 @@ macro_rules! impl_or_query_filter { macro_rules! impl_tuple_query_filter { ($(#[$meta:meta])* $($name: ident),*) => { - #[allow(unused_variables)] - #[allow(non_snake_case)] - #[allow(clippy::unused_unit)] + #[expect( + clippy::allow_attributes, + reason = "This is a tuple-related macro; as such the lints below may not always apply." + )] + #[allow( + non_snake_case, + reason = "The names of some variables are provided by the macro's caller, not by us." + )] + #[allow( + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] $(#[$meta])* // SAFETY: This only performs access that subqueries perform, and they impl `QueryFilter` and so perform no mutable access. unsafe impl<$($name: QueryFilter),*> QueryFilter for ($($name,)*) { @@ -529,12 +550,12 @@ macro_rules! impl_tuple_query_filter { #[inline(always)] unsafe fn filter_fetch( fetch: &mut Self::Fetch<'_>, - _entity: Entity, - _table_row: TableRow + entity: Entity, + table_row: TableRow ) -> bool { let ($($name,)*) = fetch; // SAFETY: The invariants are uphold by the caller. - true $(&& unsafe { $name::filter_fetch($name, _entity, _table_row) })* + true $(&& unsafe { $name::filter_fetch($name, entity, table_row) })* } } diff --git a/crates/bevy_ecs/src/query/iter.rs b/crates/bevy_ecs/src/query/iter.rs index 13eaf3f08c0a4..2fa595b925a28 100644 --- a/crates/bevy_ecs/src/query/iter.rs +++ b/crates/bevy_ecs/src/query/iter.rs @@ -2935,7 +2935,10 @@ impl PartialEq for NeutralOrd { impl Eq for NeutralOrd {} -#[allow(clippy::non_canonical_partial_ord_impl)] +#[expect( + clippy::non_canonical_partial_ord_impl, + reason = "`PartialOrd` and `Ord` on this struct must only ever return `Ordering::Equal`, so we prefer clarity" +)] impl PartialOrd for NeutralOrd { fn partial_cmp(&self, _other: &Self) -> Option { Some(Ordering::Equal) @@ -2953,13 +2956,9 @@ mod tests { use alloc::vec::Vec; use std::println; - #[allow(unused_imports)] use crate::component::Component; - #[allow(unused_imports)] use crate::entity::Entity; - #[allow(unused_imports)] use crate::prelude::World; - #[allow(unused_imports)] use crate::{self as bevy_ecs}; #[derive(Component, Debug, PartialEq, PartialOrd, Clone, Copy)] @@ -2968,7 +2967,6 @@ mod tests { #[component(storage = "SparseSet")] struct Sparse(usize); - #[allow(clippy::unnecessary_sort_by)] #[test] fn query_iter_sorts() { let mut world = World::new(); @@ -3156,7 +3154,6 @@ mod tests { } } - #[allow(clippy::unnecessary_sort_by)] #[test] fn query_iter_many_sorts() { let mut world = World::new(); diff --git a/crates/bevy_ecs/src/query/world_query.rs b/crates/bevy_ecs/src/query/world_query.rs index 3f9111ff3c566..88164e3398634 100644 --- a/crates/bevy_ecs/src/query/world_query.rs +++ b/crates/bevy_ecs/src/query/world_query.rs @@ -157,8 +157,22 @@ pub unsafe trait WorldQuery { macro_rules! impl_tuple_world_query { ($(#[$meta:meta])* $(($name: ident, $state: ident)),*) => { - #[allow(non_snake_case)] - #[allow(clippy::unused_unit)] + #[expect( + clippy::allow_attributes, + reason = "This is a tuple-related macro; as such the lints below may not always apply." + )] + #[allow( + non_snake_case, + reason = "The names of some variables are provided by the macro's caller, not by us." + )] + #[allow( + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples will generate some function bodies equivalent to `()`; however, this macro is meant for all applicable tuples, and as such it makes no sense to rewrite it just for that case." + )] $(#[$meta])* /// SAFETY: /// `fetch` accesses are the conjunction of the subqueries' accesses @@ -185,64 +199,60 @@ macro_rules! impl_tuple_world_query { } #[inline] - #[allow(clippy::unused_unit)] - unsafe fn init_fetch<'w>(_world: UnsafeWorldCell<'w>, state: &Self::State, _last_run: Tick, _this_run: Tick) -> Self::Fetch<'w> { + unsafe fn init_fetch<'w>(world: UnsafeWorldCell<'w>, state: &Self::State, last_run: Tick, this_run: Tick) -> Self::Fetch<'w> { let ($($name,)*) = state; // SAFETY: The invariants are uphold by the caller. - ($(unsafe { $name::init_fetch(_world, $name, _last_run, _this_run) },)*) + ($(unsafe { $name::init_fetch(world, $name, last_run, this_run) },)*) } const IS_DENSE: bool = true $(&& $name::IS_DENSE)*; #[inline] unsafe fn set_archetype<'w>( - _fetch: &mut Self::Fetch<'w>, - _state: &Self::State, - _archetype: &'w Archetype, - _table: &'w Table + fetch: &mut Self::Fetch<'w>, + state: &Self::State, + archetype: &'w Archetype, + table: &'w Table ) { - let ($($name,)*) = _fetch; - let ($($state,)*) = _state; + let ($($name,)*) = fetch; + let ($($state,)*) = state; // SAFETY: The invariants are uphold by the caller. - $(unsafe { $name::set_archetype($name, $state, _archetype, _table); })* + $(unsafe { $name::set_archetype($name, $state, archetype, table); })* } #[inline] - unsafe fn set_table<'w>(_fetch: &mut Self::Fetch<'w>, _state: &Self::State, _table: &'w Table) { - let ($($name,)*) = _fetch; - let ($($state,)*) = _state; + unsafe fn set_table<'w>(fetch: &mut Self::Fetch<'w>, state: &Self::State, table: &'w Table) { + let ($($name,)*) = fetch; + let ($($state,)*) = state; // SAFETY: The invariants are uphold by the caller. - $(unsafe { $name::set_table($name, $state, _table); })* + $(unsafe { $name::set_table($name, $state, table); })* } #[inline(always)] - #[allow(clippy::unused_unit)] unsafe fn fetch<'w>( - _fetch: &mut Self::Fetch<'w>, - _entity: Entity, - _table_row: TableRow + fetch: &mut Self::Fetch<'w>, + entity: Entity, + table_row: TableRow ) -> Self::Item<'w> { - let ($($name,)*) = _fetch; + let ($($name,)*) = fetch; // SAFETY: The invariants are uphold by the caller. - ($(unsafe { $name::fetch($name, _entity, _table_row) },)*) + ($(unsafe { $name::fetch($name, entity, table_row) },)*) } - fn update_component_access(state: &Self::State, _access: &mut FilteredAccess) { + fn update_component_access(state: &Self::State, access: &mut FilteredAccess) { let ($($name,)*) = state; - $($name::update_component_access($name, _access);)* + $($name::update_component_access($name, access);)* } - #[allow(unused_variables)] fn init_state(world: &mut World) -> Self::State { ($($name::init_state(world),)*) } - #[allow(unused_variables)] fn get_state(components: &Components) -> Option { Some(($($name::get_state(components)?,)*)) } - fn matches_component_set(state: &Self::State, _set_contains_id: &impl Fn(ComponentId) -> bool) -> bool { + fn matches_component_set(state: &Self::State, set_contains_id: &impl Fn(ComponentId) -> bool) -> bool { let ($($name,)*) = state; - true $(&& $name::matches_component_set($name, _set_contains_id))* + true $(&& $name::matches_component_set($name, set_contains_id))* } } }; diff --git a/crates/bevy_ecs/src/schedule/config.rs b/crates/bevy_ecs/src/schedule/config.rs index 909aca8775ba8..20308578a4726 100644 --- a/crates/bevy_ecs/src/schedule/config.rs +++ b/crates/bevy_ecs/src/schedule/config.rs @@ -563,7 +563,14 @@ macro_rules! impl_system_collection { where $($sys: IntoSystemConfigs<$param>),* { - #[allow(non_snake_case)] + #[expect( + clippy::allow_attributes, + reason = "We are inside a macro, and as such, `non_snake_case` is not guaranteed to apply." + )] + #[allow( + non_snake_case, + reason = "Variable names are provided by the macro caller, not by us." + )] fn into_configs(self) -> SystemConfigs { let ($($sys,)*) = self; SystemConfigs::Configs { @@ -788,7 +795,14 @@ macro_rules! impl_system_set_collection { $(#[$meta])* impl<$($set: IntoSystemSetConfigs),*> IntoSystemSetConfigs for ($($set,)*) { - #[allow(non_snake_case)] + #[expect( + clippy::allow_attributes, + reason = "We are inside a macro, and as such, `non_snake_case` is not guaranteed to apply." + )] + #[allow( + non_snake_case, + reason = "Variable names are provided by the macro caller, not by us." + )] fn into_configs(self) -> SystemSetConfigs { let ($($set,)*) = self; SystemSetConfigs::Configs { diff --git a/crates/bevy_ecs/src/schedule/executor/mod.rs b/crates/bevy_ecs/src/schedule/executor/mod.rs index 79c31aac0e2e6..8b549a15ea6d3 100644 --- a/crates/bevy_ecs/src/schedule/executor/mod.rs +++ b/crates/bevy_ecs/src/schedule/executor/mod.rs @@ -122,7 +122,10 @@ impl SystemSchedule { since = "0.16.0", note = "Use `ApplyDeferred` instead. This was previously a function but is now a marker struct System." )] -#[expect(non_upper_case_globals)] +#[expect( + non_upper_case_globals, + reason = "This item is deprecated; as such, its previous name needs to stay." +)] pub const apply_deferred: ApplyDeferred = ApplyDeferred; /// A special [`System`] that instructs the executor to call diff --git a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs index 1185931fd64ac..a9eaea7311789 100644 --- a/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/multi_threaded.rs @@ -758,8 +758,10 @@ unsafe fn evaluate_and_fold_conditions( conditions: &mut [BoxedCondition], world: UnsafeWorldCell, ) -> bool { - // not short-circuiting is intentional - #[allow(clippy::unnecessary_fold)] + #[expect( + clippy::unnecessary_fold, + reason = "Short-circuiting here would prevent conditions from mutating their own state as needed." + )] conditions .iter_mut() .map(|condition| { diff --git a/crates/bevy_ecs/src/schedule/executor/simple.rs b/crates/bevy_ecs/src/schedule/executor/simple.rs index 122414f45e07b..81f7deab3a302 100644 --- a/crates/bevy_ecs/src/schedule/executor/simple.rs +++ b/crates/bevy_ecs/src/schedule/executor/simple.rs @@ -149,8 +149,10 @@ impl SimpleExecutor { } fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut World) -> bool { - // not short-circuiting is intentional - #[allow(clippy::unnecessary_fold)] + #[expect( + clippy::unnecessary_fold, + reason = "Short-circuiting here would prevent conditions from mutating their own state as needed." + )] conditions .iter_mut() .map(|condition| { diff --git a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs index b42b419e9fe37..8c5a7e0261bbe 100644 --- a/crates/bevy_ecs/src/schedule/executor/single_threaded.rs +++ b/crates/bevy_ecs/src/schedule/executor/single_threaded.rs @@ -191,8 +191,10 @@ impl SingleThreadedExecutor { } fn evaluate_and_fold_conditions(conditions: &mut [BoxedCondition], world: &mut World) -> bool { - // not short-circuiting is intentional - #[allow(clippy::unnecessary_fold)] + #[expect( + clippy::unnecessary_fold, + reason = "Short-circuiting here would prevent conditions from mutating their own state as needed." + )] conditions .iter_mut() .map(|condition| { diff --git a/crates/bevy_ecs/src/schedule/graph/mod.rs b/crates/bevy_ecs/src/schedule/graph/mod.rs index 1532184f1761e..eb55d7db0d86c 100644 --- a/crates/bevy_ecs/src/schedule/graph/mod.rs +++ b/crates/bevy_ecs/src/schedule/graph/mod.rs @@ -86,7 +86,7 @@ pub(crate) struct CheckGraphResults { pub(crate) transitive_reduction: DiGraph, /// Variant of the graph with all possible transitive edges. // TODO: this will very likely be used by "if-needed" ordering - #[allow(dead_code)] + #[expect(dead_code, reason = "See the TODO above this attribute.")] pub(crate) transitive_closure: DiGraph, } diff --git a/crates/bevy_ecs/src/schedule/mod.rs b/crates/bevy_ecs/src/schedule/mod.rs index 4a5b5a09912b6..59340f0d9228d 100644 --- a/crates/bevy_ecs/src/schedule/mod.rs +++ b/crates/bevy_ecs/src/schedule/mod.rs @@ -4,7 +4,6 @@ mod condition; mod config; mod executor; mod graph; -#[allow(clippy::module_inception)] mod schedule; mod set; mod stepping; diff --git a/crates/bevy_ecs/src/schedule/schedule.rs b/crates/bevy_ecs/src/schedule/schedule.rs index e406d0555ef11..33568ec09f5f5 100644 --- a/crates/bevy_ecs/src/schedule/schedule.rs +++ b/crates/bevy_ecs/src/schedule/schedule.rs @@ -1,3 +1,7 @@ +#![expect( + clippy::module_inception, + reason = "This instance of module inception is being discussed; see #17344." +)] use alloc::{ boxed::Box, collections::BTreeSet, @@ -108,8 +112,13 @@ impl Schedules { pub(crate) fn check_change_ticks(&mut self, change_tick: Tick) { #[cfg(feature = "trace")] let _all_span = info_span!("check stored schedule ticks").entered(); - // label used when trace feature is enabled - #[allow(unused_variables)] + #[cfg_attr( + not(feature = "trace"), + expect( + unused_variables, + reason = "The `label` variable goes unused if the `trace` feature isn't active" + ) + )] for (label, schedule) in &mut self.inner { #[cfg(feature = "trace")] let name = format!("{label:?}"); diff --git a/crates/bevy_ecs/src/schedule/stepping.rs b/crates/bevy_ecs/src/schedule/stepping.rs index 867fb5f9870eb..7855053de6fb7 100644 --- a/crates/bevy_ecs/src/schedule/stepping.rs +++ b/crates/bevy_ecs/src/schedule/stepping.rs @@ -414,7 +414,10 @@ impl Stepping { // transitions, and add debugging messages for permitted // transitions. Any action transition that falls through // this match block will be performed. - #[expect(clippy::match_same_arms)] + #[expect( + clippy::match_same_arms, + reason = "Readability would be negatively impacted by combining the `(Waiting, RunAll)` and `(Continue, RunAll)` match arms." + )] match (self.action, action) { // ignore non-transition updates, and prevent a call to // enable() from overwriting a step or continue call diff --git a/crates/bevy_ecs/src/system/builder.rs b/crates/bevy_ecs/src/system/builder.rs index 7b7df57482a20..89ec9f25189fc 100644 --- a/crates/bevy_ecs/src/system/builder.rs +++ b/crates/bevy_ecs/src/system/builder.rs @@ -300,14 +300,28 @@ unsafe impl< macro_rules! impl_system_param_builder_tuple { ($(#[$meta:meta])* $(($param: ident, $builder: ident)),*) => { + #[expect( + clippy::allow_attributes, + reason = "This is in a macro; as such, the below lints may not always apply." + )] + #[allow( + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] + #[allow( + non_snake_case, + reason = "The variable names are provided by the macro caller, not by us." + )] $(#[$meta])* // SAFETY: implementors of each `SystemParamBuilder` in the tuple have validated their impls unsafe impl<$($param: SystemParam,)* $($builder: SystemParamBuilder<$param>,)*> SystemParamBuilder<($($param,)*)> for ($($builder,)*) { - fn build(self, _world: &mut World, _meta: &mut SystemMeta) -> <($($param,)*) as SystemParam>::State { - #[allow(non_snake_case)] + fn build(self, world: &mut World, meta: &mut SystemMeta) -> <($($param,)*) as SystemParam>::State { let ($($builder,)*) = self; - #[allow(clippy::unused_unit)] - ($($builder.build(_world, _meta),)*) + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples won't generate any calls to the system parameter builders." + )] + ($($builder.build(world, meta),)*) } } }; @@ -407,10 +421,21 @@ pub struct ParamSetBuilder(pub T); macro_rules! impl_param_set_builder_tuple { ($(($param: ident, $builder: ident, $meta: ident)),*) => { + #[expect( + clippy::allow_attributes, + reason = "This is in a macro; as such, the below lints may not always apply." + )] + #[allow( + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] + #[allow( + non_snake_case, + reason = "The variable names are provided by the macro caller, not by us." + )] // SAFETY: implementors of each `SystemParamBuilder` in the tuple have validated their impls unsafe impl<'w, 's, $($param: SystemParam,)* $($builder: SystemParamBuilder<$param>,)*> SystemParamBuilder> for ParamSetBuilder<($($builder,)*)> { - #[allow(non_snake_case)] - fn build(self, _world: &mut World, _system_meta: &mut SystemMeta) -> <($($param,)*) as SystemParam>::State { + fn build(self, world: &mut World, system_meta: &mut SystemMeta) -> <($($param,)*) as SystemParam>::State { let ParamSetBuilder(($($builder,)*)) = self; // Note that this is slightly different from `init_state`, which calls `init_state` on each param twice. // One call populates an empty `SystemMeta` with the new access, while the other runs against a cloned `SystemMeta` to check for conflicts. @@ -418,22 +443,25 @@ macro_rules! impl_param_set_builder_tuple { // That means that any `filtered_accesses` in the `component_access_set` will get copied to every `$meta` // and will appear multiple times in the final `SystemMeta`. $( - let mut $meta = _system_meta.clone(); - let $param = $builder.build(_world, &mut $meta); + let mut $meta = system_meta.clone(); + let $param = $builder.build(world, &mut $meta); )* // Make the ParamSet non-send if any of its parameters are non-send. if false $(|| !$meta.is_send())* { - _system_meta.set_non_send(); + system_meta.set_non_send(); } $( - _system_meta + system_meta .component_access_set .extend($meta.component_access_set); - _system_meta + system_meta .archetype_component_access .extend(&$meta.archetype_component_access); )* - #[allow(clippy::unused_unit)] + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples won't generate any calls to the system parameter builders." + )] ($($param,)*) } } diff --git a/crates/bevy_ecs/src/system/combinator.rs b/crates/bevy_ecs/src/system/combinator.rs index 1f956c5d4c00e..f6e696a106a96 100644 --- a/crates/bevy_ecs/src/system/combinator.rs +++ b/crates/bevy_ecs/src/system/combinator.rs @@ -194,7 +194,7 @@ where // be called in parallel. Since mutable access to `world` only exists within // the scope of either closure, we can be sure they will never alias one another. |input| self.a.run(input, unsafe { world.world_mut() }), - #[allow(clippy::undocumented_unsafe_blocks)] + // SAFETY: See the above safety comment. |input| self.b.run(input, unsafe { world.world_mut() }), ) } diff --git a/crates/bevy_ecs/src/system/commands/mod.rs b/crates/bevy_ecs/src/system/commands/mod.rs index 95a8711520335..52f0417ee2ec1 100644 --- a/crates/bevy_ecs/src/system/commands/mod.rs +++ b/crates/bevy_ecs/src/system/commands/mod.rs @@ -2149,7 +2149,6 @@ impl<'a, T: Component> EntityEntryCommands<'a, T> { } #[cfg(test)] -#[allow(clippy::float_cmp, clippy::approx_constant)] mod tests { use crate::{ self as bevy_ecs, @@ -2163,7 +2162,10 @@ mod tests { sync::atomic::{AtomicUsize, Ordering}, }; - #[allow(dead_code)] + #[expect( + dead_code, + reason = "This struct is used to test how `Drop` behavior works in regards to SparseSet storage, and as such is solely a wrapper around `DropCk` to make it use the SparseSet storage. Because of this, the inner field is intentionally never read." + )] #[derive(Component)] #[component(storage = "SparseSet")] struct SparseDropCk(DropCk); diff --git a/crates/bevy_ecs/src/system/exclusive_function_system.rs b/crates/bevy_ecs/src/system/exclusive_function_system.rs index 81ac03bfb8502..2b1c081d51db0 100644 --- a/crates/bevy_ecs/src/system/exclusive_function_system.rs +++ b/crates/bevy_ecs/src/system/exclusive_function_system.rs @@ -219,7 +219,14 @@ pub struct HasExclusiveSystemInput; macro_rules! impl_exclusive_system_function { ($($param: ident),*) => { - #[allow(non_snake_case)] + #[expect( + clippy::allow_attributes, + reason = "This is within a macro, and as such, the below lints may not always apply." + )] + #[allow( + non_snake_case, + reason = "Certain variable names are provided by the caller, not by us." + )] impl ExclusiveSystemParamFunction Out> for Func where Func: Send + Sync + 'static, @@ -248,7 +255,14 @@ macro_rules! impl_exclusive_system_function { } } - #[allow(non_snake_case)] + #[expect( + clippy::allow_attributes, + reason = "This is within a macro, and as such, the below lints may not always apply." + )] + #[allow( + non_snake_case, + reason = "Certain variable names are provided by the caller, not by us." + )] impl ExclusiveSystemParamFunction<(HasExclusiveSystemInput, fn(In, $($param,)*) -> Out)> for Func where Func: Send + Sync + 'static, diff --git a/crates/bevy_ecs/src/system/exclusive_system_param.rs b/crates/bevy_ecs/src/system/exclusive_system_param.rs index 678d4452331b2..e36b7ea759bdd 100644 --- a/crates/bevy_ecs/src/system/exclusive_system_param.rs +++ b/crates/bevy_ecs/src/system/exclusive_system_param.rs @@ -88,26 +88,38 @@ impl ExclusiveSystemParam for PhantomData { macro_rules! impl_exclusive_system_param_tuple { ($(#[$meta:meta])* $($param: ident),*) => { - #[allow(unused_variables)] - #[allow(non_snake_case)] + #[expect( + clippy::allow_attributes, + reason = "This is within a macro, and as such, the below lints may not always apply." + )] + #[allow( + non_snake_case, + reason = "Certain variable names are provided by the caller, not by us." + )] + #[allow( + unused_variables, + reason = "Zero-length tuples won't use any of the parameters." + )] $(#[$meta])* impl<$($param: ExclusiveSystemParam),*> ExclusiveSystemParam for ($($param,)*) { type State = ($($param::State,)*); type Item<'s> = ($($param::Item<'s>,)*); #[inline] - fn init(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { - (($($param::init(_world, _system_meta),)*)) + fn init(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + (($($param::init(world, system_meta),)*)) } #[inline] - #[allow(clippy::unused_unit)] fn get_param<'s>( state: &'s mut Self::State, system_meta: &SystemMeta, ) -> Self::Item<'s> { - let ($($param,)*) = state; + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples won't have any params to get." + )] ($($param::get_param($param, system_meta),)*) } } diff --git a/crates/bevy_ecs/src/system/function_system.rs b/crates/bevy_ecs/src/system/function_system.rs index fdf26a98c948d..593d94e5a0917 100644 --- a/crates/bevy_ecs/src/system/function_system.rs +++ b/crates/bevy_ecs/src/system/function_system.rs @@ -1012,7 +1012,14 @@ pub struct HasSystemInput; macro_rules! impl_system_function { ($($param: ident),*) => { - #[allow(non_snake_case)] + #[expect( + clippy::allow_attributes, + reason = "This is within a macro, and as such, the below lints may not always apply." + )] + #[allow( + non_snake_case, + reason = "Certain variable names are provided by the caller, not by us." + )] impl SystemParamFunction Out> for Func where Func: Send + Sync + 'static, @@ -1040,7 +1047,14 @@ macro_rules! impl_system_function { } } - #[allow(non_snake_case)] + #[expect( + clippy::allow_attributes, + reason = "This is within a macro, and as such, the below lints may not always apply." + )] + #[allow( + non_snake_case, + reason = "Certain variable names are provided by the caller, not by us." + )] impl SystemParamFunction<(HasSystemInput, fn(In, $($param,)*) -> Out)> for Func where Func: Send + Sync + 'static, diff --git a/crates/bevy_ecs/src/system/input.rs b/crates/bevy_ecs/src/system/input.rs index 469403bc7345c..12087fdf6a64a 100644 --- a/crates/bevy_ecs/src/system/input.rs +++ b/crates/bevy_ecs/src/system/input.rs @@ -259,8 +259,18 @@ macro_rules! impl_system_input_tuple { type Param<'i> = ($($name::Param<'i>,)*); type Inner<'i> = ($($name::Inner<'i>,)*); - #[allow(non_snake_case)] - #[allow(clippy::unused_unit)] + #[expect( + clippy::allow_attributes, + reason = "This is in a macro; as such, the below lints may not always apply." + )] + #[allow( + non_snake_case, + reason = "Certain variable names are provided by the caller, not by us." + )] + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples won't have anything to wrap." + )] fn wrap(this: Self::Inner<'_>) -> Self::Param<'_> { let ($($name,)*) = this; ($($name::wrap($name),)*) diff --git a/crates/bevy_ecs/src/system/mod.rs b/crates/bevy_ecs/src/system/mod.rs index 75952e04227fa..360ed03c9b1b8 100644 --- a/crates/bevy_ecs/src/system/mod.rs +++ b/crates/bevy_ecs/src/system/mod.rs @@ -130,7 +130,6 @@ mod input; mod observer_system; mod query; mod schedule_system; -#[allow(clippy::module_inception)] mod system; mod system_name; mod system_param; @@ -421,8 +420,7 @@ mod tests { let entities_array: [Entity; ENTITIES_COUNT] = entities_array.0.clone().try_into().unwrap(); - #[allow(unused_mut)] - for (i, mut w) in (0..ENTITIES_COUNT).zip(q.get_many_mut(entities_array).unwrap()) { + for (i, w) in (0..ENTITIES_COUNT).zip(q.get_many_mut(entities_array).unwrap()) { assert_eq!(i, w.0); } @@ -898,13 +896,18 @@ mod tests { } #[test] + #[expect( + dead_code, + reason = "The `NotSend1` and `NotSend2` structs is used to verify that a system will run, even if the system params include a non-Send resource. As such, the inner value doesn't matter." + )] fn non_send_option_system() { let mut world = World::default(); world.insert_resource(SystemRan::No); - #[allow(dead_code)] + // Two structs are used, one which is inserted and one which is not, to verify that wrapping + // non-Send resources in an `Option` will allow the system to run regardless of their + // existence. struct NotSend1(alloc::rc::Rc); - #[allow(dead_code)] struct NotSend2(alloc::rc::Rc); world.insert_non_send_resource(NotSend1(alloc::rc::Rc::new(0))); @@ -923,13 +926,15 @@ mod tests { } #[test] + #[expect( + dead_code, + reason = "The `NotSend1` and `NotSend2` structs are used to verify that a system will run, even if the system params include a non-Send resource. As such, the inner value doesn't matter." + )] fn non_send_system() { let mut world = World::default(); world.insert_resource(SystemRan::No); - #[allow(dead_code)] struct NotSend1(alloc::rc::Rc); - #[allow(dead_code)] struct NotSend2(alloc::rc::Rc); world.insert_non_send_resource(NotSend1(alloc::rc::Rc::new(1))); @@ -1278,9 +1283,11 @@ mod tests { } } - /// this test exists to show that read-only world-only queries can return data that lives as long as 'world #[test] - #[allow(unused)] + #[expect( + dead_code, + reason = "This test exists to show that read-only world-only queries can return data that lives as long as `'world`." + )] fn long_life_test() { struct Holder<'w> { value: &'w A, diff --git a/crates/bevy_ecs/src/system/system.rs b/crates/bevy_ecs/src/system/system.rs index f976363aded11..4f22340bff8cc 100644 --- a/crates/bevy_ecs/src/system/system.rs +++ b/crates/bevy_ecs/src/system/system.rs @@ -1,3 +1,7 @@ +#![expect( + clippy::module_inception, + reason = "This instance of module inception is being discussed; see #17353." +)] use core::fmt::Debug; use log::warn; use thiserror::Error; @@ -400,7 +404,6 @@ mod tests { #[derive(Resource, Default, PartialEq, Debug)] struct Counter(u8); - #[allow(dead_code)] fn count_up(mut counter: ResMut) { counter.0 += 1; } @@ -416,7 +419,6 @@ mod tests { assert_eq!(*world.resource::(), Counter(2)); } - #[allow(dead_code)] fn spawn_entity(mut commands: Commands) { commands.spawn_empty(); } diff --git a/crates/bevy_ecs/src/system/system_param.rs b/crates/bevy_ecs/src/system/system_param.rs index 04292231297fd..8358bc66af827 100644 --- a/crates/bevy_ecs/src/system/system_param.rs +++ b/crates/bevy_ecs/src/system/system_param.rs @@ -201,7 +201,10 @@ pub unsafe trait SystemParam: Sized { /// # Safety /// `archetype` must be from the [`World`] used to initialize `state` in [`SystemParam::init_state`]. #[inline] - #[allow(unused_variables)] + #[expect( + unused_variables, + reason = "The parameters here are intentionally unused by the default implementation; however, putting underscores here will result in the underscores being copied by rust-analyzer's tab completion." + )] unsafe fn new_archetype( state: &mut Self::State, archetype: &Archetype, @@ -214,12 +217,18 @@ pub unsafe trait SystemParam: Sized { /// /// [`Commands`]: crate::prelude::Commands #[inline] - #[allow(unused_variables)] + #[expect( + unused_variables, + reason = "The parameters here are intentionally unused by the default implementation; however, putting underscores here will result in the underscores being copied by rust-analyzer's tab completion." + )] fn apply(state: &mut Self::State, system_meta: &SystemMeta, world: &mut World) {} /// Queues any deferred mutations to be applied at the next [`ApplyDeferred`](crate::prelude::ApplyDeferred). #[inline] - #[allow(unused_variables)] + #[expect( + unused_variables, + reason = "The parameters here are intentionally unused by the default implementation; however, putting underscores here will result in the underscores being copied by rust-analyzer's tab completion." + )] fn queue(state: &mut Self::State, system_meta: &SystemMeta, world: DeferredWorld) {} /// Validates that the param can be acquired by the [`get_param`](SystemParam::get_param). @@ -249,10 +258,14 @@ pub unsafe trait SystemParam: Sized { /// registered in [`init_state`](SystemParam::init_state). /// - `world` must be the same [`World`] that was used to initialize [`state`](SystemParam::init_state). /// - All `world`'s archetypes have been processed by [`new_archetype`](SystemParam::new_archetype). + #[expect( + unused_variables, + reason = "The parameters here are intentionally unused by the default implementation; however, putting underscores here will result in the underscores being copied by rust-analyzer's tab completion." + )] unsafe fn validate_param( - _state: &Self::State, - _system_meta: &SystemMeta, - _world: UnsafeWorldCell, + state: &Self::State, + system_meta: &SystemMeta, + world: UnsafeWorldCell, ) -> bool { // By default we allow panics in [`SystemParam::get_param`] and return `true`. // Preventing panics is an optional feature. @@ -691,8 +704,14 @@ macro_rules! impl_param_set { type State = ($($param::State,)*); type Item<'w, 's> = ParamSet<'w, 's, ($($param,)*)>; - // Note: We allow non snake case so the compiler don't complain about the creation of non_snake_case variables - #[allow(non_snake_case)] + #[expect( + clippy::allow_attributes, + reason = "This is inside a macro meant for tuples; as such, `non_snake_case` won't always lint." + )] + #[allow( + non_snake_case, + reason = "Certain variable names are provided by the caller, not by us." + )] fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { $( // Pretend to add each param to the system alone, see if it conflicts @@ -2009,56 +2028,76 @@ macro_rules! impl_system_param_tuple { // SAFETY: tuple consists only of ReadOnlySystemParams unsafe impl<$($param: ReadOnlySystemParam),*> ReadOnlySystemParam for ($($param,)*) {} - // SAFETY: implementors of each `SystemParam` in the tuple have validated their impls - #[allow(clippy::undocumented_unsafe_blocks)] // false positive by clippy - #[allow(non_snake_case)] + #[expect( + clippy::allow_attributes, + reason = "This is in a macro, and as such, the below lints may not always apply." + )] + #[allow( + non_snake_case, + reason = "Certain variable names are provided by the caller, not by us." + )] + #[allow( + unused_variables, + reason = "Zero-length tuples won't use some of the parameters." + )] $(#[$meta])* + // SAFETY: implementors of each `SystemParam` in the tuple have validated their impls unsafe impl<$($param: SystemParam),*> SystemParam for ($($param,)*) { type State = ($($param::State,)*); type Item<'w, 's> = ($($param::Item::<'w, 's>,)*); #[inline] - fn init_state(_world: &mut World, _system_meta: &mut SystemMeta) -> Self::State { - (($($param::init_state(_world, _system_meta),)*)) + fn init_state(world: &mut World, system_meta: &mut SystemMeta) -> Self::State { + (($($param::init_state(world, system_meta),)*)) } #[inline] - #[allow(unused_unsafe)] - unsafe fn new_archetype(($($param,)*): &mut Self::State, _archetype: &Archetype, _system_meta: &mut SystemMeta) { + unsafe fn new_archetype(($($param,)*): &mut Self::State, archetype: &Archetype, system_meta: &mut SystemMeta) { + #[allow( + unused_unsafe, + reason = "Zero-length tuples will not run anything in the unsafe block." + )] // SAFETY: The caller ensures that `archetype` is from the World the state was initialized from in `init_state`. - unsafe { $($param::new_archetype($param, _archetype, _system_meta);)* } + unsafe { $($param::new_archetype($param, archetype, system_meta);)* } } #[inline] - fn apply(($($param,)*): &mut Self::State, _system_meta: &SystemMeta, _world: &mut World) { - $($param::apply($param, _system_meta, _world);)* + fn apply(($($param,)*): &mut Self::State, system_meta: &SystemMeta, world: &mut World) { + $($param::apply($param, system_meta, world);)* } #[inline] - fn queue(($($param,)*): &mut Self::State, _system_meta: &SystemMeta, mut _world: DeferredWorld) { - $($param::queue($param, _system_meta, _world.reborrow());)* + #[allow( + unused_mut, + reason = "The `world` parameter is unused for zero-length tuples; however, it must be mutable for other lengths of tuples." + )] + fn queue(($($param,)*): &mut Self::State, system_meta: &SystemMeta, mut world: DeferredWorld) { + $($param::queue($param, system_meta, world.reborrow());)* } #[inline] unsafe fn validate_param( state: &Self::State, - _system_meta: &SystemMeta, - _world: UnsafeWorldCell, + system_meta: &SystemMeta, + world: UnsafeWorldCell, ) -> bool { let ($($param,)*) = state; - $($param::validate_param($param, _system_meta, _world)&&)* true + $($param::validate_param($param, system_meta, world)&&)* true } #[inline] - #[allow(clippy::unused_unit)] unsafe fn get_param<'w, 's>( state: &'s mut Self::State, - _system_meta: &SystemMeta, - _world: UnsafeWorldCell<'w>, - _change_tick: Tick, + system_meta: &SystemMeta, + world: UnsafeWorldCell<'w>, + change_tick: Tick, ) -> Self::Item<'w, 's> { let ($($param,)*) = state; - ($($param::get_param($param, _system_meta, _world, _change_tick),)*) + #[allow( + clippy::unused_unit, + reason = "Zero-length tuples won't have any params to get." + )] + ($($param::get_param($param, system_meta, world, change_tick),)*) } } }; @@ -2647,7 +2686,10 @@ mod tests { // Compile test for https://github.com/bevyengine/bevy/pull/7001. #[test] fn system_param_const_generics() { - #[allow(dead_code)] + #[expect( + dead_code, + reason = "This struct is used to ensure that const generics are supported as a SystemParam; thus, the inner value never needs to be read." + )] #[derive(SystemParam)] pub struct ConstGenericParam<'w, const I: usize>(Res<'w, R>); @@ -2705,7 +2747,10 @@ mod tests { #[derive(SystemParam)] pub struct UnitParam; - #[allow(dead_code)] + #[expect( + dead_code, + reason = "This struct is used to ensure that tuple structs are supported as a SystemParam; thus, the inner values never need to be read." + )] #[derive(SystemParam)] pub struct TupleParam<'w, 's, R: Resource, L: FromWorld + Send + 'static>( Res<'w, R>, @@ -2722,7 +2767,10 @@ mod tests { #[derive(Resource)] struct PrivateResource; - #[allow(dead_code)] + #[expect( + dead_code, + reason = "This struct is used to ensure that SystemParam's derive can't leak private fields; thus, the inner values never need to be read." + )] #[derive(SystemParam)] pub struct EncapsulatedParam<'w>(Res<'w, PrivateResource>); diff --git a/crates/bevy_ecs/src/world/command_queue.rs b/crates/bevy_ecs/src/world/command_queue.rs index 76b9b6eddb9ec..5b78b7a974e3f 100644 --- a/crates/bevy_ecs/src/world/command_queue.rs +++ b/crates/bevy_ecs/src/world/command_queue.rs @@ -431,10 +431,10 @@ mod test { assert_eq!(world.entities().len(), 2); } - // This has an arbitrary value `String` stored to ensure - // when then command gets pushed, the `bytes` vector gets - // some data added to it. - #[allow(dead_code)] + #[expect( + dead_code, + reason = "The inner string is used to ensure that, when the PanicCommand gets pushed to the queue, some data is written to the `bytes` vector." + )] struct PanicCommand(String); impl Command for PanicCommand { fn apply(self, _: &mut World) { @@ -510,7 +510,10 @@ mod test { assert_is_send(SpawnCommand); } - #[allow(dead_code)] + #[expect( + dead_code, + reason = "This struct is used to test how the CommandQueue reacts to padding added by rust's compiler." + )] struct CommandWithPadding(u8, u16); impl Command for CommandWithPadding { fn apply(self, _: &mut World) {} diff --git a/crates/bevy_ecs/src/world/entity_ref.rs b/crates/bevy_ecs/src/world/entity_ref.rs index 6b3491e877457..ddbfa81e627a6 100644 --- a/crates/bevy_ecs/src/world/entity_ref.rs +++ b/crates/bevy_ecs/src/world/entity_ref.rs @@ -388,12 +388,11 @@ impl PartialEq for EntityRef<'_> { impl Eq for EntityRef<'_> {} -#[expect(clippy::non_canonical_partial_ord_impl)] impl PartialOrd for EntityRef<'_> { /// [`EntityRef`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { - self.entity().partial_cmp(&other.entity()) + Some(self.cmp(other)) } } @@ -944,12 +943,11 @@ impl PartialEq for EntityMut<'_> { impl Eq for EntityMut<'_> {} -#[expect(clippy::non_canonical_partial_ord_impl)] impl PartialOrd for EntityMut<'_> { /// [`EntityMut`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { - self.entity().partial_cmp(&other.entity()) + Some(self.cmp(other)) } } @@ -1758,7 +1756,10 @@ impl<'w> EntityWorldMut<'w> { }) }; - #[allow(clippy::undocumented_unsafe_blocks)] // TODO: document why this is safe + #[expect( + clippy::undocumented_unsafe_blocks, + reason = "Needs to be documented; see #17345." + )] unsafe { Self::move_entity_from_remove::( entity, @@ -3178,12 +3179,11 @@ impl PartialEq for FilteredEntityRef<'_> { impl Eq for FilteredEntityRef<'_> {} -#[expect(clippy::non_canonical_partial_ord_impl)] impl PartialOrd for FilteredEntityRef<'_> { /// [`FilteredEntityRef`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { - self.entity().partial_cmp(&other.entity()) + Some(self.cmp(other)) } } @@ -3505,12 +3505,11 @@ impl PartialEq for FilteredEntityMut<'_> { impl Eq for FilteredEntityMut<'_> {} -#[expect(clippy::non_canonical_partial_ord_impl)] impl PartialOrd for FilteredEntityMut<'_> { /// [`FilteredEntityMut`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { - self.entity().partial_cmp(&other.entity()) + Some(self.cmp(other)) } } @@ -3653,12 +3652,11 @@ impl PartialEq for EntityRefExcept<'_, B> { impl Eq for EntityRefExcept<'_, B> {} -#[expect(clippy::non_canonical_partial_ord_impl)] impl PartialOrd for EntityRefExcept<'_, B> { /// [`EntityRefExcept`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { - self.entity().partial_cmp(&other.entity()) + Some(self.cmp(other)) } } @@ -3793,12 +3791,11 @@ impl PartialEq for EntityMutExcept<'_, B> { impl Eq for EntityMutExcept<'_, B> {} -#[expect(clippy::non_canonical_partial_ord_impl)] impl PartialOrd for EntityMutExcept<'_, B> { /// [`EntityMutExcept`]'s comparison trait implementations match the underlying [`Entity`], /// and cannot discern between different worlds. fn partial_cmp(&self, other: &Self) -> Option { - self.entity().partial_cmp(&other.entity()) + Some(self.cmp(other)) } } diff --git a/crates/bevy_ecs/src/world/unsafe_world_cell.rs b/crates/bevy_ecs/src/world/unsafe_world_cell.rs index bf8c4a8c6b235..94d36a3dc17fd 100644 --- a/crates/bevy_ecs/src/world/unsafe_world_cell.rs +++ b/crates/bevy_ecs/src/world/unsafe_world_cell.rs @@ -1,7 +1,5 @@ //! Contains types that allow disjoint mutable access to a [`World`]. -#![warn(unsafe_op_in_unsafe_fn)] - use super::{Mut, Ref, World, WorldId}; use crate::{ archetype::{Archetype, Archetypes}, @@ -1095,7 +1093,6 @@ impl<'w> UnsafeWorldCell<'w> { /// - `storage_type` must accurately reflect where the components for `component_id` are stored. /// - the caller must ensure that no aliasing rules are violated #[inline] -#[allow(unsafe_op_in_unsafe_fn)] unsafe fn get_component( world: UnsafeWorldCell<'_>, component_id: ComponentId, @@ -1122,7 +1119,6 @@ unsafe fn get_component( /// - `storage_type` must accurately reflect where the components for `component_id` are stored. /// - the caller must ensure that no aliasing rules are violated #[inline] -#[allow(unsafe_op_in_unsafe_fn)] unsafe fn get_component_and_ticks( world: UnsafeWorldCell<'_>, component_id: ComponentId, @@ -1166,7 +1162,6 @@ unsafe fn get_component_and_ticks( /// - `storage_type` must accurately reflect where the components for `component_id` are stored. /// - the caller must ensure that no aliasing rules are violated #[inline] -#[allow(unsafe_op_in_unsafe_fn)] unsafe fn get_ticks( world: UnsafeWorldCell<'_>, component_id: ComponentId,