Fix #15081 (usability: try to report misuse of exclude options)

cli/cmdlineparser.cpp

@@ -247,7 +247,7 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
         std::list<FileWithDetails> filesResolved;
         // Execute recursiveAddFiles() to each given file parameter
         // TODO: verbose log which files were ignored?
-        const PathMatch matcher(ignored, Path::getCurrentPath());
+        PathMatch matcher(ignored, Path::getCurrentPath());
         for (const std::string &pathname : pathnamesRef) {
             const std::string err = FileLister::recursiveAddFiles(filesResolved, Path::toNativeSeparators(pathname), mSettings.library.markupExtensions(), matcher, mSettings.debugignore);
             if (!err.empty()) {
@@ -264,6 +264,12 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
             return false;
         }
 
+        const auto& unmatched = matcher.unmatched();
+        if (!unmatched.empty()) {
+            mLogger.printError("unused ignore/exclude path '" + unmatched.front() + "'. To hide warnings in certain files use suppressions instead.");
+            return false;
+        }
+
         std::list<FileWithDetails> files;
         if (!mSettings.fileFilters.empty()) {
             files = filterFiles(mSettings.fileFilters, filesResolved);
@@ -1723,12 +1729,16 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
         mPathNames = project.guiProject.pathNames;
 
     if (!project.fileSettings.empty()) {
-        project.ignorePaths(mIgnoredPaths, mSettings.debugignore);
+        const auto& unmatched = 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?
             return Result::Fail;
         }
+        if (!unmatched.empty()) {
+            mLogger.printError("unused ignore/exclude path '" + unmatched.front() + "'. To hide warnings in certain files use suppressions instead.");
+            return Result::Fail;
+        }
         mFileSettings = project.fileSettings;
     }
 

cli/filelister.cpp

@@ -46,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, bool debug = false)
+static std::string addFiles2(std::list<FileWithDetails>&files, const std::string &path, const std::set<std::string> &extra, bool recursive, PathMatch& ignored, bool debug = false)
 {
     const std::string cleanedPath = Path::toNativeSeparators(path);
 
@@ -163,7 +163,7 @@ 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, bool debug)
+std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, PathMatch& ignored, bool debug)
 {
     if (path.empty())
         return "no path specified";
@@ -205,7 +205,7 @@ static std::string addFiles2(std::list<FileWithDetails> &files,
                              const std::string &path,
                              const std::set<std::string> &extra,
                              bool recursive,
-                             const PathMatch& ignored,
+                             PathMatch& ignored,
                              bool debug)
 {
     if (ignored.match(path))
@@ -284,7 +284,7 @@ static std::string addFiles2(std::list<FileWithDetails> &files,
     return "";
 }
 
-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)
+std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, PathMatch& ignored, bool debug)
 {
     if (path.empty())
         return "no path specified";
@@ -312,7 +312,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, bool debug)
+std::string FileLister::recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, PathMatch& ignored, bool debug)
 {
     return addFiles(files, path, extra, true, ignored, debug);
 }

cli/filelister.h

@@ -44,7 +44,7 @@ class FileLister {
      * @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, bool debug = false);
