Skip to content

Commit aa93f61

Browse files
benediktwernerbenediktwerner
committed
Warn instead of error
1 parent a572f0f commit aa93f61

File tree

6 files changed

+124
-84
lines changed

6 files changed

+124
-84
lines changed

‎crates/cargo-platform/src/cfg.rs

Lines changed: 1 addition & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::error::{ParseError, ParseErrorKind, ParseErrorKind::*};
1+
use crate::error::{ParseError, ParseErrorKind::*};
22
use std::fmt;
33
use std::iter;
44
use std::str::{self, FromStr};
@@ -41,21 +41,6 @@ struct Parser<'a> {
4141
t: Tokenizer<'a>,
4242
}
4343

44-
impl Cfg {
45-
pub(crate) fn validate_as_target(&self) -> Result<(), ParseErrorKind> {
46-
match self {
47-
Cfg::Name(name) => match name.as_str() {
48-
"test" | "debug_assertions" | "proc_macro" => Err(InvalidCfgName(name.to_string())),
49-
_ => Ok(()),
50-
},
51-
Cfg::KeyPair(name, _) => match name.as_str() {
52-
"feature" => Err(InvalidCfgKey(name.to_string())),
53-
_ => Ok(()),
54-
},
55-
}
56-
}
57-
}
58-
5944
impl FromStr for Cfg {
6045
type Err = ParseError;
6146

@@ -104,19 +89,6 @@ impl CfgExpr {
10489
CfgExpr::Value(ref e) => cfg.contains(e),
10590
}
10691
}
107-
108-
pub(crate) fn validate_as_target(&self) -> Result<(), ParseErrorKind> {
109-
match *self {
110-
CfgExpr::Not(ref e) => e.validate_as_target()?,
111-
CfgExpr::All(ref e) | CfgExpr::Any(ref e) => {
112-
for e in e {
113-
e.validate_as_target()?;
114-
}
115-
}
116-
CfgExpr::Value(ref e) => e.validate_as_target()?,
117-
}
118-
Ok(())
119-
}
12092
}
12193

12294
impl FromStr for CfgExpr {
@@ -345,39 +317,3 @@ impl<'a> Token<'a> {
345317
}
346318
}
347319
}
348-
349-
#[test]
350-
fn cfg_validate_as_target() {
351-
fn p(s: &str) -> CfgExpr {
352-
s.parse().unwrap()
353-
}
354-
355-
assert!(p("unix").validate_as_target().is_ok());
356-
assert!(p("windows").validate_as_target().is_ok());
357-
assert!(p("any(not(unix), windows)").validate_as_target().is_ok());
358-
assert!(p("foo").validate_as_target().is_ok());
359-
360-
assert!(p("target_arch = \"abc\"").validate_as_target().is_ok());
361-
assert!(p("target_feature = \"abc\"").validate_as_target().is_ok());
362-
assert!(p("target_os = \"abc\"").validate_as_target().is_ok());
363-
assert!(p("target_family = \"abc\"").validate_as_target().is_ok());
364-
assert!(p("target_env = \"abc\"").validate_as_target().is_ok());
365-
assert!(p("target_endian = \"abc\"").validate_as_target().is_ok());
366-
assert!(p("target_pointer_width = \"abc\"")
367-
.validate_as_target()
368-
.is_ok());
369-
assert!(p("target_vendor = \"abc\"").validate_as_target().is_ok());
370-
assert!(p("bar = \"def\"").validate_as_target().is_ok());
371-
372-
assert!(p("test").validate_as_target().is_err());
373-
assert!(p("debug_assertions").validate_as_target().is_err());
374-
assert!(p("proc_macro").validate_as_target().is_err());
375-
assert!(p("any(not(debug_assertions), windows)")
376-
.validate_as_target()
377-
.is_err());
378-
379-
assert!(p("feature = \"abc\"").validate_as_target().is_err());
380-
assert!(p("any(not(feature = \"def\"), target_arch = \"abc\")")
381-
.validate_as_target()
382-
.is_err());
383-
}

