move clippy args to [workspace.lints.clippy] (#5715)

This moves our Clippy configuration out of xtask and into Cargo's
relatively-new lint configuration functionality.
https://blog.rust-lang.org/2023/11/16/Rust-1.74.0.html#lint-configuration-through-cargo

Unfortunately this requires littering `[lints] workspace = true` in
every Cargo.toml, at least until implicit workspace inheritance is
figured out. This adds a check to `cargo xtask check-workspace-deps` to
ensure this is the case in CI.

As part of this I removed the `#![allow(clippy::style)]` attributes from
nexus and sled-agent, which revealed that this was overriding any style
lints we re-enabled in the xtask code.

One issue is that workspace crates can't override these lints yet
(rust-lang/cargo#13157), but they can continue
to override them with `#![allow(clippy::whatever)]` attributes.

Author: iliana

Cargo.toml

@@ -162,6 +162,48 @@ default-members = [
 ]
 resolver = "2"
 
+#
+# Tree-wide lint configuration.
+# https://doc.rust-lang.org/stable/cargo/reference/manifest.html#the-lints-section
+#
+# For a list of Clippy lints, see
+# https://rust-lang.github.io/rust-clippy/master.
+#
+[workspace.lints.clippy]
+# Clippy's style nits are useful, but not worth keeping in CI.
+style = { level = "allow", priority = -1 }
+# But continue to warn on anything in the "disallowed_" namespace.
+disallowed_macros = "warn"
+disallowed_methods = "warn"
+disallowed_names = "warn"
+disallowed_script_idents = "warn"
+disallowed_types = "warn"
+# Warn on some more style lints that are relatively stable and make sense.
+iter_cloned_collect = "warn"
+iter_next_slice = "warn"
+iter_nth = "warn"
+iter_nth_zero = "warn"
+iter_skip_next = "warn"
+len_zero = "warn"
+redundant_field_names = "warn"
+# `declare_interior_mutable_const` is classified as a style lint, but it can
+# identify real bugs (e.g., declarying a `const Atomic` and using it like
+# a `static Atomic`). However, it is also subject to false positives (e.g.,
+# idiomatically declaring a static array of atomics uses `const Atomic`). We
+# warn on this to catch the former, and expect any uses of the latter to allow
+# this locally.
+#
+# Note: any const value with a type containing a `bytes::Bytes` hits this lint,
+# and you should `#![allow]` it for now. This is most likely to be seen with
+# `http::header::{HeaderName, HeaderValue}`. This is a Clippy bug which will be
+# fixed in the Rust 1.80 toolchain (rust-lang/rust-clippy#12691).
+declare_interior_mutable_const = "warn"
+# Also warn on casts, preferring explicit conversions instead.
+#
+# We'd like to warn on lossy casts in the future, but lossless casts are the
+# easiest ones to convert over.
+cast_lossless = "warn"
+
 [workspace.dependencies]
 anyhow = "1.0"
 anstyle = "1.0.6"

api_identity/Cargo.toml

@@ -10,6 +10,9 @@ license = "MPL-2.0"
 [lib]
 proc-macro = true
 
+[lints]
+workspace = true
+
 [dependencies]
 proc-macro2.workspace = true
 quote.workspace = true

bootstore/Cargo.toml

@@ -8,6 +8,9 @@ license = "MPL-2.0"
 [build-dependencies]
 omicron-rpaths.workspace = true
 
+[lints]
+workspace = true
+
 [dependencies]
 bytes.workspace = true
 camino.workspace = true

caboose-util/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow.workspace = true
 hubtools.workspace = true

certificates/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 display-error-chain.workspace = true
 foreign-types.workspace = true

clients/bootstrap-agent-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 omicron-common.workspace = true
 progenitor.workspace = true

clients/ddm-admin-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 either.workspace = true
 progenitor-client.workspace = true

clients/dns-service-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow.workspace = true
 chrono.workspace = true

clients/dpd-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 futures.workspace = true
 progenitor-client.workspace = true

clients/gateway-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 base64.workspace = true
 chrono.workspace = true

clients/installinator-artifact-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 installinator-common.workspace = true
 progenitor.workspace = true

clients/nexus-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 chrono.workspace = true
 futures.workspace = true

clients/oxide-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow.workspace = true
 base64.workspace = true

clients/oximeter-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 chrono.workspace = true
 futures.workspace = true

clients/sled-agent-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow.workspace = true
 async-trait.workspace = true

clients/wicketd-client/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 chrono.workspace = true
 installinator-common.workspace = true

common/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 # NOTE:
 #
 # This crate is depended on by several other workspaces! Be careful of adding

dev-tools/crdb-seed/Cargo.toml

@@ -5,6 +5,9 @@ edition = "2021"
 license = "MPL-2.0"
 readme = "README.md"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow.workspace = true
 dropshot.workspace = true

dev-tools/omdb/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [build-dependencies]
 omicron-rpaths.workspace = true
 

dev-tools/omicron-dev/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [build-dependencies]
 omicron-rpaths.workspace = true
 

dev-tools/oxlog/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow.workspace = true
 camino.workspace = true

dev-tools/reconfigurator-cli/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [build-dependencies]
 omicron-rpaths.workspace = true
 

dev-tools/xtask/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow.workspace = true
 camino.workspace = true

dev-tools/xtask/src/check_workspace_deps.rs

@@ -31,14 +31,24 @@ pub fn run_cmd() -> Result<()> {
 
     // Iterate the workspace packages and fill out the maps above.
     for pkg_info in workspace.workspace_packages() {
+        let manifest_path = &pkg_info.manifest_path;
+        let manifest = read_cargo_toml(manifest_path)?;
+
+        // Check that `[lints] workspace = true` is set.
+        if !manifest.lints.map(|lints| lints.workspace).unwrap_or(false) {
+            eprintln!(
+                "error: package {:?} does not have `[lints] workspace = true` set",
+                pkg_info.name
+            );
+            nerrors += 1;
+        }
+
         if pkg_info.name == WORKSPACE_HACK_PACKAGE_NAME {
             // Skip over workspace-hack because hakari doesn't yet support
             // workspace deps: https://github.com/guppy-rs/guppy/issues/7
             continue;
         }
 
-        let manifest_path = &pkg_info.manifest_path;
-        let manifest = read_cargo_toml(manifest_path)?;
         for tree in [
             &manifest.dependencies,
             &manifest.dev_dependencies,

dev-tools/xtask/src/clippy.rs

@@ -34,54 +34,7 @@ pub fn run_cmd(args: ClippyArgs) -> Result<()> {
         //
         // We disallow warnings by default.
         .arg("--deny")
-        .arg("warnings")
-        // Clippy's style nits are useful, but not worth keeping in CI.  This
-        // override belongs in src/lib.rs, and it is there, but that doesn't
-        // reliably work due to rust-lang/rust-clippy#6610.
-        .arg("--allow")
-        .arg("clippy::style")
-        // But continue to warn on anything in the "disallowed_" namespace.
-        // (These will be turned into errors by `--deny warnings` above.)
-        .arg("--warn")
-        .arg("clippy::disallowed_macros")
-        .arg("--warn")
-        .arg("clippy::disallowed_methods")
-        .arg("--warn")
-        .arg("clippy::disallowed_names")
-        .arg("--warn")
-        .arg("clippy::disallowed_script_idents")
-        .arg("--warn")
-        .arg("clippy::disallowed_types")
-        // Warn on some more style lints that are relatively stable and make
-        // sense.
-        .arg("--warn")
-        .arg("clippy::iter_cloned_collect")
-        .arg("--warn")
-        .arg("clippy::iter_next_slice")
-        .arg("--warn")
-        .arg("clippy::iter_nth")
-        .arg("--warn")
-        .arg("clippy::iter_nth_zero")
-        .arg("--warn")
-        .arg("clippy::iter_skip_next")
-        .arg("--warn")
-        .arg("clippy::len_zero")
-        .arg("--warn")
-        .arg("clippy::redundant_field_names")
-        // `declare_interior_mutable_const` is classified as a style lint, but
-        // it can identify real bugs (e.g., declarying a `const Atomic` and
-        // using it like a `static Atomic`). However, it is also subject to
-        // false positives (e.g., idiomatically declaring a static array of
-        // atomics uses `const Atomic`). We warn on this to catch the former,
-        // and expect any uses of the latter to allow this locally.
-        .arg("--warn")
-        .arg("clippy::declare_interior_mutable_const")
-        // Also warn on casts, preferring explicit conversions instead.
-        //
-        // We'd like to warn on lossy casts in the future, but lossless casts
-        // are the easiest ones to convert over.
-        .arg("--warn")
-        .arg("clippy::cast_lossless");
+        .arg("warnings");
 
     eprintln!(
         "running: {:?} {}",

dns-server/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow.workspace = true
 camino.workspace = true

end-to-end-tests/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow = { workspace = true, features = ["backtrace"] }
 async-trait.workspace = true

gateway-cli/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 anyhow.workspace = true
 clap.workspace = true

gateway-test-utils/Cargo.toml

@@ -4,6 +4,9 @@ version = "0.1.0"
 edition = "2021"
 license = "MPL-2.0"
 
+[lints]
+workspace = true
+
 [dependencies]
 camino.workspace = true
 dropshot.workspace = true