Skip to content

Commit d554bca

Browse files
committed
Auto merge of #12303 - GuillaumeGomez:unneedeed_clippy_cfg_attr, r=flip1995
Add `unnecessary_clippy_cfg` lint Follow-up of #12292. r? `@flip1995` changelog: Add `unnecessary_clippy_cfg` lint
2 parents 422f9c5 + cf6a14c commit d554bca

File tree

5 files changed

+182
-1
lines changed

5 files changed

+182
-1
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -5726,6 +5726,7 @@ Released 2018-09-13
57265726
[`unknown_clippy_lints`]: https://rust-lang.github.io/rust-clippy/master/index.html#unknown_clippy_lints
57275727
[`unnecessary_box_returns`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_box_returns
57285728
[`unnecessary_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_cast
5729+
[`unnecessary_clippy_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_clippy_cfg
57295730
[`unnecessary_fallible_conversions`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_fallible_conversions
57305731
[`unnecessary_filter_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_filter_map
57315732
[`unnecessary_find_map`]: https://rust-lang.github.io/rust-clippy/master/index.html#unnecessary_find_map

clippy_lints/src/attrs.rs

+96-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
//! checks for attributes
22
33
use clippy_config::msrvs::{self, Msrv};
4-
use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
4+
use clippy_utils::diagnostics::{
5+
span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_sugg, span_lint_and_then,
6+
};
57
use clippy_utils::is_from_proc_macro;
68
use clippy_utils::macros::{is_panic, macro_backtrace};
79
use clippy_utils::source::{first_line_of_span, is_present_in_source, snippet_opt, without_block_comments};
@@ -459,6 +461,30 @@ declare_clippy_lint! {
459461
"usage of `cfg(feature = \"cargo-clippy\")` instead of `cfg(clippy)`"
460462
}
461463

464+
declare_clippy_lint! {
465+
/// ### What it does
466+
/// Checks for `#[cfg_attr(clippy, allow(clippy::lint))]`
467+
/// and suggests to replace it with `#[allow(clippy::lint)]`.
468+
///
469+
/// ### Why is this bad?
470+
/// There is no reason to put clippy attributes behind a clippy `cfg` as they are not
471+
/// run by anything else than clippy.
472+
///
473+
/// ### Example
474+
/// ```no_run
475+
/// #![cfg_attr(clippy, allow(clippy::deprecated_cfg_attr))]
476+
/// ```
477+
///
478+
/// Use instead:
479+
/// ```no_run
480+
/// #![allow(clippy::deprecated_cfg_attr)]
481+
/// ```
482+
#[clippy::version = "1.78.0"]
483+
pub UNNECESSARY_CLIPPY_CFG,
484+
suspicious,
485+
"usage of `cfg_attr(clippy, allow(clippy::lint))` instead of `allow(clippy::lint)`"
486+
}
487+
462488
declare_lint_pass!(Attributes => [
463489
ALLOW_ATTRIBUTES_WITHOUT_REASON,
464490
INLINE_ALWAYS,
@@ -821,6 +847,7 @@ impl_lint_pass!(EarlyAttributes => [
821847
NON_MINIMAL_CFG,
822848
MAYBE_MISUSED_CFG,
823849
DEPRECATED_CLIPPY_CFG_ATTR,
850+
UNNECESSARY_CLIPPY_CFG,
824851
]);
825852

826853
impl EarlyLintPass for EarlyAttributes {
@@ -958,6 +985,74 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msr
958985
);
959986
} else {
960987
check_deprecated_cfg_recursively(cx, feature_item);
988+
if let Some(behind_cfg_attr) = items[1].meta_item() {
989+
check_clippy_cfg_attr(cx, feature_item, behind_cfg_attr, attr);
990+
}
991+
}
992+
}
993+
}
994+
995+
fn check_clippy_cfg_attr(
996+
cx: &EarlyContext<'_>,
997+
cfg_attr: &rustc_ast::MetaItem,
998+
behind_cfg_attr: &rustc_ast::MetaItem,
999+
attr: &Attribute,
1000+
) {
1001+
if cfg_attr.has_name(sym::clippy)
1002+
&& let Some(ident) = behind_cfg_attr.ident()
1003+
// FIXME: replace with `from_symbol` once https://github.com/rust-lang/rust/pull/121230
1004+
// is merged.
1005+
&& Level::from_str(ident.name.as_str()).is_some()
1006+
&& let Some(items) = behind_cfg_attr.meta_item_list()
1007+
{
1008+
let nb_items = items.len();
1009+
let mut clippy_lints = Vec::with_capacity(items.len());
1010+
for item in items {
1011+
if let Some(meta_item) = item.meta_item()
1012+
&& let [part1, _] = meta_item.path.segments.as_slice()
1013+
&& part1.ident.name == sym::clippy
1014+
{
1015+
clippy_lints.push(item.span());
1016+
}
1017+
}
1018+
if clippy_lints.is_empty() {
1019+
return;
1020+
}
1021+
if nb_items == clippy_lints.len() {
1022+
if let Some(snippet) = snippet_opt(cx, behind_cfg_attr.span) {
1023+
span_lint_and_sugg(
1024+
cx,
1025+
UNNECESSARY_CLIPPY_CFG,
1026+
attr.span,
1027+
"no need to put clippy lints behind a `clippy` cfg",
1028+
"replace with",
1029+
format!(
1030+
"#{}[{}]",
1031+
if attr.style == AttrStyle::Inner { "!" } else { "" },
1032+
snippet
1033+
),
1034+
Applicability::MachineApplicable,
1035+
);
1036+
}
1037+
} else {
1038+
let snippet = clippy_lints
1039+
.iter()
1040+
.filter_map(|sp| snippet_opt(cx, *sp))
1041+
.collect::<Vec<_>>()
1042+
.join(",");
1043+
span_lint_and_note(
1044+
cx,
1045+
UNNECESSARY_CLIPPY_CFG,
1046+
clippy_lints,
1047+
"no need to put clippy lints behind a `clippy` cfg",
1048+
None,
1049+
&format!(
1050+
"write instead: `#{}[{}({})]`",
1051+
if attr.style == AttrStyle::Inner { "!" } else { "" },
1052+
ident.name,
1053+
snippet
1054+
),
1055+
);
9611056
}
9621057
}
9631058
}

clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
6060
crate::attrs::MISMATCHED_TARGET_OS_INFO,
6161
crate::attrs::NON_MINIMAL_CFG_INFO,
6262
crate::attrs::SHOULD_PANIC_WITHOUT_EXPECT_INFO,
63+
crate::attrs::UNNECESSARY_CLIPPY_CFG_INFO,
6364
crate::attrs::USELESS_ATTRIBUTE_INFO,
6465
crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO,
6566
crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO,

tests/ui/unnecessary_clippy_cfg.rs

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
//@no-rustfix
2+
3+
#![warn(clippy::unnecessary_clippy_cfg)]
4+
#![cfg_attr(clippy, deny(clippy::non_minimal_cfg))]
5+
//~^ ERROR: no need to put clippy lints behind a `clippy` cfg
6+
#![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))]
7+
//~^ ERROR: no need to put clippy lints behind a `clippy` cfg
8+
#![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))]
9+
//~^ ERROR: no need to put clippy lints behind a `clippy` cfg
10+
#![cfg_attr(clippy, deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg))]
11+
//~^ ERROR: no need to put clippy lints behind a `clippy` cfg
12+
13+
#[cfg_attr(clippy, deny(clippy::non_minimal_cfg))]
14+
//~^ ERROR: no need to put clippy lints behind a `clippy` cfg
15+
#[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))]
16+
//~^ ERROR: no need to put clippy lints behind a `clippy` cfg
17+
#[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))]
18+
//~^ ERROR: no need to put clippy lints behind a `clippy` cfg
19+
#[cfg_attr(clippy, deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg))]
20+
//~^ ERROR: no need to put clippy lints behind a `clippy` cfg
21+
pub struct Bar;
22+
23+
fn main() {}
+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
error: no need to put clippy lints behind a `clippy` cfg
2+
--> tests/ui/unnecessary_clippy_cfg.rs:13:1
3+
|
4+
LL | #[cfg_attr(clippy, deny(clippy::non_minimal_cfg))]
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `#[deny(clippy::non_minimal_cfg)]`
6+
|
7+
= note: `-D clippy::unnecessary-clippy-cfg` implied by `-D warnings`
8+
= help: to override `-D warnings` add `#[allow(clippy::unnecessary_clippy_cfg)]`
9+
10+
error: no need to put clippy lints behind a `clippy` cfg
11+
--> tests/ui/unnecessary_clippy_cfg.rs:15:36
12+
|
13+
LL | #[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))]
14+
| ^^^^^^^^^^^^^^^^^^^^^^^
15+
|
16+
= note: write instead: `#[deny(clippy::non_minimal_cfg)]`
17+
18+
error: no need to put clippy lints behind a `clippy` cfg
19+
--> tests/ui/unnecessary_clippy_cfg.rs:17:36
20+
|
21+
LL | #[cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))]
22+
| ^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^
23+
|
24+
= note: write instead: `#[deny(clippy::non_minimal_cfg,clippy::maybe_misused_cfg)]`
25+
26+
error: no need to put clippy lints behind a `clippy` cfg
27+
--> tests/ui/unnecessary_clippy_cfg.rs:19:1
28+
|
29+
LL | #[cfg_attr(clippy, deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg))]
30+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `#[deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg)]`
31+
32+
error: no need to put clippy lints behind a `clippy` cfg
33+
--> tests/ui/unnecessary_clippy_cfg.rs:4:1
34+
|
35+
LL | #![cfg_attr(clippy, deny(clippy::non_minimal_cfg))]
36+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `#![deny(clippy::non_minimal_cfg)]`
37+
38+
error: no need to put clippy lints behind a `clippy` cfg
39+
--> tests/ui/unnecessary_clippy_cfg.rs:6:37
40+
|
41+
LL | #![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg))]
42+
| ^^^^^^^^^^^^^^^^^^^^^^^
43+
|
44+
= note: write instead: `#![deny(clippy::non_minimal_cfg)]`
45+
46+
error: no need to put clippy lints behind a `clippy` cfg
47+
--> tests/ui/unnecessary_clippy_cfg.rs:8:37
48+
|
49+
LL | #![cfg_attr(clippy, deny(dead_code, clippy::non_minimal_cfg, clippy::maybe_misused_cfg))]
50+
| ^^^^^^^^^^^^^^^^^^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^^^^
51+
|
52+
= note: write instead: `#![deny(clippy::non_minimal_cfg,clippy::maybe_misused_cfg)]`
53+
54+
error: no need to put clippy lints behind a `clippy` cfg
55+
--> tests/ui/unnecessary_clippy_cfg.rs:10:1
56+
|
57+
LL | #![cfg_attr(clippy, deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg))]
58+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `#![deny(clippy::non_minimal_cfg, clippy::maybe_misused_cfg)]`
59+
60+
error: aborting due to 8 previous errors
61+

0 commit comments

Comments
 (0)