From 0d9bcd7781cae932440802879d05da350facdd76 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 10 Nov 2023 15:58:23 -0600 Subject: [PATCH 1/5] refactor(toml): Switch from Workspace to Inherit terms `Workspace` and `MaybeWorkspace` doesn't make the intent as clear. Generally when talking about this, we say that it "inherits". This also better matches the terms in `cargo_toml` and `cargo-manifest` packages. --- src/cargo/util/toml/mod.rs | 100 ++++++++++---------- src/cargo/util/toml/schema.rs | 170 +++++++++++++++++----------------- 2 files changed, 135 insertions(+), 135 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 6d839328bdd..663eaea160f 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -204,7 +204,7 @@ impl schema::TomlManifest { package .edition .as_ref() - .and_then(|e| e.as_defined()) + .and_then(|e| e.as_value()) .map(|e| Edition::from_str(e)) .unwrap_or(Ok(Edition::Edition2015)) .map(|e| e.default_resolve_behavior()) @@ -218,14 +218,14 @@ impl schema::TomlManifest { } if let Some(license_file) = &package.license_file { let license_file = license_file - .as_defined() + .as_value() .context("license file should have been resolved before `prepare_for_publish()`")?; let license_path = Path::new(&license_file); let abs_license_path = paths::normalize_path(&package_root.join(license_path)); if abs_license_path.strip_prefix(package_root).is_err() { // This path points outside of the package root. `cargo package` // will copy it into the root, so adjust the path to this location. - package.license_file = Some(schema::MaybeWorkspace::Defined( + package.license_file = Some(schema::InheritableField::Value( license_path .file_name() .unwrap() @@ -238,7 +238,7 @@ impl schema::TomlManifest { if let Some(readme) = &package.readme { let readme = readme - .as_defined() + .as_value() .context("readme should have been resolved before `prepare_for_publish()`")?; match readme { schema::StringOrBool::String(readme) => { @@ -247,7 +247,7 @@ impl schema::TomlManifest { if abs_readme_path.strip_prefix(package_root).is_err() { // This path points outside of the package root. `cargo package` // will copy it into the root, so adjust the path to this location. - package.readme = Some(schema::MaybeWorkspace::Defined( + package.readme = Some(schema::InheritableField::Value( schema::StringOrBool::String( readme_path .file_name() @@ -317,14 +317,14 @@ impl schema::TomlManifest { fn map_deps( config: &Config, - deps: Option<&BTreeMap>, + deps: Option<&BTreeMap>, filter: impl Fn(&schema::TomlDependency) -> bool, - ) -> CargoResult>> { + ) -> CargoResult>> { let Some(deps) = deps else { return Ok(None) }; let deps = deps .iter() .filter(|(_k, v)| { - if let schema::MaybeWorkspace::Defined(def) = v { + if let schema::InheritableField::Value(def) = v { filter(def) } else { false @@ -337,10 +337,10 @@ impl schema::TomlManifest { fn map_dependency( config: &Config, - dep: &schema::MaybeWorkspaceDependency, - ) -> CargoResult { + dep: &schema::InheritableDependency, + ) -> CargoResult { let dep = match dep { - schema::MaybeWorkspace::Defined(schema::TomlDependency::Detailed(d)) => { + schema::InheritableField::Value(schema::TomlDependency::Detailed(d)) => { let mut d = d.clone(); // Path dependencies become crates.io deps. d.path.take(); @@ -355,7 +355,7 @@ impl schema::TomlManifest { } Ok(d) } - schema::MaybeWorkspace::Defined(schema::TomlDependency::Simple(s)) => { + schema::InheritableField::Value(schema::TomlDependency::Simple(s)) => { Ok(schema::TomlDetailedDependency { version: Some(s.clone()), ..Default::default() @@ -364,7 +364,7 @@ impl schema::TomlManifest { _ => unreachable!(), }; dep.map(schema::TomlDependency::Detailed) - .map(schema::MaybeWorkspace::Defined) + .map(schema::InheritableField::Value) } } @@ -503,7 +503,7 @@ impl schema::TomlManifest { .map(|version| version.resolve("version", || inherit()?.version())) .transpose()?; - package.version = version.clone().map(schema::MaybeWorkspace::Defined); + package.version = version.clone().map(schema::InheritableField::Value); let pkgid = package.to_package_id( source_id, @@ -517,7 +517,7 @@ impl schema::TomlManifest { .resolve("edition", || inherit()?.edition())? .parse() .with_context(|| "failed to parse the `edition` key")?; - package.edition = Some(schema::MaybeWorkspace::Defined(edition.to_string())); + package.edition = Some(schema::InheritableField::Value(edition.to_string())); edition } else { Edition::Edition2015 @@ -641,11 +641,11 @@ impl schema::TomlManifest { fn process_dependencies( cx: &mut Context<'_, '_>, - new_deps: Option<&BTreeMap>, + new_deps: Option<&BTreeMap>, kind: Option, workspace_config: &WorkspaceConfig, inherit_cell: &LazyCell, - ) -> CargoResult>> { + ) -> CargoResult>> { let Some(dependencies) = new_deps else { return Ok(None); }; @@ -656,7 +656,7 @@ impl schema::TomlManifest { }) }; - let mut deps: BTreeMap = BTreeMap::new(); + let mut deps: BTreeMap = BTreeMap::new(); for (n, v) in dependencies.iter() { let resolved = v .clone() @@ -677,7 +677,7 @@ impl schema::TomlManifest { cx.deps.push(dep); deps.insert( n.to_string(), - schema::MaybeWorkspace::Defined(resolved.clone()), + schema::InheritableField::Value(resolved.clone()), ); } Ok(Some(deps)) @@ -900,54 +900,54 @@ impl schema::TomlManifest { package.description = metadata .description .clone() - .map(|description| schema::MaybeWorkspace::Defined(description)); + .map(|description| schema::InheritableField::Value(description)); package.homepage = metadata .homepage .clone() - .map(|homepage| schema::MaybeWorkspace::Defined(homepage)); + .map(|homepage| schema::InheritableField::Value(homepage)); package.documentation = metadata .documentation .clone() - .map(|documentation| schema::MaybeWorkspace::Defined(documentation)); + .map(|documentation| schema::InheritableField::Value(documentation)); package.readme = metadata .readme .clone() - .map(|readme| schema::MaybeWorkspace::Defined(schema::StringOrBool::String(readme))); + .map(|readme| schema::InheritableField::Value(schema::StringOrBool::String(readme))); package.authors = package .authors .as_ref() - .map(|_| schema::MaybeWorkspace::Defined(metadata.authors.clone())); + .map(|_| schema::InheritableField::Value(metadata.authors.clone())); package.license = metadata .license .clone() - .map(|license| schema::MaybeWorkspace::Defined(license)); + .map(|license| schema::InheritableField::Value(license)); package.license_file = metadata .license_file .clone() - .map(|license_file| schema::MaybeWorkspace::Defined(license_file)); + .map(|license_file| schema::InheritableField::Value(license_file)); package.repository = metadata .repository .clone() - .map(|repository| schema::MaybeWorkspace::Defined(repository)); + .map(|repository| schema::InheritableField::Value(repository)); package.keywords = package .keywords .as_ref() - .map(|_| schema::MaybeWorkspace::Defined(metadata.keywords.clone())); + .map(|_| schema::InheritableField::Value(metadata.keywords.clone())); package.categories = package .categories .as_ref() - .map(|_| schema::MaybeWorkspace::Defined(metadata.categories.clone())); + .map(|_| schema::InheritableField::Value(metadata.categories.clone())); package.rust_version = rust_version .clone() - .map(|rv| schema::MaybeWorkspace::Defined(rv)); + .map(|rv| schema::InheritableField::Value(rv)); package.exclude = package .exclude .as_ref() - .map(|_| schema::MaybeWorkspace::Defined(exclude.clone())); + .map(|_| schema::InheritableField::Value(exclude.clone())); package.include = package .include .as_ref() - .map(|_| schema::MaybeWorkspace::Defined(include.clone())); + .map(|_| schema::InheritableField::Value(include.clone())); let profiles = me.profile.clone(); if let Some(profiles) = &profiles { @@ -960,7 +960,7 @@ impl schema::TomlManifest { .clone() .map(|publish| publish.resolve("publish", || inherit()?.publish()).unwrap()); - package.publish = publish.clone().map(|p| schema::MaybeWorkspace::Defined(p)); + package.publish = publish.clone().map(|p| schema::InheritableField::Value(p)); let publish = match publish { Some(schema::VecStringOrBool::VecString(ref vecstring)) => Some(vecstring.clone()), @@ -1029,8 +1029,8 @@ impl schema::TomlManifest { badges: me .badges .as_ref() - .map(|_| schema::MaybeWorkspace::Defined(metadata.badges.clone())), - lints: lints.map(|lints| schema::MaybeWorkspaceLints { + .map(|_| schema::InheritableField::Value(metadata.badges.clone())), + lints: lints.map(|lints| schema::InheritableLints { workspace: false, lints, }), @@ -1538,9 +1538,9 @@ impl schema::TomlPackage { } } -/// This Trait exists to make [`schema::MaybeWorkspace::Workspace`] generic. It makes deserialization of -/// [`schema::MaybeWorkspace`] much easier, as well as making error messages for -/// [`schema::MaybeWorkspace::resolve`] much nicer +/// This Trait exists to make [`schema::InheritableField::Inherit`] generic. It makes deserialization of +/// [`schema::InheritableField`] much easier, as well as making error messages for +/// [`schema::InheritableField::resolve`] much nicer /// /// Implementors should have a field `workspace` with the type of `bool`. It is used to ensure /// `workspace` is not `false` in a `Cargo.toml` @@ -1550,15 +1550,15 @@ pub trait WorkspaceInherit { fn inherit_toml_table(&self) -> &str; } -impl schema::MaybeWorkspace { +impl schema::InheritableField { fn resolve<'a>( self, label: &str, get_ws_inheritable: impl FnOnce() -> CargoResult, ) -> CargoResult { match self { - schema::MaybeWorkspace::Defined(value) => Ok(value), - schema::MaybeWorkspace::Workspace(w) => get_ws_inheritable().with_context(|| { + schema::InheritableField::Value(value) => Ok(value), + schema::InheritableField::Inherit(w) => get_ws_inheritable().with_context(|| { format!( "error inheriting `{label}` from workspace root manifest's `workspace.{}.{label}`", w.inherit_toml_table(), @@ -1573,8 +1573,8 @@ impl schema::MaybeWorkspace { get_ws_inheritable: impl FnOnce(&W) -> CargoResult, ) -> CargoResult { match self { - schema::MaybeWorkspace::Defined(value) => Ok(value), - schema::MaybeWorkspace::Workspace(w) => get_ws_inheritable(&w).with_context(|| { + schema::InheritableField::Value(value) => Ok(value), + schema::InheritableField::Inherit(w) => get_ws_inheritable(&w).with_context(|| { format!( "error inheriting `{label}` from workspace root manifest's `workspace.{}.{label}`", w.inherit_toml_table(), @@ -1583,21 +1583,21 @@ impl schema::MaybeWorkspace { } } - fn as_defined(&self) -> Option<&T> { + fn as_value(&self) -> Option<&T> { match self { - schema::MaybeWorkspace::Workspace(_) => None, - schema::MaybeWorkspace::Defined(defined) => Some(defined), + schema::InheritableField::Inherit(_) => None, + schema::InheritableField::Value(defined) => Some(defined), } } } -impl WorkspaceInherit for schema::TomlWorkspaceField { +impl WorkspaceInherit for schema::TomlInheritedField { fn inherit_toml_table(&self) -> &str { "package" } } -impl schema::TomlWorkspaceDependency { +impl schema::TomlInheritedDependency { fn resolve<'a>( &self, name: &str, @@ -1673,7 +1673,7 @@ impl schema::TomlWorkspaceDependency { } } -impl WorkspaceInherit for schema::TomlWorkspaceDependency { +impl WorkspaceInherit for schema::TomlInheritedDependency { fn inherit_toml_table(&self) -> &str { "dependencies" } @@ -2263,7 +2263,7 @@ impl schema::TomlProfile { } } -impl schema::MaybeWorkspaceLints { +impl schema::InheritableLints { fn resolve<'a>( self, get_ws_inheritable: impl FnOnce() -> CargoResult, diff --git a/src/cargo/util/toml/schema.rs b/src/cargo/util/toml/schema.rs index 694babcfc5c..93d6dcb8b47 100644 --- a/src/cargo/util/toml/schema.rs +++ b/src/cargo/util/toml/schema.rs @@ -31,20 +31,20 @@ pub struct TomlManifest { pub example: Option>, pub test: Option>, pub bench: Option>, - pub dependencies: Option>, - pub dev_dependencies: Option>, + pub dependencies: Option>, + pub dev_dependencies: Option>, #[serde(rename = "dev_dependencies")] - pub dev_dependencies2: Option>, - pub build_dependencies: Option>, + pub dev_dependencies2: Option>, + pub build_dependencies: Option>, #[serde(rename = "build_dependencies")] - pub build_dependencies2: Option>, + pub build_dependencies2: Option>, pub features: Option>>, pub target: Option>, pub replace: Option>, pub patch: Option>>, pub workspace: Option, - pub badges: Option, - pub lints: Option, + pub badges: Option, + pub lints: Option, } impl TomlManifest { @@ -56,13 +56,13 @@ impl TomlManifest { self.package.as_ref().or(self.project.as_ref()) } - pub fn dev_dependencies(&self) -> Option<&BTreeMap> { + pub fn dev_dependencies(&self) -> Option<&BTreeMap> { self.dev_dependencies .as_ref() .or(self.dev_dependencies2.as_ref()) } - pub fn build_dependencies(&self) -> Option<&BTreeMap> { + pub fn build_dependencies(&self) -> Option<&BTreeMap> { self.build_dependencies .as_ref() .or(self.build_dependencies2.as_ref()) @@ -131,19 +131,19 @@ pub struct InheritableFields { #[derive(Deserialize, Serialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] pub struct TomlPackage { - pub edition: Option, - pub rust_version: Option, + pub edition: Option, + pub rust_version: Option, pub name: String, - pub version: Option, - pub authors: Option, + pub version: Option, + pub authors: Option, pub build: Option, pub metabuild: Option, pub default_target: Option, pub forced_target: Option, pub links: Option, - pub exclude: Option, - pub include: Option, - pub publish: Option, + pub exclude: Option, + pub include: Option, + pub publish: Option, pub workspace: Option, pub im_a_teapot: Option, pub autobins: Option, @@ -153,15 +153,15 @@ pub struct TomlPackage { pub default_run: Option, // Package metadata. - pub description: Option, - pub homepage: Option, - pub documentation: Option, - pub readme: Option, - pub keywords: Option, - pub categories: Option, - pub license: Option, - pub license_file: Option, - pub repository: Option, + pub description: Option, + pub homepage: Option, + pub documentation: Option, + pub readme: Option, + pub keywords: Option, + pub categories: Option, + pub license: Option, + pub license_file: Option, + pub repository: Option, pub resolver: Option, pub metadata: Option, @@ -174,16 +174,16 @@ pub struct TomlPackage { /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. #[derive(Serialize, Copy, Clone, Debug)] #[serde(untagged)] -pub enum MaybeWorkspace { - /// The "defined" type, or the type that that is used when not inheriting from a workspace. - Defined(T), +pub enum InheritableField { + /// The type that that is used when not inheriting from a workspace. + Value(T), /// The type when inheriting from a workspace. - Workspace(W), + Inherit(W), } //. This already has a `Deserialize` impl from version_trim_whitespace -pub type MaybeWorkspaceSemverVersion = MaybeWorkspace; -impl<'de> de::Deserialize<'de> for MaybeWorkspaceSemverVersion { +pub type InheritableSemverVersion = InheritableField; +impl<'de> de::Deserialize<'de> for InheritableSemverVersion { fn deserialize(d: D) -> Result where D: de::Deserializer<'de>, @@ -192,17 +192,17 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceSemverVersion { .expecting("SemVer version") .string( |value| match value.trim().parse().map_err(de::Error::custom) { - Ok(parsed) => Ok(MaybeWorkspace::Defined(parsed)), + Ok(parsed) => Ok(InheritableField::Value(parsed)), Err(e) => Err(e), }, ) - .map(|value| value.deserialize().map(MaybeWorkspace::Workspace)) + .map(|value| value.deserialize().map(InheritableField::Inherit)) .deserialize(d) } } -pub type MaybeWorkspaceString = MaybeWorkspace; -impl<'de> de::Deserialize<'de> for MaybeWorkspaceString { +pub type InheritableString = InheritableField; +impl<'de> de::Deserialize<'de> for InheritableString { fn deserialize(d: D) -> Result where D: de::Deserializer<'de>, @@ -210,7 +210,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceString { struct Visitor; impl<'de> de::Visitor<'de> for Visitor { - type Value = MaybeWorkspaceString; + type Value = InheritableString; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { f.write_str("a string or workspace") @@ -220,7 +220,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceString { where E: de::Error, { - Ok(MaybeWorkspaceString::Defined(value)) + Ok(InheritableString::Value(value)) } fn visit_map(self, map: V) -> Result @@ -228,7 +228,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceString { V: de::MapAccess<'de>, { let mvd = de::value::MapAccessDeserializer::new(map); - TomlWorkspaceField::deserialize(mvd).map(MaybeWorkspace::Workspace) + TomlInheritedField::deserialize(mvd).map(InheritableField::Inherit) } } @@ -236,8 +236,8 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceString { } } -pub type MaybeWorkspaceRustVersion = MaybeWorkspace; -impl<'de> de::Deserialize<'de> for MaybeWorkspaceRustVersion { +pub type InheritableRustVersion = InheritableField; +impl<'de> de::Deserialize<'de> for InheritableRustVersion { fn deserialize(d: D) -> Result where D: de::Deserializer<'de>, @@ -245,7 +245,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceRustVersion { struct Visitor; impl<'de> de::Visitor<'de> for Visitor { - type Value = MaybeWorkspaceRustVersion; + type Value = InheritableRustVersion; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> Result<(), std::fmt::Error> { f.write_str("a semver or workspace") @@ -256,7 +256,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceRustVersion { E: de::Error, { let value = value.parse::().map_err(|e| E::custom(e))?; - Ok(MaybeWorkspaceRustVersion::Defined(value)) + Ok(InheritableRustVersion::Value(value)) } fn visit_map(self, map: V) -> Result @@ -264,7 +264,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceRustVersion { V: de::MapAccess<'de>, { let mvd = de::value::MapAccessDeserializer::new(map); - TomlWorkspaceField::deserialize(mvd).map(MaybeWorkspace::Workspace) + TomlInheritedField::deserialize(mvd).map(InheritableField::Inherit) } } @@ -272,8 +272,8 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceRustVersion { } } -pub type MaybeWorkspaceVecString = MaybeWorkspace, TomlWorkspaceField>; -impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecString { +pub type InheritableVecString = InheritableField, TomlInheritedField>; +impl<'de> de::Deserialize<'de> for InheritableVecString { fn deserialize(d: D) -> Result where D: de::Deserializer<'de>, @@ -281,7 +281,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecString { struct Visitor; impl<'de> de::Visitor<'de> for Visitor { - type Value = MaybeWorkspaceVecString; + type Value = InheritableVecString; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { f.write_str("a vector of strings or workspace") @@ -291,7 +291,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecString { A: de::SeqAccess<'de>, { let seq = de::value::SeqAccessDeserializer::new(v); - Vec::deserialize(seq).map(MaybeWorkspace::Defined) + Vec::deserialize(seq).map(InheritableField::Value) } fn visit_map(self, map: V) -> Result @@ -299,7 +299,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecString { V: de::MapAccess<'de>, { let mvd = de::value::MapAccessDeserializer::new(map); - TomlWorkspaceField::deserialize(mvd).map(MaybeWorkspace::Workspace) + TomlInheritedField::deserialize(mvd).map(InheritableField::Inherit) } } @@ -307,8 +307,8 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecString { } } -pub type MaybeWorkspaceStringOrBool = MaybeWorkspace; -impl<'de> de::Deserialize<'de> for MaybeWorkspaceStringOrBool { +pub type InheritableStringOrBool = InheritableField; +impl<'de> de::Deserialize<'de> for InheritableStringOrBool { fn deserialize(d: D) -> Result where D: de::Deserializer<'de>, @@ -316,7 +316,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceStringOrBool { struct Visitor; impl<'de> de::Visitor<'de> for Visitor { - type Value = MaybeWorkspaceStringOrBool; + type Value = InheritableStringOrBool; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { f.write_str("a string, a bool, or workspace") @@ -327,7 +327,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceStringOrBool { E: de::Error, { let b = de::value::BoolDeserializer::new(v); - StringOrBool::deserialize(b).map(MaybeWorkspace::Defined) + StringOrBool::deserialize(b).map(InheritableField::Value) } fn visit_string(self, v: String) -> Result @@ -335,7 +335,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceStringOrBool { E: de::Error, { let string = de::value::StringDeserializer::new(v); - StringOrBool::deserialize(string).map(MaybeWorkspace::Defined) + StringOrBool::deserialize(string).map(InheritableField::Value) } fn visit_map(self, map: V) -> Result @@ -343,7 +343,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceStringOrBool { V: de::MapAccess<'de>, { let mvd = de::value::MapAccessDeserializer::new(map); - TomlWorkspaceField::deserialize(mvd).map(MaybeWorkspace::Workspace) + TomlInheritedField::deserialize(mvd).map(InheritableField::Inherit) } } @@ -351,8 +351,8 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceStringOrBool { } } -pub type MaybeWorkspaceVecStringOrBool = MaybeWorkspace; -impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecStringOrBool { +pub type InheritableVecStringOrBool = InheritableField; +impl<'de> de::Deserialize<'de> for InheritableVecStringOrBool { fn deserialize(d: D) -> Result where D: de::Deserializer<'de>, @@ -360,7 +360,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecStringOrBool { struct Visitor; impl<'de> de::Visitor<'de> for Visitor { - type Value = MaybeWorkspaceVecStringOrBool; + type Value = InheritableVecStringOrBool; fn expecting(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { f.write_str("a boolean, a vector of strings, or workspace") @@ -371,7 +371,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecStringOrBool { E: de::Error, { let b = de::value::BoolDeserializer::new(v); - VecStringOrBool::deserialize(b).map(MaybeWorkspace::Defined) + VecStringOrBool::deserialize(b).map(InheritableField::Value) } fn visit_seq(self, v: A) -> Result @@ -379,7 +379,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecStringOrBool { A: de::SeqAccess<'de>, { let seq = de::value::SeqAccessDeserializer::new(v); - VecStringOrBool::deserialize(seq).map(MaybeWorkspace::Defined) + VecStringOrBool::deserialize(seq).map(InheritableField::Value) } fn visit_map(self, map: V) -> Result @@ -387,7 +387,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecStringOrBool { V: de::MapAccess<'de>, { let mvd = de::value::MapAccessDeserializer::new(map); - TomlWorkspaceField::deserialize(mvd).map(MaybeWorkspace::Workspace) + TomlInheritedField::deserialize(mvd).map(InheritableField::Inherit) } } @@ -395,33 +395,33 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceVecStringOrBool { } } -pub type MaybeWorkspaceBtreeMap = - MaybeWorkspace>, TomlWorkspaceField>; +pub type InheritableBtreeMap = + InheritableField>, TomlInheritedField>; -impl<'de> de::Deserialize<'de> for MaybeWorkspaceBtreeMap { +impl<'de> de::Deserialize<'de> for InheritableBtreeMap { fn deserialize(deserializer: D) -> Result where D: de::Deserializer<'de>, { let value = serde_value::Value::deserialize(deserializer)?; - if let Ok(w) = TomlWorkspaceField::deserialize( + if let Ok(w) = TomlInheritedField::deserialize( serde_value::ValueDeserializer::::new(value.clone()), ) { return if w.workspace { - Ok(MaybeWorkspace::Workspace(w)) + Ok(InheritableField::Inherit(w)) } else { Err(de::Error::custom("`workspace` cannot be false")) }; } BTreeMap::deserialize(serde_value::ValueDeserializer::::new(value)) - .map(MaybeWorkspace::Defined) + .map(InheritableField::Value) } } #[derive(Deserialize, Serialize, Copy, Clone, Debug)] #[serde(rename_all = "kebab-case")] -pub struct TomlWorkspaceField { +pub struct TomlInheritedField { #[serde(deserialize_with = "bool_no_false")] pub workspace: bool, } @@ -435,42 +435,42 @@ fn bool_no_false<'de, D: de::Deserializer<'de>>(deserializer: D) -> Result; +pub type InheritableDependency = InheritableField; -impl MaybeWorkspaceDependency { +impl InheritableDependency { pub fn unused_keys(&self) -> Vec { match self { - MaybeWorkspaceDependency::Defined(d) => d.unused_keys(), - MaybeWorkspaceDependency::Workspace(w) => w._unused_keys.keys().cloned().collect(), + InheritableDependency::Value(d) => d.unused_keys(), + InheritableDependency::Inherit(w) => w._unused_keys.keys().cloned().collect(), } } } -impl<'de> de::Deserialize<'de> for MaybeWorkspaceDependency { +impl<'de> de::Deserialize<'de> for InheritableDependency { fn deserialize(deserializer: D) -> Result where D: de::Deserializer<'de>, { let value = serde_value::Value::deserialize(deserializer)?; - if let Ok(w) = TomlWorkspaceDependency::deserialize(serde_value::ValueDeserializer::< + if let Ok(w) = TomlInheritedDependency::deserialize(serde_value::ValueDeserializer::< D::Error, >::new(value.clone())) { return if w.workspace { - Ok(MaybeWorkspace::Workspace(w)) + Ok(InheritableField::Inherit(w)) } else { Err(de::Error::custom("`workspace` cannot be false")) }; } TomlDependency::deserialize(serde_value::ValueDeserializer::::new(value)) - .map(MaybeWorkspace::Defined) + .map(InheritableField::Value) } } #[derive(Deserialize, Serialize, Clone, Debug)] #[serde(rename_all = "kebab-case")] -pub struct TomlWorkspaceDependency { +pub struct TomlInheritedDependency { pub workspace: bool, pub features: Option>, pub default_features: Option, @@ -485,7 +485,7 @@ pub struct TomlWorkspaceDependency { pub _unused_keys: BTreeMap, } -impl TomlWorkspaceDependency { +impl TomlInheritedDependency { pub fn default_features(&self) -> Option { self.default_features.or(self.default_features2) } @@ -996,23 +996,23 @@ impl TomlTarget { #[derive(Serialize, Deserialize, Debug, Clone)] #[serde(rename_all = "kebab-case")] pub struct TomlPlatform { - pub dependencies: Option>, - pub build_dependencies: Option>, + pub dependencies: Option>, + pub build_dependencies: Option>, #[serde(rename = "build_dependencies")] - pub build_dependencies2: Option>, - pub dev_dependencies: Option>, + pub build_dependencies2: Option>, + pub dev_dependencies: Option>, #[serde(rename = "dev_dependencies")] - pub dev_dependencies2: Option>, + pub dev_dependencies2: Option>, } impl TomlPlatform { - pub fn dev_dependencies(&self) -> Option<&BTreeMap> { + pub fn dev_dependencies(&self) -> Option<&BTreeMap> { self.dev_dependencies .as_ref() .or(self.dev_dependencies2.as_ref()) } - pub fn build_dependencies(&self) -> Option<&BTreeMap> { + pub fn build_dependencies(&self) -> Option<&BTreeMap> { self.build_dependencies .as_ref() .or(self.build_dependencies2.as_ref()) @@ -1022,7 +1022,7 @@ impl TomlPlatform { #[derive(Deserialize, Serialize, Debug, Clone)] #[serde(expecting = "a lints table")] #[serde(rename_all = "kebab-case")] -pub struct MaybeWorkspaceLints { +pub struct InheritableLints { #[serde(skip_serializing_if = "is_false")] #[serde(deserialize_with = "bool_no_false", default)] pub workspace: bool, From 58b0b18783930619cc8a817f06d0a204b73a04aa Mon Sep 17 00:00:00 2001 From: Ed Page Date: Sat, 11 Nov 2023 20:44:31 -0600 Subject: [PATCH 2/5] refactor(toml): Decouple dependency and package field inheritance --- src/cargo/util/toml/mod.rs | 52 +++++++++++++++++------------------ src/cargo/util/toml/schema.rs | 13 +++++++-- 2 files changed, 35 insertions(+), 30 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 663eaea160f..134ff90db18 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -32,6 +32,7 @@ use crate::util::{ mod embedded; pub mod schema; mod targets; +use self::schema::TomlDependency; use self::targets::targets; /// Loads a `Cargo.toml` from a file on disk. @@ -324,7 +325,7 @@ impl schema::TomlManifest { let deps = deps .iter() .filter(|(_k, v)| { - if let schema::InheritableField::Value(def) = v { + if let schema::InheritableDependency::Value(def) = v { filter(def) } else { false @@ -340,7 +341,7 @@ impl schema::TomlManifest { dep: &schema::InheritableDependency, ) -> CargoResult { let dep = match dep { - schema::InheritableField::Value(schema::TomlDependency::Detailed(d)) => { + schema::InheritableDependency::Value(schema::TomlDependency::Detailed(d)) => { let mut d = d.clone(); // Path dependencies become crates.io deps. d.path.take(); @@ -355,7 +356,7 @@ impl schema::TomlManifest { } Ok(d) } - schema::InheritableField::Value(schema::TomlDependency::Simple(s)) => { + schema::InheritableDependency::Value(schema::TomlDependency::Simple(s)) => { Ok(schema::TomlDetailedDependency { version: Some(s.clone()), ..Default::default() @@ -364,7 +365,7 @@ impl schema::TomlManifest { _ => unreachable!(), }; dep.map(schema::TomlDependency::Detailed) - .map(schema::InheritableField::Value) + .map(schema::InheritableDependency::Value) } } @@ -677,7 +678,7 @@ impl schema::TomlManifest { cx.deps.push(dep); deps.insert( n.to_string(), - schema::InheritableField::Value(resolved.clone()), + schema::InheritableDependency::Value(resolved.clone()), ); } Ok(Some(deps)) @@ -1567,22 +1568,6 @@ impl schema::InheritableField { } } - fn resolve_with_self<'a>( - self, - label: &str, - get_ws_inheritable: impl FnOnce(&W) -> CargoResult, - ) -> CargoResult { - match self { - schema::InheritableField::Value(value) => Ok(value), - schema::InheritableField::Inherit(w) => get_ws_inheritable(&w).with_context(|| { - format!( - "error inheriting `{label}` from workspace root manifest's `workspace.{}.{label}`", - w.inherit_toml_table(), - ) - }), - } - } - fn as_value(&self) -> Option<&T> { match self { schema::InheritableField::Inherit(_) => None, @@ -1597,6 +1582,25 @@ impl WorkspaceInherit for schema::TomlInheritedField { } } +impl schema::InheritableDependency { + fn resolve_with_self<'a>( + self, + label: &str, + get_ws_inheritable: impl FnOnce(&schema::TomlInheritedDependency) -> CargoResult, + ) -> CargoResult { + match self { + schema::InheritableDependency::Value(value) => Ok(value), + schema::InheritableDependency::Inherit(w) => { + get_ws_inheritable(&w).with_context(|| { + format!( + "error inheriting `{label}` from workspace root manifest's `workspace.dependencies.{label}`", + ) + }) + } + } + } +} + impl schema::TomlInheritedDependency { fn resolve<'a>( &self, @@ -1673,12 +1677,6 @@ impl schema::TomlInheritedDependency { } } -impl WorkspaceInherit for schema::TomlInheritedDependency { - fn inherit_toml_table(&self) -> &str { - "dependencies" - } -} - impl schema::TomlDependency

{ pub(crate) fn to_dependency_split( &self, diff --git a/src/cargo/util/toml/schema.rs b/src/cargo/util/toml/schema.rs index 93d6dcb8b47..ef8b7774ca7 100644 --- a/src/cargo/util/toml/schema.rs +++ b/src/cargo/util/toml/schema.rs @@ -435,7 +435,14 @@ fn bool_no_false<'de, D: de::Deserializer<'de>>(deserializer: D) -> Result; +#[derive(Serialize, Clone, Debug)] +#[serde(untagged)] +pub enum InheritableDependency { + /// The type that that is used when not inheriting from a workspace. + Value(TomlDependency), + /// The type when inheriting from a workspace. + Inherit(TomlInheritedDependency), +} impl InheritableDependency { pub fn unused_keys(&self) -> Vec { @@ -458,13 +465,13 @@ impl<'de> de::Deserialize<'de> for InheritableDependency { >::new(value.clone())) { return if w.workspace { - Ok(InheritableField::Inherit(w)) + Ok(InheritableDependency::Inherit(w)) } else { Err(de::Error::custom("`workspace` cannot be false")) }; } TomlDependency::deserialize(serde_value::ValueDeserializer::::new(value)) - .map(InheritableField::Value) + .map(InheritableDependency::Value) } } From f8a63f67cd686d0666d855dcb0d6012732d475ae Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 13 Nov 2023 11:11:17 -0600 Subject: [PATCH 3/5] refactor(toml): Hard-code InheritableField for package table --- src/cargo/util/toml/mod.rs | 25 +++---------------------- src/cargo/util/toml/schema.rs | 19 +++++++++---------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 134ff90db18..c830537dc5b 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -1539,19 +1539,7 @@ impl schema::TomlPackage { } } -/// This Trait exists to make [`schema::InheritableField::Inherit`] generic. It makes deserialization of -/// [`schema::InheritableField`] much easier, as well as making error messages for -/// [`schema::InheritableField::resolve`] much nicer -/// -/// Implementors should have a field `workspace` with the type of `bool`. It is used to ensure -/// `workspace` is not `false` in a `Cargo.toml` -pub trait WorkspaceInherit { - /// This is the workspace table that is being inherited from. - /// For example `[workspace.dependencies]` would be the table "dependencies" - fn inherit_toml_table(&self) -> &str; -} - -impl schema::InheritableField { +impl schema::InheritableField { fn resolve<'a>( self, label: &str, @@ -1559,10 +1547,9 @@ impl schema::InheritableField { ) -> CargoResult { match self { schema::InheritableField::Value(value) => Ok(value), - schema::InheritableField::Inherit(w) => get_ws_inheritable().with_context(|| { + schema::InheritableField::Inherit(_) => get_ws_inheritable().with_context(|| { format!( - "error inheriting `{label}` from workspace root manifest's `workspace.{}.{label}`", - w.inherit_toml_table(), + "error inheriting `{label}` from workspace root manifest's `workspace.package.{label}`", ) }), } @@ -1576,12 +1563,6 @@ impl schema::InheritableField { } } -impl WorkspaceInherit for schema::TomlInheritedField { - fn inherit_toml_table(&self) -> &str { - "package" - } -} - impl schema::InheritableDependency { fn resolve_with_self<'a>( self, diff --git a/src/cargo/util/toml/schema.rs b/src/cargo/util/toml/schema.rs index ef8b7774ca7..7b7bd6df187 100644 --- a/src/cargo/util/toml/schema.rs +++ b/src/cargo/util/toml/schema.rs @@ -174,15 +174,15 @@ pub struct TomlPackage { /// An enum that allows for inheriting keys from a workspace in a Cargo.toml. #[derive(Serialize, Copy, Clone, Debug)] #[serde(untagged)] -pub enum InheritableField { +pub enum InheritableField { /// The type that that is used when not inheriting from a workspace. Value(T), /// The type when inheriting from a workspace. - Inherit(W), + Inherit(TomlInheritedField), } //. This already has a `Deserialize` impl from version_trim_whitespace -pub type InheritableSemverVersion = InheritableField; +pub type InheritableSemverVersion = InheritableField; impl<'de> de::Deserialize<'de> for InheritableSemverVersion { fn deserialize(d: D) -> Result where @@ -201,7 +201,7 @@ impl<'de> de::Deserialize<'de> for InheritableSemverVersion { } } -pub type InheritableString = InheritableField; +pub type InheritableString = InheritableField; impl<'de> de::Deserialize<'de> for InheritableString { fn deserialize(d: D) -> Result where @@ -236,7 +236,7 @@ impl<'de> de::Deserialize<'de> for InheritableString { } } -pub type InheritableRustVersion = InheritableField; +pub type InheritableRustVersion = InheritableField; impl<'de> de::Deserialize<'de> for InheritableRustVersion { fn deserialize(d: D) -> Result where @@ -272,7 +272,7 @@ impl<'de> de::Deserialize<'de> for InheritableRustVersion { } } -pub type InheritableVecString = InheritableField, TomlInheritedField>; +pub type InheritableVecString = InheritableField>; impl<'de> de::Deserialize<'de> for InheritableVecString { fn deserialize(d: D) -> Result where @@ -307,7 +307,7 @@ impl<'de> de::Deserialize<'de> for InheritableVecString { } } -pub type InheritableStringOrBool = InheritableField; +pub type InheritableStringOrBool = InheritableField; impl<'de> de::Deserialize<'de> for InheritableStringOrBool { fn deserialize(d: D) -> Result where @@ -351,7 +351,7 @@ impl<'de> de::Deserialize<'de> for InheritableStringOrBool { } } -pub type InheritableVecStringOrBool = InheritableField; +pub type InheritableVecStringOrBool = InheritableField; impl<'de> de::Deserialize<'de> for InheritableVecStringOrBool { fn deserialize(d: D) -> Result where @@ -395,8 +395,7 @@ impl<'de> de::Deserialize<'de> for InheritableVecStringOrBool { } } -pub type InheritableBtreeMap = - InheritableField>, TomlInheritedField>; +pub type InheritableBtreeMap = InheritableField>>; impl<'de> de::Deserialize<'de> for InheritableBtreeMap { fn deserialize(deserializer: D) -> Result From 0eda16f82353562b76c2dac241210747fea82517 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 13 Nov 2023 11:44:56 -0600 Subject: [PATCH 4/5] refactor(toml): Be specific with inheriting action --- src/cargo/util/toml/mod.rs | 55 +++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index c830537dc5b..9af9e73bfc3 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -501,7 +501,7 @@ impl schema::TomlManifest { let version = package .version .clone() - .map(|version| version.resolve("version", || inherit()?.version())) + .map(|version| version.inherit_with("version", || inherit()?.version())) .transpose()?; package.version = version.clone().map(schema::InheritableField::Value); @@ -515,7 +515,7 @@ impl schema::TomlManifest { let edition = if let Some(edition) = package.edition.clone() { let edition: Edition = edition - .resolve("edition", || inherit()?.edition())? + .inherit_with("edition", || inherit()?.edition())? .parse() .with_context(|| "failed to parse the `edition` key")?; package.edition = Some(schema::InheritableField::Value(edition.to_string())); @@ -542,7 +542,7 @@ impl schema::TomlManifest { let rust_version = if let Some(rust_version) = &package.rust_version { let rust_version = rust_version .clone() - .resolve("rust_version", || inherit()?.rust_version())?; + .inherit_with("rust_version", || inherit()?.rust_version())?; let req = rust_version.to_caret_req(); if let Some(first_version) = edition.first_version() { let unsupported = @@ -661,7 +661,7 @@ impl schema::TomlManifest { for (n, v) in dependencies.iter() { let resolved = v .clone() - .resolve_with_self(n, |dep| dep.resolve(n, inheritable, cx))?; + .inherit_with(n, |dep| dep.inherit_with(n, inheritable, cx))?; let dep = resolved.to_dependency(n, cx, kind)?; let name_in_toml = dep.name_in_toml().as_str(); validate_package_name(name_in_toml, "dependency name", "")?; @@ -718,7 +718,7 @@ impl schema::TomlManifest { let lints = me .lints .clone() - .map(|mw| mw.resolve(|| inherit()?.lints())) + .map(|mw| mw.inherit_with(|| inherit()?.lints())) .transpose()?; let lints = verify_lints(lints)?; let default = schema::TomlLints::default(); @@ -799,13 +799,13 @@ impl schema::TomlManifest { let exclude = package .exclude .clone() - .map(|mw| mw.resolve("exclude", || inherit()?.exclude())) + .map(|mw| mw.inherit_with("exclude", || inherit()?.exclude())) .transpose()? .unwrap_or_default(); let include = package .include .clone() - .map(|mw| mw.resolve("include", || inherit()?.include())) + .map(|mw| mw.inherit_with("include", || inherit()?.include())) .transpose()? .unwrap_or_default(); let empty_features = BTreeMap::new(); @@ -832,70 +832,70 @@ impl schema::TomlManifest { description: package .description .clone() - .map(|mw| mw.resolve("description", || inherit()?.description())) + .map(|mw| mw.inherit_with("description", || inherit()?.description())) .transpose()?, homepage: package .homepage .clone() - .map(|mw| mw.resolve("homepage", || inherit()?.homepage())) + .map(|mw| mw.inherit_with("homepage", || inherit()?.homepage())) .transpose()?, documentation: package .documentation .clone() - .map(|mw| mw.resolve("documentation", || inherit()?.documentation())) + .map(|mw| mw.inherit_with("documentation", || inherit()?.documentation())) .transpose()?, readme: readme_for_package( package_root, package .readme .clone() - .map(|mw| mw.resolve("readme", || inherit()?.readme(package_root))) + .map(|mw| mw.inherit_with("readme", || inherit()?.readme(package_root))) .transpose()? .as_ref(), ), authors: package .authors .clone() - .map(|mw| mw.resolve("authors", || inherit()?.authors())) + .map(|mw| mw.inherit_with("authors", || inherit()?.authors())) .transpose()? .unwrap_or_default(), license: package .license .clone() - .map(|mw| mw.resolve("license", || inherit()?.license())) + .map(|mw| mw.inherit_with("license", || inherit()?.license())) .transpose()?, license_file: package .license_file .clone() - .map(|mw| mw.resolve("license", || inherit()?.license_file(package_root))) + .map(|mw| mw.inherit_with("license", || inherit()?.license_file(package_root))) .transpose()?, repository: package .repository .clone() - .map(|mw| mw.resolve("repository", || inherit()?.repository())) + .map(|mw| mw.inherit_with("repository", || inherit()?.repository())) .transpose()?, keywords: package .keywords .clone() - .map(|mw| mw.resolve("keywords", || inherit()?.keywords())) + .map(|mw| mw.inherit_with("keywords", || inherit()?.keywords())) .transpose()? .unwrap_or_default(), categories: package .categories .clone() - .map(|mw| mw.resolve("categories", || inherit()?.categories())) + .map(|mw| mw.inherit_with("categories", || inherit()?.categories())) .transpose()? .unwrap_or_default(), badges: me .badges .clone() - .map(|mw| mw.resolve("badges", || inherit()?.badges())) + .map(|mw| mw.inherit_with("badges", || inherit()?.badges())) .transpose()? .unwrap_or_default(), links: package.links.clone(), rust_version: package .rust_version - .map(|mw| mw.resolve("rust-version", || inherit()?.rust_version())) + .map(|mw| mw.inherit_with("rust-version", || inherit()?.rust_version())) .transpose()?, }; package.description = metadata @@ -956,10 +956,11 @@ impl schema::TomlManifest { profiles.validate(cli_unstable, &features, &mut warnings)?; } - let publish = package - .publish - .clone() - .map(|publish| publish.resolve("publish", || inherit()?.publish()).unwrap()); + let publish = package.publish.clone().map(|publish| { + publish + .inherit_with("publish", || inherit()?.publish()) + .unwrap() + }); package.publish = publish.clone().map(|p| schema::InheritableField::Value(p)); @@ -1540,7 +1541,7 @@ impl schema::TomlPackage { } impl schema::InheritableField { - fn resolve<'a>( + fn inherit_with<'a>( self, label: &str, get_ws_inheritable: impl FnOnce() -> CargoResult, @@ -1564,7 +1565,7 @@ impl schema::InheritableField { } impl schema::InheritableDependency { - fn resolve_with_self<'a>( + fn inherit_with<'a>( self, label: &str, get_ws_inheritable: impl FnOnce(&schema::TomlInheritedDependency) -> CargoResult, @@ -1583,7 +1584,7 @@ impl schema::InheritableDependency { } impl schema::TomlInheritedDependency { - fn resolve<'a>( + fn inherit_with<'a>( &self, name: &str, inheritable: impl FnOnce() -> CargoResult<&'a schema::InheritableFields>, @@ -2243,7 +2244,7 @@ impl schema::TomlProfile { } impl schema::InheritableLints { - fn resolve<'a>( + fn inherit_with<'a>( self, get_ws_inheritable: impl FnOnce() -> CargoResult, ) -> CargoResult { From 46d9894014fc769a9247ecfdb0f244464ba7df60 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 13 Nov 2023 14:05:54 -0600 Subject: [PATCH 5/5] refactor(toml): Split out InheritablePackage from InheritableFields This allows us to move bookkeeping / logic out of the schema --- src/cargo/core/mod.rs | 1 - src/cargo/core/workspace.rs | 2 +- src/cargo/util/toml/mod.rs | 111 +++++++++++++++++++--------------- src/cargo/util/toml/schema.rs | 15 +---- 4 files changed, 65 insertions(+), 64 deletions(-) diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs index 80809106139..9860d2617b6 100644 --- a/src/cargo/core/mod.rs +++ b/src/cargo/core/mod.rs @@ -14,7 +14,6 @@ pub use self::workspace::{ find_workspace_root, resolve_relative_path, MaybePackage, Workspace, WorkspaceConfig, WorkspaceRootConfig, }; -pub use crate::util::toml::schema::InheritableFields; pub mod compiler; pub mod dependency; diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 4667c8029d2..ead026f493d 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -23,7 +23,7 @@ use crate::util::edit_distance; use crate::util::errors::{CargoResult, ManifestError}; use crate::util::interning::InternedString; use crate::util::toml::{ - read_manifest, schema::InheritableFields, schema::TomlDependency, schema::TomlProfiles, + read_manifest, schema::TomlDependency, schema::TomlProfiles, InheritableFields, }; use crate::util::RustVersion; use crate::util::{config::ConfigRelativePath, Config, Filesystem, IntoUrl}; diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 9af9e73bfc3..c07aef05990 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -380,7 +380,7 @@ impl schema::TomlManifest { config: &Config, resolved_path: &Path, workspace_config: &WorkspaceConfig, - ) -> CargoResult { + ) -> CargoResult { match workspace_config { WorkspaceConfig::Root(root) => Ok(root.inheritable().clone()), WorkspaceConfig::Member { @@ -446,12 +446,14 @@ impl schema::TomlManifest { let workspace_config = match (me.workspace.as_ref(), package.workspace.as_ref()) { (Some(toml_config), None) => { - let mut inheritable = toml_config.package.clone().unwrap_or_default(); - inheritable.update_ws_path(package_root.to_path_buf()); - inheritable.update_deps(toml_config.dependencies.clone()); let lints = toml_config.lints.clone(); let lints = verify_lints(lints)?; - inheritable.update_lints(lints); + let inheritable = InheritableFields { + package: toml_config.package.clone(), + dependencies: toml_config.dependencies.clone(), + lints, + _ws_root: package_root.to_path_buf(), + }; if let Some(ws_deps) = &inheritable.dependencies { for (name, dep) in ws_deps { unused_dep_keys( @@ -494,7 +496,7 @@ impl schema::TomlManifest { let resolved_path = package_root.join("Cargo.toml"); - let inherit_cell: LazyCell = LazyCell::new(); + let inherit_cell: LazyCell = LazyCell::new(); let inherit = || inherit_cell.try_borrow_with(|| get_ws(config, &resolved_path, &workspace_config)); @@ -645,7 +647,7 @@ impl schema::TomlManifest { new_deps: Option<&BTreeMap>, kind: Option, workspace_config: &WorkspaceConfig, - inherit_cell: &LazyCell, + inherit_cell: &LazyCell, ) -> CargoResult>> { let Some(dependencies) = new_deps else { return Ok(None); @@ -1164,12 +1166,14 @@ impl schema::TomlManifest { .transpose()?; let workspace_config = match me.workspace { Some(ref toml_config) => { - let mut inheritable = toml_config.package.clone().unwrap_or_default(); - inheritable.update_ws_path(root.to_path_buf()); - inheritable.update_deps(toml_config.dependencies.clone()); let lints = toml_config.lints.clone(); let lints = verify_lints(lints)?; - inheritable.update_lints(lints); + let inheritable = InheritableFields { + package: toml_config.package.clone(), + dependencies: toml_config.dependencies.clone(), + lints, + _ws_root: root.to_path_buf(), + }; let ws_root_config = WorkspaceRootConfig::new( root, &toml_config.members, @@ -1363,7 +1367,7 @@ fn unused_dep_keys( fn inheritable_from_path( config: &Config, workspace_path: PathBuf, -) -> CargoResult { +) -> CargoResult { // Workspace path should have Cargo.toml at the end let workspace_path_root = workspace_path.parent().unwrap(); @@ -1446,13 +1450,13 @@ fn unique_build_targets( } /// Defines simple getter methods for inheritable fields. -macro_rules! inheritable_field_getter { +macro_rules! package_field_getter { ( $(($key:literal, $field:ident -> $ret:ty),)* ) => ( $( - #[doc = concat!("Gets the field `workspace.", $key, "`.")] + #[doc = concat!("Gets the field `workspace.package", $key, "`.")] fn $field(&self) -> CargoResult<$ret> { - let Some(val) = &self.$field else { - bail!("`workspace.{}` was not defined", $key); + let Some(val) = self.package.as_ref().and_then(|p| p.$field.as_ref()) else { + bail!("`workspace.package.{}` was not defined", $key); }; Ok(val.clone()) } @@ -1460,25 +1464,35 @@ macro_rules! inheritable_field_getter { ) } -impl schema::InheritableFields { - inheritable_field_getter! { +/// A group of fields that are inheritable by members of the workspace +#[derive(Clone, Debug, Default)] +pub struct InheritableFields { + package: Option, + dependencies: Option>, + lints: Option, + + // Bookkeeping to help when resolving values from above + _ws_root: PathBuf, +} + +impl InheritableFields { + package_field_getter! { // Please keep this list lexicographically ordered. - ("lints", lints -> schema::TomlLints), - ("package.authors", authors -> Vec), - ("package.badges", badges -> BTreeMap>), - ("package.categories", categories -> Vec), - ("package.description", description -> String), - ("package.documentation", documentation -> String), - ("package.edition", edition -> String), - ("package.exclude", exclude -> Vec), - ("package.homepage", homepage -> String), - ("package.include", include -> Vec), - ("package.keywords", keywords -> Vec), - ("package.license", license -> String), - ("package.publish", publish -> schema::VecStringOrBool), - ("package.repository", repository -> String), - ("package.rust-version", rust_version -> RustVersion), - ("package.version", version -> semver::Version), + ("authors", authors -> Vec), + ("badges", badges -> BTreeMap>), + ("categories", categories -> Vec), + ("description", description -> String), + ("documentation", documentation -> String), + ("edition", edition -> String), + ("exclude", exclude -> Vec), + ("homepage", homepage -> String), + ("include", include -> Vec), + ("keywords", keywords -> Vec), + ("license", license -> String), + ("publish", publish -> schema::VecStringOrBool), + ("repository", repository -> String), + ("rust-version", rust_version -> RustVersion), + ("version", version -> semver::Version), } /// Gets a workspace dependency with the `name`. @@ -1500,9 +1514,17 @@ impl schema::InheritableFields { Ok(dep) } + /// Gets the field `workspace.lint`. + fn lints(&self) -> CargoResult { + let Some(val) = &self.lints else { + bail!("`workspace.lints` was not defined"); + }; + Ok(val.clone()) + } + /// Gets the field `workspace.package.license-file`. fn license_file(&self, package_root: &Path) -> CargoResult { - let Some(license_file) = &self.license_file else { + let Some(license_file) = self.package.as_ref().and_then(|p| p.license_file.as_ref()) else { bail!("`workspace.package.license-file` was not defined"); }; resolve_relative_path("license-file", &self._ws_root, package_root, license_file) @@ -1510,7 +1532,10 @@ impl schema::InheritableFields { /// Gets the field `workspace.package.readme`. fn readme(&self, package_root: &Path) -> CargoResult { - let Some(readme) = readme_for_package(self._ws_root.as_path(), self.readme.as_ref()) else { + let Some(readme) = readme_for_package( + self._ws_root.as_path(), + self.package.as_ref().and_then(|p| p.readme.as_ref()), + ) else { bail!("`workspace.package.readme` was not defined"); }; resolve_relative_path("readme", &self._ws_root, package_root, &readme) @@ -1520,18 +1545,6 @@ impl schema::InheritableFields { fn ws_root(&self) -> &PathBuf { &self._ws_root } - - fn update_deps(&mut self, deps: Option>) { - self.dependencies = deps; - } - - fn update_lints(&mut self, lints: Option) { - self.lints = lints; - } - - fn update_ws_path(&mut self, ws_root: PathBuf) { - self._ws_root = ws_root; - } } impl schema::TomlPackage { @@ -1587,7 +1600,7 @@ impl schema::TomlInheritedDependency { fn inherit_with<'a>( &self, name: &str, - inheritable: impl FnOnce() -> CargoResult<&'a schema::InheritableFields>, + inheritable: impl FnOnce() -> CargoResult<&'a InheritableFields>, cx: &mut Context<'_, '_>, ) -> CargoResult { fn default_features_msg(label: &str, ws_def_feat: Option, cx: &mut Context<'_, '_>) { diff --git a/src/cargo/util/toml/schema.rs b/src/cargo/util/toml/schema.rs index 7b7bd6df187..883fbb2dc42 100644 --- a/src/cargo/util/toml/schema.rs +++ b/src/cargo/util/toml/schema.rs @@ -83,7 +83,7 @@ pub struct TomlWorkspace { pub metadata: Option, // Properties that can be inherited by members. - pub package: Option, + pub package: Option, pub dependencies: Option>, pub lints: Option, } @@ -91,14 +91,7 @@ pub struct TomlWorkspace { /// A group of fields that are inheritable by members of the workspace #[derive(Clone, Debug, Default, Deserialize, Serialize)] #[serde(rename_all = "kebab-case")] -pub struct InheritableFields { - // We use skip here since it will never be present when deserializing - // and we don't want it present when serializing - #[serde(skip)] - pub dependencies: Option>, - #[serde(skip)] - pub lints: Option, - +pub struct InheritablePackage { pub version: Option, pub authors: Option>, pub description: Option, @@ -116,10 +109,6 @@ pub struct InheritableFields { pub exclude: Option>, pub include: Option>, pub rust_version: Option, - // We use skip here since it will never be present when deserializing - // and we don't want it present when serializing - #[serde(skip)] - pub _ws_root: PathBuf, } /// Represents the `package`/`project` sections of a `Cargo.toml`.