Skip to content

Commit 1d1093d

Browse files
firewavefirewave
authored
fixed #13641 - improved ignoring directories and overall testing of ignore (#7222)
1 parent 042da30 commit 1d1093d

13 files changed

+291
-110
lines changed

‎cli/cmdlineparser.cpp

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ bool CmdLineParser::fillSettingsFromArgs(int argc, const char* const argv[])
276276
// TODO: verbose log which files were ignored?
277277
const PathMatch matcher(ignored, caseSensitive);
278278
for (const std::string &pathname : pathnamesRef) {
279-
const std::string err = FileLister::recursiveAddFiles(filesResolved, Path::toNativeSeparators(pathname), mSettings.library.markupExtensions(), matcher);
279+
const std::string err = FileLister::recursiveAddFiles(filesResolved, Path::toNativeSeparators(pathname), mSettings.library.markupExtensions(), matcher, mSettings.debugignore);
280280
if (!err.empty()) {
281281
// TODO: bail out?
282282
mLogger.printMessage(err);
@@ -619,6 +619,10 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
619619
else if (std::strcmp(argv[i], "--debug-clang-output") == 0)
620620
mSettings.debugClangOutput = true;
621621

622+
// Show debug messages for ignored files
623+
else if (std::strcmp(argv[i], "--debug-ignore") == 0)
624+
mSettings.debugignore = true;
625+
622626
// Show --debug output after the first simplifications
623627
else if (std::strcmp(argv[i], "--debug") == 0 ||
624628
std::strcmp(argv[i], "--debug-normal") == 0)
@@ -821,15 +825,6 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
821825
}
822826

823827
if (!path.empty()) {
824-
path = Path::removeQuotationMarks(std::move(path));
825-
path = Path::simplifyPath(std::move(path));
826-
827-
// TODO: this only works when it exists
828-
if (Path::isDirectory(path)) {
829-
// If directory name doesn't end with / or \, add it
830-
if (!endsWith(path, '/'))
831-
path += '/';
832-
}
833828
mIgnoredPaths.emplace_back(std::move(path));
834829
}
835830
}
@@ -1590,11 +1585,27 @@ CmdLineParser::Result CmdLineParser::parseFromArgs(int argc, const char* const a
15901585
return Result::Fail;
15911586
}
15921587

1588+
for (auto& path : mIgnoredPaths)
1589+
{
1590+
path = Path::removeQuotationMarks(std::move(path));
1591+
path = Path::simplifyPath(std::move(path));
1592+
1593+
bool isdir = false;
1594+
if (!Path::exists(path, &isdir) && mSettings.debugignore)
1595+
std::cout << "path to ignore does not exist: " << path << std::endl;
1596+
// TODO: this only works when it exists
1597+
if (isdir) {
1598+
// If directory name doesn't end with / or \, add it
1599+
if (!endsWith(path, '/'))
1600+
path += '/';
1601+
}
1602+
}
1603+
15931604
if (!project.guiProject.pathNames.empty())
15941605
mPathNames = project.guiProject.pathNames;
15951606

