Skip to content

Commit 62eab55

Browse files
committed
refactor(toml): Make it more obvious to update package-dependent fields
Inspired by my having forgotten to add `[lints]` to the if sequence. Previously, we added a comment to suggest this but the further the code is, the harder it is to track. I considered a custom `Deserialize` impl, possibly through a new type, that would error. This would be the more "pure" solution. Unfortunately, this would also have worse errors because the errors would be reported to the `Deserializer` at the document-level, rather than directly on the individual fields. Well, we don't do on individual fields now but it is something we will soon be exploring.
1 parent 90d1ea8 commit 62eab55

File tree

3 files changed

+28
-49
lines changed

3 files changed

+28
-49
lines changed

crates/cargo-util-schemas/src/manifest.rs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,31 @@ pub struct TomlManifest {
4949
pub workspace: Option<TomlWorkspace>,
5050
pub badges: Option<InheritableBtreeMap>,
5151
pub lints: Option<InheritableLints>,
52-
// when adding new fields, be sure to check whether `to_virtual_manifest` should disallow them
52+
// when adding new fields, be sure to check whether `requires_package` should disallow them
5353
}
5454

5555
impl TomlManifest {
56+
pub fn requires_package(&self) -> impl Iterator<Item = &'static str> {
57+
[
58+
self.lib.as_ref().map(|_| "lib"),
59+
self.bin.as_ref().map(|_| "bin"),
60+
self.example.as_ref().map(|_| "example"),
61+
self.test.as_ref().map(|_| "test"),
62+
self.bench.as_ref().map(|_| "bench"),
63+
self.dependencies.as_ref().map(|_| "dependencies"),
64+
self.dev_dependencies().as_ref().map(|_| "dev-dependencies"),
65+
self.build_dependencies()
66+
.as_ref()
67+
.map(|_| "build-dependencies"),
68+
self.features.as_ref().map(|_| "features"),
69+
self.target.as_ref().map(|_| "target"),
70+
self.badges.as_ref().map(|_| "badges"),
71+
self.lints.as_ref().map(|_| "lints"),
72+
]
73+
.into_iter()
74+
.flatten()
75+
}
76+
5677
pub fn has_profiles(&self) -> bool {
5778
self.profile.is_some()
5879
}

src/cargo/util/toml/mod.rs

Lines changed: 2 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,49 +1073,8 @@ fn to_virtual_manifest(
10731073
root: &Path,
10741074
config: &Config,
10751075
) -> CargoResult<(VirtualManifest, Vec<PathBuf>)> {
1076-
if me.project.is_some() {
1077-
bail!("this virtual manifest specifies a [project] section, which is not allowed");
1078-
}
1079-
if me.package.is_some() {
1080-
bail!("this virtual manifest specifies a [package] section, which is not allowed");
1081-
}
1082-
if me.lib.is_some() {
1083-
bail!("this virtual manifest specifies a [lib] section, which is not allowed");
1084-
}
1085-
if me.bin.is_some() {
1086-
bail!("this virtual manifest specifies a [[bin]] section, which is not allowed");
1087-
}
1088-
if me.example.is_some() {
1089-
bail!("this virtual manifest specifies a [[example]] section, which is not allowed");
1090-
}
1091-
if me.test.is_some() {
1092-
bail!("this virtual manifest specifies a [[test]] section, which is not allowed");
1093-
}
1094-
if me.bench.is_some() {
1095-
bail!("this virtual manifest specifies a [[bench]] section, which is not allowed");
1096-
}
1097-
if me.dependencies.is_some() {
1098-
bail!("this virtual manifest specifies a [dependencies] section, which is not allowed");
1099-
}
1100-
if me.dev_dependencies().is_some() {
1101-
bail!("this virtual manifest specifies a [dev-dependencies] section, which is not allowed");
1102-
}
1103-
if me.build_dependencies().is_some() {
1104-
bail!(
1105-
"this virtual manifest specifies a [build-dependencies] section, which is not allowed"
1106-
);
1107-
}
1108-
if me.features.is_some() {
1109-
bail!("this virtual manifest specifies a [features] section, which is not allowed");
1110-
}
1111-
if me.target.is_some() {
1112-
bail!("this virtual manifest specifies a [target] section, which is not allowed");
1113-
}
1114-
if me.badges.is_some() {
1115-
bail!("this virtual manifest specifies a [badges] section, which is not allowed");
1116-
}
1117-
if me.lints.is_some() {
1118-
bail!("this virtual manifest specifies a [lints] section, which is not allowed");
1076+
for field in me.requires_package() {
1077+
bail!("this virtual manifest specifies a `{field}` section, which is not allowed");
11191078
}
11201079

11211080
let mut nested_paths = Vec::new();

tests/testsuite/workspaces.rs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2186,7 +2186,7 @@ fn ws_rustc_err() {
21862186

21872187
#[cargo_test]
21882188
fn ws_err_unused() {
2189-
for key in &[
2189+
for table in &[
21902190
"[lib]",
21912191
"[[bin]]",
21922192
"[[example]]",
@@ -2200,6 +2200,7 @@ fn ws_err_unused() {
22002200
"[badges]",
22012201
"[lints]",
22022202
] {
2203+
let key = table.trim_start_matches('[').trim_end_matches(']');
22032204
let p = project()
22042205
.file(
22052206
"Cargo.toml",
@@ -2208,9 +2209,8 @@ fn ws_err_unused() {
22082209
[workspace]
22092210
members = ["a"]
22102211
2211-
{}
2212+
{table}
22122213
"#,
2213-
key
22142214
),
22152215
)
22162216
.file("a/Cargo.toml", &basic_lib_manifest("a"))
@@ -2223,9 +2223,8 @@ fn ws_err_unused() {
22232223
[ERROR] failed to parse manifest at `[..]/foo/Cargo.toml`
22242224
22252225
Caused by:
2226-
this virtual manifest specifies a {} section, which is not allowed
2226+
this virtual manifest specifies a `{key}` section, which is not allowed
22272227
",
2228-
key
22292228
))
22302229
.run();
22312230
}

0 commit comments

Comments
 (0)