diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f3ed4a18d..fac8924863 100644 --- a/.github/workflows/ci.yml +++ b/.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 diff --git a/.github/workflows/deploy_collab.yml b/.github/workflows/deploy_collab.yml index b866d9080c..220c4ca6f7 100644 --- a/.github/workflows/deploy_collab.yml +++ b/.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 diff --git a/.github/workflows/release_nightly.yml b/.github/workflows/release_nightly.yml index 1a7a0f704d..6e34e859b5 100644 --- a/.github/workflows/release_nightly.yml +++ b/.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' diff --git a/crates/gpui/build.rs b/crates/gpui/build.rs index 6d6f384bc5..8f63787c8b 100644 --- a/crates/gpui/build.rs +++ b/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"; diff --git a/tooling/xtask/src/main.rs b/tooling/xtask/src/main.rs index 1fed1574b3..6126978e57 100644 --- a/tooling/xtask/src/main.rs +++ b/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 clippy`). + #[arg(long, short)] + package: Option, } 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::>() .join(" ") );