Skip to content

Commit c5bbab7

Browse files
committed
fixed danmar#6366 - some unique errors were omitted [skip ci]
1 parent 03aae03 commit c5bbab7

File tree

8 files changed

+194
-11
lines changed

8 files changed

+194
-11
lines changed

Makefile

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ TESTOBJ = test/fixture.o \
286286
test/testcppcheck.o \
287287
test/testerrorlogger.o \
288288
test/testexceptionsafety.o \
289+
test/testexecutor.o \
289290
test/testfilelister.o \
290291
test/testfilesettings.o \
291292
test/testfunctions.o \
@@ -751,6 +752,9 @@ test/testerrorlogger.o: test/testerrorlogger.cpp externals/tinyxml2/tinyxml2.h l
751752
test/testexceptionsafety.o: test/testexceptionsafety.cpp externals/simplecpp/simplecpp.h lib/addoninfo.h lib/check.h lib/checkers.h lib/checkexceptionsafety.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/tokenize.h lib/tokenlist.h lib/utils.h test/fixture.h test/helpers.h
752753
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testexceptionsafety.cpp
753754

755+
test/testexecutor.o: test/testexecutor.cpp cli/executor.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/utils.h test/fixture.h
756+
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testexecutor.cpp
757+
754758
test/testfilelister.o: test/testfilelister.cpp cli/filelister.h lib/addoninfo.h lib/check.h lib/checkers.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/filesettings.h lib/library.h lib/mathlib.h lib/path.h lib/pathmatch.h lib/platform.h lib/settings.h lib/standards.h lib/utils.h test/fixture.h
755759
$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testfilelister.cpp
756760

cli/cppcheckexecutor.cpp

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -641,23 +641,24 @@ void StdLogger::reportErr(const ErrorMessage &msg)
641641
if (msg.severity == Severity::internal)
642642
return;
643643

644-
// TODO: we generate a different message here then we log below
645-
// TODO: there should be no need for verbose and default messages here
646-
// Alert only about unique errors
647-
if (!mSettings.emitDuplicates && !mShownErrors.insert(msg.toString(mSettings.verbose)).second)
648-
return;
649-
650644
ErrorMessage msgCopy = msg;
651645
msgCopy.guideline = getGuideline(msgCopy.id, mSettings.reportType,
652646
mGuidelineMapping, msgCopy.severity);
653647
msgCopy.classification = getClassification(msgCopy.guideline, mSettings.reportType);
654648

649+
// TODO: there should be no need for verbose and default messages here
650+
const std::string msgStr = msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
651+
652+
// Alert only about unique errors
653+
if (!mSettings.emitDuplicates && !mShownErrors.insert(msgStr).second)
654+
return;
655+
655656
if (mSettings.outputFormat == Settings::OutputFormat::sarif)
656657
mSarifReport.addFinding(msgCopy);
657658
else if (mSettings.outputFormat == Settings::OutputFormat::xml)
658659
reportErr(msgCopy.toXML());
659660
else
660-
reportErr(msgCopy.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation));
661+
reportErr(msgStr);
661662
}
662663

