Skip to content

Commit

Permalink
Auto merge of #10763 - GuillaumeGomez:unique_cfg_condition, r=Jarcho
Browse files Browse the repository at this point in the history
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
```
  • Loading branch information
bors committed May 20, 2023
2 parents 2a1dbba + dbc76a7 commit 940ffdf
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
68 changes: 68 additions & 0 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/non_minimal_cfg.fixed
Original file line number Diff line number Diff line change
@@ -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() {}
17 changes: 17 additions & 0 deletions tests/ui/non_minimal_cfg.rs
Original file line number Diff line number Diff line change
@@ -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() {}
28 changes: 28 additions & 0 deletions tests/ui/non_minimal_cfg.stderr
Original file line number Diff line number Diff line change
@@ -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

6 changes: 6 additions & 0 deletions tests/ui/non_minimal_cfg2.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
#![allow(unused)]

#[cfg(all())]
fn all() {}

fn main() {}
10 changes: 10 additions & 0 deletions tests/ui/non_minimal_cfg2.stderr
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 940ffdf

Please sign in to comment.