From 7b3e7ee3ccf9ae662ba572ae11b4e825f321b897 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Wed, 10 Jan 2024 23:23:52 +0200 Subject: [PATCH 1/5] Enfoce no dbg! and todo! in Rust code via clippy lints in the CI job --- .github/actions/check_formatting/action.yml | 15 ------------ .github/actions/check_style/action.yml | 24 +++++++++++++++++++ .github/workflows/ci.yml | 11 +++++---- .github/workflows/release_nightly.yml | 11 +++++---- .../collab/src/tests/channel_guest_tests.rs | 10 ++++---- crates/vim/src/editor_events.rs | 1 - 6 files changed, 40 insertions(+), 32 deletions(-) delete mode 100644 .github/actions/check_formatting/action.yml create mode 100644 .github/actions/check_style/action.yml diff --git a/.github/actions/check_formatting/action.yml b/.github/actions/check_formatting/action.yml deleted file mode 100644 index 7fef26407bd866babfb10c1aac6222968f54c1fd..0000000000000000000000000000000000000000 --- a/.github/actions/check_formatting/action.yml +++ /dev/null @@ -1,15 +0,0 @@ -name: 'Check formatting' -description: 'Checks code formatting use cargo fmt' - -runs: - using: "composite" - steps: - - name: Install Rust - shell: bash -euxo pipefail {0} - run: | - rustup set profile minimal - rustup update stable - - - name: cargo fmt - shell: bash -euxo pipefail {0} - run: cargo fmt --all -- --check diff --git a/.github/actions/check_style/action.yml b/.github/actions/check_style/action.yml new file mode 100644 index 0000000000000000000000000000000000000000..ff0481980816851ef6135c78f84b13b0cb21c09a --- /dev/null +++ b/.github/actions/check_style/action.yml @@ -0,0 +1,24 @@ +name: "Check formatting" +description: "Checks code formatting use cargo fmt" + +runs: + using: "composite" + steps: + - name: Install Rust + shell: bash -euxo pipefail {0} + run: | + rustup set profile minimal + rustup update stable + rustup component add clippy + + - name: cargo fmt + shell: bash -euxo pipefail {0} + run: cargo fmt --all -- --check + + - name: cargo clippy + shell: bash -euxo pipefail {0} + # clippy.toml is not currently supporting specifying allowed lints + # so specify those here, and disable the rest until Zed's workspace + # will have more fixes & suppression for the standard lint set + run: | + cargo clippy --workspace --all-targets --all-features -- -A clippy::all -D clippy::dbg_macro -D clippy::todo diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5ba25dbf947c14b485e7303cd12dd17f8a206927..3a92a744bb12d59fcf0539a4f8fb10f270899dd4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,8 +22,8 @@ env: RUST_BACKTRACE: 1 jobs: - rustfmt: - name: Check formatting + style: + name: Check formatting and Clippy lints runs-on: - self-hosted - test @@ -33,19 +33,20 @@ jobs: with: clean: false submodules: "recursive" + fetch-depth: 0 - name: Set up default .cargo/config.toml run: cp ./.cargo/ci-config.toml ~/.cargo/config.toml - - name: Run rustfmt - uses: ./.github/actions/check_formatting + - name: Run style checks + uses: ./.github/actions/check_style tests: name: Run tests runs-on: - self-hosted - test - needs: rustfmt + needs: style steps: - name: Checkout repo uses: actions/checkout@v3 diff --git a/.github/workflows/release_nightly.yml b/.github/workflows/release_nightly.yml index b7e6a0321e68b0ffe5c0521befcbfc13a76677a8..5d2dbe41f9aeceb18e0c35a064325dfd9bd4b31a 100644 --- a/.github/workflows/release_nightly.yml +++ b/.github/workflows/release_nightly.yml @@ -14,8 +14,8 @@ env: RUST_BACKTRACE: 1 jobs: - rustfmt: - name: Check formatting + style: + name: Check formatting and Clippy lints runs-on: - self-hosted - test @@ -25,16 +25,17 @@ jobs: with: clean: false submodules: "recursive" + fetch-depth: 0 - - name: Run rustfmt - uses: ./.github/actions/check_formatting + - name: Run style checks + uses: ./.github/actions/check_style tests: name: Run tests runs-on: - self-hosted - test - needs: rustfmt + needs: style steps: - name: Checkout repo uses: actions/checkout@v3 diff --git a/crates/collab/src/tests/channel_guest_tests.rs b/crates/collab/src/tests/channel_guest_tests.rs index 9b68ce3922ab24726130c356636dae7d6899ef35..d5933235926fa1da8c1e8538dedd2a7506504cd8 100644 --- a/crates/collab/src/tests/channel_guest_tests.rs +++ b/crates/collab/src/tests/channel_guest_tests.rs @@ -104,12 +104,10 @@ async fn test_channel_guest_promotion(cx_a: &mut TestAppContext, cx_b: &mut Test }); assert!(project_b.read_with(cx_b, |project, _| project.is_read_only())); assert!(editor_b.update(cx_b, |e, cx| e.read_only(cx))); - assert!(dbg!( - room_b - .update(cx_b, |room, cx| room.share_microphone(cx)) - .await - ) - .is_err()); + assert!(room_b + .update(cx_b, |room, cx| room.share_microphone(cx)) + .await + .is_err()); // B is promoted active_call_a diff --git a/crates/vim/src/editor_events.rs b/crates/vim/src/editor_events.rs index e3ed076698d101a12f92c16048748532ea596e1c..e4057792796cad86d7fe31ff01b78ea9434ae102 100644 --- a/crates/vim/src/editor_events.rs +++ b/crates/vim/src/editor_events.rs @@ -111,7 +111,6 @@ mod test { let mut cx1 = VisualTestContext::from_window(cx.window, &cx); let editor1 = cx.editor.clone(); - dbg!(editor1.entity_id()); let buffer = cx.new_model(|_| Buffer::new(0, 0, "a = 1\nb = 2\n")); let (editor2, cx2) = cx.add_window_view(|cx| Editor::for_buffer(buffer, None, cx)); From e0dd5a5820ff822357c35b2abfe2684c15ad97f7 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 11 Jan 2024 00:33:53 +0200 Subject: [PATCH 2/5] Debugging --- .github/actions/check_style/action.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/check_style/action.yml b/.github/actions/check_style/action.yml index ff0481980816851ef6135c78f84b13b0cb21c09a..cb567867e4cb53898e05c5d42dc0317cfa35e55c 100644 --- a/.github/actions/check_style/action.yml +++ b/.github/actions/check_style/action.yml @@ -21,4 +21,4 @@ runs: # so specify those here, and disable the rest until Zed's workspace # will have more fixes & suppression for the standard lint set run: | - cargo clippy --workspace --all-targets --all-features -- -A clippy::all -D clippy::dbg_macro -D clippy::todo + CARGO_LOG=debug cargo -vvv clippy --workspace --all-targets --all-features -- -A clippy::all -D clippy::dbg_macro -D clippy::todo From 1200f595a39e77c12c38f1739cf561e810108a28 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Thu, 11 Jan 2024 12:36:45 +0100 Subject: [PATCH 3/5] Try to run clippy just for a single target --- .github/actions/check_style/action.yml | 36 +++++++++++++------------- 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/.github/actions/check_style/action.yml b/.github/actions/check_style/action.yml index cb567867e4cb53898e05c5d42dc0317cfa35e55c..8312eeb500f54adcbe71c3ebd1c9a240eddea6f0 100644 --- a/.github/actions/check_style/action.yml +++ b/.github/actions/check_style/action.yml @@ -2,23 +2,23 @@ name: "Check formatting" description: "Checks code formatting use cargo fmt" runs: - using: "composite" - steps: - - name: Install Rust - shell: bash -euxo pipefail {0} - run: | - rustup set profile minimal - rustup update stable - rustup component add clippy + using: "composite" + steps: + - name: Install Rust + shell: bash -euxo pipefail {0} + run: | + rustup set profile minimal + rustup update stable + rustup component add clippy - - name: cargo fmt - shell: bash -euxo pipefail {0} - run: cargo fmt --all -- --check + - name: cargo fmt + shell: bash -euxo pipefail {0} + run: cargo fmt --all -- --check - - name: cargo clippy - shell: bash -euxo pipefail {0} - # clippy.toml is not currently supporting specifying allowed lints - # so specify those here, and disable the rest until Zed's workspace - # will have more fixes & suppression for the standard lint set - run: | - CARGO_LOG=debug cargo -vvv clippy --workspace --all-targets --all-features -- -A clippy::all -D clippy::dbg_macro -D clippy::todo + - name: cargo clippy + shell: bash -euxo pipefail {0} + # clippy.toml is not currently supporting specifying allowed lints + # so specify those here, and disable the rest until Zed's workspace + # will have more fixes & suppression for the standard lint set + run: | + CARGO_LOG=debug cargo -vvv clippy --workspace --all-features -- -A clippy::all -D clippy::dbg_macro -D clippy::todo From a5dd2535f140038645fcda055e7f8479d8334f2c Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 11 Jan 2024 13:56:28 +0200 Subject: [PATCH 4/5] Properly require clippy installation, try to shuffle clippy arguments co-authored-by: Piotr --- .github/actions/check_style/action.yml | 35 +++++++++++++------------- crates/editor/src/editor_tests.rs | 1 + rust-toolchain.toml | 2 +- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/.github/actions/check_style/action.yml b/.github/actions/check_style/action.yml index 8312eeb500f54adcbe71c3ebd1c9a240eddea6f0..367665dd94934a57274e753228a5462c1658da5b 100644 --- a/.github/actions/check_style/action.yml +++ b/.github/actions/check_style/action.yml @@ -2,23 +2,22 @@ name: "Check formatting" description: "Checks code formatting use cargo fmt" runs: - using: "composite" - steps: - - name: Install Rust - shell: bash -euxo pipefail {0} - run: | - rustup set profile minimal - rustup update stable - rustup component add clippy + using: "composite" + steps: + - name: Install Rust + shell: bash -euxo pipefail {0} + run: | + rustup set profile minimal + rustup update stable - - name: cargo fmt - shell: bash -euxo pipefail {0} - run: cargo fmt --all -- --check + - name: cargo fmt + shell: bash -euxo pipefail {0} + run: cargo fmt --all -- --check - - name: cargo clippy - shell: bash -euxo pipefail {0} - # clippy.toml is not currently supporting specifying allowed lints - # so specify those here, and disable the rest until Zed's workspace - # will have more fixes & suppression for the standard lint set - run: | - CARGO_LOG=debug cargo -vvv clippy --workspace --all-features -- -A clippy::all -D clippy::dbg_macro -D clippy::todo + - name: cargo clippy + shell: bash -euxo pipefail {0} + # clippy.toml is not currently supporting specifying allowed lints + # so specify those here, and disable the rest until Zed's workspace + # will have more fixes & suppression for the standard lint set + run: | + cargo clippy --workspace --all-features --all-targets -- -A clippy::all -D clippy::dbg_macro -D clippy::todo diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 520c3714d3d529dbcd2df4d4cc4d750db2a7a53c..00e33a2729d7d4b42f4cb270065291a1dca374e3 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -8242,6 +8242,7 @@ pub(crate) fn update_test_project_settings( } pub(crate) fn init_test(cx: &mut TestAppContext, f: fn(&mut AllLanguageSettingsContent)) { + dbg!("(???????????"); _ = cx.update(|cx| { let store = SettingsStore::test(cx); cx.set_global(store); diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 79d2d20a7afd81fc00eb1e142d78473562b7f335..e04ebff9ced48305cf6af00e128e4bb386f29d23 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,4 +1,4 @@ [toolchain] channel = "1.75" -components = [ "rustfmt" ] +components = [ "rustfmt", "clippy" ] targets = [ "x86_64-apple-darwin", "aarch64-apple-darwin", "wasm32-wasi" ] From 41bc49af364adf1d2193dd800546aac280fe0ce1 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Thu, 11 Jan 2024 14:01:42 +0200 Subject: [PATCH 5/5] Remove redundant install Rust steps Those were not installing Rust but configuring it via rustup, and those configurations were done on `stable` toolchain which is not what we use (see rust-toolchain.toml) co-authored-by: Piotr --- .github/actions/check_style/action.yml | 6 --- .github/actions/run_tests/action.yml | 41 +++++++++--------- .github/workflows/ci.yml | 8 ---- .github/workflows/randomized_tests.yml | 59 ++++++++++++-------------- .github/workflows/release_nightly.yml | 8 ---- crates/editor/src/editor_tests.rs | 1 - rust-toolchain.toml | 1 + 7 files changed, 47 insertions(+), 77 deletions(-) diff --git a/.github/actions/check_style/action.yml b/.github/actions/check_style/action.yml index 367665dd94934a57274e753228a5462c1658da5b..5dc7c42b02f387fe76295e9ae110f28972ef8450 100644 --- a/.github/actions/check_style/action.yml +++ b/.github/actions/check_style/action.yml @@ -4,12 +4,6 @@ description: "Checks code formatting use cargo fmt" runs: using: "composite" steps: - - name: Install Rust - shell: bash -euxo pipefail {0} - run: | - rustup set profile minimal - rustup update stable - - name: cargo fmt shell: bash -euxo pipefail {0} run: cargo fmt --all -- --check diff --git a/.github/actions/run_tests/action.yml b/.github/actions/run_tests/action.yml index 1ea51a06a6b4f26935e2c752beb0cad12139fcfb..af37af7fc429cc9311d033de6b22016ff1d3e24f 100644 --- a/.github/actions/run_tests/action.yml +++ b/.github/actions/run_tests/action.yml @@ -2,29 +2,26 @@ name: "Run tests" description: "Runs the tests" runs: - using: "composite" - steps: - - name: Install Rust - shell: bash -euxo pipefail {0} - run: | - rustup set profile minimal - rustup update stable - rustup target add wasm32-wasi - cargo install cargo-nextest + using: "composite" + steps: + - name: Install Rust + shell: bash -euxo pipefail {0} + run: | + cargo install cargo-nextest - - name: Install Node - uses: actions/setup-node@v3 - with: - node-version: "18" + - name: Install Node + uses: actions/setup-node@v3 + with: + node-version: "18" - - name: Limit target directory size - shell: bash -euxo pipefail {0} - run: script/clear-target-dir-if-larger-than 100 + - name: Limit target directory size + shell: bash -euxo pipefail {0} + run: script/clear-target-dir-if-larger-than 100 - - name: Run check - shell: bash -euxo pipefail {0} - run: cargo check --tests --workspace + - name: Run check + shell: bash -euxo pipefail {0} + run: cargo check --tests --workspace - - name: Run tests - shell: bash -euxo pipefail {0} - run: cargo nextest run --workspace --no-fail-fast + - name: Run tests + shell: bash -euxo pipefail {0} + run: cargo nextest run --workspace --no-fail-fast diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3a92a744bb12d59fcf0539a4f8fb10f270899dd4..476f263997d5caa7fb0f3cb576397b618347e82f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -76,14 +76,6 @@ jobs: APPLE_NOTARIZATION_USERNAME: ${{ secrets.APPLE_NOTARIZATION_USERNAME }} APPLE_NOTARIZATION_PASSWORD: ${{ secrets.APPLE_NOTARIZATION_PASSWORD }} steps: - - name: Install Rust - run: | - rustup set profile minimal - rustup update stable - rustup target add aarch64-apple-darwin - rustup target add x86_64-apple-darwin - rustup target add wasm32-wasi - - name: Install Node uses: actions/setup-node@v3 with: diff --git a/.github/workflows/randomized_tests.yml b/.github/workflows/randomized_tests.yml index d1b8ddfdfbbe636c1f9817e134e0fc86871d5a03..a1704d58bdcffec9ce51779bf9af47b8be7fec13 100644 --- a/.github/workflows/randomized_tests.yml +++ b/.github/workflows/randomized_tests.yml @@ -3,41 +3,36 @@ name: Randomized Tests concurrency: randomized-tests on: - push: - branches: - - randomized-tests-runner - # schedule: - # - cron: '0 * * * *' + push: + branches: + - randomized-tests-runner + # schedule: + # - cron: '0 * * * *' env: - CARGO_TERM_COLOR: always - CARGO_INCREMENTAL: 0 - RUST_BACKTRACE: 1 - ZED_SERVER_URL: https://zed.dev - ZED_CLIENT_SECRET_TOKEN: ${{ secrets.ZED_CLIENT_SECRET_TOKEN }} + CARGO_TERM_COLOR: always + CARGO_INCREMENTAL: 0 + RUST_BACKTRACE: 1 + ZED_SERVER_URL: https://zed.dev + ZED_CLIENT_SECRET_TOKEN: ${{ secrets.ZED_CLIENT_SECRET_TOKEN }} jobs: - tests: - name: Run randomized tests - runs-on: - - self-hosted - - randomized-tests - steps: - - name: Install Rust - run: | - rustup set profile minimal - rustup update stable + tests: + name: Run randomized tests + runs-on: + - self-hosted + - randomized-tests + steps: + - name: Install Node + uses: actions/setup-node@v3 + with: + node-version: "18" - - name: Install Node - uses: actions/setup-node@v3 - with: - node-version: '18' + - name: Checkout repo + uses: actions/checkout@v3 + with: + clean: false + submodules: "recursive" - - name: Checkout repo - uses: actions/checkout@v3 - with: - clean: false - submodules: 'recursive' - - - name: Run randomized tests - run: script/randomized-test-ci + - name: Run randomized tests + run: script/randomized-test-ci diff --git a/.github/workflows/release_nightly.yml b/.github/workflows/release_nightly.yml index 5d2dbe41f9aeceb18e0c35a064325dfd9bd4b31a..33ccb4cba93ce6dd6538a81426248c866c4ee81b 100644 --- a/.github/workflows/release_nightly.yml +++ b/.github/workflows/release_nightly.yml @@ -60,14 +60,6 @@ jobs: DIGITALOCEAN_SPACES_ACCESS_KEY: ${{ secrets.DIGITALOCEAN_SPACES_ACCESS_KEY }} DIGITALOCEAN_SPACES_SECRET_KEY: ${{ secrets.DIGITALOCEAN_SPACES_SECRET_KEY }} steps: - - name: Install Rust - run: | - rustup set profile minimal - rustup update stable - rustup target add aarch64-apple-darwin - rustup target add x86_64-apple-darwin - rustup target add wasm32-wasi - - name: Install Node uses: actions/setup-node@v3 with: diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 00e33a2729d7d4b42f4cb270065291a1dca374e3..520c3714d3d529dbcd2df4d4cc4d750db2a7a53c 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -8242,7 +8242,6 @@ pub(crate) fn update_test_project_settings( } pub(crate) fn init_test(cx: &mut TestAppContext, f: fn(&mut AllLanguageSettingsContent)) { - dbg!("(???????????"); _ = cx.update(|cx| { let store = SettingsStore::test(cx); cx.set_global(store); diff --git a/rust-toolchain.toml b/rust-toolchain.toml index e04ebff9ced48305cf6af00e128e4bb386f29d23..5cdc76def26ae769f69b0f69555ec5663fcf1937 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,4 +1,5 @@ [toolchain] channel = "1.75" +profile = "minimal" components = [ "rustfmt", "clippy" ] targets = [ "x86_64-apple-darwin", "aarch64-apple-darwin", "wasm32-wasi" ]