Cleanup, fixes, more tests

glankk · glankk · commit 648c23a3c8b6 · 2025-07-15T12:38:38.000+02:00
diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp
@@ -1613,6 +1613,12 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
         return Result::Fail;
     }
 
+    for (auto& path : mIgnoredPaths)
+    {
+        path = Path::removeQuotationMarks(std::move(path));
+        path = Path::fromNativeSeparators(std::move(path));
+    }
+
     if (!project.guiProject.pathNames.empty())
         mPathNames = project.guiProject.pathNames;
 
@@ -2138,7 +2144,7 @@ std::list<FileWithDetails> CmdLineParser::filterFiles(const std::vector<std::str
     std::list<FileWithDetails> files;
     PathMatch filtermatcher(fileFilters, Path::getCurrentPath());
     std::copy_if(filesResolved.cbegin(), filesResolved.cend(), std::inserter(files, files.end()), [&](const FileWithDetails& entry) {
-        return filtermatcher.match(entry.path()) || filtermatcher.match(entry.spath());
+        return filtermatcher.match(entry.path());
     });
     return files;
 }
diff --git a/cli/filelister.cpp b/cli/filelister.cpp
@@ -129,9 +129,7 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
             } else {
                 // Directory
                 if (recursive) {
-                    // append a slash if it is a directory since that is what we are doing for mIgnoredPaths directory entries.
-                    // otherwise we would ignore all its contents individually instead as a whole.
-                    if (!ignored.match(fname + '/')) {
+                    if (!ignored.match(fname)) {
                         std::list<FileWithDetails> filesSorted;
 
                         std::string err = addFiles2(filesSorted, fname, extra, recursive, ignored);
@@ -243,9 +241,7 @@ static std::string addFiles2(std::list<FileWithDetails> &files,
 #endif
         if (path_is_directory) {
             if (recursive) {
-                // append a slash if it is a directory since that is what we are doing for mIgnoredPaths directory entries.
-                // otherwise we would ignore all its contents individually instead as a whole.
-                if (!ignored.match(new_path + '/')) {
+                if (!ignored.match(new_path)) {
                     std::string err = addFiles2(files, new_path, extra, recursive, ignored, debug);
                     if (!err.empty()) {
                         return err;
diff --git a/lib/path.cpp b/lib/path.cpp
@@ -173,24 +173,19 @@ std::string Path::getCurrentExecutablePath(const char* fallback)
     return success ? std::string(buf) : std::string(fallback);
 }
 
-static bool issep(char c)
-{
-    return c == '/' || c == '\\';
-}
-
 bool Path::isAbsolute(const std::string& path)
 {
 #ifdef _WIN32
     if (path.length() < 2)
         return false;
 
-    if (issep(path[0]) && issep(path[1]))
+    if ((path[0] == '\\' || path[0] == '/') && (path[1] == '\\' || path[1] == '/'))
         return true;
 
     // On Windows, 'C:\foo\bar' is an absolute path, while 'C:foo\bar' is not
-    return std::isalpha(path[0]) && path[1] == ':' && issep(path[2]);
+    return std::isalpha(path[0]) && path[1] == ':' && (path[2] == '\\' || path[2] == '/');
 #else
-    return !path.empty() && issep(path[0]);
+    return !path.empty() && path[0] == '/';
 #endif
 }
 
diff --git a/lib/pathmatch.cpp b/lib/pathmatch.cpp
@@ -26,18 +26,18 @@
 #include <vector>
 
 
-PathMatch::PathMatch(std::vector<std::string> patterns, std::string basepath, Mode mode) :
-    mPatterns(std::move(patterns)), mBasepath(std::move(basepath)), mMode(mode)
+PathMatch::PathMatch(std::vector<std::string> patterns, std::string basepath, Syntax syntax) :
+    mPatterns(std::move(patterns)), mBasepath(std::move(basepath)), mSyntax(syntax)
 {}
 
 bool PathMatch::match(const std::string &path) const
 {
     return std::any_of(mPatterns.cbegin(), mPatterns.cend(), [=] (const std::string &pattern) {
-        return match(pattern, path, mBasepath, mMode);
+        return match(pattern, path, mBasepath, mSyntax);
     });
 }
 
-bool PathMatch::match(const std::string &pattern, const std::string &path, const std::string &basepath, Mode mode)
+bool PathMatch::match(const std::string &pattern, const std::string &path, const std::string &basepath, Syntax syntax)
 {
     if (pattern.empty())
         return false;
@@ -50,9 +50,9 @@ bool PathMatch::match(const std::string &pattern, const std::string &path, const
     bool real = Path::isAbsolute(pattern) || isRelativePattern(pattern);
 
     /* Pattern iterator */
-    PathIterator s = PathIterator::fromPattern(pattern, basepath, mode == Mode::icase);
+    PathIterator s = PathIterator::fromPattern(pattern, basepath, syntax);
     /* Path iterator */
-    PathIterator t = PathIterator::fromPath(path, basepath, mode == Mode::icase);
+    PathIterator t = PathIterator::fromPath(path, basepath, syntax);
     /* Pattern restart position */
     PathIterator p = s;
     /* Path restart position */
diff --git a/lib/pathmatch.h b/lib/pathmatch.h
@@ -33,63 +33,68 @@
 /// @{
 
 /**
- *  Path matching rules:
- *  - All patterns are canonicalized (path separators vary by platform):
- *    - '/./' => '/'
- *    - '/dir/../' => '/'
- *    - '//' => '/'
- *    - Trailing slashes are removed (root slash is preserved)
- *  - Patterns can contain globs:
- *    - '**' matches any number of characters including path separators.
- *    - '*' matches any number of characters except path separators.
- *    - '?' matches any single character except path separators.
- *  - If a pattern looks like an absolute path (e.g. starts with '/', but varies by platform):
- *    - Match all files where the pattern matches the start of the file's canonical absolute path up until a path
- *      separator or the end of the pathname.
- *  - If a pattern looks like a relative path, i.e. is '.' or '..', or
- *    starts with '.' or '..' followed by a path separator:
- *    - The pattern is interpreted as a path relative to `basepath` and then converted to an absolute path and
- *      treated as such according to the above procedure. If the pattern is relative to some other directory, it should
- *      be modified to be relative to `basepath` first (this should be done with patterns in project files, for example).
- *  - 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.
+ * Path matching rules:
+ * - All patterns are canonicalized (path separators vary by platform):
+ *   - '/./' => '/'
+ *   - '/dir/../' => '/'
+ *   - '//' => '/'
+ *   - Trailing slashes are removed (root slash is preserved)
+ * - Patterns can contain globs:
+ *   - '**' matches any number of characters including path separators.
+ *   - '*' matches any number of characters except path separators.
+ *   - '?' matches any single character except path separators.
+ * - If a pattern looks like an absolute path (e.g. starts with '/', but varies by platform):
+ *   - Match all files where the pattern matches the start of the file's canonical absolute path up until a path
+ *     separator or the end of the pathname.
+ * - If a pattern looks like a relative path, i.e. is '.' or '..', or
+ *   starts with '.' or '..' followed by a path separator:
+ *   - The pattern is interpreted as a path relative to `basepath` and then converted to an absolute path and
+ *     treated as such according to the above procedure. If the pattern is relative to some other directory, it should
+ *     be modified to be relative to `basepath` first (this should be done with patterns in project files, for example).
+ * - 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.
+ *
+ * TODO: Handle less common windows windows syntaxes:
+ *   - Drive-specific relative path: C:dir\foo.cpp
+ *   - Root-relative path: \dir\foo.cpp
  **/
 
 /**
- * @brief Simple path matching for ignoring paths in CLI.
+ * @brief Syntactic path matching for ignoring paths in CLI.
  */
 class CPPCHECKLIB PathMatch {
 public:
 
     /**
-     * @brief Match mode.
+     * @brief Path syntax.
+     *
+     * windows: Case insensitive, forward and backward slashes, UNC or drive letter root.
+     * unix: Case sensitive, forward slashes, slash root.
      *
-     * scase: Case sensitive.
-     * icase: Case insensitive.
      */
-    enum class Mode : std::uint8_t {
-        scase,
-        icase,
+    enum class Syntax : std::uint8_t {
+        windows,
+        unix,
     };
 
     /**
-     * @brief The default mode for the current platform.
+     * @brief The default syntax for the current platform.
      */
 #ifdef _WIN32
-    static constexpr Mode platform_mode = Mode::icase;
+    static constexpr Syntax platform_syntax = Syntax::windows;
 #else
-    static constexpr Mode platform_mode = Mode::scase;
+    static constexpr Syntax platform_syntax = Syntax::unix;
 #endif
 
     /**
      * The constructor.
      *
      * @param patterns List of patterns.
      * @param basepath Path to which patterns and matched paths are relative, when applicable.
-     * @param mode Case sensitivity mode.
+     * @param syntax Path syntax.
      */
-    explicit PathMatch(std::vector<std::string> patterns = {}, std::string basepath = std::string(), Mode mode = platform_mode);
+    explicit PathMatch(std::vector<std::string> patterns = {}, std::string basepath = std::string(), Syntax syntax = platform_syntax);
 
     /**
      * @brief Match path against list of patterns.
@@ -105,10 +110,10 @@ class CPPCHECKLIB PathMatch {
      * @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 Case sensitivity mode.
+     * @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(), Mode mode = platform_mode);
+    static bool match(const std::string &pattern, const std::string &path, const std::string &basepath = std::string(), Syntax syntax = platform_syntax);
 
     /**
      * @brief Check if a pattern is a relative path name.
@@ -152,9 +157,12 @@ class CPPCHECKLIB PathMatch {
     friend class TestPathMatch;
     class PathIterator;
 
+    /* List of patterns */
     std::vector<std::string> mPatterns;
+    /* Base path to with patterns and paths are relative */
     std::string mBasepath;
-    Mode mMode;
+    /* The syntax to use */
+    Syntax mSyntax;
 };
 
 /**
@@ -167,7 +175,7 @@ class CPPCHECKLIB PathMatch {
  * Both strings are optional. If both strings are present, then they're concatenated with a slash
  * (subject to canonicalization).
  *
- * Double-dots at the root level are removed. The root slash is preserved, other trailing slashes are removed.
+ * Double-dots at the root level are removed. Trailing slashes are removed, the root is preserved.
  *
  * Doing the iteration in reverse allows canonicalization to be performed without lookahead. This is useful
  * for comparing path strings, potentially relative to different base paths, without having to do prior string
@@ -182,40 +190,65 @@ class CPPCHECKLIB PathMatch {
 class PathMatch::PathIterator {
 public:
     /* Create from a pattern and base path */
-    static PathIterator fromPattern(const std::string &pattern, const std::string &basepath, bool icase)
+    static PathIterator fromPattern(const std::string &pattern, const std::string &basepath, Syntax syntax)
     {
         if (isRelativePattern(pattern))
-            return PathIterator(basepath.c_str(), pattern.c_str(), icase);
-        return PathIterator(pattern.c_str(), nullptr, icase);
+            return PathIterator(basepath.c_str(), pattern.c_str(), syntax);
+        return PathIterator(pattern.c_str(), nullptr, syntax);
     }
 
     /* Create from path and base path */
-    static PathIterator fromPath(const std::string &path, const std::string &basepath, bool icase)
+    static PathIterator fromPath(const std::string &path, const std::string &basepath, Syntax syntax)
     {
         if (Path::isAbsolute(path))
-            return PathIterator(path.c_str(), nullptr, icase);
-        return PathIterator(basepath.c_str(), path.c_str(), icase);
+            return PathIterator(path.c_str(), nullptr, syntax);
+        return PathIterator(basepath.c_str(), path.c_str(), syntax);
     }
 
     /* Constructor */
-    explicit PathIterator(const char *path_a = nullptr, const char *path_b = nullptr, bool lower = false) :
-        mStart{path_a, path_b}, mLower(lower)
+    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++) {
-            mEnd[i] = mStart[i];
+            const char *&p = mEnd[i];
+            p = mStart[i];
 
-            if (mStart[i] == nullptr || *mStart[i] == '\0')
+            if (p == nullptr || *p == '\0')
                 continue;
 
-            if (mPos.l != 0)
+            if (mPos.l == 0) {
+                /* Check length of root component */
+                if (issep(p[0])) {
+                    mPos.l++;
+                    if (syntax == Syntax::windows && issep(p[1])) {
+                        mPos.l++;
+                        if (p[2] == '.' || p[2] == '?') {
+                            mPos.l++;
+                            if (issep(p[3]))
+                                mPos.l++;
+                        }
+                    }
+                } else if (syntax == Syntax::windows && isdrive(p[0]) && p[1] == ':') {
+                    mPos.l += 2;
+                    if (issep(p[2]))
+                        mPos.l++;
+                }
+                p += mPos.l;
+                mRootLength = mPos.l;
+            } else {
+                /* Add path separator */
                 mPos.l++;
+            }
 
-            while (*mEnd[i] != '\0') {
-                mEnd[i]++;
+            while (*p != '\0') {
+                p++;
                 mPos.l++;
             }
 
-            mPos.p = mEnd[i];
+            mPos.p = p;
         }
 
         if (mPos.l == 0)
@@ -280,19 +313,19 @@ class PathMatch::PathIterator {
 
         char c = mPos.p[-1];
 
-        if (c == '\\')
-            return '/';
-
-        if (mLower)
+        if (mSyntax == Syntax::windows) {
+            if (c == '\\')
+                return '/';
             return std::tolower(c);
+        }
 
         return c;
     }
 
     /* Do canonicalization on a path component boundary */
     void skips(bool leadsep)
     {
-        while (mPos.l != 0) {
+        while (mPos.l > mRootLength) {
             Pos pos = mPos;
 
             if (leadsep) {
@@ -312,7 +345,7 @@ class PathMatch::PathIterator {
                         /* Skip 'dir/../' */
                         nextc();
                         skips(false);
-                        while (mPos.l != 0 && current() != '/')
+                        while (mPos.l > mRootLength && current() != '/')
                             nextc();
                         continue;
                     }
@@ -323,7 +356,7 @@ class PathMatch::PathIterator {
                     /* Skip leading './' */
                     break;
                 }
-            } else if (c == '/' && mPos.l != 1) {
+            } else if (c == '/') {
                 /* Skip double separator (keep root) */
                 nextc();
                 leadsep = false;
@@ -371,8 +404,10 @@ class PathMatch::PathIterator {
     const char *mEnd[2] {};
     /* Current position */
     Pos mPos {};
-    /* Lowercase conversion flag */
-    bool mLower;
+    /* Length of the root component */
+    std::size_t mRootLength {};
+    /* Syntax */
+    Syntax mSyntax;
 };
 
 /// @}
diff --git a/test/cli/proj2_test.py b/test/cli/proj2_test.py
@@ -100,7 +100,7 @@ def test_gui_project_loads_compile_commands_2(tmp_path):
     proj_dir = tmp_path / 'proj2'
     shutil.copytree(__proj_dir, proj_dir)
     __create_compile_commands(proj_dir)
-    exclude_path_1 = 'proj2/b/'
+    exclude_path_1 = 'proj2/b'
     create_gui_project_file(os.path.join(proj_dir, 'test.cppcheck'),
                             import_project='compile_commands.json',
                             exclude_paths=[exclude_path_1])
@@ -157,7 +157,7 @@ def test_gui_project_loads_relative_vs_solution_2(tmp_path):
 def test_gui_project_loads_relative_vs_solution_with_exclude(tmp_path):
     proj_dir = tmp_path / 'proj2'
     shutil.copytree(__proj_dir, proj_dir)
-    create_gui_project_file(os.path.join(tmp_path, 'test.cppcheck'), root_path='proj2', import_project='proj2/proj2.sln', exclude_paths=['b/'])
+    create_gui_project_file(os.path.join(tmp_path, 'test.cppcheck'), root_path='proj2', import_project='proj2/proj2.sln', exclude_paths=['b'])
     ret, stdout, stderr = cppcheck(['--project=test.cppcheck'], cwd=tmp_path)
     assert ret == 0, stdout
     assert stderr == __ERR_A
diff --git a/test/testpathmatch.cpp b/test/testpathmatch.cpp
