diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 906ff3d431f..06f7f881947 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -41,7 +41,7 @@ pub struct TomlManifest { pub package: Option>, pub project: Option>, pub badges: Option>>, - pub features: Option>>, + pub features: Option>, pub lib: Option, pub bin: Option>, pub example: Option>, @@ -110,7 +110,7 @@ impl TomlManifest { .or(self.build_dependencies2.as_ref()) } - pub fn features(&self) -> Option<&BTreeMap>> { + pub fn features(&self) -> Option<&BTreeMap> { self.features.as_ref() } @@ -1521,6 +1521,63 @@ impl TomlPlatform { } } +/// Definition of a feature. +#[derive(Clone, Debug, Serialize)] +#[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))] +#[serde(untagged)] +pub enum FeatureDefinition { + /// Features that this feature enables. + Array(Vec), + /// Metadata of this feature. + Metadata(FeatureMetadata), +} + +// Implementing `Deserialize` manually allows for a better error message when the `enables` key is +// missing. +impl<'de> de::Deserialize<'de> for FeatureDefinition { + fn deserialize(d: D) -> Result + where + D: de::Deserializer<'de>, + { + UntaggedEnumVisitor::new() + .seq(|seq| { + seq.deserialize::>() + .map(FeatureDefinition::Array) + }) + .map(|seq| { + seq.deserialize::() + .map(FeatureDefinition::Metadata) + }) + .deserialize(d) + } +} + +impl FeatureDefinition { + /// Returns the features that this feature enables. + pub fn enables(&self) -> &[String] { + match self { + Self::Array(features) => features, + Self::Metadata(FeatureMetadata { + enables: features, .. + }) => features, + } + } +} + +/// Metadata of a feature. +#[derive(Clone, Debug, Deserialize, Serialize)] +#[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))] +pub struct FeatureMetadata { + /// Features that this feature enables. + pub enables: Vec, + + /// This is here to provide a way to see the "unused manifest keys" when deserializing + #[serde(skip_serializing)] + #[serde(flatten)] + #[cfg_attr(feature = "unstable-schema", schemars(skip))] + pub _unused_keys: BTreeMap, +} + #[derive(Serialize, Debug, Clone)] #[cfg_attr(feature = "unstable-schema", derive(schemars::JsonSchema))] pub struct InheritableLints { diff --git a/src/cargo/ops/registry/publish.rs b/src/cargo/ops/registry/publish.rs index de0f9ac4e0a..5b4d5e01cff 100644 --- a/src/cargo/ops/registry/publish.rs +++ b/src/cargo/ops/registry/publish.rs @@ -539,7 +539,7 @@ pub(crate) fn prepare_transmit( .map(|(feat, values)| { ( feat.to_string(), - values.iter().map(|fv| fv.to_string()).collect(), + values.enables().iter().map(|fv| fv.to_string()).collect(), ) }) .collect::>>(), diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index bc80097b705..0f7074e42c2 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -11,7 +11,8 @@ use anyhow::{anyhow, bail, Context as _}; use cargo_platform::Platform; use cargo_util::paths; use cargo_util_schemas::manifest::{ - self, PackageName, PathBaseName, TomlDependency, TomlDetailedDependency, TomlManifest, + self, FeatureDefinition, FeatureMetadata, FeatureName, PackageName, PathBaseName, + TomlDependency, TomlDetailedDependency, TomlManifest, }; use cargo_util_schemas::manifest::{RustVersion, StringOrBool}; use itertools::Itertools; @@ -708,8 +709,8 @@ fn default_readme_from_package_root(package_root: &Path) -> Option { #[tracing::instrument(skip_all)] fn normalize_features( - original_features: Option<&BTreeMap>>, -) -> CargoResult>>> { + original_features: Option<&BTreeMap>, +) -> CargoResult>> { let Some(normalized_features) = original_features.cloned() else { return Ok(None); }; @@ -1296,6 +1297,8 @@ pub fn to_real_manifest( } } + validate_feature_definitions(original_toml.features.as_ref(), warnings)?; + validate_dependencies(original_toml.dependencies.as_ref(), None, None, warnings)?; validate_dependencies( original_toml.dev_dependencies(), @@ -1497,7 +1500,7 @@ pub fn to_real_manifest( .map(|(k, v)| { ( InternedString::new(k), - v.iter().map(InternedString::from).collect(), + v.enables().iter().map(InternedString::from).collect(), ) }) .collect(), @@ -1764,6 +1767,30 @@ fn to_virtual_manifest( Ok(manifest) } +fn validate_feature_definitions( + features: Option<&BTreeMap>, + warnings: &mut Vec, +) -> CargoResult<()> { + let Some(features) = features else { + return Ok(()); + }; + + for (feature, feature_definition) in features { + match feature_definition { + FeatureDefinition::Array(..) => {} + FeatureDefinition::Metadata(FeatureMetadata { _unused_keys, .. }) => { + warnings.extend( + _unused_keys + .keys() + .map(|k| format!("unused manifest key: `features.{feature}.{k}`")), + ); + } + } + } + + Ok(()) +} + #[tracing::instrument(skip_all)] fn validate_dependencies( original_deps: Option<&BTreeMap>, @@ -2902,16 +2929,23 @@ fn prepare_toml_for_publish( }; features.values_mut().for_each(|feature_deps| { - feature_deps.retain(|feature_dep| { - let feature_value = FeatureValue::new(InternedString::new(feature_dep)); - match feature_value { - FeatureValue::Dep { dep_name } | FeatureValue::DepFeature { dep_name, .. } => { - let k = &manifest::PackageName::new(dep_name.to_string()).unwrap(); - dep_name_set.contains(k) + let feature_array = feature_deps + .enables() + .iter() + .filter(|feature_dep| { + let feature_value = FeatureValue::new(InternedString::new(feature_dep)); + match feature_value { + FeatureValue::Dep { dep_name } + | FeatureValue::DepFeature { dep_name, .. } => { + let k = &manifest::PackageName::new(dep_name.to_string()).unwrap(); + dep_name_set.contains(k) + } + _ => true, } - _ => true, - } - }); + }) + .cloned() + .collect(); + *feature_deps = FeatureDefinition::Array(feature_array); }); } diff --git a/tests/testsuite/features.rs b/tests/testsuite/features.rs index c4a7611ffcf..a0636d24a13 100644 --- a/tests/testsuite/features.rs +++ b/tests/testsuite/features.rs @@ -2362,7 +2362,7 @@ fn unused_keys_in_feature_metadata() { edition = "2015" [features] - foo = { enables = ["bar"], foobar = false } + foo = { enables = ["bar"], a = false, b = true } bar = [] "#, ) @@ -2371,7 +2371,8 @@ fn unused_keys_in_feature_metadata() { p.cargo("check") .with_stderr_data(str![[r#" -[WARNING] unused manifest key: `features.foo.foobar` +[WARNING] unused manifest key: `features.foo.a` +[WARNING] unused manifest key: `features.foo.b` [CHECKING] foo v0.0.0 ([ROOT]/foo) [FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s @@ -2395,22 +2396,10 @@ fn normalize_feature_metadata() { c = { enables = ["a", "b"] } "#, ) - .file( - "src/main.rs", - r#" - #[cfg(feature = "a")] - fn a() {} - #[cfg(feature = "b")] - fn b() {} - fn main() { - a(); - b(); - } - "#, - ) + .file("src/main.rs", "") .build(); - p.cargo("package --no-verify -v").run(); + p.cargo("package --no-verify").run(); let f = File::open(&p.root().join("target/package/foo-0.0.0.crate")).unwrap(); let normalized_manifest = str![[r#" # THIS FILE IS AUTOMATICALLY GENERATED BY CARGO