Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(toml): Cleanup noticed on the way to #12801 #12902

Merged
merged 6 commits into from
Oct 31, 2023
Merged

refactor(toml): Cleanup noticed on the way to #12801 #12902

Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f2a6514
refactor(toml): Clairfy unued_keys field
epage Oct 31, 2023
d3bff4e
refactor(toml): Re-arrange tables due to toml upgrade
epage Oct 31, 2023
26905f2
docs(toml): Promote a comment to doc-comment
epage Oct 31, 2023
7b7fa1a
refactor(toml): Separate unused_keys from payload
epage Oct 31, 2023
62fde42
refactor(toml): Group members and excludes
epage Oct 31, 2023
f42b161
refactor(toml): Consistently apply rename_all
epage Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 17 additions & 20 deletions src/cargo/util/toml/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1530,25 +1530,23 @@ fn unique_build_targets(
}

#[derive(Debug, Deserialize, Serialize, Clone)]
#[serde(rename_all = "kebab-case")]
pub struct TomlWorkspace {
members: Option<Vec<String>>,
#[serde(rename = "default-members")]
default_members: Option<Vec<String>>,
exclude: Option<Vec<String>>,
default_members: Option<Vec<String>>,
resolver: Option<String>,
metadata: Option<toml::Value>,

// Properties that can be inherited by members.
package: Option<InheritableFields>,
dependencies: Option<BTreeMap<String, TomlDependency>>,
lints: Option<TomlLints>,

// Note that this field must come last due to the way toml serialization
// works which requires tables to be emitted after all values.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like metadata isn't needed to be the last anymore. Do we have a test to verify the change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like we have several tests in testsuite/package.rs that use package.metadata which means we'll serialize the prepared manifest and verify its output which would hit this situation.

Copy link
Member

@weihanglo weihanglo Oct 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was 6bc7455 but since we have tests and toml crate has changed largely, I am going to merge this. Thank you!

metadata: Option<toml::Value>,
}

/// 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
Expand All @@ -1566,15 +1564,13 @@ pub struct InheritableFields {
keywords: Option<Vec<String>>,
categories: Option<Vec<String>>,
license: Option<String>,
#[serde(rename = "license-file")]
license_file: Option<String>,
repository: Option<String>,
publish: Option<VecStringOrBool>,
edition: Option<String>,
badges: Option<BTreeMap<String, BTreeMap<String, String>>>,
exclude: Option<Vec<String>>,
include: Option<Vec<String>>,
#[serde(rename = "rust-version")]
rust_version: Option<RustVersion>,
// We use skip here since it will never be present when deserializing
// and we don't want it present when serializing
Expand Down Expand Up @@ -1710,13 +1706,11 @@ pub struct TomlPackage {
repository: Option<MaybeWorkspaceString>,
resolver: Option<String>,

// Provide a helpful error message for a common user error.
metadata: Option<toml::Value>,

/// Provide a helpful error message for a common user error.
#[serde(rename = "cargo-features", skip_serializing)]
_invalid_cargo_features: Option<InvalidCargoFeatures>,

// Note that this field must come last due to the way toml serialization
// works which requires tables to be emitted after all values.
metadata: Option<toml::Value>,
}

impl TomlPackage {
Expand Down Expand Up @@ -2034,6 +2028,7 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceBtreeMap {
}

#[derive(Deserialize, Serialize, Copy, Clone, Debug)]
#[serde(rename_all = "kebab-case")]
pub struct TomlWorkspaceField {
#[serde(deserialize_with = "bool_no_false")]
workspace: bool,
Expand Down Expand Up @@ -2064,7 +2059,7 @@ impl MaybeWorkspaceDependency {
fn unused_keys(&self) -> Vec<String> {
match self {
MaybeWorkspaceDependency::Defined(d) => d.unused_keys(),
MaybeWorkspaceDependency::Workspace(w) => w.other.keys().cloned().collect(),
MaybeWorkspaceDependency::Workspace(w) => w.unused_keys.keys().cloned().collect(),
}
}
}
Expand Down Expand Up @@ -2101,10 +2096,11 @@ pub struct TomlWorkspaceDependency {
default_features2: Option<bool>,
optional: Option<bool>,
public: Option<bool>,

/// This is here to provide a way to see the "unused manifest keys" when deserializing
#[serde(skip_serializing)]
#[serde(flatten)]
other: BTreeMap<String, toml::Value>,
unused_keys: BTreeMap<String, toml::Value>,
}

impl TomlWorkspaceDependency {
Expand Down Expand Up @@ -2212,7 +2208,7 @@ impl TomlDependency {
fn unused_keys(&self) -> Vec<String> {
match self {
TomlDependency::Simple(_) => vec![],
TomlDependency::Detailed(detailed) => detailed.other.keys().cloned().collect(),
TomlDependency::Detailed(detailed) => detailed.unused_keys.keys().cloned().collect(),
}
}
}
Expand Down Expand Up @@ -2326,10 +2322,11 @@ pub struct DetailedTomlDependency<P: Clone = String> {
lib: Option<bool>,
/// A platform name, like `x86_64-apple-darwin`
target: Option<String>,

/// This is here to provide a way to see the "unused manifest keys" when deserializing
#[serde(skip_serializing)]
#[serde(flatten)]
other: BTreeMap<String, toml::Value>,
unused_keys: BTreeMap<String, toml::Value>,
}

impl DetailedTomlDependency {
Expand Down Expand Up @@ -2635,7 +2632,7 @@ impl<P: Clone> Default for DetailedTomlDependency<P> {
artifact: Default::default(),
lib: Default::default(),
target: Default::default(),
other: Default::default(),
unused_keys: Default::default(),
}
}
}
Expand Down Expand Up @@ -3383,20 +3380,20 @@ impl TomlTarget {

/// Corresponds to a `target` entry, but `TomlTarget` is already used.
#[derive(Serialize, Deserialize, Debug, Clone)]
#[serde(rename_all = "kebab-case")]
struct TomlPlatform {
dependencies: Option<BTreeMap<String, MaybeWorkspaceDependency>>,
#[serde(rename = "build-dependencies")]
build_dependencies: Option<BTreeMap<String, MaybeWorkspaceDependency>>,
#[serde(rename = "build_dependencies")]
build_dependencies2: Option<BTreeMap<String, MaybeWorkspaceDependency>>,
#[serde(rename = "dev-dependencies")]
dev_dependencies: Option<BTreeMap<String, MaybeWorkspaceDependency>>,
#[serde(rename = "dev_dependencies")]
dev_dependencies2: Option<BTreeMap<String, MaybeWorkspaceDependency>>,
}

#[derive(Deserialize, Serialize, Debug, Clone)]
#[serde(expecting = "a lints table")]
#[serde(rename_all = "kebab-case")]
pub struct MaybeWorkspaceLints {
#[serde(skip_serializing_if = "is_false")]
#[serde(deserialize_with = "bool_no_false", default)]
Expand Down