Fixup #14021 (Add file mode specifier to PathMatch)

cli/filelister.cpp

@@ -129,7 +129,7 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
             } else {
                 // Directory
                 if (recursive) {
-                    if (!ignored.match(fname)) {
+                    if (!ignored.match(fname, PathMatch::Filemode::directory)) {
                         std::list<FileWithDetails> filesSorted;
 
                         std::string err = addFiles2(filesSorted, fname, extra, recursive, ignored);
@@ -241,7 +241,7 @@ static std::string addFiles2(std::list<FileWithDetails> &files,
 #endif
         if (path_is_directory) {
             if (recursive) {
-                if (!ignored.match(new_path)) {
+                if (!ignored.match(new_path, PathMatch::Filemode::directory)) {
                     std::string err = addFiles2(files, new_path, extra, recursive, ignored, debug);
                     if (!err.empty()) {
                         return err;

lib/pathmatch.cpp

@@ -30,29 +30,44 @@ PathMatch::PathMatch(std::vector<std::string> patterns, std::string basepath, Sy
     mPatterns(std::move(patterns)), mBasepath(std::move(basepath)), mSyntax(syntax)
 {}
 
-bool PathMatch::match(const std::string &path) const
+bool PathMatch::match(const std::string &path, Filemode mode) const
 {
     return std::any_of(mPatterns.cbegin(), mPatterns.cend(), [=] (const std::string &pattern) {
-        return match(pattern, path, mBasepath, mSyntax);
+        return match(pattern, path, mBasepath, mode, mSyntax);
     });
 }
 
-bool PathMatch::match(const std::string &pattern, const std::string &path, const std::string &basepath, Syntax syntax)
+bool PathMatch::match(const std::string &pattern, const std::string &path, const std::string &basepath, Filemode mode, Syntax syntax)
 {
+    /* Fast paths for common patterns */
     if (pattern.empty())
         return false;
 
     if (pattern == "*" || pattern == "**")
         return true;
 
-    /* A "real" path is absolute or relative to the base path. A pattern that isn't "real" can match at any
+    /* If the pattern ends with a path separator it matches only directories. If the path names a regular file then
+     * the last path component can't match. */
+    bool dir_mismatch = PathIterator::issep(pattern.back(), syntax) && mode != Filemode::directory;
+
+    if (!dir_mismatch && pattern == path)
+        return true;
+
+    /* A "real" pattern is absolute or relative to the base path. A pattern that isn't real can match at any
      * path component boundary. */
     bool real = Path::isAbsolute(pattern) || isRelativePattern(pattern);
 
     /* Pattern iterator */
     PathIterator s = PathIterator::fromPattern(pattern, basepath, syntax);
     /* Path iterator */
     PathIterator t = PathIterator::fromPath(path, basepath, syntax);
+
+    if (dir_mismatch) {
+        /* Final compponent can't match, so skip it. */
+        while (*t != '\0' && *t != '/')
+            ++t;
+    }
+
     /* Pattern restart position */
     PathIterator p = s;
     /* Path restart position */
@@ -72,8 +87,6 @@ bool PathMatch::match(const std::string &pattern, const std::string &path, const
                 slash = true;
                 ++s;
             }
-            /* Add backtrack for matching zero characters */
-            b.emplace(s.getpos(), t.getpos());
             while (*t != '\0' && (slash || *t != '/')) {
                 if (*s == *t) {
                     /* Could stop here, but do greedy match and add

lib/pathmatch.h

@@ -54,8 +54,11 @@
  * - Otherwise:
  *   - Match all files where the pattern matches any part of the file's canonical absolute path up until a
  *     path separator or the end of the pathname, and the matching part directly follows a path separator.
+ * - If a pattern ends with a path separator before canonicalization then the final path component of the pattern
+ *   only matches the final path component of the file path if the file is a directory (specified by the file mode
+ *   parameter).
  *
- * TODO: Handle less common windows windows syntaxes:
+ * TODO: Handle less common windows syntaxes:
  *   - Drive-specific relative path: C:dir\foo.cpp
  *   - Root-relative path: \dir\foo.cpp
  **/
@@ -87,6 +90,17 @@ class CPPCHECKLIB PathMatch {
     static constexpr Syntax platform_syntax = Syntax::unix;
 #endif
 
+    /**
+     * @brief File mode of a file being matched
+     *
+     * regular: File is a regular file.
+     * directory: File is a directory.
+     */
+    enum class Filemode : std::uint8_t {
+        regular,
+        directory,
+    };
+
     /**
      * The constructor.
      *
@@ -100,20 +114,22 @@ class CPPCHECKLIB PathMatch {
      * @brief Match path against list of patterns.
      *
      * @param path Path to match.
+     * @param mode The file mode of the file named by the path.
      * @return true if any of the masks match the path, false otherwise.
      */
-    bool match(const std::string &path) const;
+    bool match(const std::string &path, Filemode mode = Filemode::regular) const;
 
     /**
      * @brief Match path against a single pattern.
      *
      * @param pattern Pattern to use.
      * @param path Path to match.
      * @param basepath Path to which the pattern and path is relative, when applicable.
+     * @param mode The file mode of the file named by the path.
      * @param syntax Path syntax.
      * @return true if the pattern matches the path, false otherwise.
      */
-    static bool match(const std::string &pattern, const std::string &path, const std::string &basepath = std::string(), Syntax syntax = platform_syntax);
+    static bool match(const std::string &pattern, const std::string &path, const std::string &basepath = std::string(), Filemode mode = Filemode::regular, Syntax syntax = platform_syntax);
 
     /**
      * @brief Check if a pattern is a relative path name.
@@ -209,12 +225,6 @@ class PathMatch::PathIterator {
     explicit PathIterator(const char *path_a = nullptr, const char *path_b = nullptr, Syntax syntax = platform_syntax) :
         mStart{path_a, path_b}, mSyntax(syntax)
     {
-        const auto issep = [syntax] (char c) {
-            return c == '/' || (syntax == Syntax::windows && c == '\\');
-        };
-        const auto isdrive = [] (char c) {
-            return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
-        };
 
         for (int i = 0; i < 2; i++) {
             const char *&p = mEnd[i];
@@ -225,19 +235,19 @@ class PathMatch::PathIterator {
 
             if (mPos.l == 0) {
                 /* Check length of root component */
-                if (issep(p[0])) {
+                if (issep(p[0], syntax)) {
                     mRootLength++;
-                    if (syntax == Syntax::windows && issep(p[1])) {
+                    if (syntax == Syntax::windows && issep(p[1], syntax)) {
                         mRootLength++;
                         if (p[2] == '.' || p[2] == '?') {
                             mRootLength++;
-                            if (issep(p[3]))
+                            if (issep(p[3], syntax))
                                 mRootLength++;
                         }
                     }
                 } else if (syntax == Syntax::windows && isdrive(p[0]) && p[1] == ':') {
                     mRootLength += 2;
-                    if (issep(p[2]))
+                    if (issep(p[2], syntax))
                         mRootLength++;
                 }
                 p += mRootLength;
@@ -308,6 +318,18 @@ class PathMatch::PathIterator {
         return str;
     }
 
+    /* Syntax helper, check if a character is a path separator */
+    static bool issep(char c, Syntax syntax)
+    {
+        return c == '/' || (syntax == Syntax::windows && c == '\\');
+    }
+
+    /* Syntax helper, check if a chracter is a drive letter */
+    static bool isdrive(char c)
+    {
+        return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
+    }
+
 private:
     /* Read the current character */
     char current() const

lib/suppressions.cpp

@@ -401,7 +401,7 @@ SuppressionList::Suppression::Result SuppressionList::Suppression::isSuppressed(
             if (!thisAndNextLine || lineNumber + 1 != errmsg.lineNumber)
                 return Result::None;
         }
-        if (!fileName.empty() && fileName != errmsg.getFileName() && !PathMatch::match(fileName, errmsg.getFileName()))
+        if (!fileName.empty() && !PathMatch::match(fileName, errmsg.getFileName()))
             return Result::None;
         if (hash > 0 && hash != errmsg.hash)
             return Result::Checked;

test/testpathmatch.cpp

@@ -30,6 +30,8 @@ class TestPathMatch : public TestFixture {
 private:
     static constexpr auto unix = PathMatch::Syntax::unix;
     static constexpr auto windows = PathMatch::Syntax::windows;
+    static constexpr auto ifreg = PathMatch::Filemode::regular;
+    static constexpr auto ifdir = PathMatch::Filemode::directory;
 #ifdef _WIN32
     const std::string basepath{"C:\\test"};
 #else
@@ -73,7 +75,8 @@ class TestPathMatch : public TestFixture {
         TEST_CASE(filemaskpath3);
         TEST_CASE(filemaskpath4);
         TEST_CASE(mixedallmatch);
-        TEST_CASE(glob);
+        TEST_CASE(glob1);
+        TEST_CASE(glob2);
         TEST_CASE(globstar1);
         TEST_CASE(globstar2);
         TEST_CASE(pathiterator);
@@ -85,16 +88,16 @@ class TestPathMatch : public TestFixture {
     }
 
     void emptymaskpath1() const {
-        ASSERT(!emptyMatcher.match("src/"));
+        ASSERT(!emptyMatcher.match("src/", ifdir));
     }
 
     void emptymaskpath2() const {
-        ASSERT(!emptyMatcher.match("../src/"));
+        ASSERT(!emptyMatcher.match("../src/", ifdir));
     }
 
     void emptymaskpath3() const {
-        ASSERT(!emptyMatcher.match("/home/user/code/src/"));
-        ASSERT(!emptyMatcher.match("d:/home/user/code/src/"));
+        ASSERT(!emptyMatcher.match("/home/user/code/src/", ifdir));
+        ASSERT(!emptyMatcher.match("d:/home/user/code/src/", ifdir));
     }
 
     // Test PathMatch containing "src/"
@@ -103,17 +106,20 @@ class TestPathMatch : public TestFixture {
     }
 
     void onemasksamepath() const {
-        ASSERT(srcMatcher.match("src/"));
+        ASSERT(srcMatcher.match("src/", ifdir));
+        ASSERT(!srcMatcher.match("src/", ifreg));
     }
 
     void onemasksamepathdifferentslash() const {
         PathMatch srcMatcher2({"src\\"}, basepath, windows);
-        ASSERT(srcMatcher2.match("src/"));
+        ASSERT(srcMatcher2.match("src/", ifdir));
+        ASSERT(!srcMatcher2.match("src/", ifreg));
     }
 
     void onemasksamepathdifferentcase() const {
         PathMatch match({"sRc/"}, basepath, windows);
-        ASSERT(match.match("srC/"));
+        ASSERT(match.match("srC/", ifdir));
+        ASSERT(!match.match("srC/", ifreg));
     }
 
     void onemasksamepathwithfile() const {
@@ -125,8 +131,8 @@ class TestPathMatch : public TestFixture {
         const std::string shorterToMatch("src/");
         ASSERT(shorterToMatch.length() < longerExclude.length());
         PathMatch match({longerExclude});
-        ASSERT(match.match(longerExclude));
-        ASSERT(!match.match(shorterToMatch));
+        ASSERT(match.match(longerExclude, ifdir));
+        ASSERT(!match.match(shorterToMatch, ifdir));
     }
 
     void onemaskdifferentdir1() const {
@@ -146,40 +152,43 @@ class TestPathMatch : public TestFixture {
     }
 
     void onemasklongerpath1() const {
-        ASSERT(srcMatcher.match("/tmp/src/"));
-        ASSERT(srcMatcher.match("d:/tmp/src/"));
+        ASSERT(srcMatcher.match("/tmp/src/", ifdir));
+        ASSERT(srcMatcher.match("d:/tmp/src/", ifdir));
     }
 
     void onemasklongerpath2() const {
-        ASSERT(srcMatcher.match("src/module/"));
+        ASSERT(srcMatcher.match("src/module/", ifdir));
     }
 
     void onemasklongerpath3() const {
-        ASSERT(srcMatcher.match("project/src/module/"));
+        ASSERT(srcMatcher.match("project/src/module/", ifdir));
     }
 
     void onemaskcwd() const {
-        ASSERT(srcMatcher.match("./src"));
+        ASSERT(srcMatcher.match("./src", ifdir));
     }
 
     void twomasklongerpath1() const {
         PathMatch match({ "src/", "module/" });
-        ASSERT(!match.match("project/"));
+        ASSERT(!match.match("project/", ifdir));
     }
 
     void twomasklongerpath2() const {
         PathMatch match({ "src/", "module/" });
-        ASSERT(match.match("project/src/"));
+        ASSERT(match.match("project/src/", ifdir));
+        ASSERT(!match.match("project/src/", ifreg));
     }
 
     void twomasklongerpath3() const {
         PathMatch match({ "src/", "module/" });
-        ASSERT(match.match("project/module/"));
+        ASSERT(match.match("project/module/", ifdir));
+        ASSERT(!match.match("project/module/", ifreg));
     }
 
     void twomasklongerpath4() const {
         PathMatch match({ "src/", "module/" });
-        ASSERT(match.match("project/src/module/"));
+        ASSERT(match.match("project/src/module/", ifdir));
+        ASSERT(match.match("project/src/module/", ifreg));
     }
 
     // Test PathMatch containing "foo.cpp"
@@ -224,11 +233,12 @@ class TestPathMatch : public TestFixture {
     void mixedallmatch() const { // #13570
         // when trying to match a directory against a directory entry it erroneously modified a local variable also used for file matching
         PathMatch match({ "tests/", "file.c" });
-        ASSERT(match.match("tests/"));
+        ASSERT(match.match("tests/", ifdir));
+        ASSERT(!match.match("tests/", ifreg));
         ASSERT(match.match("lib/file.c"));
     }
 
-    void glob() const {
+    void glob1() const {
         PathMatch match({"test?.cpp"});
         ASSERT(match.match("test1.cpp"));
         ASSERT(match.match("src/test1.cpp"));
@@ -237,12 +247,22 @@ class TestPathMatch : public TestFixture {
         ASSERT(!match.match("test.cpp"));
     }
 
+    void glob2() const {
+        PathMatch match({"test*.cpp"});
+        ASSERT(match.match("test1.cpp"));
+        ASSERT(match.match("src/test1.cpp"));
+        ASSERT(match.match("test1.cpp/src"));
+        ASSERT(!match.match("test1.c"));
+        ASSERT(match.match("test.cpp"));
+    }
+
     void globstar1() const {
         PathMatch match({"src/**/foo.c"});
         ASSERT(match.match("src/lib/foo/foo.c"));
         ASSERT(match.match("src/lib/foo/bar/foo.c"));
         ASSERT(!match.match("src/lib/foo/foo.cpp"));
         ASSERT(!match.match("src/lib/foo/bar/foo.cpp"));
+        ASSERT(!match.match("src/foo.c"));
     }
 
     void globstar2() const {
@@ -251,6 +271,7 @@ class TestPathMatch : public TestFixture {
         ASSERT(match.match("src/lib/foo/bar/foo.c"));
         ASSERT(!match.match("src/lib/foo/foo.cpp"));
         ASSERT(!match.match("src/lib/foo/bar/foo.cpp"));
+        ASSERT(!match.match("src/foo.c"));
     }
 
     void pathiterator() const {