Fix over-eager file matching in skip files and --file option (#4670)

furtib · barnabasdomozi · web-flow · commit 77d04042c17b · 2025-11-25T10:42:26.000Z
* Remove '*' from skip file discorvery to match documentation better --file=a.c should not match a.cpp * Add '*' to entries that trivially folders * Revert "Add '*' to entries that trivially folders" This reverts commit b1ac6f6. * Add in regex to only match all files if the specified input is a directory * Fix regex, add comment explaining what does it do * More sophisticated removal of '\Z' in skip file regex * Handle windows paths correctly in skipfiles * Update platform specific separator * Escape that separator every time * Remove the version specific line * Remove unnecesarry import * Generalyze analyze command * Add test for filenames that are prefixes for each other * Add test for folder skip recognition * Remove trailing whitespaces * Fix linting errors * Fix typos, filenames, and Makefiles * Rewrite test to be more extensible * Comment skip_directory test * Fix lint errors * Fix test * change *.o to $(obgs) Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com> * Remove unnecesary space Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com> * Change skipme/*.o to $(OBJS) Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com> * Update comment in analyzer/tests/functional/skip/test_skip.py Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com> * Update comment in analyzer/tests/functional/skip/test_skip.py Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com> * Remove unnecesary space Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com> * Fix comments and make files * Fragment comment to avoid too long line * Update analyzer/tests/functional/skip/test_skip.py Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com> * Update analyzer/tests/functional/skip/test_files/similar/Makefile Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com> * Update analyzer/tests/functional/skip/test_skip.py Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com> --------- Co-authored-by: Barnabás Domozi <142594183+barnabasdomozi@users.noreply.github.com>
diff --git a/analyzer/tests/functional/skip/test_files/similar/Makefile b/analyzer/tests/functional/skip/test_files/similar/Makefile
@@ -0,0 +1,21 @@
+C_OBJS = $(C_SRCS:.c=_c.o)
+CPP_OBJS = $(CPP_SRCS:.cpp=_cpp.o)
+
+OBJS = $(C_OBJS) $(CPP_OBJS)
+
+CXXFLAGS += -Wno-all -Wno-extra -Wno-division-by-zero
+CFLAGS += -Wno-division-by-zero
+
+C_SRCS = simple.c
+CPP_SRCS = simple.cpp
+
+all: $(OBJS)
+
+$(CPP_OBJS): %_cpp.o: %.cpp
+	$(CXX) $(CXXFLAGS) -c $< -o $@
+
+$(C_OBJS): %_c.o: %.c
+	$(CC) $(CFLAGS) -c $< -o $@
+
+clean:
+	rm -rf $(OBJS)
diff --git a/analyzer/tests/functional/skip/test_files/similar/simple.c b/analyzer/tests/functional/skip/test_files/similar/simple.c
@@ -0,0 +1,13 @@
+// -------------------------------------------------------------------------
+//  Part of the CodeChecker project, under the Apache License v2.0 with
+//  LLVM Exceptions. See LICENSE for license information.
+//  SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+// -------------------------------------------------------------------------
+
+// This file is to be included in a --file argument.
+
+void main(int z) {
+  int x;
+  if (z == 0)
+    x = 1 / z; // warn
+}
diff --git a/analyzer/tests/functional/skip/test_files/similar/simple.cpp b/analyzer/tests/functional/skip/test_files/similar/simple.cpp
@@ -0,0 +1,12 @@
+// -------------------------------------------------------------------------
+//  Part of the CodeChecker project, under the Apache License v2.0 with
+//  LLVM Exceptions. See LICENSE for license information.
+//  SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+// -------------------------------------------------------------------------
+
+// This file is not to be added to a --file argument.
+
+void skipped_test(int z) {
+  if (z == 0)
+    int x = 1 / z; // warn
+}
diff --git a/analyzer/tests/functional/skip/test_files/skip_folder/Makefile b/analyzer/tests/functional/skip/test_files/skip_folder/Makefile
@@ -0,0 +1,13 @@
+OBJS = $(SRCS:.cpp=.o)
+
+CXXFLAGS += -Wno-all -Wno-extra -Wno-division-by-zero
+
+SRCS = skipme/skipme.cpp
+
+all: $(OBJS)
+
+.cpp.o:
+	$(CXX) $(CXXFLAGS) -c $< -o $@
+
+clean:
+	rm -rf $(OBJS)
diff --git a/analyzer/tests/functional/skip/test_files/skip_folder/skipme/skipme.cpp b/analyzer/tests/functional/skip/test_files/skip_folder/skipme/skipme.cpp
@@ -0,0 +1,12 @@
+// -------------------------------------------------------------------------
+//  Part of the CodeChecker project, under the Apache License v2.0 with
+//  LLVM Exceptions. See LICENSE for license information.
+//  SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+// -------------------------------------------------------------------------
+
+// This file is to be included in a skip list file.
+
+void skipped_test(int z) {
+  if (z == 0)
+    int x = 1 / z; // warn
+}
diff --git a/analyzer/tests/functional/skip/test_skip.py b/analyzer/tests/functional/skip/test_skip.py
@@ -65,13 +65,14 @@ def setup_method(self, _):
         self.report_dir = os.path.join(self.test_workspace, "reports")
         self.test_dir = os.path.join(os.path.dirname(__file__), 'test_files')
 
-    def __analyze_simple(self,
-                         build_json,
-                         analyzer_extra_options=None,
-                         cwd=None):
-        """ Analyze the 'simple' project. """
+    def __analyze(self,
+                  target,
+                  build_json,
+                  analyzer_extra_options=None,
+                  cwd=None):
+        """ Analyze the target project. """
         if not cwd:
-            cwd = os.path.join(self.test_dir, "simple")
+            cwd = os.path.join(self.test_dir, target)
 
         analyze_cmd = [
             self._codechecker_cmd, "analyze", "-c", build_json,
@@ -94,9 +95,9 @@ def __analyze_simple(self,
         self.assertEqual(process.returncode, 0)
         return out, err
 
-    def __log_and_analyze_simple(self, analyzer_extra_options=None):
-        """ Log and analyze the 'simple' project. """
-        test_dir = os.path.join(self.test_dir, "simple")
+    def __log_and_analyze(self, target, analyzer_extra_options=None):
+        """ Log and analyze the target project. """
+        test_dir = os.path.join(self.test_dir, target)
         build_json = os.path.join(self.test_workspace, "build.json")
 
         clean_cmd = ["make", "clean"]
@@ -113,7 +114,7 @@ def __log_and_analyze_simple(self, analyzer_extra_options=None):
                                       encoding="utf-8", errors="ignore")
         print(out)
         # Create and run analyze command.
-        return self.__analyze_simple(build_json, analyzer_extra_options)
+        return self.__analyze(target, build_json, analyzer_extra_options)
 
     def __run_parse(self, extra_options=None):
         """ Run parse command with the given extra options. """
@@ -138,7 +139,7 @@ def test_skip(self):
         """Analyze a project with a skip file."""
         # we should see a report from skip.h
         # we should not see a report from file_to_be_skipped.cpp
-        self.__log_and_analyze_simple(["--ignore", "skipfile_drop"])
+        self.__log_and_analyze("simple", ["--ignore", "skipfile_drop"])
 
         # Check if file is skipped.
         report_dir_files = os.listdir(self.report_dir)
@@ -170,13 +171,39 @@ def test_skip(self):
                             "Reports from headers should be kept"
                             " if the header is not on the skiplist")
 
+    def test_skip_directory(self):
+        """Test whether directories in skipfile entries works correctly"""
+        skip_file_contents: list[list[str]] = [
+            ["-*skipme/*"],  # The way it is in the documentation
+            ["-*skipme/"],  # If the user specified that this is a directory
+            ["-*skipme"],  # If we can't even be sure that this is a directory
+        ]
+        for skip_file_content in skip_file_contents:
+            with tempfile.NamedTemporaryFile(
+                mode="w", suffix="skipfile", encoding="utf-8"
+            ) as skip_file:
+
+                # Skip the `skipme` folder
+                skip_file.write('\n'.join(skip_file_content))
+                skip_file.flush()
+                self.__log_and_analyze(
+                    "skip_folder", ["--ignore", skip_file.name]
+                )
+
+                # Check if the folder `skipme` is skipped
+                # There shouldn't be any report
+                # generated for `skipme/skipme.cpp`
+                report_dir_files = os.listdir(self.report_dir)
+                for f in report_dir_files:
+                    self.assertFalse("skipme.cpp" in f)
+
     def test_drop_reports(self):
         """Analyze a project with a skip file."""
         # we should not see a report from skip.h
         # we should not see a report from file_to_be_skipped.cpp
 
-        self.__log_and_analyze_simple(["--ignore", "skipfile",
-                                       "--drop-reports-from-skipped-files"])
+        self.__log_and_analyze("simple", ["--ignore", "skipfile",
+                                          "--drop-reports-from-skipped-files"])
 
         # Check if file is skipped.
         report_dir_files = os.listdir(self.report_dir)
@@ -296,15 +323,15 @@ def test_analyze_skip_everything(self):
             skip_file.write('-*')
             skip_file.flush()
 
-            self.__log_and_analyze_simple([
-                "--ignore", skip_file.name])
+            self.__log_and_analyze("simple",
+                                   ["--ignore", skip_file.name])
             self.assertFalse(
                 glob.glob(os.path.join(self.report_dir, '*.plist')))
 
     def test_analyze_header_with_file_option(self):
         """ Analyze a header file with the --file option. """
         header_file = os.path.join(self.test_dir, "simple", "skip.h")
-        out, _ = self.__log_and_analyze_simple(["--file", header_file])
+        out, _ = self.__log_and_analyze("simple", ["--file", header_file])
         self.assertIn(
             f"Get dependent source files for '{header_file}'...", out)
         self.assertIn(
@@ -342,7 +369,7 @@ def check_parse_out(out):
         shutil.copytree(Path(self.test_dir, "simple"),
                         Path(self.test_workspace, "rel_simple"))
         # Obtain a compilation database, copy it to the prepared test workspace
-        self.__log_and_analyze_simple()
+        self.__log_and_analyze("simple")
         build_json = Path(self.test_workspace, "rel_simple", "build.json")
         shutil.copy(Path(self.test_workspace, "build.json"), build_json)
 
@@ -358,8 +385,17 @@ def check_parse_out(out):
             json.dump(compilation_commands, f)
 
         # Do the CodeChecker Analyze with --file
-        out, _ = self.__analyze_simple(build_json, ["--clean", "--file", Path(
-            self.test_workspace, "rel_simple", "skip_header.cpp").absolute()])
+        out, _ = self.__analyze(
+            "simple",
+            build_json,
+            [
+                "--clean",
+                "--file",
+                Path(
+                    self.test_workspace, "rel_simple", "skip_header.cpp"
+                ).absolute(),
+            ],
+        )
         check_analyze_out(out)
 
         out = self.__run_parse()
@@ -374,14 +410,18 @@ def check_parse_out(out):
 
         # Do the CodeChecker Analyze with --file
         # We also need to set cwd to test_workspace
-        out, _ = self.__analyze_simple(build_json,
-                                       ["--clean",
-                                        "--file",
-                                           Path(
-                                               self.test_workspace,
-                                               "rel_simple",
-                                               "skip_header.cpp").absolute()],
-                                       cwd=self.test_workspace)
+        out, _ = self.__analyze(
+            "simple",
+            build_json,
+            [
+                "--clean",
+                "--file",
+                Path(
+                    self.test_workspace, "rel_simple", "skip_header.cpp"
+                ).absolute(),
+            ],
+            cwd=self.test_workspace,
+        )
         check_analyze_out(out)
 
         out = self.__run_parse()
@@ -409,7 +449,7 @@ def test_analyze_header_with_file_option_and_intercept_json(self):
             json.dump(build_actions, f)
 
         header_file = os.path.join(self.test_dir, "simple", "skip.h")
-        out, _ = self.__analyze_simple(build_json, ["--file", header_file])
+        out, _ = self.__analyze("simple", build_json, ["--file", header_file])
         self.assertIn(
             f"Get dependent source files for '{header_file}'...", out)
         self.assertIn(
@@ -432,7 +472,7 @@ def test_analyze_file_option_skip_everything(self):
             skip_file.write('-*')
             skip_file.flush()
 
-            self.__log_and_analyze_simple([
+            self.__log_and_analyze("simple", [
                 "--ignore", skip_file.name,
                 "--file", "*/file_to_be_skipped.cpp"])
             self.assertFalse(
@@ -456,7 +496,7 @@ def test_analyze_file_option(self):
             ]))
             skip_file.flush()
 
