Skip to content

Commit b5ce2c7

Browse files
firewavefirewave
authored
SingleExecutor: process markup files after code when scanning projects (danmar#4972)
* SingleExecutor: added TODOs * test `SingleExecutor` with files and project * SingleExecutor: process markup files after code when scanning project * TestSingleExecutor: generate scoped files before calling executor * CI-unixish.yml: added `--output-on-failure` to CTest call * helpers.cpp: improved error reporting in `~ScopedFile()` * use unique filenames in executor tests to avoid collisions * fixed `functionStatic` selfcheck warnings
1 parent f04d47a commit b5ce2c7

File tree

8 files changed

+131
-51
lines changed

8 files changed

+131
-51
lines changed

‎.github/workflows/CI-unixish.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ jobs:
111111
- name: Run CTest
112112
run: |
113113
pushd cmake.output
114-
ctest -j$(nproc)
114+
ctest --output-on-failure -j$(nproc)
115115
116116
build_uchar:
117117

‎cli/singleexecutor.cpp‎

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ unsigned int SingleExecutor::check()
5151

5252
std::size_t processedsize = 0;
5353
unsigned int c = 0;
54+
// TODO: processes either mSettings.project.fileSettings or mFiles - process/thread implementations process both
55+
// TODO: thread/process implementations process fileSettings first
5456
if (mSettings.project.fileSettings.empty()) {
5557
for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
5658
if (!mSettings.library.markupFile(i->first)
@@ -59,31 +61,53 @@ unsigned int SingleExecutor::check()
5961
processedsize += i->second;
6062
if (!mSettings.quiet)
6163
reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize);
64+
// TODO: call analyseClangTidy()
6265
c++;
6366
}
6467
}
6568
} else {
6669
// filesettings
6770
// check all files of the project
6871
for (const ImportProject::FileSettings &fs : mSettings.project.fileSettings) {
69-
result += mCppcheck.check(fs);
70-
++c;
71-
if (!mSettings.quiet)
72-
reportStatus(c, mSettings.project.fileSettings.size(), c, mSettings.project.fileSettings.size());
73-
if (mSettings.clangTidy)
74-
mCppcheck.analyseClangTidy(fs);
72+
if (!mSettings.library.markupFile(fs.filename)
73+
|| !mSettings.library.processMarkupAfterCode(fs.filename)) {
74+
result += mCppcheck.check(fs);
75+
++c;
76+
if (!mSettings.quiet)
77+
reportStatus(c, mSettings.project.fileSettings.size(), c, mSettings.project.fileSettings.size());
78+
if (mSettings.clangTidy)
79+
mCppcheck.analyseClangTidy(fs);
80+
}
7581
}
7682
}
7783

7884
// second loop to parse all markup files which may not work until all
7985
// c/cpp files have been parsed and checked
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-
if (!mSettings.quiet)
85-
reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize);
86-
c++;
86+
// TODO: get rid of duplicated code
87+
if (mSettings.project.fileSettings.empty()) {
88+
for (std::map<std::string, std::size_t>::const_iterator i = mFiles.cbegin(); i != mFiles.cend(); ++i) {
89+
if (mSettings.library.markupFile(i->first)
90+
&& mSettings.library.processMarkupAfterCode(i->first)) {
91+
result += mCppcheck.check(i->first);
92+
processedsize += i->second;
93+
if (!mSettings.quiet)
94+
reportStatus(c + 1, mFiles.size(), processedsize, totalfilesize);
95+
// TODO: call analyseClangTidy()
96+
c++;
97+
}
98+
}
99+
}
100+
else {
101+
for (const ImportProject::FileSettings &fs : mSettings.project.fileSettings) {
102+
if (mSettings.library.markupFile(fs.filename)
103+
&& mSettings.library.processMarkupAfterCode(fs.filename)) {
104+
result += mCppcheck.check(fs);
105+
++c;
106+
if (!mSettings.quiet)
107+
reportStatus(c, mSettings.project.fileSettings.size(), c, mSettings.project.fileSettings.size());
108+
if (mSettings.clangTidy)
109+
mCppcheck.analyseClangTidy(fs);
110+
}
87111
}
88112
}
89113
if (mCppcheck.analyseWholeProgram())

