Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Make CI Clippy static analysis checks more robust #635

Merged
merged 3 commits into from
Jul 11, 2024
Merged

Make CI Clippy static analysis checks more robust #635

merged 3 commits into from
Jul 11, 2024

Commits on Jul 10, 2024

  1. 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.
    @AlexTMjugador
    AlexTMjugador committed Jul 10, 2024
    Configuration menu
    Copy the full SHA
    d9d90ee View commit details
    Browse the repository at this point in the history

  2. Remove some now unnecessary Clippy lints allowances

    @AlexTMjugador
    AlexTMjugador committed Jul 10, 2024
    Configuration menu
    Copy the full SHA
    393de6e View commit details
    Browse the repository at this point in the history

  3. Do not fail workflow on static analysis results upload failures 

    When Clippy finds lints, it already aborts the workflow. No double
failure is needed.
    @AlexTMjugador
    AlexTMjugador committed Jul 10, 2024
    Configuration menu
    Copy the full SHA
    a662c8c View commit details
    Browse the repository at this point in the history