15961607
if (!project.fileSettings.empty()) {
1597-
project.ignorePaths(mIgnoredPaths);
1608+
project.ignorePaths(mIgnoredPaths, mSettings.debugignore);
15981609
if (project.fileSettings.empty()) {
15991610
mLogger.printError("no C or C++ source files found.");
16001611
mLogger.printMessage("all paths were ignored"); // TODO: log this differently?

‎cli/filelister.cpp

Lines changed: 60 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
#include "utils.h"
3030

3131
#include <cstring>
32+
#include <iostream>
3233
#include <iterator>
3334
#include <memory>
3435

@@ -45,7 +46,7 @@
4546
// When compiling Unicode targets WinAPI automatically uses *W Unicode versions
4647
// of called functions. Thus, we explicitly call *A versions of the functions.
4748

48-
static std::string addFiles2(std::list<FileWithDetails>&files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored)
49+
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)
4950
{
5051
const std::string cleanedPath = Path::toNativeSeparators(path);
5152

@@ -85,8 +86,9 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
8586
HANDLE hFind = FindFirstFileA(searchPattern.c_str(), &ffd);
8687
if (INVALID_HANDLE_VALUE == hFind) {
8788
const DWORD err = GetLastError();
88-
if (err == ERROR_FILE_NOT_FOUND) {
89-
// no files matched
89+
if (err == ERROR_FILE_NOT_FOUND || // the pattern did not match anything
90+
err == ERROR_PATH_NOT_FOUND) // the given search path does not exist
91+
{
9092
return "";
9193
}
9294
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
106108
if ((ffd.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) == 0) {
107109
// File
108110
Standards::Language lang = Standards::Language::None;
109-
if ((!checkAllFilesInDir || Path::acceptFile(fname, extra, &lang)) && !ignored.match(fname)) {
110-
std::string nativename = Path::fromNativeSeparators(fname);
111+
if ((!checkAllFilesInDir || Path::acceptFile(fname, extra, &lang))) {
112+
if (!ignored.match(fname)) {
113+
std::string nativename = Path::fromNativeSeparators(fname);
111114

112-
// Limitation: file sizes are assumed to fit in a 'size_t'
115+
// Limitation: file sizes are assumed to fit in a 'size_t'
113116
#ifdef _WIN64
114-
const std::size_t filesize = (static_cast<std::size_t>(ffd.nFileSizeHigh) << 32) | ffd.nFileSizeLow;
117+
const std::size_t filesize = (static_cast<std::size_t>(ffd.nFileSizeHigh) << 32) | ffd.nFileSizeLow;
115118
#else
116-
const std::size_t filesize = ffd.nFileSizeLow;
119+
const std::size_t filesize = ffd.nFileSizeLow;
117120

118121
#endif
119-
files.emplace_back(std::move(nativename), lang, filesize);
122+
files.emplace_back(std::move(nativename), lang, filesize);
123+
}
124+
else if (debug)
125+
{
126+
std::cout << "ignored path: " << fname << std::endl;
127+
}
120128
}
121129
} else {
122130
// Directory
123131
if (recursive) {
124-
if (!ignored.match(fname)) {
132+
// append a slash if it is a directory since that is what we are doing for mIgnoredPaths directory entries.
133+
// otherwise we would ignore all its contents individually instead as a whole.
134+
if (!ignored.match(fname + '/')) {
125135
std::list<FileWithDetails> filesSorted;
126136

127137
std::string err = addFiles2(filesSorted, fname, extra, recursive, ignored);
@@ -135,6 +145,10 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
135145

136146
files.insert(files.end(), std::make_move_iterator(filesSorted.begin()), std::make_move_iterator(filesSorted.end()));
137147
}
148+
else if (debug)
149+
{
150+
std::cout << "ignored path: " << fname << std::endl;
151+
}
138152
}
139153
}
140154
}
@@ -151,14 +165,14 @@ static std::string addFiles2(std::list<FileWithDetails>&files, const std::string
151165
return "";
152166
}
153167

154-
std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored)
168+
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)
155169
{
156170
if (path.empty())
157171
return "no path specified";
158172

159173
std::list<FileWithDetails> filesSorted;
160174

161-
std::string err = addFiles2(filesSorted, path, extra, recursive, ignored);
175+
std::string err = addFiles2(filesSorted, path, extra, recursive, ignored, debug);
162176

163177
// files need to be sorted as the filesystems dosn't provide a stable order
164178
filesSorted.sort([](const FileWithDetails& a, const FileWithDetails& b) {
@@ -183,11 +197,15 @@ static std::string addFiles2(std::list<FileWithDetails> &files,
183197
const std::string &path,
184198
const std::set<std::string> &extra,
185199
bool recursive,
186-
const PathMatch& ignored
187-
)
200+
const PathMatch& ignored,
201+
bool debug)
188202
{
189203
if (ignored.match(path))
204+
{
205+
if (debug)
206+
std::cout << "ignored path: " << path << std::endl;
190207
return "";
208+
}
191209

192210
struct stat file_stat;
193211
if (stat(path.c_str(), &file_stat) == -1)
@@ -224,28 +242,43 @@ static std::string addFiles2(std::list<FileWithDetails> &files,
224242
const bool path_is_directory = Path::isDirectory(new_path);
225243
#endif
226244
if (path_is_directory) {
227-
if (recursive && !ignored.match(new_path)) {
228-
std::string err = addFiles2(files, new_path, extra, recursive, ignored);
229-
if (!err.empty()) {
230-
return err;
245+
if (recursive) {
246+
// append a slash if it is a directory since that is what we are doing for mIgnoredPaths directory entries.
247+
// otherwise we would ignore all its contents individually instead as a whole.
248+
if (!ignored.match(new_path + '/')) {
249+
std::string err = addFiles2(files, new_path, extra, recursive, ignored, debug);
250+
if (!err.empty()) {
251+
return err;
252+
}
253+
}
254+
else if (debug)
255+
{
256+
std::cout << "ignored path: " << new_path << std::endl;
231257
}
232258
}
233259
} else {
234260
Standards::Language lang = Standards::Language::None;
235-
if (Path::acceptFile(new_path, extra, &lang) && !ignored.match(new_path)) {
236-
if (stat(new_path.c_str(), &file_stat) == -1) {
237-
const int err = errno;
238-
return "could not stat file '" + new_path + "' (errno: " + std::to_string(err) + ")";
261+
if (Path::acceptFile(new_path, extra, &lang)) {
262+
if (!ignored.match(new_path))
263+
{
264+
if (stat(new_path.c_str(), &file_stat) == -1) {
265+
const int err = errno;
266+
return "could not stat file '" + new_path + "' (errno: " + std::to_string(err) + ")";
267+
}
268+
files.emplace_back(new_path, lang, file_stat.st_size);
269+
}
270+
else if (debug)
271+
{
272+
std::cout << "ignored path: " << new_path << std::endl;
239273
}
240-
files.emplace_back(new_path, lang, file_stat.st_size);
241274
}
242275
}
243276
}
244277

245278
return "";
246279
}
247280

248-
std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored)
281+
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)
249282
{
250283
if (path.empty())
251284
return "no path specified";
@@ -256,7 +289,7 @@ std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::s
256289

257290
std::list<FileWithDetails> filesSorted;
258291

259-
std::string err = addFiles2(filesSorted, corrected_path, extra, recursive, ignored);
292+
std::string err = addFiles2(filesSorted, corrected_path, extra, recursive, ignored, debug);
260293

261294
// files need to be sorted as the filesystems dosn't provide a stable order
262295
filesSorted.sort([](const FileWithDetails& a, const FileWithDetails& b) {
@@ -269,7 +302,7 @@ std::string FileLister::addFiles(std::list<FileWithDetails> &files, const std::s
269302

270303
#endif
271304

272-
std::string FileLister::recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored)
305+
std::string FileLister::recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored, bool debug)
273306
{
274-
return addFiles(files, path, extra, true, ignored);
307+
return addFiles(files, path, extra, true, ignored, debug);
275308
}

‎cli/filelister.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,10 @@ class FileLister {
4141
* @param path root path
4242
* @param extra Extra file extensions
4343
* @param ignored ignored paths
44+
* @param debug log if path was ignored
4445
* @return On success, an empty string is returned. On error, a error message is returned.
4546
*/
46-
static std::string recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored);
47+
static std::string recursiveAddFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, const PathMatch& ignored, bool debug = false);
4748

4849
/**
4950
* @brief (Recursively) add source files to a map.
@@ -55,9 +56,10 @@ class FileLister {
5556
* @param extra Extra file extensions
5657
* @param recursive Enable recursion
5758
* @param ignored ignored paths
59+
* @param debug log when a path was ignored
5860
* @return On success, an empty string is returned. On error, a error message is returned.
5961
*/
60-
static std::string addFiles(std::list<FileWithDetails> &files, const std::string &path, const std::set<std::string> &extra, bool recursive, const PathMatch& ignored);
62+
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);
6163
};
6264

6365
/// @}

‎lib/importproject.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@
4242
#include "json.h"
4343

4444
// TODO: align the exclusion logic with PathMatch
45-
void ImportProject::ignorePaths(const std::vector<std::string> &ipaths)
45+
void ImportProject::ignorePaths(const std::vector<std::string> &ipaths, bool debug)
4646
{
4747
for (auto it = fileSettings.cbegin(); it != fileSettings.cend();) {
4848
bool ignore = false;
@@ -63,8 +63,11 @@ void ImportProject::ignorePaths(const std::vector<std::string> &ipaths)
6363
}
6464
}
6565
}
66-
if (ignore)
66+
if (ignore) {
67+
if (debug)
68+
std::cout << "ignored path: " << it->filename() << std::endl;
6769
it = fileSettings.erase(it);
70+
}
6871
else
6972
++it;
7073
}

‎lib/importproject.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ class CPPCHECKLIB WARN_UNUSED ImportProject {
9393
std::string platform;
9494
} guiProject;
9595

96-
void ignorePaths(const std::vector<std::string> &ipaths);
96+
void ignorePaths(const std::vector<std::string> &ipaths, bool debug = false);
9797
void ignoreOtherConfigs(const std::string &cfg);
9898

9999
Type import(const std::string &filename, Settings *settings=nullptr, Suppressions *supprs=nullptr);

‎lib/pathmatch.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ class CPPCHECKLIB PathMatch {
3535

3636
/**
3737
* The constructor.
38+
*
39+
* If a path is a directory it needs to end with a file separator.
40+
*
3841
* @param paths List of masks.
3942
* @param caseSensitive Match the case of the characters when
4043
* matching paths?
@@ -43,6 +46,9 @@ class CPPCHECKLIB PathMatch {
4346

4447
/**
4548
* @brief Match path against list of masks.
49+
*
50+
* If you want to match a directory the given path needs to end with a path separator.
51+
*
4652
* @param path Path to match.
4753
* @return true if any of the masks match the path, false otherwise.
4854
*/

‎lib/settings.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,9 @@ class CPPCHECKLIB WARN_UNUSED Settings {
184184
/** @brief Is --debug-clang-output given? */
185185
bool debugClangOutput{};
186186

187+
/** @brief Is --debug-ignore given? */
188+
bool debugignore{};
189+
187190
/** @brief Internal: Is --debug-lookup or --debug-lookup=all given? */
188191
bool debuglookup{};
189192

‎test/cli/more-projects_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -705,7 +705,7 @@ def test_project_file_ignore_3(tmpdir):
705705
assert_cppcheck(args, ec_exp=1, err_exp=[], out_exp=out_lines)
706706

707707

708-
@pytest.mark.xfail
708+
@pytest.mark.xfail(strict=True)
709709
def test_json_file_ignore(tmpdir):
710710
test_file = os.path.join(tmpdir, 'test.cpp')
711711
with open(test_file, 'wt') as f:

0 commit comments

Comments
 (0)