Why use diff instead of exit status for pass/fail?
zbeekman opened this issue · 5 comments
On macOS if I pass --check=*
to clang-tidy, it flags a macro expanded from a system header. If I add the appropriate clang-tidy directive to ignore the error on that line, the pre-commit hook still fails:
clang-tidy...........................................................................Failed
hookid: clang-tidy
1080 warnings generated.
Suppressed 1090 warnings (1080 in non-user code, 10 NOLINT).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
To elaborate: Why all the fancy diff legwork? Why not just examine the exit status?
The reason is that clang-tidy does not reliably produce a non-zero exit status on warnings/errors.
This bash example demonstrates this:
cd /tmp
echo '[{"directory":"/tmp","command":"gcc a.c -o a.o","file":"/tmp/a.c"}]' > compile_commands.json
echo 'int main() { int i; return 10; }' > a.c
clang-tidy a.c --checks=*
> 2 warnings generated.
> a.c:1:28: warning: 10 is a magic number; consider replacing it with a named constant [cppcoreguidelines-avoid-magic-numbers]
> int main() { int i; return 10; }
> ^
echo $?
> 0
Hmmmm interesting. But if you silence errors/warnings with comments then the pre-commit action is still flagged as a failure. I guess I'll close this issue. You'd think that they would at least have a flag to exit non-zero, or turn warnings into errors.
@pocc Do you know if this is true even when using:
--warnings-as-errors=<string> -
Upgrades warnings to errors. Same format as
'-checks'.
This option's value is appended to the value of
the 'WarningsAsErrors' option in .clang-tidy
file, if any.
?
--warnings-as-errors
could be a valid alternative for clang-tidy. If memory serves, oclint returns 0 even on error. Thus diffing provides a more reliable method of determining pass/fail across all tools. I agree that exit status should be relied upon.
If at some point, all of the linters currently in this repo have a consistent and reliable exit status, please reopen this issue.