Skip to content

Commit

Permalink
Auto merge of rust-lang#12303 - GuillaumeGomez:unneedeed_clippy_cfg_a…
Browse files Browse the repository at this point in the history 
…ttr, r=flip1995

Add `unnecessary_clippy_cfg` lint

Follow-up of rust-lang#12292.

r? `@flip1995`

changelog: Add `unnecessary_clippy_cfg` lint
  • Loading branch information
@bors
bors committed Feb 22, 2024
2 parents 422f9c5 + cf6a14c commit d554bca
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Expand Up @@ -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
Expand Down
97 changes: 96 additions & 1 deletion clippy_lints/src/attrs.rs
View file
@@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
),
);
}
}
}
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
View file
Expand Up @@ -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,
Expand Down
23 changes: 23 additions & 0 deletions tests/ui/unnecessary_clippy_cfg.rs
View file
@@ -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() {}
61 changes: 61 additions & 0 deletions tests/ui/unnecessary_clippy_cfg.stderr
View file
@@ -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

0 comments on commit d554bca

Please sign in to comment.