EXCLUDE argument inconsistency

[alsora]

Hi,
some of the linters support a EXCLUDE argument where you can specify paths to not lint.

I made a test and it looks like the argument is interpreted differently in uncrustify and cpplint.
In my example, I want to exclude the file external/cxxopts-2.1.2/include/cxxopts.hpp.

I have the following CMake code

  ament_cpplint(EXCLUDE ${_linter_excludes})
  ament_uncrustify(
    EXCLUDE ${_linter_excludes}
    LANGUAGE C++
  )

The problem is the following.
If I do set(_linter_excludes external/cxxopts-2.1.2/include/cxxopts.hpp) then the file is correctly excluded by cpplint but not by uncrustify.
On the other hand, if I do set(_linter_excludes external) the opposite happens (the file is correctly excluded by uncrustify but not by cpplint).

A dummy solution consists in having both file-level as well as folder-level exclusions, but this definitely seems a problem if I had to do more advanced exclusion logic.

[aprotyas]

@alsora are you using from source or from the binaries? #334 landed in Rolling v0.11.3 that addresses this issue by fixing ament_uncrustify's exclusion behavior.

[alsora]

I have the latest Rolling with 0.11.4

[aprotyas]

@alsora I have a demo package where both cpplint and uncrustify exclude the file correctly if I spell out the full relative (to the CMakeLists.txt) path, i.e. set(_linter_excludes external/foo/include/bar.hpp). I'm seeing this behavior with the latest Rolling binaries v0.12.3 on Ubuntu 22.04.

Can you verify if this is still an issue for you?

[jclinton830]

Is it possible to parse a list of exclude paths instead of just one? Or if I put in the root of a folder as the exclude path would the linter exclude everything within that root directory?

[aprotyas]

Is it possible to parse a list of exclude paths instead of just one?

@jclinton830 you can provide a list of exclude paths! Check ros2/geometry2/tf2/CMakeLists.txt#L57-L66

  set(
    _linter_excludes
    include/tf2/LinearMath/Matrix3x3.h
    include/tf2/LinearMath/MinMax.h
    include/tf2/LinearMath/QuadWord.h
    include/tf2/LinearMath/Quaternion.h
    include/tf2/LinearMath/Scalar.h
    include/tf2/LinearMath/Transform.h
    include/tf2/LinearMath/Vector3.h
  )

Or if I put in the root of a folder as the exclude path would the linter exclude everything within that root directory?

This should not work, but the ament_ linter wrappers should respect wildcards, so if you can provide the correct wildcards, the linters should ignore every file after wildcard expansion.

[jclinton830]

@aprotyas Thank you!

[jclinton830]

@aprotyas If my exclude path is like seen below, would it ignore all the files and folders within that exclude directory?

set( _linter_excludes extern/googletest )

in ament_cmake_copyright in the cmakelist the EXCLUDE param is described as it can either take in a path to a directory or a file. But When I put in a path to a directory it does not ignore the nested folders within that path.

You mentioned wildcard expansion above, but when I provide the exclude path like below, it only excludes all the .cc files in that folder and the .cc files inside of the nested folders are not excluded.

set( _linter_excludes extern/googletest/*.cc )

My question is, why is set up this way? Its so time consuming to add every file wildcard separately when dealing with a large project.

[aprotyas]

@jclinton830 I can't speak for the original motivation throughout ament_lint, but I chose to maintain that behavior for ament_uncrustify because it makes it harder for users to accidentally opt out of linting, i.e. you have to explicitly tell the tool which files you don't want to lint.

Having said that, I would suggest leveraging your build system (or shell) if you're trying to ignore large sub-trees. If you're invoking the linter through CMake, you can obtain a list of files with GLOB_RECURSE. Something along these lines should work:

file(GLOB_RECURSE
    _linter_excludes
    LIST_DIRECTORIES false
    RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
    extern/googletest/*.(cc|cpp|cxx|hh|hpp|hxx)
)

Shell solutions would probably need extended globbing for ergonomic reasons.

[RFRIEDM-Trimble]

Seems regardless of the value of AMENT_LINT_AUTO_FILE_EXCLUDE, the flake8 linter ignores that value too. If this is too separate from here, I can follow another issue.

[RMichaelSwan]

Having said that, I would suggest leveraging your build system (or shell) if you're trying to ignore large sub-trees. If you're invoking the linter through CMake, you can obtain a list of files with GLOB_RECURSE. Something along these lines should work:

file(GLOB_RECURSE
    _linter_excludes
    LIST_DIRECTORIES false
    RELATIVE ${CMAKE_CURRENT_SOURCE_DIR}
    extern/googletest/*.(cc|cpp|cxx|hh|hpp|hxx)
)

Shell solutions would probably need extended globbing for ergonomic reasons.

@aprotyas this solution only works if the number of files you are excluding is small enough. I get lots of [argument list too long] errors and linters don't run because I have a submodule from an external library that has many source files that I want to ignore, but it seems I cannot ignore subdirectories. Any thoughts?