fixed #13641 - improved ignoring directories and overall testing of ignore

cli/cmdlineparser.cpp

@@ -276,7 +276,7 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
         // TODO: verbose log which files were ignored?
         const PathMatch matcher(ignored, caseSensitive);
         for (const std::string &pathname : pathnamesRef) {
-            const std::string err = FileLister::recursiveAddFiles(filesResolved, Path::toNativeSeparators(pathname), mSettings.library.markupExtensions(), matcher);
+            const std::string err = FileLister::recursiveAddFiles(filesResolved, Path::toNativeSeparators(pathname), mSettings.library.markupExtensions(), matcher, mSettings.debugignore);
             if (!err.empty()) {
                 // TODO: bail out?
                 mLogger.printMessage(err);
@@ -619,6 +619,10 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
             else if (std::strcmp(argv[i], "--debug-clang-output") == 0)
                 mSettings.debugClangOutput = true;
 
+            // Show debug messages for ignored files
+            else if (std::strcmp(argv[i], "--debug-ignore") == 0)
+                mSettings.debugignore = true;
+
             // Show --debug output after the first simplifications
             else if (std::strcmp(argv[i], "--debug") == 0 ||
                      std::strcmp(argv[i], "--debug-normal") == 0)
@@ -821,15 +825,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
                 }
 
                 if (!path.empty()) {
-                    path = Path::removeQuotationMarks(std::move(path));
-                    path = Path::simplifyPath(std::move(path));
-
-                    // TODO: this only works when it exists
-                    if (Path::isDirectory(path)) {
-                        // If directory name doesn't end with / or \, add it
-                        if (!endsWith(path, '/'))
-                            path += '/';
-                    }
                     mIgnoredPaths.emplace_back(std::move(path));
                 }
             }
