ZedThree / clang-tidy-review

Create a pull request review based on clang-tidy warnings
MIT License
88 stars 44 forks source link

clang-tidy-review cannot handle clang-diagnostics-* issues #39

Closed FlorianReimold closed 2 years ago

FlorianReimold commented 2 years ago

In a build where e.g. some header files cannot be built, clang-tidy creates a clang-diagnostics error. This is quite helpful to see, however clang-tidy-review decides to crash when encountering those, as clang-tidy returns a non-zero errorcode in that situation:

https://github.com/continental/ecal/runs/6524850948?check_suite_focus=true#step:4:1955

Maybe it should be enough to simply remove the raise statement, when clang-tidy encounters a compile warning?

https://github.com/ZedThree/clang-tidy-review/blob/1cf54e5e9d293b1a5155a3ecda193519dcdfdd62/review.py#L546

ZedThree commented 2 years ago

I don't think removing the raise statement would be enough, it currently parses the output fixes YAML file, rather than the output of clang-tidy. So we'd need to parse the error in order to turn it into a comment or something.

I think also clang-tidy will stop as soon as it hits an error like this, so maybe it would also be helpful to do each file individually?

FlorianReimold commented 2 years ago

Hm, ok, i was afraid that it wouldn't be as simple as that 😅

Currently (due to the raise) a single compile error will also prevent clang-tidy-review to post information about other warnings, e.g. from clang-analyzer-*. I feel like those should be posted in any way. QtCreator does it the same way - the compile issues (clang-diagnostics-* you have to manually grab from the output, but everything else it will still display in the GUI.

Just not exiting but posting as much as it is able to would already be a proper solution in my mind.

Anyhow, I don't think clang-tidy exits directly in that case, I think it works just fine, but return 1 at the end as a hint that it wasn't able to build all files.

ZedThree commented 2 years ago

Fixed in #40 and released in v0.8.4

Thanks again @FlorianReimold !