diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | clippy_lints/src/attrs.rs | 97 | ||||
| -rw-r--r-- | clippy_lints/src/declared_lints.rs | 1 | ||||
| -rw-r--r-- | tests/ui/unnecessary_clippy_cfg.rs | 23 | ||||
| -rw-r--r-- | tests/ui/unnecessary_clippy_cfg.stderr | 61 |
5 files changed, 182 insertions, 1 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index b35475c7340..c2f47ad9720 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5726,6 +5726,7 @@ Released 2018-09-13 [`unknown_clippy_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints [`unnecessary_box_returns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns [`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast +[`unnecessary_clippy_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_clippy_cfg [`unnecessary_fallible_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions [`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map [`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 755f4ad3c89..f90a7dbbfe5 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -1,7 +1,9 @@ //! checks for attributes use clippy_config::msrvs::{self, Msrv}; -use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; +use clippy_utils::diagnostics::{ + span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then, +}; use clippy_utils::is_from_proc_macro; use clippy_utils::macros::{is_panic, macro_backtrace}; use clippy_utils::source::{first_line_of_span, is_present_in_source, snippet_opt, without_block_comments}; @@ -459,6 +461,30 @@ declare_clippy_lint! { "usage of `cfg(feature = \"cargo-clippy\")` instead of `cfg(clippy)`" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `#[cfg_attr(clippy, allow(clippy::lint))]` + /// and suggests to replace it with `#[allow(clippy::lint)]`. + /// + /// ### Why is this bad? + /// There is no reason to put clippy attributes behind a clippy `cfg` as they are not + /// run by anything else than clippy. + /// + /// ### Example + /// ```no_run + /// #![cfg_attr(clippy, allow(clippy::deprecated_cfg_attr))] + /// ``` + /// + /// Use instead: + /// ```no_run + /// #![allow(clippy::deprecated_cfg_attr)] + /// ``` + #[clippy::version = "1.78.0"] + pub UNNECESSARY_CLIPPY_CFG, + suspicious, + "usage of `cfg_attr(clippy, allow(clippy::lint))` instead of `allow(clippy::lint)`" +} + declare_lint_pass!(Attributes => [ ALLOW_ATTRIBUTES_WITHOUT_REASON, INLINE_ALWAYS, @@ -821,6 +847,7 @@ impl_lint_pass!(EarlyAttributes => [ NON_MINIMAL_CFG, MAYBE_MISUSED_CFG, DEPRECATED_CLIPPY_CFG_ATTR, + UNNECESSARY_CLIPPY_CFG, ]); impl EarlyLintPass for EarlyAttributes { @@ -958,6 +985,74 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msr ); } else { check_deprecated_cfg_recursively(cx, feature_item); + if let Some(behind_cfg_attr) = items[1].meta_item() { + check_clippy_cfg_attr(cx, feature_item, behind_cfg_attr, attr); + } + } + } +} + +fn check_clippy_cfg_attr( + cx: &EarlyContext<'_>, + cfg_attr: &rustc_ast::MetaItem, + behind_cfg_attr: &rustc_ast::MetaItem, + attr: &Attribute, +) { + if cfg_attr.has_name(sym::clippy) + && let Some(ident) = behind_cfg_attr.ident() + // FIXME: replace with `from_symbol` once https://github.com/rust-lang/rust/pull/121230 + // is merged. + && Level::from_str(ident.name.as_str()).is_some() + && let Some(items) = behind_cfg_attr.meta_item_list() + { + let nb_items = items.len(); + let mut clippy_lints = Vec::with_capacity(items.len()); + for item in items { + if let Some(meta_item) = item.meta_item() + && let [part1, _] = meta_item.path.segments.as_slice() + && part1.ident.name == sym::clippy + { + clippy_lints.push(item.span()); + } + } + if clippy_lints.is_empty() { + return; + } + if nb_items == clippy_lints.len() { + if let Some(snippet) = snippet_opt(cx, behind_cfg_attr.span) { + span_lint_and_sugg( + cx, + UNNECESSARY_CLIPPY_CFG, + attr.span, + "no need to put clippy lints behind a `clippy` cfg", + "replace with", + format!( + "#{}[{}]", + if attr.style == AttrStyle::Inner { "!" } else { "" }, + snippet + ), + Applicability::MachineApplicable, + ); + } + } else { + let snippet = clippy_lints + .iter() + .filter_map(|sp| snippet_opt(cx, *sp)) + .collect::<Vec<_>>() + .join(","); + span_lint_and_note( + cx, + UNNECESSARY_CLIPPY_CFG, + clippy_lints, + "no need to put clippy lints behind a `clippy` cfg", + None, + &format!( + "write instead: `#{}[{}({})]`", + if attr.style == AttrStyle::Inner { "!" } else { "" }, + ident.name, + snippet + ), + ); } } } diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 60744fee34d..76595e2e26e 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -60,6 +60,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::attrs::MISMATCHED_TARGET_OS_INFO, crate::attrs::NON_MINIMAL_CFG_INFO, crate::attrs::SHOULD_PANIC_WITHOUT_EXPECT_INFO, + crate::attrs::UNNECESSARY_CLIPPY_CFG_INFO, crate::attrs::USELESS_ATTRIBUTE_INFO, crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO, crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO, diff --git a/tests/ui/unnecessary_clippy_cfg.rs b/tests/ui/unnecessary_clippy_cfg.rs new file mode 100644 index 00000000000..ff960520f5e --- /dev/null +++ b/tests/ui/unnecessary_clippy_cfg.rs @@ -0,0 +1,23 @@ +//@no-rustfix + +#![warn(clippy::unnecessary_clippy_cfg)] +#![cfg_attr(clippy, deny(clippy::non_minimal_cfg))] +//~^ ERROR: no need to put clippy lints behind a `clippy` cfg +#![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))] +//~^ ERROR: no need to put clippy lints behind a `clippy` cfg +#![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] +//~^ ERROR: no need to put clippy lints behind a `clippy` cfg +#![cfg_attr(clippy, deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] +//~^ ERROR: no need to put clippy lints behind a `clippy` cfg + +#[cfg_attr(clippy, deny(clippy::non_minimal_cfg))] +//~^ ERROR: no need to put clippy lints behind a `clippy` cfg +#[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))] +//~^ ERROR: no need to put clippy lints behind a `clippy` cfg +#[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] +//~^ ERROR: no need to put clippy lints behind a `clippy` cfg +#[cfg_attr(clippy, deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] +//~^ ERROR: no need to put clippy lints behind a `clippy` cfg +pub struct Bar; + +fn main() {} diff --git a/tests/ui/unnecessary_clippy_cfg.stderr b/tests/ui/unnecessary_clippy_cfg.stderr new file mode 100644 index 00000000000..fbc05743ca7 --- /dev/null +++ b/tests/ui/unnecessary_clippy_cfg.stderr @@ -0,0 +1,61 @@ +error: no need to put clippy lints behind a `clippy` cfg + --> tests/ui/unnecessary_clippy_cfg.rs:13:1 + | +LL | #[cfg_attr(clippy, deny(clippy::non_minimal_cfg))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `#[deny(clippy::non_minimal_cfg)]` + | + = note: `-D clippy::unnecessary-clippy-cfg` implied by `-D warnings` + = help: to override `-D warnings` add `#[allow(clippy::unnecessary_clippy_cfg)]` + +error: no need to put clippy lints behind a `clippy` cfg + --> tests/ui/unnecessary_clippy_cfg.rs:15:36 + | +LL | #[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: write instead: `#[deny(clippy::non_minimal_cfg)]` + +error: no need to put clippy lints behind a `clippy` cfg + --> tests/ui/unnecessary_clippy_cfg.rs:17:36 + | +LL | #[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] + | ^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: write instead: `#[deny(clippy::non_minimal_cfg,clippy::maybe_misused_cfg)]` + +error: no need to put clippy lints behind a `clippy` cfg + --> tests/ui/unnecessary_clippy_cfg.rs:19:1 + | +LL | #[cfg_attr(clippy, deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `#[deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg)]` + +error: no need to put clippy lints behind a `clippy` cfg + --> tests/ui/unnecessary_clippy_cfg.rs:4:1 + | +LL | #![cfg_attr(clippy, deny(clippy::non_minimal_cfg))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `#![deny(clippy::non_minimal_cfg)]` + +error: no need to put clippy lints behind a `clippy` cfg + --> tests/ui/unnecessary_clippy_cfg.rs:6:37 + | +LL | #![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))] + | ^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: write instead: `#![deny(clippy::non_minimal_cfg)]` + +error: no need to put clippy lints behind a `clippy` cfg + --> tests/ui/unnecessary_clippy_cfg.rs:8:37 + | +LL | #![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] + | ^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = note: write instead: `#![deny(clippy::non_minimal_cfg,clippy::maybe_misused_cfg)]` + +error: no need to put clippy lints behind a `clippy` cfg + --> tests/ui/unnecessary_clippy_cfg.rs:10:1 + | +LL | #![cfg_attr(clippy, deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg))] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `#![deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg)]` + +error: aborting due to 8 previous errors + |