Skip to content

Commit 7bca9fc

Browse files
authored
[PM-24643] Add an easy to use formatting linting command to the sdk (#1140)
## 🎟️ Tracking <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> ## 📔 Objective - Makes linting easier by using the same script for CI and local checks - Makes linting faster in CI (17m -> 8m) by parallelizing the checks ## 🚨 Breaking Changes <!-- Does this PR introduce any breaking changes? If so, please describe the impact and migration path for clients. If you're unsure, the automated TypeScript compatibility check will run when you open/update this PR and provide feedback. For breaking changes: 1. Describe what changed in the client interface 2. Explain why the change was necessary 3. Provide migration steps for client developers 4. Link to any paired client PRs if needed Otherwise, you can remove this section. -->
1 parent 77391b7 commit 7bca9fc

7 files changed

Lines changed: 241 additions & 83 deletions

File tree

.claude/CLAUDE.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,8 +145,11 @@ discriminant + optional fields) belongs at the API→domain boundary.
145145

146146
**Format & Lint:**
147147

148-
- `cargo +nightly fmt --workspace` - Code formatting
149-
- Use `cargo clippy` to lint code and catch common mistakes
148+
- `npm run lint` - Run every formatting/linting check CI runs (fmt, clippy, sort, udeps, dylint,
149+
doc, prettier, dep-ownership, cargo-lock). Matches `.github/workflows/lint.yml`.
150+
- `npm run lint:fix` - Same set, auto-fixing where the tool supports it.
151+
- `npm run lint -- --only <check>` - Run a single check (e.g. `--only clippy`).
152+
- Underlying script: `scripts/lint.sh`.
150153

151154
**WASM Testing:**
152155

