Skip to content

Commit d9d90ee

Browse files
committed
Make CI Clippy static analysis checks more robust
I have identified two potential improvements for how we perform static analysis on our code in our CI pipeline: - The `giraffate/clippy-action` we currently use has not been updated to Node 20, and GitHub has repeatedly indicated that they will phase out actions that do not support the latest Node versions. Despite my efforts to help with the update by submitting a pull request upstream, it has been ignored for months despite its perceived ease of review, raising concerns about the ongoing maintenance of the action. This situation suggests we should explore alternative methods for integrating Clippy with GitHub's UI. - As evidenced by PR 632, thoroughly testing Rust crates for every possible feature combination is often overlooked due to the tedious nature of the task. Our current CI setup only checks two feature combinations, which is far from comprehensive. To address the first improvement, these changes drop `clippy-action` entirely in favor of utilizing GitHub's native CodeQL SARIF (Static Analysis Results Interchange Format) file integration. Since Clippy cannot directly output lints in SARIF, `clippy-sarif` is used to convert Clippy's JSON output to SARIF. Additionally, `sarif-fmt` is added to turn SARIF into a human-friendly display format in the workflow run logs. For the second improvement, let's use `cargo hack` with the `--feature-powerset` flag to run Clippy for every possible feature combination. This approach strikes a good balance between CI runtime and thoroughness, as the number of feature combinations grows superlinearly with the number of features: running `cargo nextest` for every powerset element would lead to excessively long CI times.
1 parent f7b837a commit d9d90ee

File tree

3 files changed

+33
-16
lines changed

3 files changed

+33
-16
lines changed

.github/workflows/oxipng.yml

+27-12
Original file line numberDiff line numberDiff line change
@@ -102,25 +102,33 @@ jobs:
102102
- name: Install nextest
103103
uses: taiki-e/install-action@nextest
104104

105-
- name: Run rustfmt
105+
- name: Install cargo-hack
106106
if: matrix.target == 'x86_64-unknown-linux-gnu'
107-
run: cargo fmt --check
107+
uses: taiki-e/install-action@cargo-hack
108108

109-
- name: Run Clippy (no default features)
109+
- name: Install clippy-sarif
110110
if: matrix.target == 'x86_64-unknown-linux-gnu'
111-
uses: giraffate/clippy-action@v1
111+
uses: taiki-e/install-action@v2
112112
with:
113-
clippy_flags: --no-deps --all-targets --no-default-features -- -D warnings
114-
reporter: github-check
115-
fail_on_error: true
113+
tool: clippy-sarif
116114

117-
- name: Run Clippy (all features)
115+
- name: Install sarif-fmt
118116
if: matrix.target == 'x86_64-unknown-linux-gnu'
119-
uses: giraffate/clippy-action@v1
117+
uses: taiki-e/install-action@v2
120118
with:
121-
clippy_flags: --no-deps --all-targets --all-features -- -D warnings
122-
reporter: github-check
123-
fail_on_error: true
119+
tool: sarif-fmt
120+
121+
- name: Run rustfmt
122+
if: matrix.target == 'x86_64-unknown-linux-gnu'
123+
run: cargo fmt --check
124+
125+
- name: Run Clippy for all feature combinations
126+
if: matrix.target == 'x86_64-unknown-linux-gnu'
127+
run: >
128+
cargo hack clippy --no-deps --all-targets --feature-powerset --exclude-features sanity-checks --message-format=json -- -D warnings
129+
| clippy-sarif
130+
| tee clippy-results.sarif
131+
| sarif-fmt
124132
125133
- name: Run tests
126134
run: |
@@ -145,6 +153,13 @@ jobs:
145153
target/${{ env.CARGO_BUILD_TARGET }}/release/oxipng
146154
target/${{ env.CARGO_BUILD_TARGET }}/release/oxipng.exe
147155
156+
- name: Upload analysis results to GitHub
157+
uses: github/codeql-action/upload-sarif@v3
158+
if: always() && matrix.target == 'x86_64-unknown-linux-gnu'
159+
with:
160+
sarif_file: clippy-results.sarif
161+
category: clippy
162+
148163
msrv-check:
149164
name: MSRV check
150165

src/main.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -329,10 +329,10 @@ fn parse_opts_into_struct(
329329
if let Some(iterations) = NonZeroU8::new(15) {
330330
opts.deflate = Deflaters::Zopfli { iterations };
331331
}
332-
} else if let Deflaters::Libdeflater { compression } = &mut opts.deflate {
333-
if let Some(x) = matches.get_one::<i64>("compression") {
334-
*compression = *x as u8;
335-
}
332+
} else if let (Deflaters::Libdeflater { compression }, Some(x)) =
333+
(&mut opts.deflate, matches.get_one::<i64>("compression"))
334+
{
335+
*compression = *x as u8;
336336
}
337337

338338
#[cfg(feature = "parallel")]

src/rayon.rs

+2
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
#![allow(dead_code)]
2+
13
pub mod prelude {
24
pub use super::*;
35
}

0 commit comments

Comments
 (0)