@@ -1590,11 +1585,27 @@ 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::simplifyPath(std::move(path));
+
+        bool isdir = false;
+        if (!Path::exists(path, &isdir) && mSettings.debugignore)
+            std::cout << "path to ignore does not exist: " << path << std::endl;
+        // TODO: this only works when it exists
+        if (isdir) {
+            // If directory name doesn't end with / or \, add it
+            if (!endsWith(path, '/'))
+                path += '/';
+        }
+    }
+
     if (!project.guiProject.pathNames.empty())
         mPathNames = project.guiProject.pathNames;
 
     if (!project.fileSettings.empty()) {
-        project.ignorePaths(mIgnoredPaths);
+        project.ignorePaths(mIgnoredPaths, mSettings.debugignore);
         if (project.fileSettings.empty()) {
             mLogger.printError("no C or C++ source files found.");
             mLogger.printMessage("all paths were ignored"); // TODO: log this differently?

cli/filelister.cpp

@@ -29,6 +29,7 @@
 #include "utils.h"
 
 #include <cstring>
+#include <iostream>
 #include <iterator>
 #include <memory>
 
@@ -45,7 +46,7 @@
 // When compiling Unicode targets WinAPI automatically uses *W Unicode versions
 // of called functions. Thus, we explicitly call *A versions of the functions.
 
-static std::string addFiles2(std::list<FileWithDetails>&files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored)
+static std::string addFiles2(std::list<FileWithDetails>&files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored, bool debug = false)
 {
     const std::string cleanedPath = Path::toNativeSeparators(path);
 
@@ -85,8 +86,9 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
     HANDLE hFind = FindFirstFileA(searchPattern.c_str(), &ffd);
     if (INVALID_HANDLE_VALUE == hFind) {
         const DWORD err = GetLastError();
-        if (err == ERROR_FILE_NOT_FOUND) {
-            // no files matched
+        if (err == ERROR_FILE_NOT_FOUND || // the pattern did not match anything
+            err == ERROR_PATH_NOT_FOUND)   // the given search path does not exist
+        {
             return "";
         }
         return "finding files failed. Search pattern: '" + searchPattern + "'. (error: " + std::to_string(err) + ")";
@@ -106,22 +108,30 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
             if ((ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0) {
                 // File
                 Standards::Language lang = Standards::Language::None;
-                if ((!checkAllFilesInDir || Path::acceptFile(fname, extra, &lang)) && !ignored.match(fname)) {
-                    std::string nativename = Path::fromNativeSeparators(fname);
+                if ((!checkAllFilesInDir || Path::acceptFile(fname, extra, &lang))) {
+                    if (!ignored.match(fname)) {
+                        std::string nativename = Path::fromNativeSeparators(fname);
 
-                    // Limitation: file sizes are assumed to fit in a 'size_t'
+                        // Limitation: file sizes are assumed to fit in a 'size_t'
 #ifdef _WIN64
-                    const std::size_t filesize = (static_cast<std::size_t>(ffd.nFileSizeHigh) << 32) | ffd.nFileSizeLow;
+                        const std::size_t filesize = (static_cast<std::size_t>(ffd.nFileSizeHigh) << 32) | ffd.nFileSizeLow;
 #else
-                    const std::size_t filesize = ffd.nFileSizeLow;
+                        const std::size_t filesize = ffd.nFileSizeLow;
 
 #endif
-                    files.emplace_back(std::move(nativename), lang, filesize);
+                        files.emplace_back(std::move(nativename), lang, filesize);
+                    }
+                    else if (debug)
+                    {
+                        std::cout << "ignored path: " << fname << std::endl;
+                    }
                 }
             } else {
                 // Directory
                 if (recursive) {
-                    if (!ignored.match(fname)) {
+                    // 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 + '/')) {
                         std::list<FileWithDetails> filesSorted;
 
                         std::string err = addFiles2(filesSorted, fname, extra, recursive, ignored);
@@ -135,6 +145,10 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
 
                         files.insert(files.end(), std::make_move_iterator(filesSorted.begin()), std::make_move_iterator(filesSorted.end()));
                     }
+                    else if (debug)
+                    {
+                        std::cout << "ignored path: " << fname << std::endl;
+                    }
                 }
             }
         }
@@ -151,14 +165,14 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
     return "";
 }
 
-std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored)
+std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored, bool debug)
 {
     if (path.empty())
         return "no path specified";
 
     std::list<FileWithDetails> filesSorted;
 
-    std::string err = addFiles2(filesSorted, path, extra, recursive, ignored);
+    std::string err = addFiles2(filesSorted, path, extra, recursive, ignored, debug);
 
     // files need to be sorted as the filesystems dosn't provide a stable order
     filesSorted.sort([](const FileWithDetails& a, const FileWithDetails& b) {
@@ -183,11 +197,15 @@ static std::string addFiles2(std::list<FileWithDetails> &files,
                              const std::string &path,
                              const std::set<std::string> &extra,
                              bool recursive,
-                             const PathMatch& ignored
-                             )
+                             const PathMatch& ignored,
+                             bool debug)
 {
     if (ignored.match(path))
+    {
+        if (debug)
+            std::cout << "ignored path: " << path << std::endl;
         return "";
+    }
 
     struct stat file_stat;
     if (stat(path.c_str(), &file_stat) == -1)
@@ -224,28 +242,43 @@ static std::string addFiles2(std::list<FileWithDetails> &files,
         const bool path_is_directory = Path::isDirectory(new_path);
 #endif
         if (path_is_directory) {
-            if (recursive && !ignored.match(new_path)) {
-                std::string err = addFiles2(files, new_path, extra, recursive, ignored);
-                if (!err.empty()) {
-                    return err;
+            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 + '/')) {
+                    std::string err = addFiles2(files, new_path, extra, recursive, ignored, debug);
+                    if (!err.empty()) {
+                        return err;
+                    }
+                }
+                else if (debug)
+                {
+                    std::cout << "ignored path: " << new_path << std::endl;
                 }
             }
         } else {
             Standards::Language lang = Standards::Language::None;
-            if (Path::acceptFile(new_path, extra, &lang) && !ignored.match(new_path)) {
-                if (stat(new_path.c_str(), &file_stat) == -1) {
-                    const int err = errno;
-                    return "could not stat file '" + new_path + "' (errno: " + std::to_string(err) + ")";
+            if (Path::acceptFile(new_path, extra, &lang)) {
+                if (!ignored.match(new_path))
+                {
+                    if (stat(new_path.c_str(), &file_stat) == -1) {
+                        const int err = errno;
+                        return "could not stat file '" + new_path + "' (errno: " + std::to_string(err) + ")";
+                    }
+                    files.emplace_back(new_path, lang, file_stat.st_size);
+                }
+                else if (debug)
+                {
+                    std::cout << "ignored path: " << new_path << std::endl;
                 }
-                files.emplace_back(new_path, lang, file_stat.st_size);
             }
         }
     }
 
     return "";
 }
 
-std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored)
+std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored, bool debug)
 {
     if (path.empty())
         return "no path specified";
@@ -256,7 +289,7 @@ std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::s
 
     std::list<FileWithDetails> filesSorted;
 
-    std::string err = addFiles2(filesSorted, corrected_path, extra, recursive, ignored);
+    std::string err = addFiles2(filesSorted, corrected_path, extra, recursive, ignored, debug);
 
     // files need to be sorted as the filesystems dosn't provide a stable order
     filesSorted.sort([](const FileWithDetails& a, const FileWithDetails& b) {
@@ -269,7 +302,7 @@ std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::s
 
 #endif
 
-std::string FileLister::recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored)
+std::string FileLister::recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored, bool debug)
 {
-    return addFiles(files, path, extra, true, ignored);
+    return addFiles(files, path, extra, true, ignored, debug);
 }

cli/filelister.h

@@ -41,9 +41,10 @@ class FileLister {
      * @param path root path
      * @param extra Extra file extensions
      * @param ignored ignored paths
+     * @param debug log if path was ignored
      * @return On success, an empty string is returned. On error, a error message is returned.
      */
-    static std::string recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored);
+    static std::string recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored, bool debug = false);
 
     /**
      * @brief (Recursively) add source files to a map.
@@ -55,9 +56,10 @@ class FileLister {
      * @param extra Extra file extensions
      * @param recursive Enable recursion
      * @param ignored ignored paths
+     * @param debug log when a path was ignored
      * @return On success, an empty string is returned. On error, a error message is returned.
      */
-    static std::string addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored);
+    static std::string addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored, bool debug = false);
 };
 
 /// @}

lib/importproject.cpp

@@ -42,7 +42,7 @@
 #include "json.h"
 
 // TODO: align the exclusion logic with PathMatch
-void ImportProject::ignorePaths(const std::vector<std::string> &ipaths)
+void ImportProject::ignorePaths(const std::vector<std::string> &ipaths, bool debug)
 {
     for (auto it = fileSettings.cbegin(); it != fileSettings.cend();) {
         bool ignore = false;
@@ -63,8 +63,11 @@ void ImportProject::ignorePaths(const std::vector<std::string> &ipaths)
                 }
             }
         }