‎lib/library.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ namespace tinyxml2 {
5353
class CPPCHECKLIB Library {
5454
// TODO: get rid of this
5555
friend class TestSymbolDatabase; // For testing only
56-
friend class TestSingleExecutor; // For testing only
56+
friend class TestSingleExecutorBase; // For testing only
5757
friend class TestThreadExecutor; // For testing only
5858
friend class TestProcessExecutor; // For testing only
5959

‎releasenotes.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,4 @@ release notes for cppcheck-2.11
1414
- `constVariableReference`
1515
- `constVariablePointer`
1616
- More command-line parameters will now check if the given integer argument is actually valid. Several other internal string-to-integer conversions will not be error checked.
17+
- scanning projects (with -j1) will now defer the analysis of markup files until the whole code was processed

‎test/helpers.cpp‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include "preprocessor.h"
2323

2424
#include <cstdio>
25+
#include <iostream>
2526
#include <fstream>
2627
#include <stdexcept>
2728
#include <utility>
@@ -60,12 +61,21 @@ ScopedFile::ScopedFile(std::string name, const std::string &content, std::string
6061
}
6162

6263
ScopedFile::~ScopedFile() {
63-
std::remove(mFullPath.c_str());
64+
const int remove_res = std::remove(mFullPath.c_str());
65+
if (remove_res != 0) {
66+
std::cout << "ScopedFile(" << mFullPath + ") - could not delete file (" << remove_res << ")";
67+
}
6468
if (!mPath.empty() && mPath != Path::getCurrentPath()) {
6569
#ifdef _WIN32
66-
RemoveDirectoryA(mPath.c_str());
70+
if (!RemoveDirectoryA(mPath.c_str())) {
71+
std::cout << "ScopedFile(" << mFullPath + ") - could not delete folder (" << GetLastError() << ")";
72+
}
6773
#else
68-
rmdir(mPath.c_str());
74+
const int rmdir_res = rmdir(mPath.c_str());
75+
if (rmdir_res == -1) {
76+
const int err = errno;
77+
std::cout << "ScopedFile(" << mFullPath + ") - could not delete folder (" << err << ")";
78+
}
6979
#endif
7080
}
7181
}

‎test/testprocessexecutor.cpp‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ class TestProcessExecutor : public TestFixture {
4141
private:
4242
Settings settings = settingsBuilder().library("std.cfg").build();
4343

44+
static std::string fprefix()
45+
{
46+
return "process";
47+
}
48+
4449
/**
4550
* Execute check using n jobs for y files which are have
4651
* identical data, given within data.
@@ -53,7 +58,7 @@ class TestProcessExecutor : public TestFixture {
5358
if (filesList.empty()) {
5459
for (int i = 1; i <= files; ++i) {
5560
std::ostringstream oss;
56-
oss << "file_" << i << ".cpp";
61+
oss << fprefix() << "_" << i << ".cpp";
5762
filemap[oss.str()] = data.size();
5863
}
5964
}
@@ -186,7 +191,7 @@ class TestProcessExecutor : public TestFixture {
186191
settings.library.mProcessAfterCode.emplace(".cp1", true);
187192

188193
const std::vector<std::string> files = {
189-
"file_1.cp1", "file_2.cpp", "file_3.cp1", "file_4.cpp"
194+
fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp"
190195
};
191196

192197
// the checks are not executed on the markup files => expected result is 2
@@ -198,21 +203,21 @@ class TestProcessExecutor : public TestFixture {
198203
"}",
199204
SHOWTIME_MODES::SHOWTIME_NONE, nullptr, files);
200205
// TODO: order of "Checking" and "checked" is affected by thread
201-
/*TODO_ASSERT_EQUALS("Checking file_2.cpp ...\n"
206+
/*TODO_ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n"
202207
"1/4 files checked 25% done\n"
203-
"Checking file_4.cpp ...\n"
208+
"Checking " + fprefix() + "_4.cpp ...\n"
204209
"2/4 files checked 50% done\n"
205-
"Checking file_1.cp1 ...\n"
210+
"Checking " + fprefix() + "_1.cp1 ...\n"
206211
"3/4 files checked 75% done\n"
207-
"Checking file_3.cp1 ...\n"
212+
"Checking " + fprefix() + "_3.cp1 ...\n"
208213
"4/4 files checked 100% done\n",
209-
"Checking file_1.cp1 ...\n"
214+
"Checking " + fprefix() + "_1.cp1 ...\n"
210215
"1/4 files checked 25% done\n"
211-
"Checking file_2.cpp ...\n"
216+
"Checking " + fprefix() + "_2.cpp ...\n"
212217
"2/4 files checked 50% done\n"
213-
"Checking file_3.cp1 ...\n"
218+
"Checking " + fprefix() + "_3.cp1 ...\n"
214219
"3/4 files checked 75% done\n"
215-
"Checking file_4.cpp ...\n"
220+
"Checking " + fprefix() + "_4.cpp ...\n"
216221
"4/4 files checked 100% done\n",
217222
output.str());*/
218223
settings = settingsOld;

‎test/testsingleexecutor.cpp‎

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,12 +36,20 @@
3636
#include <utility>
3737
#include <vector>
3838

39-
class TestSingleExecutor : public TestFixture {
40-
public:
41-
TestSingleExecutor() : TestFixture("TestSingleExecutor") {}
39+
class TestSingleExecutorBase : public TestFixture {
40+
protected:
41+
TestSingleExecutorBase(const char * const name, bool useFS) : TestFixture(name), useFS(useFS) {}
4242

4343
private:
4444
Settings settings = settingsBuilder().library("std.cfg").build();
45+
bool useFS;
46+
47+
std::string fprefix() const
48+
{
49+
if (useFS)
50+
return "singlefs";
51+
return "single";
52+
}
4553

4654
static std::string zpad3(int i)
4755
{
@@ -55,18 +63,29 @@ class TestSingleExecutor : public TestFixture {
5563
void check(int files, int result, const std::string &data, SHOWTIME_MODES showtime = SHOWTIME_MODES::SHOWTIME_NONE, const char* const plistOutput = nullptr, const std::vector<std::string>& filesList = {}) {
5664
errout.str("");
5765
output.str("");
66+
settings.project.fileSettings.clear();
5867

5968
std::map<std::string, std::size_t> filemap;
6069
if (filesList.empty()) {
6170
for (int i = 1; i <= files; ++i) {
62-
const std::string s = "file_" + zpad3(i) + ".cpp";
71+
const std::string s = fprefix() + "_" + zpad3(i) + ".cpp";
6372
filemap[s] = data.size();
73+
if (useFS) {
74+
ImportProject::FileSettings fs;
75+
fs.filename = s;
76+
settings.project.fileSettings.emplace_back(std::move(fs));
77+
}
6478
}
6579
}
6680
else {
6781
for (const auto& f : filesList)
6882
{
6983
filemap[f] = data.size();
84+
if (useFS) {
85+
ImportProject::FileSettings fs;
86+
fs.filename = f;
87+
settings.project.fileSettings.emplace_back(std::move(fs));
88+
}
7089
}
7190
}
7291

@@ -78,13 +97,18 @@ class TestSingleExecutor : public TestFixture {
7897
return false;
7998
});
8099
cppcheck.settings() = settings;
81-
// TODO: test with settings.project.fileSettings;
82-
SingleExecutor executor(cppcheck, filemap, settings, *this);
100+
83101
std::vector<std::unique_ptr<ScopedFile>> scopedfiles;
84102
scopedfiles.reserve(filemap.size());
85103
for (std::map<std::string, std::size_t>::const_iterator i = filemap.cbegin(); i != filemap.cend(); ++i)
86104
scopedfiles.emplace_back(new ScopedFile(i->first, data));
87105

106+
// clear files list so only fileSettings are used
107+
if (useFS)
108+
filemap.clear();
109+
110+
// TODO: test with settings.project.fileSettings;
111+
SingleExecutor executor(cppcheck, filemap, settings, *this);
88112
ASSERT_EQUALS(result, executor.check());
89113
}
90114

@@ -112,7 +136,7 @@ class TestSingleExecutor : public TestFixture {
112136
"}");
113137
std::string expected;
114138
for (int i = 1; i <= 100; ++i) {
115-
expected += "Checking file_" + zpad3(i) + ".cpp ...\n";
139+
expected += "Checking " + fprefix() + "_" + zpad3(i) + ".cpp ...\n";
116140
expected += std::to_string(i) + "/100 files checked " + std::to_string(i) + "% done\n";
117141
}
118142
ASSERT_EQUALS(expected, output.str());
@@ -190,7 +214,7 @@ class TestSingleExecutor : public TestFixture {
190214
settings.library.mProcessAfterCode.emplace(".cp1", true);
191215

192216
const std::vector<std::string> files = {
193-
"file_1.cp1", "file_2.cpp", "file_3.cp1", "file_4.cpp"
217+
fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp"
194218
};
195219

196220
// checks are not executed on markup files => expected result is 2
@@ -202,13 +226,13 @@ class TestSingleExecutor : public TestFixture {
202226
"}",
203227
SHOWTIME_MODES::SHOWTIME_NONE, nullptr, files);
204228
// TODO: filter out the "files checked" messages
205-
ASSERT_EQUALS("Checking file_2.cpp ...\n"
229+
ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n"
206230
"1/4 files checked 25% done\n"
207-
"Checking file_4.cpp ...\n"
231+
"Checking " + fprefix() + "_4.cpp ...\n"
208232
"2/4 files checked 50% done\n"
209-
"Checking file_1.cp1 ...\n"
233+
"Checking " + fprefix() + "_1.cp1 ...\n"
210234
"3/4 files checked 75% done\n"
211-
"Checking file_3.cp1 ...\n"
235+
"Checking " + fprefix() + "_3.cp1 ...\n"
212236
"4/4 files checked 100% done\n", output.str());
213237
settings = settingsOld;
214238
}
@@ -217,4 +241,15 @@ class TestSingleExecutor : public TestFixture {
217241
// TODO: test whole program analysis
218242
};
219243

220-
REGISTER_TEST(TestSingleExecutor)
244+
class TestSingleExecutorFiles : public TestSingleExecutorBase {
245+
public:
246+
TestSingleExecutorFiles() : TestSingleExecutorBase("TestSingleExecutorFiles", false) {}
247+
};
248+
249+
class TestSingleExecutorFS : public TestSingleExecutorBase {
250+
public:
251+
TestSingleExecutorFS() : TestSingleExecutorBase("TestSingleExecutorFS", true) {}
252+
};
253+
254+
REGISTER_TEST(TestSingleExecutorFiles)
255+
REGISTER_TEST(TestSingleExecutorFS)

‎test/testthreadexecutor.cpp‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ class TestThreadExecutor : public TestFixture {
4141
private:
4242
Settings settings = settingsBuilder().library("std.cfg").build();
4343

44+
static std::string fprefix()
45+
{
46+
return "thread";
47+
}
48+
4449
/**
4550
* Execute check using n jobs for y files which are have
4651
* identical data, given within data.
@@ -53,7 +58,7 @@ class TestThreadExecutor : public TestFixture {
5358
if (filesList.empty()) {
5459
for (int i = 1; i <= files; ++i) {
5560
std::ostringstream oss;
56-
oss << "file_" << i << ".cpp";
61+
oss << fprefix() << "_" << i << ".cpp";
5762
filemap[oss.str()] = data.size();
5863
}
5964
}
@@ -184,7 +189,7 @@ class TestThreadExecutor : public TestFixture {
184189
settings.library.mProcessAfterCode.emplace(".cp1", true);
185190

186191
const std::vector<std::string> files = {
187-
"file_1.cp1", "file_2.cpp", "file_3.cp1", "file_4.cpp"
192+
fprefix() + "_1.cp1", fprefix() + "_2.cpp", fprefix() + "_3.cp1", fprefix() + "_4.cpp"
188193
};
189194

190195
// checks are not executed on markup files => expected result is 2
@@ -196,21 +201,21 @@ class TestThreadExecutor : public TestFixture {
196201
"}",
197202
SHOWTIME_MODES::SHOWTIME_NONE, nullptr, files);
198203
// TODO: order of "Checking" and "checked" is affected by thread
199-
/*TODO_ASSERT_EQUALS("Checking file_2.cpp ...\n"
204+
/*TODO_ASSERT_EQUALS("Checking " + fprefix() + "_2.cpp ...\n"
200205
"1/4 files checked 25% done\n"
201-
"Checking file_4.cpp ...\n"
206+
"Checking " + fprefix() + "_4.cpp ...\n"
202207
"2/4 files checked 50% done\n"
203-
"Checking file_1.cp1 ...\n"
208+
"Checking " + fprefix() + "_1.cp1 ...\n"
204209
"3/4 files checked 75% done\n"
205-
"Checking file_3.cp1 ...\n"
210+
"Checking " + fprefix() + "_3.cp1 ...\n"
206211
"4/4 files checked 100% done\n",
207-
"Checking file_1.cp1 ...\n"
212+
"Checking " + fprefix() + "_1.cp1 ...\n"
208213
"1/4 files checked 25% done\n"
209-
"Checking file_2.cpp ...\n"
214+
"Checking " + fprefix() + "_2.cpp ...\n"
210215
"2/4 files checked 50% done\n"
211-
"Checking file_3.cp1 ...\n"
216+
"Checking " + fprefix() + "_3.cp1 ...\n"
212217
"3/4 files checked 75% done\n"
213-
"Checking file_4.cpp ...\n"
218+
"Checking " + fprefix() + "_4.cpp ...\n"
214219
"4/4 files checked 100% done\n",
215220
output.str());*/
216221
settings = settingsOld;

0 commit comments

Comments
 (0)