From 6e6376c72fdb0fdb5ab386f27c94782c45cd8a99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Wed, 30 Sep 2026 21:33:13 +0200 Subject: [PATCH 1/2] Fix #15081 (usability: try to report misuse of exclude options) --- cli/cmdlineparser.cpp | 14 +++- cli/filelister.cpp | 10 +-- cli/filelister.h | 4 +- gui/filelist.cpp | 17 ++++- gui/filelist.h | 14 +++- gui/mainwindow.cpp | 17 ++++- gui/mainwindow.h | 3 +- gui/test/filelist/testfilelist.cpp | 37 ++++++++++ gui/test/filelist/testfilelist.h | 3 + lib/importproject.cpp | 39 ++++------ lib/importproject.h | 12 +-- lib/pathmatch.cpp | 30 +++++++- lib/pathmatch.h | 14 +++- releasenotes.txt | 4 +- test/helpers.cpp | 3 +- test/testcmdlineparser.cpp | 55 ++++++++++++++ test/testfilelister.cpp | 24 ++++-- test/testimportproject.cpp | 2 +- test/testpathmatch.cpp | 114 ++++++++++++++++++++++------- tools/dmake/dmake.cpp | 2 +- 20 files changed, 323 insertions(+), 95 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 842d597b0a4..60a7ab1101e 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -247,7 +247,7 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[]) std::list 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 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; } diff --git a/cli/filelister.cpp b/cli/filelister.cpp index 9b95689679c..c38da595457 100644 --- a/cli/filelister.cpp +++ b/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&files, const std::string &path, const std::set &extra, bool recursive, const PathMatch& ignored, bool debug = false) +static std::string addFiles2(std::list&files, const std::string &path, const std::set &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&files, const std::string return ""; } -std::string FileLister::addFiles(std::list &files, const std::string &path, const std::set &extra, bool recursive, const PathMatch& ignored, bool debug) +std::string FileLister::addFiles(std::list &files, const std::string &path, const std::set &extra, bool recursive, PathMatch& ignored, bool debug) { if (path.empty()) return "no path specified"; @@ -201,7 +201,7 @@ static std::string addFiles2(std::list &files, const std::string &path, const std::set &extra, bool recursive, - const PathMatch& ignored, + PathMatch& ignored, bool debug) { if (ignored.match(path)) @@ -280,7 +280,7 @@ static std::string addFiles2(std::list &files, return ""; } -std::string FileLister::addFiles(std::list &files, const std::string &path, const std::set &extra, bool recursive, const PathMatch& ignored, bool debug) +std::string FileLister::addFiles(std::list &files, const std::string &path, const std::set &extra, bool recursive, PathMatch& ignored, bool debug) { if (path.empty()) return "no path specified"; @@ -304,7 +304,7 @@ std::string FileLister::addFiles(std::list &files, const std::s #endif -std::string FileLister::recursiveAddFiles(std::list &files, const std::string &path, const std::set &extra, const PathMatch& ignored, bool debug) +std::string FileLister::recursiveAddFiles(std::list &files, const std::string &path, const std::set &extra, PathMatch& ignored, bool debug) { return addFiles(files, path, extra, true, ignored, debug); } diff --git a/cli/filelister.h b/cli/filelister.h index 12b8ddeed7e..e49ecb76179 100644 --- a/cli/filelister.h +++ b/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 &files, const std::string &path, const std::set &extra, const PathMatch& ignored, bool debug = false); + static std::string recursiveAddFiles(std::list &files, const std::string &path, const std::set &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 &files, const std::string &path, const std::set &extra, bool recursive, const PathMatch& ignored, bool debug = false); + static std::string addFiles(std::list &files, const std::string &path, const std::set &extra, bool recursive, PathMatch& ignored, bool debug = false); }; /// @} diff --git a/gui/filelist.cpp b/gui/filelist.cpp index cdfa6676a4e..1fa04313c18 100644 --- a/gui/filelist.cpp +++ b/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(); } +QStringList FileList::getUnmatchedExcludes() const +{ + return mUnmatchedExcludes; +} + void FileList::addExcludeList(const QStringList &paths) { mExcludedPaths = paths; @@ -117,9 +123,9 @@ static std::vector 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; } diff --git a/gui/filelist.h b/gui/filelist.h index 59f1284998d..030d4083ec4 100644 --- a/gui/filelist.h +++ b/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 + */ + QStringList getUnmatchedExcludes() const; /** * @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 diff --git a/gui/mainwindow.cpp b/gui/mainwindow.cpp index 3ed37eb7b25..71ebd12e26e 100644 --- a/gui/mainwindow.cpp +++ b/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 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() diff --git a/gui/mainwindow.h b/gui/mainwindow.h index c654a2c94f0..a2cfd56c3a0 100644 --- a/gui/mainwindow.h +++ b/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. diff --git a/gui/test/filelist/testfilelist.cpp b/gui/test/filelist/testfilelist.cpp index 003c43c1ba0..ecab8eb0b8c 100644 --- a/gui/test/filelist/testfilelist.cpp +++ b/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) diff --git a/gui/test/filelist/testfilelist.h b/gui/test/filelist/testfilelist.h index f8d9e3a5b7b..0520cef1aa9 100644 --- a/gui/test/filelist/testfilelist.h +++ b/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; }; diff --git a/lib/importproject.cpp b/lib/importproject.cpp index 61ef7a2d38a..799a5b688eb 100644 --- a/lib/importproject.cpp +++ b/lib/importproject.cpp @@ -191,7 +191,7 @@ void ImportProject::parseArgs(FileSettings &fs, const std::vector & fsSetDefines(fs, std::move(defs)); } -void ImportProject::ignorePaths(const std::vector &ipaths, bool debug) +std::vector ImportProject::ignorePaths(const std::vector &ipaths, bool debug) { PathMatch matcher(ipaths, Path::getCurrentPath()); for (auto it = fileSettings.cbegin(); it != fileSettings.cend();) { @@ -203,6 +203,7 @@ void ImportProject::ignorePaths(const std::vector &ipaths, bool deb else ++it; } + return matcher.unmatched(); } void ImportProject::ignoreOtherConfigs(const std::string &cfg) @@ -323,28 +324,25 @@ ImportProject::Type ImportProject::import(const std::string &filename, Settings if (!mPath.empty() && !endsWith(mPath,'/')) mPath += '/'; - const std::vector fileFilters = - settings ? settings->fileFilters : std::vector(); - if (endsWith(filename, ".json")) { if (importCompileCommands(fin)) { setRelativePaths(filename); return ImportProject::Type::COMPILE_DB; } } else if (endsWith(filename, ".sln")) { - if (importSln(fin, mPath, fileFilters)) { + if (importSln(fin, mPath)) { 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")) { std::map variables; std::vector sharedItemsProjects; - if (importVcxproj(filename, variables, "", fileFilters, sharedItemsProjects)) { + if (importVcxproj(filename, variables, "", sharedItemsProjects)) { setRelativePaths(filename); return ImportProject::Type::VS_VCXPROJ; } @@ -463,7 +461,7 @@ bool ImportProject::importCompileCommands(std::istream &istr) return true; } -bool ImportProject::importSln(std::istream &istr, const std::string &path, const std::vector &fileFilters) +bool ImportProject::importSln(std::istream &istr, const std::string &path) { std::string line; @@ -499,7 +497,7 @@ bool ImportProject::importSln(std::istream &istr, const std::string &path, const if (!Path::isAbsolute(vcxproj)) vcxproj = path + vcxproj; vcxproj = Path::fromNativeSeparators(std::move(vcxproj)); - if (!importVcxproj(vcxproj, variables, "", fileFilters, sharedItemsProjects)) { + if (!importVcxproj(vcxproj, variables, "", sharedItemsProjects)) { errors.emplace_back("failed to load '" + vcxproj + "' from Visual Studio solution"); return false; } @@ -514,7 +512,7 @@ bool ImportProject::importSln(std::istream &istr, const std::string &path, const return true; } -bool ImportProject::importSlnx(const std::string& filename, const std::vector& fileFilters) +bool ImportProject::importSlnx(const std::string& filename) { tinyxml2::XMLDocument doc; const tinyxml2::XMLError error = doc.LoadFile(filename.c_str()); @@ -555,7 +553,7 @@ bool ImportProject::importSlnx(const std::string& filename, const std::vector &variables, const std::string &additionalIncludeDirectories, - const std::vector &fileFilters, std::vector &cache) { tinyxml2::XMLDocument doc; @@ -947,10 +944,10 @@ bool ImportProject::importVcxproj(const std::string &filename, errors.emplace_back(std::string("Visual Studio project file is not a valid XML - ") + tinyxml2::XMLDocument::ErrorIDToName(error)); return false; } - return importVcxproj(filename, doc, variables, additionalIncludeDirectories, fileFilters, cache); + return importVcxproj(filename, doc, variables, additionalIncludeDirectories, cache); } -bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::XMLDocument &doc, std::map &variables, const std::string &additionalIncludeDirectories, const std::vector &fileFilters, std::vector &cache) +bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::XMLDocument &doc, std::map &variables, const std::string &additionalIncludeDirectories, std::vector &cache) { variables["ProjectDir"] = Path::simplifyPath(Path::getPathFromFilename(filename)); @@ -1029,7 +1026,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X return false; } - SharedItemsProject toAdd = importVcxitems(pathToSharedItemsFile, fileFilters, cache); + SharedItemsProject toAdd = importVcxitems(pathToSharedItemsFile, cache); if (!toAdd.successful) { errors.emplace_back("Could not load shared items project \"" + pathToSharedItemsFile + "\" from original path \"" + std::string(projectAttribute) + "\"."); return false; @@ -1058,11 +1055,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X } // Project files - PathMatch filtermatcher(fileFilters, Path::getCurrentPath()); for (const ItemGroupClCompile& compile : compileList) { - if (!fileFilters.empty() && !filtermatcher.match(compile.mFilename)) - continue; - for (const ProjectConfiguration &p : projectConfigurationList) { if (!guiProject.checkVsConfigs.empty()) { @@ -1129,7 +1122,7 @@ bool ImportProject::importVcxproj(const std::string &filename, const tinyxml2::X return true; } -ImportProject::SharedItemsProject ImportProject::importVcxitems(const std::string& filename, const std::vector& fileFilters, std::vector &cache) +ImportProject::SharedItemsProject ImportProject::importVcxitems(const std::string& filename, std::vector &cache) { auto isInCacheCheck = [filename](const ImportProject::SharedItemsProject& e) -> bool { return filename == e.pathToProjectFile; @@ -1142,8 +1135,6 @@ ImportProject::SharedItemsProject ImportProject::importVcxitems(const std::strin SharedItemsProject result; result.pathToProjectFile = filename; - PathMatch filtermatcher(fileFilters, Path::getCurrentPath()); - tinyxml2::XMLDocument doc; const tinyxml2::XMLError error = doc.LoadFile(filename.c_str()); if (error != tinyxml2::XML_SUCCESS) { @@ -1164,10 +1155,6 @@ ImportProject::SharedItemsProject ImportProject::importVcxitems(const std::strin std::string file(include); findAndReplace(file, "$(MSBuildThisFileDirectory)", "./"); - // Skip file if it doesn't match the filter - if (!fileFilters.empty() && !filtermatcher.match(file)) - continue; - result.sourceFiles.emplace_back(file); } else { errors.emplace_back("Could not find shared items source file"); diff --git a/lib/importproject.h b/lib/importproject.h index b8bbbed3fa3..c7e4f5c4f3b 100644 --- a/lib/importproject.h +++ b/lib/importproject.h @@ -102,7 +102,7 @@ class CPPCHECKLIB WARN_UNUSED ImportProject { std::string platform; } guiProject; - void ignorePaths(const std::vector &ipaths, bool debug = false); + std::vector ignorePaths(const std::vector &ipaths, bool debug = false); void ignoreOtherConfigs(const std::string &cfg); Type import(const std::string &filename, Settings *settings=nullptr, Suppressions *supprs=nullptr); @@ -119,15 +119,15 @@ class CPPCHECKLIB WARN_UNUSED ImportProject { std::vector sourceFiles; }; - bool importVcxproj(const std::string &filename, std::map &variables, const std::string &additionalIncludeDirectories, const std::vector &fileFilters, std::vector &cache); - bool importVcxproj(const std::string &filename, const tinyxml2::XMLDocument &doc, std::map &variables, const std::string &additionalIncludeDirectories, const std::vector &fileFilters, std::vector &cache); + bool importVcxproj(const std::string &filename, std::map &variables, const std::string &additionalIncludeDirectories, std::vector &cache); + bool importVcxproj(const std::string &filename, const tinyxml2::XMLDocument &doc, std::map &variables, const std::string &additionalIncludeDirectories, std::vector &cache); private: static void parseArgs(FileSettings &fs, const std::vector &args); - bool importSln(std::istream &istr, const std::string &path, const std::vector &fileFilters); - bool importSlnx(const std::string& filename, const std::vector& fileFilters); - SharedItemsProject importVcxitems(const std::string &filename, const std::vector &fileFilters, std::vector &cache); + bool importSln(std::istream &istr, const std::string &path); + bool importSlnx(const std::string& filename); + SharedItemsProject importVcxitems(const std::string &filename, std::vector &cache); bool importBcb6Prj(const std::string &projectFilename); std::string mPath; diff --git a/lib/pathmatch.cpp b/lib/pathmatch.cpp index 87420828f17..992ce4ee634 100644 --- a/lib/pathmatch.cpp +++ b/lib/pathmatch.cpp @@ -21,6 +21,7 @@ #include "path.h" #include +#include #include #include #include @@ -31,11 +32,17 @@ PathMatch::PathMatch(std::vector patterns, std::string basepath, Sy mPatterns(std::move(patterns)), mBasepath(std::move(basepath)), mSyntax(syntax) {} -bool PathMatch::match(const std::string &path, Filemode mode) const +bool PathMatch::match(const std::string &path, Filemode mode) { - return std::any_of(mPatterns.cbegin(), mPatterns.cend(), [&] (const std::string &pattern) { - return match(pattern, path, mBasepath, mode, mSyntax); - }); + // check all patterns so every matching pattern is recorded + bool ret = false; + for (const std::string &pattern : mPatterns) { + if (match(pattern, path, mBasepath, mode, mSyntax)) { + mMatchedPatterns.insert(pattern); + ret = true; + } + } + return ret; } bool PathMatch::match(const std::string &pattern, const std::string &path, const std::string &basepath, Filemode mode, Syntax syntax) @@ -149,3 +156,18 @@ bool PathMatch::match(const std::string &pattern, const std::string &path, const return false; } } + +std::vector PathMatch::unmatched() const { + std::vector ret; + std::copy_if(mPatterns.cbegin(), mPatterns.cend(), std::back_inserter(ret), [this](const std::string& s) { + if (mMatchedPatterns.count(s) != 0) + return false; + // paths inside a matched directory are not traversed, so a pattern that is + // covered by a matched pattern is considered used + const Filemode mode = !s.empty() && PathIterator::issep(s.back(), mSyntax) ? Filemode::directory : Filemode::regular; + return std::none_of(mMatchedPatterns.cbegin(), mMatchedPatterns.cend(), [&](const std::string& matched) { + return match(matched, s, mBasepath, mode, mSyntax); + }); + }); + return ret; +} diff --git a/lib/pathmatch.h b/lib/pathmatch.h index 72fc89b7faa..5832a8fb192 100644 --- a/lib/pathmatch.h +++ b/lib/pathmatch.h @@ -24,6 +24,7 @@ #include #include #include +#include #include #include @@ -117,7 +118,7 @@ class CPPCHECKLIB PathMatch { * @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, Filemode mode = Filemode::regular) const; + bool match(const std::string &path, Filemode mode = Filemode::regular); /** * @brief Match path against a single pattern. @@ -169,12 +170,23 @@ class CPPCHECKLIB PathMatch { return pattern; } + /** + * @brief Get the patterns that have not matched any path. + * + * A pattern that has not matched itself is still considered used if it is covered by + * a pattern that has matched, since paths inside an ignored directory are not traversed. + * + * @return The patterns that have not matched any path passed to match(). + */ + std::vector unmatched() const; + protected: class PathIterator; private: /* List of patterns */ std::vector mPatterns; + std::set mMatchedPatterns; /* Base path to with patterns and paths are relative */ std::string mBasepath; /* The syntax to use */ diff --git a/releasenotes.txt b/releasenotes.txt index 68b03d2b083..eb3e773fcfa 100644 --- a/releasenotes.txt +++ b/releasenotes.txt @@ -11,7 +11,7 @@ C/C++ support: - GUI: -- +- An error is now reported when an exclude path in the project does not match any file. To hide warnings in certain files, use suppressions instead. Suppressions: - @@ -23,4 +23,4 @@ Infrastructure & dependencies: - Other: -- +- An error is now reported when an ignore path given with -i (or an exclude path from a .cppcheck project) does not match any file. To hide warnings in certain files, use suppressions instead. diff --git a/test/helpers.cpp b/test/helpers.cpp index 06ad0c4c298..7c26feae6c1 100644 --- a/test/helpers.cpp +++ b/test/helpers.cpp @@ -84,7 +84,8 @@ ScopedFile::~ScopedFile() { // TODO: simplify the function call // hack to be able to delete *.plist output files std::list files; - const std::string res = FileLister::addFiles(files, mPath, {".plist"}, false, PathMatch()); + PathMatch ignored; + const std::string res = FileLister::addFiles(files, mPath, {".plist"}, false, ignored); if (!res.empty()) { std::cout << "ScopedFile(" << mPath + ") - generating file list failed (" << res << ")" << std::endl; } diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index 6f963e07212..56fee32b0f6 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -519,6 +519,11 @@ class TestCmdlineParser : public TestFixture { TEST_CASE(ignorefilepaths7); TEST_CASE(ignorefilepaths8); TEST_CASE(ignorefilepaths9); + TEST_CASE(ignoreUnused1); + TEST_CASE(ignoreUnused2); + TEST_CASE(ignoreUsed); + TEST_CASE(ignoreUnusedProject); + TEST_CASE(ignoreUsedProject); TEST_CASE(nonexistentpath); @@ -3649,6 +3654,56 @@ class TestCmdlineParser : public TestFixture { ASSERT_EQUALS("cppcheck: error: could not find or open any of the paths given.\ncppcheck: Maybe all paths were ignored?\n", logger->str()); } + void ignoreUnused1() { + REDIRECT; + ScopedFile file("file.cpp", ""); + const char * const argv[] = {"cppcheck", "-ifoo.cpp", "file.cpp"}; + ASSERT(!fillSettingsFromArgs(argv)); + ASSERT_EQUALS("cppcheck: error: unused ignore/exclude path 'foo.cpp'. To hide warnings in certain files use suppressions instead.\n", logger->str()); + } + + void ignoreUnused2() { + REDIRECT; + ScopedFile file1("file1.cpp", ""); + ScopedFile file2("file2.cpp", ""); + const char * const argv[] = {"cppcheck", "-ifile1.cpp", "-ifoo.cpp", "file1.cpp", "file2.cpp"}; + ASSERT(!fillSettingsFromArgs(argv)); + ASSERT_EQUALS("cppcheck: error: unused ignore/exclude path 'foo.cpp'. To hide warnings in certain files use suppressions instead.\n", logger->str()); + } + + void ignoreUsed() { + REDIRECT; + ScopedFile file1("file1.cpp", ""); + ScopedFile file2("file2.cpp", ""); + const char * const argv[] = {"cppcheck", "-ifile1.cpp", "file1.cpp", "file2.cpp"}; + ASSERT(fillSettingsFromArgs(argv)); + ASSERT_EQUALS("", logger->str()); + ASSERT_EQUALS(1, parser->getFiles().size()); + ASSERT_EQUALS("file2.cpp", parser->getFiles().cbegin()->path()); + } + + void ignoreUnusedProject() { + REDIRECT; + ScopedFile file("compile_commands.json", + R"([{"directory": "/tmp", "command": "gcc -c file1.c", "file": "file1.c"}, + {"directory": "/tmp", "command": "gcc -c file2.c", "file": "file2.c"}])"); + const char * const argv[] = {"cppcheck", "--project=compile_commands.json", "-ifile1.c", "-ifoo.c"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Fail, parseFromArgs(argv)); + ASSERT_EQUALS("cppcheck: error: unused ignore/exclude path 'foo.c'. To hide warnings in certain files use suppressions instead.\n", logger->str()); + } + + void ignoreUsedProject() { + REDIRECT; + ScopedFile file("compile_commands.json", + R"([{"directory": "/tmp", "command": "gcc -c file1.c", "file": "file1.c"}, + {"directory": "/tmp", "command": "gcc -c file2.c", "file": "file2.c"}])"); + const char * const argv[] = {"cppcheck", "--project=compile_commands.json", "-ifile1.c"}; + ASSERT_EQUALS_ENUM(CmdLineParser::Result::Success, parseFromArgs(argv)); + ASSERT_EQUALS("", logger->str()); + ASSERT_EQUALS(1, parser->getFileSettings().size()); + ASSERT_EQUALS("/tmp/file2.c", parser->getFileSettings().cbegin()->filename()); + } + void nonexistentpath() { REDIRECT; const char * const argv[] = {"cppcheck", "file.cpp"}; diff --git a/test/testfilelister.cpp b/test/testfilelister.cpp index 13057735513..e1c55564743 100644 --- a/test/testfilelister.cpp +++ b/test/testfilelister.cpp @@ -60,7 +60,8 @@ class TestFileLister : public TestFixture { // Recursively add add files.. std::list files; - std::string err = FileLister::recursiveAddFiles(files, adddir, {}, PathMatch()); + PathMatch ignored; + std::string err = FileLister::recursiveAddFiles(files, adddir, {}, ignored); ASSERT_EQUALS("", err); ASSERT(!files.empty()); @@ -106,7 +107,8 @@ class TestFileLister : public TestFixture { void recursiveAddFilesEmptyPath() const { std::list files; - const std::string err = FileLister::recursiveAddFiles(files, "", {}, PathMatch()); + PathMatch ignored; + const std::string err = FileLister::recursiveAddFiles(files, "", {}, ignored); ASSERT_EQUALS("no path specified", err); } @@ -124,7 +126,8 @@ class TestFileLister : public TestFixture { const std::string basedir = findBaseDir(); std::list files; - std::string err = FileLister::recursiveAddFiles(files, basedir + "lib/token.cpp", {}, PathMatch()); + PathMatch ignored; + std::string err = FileLister::recursiveAddFiles(files, basedir + "lib/token.cpp", {}, ignored); ASSERT_EQUALS("", err); ASSERT_EQUALS(1, files.size()); ASSERT_EQUALS(basedir + "lib/token.cpp", files.begin()->path()); @@ -157,27 +160,32 @@ class TestFileLister : public TestFixture { { const std::string addfile = Path::join(Path::join(adddir, "cli"), "main.cpp"); - const std::string err = FileLister::addFiles(files, addfile, {}, true,PathMatch()); + PathMatch ignored; + const std::string err = FileLister::addFiles(files, addfile, {}, true, ignored); ASSERT_EQUALS("", err); } { const std::string addfile = Path::join(Path::join(adddir, "lib"), "token.cpp"); - const std::string err = FileLister::addFiles(files, addfile, {}, true,PathMatch()); + PathMatch ignored; + const std::string err = FileLister::addFiles(files, addfile, {}, true, ignored); ASSERT_EQUALS("", err); } { const std::string addfile = Path::join(Path::join(adddir, "cli"), "token.cpp"); // does not exist - const std::string err = FileLister::addFiles(files, addfile, {}, true,PathMatch()); + PathMatch ignored; + const std::string err = FileLister::addFiles(files, addfile, {}, true, ignored); ASSERT_EQUALS("", err); } { const std::string addfile = Path::join(Path::join(adddir, "lib2"), "token.cpp"); // does not exist - const std::string err = FileLister::addFiles(files, addfile, {}, true,PathMatch()); + PathMatch ignored; + const std::string err = FileLister::addFiles(files, addfile, {}, true, ignored); ASSERT_EQUALS("", err); } { const std::string addfile = Path::join(Path::join(adddir, "lib"), "matchcompiler.h"); - const std::string err = FileLister::addFiles(files, addfile, {}, true,PathMatch()); + PathMatch ignored; + const std::string err = FileLister::addFiles(files, addfile, {}, true, ignored); ASSERT_EQUALS("", err); } diff --git a/test/testimportproject.cpp b/test/testimportproject.cpp index 873272030f6..b7e74121ce2 100644 --- a/test/testimportproject.cpp +++ b/test/testimportproject.cpp @@ -637,7 +637,7 @@ class TestImportProject : public TestFixture { TestImporter project; std::map variables; std::vector cache; - ASSERT_EQUALS(project.importVcxproj("test.vcxproj", doc, variables, {}, {}, cache), true); + ASSERT_EQUALS(project.importVcxproj("test.vcxproj", doc, variables, {}, cache), true); ASSERT_EQUALS(project.fileSettings.size(), 2); ASSERT(project.fileSettings.front().defines.find(";UNICODE=1;") != std::string::npos); ASSERT(project.fileSettings.front().defines.find(";_UNICODE=1") != std::string::npos); diff --git a/test/testpathmatch.cpp b/test/testpathmatch.cpp index 7e721502a11..b98414d55e2 100644 --- a/test/testpathmatch.cpp +++ b/test/testpathmatch.cpp @@ -42,10 +42,10 @@ class TestPathMatch : public TestFixture { #else const std::string basepath{"/test"}; #endif - const PathMatch emptyMatcher{{}, basepath}; - const PathMatch srcMatcher{{"src/"}, basepath}; - const PathMatch fooCppMatcher{{"foo.cpp"}, basepath}; - const PathMatch srcFooCppMatcher{{"src/foo.cpp"}, basepath}; + PathMatch emptyMatcher{{}, basepath}; + PathMatch srcMatcher{{"src/"}, basepath}; + PathMatch fooCppMatcher{{"foo.cpp"}, basepath}; + PathMatch srcFooCppMatcher{{"src/foo.cpp"}, basepath}; void run() override { TEST_CASE(emptymaskemptyfile); @@ -85,32 +85,38 @@ class TestPathMatch : public TestFixture { TEST_CASE(globstar1); TEST_CASE(globstar2); TEST_CASE(pathiterator); + TEST_CASE(unmatched1); + TEST_CASE(unmatched2); + TEST_CASE(unmatched3); + TEST_CASE(unmatched4); + TEST_CASE(unmatched5); + TEST_CASE(unmatched6); } // Test empty PathMatch - void emptymaskemptyfile() const { + void emptymaskemptyfile() { ASSERT(!emptyMatcher.match("")); } - void emptymaskpath1() const { + void emptymaskpath1() { ASSERT(!emptyMatcher.match("src/", ifdir)); } - void emptymaskpath2() const { + void emptymaskpath2() { ASSERT(!emptyMatcher.match("../src/", ifdir)); } - void emptymaskpath3() const { + void emptymaskpath3() { ASSERT(!emptyMatcher.match("/home/user/code/src/", ifdir)); ASSERT(!emptyMatcher.match("d:/home/user/code/src/", ifdir)); } // Test PathMatch containing "src/" - void onemaskemptypath() const { + void onemaskemptypath() { ASSERT(!srcMatcher.match("")); } - void onemasksamepath() const { + void onemasksamepath() { ASSERT(srcMatcher.match("src/", ifdir)); ASSERT(!srcMatcher.match("src/", ifreg)); } @@ -127,7 +133,7 @@ class TestPathMatch : public TestFixture { ASSERT(!match.match("srC/", ifreg)); } - void onemasksamepathwithfile() const { + void onemasksamepathwithfile() { ASSERT(srcMatcher.match("src/file.txt")); } @@ -140,36 +146,36 @@ class TestPathMatch : public TestFixture { ASSERT(!match.match(shorterToMatch, ifdir)); } - void onemaskdifferentdir1() const { + void onemaskdifferentdir1() { ASSERT(!srcMatcher.match("srcfiles/file.txt")); } - void onemaskdifferentdir2() const { + void onemaskdifferentdir2() { ASSERT(!srcMatcher.match("proj/srcfiles/file.txt")); } - void onemaskdifferentdir3() const { + void onemaskdifferentdir3() { ASSERT(!srcMatcher.match("proj/mysrc/file.txt")); } - void onemaskdifferentdir4() const { + void onemaskdifferentdir4() { ASSERT(!srcMatcher.match("proj/mysrcfiles/file.txt")); } - void onemasklongerpath1() const { + void onemasklongerpath1() { ASSERT(srcMatcher.match("/tmp/src/", ifdir)); ASSERT(srcMatcher.match("d:/tmp/src/", ifdir)); } - void onemasklongerpath2() const { + void onemasklongerpath2() { ASSERT(srcMatcher.match("src/module/", ifdir)); } - void onemasklongerpath3() const { + void onemasklongerpath3() { ASSERT(srcMatcher.match("project/src/module/", ifdir)); } - void onemaskcwd() const { + void onemaskcwd() { ASSERT(srcMatcher.match("./src", ifdir)); } @@ -197,7 +203,7 @@ class TestPathMatch : public TestFixture { } // Test PathMatch containing "foo.cpp" - void filemask1() const { + void filemask1() { ASSERT(fooCppMatcher.match("foo.cpp")); } @@ -206,32 +212,32 @@ class TestPathMatch : public TestFixture { ASSERT(match.match("fOo.cpp")); } - void filemask2() const { + void filemask2() { ASSERT(fooCppMatcher.match("../foo.cpp")); } - void filemask3() const { + void filemask3() { ASSERT(fooCppMatcher.match("src/foo.cpp")); } - void filemaskcwd() const { + void filemaskcwd() { ASSERT(fooCppMatcher.match("./lib/foo.cpp")); } // Test PathMatch containing "src/foo.cpp" - void filemaskpath1() const { + void filemaskpath1() { ASSERT(srcFooCppMatcher.match("src/foo.cpp")); } - void filemaskpath2() const { + void filemaskpath2() { ASSERT(srcFooCppMatcher.match("proj/src/foo.cpp")); } - void filemaskpath3() const { + void filemaskpath3() { ASSERT(!srcFooCppMatcher.match("foo.cpp")); } - void filemaskpath4() const { + void filemaskpath4() { ASSERT(!srcFooCppMatcher.match("bar/foo.cpp")); } @@ -308,6 +314,60 @@ class TestPathMatch : public TestFixture { /* The server and share should actually be considered part of the root and not be removed */ ASSERT_EQUALS("//", PathIterator("\\\\Server\\Share\\Directory", "../..\\../..", windows).read()); } + + void unmatched1() const { + // nothing has been matched yet => all patterns are unmatched + PathMatch match({"src/", "foo.cpp"}); + ASSERT_EQUALS(2, match.unmatched().size()); + ASSERT_EQUALS("src/", match.unmatched()[0]); + ASSERT_EQUALS("foo.cpp", match.unmatched()[1]); + } + + void unmatched2() const { + PathMatch match({"src/", "foo.cpp"}); + ASSERT(match.match("src/bar.cpp")); + ASSERT_EQUALS(1, match.unmatched().size()); + ASSERT_EQUALS("foo.cpp", match.unmatched()[0]); + ASSERT(match.match("lib/foo.cpp")); + ASSERT_EQUALS(0, match.unmatched().size()); + } + + void unmatched3() const { + // failed matches does not affect the unmatched patterns + PathMatch match({"src/", "foo.cpp"}); + ASSERT(!match.match("lib/bar.cpp")); + ASSERT(!match.match("src/", ifreg)); + ASSERT_EQUALS(2, match.unmatched().size()); + ASSERT(match.match("src/", ifdir)); + ASSERT_EQUALS(1, match.unmatched().size()); + ASSERT_EQUALS("foo.cpp", match.unmatched()[0]); + } + + void unmatched4() const { + // glob patterns + PathMatch match({"test*.cpp", "src/**/foo.c"}); + ASSERT(match.match("src/lib/foo/foo.c")); + ASSERT_EQUALS(1, match.unmatched().size()); + ASSERT_EQUALS("test*.cpp", match.unmatched()[0]); + ASSERT(!match.match("src/foo.c")); + ASSERT(match.match("test1.cpp")); + ASSERT_EQUALS(0, match.unmatched().size()); + } + + void unmatched5() const { + // all matching patterns are recorded + PathMatch match({"src/", "src/foo.cpp"}); + ASSERT(match.match("src/foo.cpp")); + ASSERT_EQUALS(0, match.unmatched().size()); + } + + void unmatched6() const { + // a pattern inside a matched directory is considered used + PathMatch match({"src/", "src/foo.cpp", "src/lib/", "lib/foo.cpp"}); + ASSERT(match.match("src/", ifdir)); + ASSERT_EQUALS(1, match.unmatched().size()); + ASSERT_EQUALS("lib/foo.cpp", match.unmatched()[0]); + } }; REGISTER_TEST(TestPathMatch) diff --git a/tools/dmake/dmake.cpp b/tools/dmake/dmake.cpp index eaf0a727646..f8d5f9d7a17 100644 --- a/tools/dmake/dmake.cpp +++ b/tools/dmake/dmake.cpp @@ -171,7 +171,7 @@ static std::string getCppFiles(std::vector &files, const std::strin std::list filelist; const std::set extra; const std::vector masks; - const PathMatch matcher(masks, Path::getCurrentPath()); + PathMatch matcher(masks, Path::getCurrentPath()); std::string err = FileLister::addFiles(filelist, path, extra, recursive, matcher); if (!err.empty()) return err; From 713e0fa6dcf2b4c9c358c6c7c9e05956530421fe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Marjam=C3=A4ki?= Date: Fri, 2 Oct 2026 15:59:01 +0200 Subject: [PATCH 2/2] cppcheck warning --- gui/filelist.cpp | 2 +- gui/filelist.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gui/filelist.cpp b/gui/filelist.cpp index 1fa04313c18..c282c6aea3e 100644 --- a/gui/filelist.cpp +++ b/gui/filelist.cpp @@ -104,7 +104,7 @@ QStringList FileList::getFileList() return applyExcludeList(); } -QStringList FileList::getUnmatchedExcludes() const +const QStringList& FileList::getUnmatchedExcludes() & { return mUnmatchedExcludes; } diff --git a/gui/filelist.h b/gui/filelist.h index 030d4083ec4..0c1ee53d481 100644 --- a/gui/filelist.h +++ b/gui/filelist.h @@ -68,7 +68,7 @@ class FileList { * @brief Return list of exclude paths that did not match any file in the last getFileList() call * @return list of unmatched excludes */ - QStringList getUnmatchedExcludes() const; + const QStringList& getUnmatchedExcludes() &; /** * @brief Add list of paths to exclusion list.