bevy_reflect: Apply #[deny(clippy::allow_attributes, clippy::allow_attributes_without_reason)] (#17092)

# Objective
We want to deny the following lints:
* `clippy::allow_attributes` - Because there's no reason to
`#[allow(...)]` an attribute if it wouldn't lint against anything; you
should always use `#[expect(...)]`
* `clippy::allow_attributes_without_reason` - Because documenting the
reason for allowing/expecting a lint is always good

## Solution
Set the `clippy::allow_attributes` and
`clippy::allow_attributes_without_reason` lints to `deny`, and bring
`bevy_reflect` in line with the new restrictions.

No code changes have been made - except if a lint that was previously
`allow(...)`'d could be removed via small code changes. For example,
`unused_variables` can be handled by adding a `_` to the beginning of a
field's name.

## Testing
I ran `cargo clippy`, and received no errors.
This commit is contained in:
MichiRecRoom 2025-01-03 17:22:34 -05:00 committed by GitHub
parent 66a0e74a21
commit 120b733ab5
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 57 additions and 25 deletions

View File

@ -152,7 +152,6 @@ macro_rules! impl_custom_attribute_methods {
$self.custom_attributes().get::<T>()
}
#[allow(rustdoc::redundant_explicit_links)]
/// Gets a custom attribute by its [`TypeId`](core::any::TypeId).
///
/// This is the dynamic equivalent of [`get_attribute`](Self::get_attribute).

View File

