Skip to content

Commit 704b862

Browse files
firewavefirewave
authored
cleaned up the file filtering code and improved testing of it (danmar#5622)
1 parent de2cfb0 commit 704b862

File tree

6 files changed

+240
-75
lines changed

6 files changed

+240
-75
lines changed

‎cli/cppcheckexecutor.cpp‎

Lines changed: 30 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -189,12 +189,9 @@ bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* c
189189
const std::vector<std::string>& pathnames = parser.getPathNames();
190190
const std::list<FileSettings>& fileSettings = parser.getFileSettings();
191191

192-
#if defined(_WIN32)
193-
// For Windows we want case-insensitive path matching
194-
const bool caseSensitive = false;
195-
#else
196-
const bool caseSensitive = true;
197-
#endif
192+
// the inputs can only be used exclusively - CmdLineParser should already handle this
193+
assert(!(!pathnames.empty() && !fileSettings.empty()));
194+
198195
if (!fileSettings.empty()) {
199196
if (!settings.fileFilters.empty()) {
200197
// filter only for the selected filenames from all project files
@@ -209,37 +206,48 @@ bool CppCheckExecutor::parseFromArgs(Settings &settings, int argc, const char* c
209206
else {
210207
mFileSettings = fileSettings;
211208
}
212-
} else if (!pathnames.empty()) {
209+
}
210+
211+
if (!pathnames.empty()) {
212+
// TODO: this should be a vector or list so the order is kept
213+
std::map<std::string, std::size_t> files;
214+
// TODO: this needs to be inlined into PathMatch as it depends on the underlying filesystem
215+
#if defined(_WIN32)
216+
// For Windows we want case-insensitive path matching
217+
const bool caseSensitive = false;
218+
#else
219+
const bool caseSensitive = true;
220+
#endif
213221
// Execute recursiveAddFiles() to each given file parameter
214222
const PathMatch matcher(ignored, caseSensitive);
215223
for (const std::string &pathname : pathnames) {
216-
std::string err = FileLister::recursiveAddFiles(mFiles, Path::toNativeSeparators(pathname), settings.library.markupExtensions(), matcher);
224+
const std::string err = FileLister::recursiveAddFiles(files, Path::toNativeSeparators(pathname), settings.library.markupExtensions(), matcher);
217225
if (!err.empty()) {
218226
// TODO: bail out?
219227
logger.printMessage(err);
220228
}
221229
}
230+
231+
if (!settings.fileFilters.empty()) {
232+
std::copy_if(files.cbegin(), files.cend(), std::inserter(mFiles, mFiles.end()), [&](const decltype(files)::value_type& entry) {
233+
return matchglobs(settings.fileFilters, entry.first);
234+
});
235+
if (mFiles.empty()) {
236+
logger.printError("could not find any files matching the filter.");
237+
return false;
238+
}
239+
}
240+
else {
241+
mFiles = files;
242+
}
222243
}
223244

224-
if (mFiles.empty() && fileSettings.empty()) {
245+
if (mFiles.empty() && mFileSettings.empty()) {
225246
logger.printError("could not find or open any of the paths given.");
226247
if (!ignored.empty())
227248
logger.printMessage("Maybe all paths were ignored?");
228249
return false;
229250
}
230-
if (!settings.fileFilters.empty() && fileSettings.empty()) {
231-
std::map<std::string, std::size_t> newMap;
232-
for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i)
233-
if (matchglobs(settings.fileFilters, i->first)) {
234-
newMap[i->first] = i->second;
235-
}
236-
mFiles = newMap;
237-
if (mFiles.empty()) {
238-
logger.printError("could not find any files matching the filter.");
239-
return false;
240-
}
241-
242-
}
243251

244252
return true;
245253
}

‎cli/executor.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,14 +24,18 @@
2424
#include "suppressions.h"
2525

2626
#include <algorithm>
27+
#include <cassert>
2728
#include <sstream> // IWYU pragma: keep
2829
#include <utility>
2930

3031
struct FileSettings;
3132

3233
Executor::Executor(const std::map<std::string, std::size_t> &files, const std::list<FileSettings>& fileSettings, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger)
3334
: mFiles(files), mFileSettings(fileSettings), mSettings(settings), mSuppressions(suppressions), mErrorLogger(errorLogger)
34-
{}
35+
{
36+
// the two inputs may only be used exclusively
37+
assert(!(!files.empty() && !fileSettings.empty()));
38+
}
3539

3640
bool Executor::hasToLog(const ErrorMessage &msg)
3741
{

‎cli/singleexecutor.cpp‎

Lines changed: 39 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -49,65 +49,56 @@ unsigned int SingleExecutor::check()
4949

5050
std::size_t processedsize = 0;
5151
unsigned int c = 0;
52-
// TODO: processes either mSettings.project.fileSettings or mFiles - process/thread implementations process both
53-
// TODO: thread/process implementations process fileSettings first
54-
if (mFileSettings.empty()) {
55-
for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
56-
if (!mSettings.library.markupFile(i->first)
57-
|| !mSettings.library.processMarkupAfterCode(i->first)) {
58-
result += mCppcheck.check(i->first);
59-
processedsize += i->second;
60-
if (!mSettings.quiet)
61-
reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize);
62-
// TODO: call analyseClangTidy()?
63-
c++;
64-
}
52+
53+
for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
54+
if (!mSettings.library.markupFile(i->first) || !mSettings.library.processMarkupAfterCode(i->first)) {
55+
result += mCppcheck.check(i->first);
56+
processedsize += i->second;
57+
++c;
58+
if (!mSettings.quiet)
59+
reportStatus(c, mFiles.size(), processedsize, totalfilesize);
60+
// TODO: call analyseClangTidy()?
6561
}
66-
} else {
67-
// filesettings
68-
// check all files of the project
69-
for (const FileSettings &fs : mFileSettings) {
70-
if (!mSettings.library.markupFile(fs.filename)
71-
|| !mSettings.library.processMarkupAfterCode(fs.filename)) {
72-
result += mCppcheck.check(fs);
73-
++c;
74-
if (!mSettings.quiet)
75-
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
76-
if (mSettings.clangTidy)
77-
mCppcheck.analyseClangTidy(fs);
78-
}
62+
}
63+
64+
// filesettings
65+
// check all files of the project
66+
for (const FileSettings &fs : mFileSettings) {
67+
if (!mSettings.library.markupFile(fs.filename) || !mSettings.library.processMarkupAfterCode(fs.filename)) {
68+
result += mCppcheck.check(fs);
69+
++c;
70+
if (!mSettings.quiet)
71+
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
72+
if (mSettings.clangTidy)
73+
mCppcheck.analyseClangTidy(fs);
7974
}
8075
}
8176

8277
// second loop to parse all markup files which may not work until all
8378
// c/cpp files have been parsed and checked
8479
// TODO: get rid of duplicated code
85-
if (mFileSettings.empty()) {
86-
for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
87-
if (mSettings.library.markupFile(i->first)
88-
&& mSettings.library.processMarkupAfterCode(i->first)) {
89-
result += mCppcheck.check(i->first);
90-
processedsize += i->second;
91-
if (!mSettings.quiet)
92-
reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize);
93-
// TODO: call analyseClangTidy()?
94-
c++;
95-
}
80+
for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
81+
if (mSettings.library.markupFile(i->first) && mSettings.library.processMarkupAfterCode(i->first)) {
82+
result += mCppcheck.check(i->first);
83+
processedsize += i->second;
84+
++c;
85+
if (!mSettings.quiet)
86+
reportStatus(c, mFiles.size(), processedsize, totalfilesize);
87+
// TODO: call analyseClangTidy()?
9688
}
9789
}
98-
else {
99-
for (const FileSettings &fs : mFileSettings) {
100-
if (mSettings.library.markupFile(fs.filename)
101-
&& mSettings.library.processMarkupAfterCode(fs.filename)) {
102-
result += mCppcheck.check(fs);
103-
++c;
104-
if (!mSettings.quiet)
105-
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
106-
if (mSettings.clangTidy)
107-
mCppcheck.analyseClangTidy(fs);
108-
}
90+
91+
for (const FileSettings &fs : mFileSettings) {
92+
if (mSettings.library.markupFile(fs.filename) && mSettings.library.processMarkupAfterCode(fs.filename)) {
93+
result += mCppcheck.check(fs);
94+
++c;
95+
if (!mSettings.quiet)
96+
reportStatus(c, mFileSettings.size(), c, mFileSettings.size());
97+
if (mSettings.clangTidy)
98+
mCppcheck.analyseClangTidy(fs);
10999
}
110100
}
101+
111102
if (mCppcheck.analyseWholeProgram())
112103
result++;
113104

‎test/cli/test-more-projects.py‎

Lines changed: 97 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import json
33
import os
44
import pytest
5-
from testutils import cppcheck
5+
from testutils import cppcheck, assert_cppcheck
66

77

88
def test_project_force_U(tmpdir):
@@ -291,3 +291,99 @@ def test_clang_tidy(tmpdir):
291291
'Checking {} ...'.format(test_file)
292292
]
293293
assert stderr == ''
294+
295+
296+
def test_project_file_filter(tmpdir):
297+
test_file = os.path.join(tmpdir, 'test.cpp')
298+
with open(test_file, 'wt') as f:
299+
pass
300+
301+
project_file = os.path.join(tmpdir, 'test.cppcheck')
302+
with open(project_file, 'wt') as f:
303+
f.write(
304+
"""<?xml version="1.0" encoding="UTF-8"?>
305+
<project>
306+
<paths>
307+
<dir name="{}"/>
308+
</paths>
309+
</project>""".format(test_file))
310+
311+
args = ['--file-filter=*.cpp', '--project={}'.format(project_file)]
312+
out_lines = [
313+
'Checking {} ...'.format(test_file)
314+
]
315+
316+
assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines)
317+
318+
319+
def test_project_file_filter_2(tmpdir):
320+
test_file_1 = os.path.join(tmpdir, 'test.cpp')
321+
with open(test_file_1, 'wt') as f:
322+
pass
323+
test_file_2 = os.path.join(tmpdir, 'test.c')
324+
with open(test_file_2, 'wt') as f:
325+
pass
326+
327+
project_file = os.path.join(tmpdir, 'test.cppcheck')
328+
with open(project_file, 'wt') as f:
329+
f.write(
330+
"""<?xml version="1.0" encoding="UTF-8"?>
331+
<project>
332+
<paths>
333+
<dir name="{}"/>
334+
<dir name="{}"/>
335+
</paths>
336+
</project>""".format(test_file_1, test_file_2))
337+
338+
args = ['--file-filter=*.cpp', '--project={}'.format(project_file)]
339+
out_lines = [
340+
'Checking {} ...'.format(test_file_1)
341+
]
342+
343+
assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines)
344+
345+
346+
def test_project_file_filter_3(tmpdir):
347+
test_file_1 = os.path.join(tmpdir, 'test.cpp')
348+
with open(test_file_1, 'wt') as f:
349+
pass
350+
test_file_2 = os.path.join(tmpdir, 'test.c')
351+
with open(test_file_2, 'wt') as f:
352+
pass
353+
354+
project_file = os.path.join(tmpdir, 'test.cppcheck')
355+
with open(project_file, 'wt') as f:
356+
f.write(
357+
"""<?xml version="1.0" encoding="UTF-8"?>
358+
<project>
359+
<paths>
360+
<dir name="{}"/>
361+
<dir name="{}"/>
362+
</paths>
363+
</project>""".format(test_file_1, test_file_2))
364+
365+
args = ['--file-filter=*.c', '--project={}'.format(project_file)]
366+
out_lines = [
367+
'Checking {} ...'.format(test_file_2)
368+
]
369+
370+
assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines)
371+
372+
373+
def test_project_file_filter_no_match(tmpdir):
374+
project_file = os.path.join(tmpdir, 'test.cppcheck')
375+
with open(project_file, 'wt') as f:
376+
f.write(
377+
"""<?xml version="1.0" encoding="UTF-8"?>
378+
<project>
379+
<paths>
380+
<dir name="test.cpp"/>
381+
</paths>
382+
</project>""")
383+
384+
args = ['--file-filter=*.c', '--project={}'.format(project_file)]
385+
out_lines = [
386+
'cppcheck: error: could not find any files matching the filter.'
387+
]
388+
389+
assert_cppcheck(args, ec_exp=1, err_exp=[], out_exp=out_lines)

