Skip to content

Commit 96fe1c9

Browse files
committed
Auto merge of #12584 - epage:lints, r=arlosi
fix(lints): Fail when overriding inherited lints ### What does this PR try to resolve? Overriding of inherited lints was reserved for the future but as pointed out in #12115 (comment), we aren't failing on these when we should but silently ignoring the overrides. This turns it into a hard error. In fixing this, I had to add a `#[serde(expecting)]` attribute to maintain behavior on an error case (otherwise it would say "expecting struct WorkspaceLints"). Since this drew the error message to my attention, I also tweaked it to make it more specific. ### How should we test and review this PR? Commits are broken down by the relevant tests and fixes to make the intended behavior changes obvious.
2 parents 333ca23 + 6568e2c commit 96fe1c9

File tree

2 files changed

+79
-27
lines changed

2 files changed

+79
-27
lines changed

src/cargo/util/toml/mod.rs

+34-26
Original file line numberDiff line numberDiff line change
@@ -1475,29 +1475,6 @@ impl<'de> de::Deserialize<'de> for MaybeWorkspaceBtreeMap {
14751475
}
14761476
}
14771477

1478-
type MaybeWorkspaceLints = MaybeWorkspace<TomlLints, TomlWorkspaceField>;
1479-
1480-
impl<'de> de::Deserialize<'de> for MaybeWorkspaceLints {
1481-
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
1482-
where
1483-
D: de::Deserializer<'de>,
1484-
{
1485-
let value = serde_value::Value::deserialize(deserializer)?;
1486-
1487-
if let Ok(w) = TomlWorkspaceField::deserialize(
1488-
serde_value::ValueDeserializer::<D::Error>::new(value.clone()),
1489-
) {
1490-
return if w.workspace() {
1491-
Ok(MaybeWorkspace::Workspace(w))
1492-
} else {
1493-
Err(de::Error::custom("`workspace` cannot be false"))
1494-
};
1495-
}
1496-
TomlLints::deserialize(serde_value::ValueDeserializer::<D::Error>::new(value))
1497-
.map(MaybeWorkspace::Defined)
1498-
}
1499-
}
1500-
15011478
#[derive(Deserialize, Serialize, Copy, Clone, Debug)]
15021479
pub struct TomlWorkspaceField {
15031480
#[serde(deserialize_with = "bool_no_false")]
@@ -2277,7 +2254,7 @@ impl TomlManifest {
22772254

22782255
let lints =
22792256
parse_unstable_lints::<MaybeWorkspaceLints>(me.lints.clone(), config, cx.warnings)?
2280-
.map(|mw| mw.resolve("lints", || inherit()?.lints()))
2257+
.map(|mw| mw.resolve(|| inherit()?.lints()))
22812258
.transpose()?;
22822259
let lints = verify_lints(lints)?;
22832260
let default = TomlLints::default();
@@ -2579,8 +2556,7 @@ impl TomlManifest {
25792556
.badges
25802557
.as_ref()
25812558
.map(|_| MaybeWorkspace::Defined(metadata.badges.clone())),
2582-
lints: lints
2583-
.map(|lints| toml::Value::try_from(MaybeWorkspaceLints::Defined(lints)).unwrap()),
2559+
lints: lints.map(|lints| toml::Value::try_from(lints).unwrap()),
25842560
};
25852561
let mut manifest = Manifest::new(
25862562
summary,
@@ -3522,6 +3498,38 @@ impl fmt::Debug for PathValue {
35223498
}
35233499
}
35243500

3501+
#[derive(Deserialize, Serialize, Debug, Clone)]
3502+
#[serde(expecting = "a lints table")]
3503+
pub struct MaybeWorkspaceLints {
3504+
#[serde(skip_serializing_if = "is_false")]
3505+
#[serde(deserialize_with = "bool_no_false", default)]
3506+
workspace: bool,
3507+
#[serde(flatten)]
3508+
lints: TomlLints,
3509+
}
3510+
3511+
fn is_false(b: &bool) -> bool {
3512+
!b
3513+
}
3514+
3515+
impl MaybeWorkspaceLints {
3516+
fn resolve<'a>(
3517+
self,
3518+
get_ws_inheritable: impl FnOnce() -> CargoResult<TomlLints>,
3519+
) -> CargoResult<TomlLints> {
3520+
if self.workspace {
3521+
if !self.lints.is_empty() {
3522+
anyhow::bail!("cannot override `workspace.lints` in `lints`, either remove the overrides or `lints.workspace = true` and manually specify the lints");
3523+
}
3524+
get_ws_inheritable().with_context(|| {
3525+
"error inheriting `lints` from workspace root manifest's `workspace.lints`"
3526+
})
3527+
} else {
3528+
Ok(self.lints)
3529+
}
3530+
}
3531+
}
3532+
35253533
pub type TomlLints = BTreeMap<String, TomlToolLints>;
35263534

35273535
pub type TomlToolLints = BTreeMap<String, TomlLint>;

tests/testsuite/lints.rs

+45-1
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ fn malformed_on_nightly() {
172172
error: failed to parse manifest[..]
173173
174174
Caused by:
175-
invalid type: integer `20`, expected a map
175+
invalid type: integer `20`, expected a lints table
176176
",
177177
)
178178
.run();
@@ -413,6 +413,50 @@ error: usage of an `unsafe` block
413413
.run();
414414
}
415415

416+
#[cargo_test]
417+
fn workspace_and_package_lints() {
418+
let foo = project()
419+
.file(
420+
"Cargo.toml",
421+
r#"
422+
[package]
423+
name = "foo"
424+
version = "0.0.1"
425+
authors = []
426+
427+
[lints]
428+
workspace = true
429+
[lints.rust]
430+
"unsafe_code" = "allow"
431+
432+
[workspace.lints.rust]
433+
"unsafe_code" = "deny"
434+
"#,
435+
)
436+
.file(
437+
"src/lib.rs",
438+
"
439+
pub fn foo(num: i32) -> u32 {
440+
unsafe { std::mem::transmute(num) }
441+
}
442+
",
443+
)
444+
.build();
445+
446+
foo.cargo("check -Zlints")
447+
.masquerade_as_nightly_cargo(&["lints"])
448+
.with_status(101)
449+
.with_stderr(
450+
"\
451+
[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
452+
453+
Caused by:
454+
cannot override `workspace.lints` in `lints`, either remove the overrides or `lints.workspace = true` and manually specify the lints
455+
",
456+
)
457+
.run();
458+
}
459+
416460
#[cargo_test]
417461
fn attribute_has_precedence() {
418462
let foo = project()

0 commit comments

Comments
 (0)