Deny all Clippy warnings (#8720)

This PR makes Clippy deny all warnings across the workspace.

We now enumerate all of the rules that have violations and temporarily
allow them, with the goal being to drive the list down over time.

On Windows we don't yet use `--deny warnings`, as the Windows build
still has some warnings.

Release Notes:

- N/A

.github/workflows/ci.yml

@@ -93,8 +93,7 @@ jobs:
           fi
 
       - name: cargo clippy
-        shell: bash -euxo pipefail {0}
-        run: script/clippy
+        run: cargo xtask clippy
 
       - name: Run tests
         uses: ./.github/actions/run_tests
@@ -126,8 +125,7 @@ jobs:
         run: script/linux
 
       - name: cargo clippy
-        shell: bash -euxo pipefail {0}
-        run: script/clippy
+        run: cargo xtask clippy
 
       - name: Build Zed
         run: cargo build -p zed
@@ -149,8 +147,7 @@ jobs:
           save-if: ${{ github.ref == 'refs/heads/main' }}
 
       - name: cargo clippy
-        shell: bash -euxo pipefail {0}
-        run: script/clippy
+        run: cargo xtask clippy
 
       - name: Build Zed
         run: cargo build -p zed

.github/workflows/deploy_collab.yml

@@ -28,8 +28,7 @@ jobs:
         uses: ./.github/actions/check_style
 
       - name: Run clippy
-        shell: bash -euxo pipefail {0}
-        run: script/clippy
+        run: cargo xtask clippy
 
   tests:
     name: Run tests

.github/workflows/release_nightly.yml

@@ -32,8 +32,7 @@ jobs:
         uses: ./.github/actions/check_style
 
       - name: Run clippy
-        shell: bash -euxo pipefail {0}
-        run: script/clippy
+        run: cargo xtask clippy
   tests:
     name: Run tests
     if: github.repository_owner == 'zed-industries'

crates/gpui/build.rs

@@ -125,7 +125,7 @@ fn emit_stitched_shaders(header_path: &Path) {
         let shader_contents = std::fs::read_to_string(shader_path)?;
         let stitched_contents = format!("{header_contents}\n{shader_contents}");
         let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()).join("stitched_shaders.metal");
-        let _ = std::fs::write(&out_path, stitched_contents)?;
+        std::fs::write(&out_path, stitched_contents)?;
         Ok(out_path)
     }
     let shader_source_path = "./src/platform/mac/shaders.metal";

tooling/xtask/src/main.rs

