rust
/
cargo
mirror of https://github.com/rust-lang/cargo
0
0
Fork 0
Code Issues Releases Wiki Activity

Warn instead of error

This commit is contained in:
Benedikt Werner 2019-12-07 09:10:06 +01:00
parent a572f0f274
commit aa93f61f21
No known key found for this signature in database
GPG Key ID: 1DBFF0F8E9E121EB
6 changed files with 124 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

66
crates/cargo-platform/src/cfg.rs
View File

@ -1,4 +1,4 @@
use crate::error::{ParseError, ParseErrorKind, ParseErrorKind::*};
use crate::error::{ParseError, ParseErrorKind::*};
use std::fmt;
use std::iter;
use std::str::{self, FromStr};
@ -41,21 +41,6 @@ struct Parser<'a> {
t: Tokenizer<'a>,
}
impl Cfg {
pub(crate) fn validate_as_target(&self) -> Result<(), ParseErrorKind> {
match self {
Cfg::Name(name) => match name.as_str() {
"test" | "debug_assertions" | "proc_macro" => Err(InvalidCfgName(name.to_string())),
_ => Ok(()),
},
Cfg::KeyPair(name, _) => match name.as_str() {
"feature" => Err(InvalidCfgKey(name.to_string())),
_ => Ok(()),
},
}
}
}
impl FromStr for Cfg {
type Err = ParseError;
@ -104,19 +89,6 @@ impl CfgExpr {
CfgExpr::Value(ref e) => cfg.contains(e),
}
}
pub(crate) fn validate_as_target(&self) -> Result<(), ParseErrorKind> {
match *self {
CfgExpr::Not(ref e) => e.validate_as_target()?,
CfgExpr::All(ref e) | CfgExpr::Any(ref e) => {
for e in e {
e.validate_as_target()?;
}
}
CfgExpr::Value(ref e) => e.validate_as_target()?,
}
Ok(())
}
}
impl FromStr for CfgExpr {
@ -345,39 +317,3 @@ impl<'a> Token<'a> {
}
}
}
#[test]
fn cfg_validate_as_target() {
fn p(s: &str) -> CfgExpr {
s.parse().unwrap()
}
assert!(p("unix").validate_as_target().is_ok());
assert!(p("windows").validate_as_target().is_ok());
assert!(p("any(not(unix), windows)").validate_as_target().is_ok());
assert!(p("foo").validate_as_target().is_ok());
assert!(p("target_arch = \"abc\"").validate_as_target().is_ok());
assert!(p("target_feature = \"abc\"").validate_as_target().is_ok());
assert!(p("target_os = \"abc\"").validate_as_target().is_ok());
assert!(p("target_family = \"abc\"").validate_as_target().is_ok());
assert!(p("target_env = \"abc\"").validate_as_target().is_ok());
assert!(p("target_endian = \"abc\"").validate_as_target().is_ok());
assert!(p("target_pointer_width = \"abc\"")
.validate_as_target()
.is_ok());
assert!(p("target_vendor = \"abc\"").validate_as_target().is_ok());
assert!(p("bar = \"def\"").validate_as_target().is_ok());
assert!(p("test").validate_as_target().is_err());
assert!(p("debug_assertions").validate_as_target().is_err());
assert!(p("proc_macro").validate_as_target().is_err());
assert!(p("any(not(debug_assertions), windows)")
.validate_as_target()
.is_err());
assert!(p("feature = \"abc\"").validate_as_target().is_err());
assert!(p("any(not(feature = \"def\"), target_arch = \"abc\")")
.validate_as_target()
.is_err());
}

4
crates/cargo-platform/src/error.rs
View File

@ -17,8 +17,6 @@ pub enum ParseErrorKind {
IncompleteExpr(&'static str),
UnterminatedExpression(String),
InvalidTarget(String),
InvalidCfgName(String),
InvalidCfgKey(String),
#[doc(hidden)]
__Nonexhaustive,
@ -55,8 +53,6 @@ impl fmt::Display for ParseErrorKind {
write!(f, "unexpected content `{}` found after cfg expression", s)
}
InvalidTarget(s) => write!(f, "invalid target specifier: {}", s),
InvalidCfgName(name) => write!(f, "invalid name in target cfg: {}", name),
InvalidCfgKey(name) => write!(f, "invalid key in target cfg: {}", name),
__Nonexhaustive => unreachable!(),
}
}

47
crates/cargo-platform/src/lib.rs
View File