.github/renovate.json5

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
matchStrings: [
1010
"cargo install (?<depName>cargo-dylint) (?:[\\w-]+ )?--version (?<currentValue>\\d+\\.\\d+\\.\\d+) --locked",
1111
"cargo install (?<depName>dylint-link) (?:[\\w-]+ )?--version (?<currentValue>\\d+\\.\\d+\\.\\d+) --locked",
12+
"cargo install (?<depName>cargo-sort) --version (?<currentValue>\\d+\\.\\d+\\.\\d+) --locked",
13+
"cargo install (?<depName>cargo-udeps) --version (?<currentValue>\\d+\\.\\d+\\.\\d+) --locked",
1214
],
1315
datasourceTemplate: "cargo",
1416
versioningTemplate: "cargo",
@@ -68,6 +70,8 @@
6870
matchPackageNames: [
6971
"async-trait",
7072
"cargo-dylint",
73+
"cargo-sort",
74+
"cargo-udeps",
7175
"ciborium",
7276
"chrono",
7377
"data-encoding",

.github/workflows/lint.yml

Lines changed: 62 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -12,128 +12,128 @@ env:
1212
CARGO_TERM_COLOR: always
1313

1414
jobs:
15-
style:
15+
check:
1616
name: Check Style
17+
runs-on: ubuntu-24.04
18+
needs: style
19+
if: always()
20+
steps:
21+
- name: Verify all style checks passed
22+
if: ${{ needs.style.result != 'success' }}
23+
run: exit 1
24+
25+
style:
26+
name: ${{ matrix.check }}
1727

1828
runs-on: ubuntu-24.04
1929

2030
permissions:
2131
contents: read
2232
security-events: write
2333

34+
strategy:
35+
fail-fast: false
36+
matrix:
37+
include:
38+
- check: fmt
39+
needs_rust: true
40+
needs_rust_nightly: true
41+
- check: clippy
42+
needs_rust: true
43+
- check: sort
44+
needs_rust: true
45+
install: cargo install cargo-sort --version 2.0.2 --locked
46+
- check: udeps
47+
needs_rust: true
48+
needs_rust_nightly: true
49+
install: cargo install cargo-udeps --version 0.1.57 --locked
50+
- check: dylint
51+
needs_rust: true
52+
install: cargo install cargo-dylint dylint-link --version 5.0.0 --locked
53+
- check: doc
54+
needs_rust: true
55+
- check: prettier
56+
needs_node: true
57+
- check: dep-ownership
58+
needs_node: true
59+
- check: cargo-lock
60+
needs_rust: true
61+
2462
steps:
2563
- name: Checkout
2664
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
2765
with:
2866
persist-credentials: false
2967

30-
- name: Set Rust Toolchain
68+
- name: Set Rust toolchain version
69+
if: matrix.needs_rust
3170
id: toolchain
3271
shell: bash
3372
run: |
3473
RUST_TOOLCHAIN="$(grep -oP '^channel.*"(\K.*?)(?=")' rust-toolchain.toml)"
3574
echo "RUST_TOOLCHAIN=${RUST_TOOLCHAIN}" | tee -a "${GITHUB_OUTPUT}"
3675
37-
- name: Set Rust Nightly Toolchain
76+
- name: Set Rust nightly toolchain version
77+
if: matrix.needs_rust_nightly
3878
id: nightly-toolchain
3979
shell: bash
4080
run: |
4181
RUST_NIGHTLY_TOOLCHAIN="$(grep -oP '^nightly-channel.*"(\K.*?)(?=")' rust-toolchain.toml)"
4282
echo "RUST_NIGHTLY_TOOLCHAIN=${RUST_NIGHTLY_TOOLCHAIN}" | tee -a "${GITHUB_OUTPUT}"
4383
44-
- name: Install rust
84+
- name: Install Rust
85+
if: matrix.needs_rust
4586
uses: dtolnay/rust-toolchain@29eef336d9b2848a0b548edc03f92a220660cdb8 # stable
4687
with:
4788
toolchain: "${{ steps.toolchain.outputs.RUST_TOOLCHAIN }}"
4889
components: clippy, rustfmt
4990

50-
- name: Install rust nightly
91+
- name: Install Rust nightly
92+
if: matrix.needs_rust_nightly
5193
env:
5294
RUST_NIGHTLY_TOOLCHAIN: ${{ steps.nightly-toolchain.outputs.RUST_NIGHTLY_TOOLCHAIN }}
5395
run: |
5496
rustup toolchain install "${RUST_NIGHTLY_TOOLCHAIN}"
5597
rustup component add rustfmt --toolchain "${RUST_NIGHTLY_TOOLCHAIN}"-x86_64-unknown-linux-gnu
5698
5799
- name: Cache cargo registry
100+
if: matrix.needs_rust
58101
uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2.8.2
59102

60-
- name: Cargo fmt
61-
env:
62-
RUST_NIGHTLY_TOOLCHAIN: ${{ steps.nightly-toolchain.outputs.RUST_NIGHTLY_TOOLCHAIN }}
63-
run: cargo +"${RUST_NIGHTLY_TOOLCHAIN}" fmt --check
103+
- name: Install per-check cargo tool
104+
if: matrix.install
105+
run: ${{ matrix.install }}
64106

65107
- name: Install clippy-sarif and sarif-fmt
108+
if: matrix.check == 'clippy'
66109
run: cargo install clippy-sarif sarif-fmt --locked --git https://github.com/psastras/sarif-rs.git --rev 11c33a53f6ffeaed736856b86fb6b7b09fabdfd8
67110

68-
- name: Cargo clippy-sarif
69-
run: cargo clippy --all-features --all-targets --message-format=json |
70-
clippy-sarif | tee clippy_result.sarif | sarif-fmt
111+
- name: Run clippy with SARIF output
112+
if: matrix.check == 'clippy'
71113
env:
72114
RUSTFLAGS: "-D warnings"
115+
run: cargo clippy --all-features --all-targets --message-format=json |
116+
clippy-sarif | tee clippy_result.sarif | sarif-fmt
73117

74-
- name: Upload Clippy results to GitHub
118+
- name: Upload clippy results to GitHub
119+
if: matrix.check == 'clippy'
75120
uses: github/codeql-action/upload-sarif@6bc82e05fd0ea64601dd4b465378bbcf57de0314 # v4.32.1
76121
with:
77122
sarif_file: clippy_result.sarif
78123
sha: ${{ contains(github.event_name, 'pull_request') && github.event.pull_request.head.sha || github.sha }}
79124
ref: ${{ contains(github.event_name, 'pull_request') && format('refs/pull/{0}/head', github.event.pull_request.number) || github.ref }}
80125

81-
# Run it again but this time without the sarif output so that the
82-
# status code of the command is caught and reported as failed in GitHub.
83-
# This should be cached from the previous step and should be fast.
84-
- name: Cargo clippy
85-
run: cargo clippy --all-features --all-targets
86-
env:
87-
RUSTFLAGS: "-D warnings"
88-
89-
- name: Install cargo-sort
90-
run: cargo install cargo-sort --version 2.0.2 --locked
91-
92-
- name: Cargo sort
93-
run: cargo sort --workspace --grouped --check
94-
95-
- name: Install cargo-udeps
96-
run: cargo install cargo-udeps --version 0.1.57 --locked
97-
98-
- name: Cargo udeps
99-
env:
100-
RUST_NIGHTLY_TOOLCHAIN: ${{ steps.nightly-toolchain.outputs.RUST_NIGHTLY_TOOLCHAIN }}
101-
run: cargo +"${RUST_NIGHTLY_TOOLCHAIN}" udeps --workspace --all-features
102-
103-
- name: Install cargo-dylint
104-
run: cargo install cargo-dylint dylint-link --version 5.0.0 --locked
105-
106-
- name: Cargo dylint
107-
run: cargo dylint --all -- --all-features --all-targets
108-
env:
109-
RUSTFLAGS: "-D warnings"
110-
111-
- name: Check Cargo.lock unchanged
112-
run: |
113-
if git diff --exit-code Cargo.lock; then
114-
echo "Cargo.lock is unchanged"
115-
else
116-
echo "Error: Cargo.lock has been modified. Please run `cargo check` and commit the changes to Cargo.lock."
117-
exit 1
118-
fi
119-
120126
- name: Set up Node
127+
if: matrix.needs_node
121128
uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6.2.0
122129
with:
123130
cache: "npm"
124131
cache-dependency-path: "package-lock.json"
125132
node-version: "16"
126133

127134
- name: NPM setup
135+
if: matrix.needs_node
128136
run: npm ci
129137

130-
- name: Lint unowned dependencies
131-
run: npm run lint:dep-ownership
132-
133-
- name: Node Lint
134-
run: npm run lint
135-
136-
- name: Verify rust documentation links
137-
run: cargo doc --no-deps --all-features --document-private-items
138-
env:
139-
RUSTDOCFLAGS: "-D warnings"
138+
- name: Run lint --only ${{ matrix.check }}
139+
run: ./scripts/lint.sh --only ${{ matrix.check }}

.prettierignore

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,7 @@ schemas
33
about.hbs
44

55
# Language bindings
6-
crates/bitwarden-wasm-internal/npm/bitwarden_wasm_internal_bg.wasm.js
7-
crates/bitwarden-wasm-internal/bitwarden_license/npm/bitwarden_wasm_internal_bg.wasm.js
6+
crates/bitwarden-wasm-internal/**/bitwarden_wasm_internal*.js
87
crates/bitwarden-uniffi/kotlin/*
98
crates/bitwarden-uniffi/swift/*
109

README.md

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -435,10 +435,27 @@ The list of developer tools is:
435435
This repository uses various tools to check formatting and linting before it's merged. It's
436436
recommended to run the checks before submitting a PR.
437437

438+
### Running the checks
439+
440+
```
441+
npm run lint # run every check (check-only, matches CI)
442+
npm run lint:fix # auto-fix where supported, then run the rest
443+
444+
# Run a single check:
445+
npm run lint -- --only fmt
446+
npm run lint -- --only clippy
447+
```
448+
449+
The list of available checks: `fmt`, `clippy`, `sort`, `udeps`, `dylint`, `doc`, `prettier`,
450+
`dep-ownership`, `cargo-lock`.
451+
452+
The command is implemented in [scripts/lint.sh](./scripts/lint.sh) and mirrors
453+
[.github/workflows/lint.yml](./.github/workflows/lint.yml), so a local pass means CI will pass.
454+
438455
### Installation
439456

440-
Please see the [lint.yml](./.github/workflows/lint.yml) file, for example, installation commands and
441-
versions. Here are the cli tools we use:
457+
The tools each check needs (and pinned versions) are installed in the lint workflow above. The
458+
underlying tools are:
442459

443460
- Nightly [cargo fmt](https://github.com/rust-lang/rustfmt) and
444461
[cargo udeps](https://github.com/est31/cargo-udeps)
@@ -447,20 +464,7 @@ versions. Here are the cli tools we use:
447464
- [cargo sort](https://github.com/DevinR528/cargo-sort)
448465
- [prettier](https://github.com/prettier/prettier)
449466

450-
### Checks
451-
452-
To verify if changes need to be made, here are examples for the above tools:
453-
454-
```
455-
export RUSTFLAGS="-D warnings"
456-
457-
cargo +nightly fmt --check
458-
cargo +nightly udeps --workspace --all-features
459-
cargo clippy --all-features --all-targets
460-
cargo dylint --all -- --all-features --all-targets
461-
cargo sort --workspace --grouped --check
462-
npm run lint
463-
```
467+
If a tool is missing locally, `npm run lint` will tell you which one and how to install it.
464468

465469
## Documentation
466470

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@
1414
"author": "Bitwarden Inc. <hello@bitwarden.com> (https://bitwarden.com)",
1515
"main": "index.js",
1616
"scripts": {
17-
"lint": "prettier --check .",
17+
"lint": "bash scripts/lint.sh",
18+
"lint:fix": "bash scripts/lint.sh --fix",
19+
"lint:prettier": "prettier --check .",
1820
"lint:dep-ownership": "tsx scripts/dep-ownership.ts",
1921
"prettier": "prettier --write .",
2022
"test": "echo \"Error: no test specified\" && exit 1",

0 commit comments

Comments
 (0)