From ef6954cd017245f5576eddc76f4e368f165b9569 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 8 Jan 2024 08:27:34 -0600 Subject: [PATCH 1/4] test(lints): Check unused key status --- tests/testsuite/lints.rs | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/tests/testsuite/lints.rs b/tests/testsuite/lints.rs index 21139d00ace..f8f7e6e4abd 100644 --- a/tests/testsuite/lints.rs +++ b/tests/testsuite/lints.rs @@ -148,6 +148,36 @@ Caused by: .run(); } +#[cargo_test] +fn warn_on_unused_key() { + let foo = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + + [workspace.lints.rust] + rust-2018-idioms = { level = "allow", unused = true } + [lints.rust] + rust-2018-idioms = { level = "allow", unused = true } + "#, + ) + .file("src/lib.rs", "") + .build(); + + foo.cargo("check") + .with_stderr( + "\ +[WARNING] [CWD]/Cargo.toml: unused manifest key: workspace.lints.rust.rust-2018-idioms.unused +[CHECKING] foo v0.0.1 ([CWD]) +[FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s +", + ) + .run(); +} + #[cargo_test] fn fail_on_tool_injection() { let foo = project() From 4760ef782220c16bf464da4baa059dca2b217eea Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 8 Jan 2024 08:49:04 -0600 Subject: [PATCH 2/4] test(lints): Verify we can't un-inherit --- tests/testsuite/lints.rs | 45 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tests/testsuite/lints.rs b/tests/testsuite/lints.rs index f8f7e6e4abd..69ac8fb2db8 100644 --- a/tests/testsuite/lints.rs +++ b/tests/testsuite/lints.rs @@ -306,6 +306,51 @@ error: usage of an `unsafe` block .run(); } +#[cargo_test] +fn workspace_cant_be_false() { + let foo = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.1" + authors = [] + + [lints] + workspace = false + + [workspace.lints.rust] + "unsafe_code" = "deny" + "#, + ) + .file( + "src/lib.rs", + " +pub fn foo(num: i32) -> u32 { + unsafe { std::mem::transmute(num) } +} +", + ) + .build(); + + foo.cargo("check") + .with_status(101) + .with_stderr_contains( + "\ +error: failed to parse manifest at `[CWD]/Cargo.toml` + +Caused by: + TOML parse error at line 8, column 29 + | + 8 | workspace = false + | ^^^^^ + `workspace` cannot be false +", + ) + .run(); +} + #[cargo_test] fn workspace_lint_deny() { let foo = project() From e37a04a9d420787312229d858090004516423c89 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 8 Jan 2024 09:06:03 -0600 Subject: [PATCH 3/4] refactor(schema): Use dedicated type for 'workspace' field value --- crates/cargo-util-schemas/src/manifest.rs | 35 +++++++++++++++++------ 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest.rs b/crates/cargo-util-schemas/src/manifest.rs index 0bf921418c0..2c0048d8bad 100644 --- a/crates/cargo-util-schemas/src/manifest.rs +++ b/crates/cargo-util-schemas/src/manifest.rs @@ -410,11 +410,7 @@ impl<'de> de::Deserialize<'de> for InheritableBtreeMap { if let Ok(w) = TomlInheritedField::deserialize( serde_value::ValueDeserializer::::new(value.clone()), ) { - return if w.workspace { - Ok(InheritableField::Inherit(w)) - } else { - Err(de::Error::custom("`workspace` cannot be false")) - }; + return Ok(InheritableField::Inherit(w)); } BTreeMap::deserialize(serde_value::ValueDeserializer::::new(value)) .map(InheritableField::Value) @@ -424,13 +420,14 @@ impl<'de> de::Deserialize<'de> for InheritableBtreeMap { #[derive(Deserialize, Serialize, Copy, Clone, Debug)] #[serde(rename_all = "kebab-case")] pub struct TomlInheritedField { - #[serde(deserialize_with = "bool_no_false")] - workspace: bool, + workspace: WorkspaceValue, } impl TomlInheritedField { pub fn new() -> Self { - TomlInheritedField { workspace: true } + TomlInheritedField { + workspace: WorkspaceValue, + } } } @@ -449,6 +446,28 @@ fn bool_no_false<'de, D: de::Deserializer<'de>>(deserializer: D) -> Result for WorkspaceValue { + type Error = String; + fn try_from(other: bool) -> Result { + if other { + Ok(WorkspaceValue) + } else { + Err("`workspace` cannot be false".to_owned()) + } + } +} + +impl From for bool { + fn from(_: WorkspaceValue) -> bool { + true + } +} + #[derive(Serialize, Clone, Debug)] #[serde(untagged)] pub enum InheritableDependency { From 6e86530ae2500591ac0a70a9831494196f56a89c Mon Sep 17 00:00:00 2001 From: Ed Page Date: Mon, 8 Jan 2024 09:12:46 -0600 Subject: [PATCH 4/4] fix(manifest): Provide unused key warnings for lints table The use of `flatten` was getting in the way of `serde_ignored`. A common workaround is to add our own `unused` tracking but that would cause duplicates with `workspace.lints` (or we'd just ignore it). Since the manual deserializer was relatively simple, I went that route. Fixes #12917 --- crates/cargo-util-schemas/src/manifest.rs | 62 ++++++++++++++++++----- tests/testsuite/lints.rs | 1 + 2 files changed, 50 insertions(+), 13 deletions(-) diff --git a/crates/cargo-util-schemas/src/manifest.rs b/crates/cargo-util-schemas/src/manifest.rs index 2c0048d8bad..c17b47d3c6f 100644 --- a/crates/cargo-util-schemas/src/manifest.rs +++ b/crates/cargo-util-schemas/src/manifest.rs @@ -437,15 +437,6 @@ impl Default for TomlInheritedField { } } -fn bool_no_false<'de, D: de::Deserializer<'de>>(deserializer: D) -> Result { - let b: bool = Deserialize::deserialize(deserializer)?; - if b { - Ok(b) - } else { - Err(de::Error::custom("`workspace` cannot be false")) - } -} - #[derive(Deserialize, Serialize, Copy, Clone, Debug)] #[serde(try_from = "bool")] #[serde(into = "bool")] @@ -1269,12 +1260,9 @@ impl TomlPlatform { } } -#[derive(Deserialize, Serialize, Debug, Clone)] -#[serde(expecting = "a lints table")] -#[serde(rename_all = "kebab-case")] +#[derive(Serialize, Debug, Clone)] pub struct InheritableLints { #[serde(skip_serializing_if = "is_false")] - #[serde(deserialize_with = "bool_no_false", default)] pub workspace: bool, #[serde(flatten)] pub lints: TomlLints, @@ -1284,6 +1272,54 @@ fn is_false(b: &bool) -> bool { !b } +impl<'de> Deserialize<'de> for InheritableLints { + fn deserialize(deserializer: D) -> Result + where + D: de::Deserializer<'de>, + { + struct InheritableLintsVisitor; + + impl<'de> de::Visitor<'de> for InheritableLintsVisitor { + // The type that our Visitor is going to produce. + type Value = InheritableLints; + + // Format a message stating what data this Visitor expects to receive. + fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str("a lints table") + } + + // Deserialize MyMap from an abstract "map" provided by the + // Deserializer. The MapAccess input is a callback provided by + // the Deserializer to let us see each entry in the map. + fn visit_map(self, mut access: M) -> Result + where + M: de::MapAccess<'de>, + { + let mut lints = TomlLints::new(); + let mut workspace = false; + + // While there are entries remaining in the input, add them + // into our map. + while let Some(key) = access.next_key()? { + if key == "workspace" { + workspace = match access.next_value()? { + Some(WorkspaceValue) => true, + None => false, + }; + } else { + let value = access.next_value()?; + lints.insert(key, value); + } + } + + Ok(InheritableLints { workspace, lints }) + } + } + + deserializer.deserialize_map(InheritableLintsVisitor) + } +} + pub type TomlLints = BTreeMap; pub type TomlToolLints = BTreeMap; diff --git a/tests/testsuite/lints.rs b/tests/testsuite/lints.rs index 69ac8fb2db8..980ec091947 100644 --- a/tests/testsuite/lints.rs +++ b/tests/testsuite/lints.rs @@ -170,6 +170,7 @@ fn warn_on_unused_key() { foo.cargo("check") .with_stderr( "\ +[WARNING] [CWD]/Cargo.toml: unused manifest key: lints.rust.rust-2018-idioms.unused [WARNING] [CWD]/Cargo.toml: unused manifest key: workspace.lints.rust.rust-2018-idioms.unused [CHECKING] foo v0.0.1 ([CWD]) [FINISHED] dev [unoptimized + debuginfo] target(s) in [..]s