@@ -26,31 +26,129 @@ fn main() -> Result<()> {
 
 #[derive(Parser)]
 struct ClippyArgs {
-    /// Whether to deny warnings (`clippy --deny warnings`).
+    /// Automatically apply lint suggestions (`clippy --fix`).
     #[arg(long)]
-    deny_warnings: bool,
+    fix: bool,
+
+    /// The package to run Clippy against (`cargo -p <PACKAGE> clippy`).
+    #[arg(long, short)]
+    package: Option<String>,
 }
 
 fn run_clippy(args: ClippyArgs) -> Result<()> {
     let cargo = std::env::var("CARGO").unwrap_or_else(|_| "cargo".to_string());
 
     let mut clippy_command = Command::new(&cargo);
+    clippy_command.arg("clippy");
+
+    if let Some(package) = args.package {
+        clippy_command.args(["--package", &package]);
+    } else {
+        clippy_command.arg("--workspace");
+    }
+
     clippy_command
-        .arg("clippy")
-        .arg("--workspace")
         .arg("--release")
         .arg("--all-targets")
         .arg("--all-features");
 
-    clippy_command.arg("--");
-
-    if args.deny_warnings {
-        clippy_command.args(["--deny", "warnings"]);
+    if args.fix {
+        clippy_command.arg("--fix");
     }
 
-    // Allow all Clippy lints by default, as we have a lot of violations at the moment.
-    // We can tighten things up once we have a better handle on them.
-    clippy_command.args(["--allow", "clippy::all"]);
+    clippy_command.arg("--");
+
+    // Deny all warnings.
+    // We don't do this yet on Windows, as it still has some warnings present.
+    #[cfg(not(target_os = "windows"))]
+    clippy_command.args(["--deny", "warnings"]);
+
+    /// These are all of the rules that currently have violations in the Zed
+    /// codebase.
+    ///
+    /// We'll want to drive this list down by either:
+    /// 1. fixing violations of the rule and begin enforcing it
+    /// 2. deciding we want to allow the rule permanently, at which point
+    ///    we should codify that separately in this script.
+    const MIGRATORY_RULES_TO_ALLOW: &[&str] = &[
+        // There's a bunch of rules currently failing in the `style` group, so
+        // allow all of those, for now.
+        "clippy::style",
+        // Individual rules that have violations in the codebase:
+        "clippy::almost_complete_range",
+        "clippy::arc_with_non_send_sync",
+        "clippy::await_holding_lock",
+        "clippy::bind_instead_of_map",
+        "clippy::bool_comparison",
+        "clippy::borrow_deref_ref",
+        "clippy::borrowed_box",
+        "clippy::cast_abs_to_unsigned",
+        "clippy::clone_on_copy",
+        "clippy::cmp_owned",
+        "clippy::crate_in_macro_def",
+        "clippy::default_constructed_unit_structs",
+        "clippy::derivable_impls",
+        "clippy::derive_ord_xor_partial_ord",
+        "clippy::drain_collect",
+        "clippy::eq_op",
+        "clippy::expect_fun_call",
+        "clippy::explicit_auto_deref",
+        "clippy::explicit_counter_loop",
+        "clippy::extra_unused_lifetimes",
+        "clippy::filter_map_identity",
+        "clippy::identity_op",
+        "clippy::implied_bounds_in_impls",
+        "clippy::iter_kv_map",
+        "clippy::iter_overeager_cloned",
+        "clippy::let_underscore_future",
+        "clippy::manual_find",
+        "clippy::manual_flatten",
+        "clippy::map_entry",
+        "clippy::map_flatten",
+        "clippy::map_identity",
+        "clippy::needless_arbitrary_self_type",
+        "clippy::needless_borrowed_reference",
+        "clippy::needless_lifetimes",
+        "clippy::needless_option_as_deref",
+        "clippy::needless_question_mark",
+        "clippy::needless_update",
+        "clippy::never_loop",
+        "clippy::non_canonical_clone_impl",
+        "clippy::non_canonical_partial_ord_impl",
+        "clippy::nonminimal_bool",
+        "clippy::option_as_ref_deref",
+        "clippy::option_map_unit_fn",
+        "clippy::redundant_closure_call",
+        "clippy::redundant_guards",
+        "clippy::redundant_locals",
+        "clippy::reversed_empty_ranges",
+        "clippy::search_is_some",
+        "clippy::single_char_pattern",
+        "clippy::single_range_in_vec_init",
+        "clippy::suspicious_to_owned",
+        "clippy::to_string_in_format_args",
+        "clippy::too_many_arguments",
+        "clippy::type_complexity",
+        "clippy::unit_arg",
+        "clippy::unnecessary_cast",
+        "clippy::unnecessary_filter_map",
+        "clippy::unnecessary_find_map",
+        "clippy::unnecessary_operation",
+        "clippy::unnecessary_to_owned",
+        "clippy::unnecessary_unwrap",
+        "clippy::useless_conversion",
+        "clippy::useless_format",
+        "clippy::vec_init_then_push",
+    ];
+
+    // When fixing violations automatically we don't care about the
+    // rules we're already violating, since it may be possible to
+    // have them fixed automatically.
+    if !args.fix {
+        for rule in MIGRATORY_RULES_TO_ALLOW {
+            clippy_command.args(["--allow", rule]);
+        }
+    }
 
     // Deny `dbg!` and `todo!`s.
     clippy_command
@@ -61,7 +159,7 @@ fn run_clippy(args: ClippyArgs) -> Result<()> {
         "running: {cargo} {}",
         clippy_command
             .get_args()
-            .map(|arg| format!("{}", arg.to_str().unwrap()))
+            .map(|arg| arg.to_str().unwrap())
             .collect::<Vec<_>>()
             .join(" ")
     );