about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-05-20 02:47:51 +0000
committerbors <bors@rust-lang.org>2023-05-20 02:47:51 +0000
commit940ffdfd1eb938d0c1f0222e8e56c99cac9d4601 (patch)
tree1ac337ab5fc050d60531257a3cdc5ac61f061e49
parent2a1dbbaec57440790eb6a457f628a0d295a6b80d (diff)
parentdbc76a766333bf510de601111f31e3eb42fb0434 (diff)
downloadrust-940ffdfd1eb938d0c1f0222e8e56c99cac9d4601.tar.gz
rust-940ffdfd1eb938d0c1f0222e8e56c99cac9d4601.zip
Auto merge of #10763 - GuillaumeGomez:unique_cfg_condition, r=Jarcho
Add `MINIMAL_CFG_CONDITION` lint

I encountered a few cases where some code had:

```rust
#[cfg(any(unix))]
```

In this case, the `any` is useless. This lint checks this and also for the `all` condition.

```
changelog: [`unique_cfg_condition`]: Add new `UNIQUE_CFG_CONDITION` lint
```
-rw-r--r--CHANGELOG.md1
-rw-r--r--clippy_lints/src/attrs.rs68
-rw-r--r--clippy_lints/src/declared_lints.rs1
-rw-r--r--tests/ui/non_minimal_cfg.fixed17
-rw-r--r--tests/ui/non_minimal_cfg.rs17
-rw-r--r--tests/ui/non_minimal_cfg.stderr28
-rw-r--r--tests/ui/non_minimal_cfg2.rs6
-rw-r--r--tests/ui/non_minimal_cfg2.stderr10
8 files changed, 148 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index dafa3f3a139..79f2a47110b 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
diff --git a/clippy_lints/src/attrs.rs b/clippy_lints/src/attrs.rs
index 93d88391a79..897495ba108 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,48 @@ 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);
+                            }
+                        },
+                    );
+                } 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",
+                        |_| {},
+                    );
+                }
+            }
+        }
+    }
+}
+
+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 40c64efaa37..aee43edc478 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,
diff --git a/tests/ui/non_minimal_cfg.fixed b/tests/ui/non_minimal_cfg.fixed
new file mode 100644
index 00000000000..430caafb33e
--- /dev/null
+++ b/tests/ui/non_minimal_cfg.fixed
@@ -0,0 +1,17 @@
+//@run-rustfix
+
+#![allow(unused)]
+
+#[cfg(windows)]
+fn hermit() {}
+
+#[cfg(windows)]
+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
new file mode 100644
index 00000000000..a38ce1c21d6
--- /dev/null
+++ b/tests/ui/non_minimal_cfg.rs
@@ -0,0 +1,17 @@
+//@run-rustfix
+
+#![allow(unused)]
+
+#[cfg(all(windows))]
+fn hermit() {}
+
+#[cfg(any(windows))]
+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_cfg.stderr b/tests/ui/non_minimal_cfg.stderr
new file mode 100644
index 00000000000..cdfd728aa61
--- /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
+
diff --git a/tests/ui/non_minimal_cfg2.rs b/tests/ui/non_minimal_cfg2.rs
new file mode 100644
index 00000000000..a4c6abce387
--- /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 00000000000..2a9a36fbcef
--- /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
+