-            self.__log_and_analyze_simple([
+            self.__log_and_analyze("simple", [
                 "--ignore", skip_file.name,
                 "--file", "*/skip_header.cpp"])
             print(glob.glob(
@@ -473,7 +513,7 @@ def test_analyze_file_option(self):
             ]))
             skip_file.flush()
 
-            self.__log_and_analyze_simple([
+            self.__log_and_analyze("simple", [
                 "--ignore", skip_file.name,
                 "--file", "*/skip_header.cpp"])
             print(glob.glob(
@@ -482,11 +522,23 @@ def test_analyze_file_option(self):
                 any('skip_header.cpp' not in f for f in glob.glob(
                     os.path.join(self.report_dir, '*.plist'))))
 
+    def test_analyze_similar_file_option(self):
+        """
+        Test analyze command --file option for edge case,
+        where one filename is a prefix of another.
+        """
+        self.__log_and_analyze("similar", ["--file", "*/simple.c"])
+        print(glob.glob(
+            os.path.join(self.report_dir, '*.plist')))
+        self.assertFalse(
+            any('simple.cpp' in f for f in glob.glob(
+                os.path.join(self.report_dir, '*.plist'))))
+
     def test_analyze_only_file_option(self):
         """
         Test analyze command --file option without a skip file.
         """
-        self.__log_and_analyze_simple([
+        self.__log_and_analyze("simple", [
             "--file", "*/skip_header.cpp"])
         self.assertFalse(
             any('skip_header.cpp' not in f for f in glob.glob(
@@ -496,7 +548,7 @@ def test_parse_file_option(self):
         """ Test parse command --file option. """
         skipfile = os.path.join(self.test_dir, "simple", "skipfile")
 
-        self.__log_and_analyze_simple()
+        self.__log_and_analyze("simple")
 
         # Only reports from the given files are returned.
         out, _, returncode = self.__run_parse(
diff --git a/codechecker_common/skiplist_handler.py b/codechecker_common/skiplist_handler.py
@@ -52,8 +52,18 @@ def __gen_regex(self, skip_lines):
         """
         for skip_line in skip_lines:
             norm_skip_path = os.path.normpath(skip_line[1:].strip())
+            # fnmatch places a '\Z' (end of input) at the end, in this case,
+            # this is equivalent to '$' (end of line). This must be removed.
+            # Optionally we match the user given path with a '/.*' ending.
+            # This, if the given input is a folder will
+            # resolve all files contained within.
+            # Note: normalization removes '/' from the end, see:
+            # https://docs.python.org/3/library/os.path.html#os.path.normpath
+            translated_glob = fnmatch.translate(norm_skip_path)
+            if translated_glob.endswith(r"\Z"):
+                translated_glob = translated_glob[:-2]
             rexpr = re.compile(
-                fnmatch.translate(norm_skip_path + '*'))
+                translated_glob + fr"(?:\{os.path.sep}.*)?$")
             self.__skip.append((skip_line, rexpr))
 
     def __check_line_format(self, skip_lines):