‎test/cli/test-other.py‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import os
55
import pytest
66

7-
from testutils import cppcheck
7+
from testutils import cppcheck, assert_cppcheck
88

99

1010
def __test_missing_include(tmpdir, use_j):
@@ -578,3 +578,57 @@ def test_missing_addon(tmpdir):
578578
'Did not find addon misra3.py'
579579
]
580580
assert stderr == ""
581+
582+
583+
def test_file_filter(tmpdir):
584+
test_file = os.path.join(tmpdir, 'test.cpp')
585+
with open(test_file, 'wt') as f:
586+
pass
587+
588+
args = ['--file-filter=*.cpp', test_file]
589+
out_lines = [
590+
'Checking {} ...'.format(test_file)
591+
]
592+
593+
assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines)
594+
595+
596+
def test_file_filter_2(tmpdir):
597+
test_file_1 = os.path.join(tmpdir, 'test.cpp')
598+
with open(test_file_1, 'wt') as f:
599+
pass
600+
test_file_2 = os.path.join(tmpdir, 'test.c')
601+
with open(test_file_2, 'wt') as f:
602+
pass
603+
604+
args = ['--file-filter=*.cpp', test_file_1, test_file_2]
605+
out_lines = [
606+
'Checking {} ...'.format(test_file_1)
607+
]
608+
609+
assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines)
610+
611+
612+
def test_file_filter_3(tmpdir):
613+
test_file_1 = os.path.join(tmpdir, 'test.cpp')
614+
with open(test_file_1, 'wt') as f:
615+
pass
616+
test_file_2 = os.path.join(tmpdir, 'test.c')
617+
with open(test_file_2, 'wt') as f:
618+
pass
619+
620+
args = ['--file-filter=*.c', test_file_1, test_file_2]
621+
out_lines = [
622+
'Checking {} ...'.format(test_file_2)
623+
]
624+
625+
assert_cppcheck(args, ec_exp=0, err_exp=[], out_exp=out_lines)
626+
627+
628+
def test_file_filter_no_match(tmpdir):
629+
args = ['--file-filter=*.c', 'test.cpp']
630+
out_lines = [
631+
'cppcheck: error: could not find any files matching the filter.'
632+
]
633+
634+
assert_cppcheck(args, ec_exp=1, err_exp=[], out_exp=out_lines)

0 commit comments

Comments
 (0)