‎crates/cargo-platform/src/error.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ pub enum ParseErrorKind {
1717
IncompleteExpr(&'static str),
1818
UnterminatedExpression(String),
1919
InvalidTarget(String),
20-
InvalidCfgName(String),
21-
InvalidCfgKey(String),
2220

2321
#[doc(hidden)]
2422
__Nonexhaustive,
@@ -55,8 +53,6 @@ impl fmt::Display for ParseErrorKind {
5553
write!(f, "unexpected content `{}` found after cfg expression", s)
5654
}
5755
InvalidTarget(s) => write!(f, "invalid target specifier: {}", s),
58-
InvalidCfgName(name) => write!(f, "invalid name in target cfg: {}", name),
59-
InvalidCfgKey(name) => write!(f, "invalid key in target cfg: {}", name),
6056
__Nonexhaustive => unreachable!(),
6157
}
6258
}

‎crates/cargo-platform/src/lib.rs

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,48 @@ impl Platform {
6161
}
6262
Ok(())
6363
}
64+
65+
pub fn check_cfg_attributes(&self, warnings: &mut Vec<String>) {
66+
fn check_cfg_expr(expr: &CfgExpr, warnings: &mut Vec<String>) {
67+
match *expr {
68+
CfgExpr::Not(ref e) => check_cfg_expr(e, warnings),
69+
CfgExpr::All(ref e) | CfgExpr::Any(ref e) => {
70+
for e in e {
71+
check_cfg_expr(e, warnings);
72+
}
73+
}
74+
CfgExpr::Value(ref e) => match e {
75+
Cfg::Name(name) => match name.as_str() {
76+
"test" | "debug_assertions" | "proc_macro" =>
77+
warnings.push(format!(
78+
"Found `{}` in `target.'cfg(...)'.dependencies`. \
79+
This value is not supported for selecting dependencies \
80+
and will not work as expected. \
81+
To learn more visit \
82+
https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html#platform-specific-dependencies",
83+
name
84+
)),
85+
_ => (),
86+
},
87+
Cfg::KeyPair(name, _) => match name.as_str() {
88+
"feature" =>
89+
warnings.push(String::from(
90+
"Found `feature = ...` in `target.'cfg(...)'.dependencies`. \
91+
This key is not supported for selecting dependencies \
92+
and will not work as expected. \
93+
Use the [features] section instead: \
94+
https://doc.rust-lang.org/cargo/reference/manifest.html#the-features-section"
95+
)),
96+
_ => (),
97+
},
98+
}
99+
}
100+
}
101+
102+
if let Platform::Cfg(cfg) = self {
103+
check_cfg_expr(cfg, warnings);
104+
}
105+
}
64106
}
65107