@ -112,7 +112,6 @@ impl ReflectFromReflect {
///
/// This will convert the object to a concrete type if it wasn't already, and return
/// the value as `Box<dyn Reflect>`.
#[allow(clippy::wrong_self_convention)]
pub fn from_reflect(&self, reflect_value: &dyn PartialReflect) -> Option<Box<dyn Reflect>> {
(self.from_reflect)(reflect_value)
}

View File

@ -612,7 +612,6 @@ macro_rules! impl_typed_function {
FunctionInfo::new(
create_info::<Function>()
.with_args({
#[allow(unused_mut)]
let mut _index = 0;
vec![
$(ArgInfo::new::<$Arg>({
@ -638,7 +637,6 @@ macro_rules! impl_typed_function {
FunctionInfo::new(
create_info::<Function>()
.with_args({
#[allow(unused_mut)]
let mut _index = 1;
vec![
ArgInfo::new::<&Receiver>(0),
@ -665,7 +663,6 @@ macro_rules! impl_typed_function {
FunctionInfo::new(
create_info::<Function>()
.with_args({
#[allow(unused_mut)]
let mut _index = 1;
vec![
ArgInfo::new::<&mut Receiver>(0),
@ -692,7 +689,6 @@ macro_rules! impl_typed_function {
FunctionInfo::new(
create_info::<Function>()
.with_args({
#[allow(unused_mut)]
let mut _index = 1;
vec![
ArgInfo::new::<&mut Receiver>(0),

View File

@ -88,7 +88,14 @@ macro_rules! impl_reflect_fn {
// This clause essentially asserts that `Arg::This` is the same type as `Arg`
Function: for<'a> Fn($($Arg::This<'a>),*) -> ReturnType + 'env,
{
#[allow(unused_mut)]
#[expect(
clippy::allow_attributes,
reason = "This lint is part of a macro, which may not always trigger the `unused_mut` lint."
)]
#[allow(
unused_mut,
reason = "Some invocations of this macro may trigger the `unused_mut` lint, where others won't."
)]
fn reflect_call<'a>(&self, mut args: ArgList<'a>) -> FunctionResult<'a> {
const COUNT: usize = count_tokens!($($Arg)*);

View File

@ -95,7 +95,14 @@ macro_rules! impl_reflect_fn_mut {
// This clause essentially asserts that `Arg::This` is the same type as `Arg`
Function: for<'a> FnMut($($Arg::This<'a>),*) -> ReturnType + 'env,
{
#[allow(unused_mut)]
#[expect(
clippy::allow_attributes,
reason = "This lint is part of a macro, which may not always trigger the `unused_mut` lint."
)]
#[allow(
unused_mut,
reason = "Some invocations of this macro may trigger the `unused_mut` lint, where others won't."
)]
fn reflect_call_mut<'a>(&mut self, mut args: ArgList<'a>) -> FunctionResult<'a> {
const COUNT: usize = count_tokens!($($Arg)*);

View File

@ -10,7 +10,10 @@ macro_rules! reflect_enum {
impl_reflect!($(#[$meta])* enum $ident { $($ty)* });
#[assert_type_match($ident, test_only)]
#[allow(clippy::upper_case_acronyms)]
#[expect(
clippy::upper_case_acronyms,
reason = "The variants used are not acronyms."
)]
enum $ident { $($ty)* }
};
}

View File

@ -1,5 +1,7 @@
// Temporary workaround for impl_reflect!(Option/Result false-positive
#![allow(unused_qualifications)]
#![expect(
unused_qualifications,
reason = "Temporary workaround for impl_reflect!(Option/Result false-positive"
)]
use crate::{
self as bevy_reflect, impl_type_path, map_apply, map_partial_eq, map_try_apply,
@ -236,7 +238,6 @@ macro_rules! impl_reflect_for_atomic {
#[cfg(feature = "functions")]
crate::func::macros::impl_function_traits!($ty);
#[allow(unused_mut)]
impl GetTypeRegistration for $ty
where
$ty: Any + Send + Sync,

View File

@ -1,4 +1,9 @@
#![expect(missing_docs, reason = "Not all docs are written yet, see #3492.")]
#![deny(
clippy::allow_attributes,
clippy::allow_attributes_without_reason,
reason = "See #17111; To be removed once all crates are in-line with these attributes"
)]
#![cfg_attr(
any(docsrs, docsrs_dep),
expect(
@ -686,8 +691,7 @@ pub mod __macro_exports {
note = "consider annotating `{Self}` with `#[derive(Reflect)]`"
)]
pub trait RegisterForReflection {
#[allow(unused_variables)]
fn __register(registry: &mut TypeRegistry) {}
fn __register(_registry: &mut TypeRegistry) {}
}
impl<T: GetTypeRegistration> RegisterForReflection for T {
@ -712,7 +716,10 @@ pub mod __macro_exports {
}
#[cfg(test)]
#[allow(clippy::disallowed_types, clippy::approx_constant)]
#[expect(
clippy::approx_constant,
reason = "We don't need the exact value of Pi here."
)]
mod tests {
use ::serde::{de::DeserializeSeed, Deserialize, Serialize};
use alloc::{
@ -876,7 +883,6 @@ mod tests {
}
#[test]
#[allow(clippy::disallowed_types)]
fn reflect_unit_struct() {
#[derive(Reflect)]
struct Foo(u32, u64);
@ -2148,7 +2154,7 @@ mod tests {
enum_struct: SomeEnum,
custom: CustomDebug,
#[reflect(ignore)]
#[allow(dead_code)]
#[expect(dead_code, reason = "This value is intended to not be reflected.")]
ignored: isize,
}

View File

@ -502,7 +502,10 @@ impl core::ops::IndexMut<usize> for ParsedPath {
}
#[cfg(test)]
#[allow(clippy::float_cmp, clippy::approx_constant)]
#[expect(
clippy::approx_constant,
reason = "We don't need the exact value of Pi here."
)]
mod tests {
use super::*;
use crate as bevy_reflect;

View File

@ -64,7 +64,10 @@ impl<'a> PathParser<'a> {
// the last byte before an ASCII utf-8 character (ie: it is a char
// boundary).
// - The slice always starts after a symbol ie: an ASCII character's boundary.
#[allow(unsafe_code)]
#[expect(
unsafe_code,
reason = "We have fulfilled the Safety requirements for `from_utf8_unchecked`."
)]
let ident = unsafe { from_utf8_unchecked(ident) };
self.remaining = remaining;

View File

@ -199,7 +199,7 @@ mod tests {
#[reflect_trait]
trait Enemy: Reflect + Debug {
#[allow(dead_code, reason = "this method is purely for testing purposes")]
#[expect(dead_code, reason = "this method is purely for testing purposes")]
fn hp(&self) -> u8;
}

View File

@ -79,8 +79,7 @@ pub trait GetTypeRegistration: 'static {
///
/// This method is called by [`TypeRegistry::register`] to register any other required types.
/// Often, this is done for fields of structs and enum variants to ensure all types are properly registered.
#[allow(unused_variables)]
fn register_type_dependencies(registry: &mut TypeRegistry) {}
fn register_type_dependencies(_registry: &mut TypeRegistry) {}
}
impl Default for TypeRegistry {
@ -785,7 +784,10 @@ pub struct ReflectFromPtr {
from_ptr_mut: unsafe fn(PtrMut) -> &mut dyn Reflect,
}
#[allow(unsafe_code)]
#[expect(
unsafe_code,
reason = "We must interact with pointers here, which are inherently unsafe."
)]
impl ReflectFromPtr {
/// Returns the [`TypeId`] that the [`ReflectFromPtr`] was constructed for.
pub fn type_id(&self) -> TypeId {
@ -837,7 +839,10 @@ impl ReflectFromPtr {
}
}
#[allow(unsafe_code)]
#[expect(
unsafe_code,
reason = "We must interact with pointers here, which are inherently unsafe."
)]
impl<T: Reflect> FromType<T> for ReflectFromPtr {
fn from_type() -> Self {
ReflectFromPtr {
@ -857,7 +862,10 @@ impl<T: Reflect> FromType<T> for ReflectFromPtr {
}
#[cfg(test)]
#[allow(unsafe_code)]
#[expect(
unsafe_code,
reason = "We must interact with pointers here, which are inherently unsafe."
)]
mod test {
use super::*;
use crate as bevy_reflect;