+    static std::string recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, PathMatch& ignored, bool debug = false);
 
     /**
      * @brief (Recursively) add source files to a map.
@@ -59,7 +59,7 @@ class FileLister {
      * @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, bool debug = false);
+    static std::string addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, PathMatch& ignored, bool debug = false);
 };
 
 /// @}

gui/filelist.cpp

@@ -90,8 +90,9 @@ void FileList::addPathList(const QStringList &paths)
     }
 }
 
-QStringList FileList::getFileList() const
+QStringList FileList::getFileList()
 {
+    mUnmatchedExcludes.clear();
     if (mExcludedPaths.empty()) {
         QStringList names;
         for (const QFileInfo& item : mFileList) {
@@ -103,6 +104,11 @@ QStringList FileList::getFileList() const
     return applyExcludeList();
 }
 
+const QStringList& FileList::getUnmatchedExcludes() &
+{
+    return mUnmatchedExcludes;
+}
+
 void FileList::addExcludeList(const QStringList &paths)
 {
     mExcludedPaths = paths;
@@ -117,9 +123,9 @@ static std::vector<std::string> toStdStringList(const QStringList &stringList)
     return ret;
 }
 
-QStringList FileList::applyExcludeList() const
+QStringList FileList::applyExcludeList()
 {
-    const PathMatch pathMatch(toStdStringList(mExcludedPaths), QDir::currentPath().toStdString());
+    PathMatch pathMatch(toStdStringList(mExcludedPaths), QDir::currentPath().toStdString());
 
     QStringList paths;
     for (const QFileInfo& item : mFileList) {
@@ -129,5 +135,10 @@ QStringList FileList::applyExcludeList() const
         if (!pathMatch.match(canonical.toStdString()))
             paths << canonical;
     }
+
+    for (const std::string& excludePath: pathMatch.unmatched()) {
+        mUnmatchedExcludes << QString::fromStdString(excludePath);
+    }
+
     return paths;
 }

gui/filelist.h

@@ -62,7 +62,13 @@ class FileList {
      * @brief Return list of filenames (to check).
      * @return list of filenames to check.
      */
-    QStringList getFileList() const;
+    QStringList getFileList();
+
+    /**
+     * @brief Return list of exclude paths that did not match any file in the last getFileList() call
+     * @return list of unmatched excludes
+     */
+    const QStringList& getUnmatchedExcludes() &;
 
     /**
      * @brief Add list of paths to exclusion list.
@@ -88,14 +94,16 @@ class FileList {
      * @brief Get filtered list of paths.
      * This method takes the list of paths and applies the exclude lists to
      * it. And then returns the list of paths that did not match the
-     * exclude filters.
+     * exclude filters. The exclude paths that did not match any file
+     * are stored in mUnmatchedExcludes.
      * @return Filtered list of paths.
      */
-    QStringList applyExcludeList() const;
+    QStringList applyExcludeList();
 
 private:
     QFileInfoList mFileList;
     QStringList mExcludedPaths;
+    QStringList mUnmatchedExcludes;
 };
 
 #endif // FILELIST_H

gui/mainwindow.cpp

@@ -647,7 +647,7 @@ void MainWindow::doAnalyzeProject(ImportProject p, const bool checkLib, const bo
     mUI->mResults->setCheckSettings(checkSettings);
 }
 