663664
/**

cli/executor.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ bool Executor::hasToLog(const ErrorMessage &msg)
4646
if (!mSuppressions.nomsg.isSuppressed(msg, {}))
4747
{
4848
// TODO: there should be no need for verbose and default messages here
49-
std::string errmsg = msg.toString(mSettings.verbose);
49+
std::string errmsg = msg.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
5050
if (errmsg.empty())
5151
return false;
5252

lib/cppcheck.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ class CppCheck::CppCheckLogger : public ErrorLogger
203203
}
204204

205205
// TODO: there should be no need for the verbose and default messages here
206-
std::string errmsg = msg.toString(mSettings.verbose);
206+
std::string errmsg = msg.toString(mSettings.verbose, mSettings.templateFormat, mSettings.templateLocation);
207207
if (errmsg.empty())
208208
return;
209209

test/cli/other_test.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3160,4 +3160,31 @@ def test_debug_valueflow_xml(tmp_path): # #13606
31603160
assert 'floatvalue' in value_elem[1].attrib
31613161
assert value_elem[1].attrib['floatvalue'] == '1e-07'
31623162
assert 'floatvalue' in value_elem[2].attrib
3163-
assert value_elem[2].attrib['floatvalue'] == '1e-07'
3163+
assert value_elem[2].attrib['floatvalue'] == '1e-07'
3164+
3165+
3166+
def test_unique_error(tmp_path): # #6366
3167+
test_file = tmp_path / 'test.c'
3168+
with open(test_file, 'wt') as f:
3169+
f.write(
3170+
"""void f()
3171+
{
3172+
const long m[9] = {};
3173+
long a=m[9], b=m[9];
3174+
(void)a;
3175+
(void)b;
3176+
}
3177+
""")
3178+
3179+
args = [
3180+
'-q',
3181+
'--template=simple',
3182+
str(test_file)
3183+
]
3184+
exitcode, stdout, stderr = cppcheck(args)
3185+
assert exitcode == 0, stdout
3186+
assert stdout.splitlines() == []
3187+
assert stderr.splitlines() == [
3188+
"{}:4:13: error: Array 'm[9]' accessed at index 9, which is out of bounds. [arrayIndexOutOfBounds]".format(test_file),
3189+
"{}:4:21: error: Array 'm[9]' accessed at index 9, which is out of bounds. [arrayIndexOutOfBounds]".format(test_file)
3190+
]

test/testcppcheck.cpp

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ class TestCppcheck : public TestFixture {
5959
TEST_CASE(checkWithFS);
6060
TEST_CASE(suppress_error_library);
6161
TEST_CASE(unique_errors);
62+
TEST_CASE(unique_errors_2);
6263
TEST_CASE(isPremiumCodingStandardId);
6364
TEST_CASE(getDumpFileContentsRawTokens);
6465
TEST_CASE(getDumpFileContentsLibrary);
@@ -170,7 +171,7 @@ class TestCppcheck : public TestFixture {
170171
ASSERT_EQUALS(0, errorLogger.ids.size());
171172
}
172173

173-
// TODO: hwo to actually get duplicated findings
174+
// TODO: how to actually get duplicated findings
174175
void unique_errors() const
175176
{
176177
ScopedFile file("inc.h",
@@ -196,11 +197,57 @@ class TestCppcheck : public TestFixture {
196197
// the internal errorlist is cleared after each check() call
197198
ASSERT_EQUALS(2, errorLogger.errmsgs.size());
198199
auto it = errorLogger.errmsgs.cbegin();
200+
ASSERT_EQUALS("a.cpp", it->file0);
199201
ASSERT_EQUALS("nullPointer", it->id);
200202
++it;
203+
ASSERT_EQUALS("b.cpp", it->file0);
201204
ASSERT_EQUALS("nullPointer", it->id);
202205
}
203206

207+
void unique_errors_2() const
208+
{
209+
ScopedFile test_file("c.cpp",
210+
"void f()\n"
211+
"{\n"
212+
"const long m[9] = {};\n"
213+
"long a=m[9], b=m[9];\n"
214+
"(void)a;\n"
215+
"(void)b;\n"
216+
"}");
217+
218+
Settings s;
219+
// this is the "simple" format
220+
s.templateFormat = "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]";
221+
Suppressions supprs;
222+
ErrorLogger2 errorLogger;
223+
CppCheck cppcheck(s, supprs, errorLogger, false, {});
224+
ASSERT_EQUALS(1, cppcheck.check(FileWithDetails(test_file.path())));
225+
// TODO: how to properly disable these warnings?
226+
errorLogger.errmsgs.erase(std::remove_if(errorLogger.errmsgs.begin(), errorLogger.errmsgs.end(), [](const ErrorMessage& msg) {
227+
return msg.id == "logChecker";
228+
}), errorLogger.errmsgs.end());
229+
// the internal errorlist is cleared after each check() call
230+
ASSERT_EQUALS(2, errorLogger.errmsgs.size());
231+
auto it = errorLogger.errmsgs.cbegin();
232+
ASSERT_EQUALS("c.cpp", it->file0);
233+
ASSERT_EQUALS(1, it->callStack.size());
234+
{
235+
auto stack = it->callStack.cbegin();
236+
ASSERT_EQUALS(4, stack->line);
237+
ASSERT_EQUALS(9, stack->column);
238+
}
239+
ASSERT_EQUALS("arrayIndexOutOfBounds", it->id);
240+
++it;
241+
ASSERT_EQUALS("c.cpp", it->file0);
242+
ASSERT_EQUALS(1, it->callStack.size());
243+
{
244+
auto stack = it->callStack.cbegin();
245+
ASSERT_EQUALS(4, stack->line);
246+
ASSERT_EQUALS(17, stack->column);
247+
}
248+
ASSERT_EQUALS("arrayIndexOutOfBounds", it->id);
249+
}
250+
204251
void isPremiumCodingStandardId() const {
205252
Suppressions supprs;
206253
ErrorLogger2 errorLogger;

test/testexecutor.cpp

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,103 @@
1+
/*
2+
* Cppcheck - A tool for static C/C++ code analysis
3+
* Copyright (C) 2007-2025 Cppcheck team.
4+
*
5+
* This program is free software: you can redistribute it and/or modify
6+
* it under the terms of the GNU General Public License as published by
7+
* the Free Software Foundation, either version 3 of the License, or
8+
* (at your option) any later version.
9+
*
10+
* This program is distributed in the hope that it will be useful,
11+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
12+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
13+
* GNU General Public License for more details.
14+
*
15+
* You should have received a copy of the GNU General Public License
16+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
17+
*/
18+
19+
#include "executor.h"
20+
#include "fixture.h"
21+
#include "suppressions.h"
22+
23+
#include <stdexcept>
24+
25+
class DummyExecutor : public Executor
26+
{
27+
public:
28+
DummyExecutor(const std::list<FileWithDetails> &files, const std::list<FileSettings>& fileSettings, const Settings &settings, Suppressions &suppressions, ErrorLogger &errorLogger)
29+
: Executor(files, fileSettings, settings, suppressions, errorLogger)
30+
{}
31+
32+
unsigned int check() override
33+
{
34+
throw std::runtime_error("not implemented");
35+
}
36+
37+
bool hasToLog_(const ErrorMessage &msg)
38+
{
39+
return hasToLog(msg);
40+
}
41+
};
42+
43+
class TestExecutor : public TestFixture {
44+
public:
45+
TestExecutor() : TestFixture("TestExecutor") {}
46+
47+
private:
48+
void run() override {
49+
TEST_CASE(hasToLogDefault);
50+
TEST_CASE(hasToLogSimple);
51+
}
52+
53+
void hasToLogDefault() {
54+
const std::list<FileWithDetails> files{FileWithDetails{"test.c"}};
55+
const std::list<FileSettings> fileSettings;
56+
Suppressions supprs;
57+
DummyExecutor executor(files, fileSettings, settingsDefault, supprs, *this);
58+
59+
ErrorMessage::FileLocation loc1("test.c", 1, 2);
60+
ErrorMessage msg({std::move(loc1)}, "test.c", Severity::error, "error", "id", Certainty::normal);
61+
62+
ASSERT(executor.hasToLog_(msg));
63+
ASSERT(!executor.hasToLog_(msg));
64+
65+
ErrorMessage::FileLocation loc2("test.c", 1, 12);
66+
msg.callStack = {std::move(loc2)};
67+
68+
// TODO: the default message does not include the column
69+
TODO_ASSERT(executor.hasToLog_(msg));
70+
71+
msg.id = "id2";
72+
73+
// TODO: the default message does not include the id
74+
TODO_ASSERT(executor.hasToLog_(msg));
75+
}
76+
77+
void hasToLogSimple() {
78+
const std::list<FileWithDetails> files{FileWithDetails{"test.c"}};
79+
const std::list<FileSettings> fileSettings;
80+
Settings settings;
81+
// this is the "simple" format
82+
settings.templateFormat = "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]";
83+
Suppressions supprs;
84+
DummyExecutor executor(files, fileSettings, settings, supprs, *this);
85+
86+
ErrorMessage::FileLocation loc1("test.c", 1, 2);
87+
ErrorMessage msg({std::move(loc1)}, "test.c", Severity::error, "error", "id", Certainty::normal);
88+
89+
ASSERT(executor.hasToLog_(msg));
90+
ASSERT(!executor.hasToLog_(msg));
91+
92+
ErrorMessage::FileLocation loc2("test.c", 1, 12);
93+
msg.callStack = {std::move(loc2)};
94+
95+
ASSERT(executor.hasToLog_(msg));
96+
97+
msg.id = "id2";
98+
99+
ASSERT(executor.hasToLog_(msg));
100+
}
101+
};
102+
103+
REGISTER_TEST(TestExecutor)

test/testrunner.vcxproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
<ClCompile Include="testcppcheck.cpp" />
6262
<ClCompile Include="testerrorlogger.cpp" />
6363
<ClCompile Include="testexceptionsafety.cpp" />
64+
<ClCompile Include="testexecutor.cpp" />
6465
<ClCompile Include="testfilelister.cpp" />
6566
<ClCompile Include="testfilesettings.cpp" />
6667
<ClCompile Include="testfunctions.cpp" />

0 commit comments

Comments
 (0)