From 13255d0fc1079d3d98b1763c399e7d8956cd3821 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 8 Nov 2024 10:21:29 -0600 Subject: [PATCH 1/4] fix(schemas): Support deserializing str's as well as Strings Not needed for `toml` but could be useful for other formats. --- crates/cargo-util-schemas/src/manifest/mod.rs | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/crates/cargo-util-schemas/src/manifest/mod.rs b/crates/cargo-util-schemas/src/manifest/mod.rs index 8442e62cc52..0db29ad492b 100644 --- a/crates/cargo-util-schemas/src/manifest/mod.rs +++ b/crates/cargo-util-schemas/src/manifest/mod.rs @@ -418,6 +418,13 @@ impl<'de> de::Deserialize<'de> for InheritableString { Ok(InheritableString::Value(value)) } + fn visit_str(self, value: &str) -> Result + where + E: de::Error, + { + self.visit_string(value.to_owned()) + } + fn visit_map(self, map: V) -> Result where V: de::MapAccess<'de>, @@ -454,6 +461,13 @@ impl<'de> de::Deserialize<'de> for InheritableRustVersion { Ok(InheritableRustVersion::Value(value)) } + fn visit_str(self, value: &str) -> Result + where + E: de::Error, + { + self.visit_string(value.to_owned()) + } + fn visit_map(self, map: V) -> Result where V: de::MapAccess<'de>, @@ -533,6 +547,13 @@ impl<'de> de::Deserialize<'de> for InheritableStringOrBool { StringOrBool::deserialize(string).map(InheritableField::Value) } + fn visit_str(self, value: &str) -> Result + where + E: de::Error, + { + self.visit_string(value.to_owned()) + } + fn visit_map(self, map: V) -> Result where V: de::MapAccess<'de>, From 650fb9d8f64d57d73417206d58b8208fb4af1b04 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 8 Nov 2024 10:19:04 -0600 Subject: [PATCH 2/4] style: Clarify code intention --- src/cargo/core/resolver/dep_cache.rs | 1 - src/cargo/util/toml/mod.rs | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index e0964fcc07d..7d584d2c8bd 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -207,7 +207,6 @@ impl<'a> RegistryQueryer<'a> { } } - let first_version = first_version; self.version_prefs.sort_summaries(&mut ret, first_version); let out = Poll::Ready(Rc::new(ret)); diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index edbfe996292..be8e7512776 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -507,7 +507,7 @@ fn normalize_toml( normalized_toml.badges = original_toml.badges.clone(); } else { - for field in original_toml.requires_package() { + if let Some(field) = original_toml.requires_package().next() { bail!("this virtual manifest specifies a `{field}` section, which is not allowed"); } } From 49d869682369909fc80aeed14bff040540c97b2a Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 8 Nov 2024 13:58:45 -0600 Subject: [PATCH 3/4] style: Workaround clippy --- crates/rustfix/src/replace.rs | 1 + src/cargo/util/mod.rs | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/rustfix/src/replace.rs b/crates/rustfix/src/replace.rs index 3eb130203d0..f1a2e7eca2c 100644 --- a/crates/rustfix/src/replace.rs +++ b/crates/rustfix/src/replace.rs @@ -255,6 +255,7 @@ mod tests { } #[test] + #[allow(clippy::reversed_empty_ranges)] fn replace_invalid_range() { let mut d = Data::new(b"foo!"); diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 3236920da61..e2f32048d6e 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -176,8 +176,8 @@ mod test { ); assert_eq!(human_readable_bytes(1024 * 1024 * 1024), (1., "GiB")); assert_eq!( - human_readable_bytes((1024. * 1024. * 1024. * 3.1415) as u64), - (3.1415, "GiB") + human_readable_bytes((1024. * 1024. * 1024. * 1.2345) as u64), + (1.2345, "GiB") ); assert_eq!(human_readable_bytes(1024 * 1024 * 1024 * 1024), (1., "TiB")); assert_eq!( From fd544063ca9fec325d5c4ca64ad9f2c75fa02f73 Mon Sep 17 00:00:00 2001 From: Ed Page Date: Fri, 8 Nov 2024 08:37:20 -0600 Subject: [PATCH 4/4] chore(ci): Check for clippy `correctness` The fact that we don't check for `derive_ord_xor_partial_ord ` almost bit us in #14663. Instead of just enabling this one lint, I figured it'd be good to audit all of the `deny` by default lints. That is long enough that I was concerned about maintaining it (or bikeshedding which to enable or disable). All `deny` by default lints are `correctness` lints and I figure we could just enable the group. We normally opt-in to individual clippy lints. From what I remember of that conversation, it mostly stems from how liberal clippy is with making a lint `warn` by default. It also makes an unpinned CI more brittle. I figured clippy is more conservative about `deny` by default lints and slower to add them that this is unlikely to be a problem. --- Cargo.toml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 3245a05ca0b..ee8eecdfeb5 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -121,7 +121,8 @@ rust_2018_idioms = "warn" # TODO: could this be removed? private_intra_doc_links = "allow" [workspace.lints.clippy] -all = { level = "allow", priority = -1 } +all = { level = "allow", priority = -2 } +correctness = { level = "warn", priority = -1 } dbg_macro = "warn" disallowed_methods = "warn" print_stderr = "warn"