-void MainWindow::doAnalyzeFiles(const QStringList &files, const bool checkLib, const bool checkConfig)
+void MainWindow::doAnalyzeFiles(const QStringList &files, const bool checkLib, const bool checkConfig, const bool checkUnusedExcludes)
 {
     if (files.isEmpty())
         return;
@@ -683,6 +683,17 @@ void MainWindow::doAnalyzeFiles(const QStringList &files, const bool checkLib, c
         return;
     }
 
+    const QStringList unmatchedExcludePaths = checkUnusedExcludes ? pathList.getUnmatchedExcludes() : QStringList();
+    if (!unmatchedExcludePaths.isEmpty()) {
+        QMessageBox msg(QMessageBox::Warning,
+                        "Cppcheck",
+                        tr("Unused exclude paths:\n%1\nto hide warnings in certain files use suppressions instead").arg(unmatchedExcludePaths.join("\n")),
+                        QMessageBox::Ok,
+                        this);
+        msg.exec();
+        return;
+    }
+
     std::list<FileWithDetails> fdetails = enrichFilesForAnalysis(fileNames, checkSettings);
 
     // TODO: lock UI here?
@@ -1981,7 +1992,9 @@ void MainWindow::analyzeProject(const ProjectFile *projectFile, const QStringLis
     if (paths.isEmpty()) {
         paths << mCurrentDirectory;
     }
-    doAnalyzeFiles(paths, checkLib, checkConfig);
+    // the exclude paths can only be validated when the whole project is analyzed
+    const bool checkUnusedExcludes = recheckFiles.isEmpty();
+    doAnalyzeFiles(paths, checkLib, checkConfig, checkUnusedExcludes);
 }
 
 void MainWindow::newProjectFile()

gui/mainwindow.h

@@ -317,8 +317,9 @@ private slots:
      * @param files List of files and/or directories to analyze
      * @param checkLib Flag to indicate if library should be checked
      * @param checkConfig Flag to indicate if the configuration should be checked.
+     * @param checkUnusedExcludes Flag to indicate if unused exclude paths should be reported.
      */
-    void doAnalyzeFiles(const QStringList &files, bool checkLib = false, bool checkConfig = false);
+    void doAnalyzeFiles(const QStringList &files, bool checkLib = false, bool checkConfig = false, bool checkUnusedExcludes = false);
 
     /**
      * @brief Get our default cppcheck settings and read project file.

gui/test/filelist/testfilelist.cpp

@@ -184,4 +184,41 @@ void TestFileList::filterFiles5() const
     QVERIFY(!files.contains(base + "/dir1/dir11/foo11.cpp"));
 }
 
+void TestFileList::unmatchedExcludes1() const
+{
+    FileList list;
+    QStringList filters;
+    filters << "foo1.cpp" << "foo3.cc";
+    list.addExcludeList(filters);
+    list.addDirectory(QString(SRCDIR) + "/../data/files");
+    QVERIFY(!list.getFileList().isEmpty());
+    QVERIFY(list.getUnmatchedExcludes().isEmpty());
+}
+
+void TestFileList::unmatchedExcludes2() const
+{
+    FileList list;
+    QStringList filters;
+    filters << "foo1.cpp" << "bar.cpp" << "dir3/";
+    list.addExcludeList(filters);
+    list.addDirectory(QString(SRCDIR) + "/../data/files", true);
+    // unmatched excludes does not affect the file list
+    QCOMPARE(list.getFileList().size(), 9);
+    const QStringList unmatched = list.getUnmatchedExcludes();
+    QCOMPARE(unmatched.size(), 2);
+    QCOMPARE(unmatched[0], QString("bar.cpp"));
+    QCOMPARE(unmatched[1], QString("dir3/"));
+}
+
+void TestFileList::unmatchedExcludes3() const
+{
+    FileList list;
+    QStringList filters;
+    filters << "dir1/";
+    list.addExcludeList(filters);
+    list.addDirectory(QString(SRCDIR) + "/../data/files", true);
+    QVERIFY(!list.getFileList().isEmpty());
+    QVERIFY(list.getUnmatchedExcludes().isEmpty());
+}
+
 QTEST_MAIN(TestFileList)

gui/test/filelist/testfilelist.h

@@ -33,4 +33,7 @@ private slots:
     void filterFiles3() const;
     void filterFiles4() const;
     void filterFiles5() const;
+    void unmatchedExcludes1() const;
+    void unmatchedExcludes2() const;
+    void unmatchedExcludes3() const;
 };

lib/importproject.cpp

@@ -235,7 +235,7 @@ void ImportProject::parseArgs(FileSettings &fs, const std::vector<std::string> &
     fsSetDefines(fs, std::move(defs));
 }
 
-void ImportProject::ignorePaths(const std::vector<std::string> &ipaths, bool debug)
+std::vector<std::string> ImportProject::ignorePaths(const std::vector<std::string> &ipaths, bool debug)
 {
     PathMatch matcher(ipaths, Path::getCurrentPath());
     for (auto it = fileSettings.cbegin(); it != fileSettings.cend();) {
@@ -247,6 +247,7 @@ void ImportProject::ignorePaths(const std::vector<std::string> &ipaths, bool deb
         else
             ++it;
     }
+    return matcher.unmatched();
 }
 
 void ImportProject::ignoreOtherConfigs(const std::string &cfg)
@@ -1759,27 +1760,24 @@ ImportProject::Type ImportProject::import(const std::string &filename, Settings
     if (!mPath.empty() && !endsWith(mPath,'/'))
         mPath += '/';
 
-    const std::vector<std::string> fileFilters =
-        settings ? settings->fileFilters : std::vector<std::string>();
-
     if (endsWith(filename, ".json")) {
         if (processCompileCommands(fin)) {
             setRelativePaths(filename);
             return ImportProject::Type::COMPILE_DB;
         }
     } else if (endsWith(filename, ".sln")) {
-        if (importSln(fin, filename, fileFilters)) {
+        if (importSln(fin, filename)) {
             setRelativePaths(filename);
             return ImportProject::Type::VS_SLN;
         }
     } else if (endsWith(filename, ".slnx")) {
-        if (importSlnx(filename, fileFilters)) {
+        if (importSlnx(filename)) {
             setRelativePaths(filename);
             return ImportProject::Type::VS_SLNX;
         }
     } else if (endsWith(filename, ".vcxproj")) {
         PropertiesMap mVariables;
-        if (importVcxproj(toAbsolute(filename), mVariables, fileFilters)) {
+        if (importVcxproj(toAbsolute(filename), mVariables)) {
             setRelativePaths(filename);
             return ImportProject::Type::VS_VCXPROJ;
         }
@@ -1907,7 +1905,7 @@ void ImportProject::setSolution(const std::string &filename, PropertiesMap &prop
     properties["SolutionName"] = fileStem(properties["SolutionFileName"]);
 }
 
-bool ImportProject::importSln(std::istream &istr, const std::string &filename, const std::vector<std::string> &fileFilters)
+bool ImportProject::importSln(std::istream &istr, const std::string &filename)
 {
     std::string line;
 
@@ -2000,7 +1998,7 @@ bool ImportProject::importSln(std::istream &istr, const std::string &filename, c
 
     for (const std::string &vcxproj : vcxprojs) {
         PropertiesMap mVariables = solutionVariables;
-        if (!importVcxproj(vcxproj, mVariables, fileFilters)) {
+        if (!importVcxproj(vcxproj, mVariables)) {
             errors.emplace_back("failed to load '" + vcxproj + "' from Visual Studio solution");
             return false;
         }
@@ -2009,7 +2007,7 @@ bool ImportProject::importSln(std::istream &istr, const std::string &filename, c
     return true;
 }
 
-bool ImportProject::importSlnx(const std::string& filename, const std::vector<std::string>& fileFilters)
+bool ImportProject::importSlnx(const std::string& filename)
 {
     debugs.clear();
 
@@ -2054,7 +2052,7 @@ bool ImportProject::importSlnx(const std::string& filename, const std::vector<st
         vcxproj = Path::fromNativeSeparators(std::move(vcxproj));
 
         PropertiesMap mVariables = solutionVariables;
-        if (!importVcxproj(vcxproj, mVariables, fileFilters)) {
+        if (!importVcxproj(vcxproj, mVariables)) {
             errors.emplace_back("failed to load '" + vcxproj + "' from Visual Studio solution");
             return false;
         }
@@ -4583,8 +4581,7 @@ ImportProject::ImportResult ImportProject::processImport(const std::string &file
 }
 
 bool ImportProject::importVcxproj(const std::string &filename,
-                                  PropertiesMap &properties,
-                                  const std::vector<std::string> &fileFilters)
+                                  PropertiesMap &properties)
 {
     tinyxml2::XMLDocument doc;
     const tinyxml2::XMLError error = doc.LoadFile(filename.c_str());
@@ -4908,11 +4905,7 @@ bool ImportProject::importVcxproj(const std::string &filename,
         // we can only set it globally but in this context it needs to be treated per file
 
         // Project files
-        PathMatch filtermatcher(fileFilters, Path::getCurrentPath());
         for (const ItemGroupClCompile &compile : compileList) {
-            if (!fileFilters.empty() && !filtermatcher.match(compile.filename))
-                continue;
-
             const std::string &excl = compile.get("ExcludedFromBuild");
             if (!excl.empty() && caseInsensitiveStringCompare(excl, "true") == 0)
                 continue;