From ab66a86815a2bb562edef92ef8c308b6958ea734 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 9 May 2023 11:51:36 +0200 Subject: [PATCH 1/4] Add new `UNIQUE_CFG_CONDITION` lint --- clippy_lints/src/attrs.rs | 60 ++++++++++++++++++++++++++++++ clippy_lints/src/declared_lints.rs | 1 + 2 files changed, 61 insertions(+) diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 93d88391a79b..6678002820ac 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -338,6 +338,30 @@ declare_clippy_lint! { "ensures that all `allow` and `expect` attributes have a reason" } +declare_clippy_lint! { + /// ### What it does + /// Checks for `any` and `all` combinators in `cfg` with only one condition. + /// + /// ### Why is this bad? + /// If there is only one condition, no need to wrap it into `any` or `all` combinators. + /// + /// ### Example + /// ```rust + /// #[cfg(any(unix))] + /// pub struct Bar; + /// ``` + /// + /// Use instead: + /// ```rust + /// #[cfg(unix)] + /// pub struct Bar; + /// ``` + #[clippy::version = "1.71.0"] + pub NON_MINIMAL_CFG, + style, + "ensure that all `cfg(any())` and `cfg(all())` have more than one condition" +} + declare_lint_pass!(Attributes => [ ALLOW_ATTRIBUTES_WITHOUT_REASON, INLINE_ALWAYS, @@ -651,6 +675,7 @@ impl_lint_pass!(EarlyAttributes => [ MISMATCHED_TARGET_OS, EMPTY_LINE_AFTER_OUTER_ATTR, EMPTY_LINE_AFTER_DOC_COMMENTS, + NON_MINIMAL_CFG, ]); impl EarlyLintPass for EarlyAttributes { @@ -661,6 +686,7 @@ impl EarlyLintPass for EarlyAttributes { fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) { check_deprecated_cfg_attr(cx, attr, &self.msrv); check_mismatched_target_os(cx, attr); + check_minimal_cfg_condition(cx, attr); } extract_msrv_attr!(EarlyContext); @@ -750,6 +776,40 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msr } } +fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) { + for item in items.iter() { + if let NestedMetaItem::MetaItem(meta) = item { + if !meta.has_name(sym::any) && !meta.has_name(sym::all) { + continue; + } + if let MetaItemKind::List(list) = &meta.kind { + check_nested_cfg(cx, list); + if list.len() == 1 { + span_lint_and_then( + cx, + NON_MINIMAL_CFG, + meta.span, + "unneeded sub `cfg` when there is only one condition", + |diag| { + if let Some(snippet) = snippet_opt(cx, list[0].span()) { + diag.span_suggestion(meta.span, "try", snippet, Applicability::MaybeIncorrect); + } + }, + ); + } + } + } + } +} + +fn check_minimal_cfg_condition(cx: &EarlyContext<'_>, attr: &Attribute) { + if attr.has_name(sym::cfg) && + let Some(items) = attr.meta_item_list() + { + check_nested_cfg(cx, &items); + } +} + fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) { fn find_os(name: &str) -> Option<&'static str> { UNIX_SYSTEMS diff --git a/clippy_lints/src/declared_lints.rs b/clippy_lints/src/declared_lints.rs index 40c64efaa37d..aee43edc4780 100644 --- a/clippy_lints/src/declared_lints.rs +++ b/clippy_lints/src/declared_lints.rs @@ -52,6 +52,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[ crate::attrs::EMPTY_LINE_AFTER_OUTER_ATTR_INFO, crate::attrs::INLINE_ALWAYS_INFO, crate::attrs::MISMATCHED_TARGET_OS_INFO, + crate::attrs::NON_MINIMAL_CFG_INFO, crate::attrs::USELESS_ATTRIBUTE_INFO, crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO, crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO, From fcf19481aa17ea7bfaabc0bfae07aebc40248146 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 9 May 2023 12:13:49 +0200 Subject: [PATCH 2/4] Add UI test for UNIQUE_CFG_CONDITION --- tests/ui/non_minimal_cfg.fixed | 14 ++++++++++++++ tests/ui/non_minimal_cfg.rs | 14 ++++++++++++++ tests/ui/non_minimal_cfg.stderr | 28 ++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+) create mode 100644 tests/ui/non_minimal_cfg.fixed create mode 100644 tests/ui/non_minimal_cfg.rs create mode 100644 tests/ui/non_minimal_cfg.stderr diff --git a/tests/ui/non_minimal_cfg.fixed b/tests/ui/non_minimal_cfg.fixed new file mode 100644 index 000000000000..ca9a48a3e377 --- /dev/null +++ b/tests/ui/non_minimal_cfg.fixed @@ -0,0 +1,14 @@ +//@run-rustfix + +#![allow(unused)] + +#[cfg(windows)] +fn hermit() {} + +#[cfg(windows)] +fn wasi() {} + +#[cfg(all(unix, not(windows)))] +fn the_end() {} + +fn main() {} diff --git a/tests/ui/non_minimal_cfg.rs b/tests/ui/non_minimal_cfg.rs new file mode 100644 index 000000000000..bf322236fef7 --- /dev/null +++ b/tests/ui/non_minimal_cfg.rs @@ -0,0 +1,14 @@ +//@run-rustfix + +#![allow(unused)] + +#[cfg(all(windows))] +fn hermit() {} + +#[cfg(any(windows))] +fn wasi() {} + +#[cfg(all(any(unix), all(not(windows))))] +fn the_end() {} + +fn main() {} diff --git a/tests/ui/non_minimal_cfg.stderr b/tests/ui/non_minimal_cfg.stderr new file mode 100644 index 000000000000..cdfd728aa611 --- /dev/null +++ b/tests/ui/non_minimal_cfg.stderr @@ -0,0 +1,28 @@ +error: unneeded sub `cfg` when there is only one condition + --> $DIR/non_minimal_cfg.rs:5:7 + | +LL | #[cfg(all(windows))] + | ^^^^^^^^^^^^ help: try: `windows` + | + = note: `-D clippy::non-minimal-cfg` implied by `-D warnings` + +error: unneeded sub `cfg` when there is only one condition + --> $DIR/non_minimal_cfg.rs:8:7 + | +LL | #[cfg(any(windows))] + | ^^^^^^^^^^^^ help: try: `windows` + +error: unneeded sub `cfg` when there is only one condition + --> $DIR/non_minimal_cfg.rs:11:11 + | +LL | #[cfg(all(any(unix), all(not(windows))))] + | ^^^^^^^^^ help: try: `unix` + +error: unneeded sub `cfg` when there is only one condition + --> $DIR/non_minimal_cfg.rs:11:22 + | +LL | #[cfg(all(any(unix), all(not(windows))))] + | ^^^^^^^^^^^^^^^^^ help: try: `not(windows)` + +error: aborting due to 4 previous errors + From 5328852ad7d6801acd03eefc8bc88c3ee70929b5 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 9 May 2023 12:14:04 +0200 Subject: [PATCH 3/4] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index dafa3f3a1393..79f2a47110b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4899,6 +4899,7 @@ Released 2018-09-13 [`no_effect_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding [`no_mangle_with_rust_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_mangle_with_rust_abi [`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal +[`non_minimal_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_minimal_cfg [`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions [`non_send_fields_in_send_ty`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty [`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool From dbc76a766333bf510de601111f31e3eb42fb0434 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 19 May 2023 16:23:17 +0200 Subject: [PATCH 4/4] Add check for empty cfg `all` condition --- clippy_lints/src/attrs.rs | 8 ++++++++ tests/ui/non_minimal_cfg.fixed | 3 +++ tests/ui/non_minimal_cfg.rs | 3 +++ tests/ui/non_minimal_cfg2.rs | 6 ++++++ tests/ui/non_minimal_cfg2.stderr | 10 ++++++++++ 5 files changed, 30 insertions(+) create mode 100644 tests/ui/non_minimal_cfg2.rs create mode 100644 tests/ui/non_minimal_cfg2.stderr diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs index 6678002820ac..897495ba1087 100644 --- a/clippy_lints/src/attrs.rs +++ b/clippy_lints/src/attrs.rs @@ -796,6 +796,14 @@ fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) { } }, ); + } else if list.is_empty() && meta.has_name(sym::all) { + span_lint_and_then( + cx, + NON_MINIMAL_CFG, + meta.span, + "unneeded sub `cfg` when there is no condition", + |_| {}, + ); } } } diff --git a/tests/ui/non_minimal_cfg.fixed b/tests/ui/non_minimal_cfg.fixed index ca9a48a3e377..430caafb33e1 100644 --- a/tests/ui/non_minimal_cfg.fixed +++ b/tests/ui/non_minimal_cfg.fixed @@ -11,4 +11,7 @@ fn wasi() {} #[cfg(all(unix, not(windows)))] fn the_end() {} +#[cfg(any())] +fn any() {} + fn main() {} diff --git a/tests/ui/non_minimal_cfg.rs b/tests/ui/non_minimal_cfg.rs index bf322236fef7..a38ce1c21d6e 100644 --- a/tests/ui/non_minimal_cfg.rs +++ b/tests/ui/non_minimal_cfg.rs @@ -11,4 +11,7 @@ fn wasi() {} #[cfg(all(any(unix), all(not(windows))))] fn the_end() {} +#[cfg(any())] +fn any() {} + fn main() {} diff --git a/tests/ui/non_minimal_cfg2.rs b/tests/ui/non_minimal_cfg2.rs new file mode 100644 index 000000000000..a4c6abce3876 --- /dev/null +++ b/tests/ui/non_minimal_cfg2.rs @@ -0,0 +1,6 @@ +#![allow(unused)] + +#[cfg(all())] +fn all() {} + +fn main() {} diff --git a/tests/ui/non_minimal_cfg2.stderr b/tests/ui/non_minimal_cfg2.stderr new file mode 100644 index 000000000000..2a9a36fbcef3 --- /dev/null +++ b/tests/ui/non_minimal_cfg2.stderr @@ -0,0 +1,10 @@ +error: unneeded sub `cfg` when there is no condition + --> $DIR/non_minimal_cfg2.rs:3:7 + | +LL | #[cfg(all())] + | ^^^^^ + | + = note: `-D clippy::non-minimal-cfg` implied by `-D warnings` + +error: aborting due to previous error +