diff --git a/crates/resolver-tests/src/lib.rs b/crates/resolver-tests/src/lib.rs index b985aebaaf7..b32dfc04330 100644 --- a/crates/resolver-tests/src/lib.rs +++ b/crates/resolver-tests/src/lib.rs @@ -24,7 +24,7 @@ use proptest::string::string_regex; use varisat::{self, ExtendFormula}; pub fn resolve(deps: Vec, registry: &[Summary]) -> CargoResult> { - resolve_with_config(deps, registry, None) + resolve_with_config(deps, registry, &Config::default().unwrap()) } pub fn resolve_and_validated( @@ -32,7 +32,7 @@ pub fn resolve_and_validated( registry: &[Summary], sat_resolve: Option, ) -> CargoResult> { - let resolve = resolve_with_config_raw(deps.clone(), registry, None); + let resolve = resolve_with_config_raw(deps.clone(), registry, &Config::default().unwrap()); match resolve { Err(e) => { @@ -109,7 +109,7 @@ pub fn resolve_and_validated( pub fn resolve_with_config( deps: Vec, registry: &[Summary], - config: Option<&Config>, + config: &Config, ) -> CargoResult> { let resolve = resolve_with_config_raw(deps, registry, config)?; Ok(resolve.sort()) @@ -118,7 +118,7 @@ pub fn resolve_with_config( pub fn resolve_with_config_raw( deps: Vec, registry: &[Summary], - config: Option<&Config>, + config: &Config, ) -> CargoResult { struct MyRegistry<'a> { list: &'a [Summary], @@ -170,7 +170,14 @@ pub fn resolve_with_config_raw( list: registry, used: HashSet::new(), }; - let summary = Summary::new(pkg_id("root"), deps, &BTreeMap::new(), None::<&String>).unwrap(); + let summary = Summary::new( + config, + pkg_id("root"), + deps, + &BTreeMap::new(), + None::<&String>, + ) + .unwrap(); let opts = ResolveOpts::everything(); let start = Instant::now(); let resolve = resolver::resolve( @@ -178,7 +185,7 @@ pub fn resolve_with_config_raw( &[], &mut registry, &HashSet::new(), - config, + Some(config), true, ); @@ -564,7 +571,14 @@ pub fn pkg_dep(name: T, dep: Vec) -> Summary { } else { None }; - Summary::new(name.to_pkgid(), dep, &BTreeMap::new(), link).unwrap() + Summary::new( + &Config::default().unwrap(), + name.to_pkgid(), + dep, + &BTreeMap::new(), + link, + ) + .unwrap() } pub fn pkg_id(name: &str) -> PackageId { @@ -585,7 +599,14 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary { } else { None }; - Summary::new(pkg_id_loc(name, loc), Vec::new(), &BTreeMap::new(), link).unwrap() + Summary::new( + &Config::default().unwrap(), + pkg_id_loc(name, loc), + Vec::new(), + &BTreeMap::new(), + link, + ) + .unwrap() } pub fn remove_dep(sum: &Summary, ind: usize) -> Summary { @@ -593,6 +614,7 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary { deps.remove(ind); // note: more things will need to be copied over in the future, but it works for now. Summary::new( + &Config::default().unwrap(), sum.package_id(), deps, &BTreeMap::new(), diff --git a/crates/resolver-tests/tests/resolve.rs b/crates/resolver-tests/tests/resolve.rs index c2ee408f485..dfa78ac11e1 100644 --- a/crates/resolver-tests/tests/resolve.rs +++ b/crates/resolver-tests/tests/resolve.rs @@ -87,7 +87,7 @@ proptest! { let mres = resolve_with_config( vec![dep_req(&this.name(), &format!("={}", this.version()))], ®, - Some(&config), + &config, ); prop_assert_eq!( @@ -584,7 +584,7 @@ fn test_resolving_minimum_version_with_transitive_deps() { let res = resolve_with_config( vec![dep_req("foo", "1.0.0"), dep_req("bar", "1.0.0")], ®, - Some(&config), + &config, ) .unwrap(); diff --git a/src/cargo/core/summary.rs b/src/cargo/core/summary.rs index c6d453f7c42..8c564ed18ae 100644 --- a/src/cargo/core/summary.rs +++ b/src/cargo/core/summary.rs @@ -1,6 +1,6 @@ use crate::core::{Dependency, PackageId, SourceId}; use crate::util::interning::InternedString; -use crate::util::CargoResult; +use crate::util::{CargoResult, Config}; use anyhow::bail; use semver::Version; use std::collections::{BTreeMap, HashMap, HashSet}; @@ -31,6 +31,7 @@ struct Inner { impl Summary { pub fn new( + config: &Config, pkg_id: PackageId, dependencies: Vec, features: &BTreeMap>, @@ -49,7 +50,8 @@ impl Summary { ) } } - let (feature_map, has_namespaced_features) = build_feature_map(features, &dependencies)?; + let (feature_map, has_namespaced_features) = + build_feature_map(config, pkg_id, features, &dependencies)?; Ok(Summary { inner: Rc::new(Inner { package_id: pkg_id, @@ -161,6 +163,8 @@ impl Hash for Summary { /// included a `dep:` prefixed namespaced feature (used for gating on /// nightly). fn build_feature_map( + config: &Config, + pkg_id: PackageId, features: &BTreeMap>, dependencies: &[Dependency], ) -> CargoResult<(FeatureMap, bool)> { @@ -223,6 +227,7 @@ fn build_feature_map( feature ); } + validate_feature_name(config, pkg_id, feature)?; for fv in fvs { // Find data for the referenced dependency... let dep_data = { @@ -400,3 +405,34 @@ impl fmt::Display for FeatureValue { } pub type FeatureMap = BTreeMap>; + +fn validate_feature_name(config: &Config, pkg_id: PackageId, name: &str) -> CargoResult<()> { + let mut chars = name.chars(); + const FUTURE: &str = "This was previously accepted but is being phased out; \ + it will become a hard error in a future release.\n\ + For more information, see issue #8813 , \ + and please leave a comment if this will be a problem for your project."; + if let Some(ch) = chars.next() { + if !(unicode_xid::UnicodeXID::is_xid_start(ch) || ch == '_' || ch.is_digit(10)) { + config.shell().warn(&format!( + "invalid character `{}` in feature `{}` in package {}, \ + the first character must be a Unicode XID start character or digit \ + (most letters or `_` or `0` to `9`)\n\ + {}", + ch, name, pkg_id, FUTURE + ))?; + } + } + for ch in chars { + if !(unicode_xid::UnicodeXID::is_xid_continue(ch) || ch == '-' || ch == '+') { + config.shell().warn(&format!( + "invalid character `{}` in feature `{}` in package {}, \ + characters must be Unicode XID characters or `+` \ + (numbers, `+`, `-`, `_`, or most letters)\n\ + {}", + ch, name, pkg_id, FUTURE + ))?; + } + } + Ok(()) +} diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 5fe8d0352d7..cc611fb970e 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -270,6 +270,7 @@ impl<'cfg> RegistryIndex<'cfg> { 'a: 'b, { let source_id = self.source_id; + let config = self.config; let namespaced_features = self.config.cli_unstable().namespaced_features; // First up actually parse what summaries we have available. If Cargo @@ -289,13 +290,15 @@ impl<'cfg> RegistryIndex<'cfg> { .versions .iter_mut() .filter_map(move |(k, v)| if req.matches(k) { Some(v) } else { None }) - .filter_map(move |maybe| match maybe.parse(raw_data, source_id) { - Ok(summary) => Some(summary), - Err(e) => { - info!("failed to parse `{}` registry package: {}", name, e); - None - } - }) + .filter_map( + move |maybe| match maybe.parse(config, raw_data, source_id) { + Ok(summary) => Some(summary), + Err(e) => { + info!("failed to parse `{}` registry package: {}", name, e); + None + } + }, + ) .filter(move |is| is.summary.unstable_gate(namespaced_features).is_ok())) } @@ -521,7 +524,7 @@ impl Summaries { // allow future cargo implementations to break the // interpretation of each line here and older cargo will simply // ignore the new lines. - let summary = match IndexSummary::parse(line, source_id) { + let summary = match IndexSummary::parse(config, line, source_id) { Ok(summary) => summary, Err(e) => { log::info!("failed to parse {:?} registry package: {}", relative, e); @@ -684,12 +687,17 @@ impl MaybeIndexSummary { /// Does nothing if this is already `Parsed`, and otherwise the `raw_data` /// passed in is sliced with the bounds in `Unparsed` and then actually /// parsed. - fn parse(&mut self, raw_data: &[u8], source_id: SourceId) -> CargoResult<&IndexSummary> { + fn parse( + &mut self, + config: &Config, + raw_data: &[u8], + source_id: SourceId, + ) -> CargoResult<&IndexSummary> { let (start, end) = match self { MaybeIndexSummary::Unparsed { start, end } => (*start, *end), MaybeIndexSummary::Parsed(summary) => return Ok(summary), }; - let summary = IndexSummary::parse(&raw_data[start..end], source_id)?; + let summary = IndexSummary::parse(config, &raw_data[start..end], source_id)?; *self = MaybeIndexSummary::Parsed(summary); match self { MaybeIndexSummary::Unparsed { .. } => unreachable!(), @@ -709,7 +717,7 @@ impl IndexSummary { /// a package. /// /// The `line` provided is expected to be valid JSON. - fn parse(line: &[u8], source_id: SourceId) -> CargoResult { + fn parse(config: &Config, line: &[u8], source_id: SourceId) -> CargoResult { let RegistryPackage { name, vers, @@ -725,7 +733,7 @@ impl IndexSummary { .into_iter() .map(|dep| dep.into_dep(source_id)) .collect::>>()?; - let mut summary = Summary::new(pkgid, deps, &features, links)?; + let mut summary = Summary::new(config, pkgid, deps, &features, links)?; summary.set_checksum(cksum); Ok(IndexSummary { summary, diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 0b99dc80a23..49a58edaaa9 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1191,6 +1191,7 @@ impl TomlManifest { let empty_features = BTreeMap::new(); let summary = Summary::new( + config, pkgid, deps, me.features.as_ref().unwrap_or(&empty_features), diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index 0097a6e0c8d..9c8713450fb 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -1974,3 +1974,122 @@ fn nonexistent_required_features() { ) .run(); } + +#[cargo_test] +fn invalid_feature_names() { + // Warnings for more restricted feature syntax. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.1.0" + + [features] + # Some valid, but unusual names, shouldn't warn. + "c++17" = [] + "128bit" = [] + "_foo" = [] + "feat-name" = [] + "feat_name" = [] + + # Invalid names. + "+foo" = [] + "-foo" = [] + ".foo" = [] + "foo/bar" = [] + "foo:bar" = [] + "foo.bar" = [] + "foo?" = [] + "?foo" = [] + "ⒶⒷⒸ" = [] + "a¼" = [] + "#, + ) + .file("src/lib.rs", "") + .build(); + + // Unfortunately the warnings are duplicated due to the Summary being + // loaded twice (once in the Workspace, and once in PackageRegistry) and + // Cargo does not have a de-duplication system. This should probably be + // OK, since I'm not expecting this to affect anyone. + p.cargo("check") + .with_stderr("\ +[WARNING] invalid character `+` in feature `+foo` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `-` in feature `-foo` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `.` in feature `.foo` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `?` in feature `?foo` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `¼` in feature `a¼` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `.` in feature `foo.bar` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `/` in feature `foo/bar` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `:` in feature `foo:bar` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `?` in feature `foo?` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `Ⓐ` in feature `ⒶⒷⒸ` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `Ⓑ` in feature `ⒶⒷⒸ` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `Ⓒ` in feature `ⒶⒷⒸ` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `+` in feature `+foo` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `-` in feature `-foo` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `.` in feature `.foo` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `?` in feature `?foo` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `¼` in feature `a¼` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `.` in feature `foo.bar` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `/` in feature `foo/bar` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `:` in feature `foo:bar` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `?` in feature `foo?` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `Ⓐ` in feature `ⒶⒷⒸ` in package foo v0.1.0 ([ROOT]/foo), the first character must be a Unicode XID start character or digit (most letters or `_` or `0` to `9`) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `Ⓑ` in feature `ⒶⒷⒸ` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[WARNING] invalid character `Ⓒ` in feature `ⒶⒷⒸ` in package foo v0.1.0 ([ROOT]/foo), characters must be Unicode XID characters or `+` (numbers, `+`, `-`, `_`, or most letters) +This was previously accepted but is being phased out; it will become a hard error in a future release. +For more information, see issue #8813 , and please leave a comment if this will be a problem for your project. +[CHECKING] foo v0.1.0 [..] +[FINISHED] [..] +") + .run(); +}