Skip to content

Commit

Permalink
Implement mismatched_target_os lint
Browse files Browse the repository at this point in the history
  • Loading branch information
ebroto committed Apr 22, 2020
1 parent 1d4dd3d commit dfa13ab
Show file tree
Hide file tree
Showing 8 changed files with 308 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1361,6 +1361,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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 362 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are over 350 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
157 changes: 130 additions & 27 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,30 @@ use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol;
use semver::Version;

// NOTE: windows is excluded from the list because it's also a valid target family.
static OPERATING_SYSTEMS: &[&str] = &[
"android",
"cloudabi",
"dragonfly",
"emscripten",
"freebsd",
"fuchsia",
"haiku",
"hermit",
"illumos",
"ios",
"l4re",
"linux",
"macos",
"netbsd",
"none",
"openbsd",
"redox",
"solaris",
"vxworks",
"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 +213,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,35 +552,82 @@ 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_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 let Some(ident) = meta.ident() {
let name = &*ident.name.as_str();
if let Some(os) = OPERATING_SYSTEMS.iter().find(|&&os| os == name) {
mismatched.push((os, ident.span));
}
}
},
_ => {},
}
}
}
mismatched
}

if_chain! {
if attr.check_name(sym!(cfg));
if let Some(list) = attr.meta_item_list();
then {
let mismatched = find_mismatched_target_os(&list);
for (os, span) in mismatched {
let mess = format!("`{}` is not a valid target family", os);
let sugg = format!("target_os = \"{}\"", os);

span_lint_and_then(cx, MISMATCHED_TARGET_OS, span, &mess, |diag| {
diag.span_suggestion(span, "try", sugg, Applicability::MaybeIncorrect);
diag.help("Did you mean `unix`?");
});
}
}
}
Expand Down
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 @@ -1189,6 +1190,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 @@ -1611,6 +1613,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
41 changes: 41 additions & 0 deletions tests/ui/mismatched_target_os.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// run-rustfix

#![warn(clippy::mismatched_target_os)]
#![allow(unused)]

#[cfg(target_os = "linux")]
fn linux() {}

#[cfg(target_os = "freebsd")]
fn freebsd() {}

#[cfg(target_os = "dragonfly")]
fn dragonfly() {}

#[cfg(target_os = "openbsd")]
fn openbsd() {}

#[cfg(target_os = "netbsd")]
fn netbsd() {}

#[cfg(target_os = "macos")]
fn macos() {}

#[cfg(target_os = "ios")]
fn ios() {}

#[cfg(target_os = "android")]
fn android() {}

#[cfg(all(not(any(windows, target_os = "linux")), target_os = "freebsd"))]
fn list() {}

// windows is a valid target family, should be ignored
#[cfg(windows)]
fn windows() {}

// correct use, should be ignored
#[cfg(target_os = "freebsd")]
fn freebsd() {}

fn main() {}
41 changes: 41 additions & 0 deletions tests/ui/mismatched_target_os.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
// run-rustfix

#![warn(clippy::mismatched_target_os)]
#![allow(unused)]

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

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

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

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

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

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

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

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

#[cfg(all(not(any(windows, linux)), freebsd))]
fn list() {}

// windows is a valid target family, should be ignored
#[cfg(windows)]
fn windows() {}

// correct use, should be ignored
#[cfg(target_os = "freebsd")]
fn freebsd() {}

fn main() {}
Loading

0 comments on commit dfa13ab

Please sign in to comment.