Skip to content

Commit

Permalink
Auto merge of #12954 - epage:extra, r=Muscraft
Browse files Browse the repository at this point in the history
refactor(toml): Improve consistency

### What does this PR try to resolve?

As I was working towards #12801, these inconsistencies bothered me.  Hopefully this will also make it easier to discover special cases in the schema and deal with them properly.

### How should we test and review this PR?

### Additional information
  • Loading branch information
bors committed Nov 11, 2023
2 parents 90a703e + 86a11ba commit f1efa42
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 22 deletions.
17 changes: 6 additions & 11 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,7 @@ impl schema::TomlManifest {
package_root: &Path,
) -> CargoResult<schema::TomlManifest> {
let config = ws.config();
let mut package = self
.package
.as_ref()
.or_else(|| self.project.as_ref())
.unwrap()
.clone();
let mut package = self.package().unwrap().clone();
package.workspace = None;
let current_resolver = package
.resolver
Expand Down Expand Up @@ -1508,20 +1503,20 @@ impl schema::InheritableFields {
let Some(license_file) = &self.license_file else {
bail!("`workspace.package.license-file` was not defined");
};
resolve_relative_path("license-file", &self.ws_root, package_root, license_file)
resolve_relative_path("license-file", &self._ws_root, package_root, license_file)
}

/// Gets the field `workspace.package.readme`.
fn readme(&self, package_root: &Path) -> CargoResult<schema::StringOrBool> {
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.readme.as_ref()) else {
bail!("`workspace.package.readme` was not defined");
};
resolve_relative_path("readme", &self.ws_root, package_root, &readme)
resolve_relative_path("readme", &self._ws_root, package_root, &readme)
.map(schema::StringOrBool::String)
}

fn ws_root(&self) -> &PathBuf {
&self.ws_root
&self._ws_root
}

fn update_deps(&mut self, deps: Option<BTreeMap<String, schema::TomlDependency>>) {
Expand All @@ -1533,7 +1528,7 @@ impl schema::InheritableFields {
}

fn update_ws_path(&mut self, ws_root: PathBuf) {
self.ws_root = ws_root;
self._ws_root = ws_root;
}
}

Expand Down
30 changes: 20 additions & 10 deletions src/cargo/util/toml/schema.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
//! `Cargo.toml` / Manifest schema definition
//!
//! ## Style
//!
//! - Fields duplicated for an alias will have an accessor with the primary field's name
//! - Keys that exist for bookkeeping but don't correspond to the schema have a `_` prefix
use std::collections::BTreeMap;
use std::fmt::{self, Display, Write};
use std::path::PathBuf;
Expand Down Expand Up @@ -45,6 +52,10 @@ impl TomlManifest {
self.profile.is_some()
}

pub fn package(&self) -> Option<&Box<TomlPackage>> {
self.package.as_ref().or(self.project.as_ref())
}

pub fn dev_dependencies(&self) -> Option<&BTreeMap<String, MaybeWorkspaceDependency>> {
self.dev_dependencies
.as_ref()
Expand Down Expand Up @@ -108,7 +119,7 @@ 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 ws_root: PathBuf,
pub _ws_root: PathBuf,
}

/// Represents the `package`/`project` sections of a `Cargo.toml`.
Expand Down Expand Up @@ -430,7 +441,7 @@ impl MaybeWorkspaceDependency {
pub fn unused_keys(&self) -> Vec<String> {
match self {
MaybeWorkspaceDependency::Defined(d) => d.unused_keys(),
MaybeWorkspaceDependency::Workspace(w) => w.unused_keys.keys().cloned().collect(),
MaybeWorkspaceDependency::Workspace(w) => w._unused_keys.keys().cloned().collect(),
}
}
}
Expand Down Expand Up @@ -471,7 +482,7 @@ pub struct TomlWorkspaceDependency {
/// This is here to provide a way to see the "unused manifest keys" when deserializing
#[serde(skip_serializing)]
#[serde(flatten)]
pub unused_keys: BTreeMap<String, toml::Value>,
pub _unused_keys: BTreeMap<String, toml::Value>,
}

impl TomlWorkspaceDependency {
Expand Down Expand Up @@ -510,7 +521,7 @@ impl TomlDependency {
pub fn unused_keys(&self) -> Vec<String> {
match self {
TomlDependency::Simple(_) => vec![],
TomlDependency::Detailed(detailed) => detailed.unused_keys.keys().cloned().collect(),
TomlDependency::Detailed(detailed) => detailed._unused_keys.keys().cloned().collect(),
}
}
}
Expand Down Expand Up @@ -568,7 +579,7 @@ pub struct DetailedTomlDependency<P: Clone = String> {
/// This is here to provide a way to see the "unused manifest keys" when deserializing
#[serde(skip_serializing)]
#[serde(flatten)]
pub unused_keys: BTreeMap<String, toml::Value>,
pub _unused_keys: BTreeMap<String, toml::Value>,
}

impl<P: Clone> DetailedTomlDependency<P> {
Expand Down Expand Up @@ -598,7 +609,7 @@ impl<P: Clone> Default for DetailedTomlDependency<P> {
artifact: Default::default(),
lib: Default::default(),
target: Default::default(),
unused_keys: Default::default(),
_unused_keys: Default::default(),
}
}
}
Expand Down Expand Up @@ -950,10 +961,9 @@ pub struct TomlTarget {
pub doc: Option<bool>,
pub plugin: Option<bool>,
pub doc_scrape_examples: Option<bool>,
#[serde(rename = "proc-macro")]
pub proc_macro_raw: Option<bool>,
pub proc_macro: Option<bool>,
#[serde(rename = "proc_macro")]
pub proc_macro_raw2: Option<bool>,
pub proc_macro2: Option<bool>,
pub harness: Option<bool>,
pub required_features: Option<Vec<String>>,
pub edition: Option<String>,
Expand All @@ -965,7 +975,7 @@ impl TomlTarget {
}

pub fn proc_macro(&self) -> Option<bool> {
self.proc_macro_raw.or(self.proc_macro_raw2).or_else(|| {
self.proc_macro.or(self.proc_macro2).or_else(|| {
if let Some(types) = self.crate_types() {
if types.contains(&"proc-macro".to_string()) {
return Some(true);
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/util/toml/targets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1004,7 +1004,7 @@ fn name_or_panic(target: &TomlTarget) -> &str {
}

fn validate_proc_macro(target: &TomlTarget, kind: &str, warnings: &mut Vec<String>) {
if target.proc_macro_raw.is_some() && target.proc_macro_raw2.is_some() {
if target.proc_macro.is_some() && target.proc_macro2.is_some() {
warn_on_deprecated(
"proc-macro",
name_or_panic(target),
Expand Down

0 comments on commit f1efa42

Please sign in to comment.