-        if (ignore)
+        if (ignore) {
+            if (debug)
+                std::cout << "ignored path: " << it->filename() << std::endl;
             it = fileSettings.erase(it);
+        }
         else
             ++it;
     }

lib/importproject.h

@@ -93,7 +93,7 @@ class CPPCHECKLIB WARN_UNUSED ImportProject {
         std::string platform;
     } guiProject;
 
-    void ignorePaths(const std::vector<std::string> &ipaths);
+    void ignorePaths(const std::vector<std::string> &ipaths, bool debug = false);
     void ignoreOtherConfigs(const std::string &cfg);
 
     Type import(const std::string &filename, Settings *settings=nullptr, Suppressions *supprs=nullptr);

lib/pathmatch.h

@@ -35,6 +35,9 @@ class CPPCHECKLIB PathMatch {
 
     /**
      * The constructor.
+     *
+     * If a path is a directory it needs to end with a file separator.
+     *
      * @param paths List of masks.
      * @param caseSensitive Match the case of the characters when
      *   matching paths?
@@ -43,6 +46,9 @@ class CPPCHECKLIB PathMatch {
 
     /**
      * @brief Match path against list of masks.
+     *
+     * If you want to match a directory the given path needs to end with a path separator.
+     *
      * @param path Path to match.
      * @return true if any of the masks match the path, false otherwise.
      */

lib/settings.h

@@ -184,6 +184,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
     /** @brief Is --debug-clang-output given? */
     bool debugClangOutput{};
 
+    /** @brief Is --debug-ignore given? */
+    bool debugignore{};
+
     /** @brief Internal: Is --debug-lookup or --debug-lookup=all given? */
     bool debuglookup{};
 

test/cli/more-projects_test.py

@@ -705,7 +705,7 @@ def test_project_file_ignore_3(tmpdir):
     assert_cppcheck(args, ec_exp=1, err_exp=[], out_exp=out_lines)
 
 
-@pytest.mark.xfail
+@pytest.mark.xfail(strict=True)
 def test_json_file_ignore(tmpdir):
     test_file = os.path.join(tmpdir, 'test.cpp')
     with open(test_file, 'wt') as f: