Skip to content

Commit

Permalink
Auto merge of #5506 - ebroto:mismatched_target_os, r=flip1995
Browse files Browse the repository at this point in the history
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 #3949
  • Loading branch information
bors committed Apr 27, 2020
2 parents 87a6f3f + 303e7d1 commit 5d8a3e8
Show file tree
Hide file tree
Showing 10 changed files with 582 additions and 28 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
179 changes: 152 additions & 27 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
}
}
});
}
}
}
5 changes: 4 additions & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
7 changes: 7 additions & 0 deletions src/lintlist/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
30 changes: 30 additions & 0 deletions tests/ui/mismatched_target_os_non_unix.fixed
Original file line number Diff line number Diff line change
@@ -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() {}
30 changes: 30 additions & 0 deletions tests/ui/mismatched_target_os_non_unix.rs
Original file line number Diff line number Diff line change
@@ -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() {}
51 changes: 51 additions & 0 deletions tests/ui/mismatched_target_os_non_unix.stderr
Original file line number Diff line number Diff line change
@@ -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

Loading

0 comments on commit 5d8a3e8

Please sign in to comment.