Skip to content

Commit 2e7fc43

Browse files
authored
chore(ci): Check for clippy correctness (#14796)
### What does this PR try to resolve? The fact that we don't check for `derive_ord_xor_partial_ord ` almost bit us in #14663. If we used clippy's default lint levels, this would have been caught automatically. 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. As for what existing problems this found, - Some missing serde functions that would be useful for formats besides toml (since `toml` never uses borrowed strings) - Code that behaves differently than the syntax says (a 0-1 iteration loop) - A redundant assignment (wasn't even removing `mut`ness ### How should we test and review this PR? ### Additional information
2 parents 31c96be + fd54406 commit 2e7fc43

File tree

6 files changed

+27
-5
lines changed

6 files changed

+27
-5
lines changed

Cargo.toml

+2-1
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ rust_2018_idioms = "warn" # TODO: could this be removed?
121121
private_intra_doc_links = "allow"
122122

123123
[workspace.lints.clippy]
124-
all = { level = "allow", priority = -1 }
124+
all = { level = "allow", priority = -2 }
125+
correctness = { level = "warn", priority = -1 }
125126
dbg_macro = "warn"
126127
disallowed_methods = "warn"
127128
print_stderr = "warn"

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

+21
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,13 @@ impl<'de> de::Deserialize<'de> for InheritableString {
418418
Ok(InheritableString::Value(value))
419419
}
420420

421+
fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
422+
where
423+
E: de::Error,
424+
{
425+
self.visit_string(value.to_owned())
426+
}
427+
421428
fn visit_map<V>(self, map: V) -> Result<Self::Value, V::Error>
422429
where
423430
V: de::MapAccess<'de>,
@@ -454,6 +461,13 @@ impl<'de> de::Deserialize<'de> for InheritableRustVersion {
454461
Ok(InheritableRustVersion::Value(value))
455462
}
456463

464+
fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
465+
where
466+
E: de::Error,
467+
{
468+
self.visit_string(value.to_owned())
469+
}
470+
457471
fn visit_map<V>(self, map: V) -> Result<Self::Value, V::Error>
458472
where
459473
V: de::MapAccess<'de>,
@@ -533,6 +547,13 @@ impl<'de> de::Deserialize<'de> for InheritableStringOrBool {
533547
StringOrBool::deserialize(string).map(InheritableField::Value)
534548
}
535549

550+
fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
551+
where
552+
E: de::Error,
553+
{
554+
self.visit_string(value.to_owned())
555+
}
556+
536557
fn visit_map<V>(self, map: V) -> Result<Self::Value, V::Error>
537558
where
538559
V: de::MapAccess<'de>,

crates/rustfix/src/replace.rs

+1
Original file line numberDiff line numberDiff line change
@@ -255,6 +255,7 @@ mod tests {
255255
}
256256

257257
#[test]
258+
#[allow(clippy::reversed_empty_ranges)]
258259
fn replace_invalid_range() {
259260
let mut d = Data::new(b"foo!");
260261

src/cargo/core/resolver/dep_cache.rs

-1
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,6 @@ impl<'a> RegistryQueryer<'a> {
207207
}
208208
}
209209

210-
let first_version = first_version;
211210
self.version_prefs.sort_summaries(&mut ret, first_version);
212211

213212
let out = Poll::Ready(Rc::new(ret));

src/cargo/util/mod.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -176,8 +176,8 @@ mod test {
176176
);
177177
assert_eq!(human_readable_bytes(1024 * 1024 * 1024), (1., "GiB"));
178178
assert_eq!(
179-
human_readable_bytes((1024. * 1024. * 1024. * 3.1415) as u64),
180-
(3.1415, "GiB")
179+
human_readable_bytes((1024. * 1024. * 1024. * 1.2345) as u64),
180+
(1.2345, "GiB")
181181
);
182182
assert_eq!(human_readable_bytes(1024 * 1024 * 1024 * 1024), (1., "TiB"));
183183
assert_eq!(

src/cargo/util/toml/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ fn normalize_toml(
507507

508508
normalized_toml.badges = original_toml.badges.clone();
509509
} else {
510-
for field in original_toml.requires_package() {
510+
if let Some(field) = original_toml.requires_package().next() {
511511
bail!("this virtual manifest specifies a `{field}` section, which is not allowed");
512512
}
513513
}

0 commit comments

Comments
 (0)