@ -61,6 +61,48 @@ impl Platform {
}
Ok(())
}
pub fn check_cfg_attributes(&self, warnings: &mut Vec<String>) {
fn check_cfg_expr(expr: &CfgExpr, warnings: &mut Vec<String>) {
match *expr {
CfgExpr::Not(ref e) => check_cfg_expr(e, warnings),
CfgExpr::All(ref e) | CfgExpr::Any(ref e) => {
for e in e {
check_cfg_expr(e, warnings);
}
}
CfgExpr::Value(ref e) => match e {
Cfg::Name(name) => match name.as_str() {
"test" | "debug_assertions" | "proc_macro" =>
warnings.push(format!(
"Found `{}` in `target.'cfg(...)'.dependencies`. \
This value is not supported for selecting dependencies \
and will not work as expected. \
To learn more visit \
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies",
name
)),
_ => (),
},
Cfg::KeyPair(name, _) => match name.as_str() {
"feature" =>
warnings.push(String::from(
"Found `feature = ...` in `target.'cfg(...)'.dependencies`. \
This key is not supported for selecting dependencies \
and will not work as expected. \
Use the [features] section instead: \
https://doc.rust-lang.org/cargo/reference/manifest.html#the-features-section"
)),
_ => (),
},
}
}
}
if let Platform::Cfg(cfg) = self {
check_cfg_expr(cfg, warnings);
}
}
}
impl serde::Serialize for Platform {
@ -88,10 +130,7 @@ impl FromStr for Platform {
fn from_str(s: &str) -> Result<Platform, ParseError> {
if s.starts_with("cfg(") && s.ends_with(')') {
let s = &s[4..s.len() - 1];
let cfg: CfgExpr = s.parse()?;
cfg.validate_as_target()
.map_err(|kind| ParseError::new(s, kind))?;
Ok(Platform::Cfg(cfg))
s.parse().map(Platform::Cfg)
} else {
Platform::validate_named_platform(s)?;
Ok(Platform::Name(s.to_string()))

83
crates/cargo-platform/tests/test_cfg.rs
View File

@ -155,16 +155,6 @@ fn bad_target_name() {
"failed to parse `!foo` as a cfg expression: \
invalid target specifier: unexpected character ! in target name",
);
bad::<Platform>(
"cfg(debug_assertions)",
"failed to parse `debug_assertions` as a cfg expression: \
invalid name in target cfg: debug_assertions",
);
bad::<Platform>(
"cfg(feature = \"abc\")",
"failed to parse `feature = \"abc\"` as a cfg expression: \
invalid key in target cfg: feature",
);
}
#[test]
@ -186,3 +176,76 @@ fn round_trip_platform() {
all(target_os = \"freebsd\", target_arch = \"x86_64\")))",
);
}
#[test]
fn check_cfg_attributes() {
fn ok(s: &str) {
let p = Platform::Cfg(s.parse().unwrap());
let mut warnings = Vec::new();
p.check_cfg_attributes(&mut warnings);
assert!(
warnings.is_empty(),
"Expected no warnings but got: {:?}",
warnings,
);
}
fn warn(s: &str, names: &[&str]) {
let p = Platform::Cfg(s.parse().unwrap());
let mut warnings = Vec::new();
p.check_cfg_attributes(&mut warnings);
assert_eq!(
warnings.len(),
names.len(),
"Expecter warnings about {:?} but got {:?}",
names,
warnings,
);
for (name, warning) in names.iter().zip(warnings.iter()) {
assert!(
warning.contains(name),
"Expected warning about '{}' but got: {}",
name,
warning,
);
}
}
ok("unix");
ok("windows");
ok("any(not(unix), windows)");
ok("foo");
ok("target_arch = \"abc\"");
ok("target_feature = \"abc\"");
ok("target_os = \"abc\"");
ok("target_family = \"abc\"");
ok("target_env = \"abc\"");
ok("target_endian = \"abc\"");
ok("target_pointer_width = \"abc\"");
ok("target_vendor = \"abc\"");
ok("bar = \"def\"");
warn("test", &["test"]);
warn("debug_assertions", &["debug_assertions"]);
warn("proc_macro", &["proc_macro"]);
warn("feature = \"abc\"", &["feature"]);
warn("any(not(debug_assertions), windows)", &["debug_assertions"]);
warn(
"any(not(feature = \"def\"), target_arch = \"abc\")",
&["feature"],
);
warn(
"any(not(target_os = \"windows\"), proc_macro)",
&["proc_macro"],
);
warn(
"any(not(feature = \"windows\"), proc_macro)",
&["feature", "proc_macro"],
);
warn(
"all(not(debug_assertions), any(windows, proc_macro))",
&["debug_assertions", "proc_macro"],
);
}

6
src/cargo/util/toml/mod.rs
View File

@ -1085,7 +1085,11 @@ impl TomlManifest {
process_dependencies(&mut cx, build_deps, Some(Kind::Build))?;
for (name, platform) in me.target.iter().flatten() {
cx.platform = Some(name.parse()?);
cx.platform = {
let platform: Platform = name.parse()?;
platform.check_cfg_attributes(&mut cx.warnings);
Some(platform)
};
process_dependencies(&mut cx, platform.dependencies.as_ref(), None)?;
let build_deps = platform
.build_dependencies

2
src/doc/src/reference/specifying-dependencies.md
View File

@ -477,6 +477,8 @@ Use [the `[features]` section](manifest.md#the-features-section)
instead.
The same applies to `cfg(debug_assertions)`, `cfg(test)` and `cfg(prog_macro)`.
These values will not work as expected and will always have the default value
returned by `rustc --print=cfg`.
There is currently no way to add dependencies based on these configuration values.
In addition to `#[cfg]` syntax, Cargo also supports listing out the full target