66108
impl serde::Serialize for Platform {
@@ -88,10 +130,7 @@ impl FromStr for Platform {
88130
fn from_str(s: &str) -> Result<Platform, ParseError> {
89131
if s.starts_with("cfg(") && s.ends_with(')') {
90132
let s = &s[4..s.len() - 1];
91-
let cfg: CfgExpr = s.parse()?;
92-
cfg.validate_as_target()
93-
.map_err(|kind| ParseError::new(s, kind))?;
94-
Ok(Platform::Cfg(cfg))
133+
s.parse().map(Platform::Cfg)
95134
} else {
96135
Platform::validate_named_platform(s)?;
97136
Ok(Platform::Name(s.to_string()))

‎crates/cargo-platform/tests/test_cfg.rs

Lines changed: 73 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -155,16 +155,6 @@ fn bad_target_name() {
155155
"failed to parse `!foo` as a cfg expression: \
156156
invalid target specifier: unexpected character ! in target name",
157157
);
158-
bad::<Platform>(
159-
"cfg(debug_assertions)",
160-
"failed to parse `debug_assertions` as a cfg expression: \
161-
invalid name in target cfg: debug_assertions",
162-
);
163-
bad::<Platform>(
164-
"cfg(feature = \"abc\")",
165-
"failed to parse `feature = \"abc\"` as a cfg expression: \
166-
invalid key in target cfg: feature",
167-
);
168158
}
169159

170160
#[test]
@@ -186,3 +176,76 @@ fn round_trip_platform() {
186176
all(target_os = \"freebsd\", target_arch = \"x86_64\")))",
187177
);
188178
}
179+
180+
#[test]
181+
fn check_cfg_attributes() {
182+
fn ok(s: &str) {
183+
let p = Platform::Cfg(s.parse().unwrap());
184+
let mut warnings = Vec::new();
185+
p.check_cfg_attributes(&mut warnings);
186+
assert!(
187+
warnings.is_empty(),
188+
"Expected no warnings but got: {:?}",
189+
warnings,
190+
);
191+
}
192+
193+
fn warn(s: &str, names: &[&str]) {
194+
let p = Platform::Cfg(s.parse().unwrap());
195+
let mut warnings = Vec::new();
196+
p.check_cfg_attributes(&mut warnings);
197+
assert_eq!(
198+
warnings.len(),
199+
names.len(),
200+
"Expecter warnings about {:?} but got {:?}",
201+
names,
202+
warnings,
203+
);
204+
for (name, warning) in names.iter().zip(warnings.iter()) {
205+
assert!(
206+
warning.contains(name),
207+
"Expected warning about '{}' but got: {}",
208+
name,
209+
warning,
210+
);
211+
}
212+
}
213+
214+
ok("unix");
215+
ok("windows");
216+
ok("any(not(unix), windows)");
217+
ok("foo");
218+
219+
ok("target_arch = \"abc\"");
220+
ok("target_feature = \"abc\"");
221+
ok("target_os = \"abc\"");
222+
ok("target_family = \"abc\"");
223+
ok("target_env = \"abc\"");
224+
ok("target_endian = \"abc\"");
225+
ok("target_pointer_width = \"abc\"");
226+
ok("target_vendor = \"abc\"");
227+
ok("bar = \"def\"");
228+
229+
warn("test", &["test"]);
230+
warn("debug_assertions", &["debug_assertions"]);
231+
warn("proc_macro", &["proc_macro"]);
232+
warn("feature = \"abc\"", &["feature"]);
233+
234+
warn("any(not(debug_assertions), windows)", &["debug_assertions"]);
235+
warn(
236+
"any(not(feature = \"def\"), target_arch = \"abc\")",
237+
&["feature"],
238+
);
239+
warn(
240+
"any(not(target_os = \"windows\"), proc_macro)",
241+
&["proc_macro"],
242+
);
243+
warn(
244+
"any(not(feature = \"windows\"), proc_macro)",
245+
&["feature", "proc_macro"],
246+
);
247+
warn(
248+
"all(not(debug_assertions), any(windows, proc_macro))",
249+
&["debug_assertions", "proc_macro"],
250+
);
251+
}

‎src/cargo/util/toml/mod.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1085,7 +1085,11 @@ impl TomlManifest {
10851085
process_dependencies(&mut cx, build_deps, Some(Kind::Build))?;
10861086

10871087
for (name, platform) in me.target.iter().flatten() {
1088-
cx.platform = Some(name.parse()?);
1088+
cx.platform = {
1089+
let platform: Platform = name.parse()?;
1090+
platform.check_cfg_attributes(&mut cx.warnings);
1091+
Some(platform)
1092+
};
10891093
process_dependencies(&mut cx, platform.dependencies.as_ref(), None)?;
10901094
let build_deps = platform
10911095
.build_dependencies

‎src/doc/src/reference/specifying-dependencies.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -477,6 +477,8 @@ Use [the `[features]` section](manifest.md#the-features-section)
477477
instead.
478478

479479
The same applies to `cfg(debug_assertions)`, `cfg(test)` and `cfg(prog_macro)`.
480+
These values will not work as expected and will always have the default value
481+
returned by `rustc --print=cfg`.
480482
There is currently no way to add dependencies based on these configuration values.
481483

482484
In addition to `#[cfg]` syntax, Cargo also supports listing out the full target

0 commit comments

Comments
 (0)