psastras / sarif-rs

A group of Rust projects for interacting with the SARIF format
https://psastras.github.io/sarif-rs/
MIT License
79 stars 17 forks source link

clippy-sarif: call clippy as a subprocess #721

Open psandana opened 3 weeks ago

psandana commented 3 weeks ago

Proposal Add a new way of running clippy-sarif, that runs clippy underneath to capture clippy output and exit status.

Motivation In CI systems it is frequently used to break pipelines for certain checks. clippy is one of those tools you want to break builds upon unsuccessful runs.

When running cargo clippy --message-format=json | clippy-sarif --output myfile.sarif on Windows CI/CD system like ADO Pipelines or GH, if clippy itself does exit with error status, it is not propagated by the pipe | and then clippy-sarif run makes the step or task to actually be marked as successful.

It would help to make somehow clippy-sarif to detect clippy exit code, to propagate it to the caller.

Benefits Running clippy by clippy-sarif has the following benefits:

  1. Capture clippy exit code and propagate.
  2. Improved capture of output from clippy (can fork it to screen at the same time).
  3. Control on clippy output by providing the --message-format=json argument.

Considerations This "new" mode of running clippy-sarif can be made compatible with the existing one that uses standard output. It is possible to completely remove the previous mode and replace with this new one, if there is a wish to only keep one mode. Another option is to allow clippy-sarif to have an --input argument consuming data from a .json file output by clippy. Example:

cargo clippy --message-format=json | tee clippy.json
clippy-sarif --input clippy.json --output clippy.sarif
psastras commented 2 weeks ago

@psandana would something like bash/zsh pipefail option be sufficient for your needs?

I'd prefer to direct users to that if its generally sufficient rather than supporting a similar functionality here.

psandana commented 2 weeks ago

@psandana would something like bash/zsh pipefail option be sufficient for your needs?

I'd prefer to direct users to that if its generally sufficient rather than supporting a similar functionality here.

On linux it might work. But on Windows it is not. I'm still trying to check if some PowerShell recommendations may work. But still I find more ergonomic to just call one program, not 2.