[ament_uncrustify] Fix file exclusion behavior (#334)

* [ament_uncrustify] Fix file exclusion behavior

This PR fixes the file exclusion behavior reported in #326.

Specifically, the exclusion list is matched against
files/directories as the search path is traversed.

Tries to maintain consistency with #327.

Signed-off-by: Abrar Rahman Protyasha <[email protected]>

* [ament_uncrustify] Add file exclusion tests

Signed-off-by: Abrar Rahman Protyasha <[email protected]>

* [ament_uncrustify] Remove erroneous pytest marker

Signed-off-by: Abrar Rahman Protyasha <[email protected]>

ament_uncrustify/ament_uncrustify/main.py

@@ -19,6 +19,7 @@
 from configparser import ConfigParser
 import difflib
 import filecmp
+import glob
 import os
 import re
 import shutil
@@ -60,9 +61,10 @@ def main(argv=sys.argv[1:]):
                  "'.%s'" % e for e in sorted(c_extensions + cpp_extensions)]))
     parser.add_argument(
         '--exclude',
+        metavar='filename',
         nargs='*',
         default=[],
-        help='Exclude specific file names and directory names from the check')
+        help='Exclude specific file names from the check.')
     parser.add_argument(
         '--language',
         choices=['C', 'C++', 'CPP'],
@@ -112,7 +114,7 @@ def main(argv=sys.argv[1:]):
 
         files_by_language = get_files(
             args.paths, {'C': c_extensions, 'CPP': cpp_extensions},
-            excludes=args.exclude, language=language_)
+            exclude_patterns=args.exclude, language=language_)
         if not files_by_language:
             print('No files found', file=sys.stderr)
             return 1
@@ -215,12 +217,18 @@ def find_executable(file_name, additional_paths=None):
     return shutil.which(file_name, path=path)
 
 
-def get_files(paths, extension_types, excludes=[], language=None):
+def get_files(paths, extension_types, exclude_patterns=[], language=None):
+    excludes = []
+    for exclude_pattern in exclude_patterns:
+        excludes.extend(glob.glob(exclude_pattern))
+    excludes = {os.path.realpath(x) for x in excludes}
+
     extensions_with_dot_to_language = {
         f'.{extension}': language or ext_language
         for ext_language, extensions in extension_types.items()
         for extension in extensions
     }
+
     files = defaultdict(list)
     for path in paths:
         if os.path.isdir(path):
@@ -230,23 +238,23 @@ def get_files(paths, extension_types, excludes=[], language=None):
                     continue
                 # ignore folder starting with . or _
                 dirnames[:] = [d for d in dirnames if d[0] not in ['.', '_']]
-                # ignore excluded folders
-                dirnames[:] = [d for d in dirnames if d not in excludes]
                 dirnames.sort()
 
                 # select files by extension
                 for filename in sorted(filenames):
-                    if filename in excludes:
-                        continue
                     _, ext = os.path.splitext(filename)
                     language = extensions_with_dot_to_language.get(ext, None)
                     if language is not None:
-                        files[language].append(
-                            os.path.normpath(os.path.join(dirpath, filename)))
+                        filepath = os.path.join(dirpath, filename)
+                        if os.path.realpath(filepath) not in excludes:
+                            files[language].append(
+                                os.path.normpath(os.path.join(dirpath, filename)))
         if os.path.isfile(path):
             _, ext = os.path.splitext(path)
             language = extensions_with_dot_to_language.get(ext, None)
-            files[language].append(os.path.normpath(path))
+            if language is not None:
+                if os.path.realpath(path) not in excludes:
+                    files[language].append(os.path.normpath(path))
     return files
 
 

ament_uncrustify/test/cases/test.cpp

@@ -0,0 +1,24 @@
+// Copyright 2021 Open Source Robotics Foundation, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <cstdio>
+
+int main(int argc, char ** argv)
+{
+  (void) argc;
+  (void) argv;
+
+  printf("hello world test package\n");
+  return 0;
+}

ament_uncrustify/test/test_ament_uncrustify.py

@@ -0,0 +1,56 @@
+# Copyright 2021 Open Source Robotics Foundation, Inc.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import os
+
+from ament_uncrustify.main import main
+
+
+cases_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'cases')
+
+
+def test_incorrect_exclusion():
+    """
+    Checks that excluding a single filename does not work.
+
+    `ament_uncrustify <path> --exclude <file-name>` should not exclude anything.
+    """
+    rc = main(argv=[os.path.join(cases_path, 'test.cpp'), '--exclude', 'test.cpp'])
+    assert rc == 0, 'Found errors'
+
+
+def test_correct_exclusion():
+    """
+    Checks that excluding a file relatively/absolutely works as expected.
+
+    `ament_copyright <path/filename> --exclude <path/filename>` should exclude <path/filename>.
+    """
+    rc = main(
+        argv=[
+            os.path.join(cases_path, 'test.cpp'),
+            '--exclude',
+            os.path.join(cases_path, 'test.cpp')
+        ])
+    assert rc == 1, 'Files were found'
+
+
+def test_wildcard_exclusion():
+    """A wildcard expression which expands to the relative path of an existing file should work."""
+    rc = main(
+        argv=[
+            os.path.join(cases_path, 'test.cpp'),
+            '--exclude',
+            os.path.join(cases_path, '*')
+        ])
+    assert rc == 1, 'Files were found'