From e65fb0905a55c805351b37de8ddef3d6cef0fafd Mon Sep 17 00:00:00 2001 From: Uli Schlachter Date: Sun, 17 Mar 2024 11:33:02 +0100 Subject: [PATCH] Try to clean up some lints and enable more I came across the unused_extern_crates lint, which is allow-by-default. It is however part of the rust_2018_idioms lint group, which we are already denying. Anyway, this discovery led me down a linting rabbit hole. There is the unused_import_braces lint which arguable lints against something that rustfmt already deals with. Both the lint and rustfmt deal with the braces in "use test::{A};". Thus, this lint can just be dropped. The "unused" lint group includes "unused_must_use" and some other things that sound helpful, so let's just switch to the more generic lint. This found some unused_macro_rules in x11rb/src/tracing.rs, which need to be allowed, so this lint can just be denied and not forbidden. But some of the lints that we forbid are part of the "unused" group and #[deny]ing something that was previously #[forbid]en is an error. Thus, I am switching the order of these and move all the #[deny]s up. Next, I thought that "unused_qualifications" should be part of "unused", but it actually is not. The following code warns about the allows for unused_extern_crates and unused_must_use, but not unused_qualifications, so this is not part of the unused group. Same for unused_results. #![forbid(unused)] #![allow(unused_extern_crates)] #![allow(unused_must_use)] #![allow(unused_qualifications)] #![allow(unused_results)] Signed-off-by: Uli Schlachter --- extract-generated-code-doc/src/main.rs | 3 +-- generator/src/main.rs | 3 +-- x11rb-async/src/lib.rs | 20 +++++++++++--------- x11rb-protocol/src/lib.rs | 24 +++++++++++++----------- x11rb/src/lib.rs | 20 +++++++++++--------- x11rb/src/tracing.rs | 2 ++ xcbgen-rs/src/lib.rs | 3 +-- xtrace-example/src/main.rs | 14 ++++++++------ 8 files changed, 48 insertions(+), 41 deletions(-) diff --git a/extract-generated-code-doc/src/main.rs b/extract-generated-code-doc/src/main.rs index 75cd3a0a..a26371a6 100644 --- a/extract-generated-code-doc/src/main.rs +++ b/extract-generated-code-doc/src/main.rs @@ -3,8 +3,7 @@ trivial_numeric_casts, unsafe_code, unreachable_pub, - unused_import_braces, - unused_must_use, + unused, unused_qualifications )] #![forbid(unsafe_code)] diff --git a/generator/src/main.rs b/generator/src/main.rs index 73f3e770..b4873078 100644 --- a/generator/src/main.rs +++ b/generator/src/main.rs @@ -3,8 +3,7 @@ trivial_numeric_casts, unsafe_code, unreachable_pub, - unused_import_braces, - unused_must_use, + unused, unused_qualifications )] #![forbid(unsafe_code)] diff --git a/x11rb-async/src/lib.rs b/x11rb-async/src/lib.rs index d5d10d2f..57594a4e 100644 --- a/x11rb-async/src/lib.rs +++ b/x11rb-async/src/lib.rs @@ -41,6 +41,17 @@ //! * `extra-traits`: Implement extra traits for X11 types. This improves the output of the `Debug` //! impl and adds `PartialEq`, `Eq`, `PartialOrd`, `Ord`, and `Hash` where possible. +// A list of lints that are only #![deny] and not the stronger #![forbid]. Each one has a comment +// explaining why it gets the weaker treatment. +#![deny( + // Contains unreachable_code and "?" generates an #[allow] for this + unused, + // #[derive] generates an #[allow] for this; not part of "unused" + unused_qualifications, + // Not everything in x11rb::protocol has doc comments + missing_docs, +)] + #![forbid( missing_copy_implementations, missing_debug_implementations, @@ -50,20 +61,11 @@ trivial_casts, trivial_numeric_casts, unreachable_pub, - unused_import_braces, unused_must_use, unused_results, clippy::cast_lossless, clippy::needless_pass_by_value, )] -// A list of lints that are only #![deny] and not the stronger #![forbid]. Each one has a comment -// explaining why it gets the weaker treatment. -#![deny( - // #[derive] generates an #[allow] for this - unused_qualifications, - // Not everything in x11rb::protocol has doc comments - missing_docs, -)] #![cfg_attr(not(feature = "allow-unsafe-code"), forbid(unsafe_code))] // -- Public Modules -- diff --git a/x11rb-protocol/src/lib.rs b/x11rb-protocol/src/lib.rs index de3fa10b..76fe1600 100644 --- a/x11rb-protocol/src/lib.rs +++ b/x11rb-protocol/src/lib.rs @@ -40,6 +40,19 @@ //! * `extra-traits`: Implement extra traits for types. This improves the output of the `Debug` //! impl and adds `PartialEq`, `Eq`, `PartialOrd`, `Ord`, and `Hash` where possible. +// A list of lints that are only #![deny] and not the stronger #![forbid]. Each one has a comment +// explaining why it gets the weaker treatment. +#![deny( + // Contains unreachable_code and "?" generates an #[allow] for this + unused, + // #[derive] generates an #[allow] for this; not part of "unused" + unused_qualifications, + // serde's Deserialize/Serialize impls add allows for this + rust_2018_idioms, + // Not everything in x11rb_protocol::protocol has doc comments + missing_docs, +)] + #![forbid( missing_copy_implementations, missing_debug_implementations, @@ -49,22 +62,11 @@ trivial_numeric_casts, unreachable_pub, unsafe_code, - unused_import_braces, unused_must_use, unused_results, clippy::cast_lossless, clippy::needless_pass_by_value, )] -// A list of lints that are only #![deny] and not the stronger #![forbid]. Each one has a comment -// explaining why it gets the weaker treatment. -#![deny( - // #[derive] generates an #[allow] for this - unused_qualifications, - // serde's Deserialize/Serialize impls add allows for this - rust_2018_idioms, - // Not everything in x11rb_protocol::protocol has doc comments - missing_docs, -)] #![no_std] // std crate imports diff --git a/x11rb/src/lib.rs b/x11rb/src/lib.rs index 39785f69..4b823a6e 100644 --- a/x11rb/src/lib.rs +++ b/x11rb/src/lib.rs @@ -120,6 +120,17 @@ //! The [event_loop_integration](event_loop_integration/index.html) module contains some hints for //! integrating x11rb with an event loop as doc comments. +// A list of lints that are only #![deny] and not the stronger #![forbid]. Each one has a comment +// explaining why it gets the weaker treatment. +#![deny( + // Contains unreachable_code and "?" generates an #[allow] for this + unused, + // #[derive] generates an #[allow] for this; not part of "unused" + unused_qualifications, + // Not everything in x11rb::protocol has doc comments + missing_docs, +)] + #![forbid( missing_copy_implementations, missing_debug_implementations, @@ -129,20 +140,11 @@ trivial_casts, trivial_numeric_casts, unreachable_pub, - unused_import_braces, unused_must_use, unused_results, clippy::cast_lossless, clippy::needless_pass_by_value, )] -// A list of lints that are only #![deny] and not the stronger #![forbid]. Each one has a comment -// explaining why it gets the weaker treatment. -#![deny( - // #[derive] generates an #[allow] for this - unused_qualifications, - // Not everything in x11rb::protocol has doc comments - missing_docs, -)] #![cfg_attr(not(feature = "allow-unsafe-code"), forbid(unsafe_code))] // Only contains documentation, but no "actual rust" diff --git a/x11rb/src/tracing.rs b/x11rb/src/tracing.rs index da176ec9..415bd64f 100644 --- a/x11rb/src/tracing.rs +++ b/x11rb/src/tracing.rs @@ -102,6 +102,7 @@ macro_rules! warning_span { ( $name:expr ) => { $crate::tracing::implementation::span!($crate::tracing::Level::Warn, $name, ) }; ( $name:expr, $($fields:tt)* ) => { $crate::tracing::implementation::span!($crate::tracing::Level::Warn, $name, $($fields)*) }; } +#[allow(unused_macro_rules)] macro_rules! info_span { ( $name:expr ) => { $crate::tracing::implementation::span!($crate::tracing::Level::Info, $name, ) }; ( $name:expr, $($fields:tt)* ) => { $crate::tracing::implementation::span!($crate::tracing::Level::Info, $name, $($fields)*) }; @@ -110,6 +111,7 @@ macro_rules! debug_span { ( $name:expr ) => { $crate::tracing::implementation::span!($crate::tracing::Level::Debug, $name, ) }; ( $name:expr, $($fields:tt)* ) => { $crate::tracing::implementation::span!($crate::tracing::Level::Debug, $name, $($fields)*) }; } +#[allow(unused_macro_rules)] macro_rules! trace_span { ( $name:expr ) => { $crate::tracing::implementation::span!($crate::tracing::Level::Trace, $name, ) }; ( $name:expr, $($fields:tt)* ) => { $crate::tracing::implementation::span!($crate::tracing::Level::Trace, $name, $($fields)*) }; diff --git a/xcbgen-rs/src/lib.rs b/xcbgen-rs/src/lib.rs index 51dcaf50..d942308e 100644 --- a/xcbgen-rs/src/lib.rs +++ b/xcbgen-rs/src/lib.rs @@ -11,8 +11,7 @@ trivial_numeric_casts, unsafe_code, unreachable_pub, - unused_import_braces, - unused_must_use, + unused, unused_qualifications, missing_copy_implementations, missing_debug_implementations, diff --git a/xtrace-example/src/main.rs b/xtrace-example/src/main.rs index e4459b94..86e69cba 100644 --- a/xtrace-example/src/main.rs +++ b/xtrace-example/src/main.rs @@ -36,17 +36,19 @@ //! The code in `connection_inner` then handles decoding X11 packets, updating the state of the //! connection, and generating output via `println!`. +#![deny( + // Contains unreachable_code and "?" generates an #[allow] for this + unused, + // #[derive] generates an #[allow] for this; not part of "unused" + unused_qualifications, +)] + #![forbid( missing_docs, rust_2018_idioms, trivial_casts, trivial_numeric_casts, - unused_import_braces, - unused_results -)] -#![deny( - // #[derive] generates an #[allow] for this - unused_qualifications, + unused_results, )] use smol::Async;