Apply suggestions from PR review

- 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
This commit is contained in:
Eduardo Broto 2020-04-25 20:55:46 +02:00
parent 149f6d6046
commit d24a106395
4 changed files with 234 additions and 42 deletions

View File

@ -20,30 +20,28 @@ use rustc_span::source_map::Span;
use rustc_span::symbol::Symbol; use rustc_span::symbol::Symbol;
use semver::Version; use semver::Version;
// NOTE: windows is excluded from the list because it's also a valid target family. static UNIX_SYSTEMS: &[&str] = &[
static OPERATING_SYSTEMS: &[&str] = &[
"android", "android",
"cloudabi",
"dragonfly", "dragonfly",
"emscripten", "emscripten",
"freebsd", "freebsd",
"fuchsia", "fuchsia",
"haiku", "haiku",
"hermit",
"illumos", "illumos",
"ios", "ios",
"l4re", "l4re",
"linux", "linux",
"macos", "macos",
"netbsd", "netbsd",
"none",
"openbsd", "openbsd",
"redox", "redox",
"solaris", "solaris",
"vxworks", "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! { declare_clippy_lint! {
/// **What it does:** Checks for items annotated with `#[inline(always)]`, /// **What it does:** Checks for items annotated with `#[inline(always)]`,
/// unless the annotated function is empty or simply panics. /// unless the annotated function is empty or simply panics.
@ -592,8 +590,21 @@ fn check_deprecated_cfg_attr(cx: &EarlyContext<'_>, attr: &Attribute) {
} }
fn check_mismatched_target_os(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)> { fn find_mismatched_target_os(items: &[NestedMetaItem]) -> Vec<(&str, Span)> {
let mut mismatched = Vec::new(); let mut mismatched = Vec::new();
for item in items { for item in items {
if let NestedMetaItem::MetaItem(meta) = item { if let NestedMetaItem::MetaItem(meta) = item {
match &meta.kind { match &meta.kind {
@ -601,9 +612,10 @@ fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
mismatched.extend(find_mismatched_target_os(&list)); mismatched.extend(find_mismatched_target_os(&list));
}, },
MetaItemKind::Word => { MetaItemKind::Word => {
if let Some(ident) = meta.ident() { if_chain! {
let name = &*ident.name.as_str(); if let Some(ident) = meta.ident();
if let Some(os) = OPERATING_SYSTEMS.iter().find(|&&os| os == name) { if let Some(os) = find_os(&*ident.name.as_str());
then {
mismatched.push((os, ident.span)); mismatched.push((os, ident.span));
} }
} }
@ -612,23 +624,33 @@ fn check_mismatched_target_os(cx: &EarlyContext<'_>, attr: &Attribute) {
} }
} }
} }
mismatched mismatched
} }
if_chain! { if_chain! {
if attr.check_name(sym!(cfg)); if attr.check_name(sym!(cfg));
if let Some(list) = attr.meta_item_list(); if let Some(list) = attr.meta_item_list();
let mismatched = find_mismatched_target_os(&list);
if let Some((_, span)) = mismatched.iter().peekable().peek();
then { then {
let mismatched = find_mismatched_target_os(&list); let mess = "operating system used in target family position";
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| { 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.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;
}
}
});
} }
} }
} }

View File

@ -3,6 +3,8 @@
#![warn(clippy::mismatched_target_os)] #![warn(clippy::mismatched_target_os)]
#![allow(unused)] #![allow(unused)]
// unix
#[cfg(target_os = "linux")] #[cfg(target_os = "linux")]
fn linux() {} fn linux() {}
@ -27,6 +29,45 @@ fn ios() {}
#[cfg(target_os = "android")] #[cfg(target_os = "android")]
fn 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"))] #[cfg(all(not(any(windows, target_os = "linux")), target_os = "freebsd"))]
fn list() {} fn list() {}

View File

@ -3,6 +3,8 @@
#![warn(clippy::mismatched_target_os)] #![warn(clippy::mismatched_target_os)]
#![allow(unused)] #![allow(unused)]
// unix
#[cfg(linux)] #[cfg(linux)]
fn linux() {} fn linux() {}
@ -27,6 +29,45 @@ fn ios() {}
#[cfg(android)] #[cfg(android)]
fn 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))] #[cfg(all(not(any(windows, linux)), freebsd))]
fn list() {} fn list() {}

View File

