[ament_lint_auto] General file exclusion lists

[aprotyas]

What?

A mechanism to provide a list of files to be excluded to ament_lint_auto, with the intention being that all linters running as a result of having invoked ament_lint_auto will inherit these exclusions.

Why?

The main utility with this mechanism would be in declaring a list of common C++/Python source files that should not be linted within a package - common examples would be third-party files or templates.

In working on ros2/geometry2#469, I had to deal with the former type of files mentioned above. I noticed that it would be convenient if we had a way to provide a general file exclusion list to ament_lint_auto. It would simplify this workflow:

Current CMakeLists.txt snippet

# Exclusion list contains ${CMAKE_SOURCE_DIR}/foo
set(_linter_excludes foo)
find_package(ament_lint_auto REQUIRED)
list(APPEND AMENT_LINT_AUTO_EXCLUDE
  ament_cmake_cppcheck
  ament_cmake_cpplint
  ament_cmake_uncrustify
)
ament_lint_auto_find_test_dependencies()
ament_cppcheck(EXCLUDE ${_linter_excludes})
ament_cpplint(EXCLUDE ${_linter_excludes})
ament_uncrustify(EXCLUDE ${_linter_excludes})

To the following workflow:

Desired CMakeLists.txt snippet

find_package(ament_lint_auto REQUIRED)
# Linters will skip ${CMAKE_SOURCE_DIR}/foo
list(APPEND AMENT_LINT_AUTO_FILE_EXCLUDE
  foo
)
ament_lint_auto_find_test_dependencies()

The pain point in the former being the need to manually exclude each of the linters that's supposed to receive the same file exclusion list -- if so, why not just signal that file exclusion list to the linter aggregator?

Implementation consideration

Introducing the ability to populate/append a list variable such as AMENT_LINT_AUTO_FILE_EXCLUDE with the general file exclusion list is probably the path with least friction. Having said that, I have not fully thought through the implementation or if there are any ways a general file exclusion list like this can bite.

Thoughts?

[hidmic]

I'm onboard.

[aprotyas]

After looking into this a little further, this might be a bit convoluted. Here's why:

• ament_lint_auto, as with other auto tools, gets executed by ament_execute_extensions.

  ament_lint/ament_lint_auto/cmake/ament_lint_auto_package_hook.cmake

  Line 21 in d502e7b

  ament_execute_extensions(ament_lint_auto EXCLUDE "${AMENT_LINT_AUTO_EXCLUDE}")

• ament_execute_extensions already accepts an EXCLUDE argument, which is used to exclude ament packages from being executed by the auto tool -- reasonable and expected.
  • Refer to Add option to exclude packages in ament_execute_extensions: ament_cmake#165.
• Wedging in the concept of file exclusion in the ament_execute_extensions layer is a bit out of place since it's only applicable for ament_lint_auto. Plus, the EXCLUDE argument already means something else in that context. (of course, the argument for file exclusion can be named differently, but again it feels wedged in for a single purpose)

Is there a way for file exclusion to reside in harmony with the central execution system used here? What needs to happen is, ament_lint_auto needs to populate the EXCLUDE argument for each linter with the file exclusion list. Any thoughts @hidmic?

[hidmic]

@aprotyas ament_lint_auto does not invoke linters' main APIs, it is linters that register themselves as ament_lint_auto extensions (e.g. here). One could argue that linters should conform to ament_lint_auto conventions in doing so.

In other words, I don't see an issue with abiding to AMENT_LINT_AUTO_FILE_EXCLUDE from linters' hooks.