EXCLUDE argument parsed differently in different linters

[ayrton04]

I think what I am reporting may be related to this issue, but I'm not positive.

When using EXCLUDE for various linters, the argument in question is parsed differently. In cppcheck, it is parsed as a multi_value_keyword:

https://github.com/ament/ament_lint/blob/4bd8e5bc0ae0ed891400a20b7e2fbd0835ca05b8/ament_cmake_cppcheck/cmake/ament_cppcheck.cmake#L38

This allows the user to provide a list of files or directories to exclude. The same is done for copyright, uncrustify, and possibly others.

All of those behave correctly.

However, some of the linter CMake functions have EXCLUDE listed as a one_value_keyword, despite the documentation for those methods suggesting that they accept lists of files and directories. Examples are cpplint and flake8:

https://github.com/ament/ament_lint/blob/4bd8e5bc0ae0ed891400a20b7e2fbd0835ca05b8/ament_cmake_cpplint/cmake/ament_cpplint.cmake#L37

These don't function as they should, because the EXCLUDE argument is only using one value from the provided list. The reason they sometimes appear to work is because of how the arguments are being parsed. Consider this example:

  list(APPEND LIST_OF_FILES path/to/file_a.cpp)
  list(APPEND LIST_OF_FILES path/to/file_b.cpp)
  list(APPEND LIST_OF_FILES path/to/file_c.cpp)

  ament_cpplint(
    EXCLUDE "${LIST_OF_FILES}")

This will get parsed as such:

1. path/to/file_a.cpp will get parsed as the EXCLUDE argument (singular)
2. path/to/file_b.cpp and path/to_file_c.cpp will be parsed as the ARG_UNPARSED_ARGUMENTS.

The net result in this example is that the final command "works": we still construct a python script argument of --exclude path/to/file_a.cpp path/to/file_b.cpp path/to/file_c.cpp, but it only works because we are just tacking on the unparsed arguments to the command.

Now consider this example:

  list(APPEND LIST_OF_FILES path/to/file_a.cpp)
  list(APPEND LIST_OF_FILES path/to/file_b.cpp)
  list(APPEND LIST_OF_FILES path/to/file_c.cpp)

  ament_cpplint(
    EXCLUDE "${LIST_OF_FILES}"
    MAX_LINE_LENGTH 120)

This will not behave as intended. The final command will have arguments --exclude path/to/file_a.cpp --linelength 120 path/to/file_b.cpp path/to/file_c.cpp. This will cause the linter to not exclude the last two files.

If we instead make the argument list for the erroneous methods use multi-value keywords (shown here for cpplint), it works as intended:

  cmake_parse_arguments(ARG "" "MAX_LINE_LENGTH;ROOT;TESTNAME;TIMEOUT" "EXCLUDE;FILTERS" ${ARGN})

TL; DR, I think all of the CMake functions for linting should have the EXCLUDE parameter as one of the multi-value keywords. I am happy to submit a PR, if that would help, or if a minimal example program is needed, I can provide it. But the difference in parameter parsing seems to be clear from the code.

[wjwwood]

From our issue triaging meeting, looks like a bug/oversight which would be nice to fix (make consistent) with multi-value keyword arguments, help implementing it would be appreciated.

[ayrton04]

Happy to!