Skip to content

Commit

Permalink
Apply suggestions from PR review
Browse files Browse the repository at this point in the history
- Show just one error message with multiple suggestions in case of
  using multiple times an OS in target family position
- Only suggest #[cfg(unix)] when the OS is in the Unix family
- Test all the operating systems
  • Loading branch information
ebroto committed Apr 25, 2020
1 parent 85a3fa0 commit be57434
Show file tree
Hide file tree
Showing 4 changed files with 233 additions and 41 deletions.
56 changes: 39 additions & 17 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,30 +20,28 @@ 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] = &[
static UNIX_SYSTEMS: &[&str] = &[
"android",
"cloudabi",
"dragonfly",
"emscripten",
"freebsd",
"fuchsia",
"haiku",
"hermit",
"illumos",
"ios",
"l4re",
"linux",
"macos",
"netbsd",
"none",
"openbsd",
"redox",
"solaris",
"vxworks",
"wasi",
];

// 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 @@ -592,18 +590,32 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute) {
}

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 let Some(ident) = meta.ident() {
let name = &*ident.name.as_str();
if let Some(os) = OPERATING_SYSTEMS.iter().find(|&&os| os == name) {
if_chain! {
if let Some(ident) = meta.ident();
if let Some(os) = find_os(&*ident.name.as_str());
then {
mismatched.push((os, ident.span));
}
}
Expand All @@ -612,23 +624,33 @@ fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
}
}
}

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 let Some((_, span)) = mismatched.iter().peekable().peek();
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);
let mess = "operating system used in target family position";

span_lint_and_then(cx, MISMATCHED_TARGET_OS, span, &mess, |diag| {
span_lint_and_then(cx, MISMATCHED_TARGET_OS, *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);
diag.help("Did you mean `unix`?");
});
}

if !unix_suggested && is_unix(os) {
diag.help("Did you mean `unix`?");
unix_suggested = true;
}
}
});
}
}
}
41 changes: 41 additions & 0 deletions tests/ui/mismatched_target_os.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#![warn(clippy::mismatched_target_os)]
#![allow(unused)]

// unix

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

Expand All @@ -27,6 +29,45 @@ fn ios() {}
#[cfg(target_os = "android")]
fn android() {}

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

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

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

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

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

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

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

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

// non-unix

#[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 = "linux")), target_os = "freebsd"))]
fn list() {}

Expand Down
41 changes: 41 additions & 0 deletions tests/ui/mismatched_target_os.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
#![warn(clippy::mismatched_target_os)]
#![allow(unused)]

// unix

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

Expand All @@ -27,6 +29,45 @@ fn ios() {}
#[cfg(android)]
fn android() {}

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

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

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

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

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

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

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

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

// non-unix

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

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

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

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

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

Expand Down
Loading

0 comments on commit be57434

Please sign in to comment.