@ -1,5 +1,5 @@
error: `linux` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:6:7 --> $DIR/mismatched_target_os.rs:8:7
| |
LL | #[cfg(linux)] LL | #[cfg(linux)]
| ^^^^^ help: try: `target_os = "linux"` | ^^^^^ help: try: `target_os = "linux"`
@ -7,77 +7,165 @@ LL | #[cfg(linux)]
= note: `-D clippy::mismatched-target-os` implied by `-D warnings` = note: `-D clippy::mismatched-target-os` implied by `-D warnings`
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: `freebsd` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:9:7 --> $DIR/mismatched_target_os.rs:11:7
| |
LL | #[cfg(freebsd)] LL | #[cfg(freebsd)]
| ^^^^^^^ help: try: `target_os = "freebsd"` | ^^^^^^^ help: try: `target_os = "freebsd"`
| |
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: `dragonfly` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:12:7 --> $DIR/mismatched_target_os.rs:14:7
| |
LL | #[cfg(dragonfly)] LL | #[cfg(dragonfly)]
| ^^^^^^^^^ help: try: `target_os = "dragonfly"` | ^^^^^^^^^ help: try: `target_os = "dragonfly"`
| |
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: `openbsd` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:15:7 --> $DIR/mismatched_target_os.rs:17:7
| |
LL | #[cfg(openbsd)] LL | #[cfg(openbsd)]
| ^^^^^^^ help: try: `target_os = "openbsd"` | ^^^^^^^ help: try: `target_os = "openbsd"`
| |
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: `netbsd` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:18:7 --> $DIR/mismatched_target_os.rs:20:7
| |
LL | #[cfg(netbsd)] LL | #[cfg(netbsd)]
| ^^^^^^ help: try: `target_os = "netbsd"` | ^^^^^^ help: try: `target_os = "netbsd"`
| |
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: `macos` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:21:7 --> $DIR/mismatched_target_os.rs:23:7
| |
LL | #[cfg(macos)] LL | #[cfg(macos)]
| ^^^^^ help: try: `target_os = "macos"` | ^^^^^ help: try: `target_os = "macos"`
| |
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: `ios` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:24:7 --> $DIR/mismatched_target_os.rs:26:7
| |
LL | #[cfg(ios)] LL | #[cfg(ios)]
| ^^^ help: try: `target_os = "ios"` | ^^^ help: try: `target_os = "ios"`
| |
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: `android` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:27:7 --> $DIR/mismatched_target_os.rs:29:7
| |
LL | #[cfg(android)] LL | #[cfg(android)]
| ^^^^^^^ help: try: `target_os = "android"` | ^^^^^^^ help: try: `target_os = "android"`
| |
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: `linux` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:30:28 --> $DIR/mismatched_target_os.rs:32:7
| |
LL | #[cfg(all(not(any(windows, linux)), freebsd))] LL | #[cfg(emscripten)]
| ^^^^^ help: try: `target_os = "linux"` | ^^^^^^^^^^ help: try: `target_os = "emscripten"`
| |
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: `freebsd` is not a valid target family error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:30:37 --> $DIR/mismatched_target_os.rs:35:7
| |
LL | #[cfg(all(not(any(windows, linux)), freebsd))] LL | #[cfg(fuchsia)]
| ^^^^^^^ help: try: `target_os = "freebsd"` | ^^^^^^^ help: try: `target_os = "fuchsia"`
| |
= help: Did you mean `unix`? = help: Did you mean `unix`?
error: aborting due to 10 previous errors error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:38:7
|
LL | #[cfg(haiku)]
| ^^^^^ help: try: `target_os = "haiku"`
|
= help: Did you mean `unix`?
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:41:7
|
LL | #[cfg(illumos)]
| ^^^^^^^ help: try: `target_os = "illumos"`
|
= help: Did you mean `unix`?
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:44:7
|
LL | #[cfg(l4re)]
| ^^^^ help: try: `target_os = "l4re"`
|
= help: Did you mean `unix`?
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:47:7
|
LL | #[cfg(redox)]
| ^^^^^ help: try: `target_os = "redox"`
|
= help: Did you mean `unix`?
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:50:7
|
LL | #[cfg(solaris)]
| ^^^^^^^ help: try: `target_os = "solaris"`
|
= help: Did you mean `unix`?
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:53:7
|
LL | #[cfg(vxworks)]
| ^^^^^^^ help: try: `target_os = "vxworks"`
|
= help: Did you mean `unix`?
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:58:7
|
LL | #[cfg(cloudabi)]
| ^^^^^^^^ help: try: `target_os = "cloudabi"`
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:61:7
|
LL | #[cfg(hermit)]
| ^^^^^^ help: try: `target_os = "hermit"`
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:64:7
|
LL | #[cfg(wasi)]
| ^^^^ help: try: `target_os = "wasi"`
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:67:7
|
LL | #[cfg(none)]
| ^^^^ help: try: `target_os = "none"`
error: operating system used in target family position
--> $DIR/mismatched_target_os.rs:71:28
|
LL | #[cfg(all(not(any(windows, linux)), freebsd))]
| ^^^^^
|
= help: Did you mean `unix`?
help: try
|
LL | #[cfg(all(not(any(windows, target_os = "linux")), freebsd))]
| ^^^^^^^^^^^^^^^^^^^
help: try
|
LL | #[cfg(all(not(any(windows, linux)), target_os = "freebsd"))]
| ^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 21 previous errors