ament / ament_lint

Apache License 2.0
38 stars 107 forks source link

ament_clang_tidy - Fix Reporting when WarningsAsErrors is specified in config (backport #397) #490

Open mergify[bot] opened 7 months ago

mergify[bot] commented 7 months ago

Issue Summary

When WarningsAsErrors is specified in .clang-tidy, this can lead to some failures in result reporting. Specifically, the following test program (src/factorial.cpp):

int Factorial(int n) {
    int result = 1;
    int fake = 1;
    for (int i = 1; i <= n; i++) {
        result *= i;
    }
    return result;
}

fails with this output:

The invocation of "clang-tidy" failed with error code 1: Command '['/usr/bin/clang-tidy', '-p', 'path/test_examples_cmake', '--header-filter', 'include/test_examples_cmake/.*', 'path']' returned non-zero exit status 1.

when running ament_clang_tidy.

After applying this fix, I get the following output, which is expected

src/factorial.cpp:36:9: error: unused variable 'fake' [clang-diagnostic-unused-variable,-warnings-as-errors]
    int fake = 1;
        ^

Relevant section of .clang-tidy, for reference:

Checks:
    'bugprone*,
     clang-analyzer*,
     cert*,
     performance*,
     portability*,
     readability*,
     hicpp*,
     -hicpp-signed-bitwise,
     google-readability-todo,
    '
FormatStyle:     file
WarningsAsErrors: '*'

Fix

By trying to access the output attribute of the raised exception, we can populate the output in cases where exceptions were raised and there was valid output.


This is an automatic backport of pull request #397 done by Mergify.

sloretz commented 6 months ago

Assigned @clalancette to run CI and merge 🧇

clalancette commented 6 months ago

I'm going to suggest we hold off on this one until after the Jazzy release. It is low-risk, but also low-impact, so I think it is fine to put it in for a patch release.