Auto merge of rust-lang#5506 - ebroto:mismatched_target_os, r=flip1995

Implement mismatched_target_os lint

I've extended the check suggested in the issue to all the currently supported operating systems instead of limiting it to `linux` and `macos`, let me know if we want to do this.

Also, I've restored the text `There are over XXX lints ...` in the README as it was matched against by `cargo dev new_lint`.

changelog: Added `mismatched_target_os` lint to warn when an operating system is used in target family position in a #[cfg] attribute

Closes rust-lang#3949

Author: bors

CHANGELOG.md

@@ -1444,6 +1444,7 @@ Released 2018-09-13
 [`mem_replace_with_uninit`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_uninit
 [`min_max`]: https://rust-lang.github.io/rust-clippy/master/index.html#min_max
 [`misaligned_transmute`]: https://rust-lang.github.io/rust-clippy/master/index.html#misaligned_transmute
+[`mismatched_target_os`]: https://rust-lang.github.io/rust-clippy/master/index.html#mismatched_target_os
 [`misrefactored_assign_op`]: https://rust-lang.github.io/rust-clippy/master/index.html#misrefactored_assign_op
 [`missing_const_for_fn`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_const_for_fn
 [`missing_docs_in_private_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_docs_in_private_items

clippy_lints/src/attrs.rs

@@ -20,6 +20,28 @@ use rustc_span::source_map::Span;
 use rustc_span::symbol::Symbol;
 use semver::Version;
 
+static UNIX_SYSTEMS: &[&str] = &[
+    "android",
+    "dragonfly",
+    "emscripten",
+    "freebsd",
+    "fuchsia",
+    "haiku",
+    "illumos",
+    "ios",
+    "l4re",
+    "linux",
+    "macos",
+    "netbsd",
+    "openbsd",
+    "redox",
+    "solaris",
+    "vxworks",
+];
+
+// NOTE: windows is excluded from the list because it's also a valid target family.
+static NON_UNIX_SYSTEMS: &[&str] = &["cloudabi", "hermit", "none", "wasi"];
+
 declare_clippy_lint! {
     /// **What it does:** Checks for items annotated with `#[inline(always)]`,
     /// unless the annotated function is empty or simply panics.
@@ -189,6 +211,38 @@ declare_clippy_lint! {
     "usage of `cfg_attr(rustfmt)` instead of tool attributes"
 }
 
+declare_clippy_lint! {
+    /// **What it does:** Checks for cfg attributes having operating systems used in target family position.
+    ///
+    /// **Why is this bad?** The configuration option will not be recognised and the related item will not be included
+    /// by the conditional compilation engine.
+    ///
+    /// **Known problems:** None.
+    ///
+    /// **Example:**
+    ///
+    /// Bad:
+    /// ```rust
+    /// #[cfg(linux)]
+    /// fn conditional() { }
+    /// ```
+    ///
+    /// Good:
+    /// ```rust
+    /// #[cfg(target_os = "linux")]
+    /// fn conditional() { }
+    /// ```
+    ///
+    /// Or:
+    /// ```rust
+    /// #[cfg(unix)]
+    /// fn conditional() { }
+    /// ```
+    pub MISMATCHED_TARGET_OS,
+    correctness,
+    "usage of `cfg(operating_system)` instead of `cfg(target_os = \"operating_system\")`"
+}
+
 declare_lint_pass!(Attributes => [
     INLINE_ALWAYS,
     DEPRECATED_SEMVER,
@@ -496,36 +550,107 @@ fn is_word(nmi: &NestedMetaItem, expected: Symbol) -> bool {
     }
 }
 
-declare_lint_pass!(DeprecatedCfgAttribute => [DEPRECATED_CFG_ATTR]);
+declare_lint_pass!(EarlyAttributes => [DEPRECATED_CFG_ATTR, MISMATCHED_TARGET_OS]);
 
-impl EarlyLintPass for DeprecatedCfgAttribute {
+impl EarlyLintPass for EarlyAttributes {
     fn check_attribute(&mut self, cx: &EarlyContext<'_>, attr: &Attribute) {
-        if_chain! {
-            // check cfg_attr
-            if attr.check_name(sym!(cfg_attr));
-            if let Some(items) = attr.meta_item_list();
-            if items.len() == 2;
-            // check for `rustfmt`
-            if let Some(feature_item) = items[0].meta_item();
-            if feature_item.check_name(sym!(rustfmt));
-            // check for `rustfmt_skip` and `rustfmt::skip`
-            if let Some(skip_item) = &items[1].meta_item();
-            if skip_item.check_name(sym!(rustfmt_skip)) ||
-                skip_item.path.segments.last().expect("empty path in attribute").ident.name == sym!(skip);
-            // Only lint outer attributes, because custom inner attributes are unstable
-            // Tracking issue: https://github.com/rust-lang/rust/issues/54726
-            if let AttrStyle::Outer = attr.style;
-            then {
-                span_lint_and_sugg(
-                    cx,
-                    DEPRECATED_CFG_ATTR,
-                    attr.span,
-                    "`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes",
-                    "use",
-                    "#[rustfmt::skip]".to_string(),
-                    Applicability::MachineApplicable,
-                );
+        check_deprecated_cfg_attr(cx, attr);
+        check_mismatched_target_os(cx, attr);
+    }
+}
+
+fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute) {
+    if_chain! {
+        // check cfg_attr
+        if attr.check_name(sym!(cfg_attr));
+        if let Some(items) = attr.meta_item_list();
+        if items.len() == 2;
+        // check for `rustfmt`
+        if let Some(feature_item) = items[0].meta_item();
+        if feature_item.check_name(sym!(rustfmt));
+        // check for `rustfmt_skip` and `rustfmt::skip`
+        if let Some(skip_item) = &items[1].meta_item();
+        if skip_item.check_name(sym!(rustfmt_skip)) ||
+            skip_item.path.segments.last().expect("empty path in attribute").ident.name == sym!(skip);
+        // Only lint outer attributes, because custom inner attributes are unstable
+        // Tracking issue: https://github.com/rust-lang/rust/issues/54726
+        if let AttrStyle::Outer = attr.style;
+        then {
+            span_lint_and_sugg(
+                cx,
+                DEPRECATED_CFG_ATTR,
+                attr.span,
+                "`cfg_attr` is deprecated for rustfmt and got replaced by tool attributes",
+                "use",
+                "#[rustfmt::skip]".to_string(),
+                Applicability::MachineApplicable,
+            );
+        }
+    }
+}
+
+fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
+    fn find_os(name: &str) -> Option<&'static str> {
+        UNIX_SYSTEMS
+            .iter()
+            .chain(NON_UNIX_SYSTEMS.iter())
+            .find(|&&os| os == name)
+            .copied()
+    }
+
+    fn is_unix(name: &str) -> bool {
+        UNIX_SYSTEMS.iter().any(|&os| os == name)
+    }
+
+    fn find_mismatched_target_os(items: &[NestedMetaItem]) -> Vec<(&str, Span)> {
+        let mut mismatched = Vec::new();
+
+        for item in items {
+            if let NestedMetaItem::MetaItem(meta) = item {
+                match &meta.kind {
+                    MetaItemKind::List(list) => {
+                        mismatched.extend(find_mismatched_target_os(&list));
+                    },
+                    MetaItemKind::Word => {
+                        if_chain! {
+                            if let Some(ident) = meta.ident();
+                            if let Some(os) = find_os(&*ident.name.as_str());
+                            then {
+                                mismatched.push((os, ident.span));
+                            }
+                        }
+                    },
+                    _ => {},
+                }
             }
         }
+
+        mismatched
+    }
+
+    if_chain! {
+        if attr.check_name(sym!(cfg));
+        if let Some(list) = attr.meta_item_list();
+        let mismatched = find_mismatched_target_os(&list);
+        if !mismatched.is_empty();
+        then {
+            let mess = "operating system used in target family position";
+
+            span_lint_and_then(cx, MISMATCHED_TARGET_OS, attr.span, &mess, |diag| {
+                // Avoid showing the unix suggestion multiple times in case
+                // we have more than one mismatch for unix-like systems
+                let mut unix_suggested = false;
+
+                for (os, span) in mismatched {
+                    let sugg = format!("target_os = \"{}\"", os);
+                    diag.span_suggestion(span, "try", sugg, Applicability::MaybeIncorrect);
+
+                    if !unix_suggested && is_unix(os) {
+                        diag.help("Did you mean `unix`?");
+                        unix_suggested = true;
+                    }
+                }
+            });
+        }
     }
 }

clippy_lints/src/lib.rs

@@ -350,7 +350,7 @@ pub fn register_pre_expansion_lints(store: &mut rustc_lint::LintStore, conf: &Co
     store.register_pre_expansion_pass(move || box non_expressive_names::NonExpressiveNames {
         single_char_binding_names_threshold,
     });
-    store.register_pre_expansion_pass(|| box attrs::DeprecatedCfgAttribute);
+    store.register_pre_expansion_pass(|| box attrs::EarlyAttributes);
     store.register_pre_expansion_pass(|| box dbg_macro::DbgMacro);
 }
 
@@ -496,6 +496,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         &attrs::DEPRECATED_SEMVER,
         &attrs::EMPTY_LINE_AFTER_OUTER_ATTR,
         &attrs::INLINE_ALWAYS,
+        &attrs::MISMATCHED_TARGET_OS,
         &attrs::UNKNOWN_CLIPPY_LINTS,
         &attrs::USELESS_ATTRIBUTE,
         &await_holding_lock::AWAIT_HOLDING_LOCK,
@@ -1190,6 +1191,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
         LintId::of(&attrs::DEPRECATED_CFG_ATTR),
         LintId::of(&attrs::DEPRECATED_SEMVER),
+        LintId::of(&attrs::MISMATCHED_TARGET_OS),
         LintId::of(&attrs::UNKNOWN_CLIPPY_LINTS),
         LintId::of(&attrs::USELESS_ATTRIBUTE),
         LintId::of(&bit_mask::BAD_BIT_MASK),
@@ -1610,6 +1612,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&approx_const::APPROX_CONSTANT),
         LintId::of(&atomic_ordering::INVALID_ATOMIC_ORDERING),
         LintId::of(&attrs::DEPRECATED_SEMVER),
+        LintId::of(&attrs::MISMATCHED_TARGET_OS),
         LintId::of(&attrs::USELESS_ATTRIBUTE),
         LintId::of(&bit_mask::BAD_BIT_MASK),
         LintId::of(&bit_mask::INEFFECTIVE_BIT_MASK),

src/lintlist/mod.rs

@@ -1228,6 +1228,13 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
         deprecation: None,
         module: "minmax",
     },
+    Lint {
+        name: "mismatched_target_os",
+        group: "correctness",
+        desc: "usage of `cfg(operating_system)` instead of `cfg(target_os = \"operating_system\")`",
+        deprecation: None,
+        module: "attrs",
+    },
     Lint {
         name: "misrefactored_assign_op",
         group: "complexity",

tests/ui/mismatched_target_os_non_unix.fixed

@@ -0,0 +1,30 @@
+// run-rustfix
+
+#![warn(clippy::mismatched_target_os)]
+#![allow(unused)]
+
+#[cfg(target_os = "cloudabi")]
+fn cloudabi() {}
+
+#[cfg(target_os = "hermit")]
+fn hermit() {}
+
+#[cfg(target_os = "wasi")]
+fn wasi() {}
+
+#[cfg(target_os = "none")]
+fn none() {}
+
+// list with conditions
+#[cfg(all(not(any(windows, target_os = "cloudabi")), target_os = "wasi"))]
+fn list() {}
+
+// windows is a valid target family, should be ignored
+#[cfg(windows)]
+fn windows() {}
+
+// correct use, should be ignored
+#[cfg(target_os = "hermit")]
+fn correct() {}
+
+fn main() {}

tests/ui/mismatched_target_os_non_unix.rs

@@ -0,0 +1,30 @@
+// run-rustfix
+
+#![warn(clippy::mismatched_target_os)]
+#![allow(unused)]
+
+#[cfg(cloudabi)]
+fn cloudabi() {}
+
+#[cfg(hermit)]
+fn hermit() {}
+
+#[cfg(wasi)]
+fn wasi() {}
+
+#[cfg(none)]
+fn none() {}
+
+// list with conditions
+#[cfg(all(not(any(windows, cloudabi)), wasi))]
+fn list() {}
+
+// windows is a valid target family, should be ignored
+#[cfg(windows)]
+fn windows() {}
+
+// correct use, should be ignored
+#[cfg(target_os = "hermit")]
+fn correct() {}
+
+fn main() {}

tests/ui/mismatched_target_os_non_unix.stderr

@@ -0,0 +1,51 @@
+error: operating system used in target family position
+  --> $DIR/mismatched_target_os_non_unix.rs:6:1
+   |
+LL | #[cfg(cloudabi)]
+   | ^^^^^^--------^^
+   |       |
+   |       help: try: `target_os = "cloudabi"`
+   |
+   = note: `-D clippy::mismatched-target-os` implied by `-D warnings`
+
+error: operating system used in target family position
+  --> $DIR/mismatched_target_os_non_unix.rs:9:1
+   |
+LL | #[cfg(hermit)]
+   | ^^^^^^------^^
+   |       |
+   |       help: try: `target_os = "hermit"`
+
+error: operating system used in target family position
+  --> $DIR/mismatched_target_os_non_unix.rs:12:1
+   |
+LL | #[cfg(wasi)]
+   | ^^^^^^----^^
+   |       |
+   |       help: try: `target_os = "wasi"`
+
+error: operating system used in target family position
+  --> $DIR/mismatched_target_os_non_unix.rs:15:1
+   |
+LL | #[cfg(none)]
+   | ^^^^^^----^^
+   |       |
+   |       help: try: `target_os = "none"`
+
+error: operating system used in target family position
+  --> $DIR/mismatched_target_os_non_unix.rs:19:1
+   |
+LL | #[cfg(all(not(any(windows, cloudabi)), wasi))]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+help: try
+   |
+LL | #[cfg(all(not(any(windows, target_os = "cloudabi")), wasi))]
+   |                            ^^^^^^^^^^^^^^^^^^^^^^
+help: try
+   |
+LL | #[cfg(all(not(any(windows, cloudabi)), target_os = "wasi"))]
+   |                                        ^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 5 previous errors
+