Skip to content

Commit 940ffdf

Browse files
committed
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 ```
2 parents 2a1dbba + dbc76a7 commit 940ffdf

8 files changed

+148
-0
lines changed

Diff for: CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -4899,6 +4899,7 @@ Released 2018-09-13
48994899
[`no_effect_underscore_binding`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_effect_underscore_binding
49004900
[`no_mangle_with_rust_abi`]: https://rust-lang.github.io/rust-clippy/master/index.html#no_mangle_with_rust_abi
49014901
[`non_ascii_literal`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_ascii_literal
4902+
[`non_minimal_cfg`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_minimal_cfg
49024903
[`non_octal_unix_permissions`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_octal_unix_permissions
49034904
[`non_send_fields_in_send_ty`]: https://rust-lang.github.io/rust-clippy/master/index.html#non_send_fields_in_send_ty
49044905
[`nonminimal_bool`]: https://rust-lang.github.io/rust-clippy/master/index.html#nonminimal_bool

Diff for: clippy_lints/src/attrs.rs

+68
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,30 @@ declare_clippy_lint! {
338338
"ensures that all `allow` and `expect` attributes have a reason"
339339
}
340340

341+
declare_clippy_lint! {
342+
/// ### What it does
343+
/// Checks for `any` and `all` combinators in `cfg` with only one condition.
344+
///
345+
/// ### Why is this bad?
346+
/// If there is only one condition, no need to wrap it into `any` or `all` combinators.
347+
///
348+
/// ### Example
349+
/// ```rust
350+
/// #[cfg(any(unix))]
351+
/// pub struct Bar;
352+
/// ```
353+
///
354+
/// Use instead:
355+
/// ```rust
356+
/// #[cfg(unix)]
357+
/// pub struct Bar;
358+
/// ```
359+
#[clippy::version = "1.71.0"]
360+
pub NON_MINIMAL_CFG,
361+
style,
362+
"ensure that all `cfg(any())` and `cfg(all())` have more than one condition"
363+
}
364+
341365
declare_lint_pass!(Attributes => [
342366
ALLOW_ATTRIBUTES_WITHOUT_REASON,
343367
INLINE_ALWAYS,
@@ -651,6 +675,7 @@ impl_lint_pass!(EarlyAttributes => [
651675
MISMATCHED_TARGET_OS,
652676
EMPTY_LINE_AFTER_OUTER_ATTR,
653677
EMPTY_LINE_AFTER_DOC_COMMENTS,
678+
NON_MINIMAL_CFG,
654679
]);
655680

656681
impl EarlyLintPass for EarlyAttributes {
@@ -661,6 +686,7 @@ impl EarlyLintPass for EarlyAttributes {
661686
fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
662687
check_deprecated_cfg_attr(cx, attr, &self.msrv);
663688
check_mismatched_target_os(cx, attr);
689+
check_minimal_cfg_condition(cx, attr);
664690
}
665691

666692
extract_msrv_attr!(EarlyContext);
@@ -750,6 +776,48 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute, msrv: &Msr
750776
}
751777
}
752778

779+
fn check_nested_cfg(cx: &EarlyContext<'_>, items: &[NestedMetaItem]) {
780+
for item in items.iter() {
781+
if let NestedMetaItem::MetaItem(meta) = item {
782+
if !meta.has_name(sym::any) && !meta.has_name(sym::all) {
783+
continue;
784+
}
785+
if let MetaItemKind::List(list) = &meta.kind {
786+
check_nested_cfg(cx, list);
787+
if list.len() == 1 {
788+
span_lint_and_then(
789+
cx,
790+
NON_MINIMAL_CFG,
791+
meta.span,
792+
"unneeded sub `cfg` when there is only one condition",
793+
|diag| {
794+
if let Some(snippet) = snippet_opt(cx, list[0].span()) {
795+
diag.span_suggestion(meta.span, "try", snippet, Applicability::MaybeIncorrect);
796+
}
797+
},
798+
);
799+
} else if list.is_empty() && meta.has_name(sym::all) {
800+
span_lint_and_then(
801+
cx,
802+
NON_MINIMAL_CFG,
803+
meta.span,
804+
"unneeded sub `cfg` when there is no condition",
805+
|_| {},
806+
);
807+
}
808+
}
809+
}
810+
}
811+
}
812+
813+
fn check_minimal_cfg_condition(cx: &EarlyContext<'_>, attr: &Attribute) {
814+
if attr.has_name(sym::cfg) &&
815+
let Some(items) = attr.meta_item_list()
816+
{
817+
check_nested_cfg(cx, &items);
818+
}
819+
}
820+
753821
fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
754822
fn find_os(name: &str) -> Option<&'static str> {
755823
UNIX_SYSTEMS

Diff for: clippy_lints/src/declared_lints.rs

+1
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
5252
crate::attrs::EMPTY_LINE_AFTER_OUTER_ATTR_INFO,
5353
crate::attrs::INLINE_ALWAYS_INFO,
5454
crate::attrs::MISMATCHED_TARGET_OS_INFO,
55+
crate::attrs::NON_MINIMAL_CFG_INFO,
5556
crate::attrs::USELESS_ATTRIBUTE_INFO,
5657
crate::await_holding_invalid::AWAIT_HOLDING_INVALID_TYPE_INFO,
5758
crate::await_holding_invalid::AWAIT_HOLDING_LOCK_INFO,

Diff for: tests/ui/non_minimal_cfg.fixed

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//@run-rustfix
2+
3+
#![allow(unused)]
4+
5+
#[cfg(windows)]
6+
fn hermit() {}
7+
8+
#[cfg(windows)]
9+
fn wasi() {}
10+
11+
#[cfg(all(unix, not(windows)))]
12+
fn the_end() {}
13+
14+
#[cfg(any())]
15+
fn any() {}
16+
17+
fn main() {}

Diff for: tests/ui/non_minimal_cfg.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
//@run-rustfix
2+
3+
#![allow(unused)]
4+
5+
#[cfg(all(windows))]
6+
fn hermit() {}
7+
8+
#[cfg(any(windows))]
9+
fn wasi() {}
10+
11+
#[cfg(all(any(unix), all(not(windows))))]
12+
fn the_end() {}
13+
14+
#[cfg(any())]
15+
fn any() {}
16+
17+
fn main() {}

Diff for: tests/ui/non_minimal_cfg.stderr

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
error: unneeded sub `cfg` when there is only one condition
2+
--> $DIR/non_minimal_cfg.rs:5:7
3+
|
4+
LL | #[cfg(all(windows))]
5+
| ^^^^^^^^^^^^ help: try: `windows`
6+
|
7+
= note: `-D clippy::non-minimal-cfg` implied by `-D warnings`
8+
9+
error: unneeded sub `cfg` when there is only one condition
10+
--> $DIR/non_minimal_cfg.rs:8:7
11+
|
12+
LL | #[cfg(any(windows))]
13+
| ^^^^^^^^^^^^ help: try: `windows`
14+
15+
error: unneeded sub `cfg` when there is only one condition
16+
--> $DIR/non_minimal_cfg.rs:11:11
17+
|
18+
LL | #[cfg(all(any(unix), all(not(windows))))]
19+
| ^^^^^^^^^ help: try: `unix`
20+
21+
error: unneeded sub `cfg` when there is only one condition
22+
--> $DIR/non_minimal_cfg.rs:11:22
23+
|
24+
LL | #[cfg(all(any(unix), all(not(windows))))]
25+
| ^^^^^^^^^^^^^^^^^ help: try: `not(windows)`
26+
27+
error: aborting due to 4 previous errors
28+

Diff for: tests/ui/non_minimal_cfg2.rs

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#![allow(unused)]
2+
3+
#[cfg(all())]
4+
fn all() {}
5+
6+
fn main() {}

Diff for: tests/ui/non_minimal_cfg2.stderr

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
error: unneeded sub `cfg` when there is no condition
2+
--> $DIR/non_minimal_cfg2.rs:3:7
3+
|
4+
LL | #[cfg(all())]
5+
| ^^^^^
6+
|
7+
= note: `-D clippy::non-minimal-cfg` implied by `-D warnings`
8+
9+
error: aborting due to previous error
10+

0 commit comments

Comments
 (0)