Make CI Clippy static analysis checks more robust #635
Conversation
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.
|
This pull request sets up GitHub code scanning for this repository. Once the scans have completed and the checks have passed, the analysis results for this pull request branch will appear on this overview. Once you merge this pull request, the 'Security' tab will show more code scanning analysis results (for example, for the default branch). Depending on your configuration and choice of analysis tool, future pull requests will be annotated with code scanning analysis results. For more information about GitHub code scanning, check out the documentation. |
|
The bot comment above is expected due to the usage of GitHub's CodeQL SARIF integration. Please heed it accordingly before this PR is merged. |
When Clippy finds lints, it already aborts the workflow. No double failure is needed.
There was a problem hiding this comment.
This looks great @AlexTMjugador! Running all feature combos should be a big help.
I was also thinking it might be good to use the GitHub arm runners for the arm linux builds now that they're generally available, is that something you would want to do?
[edit] Actually I may be wrong about this, it might only be available for teams and enterprises...
Indeed, that's sadly the case at the moment. However, in the blog post I link to GitHub said that it expects to "begin offering Arm runners for open source and personal accounts by the end of the year", so if all goes according to plan we can get rid of QEMU soon enough 😄 |
|
Yeah, sounds good! Let's go ahead and merge this. |
Yeah, I deem the current mainline stable enough and those bugfixes may indeed come in handy. Let's do it! |
AlexTMjugador
added
the
github_actions
I have identified two potential improvements for how we perform static analysis on our code in our CI pipeline:
giraffate/clippy-actionwe 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.To address the first improvement, these changes drop
clippy-actionentirely 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-sarifis used to convert Clippy's JSON output to SARIF. Additionally,sarif-fmtis added to turn SARIF into a human-friendly display format in the workflow run logs.For the second improvement, let's use
cargo hackwith the--feature-powersetflag 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: runningcargo nextestfor every powerset element would lead to excessively long CI times.While at it, I have fixed the Clippy lints that were catched by the more exhaustive checks.