Skip to content

Commit 07eb1f0

Browse files
firewavefirewave
authored
dropped internal default template format not necessary for production code (#7342)
1 parent 19e69bc commit 07eb1f0

13 files changed

Lines changed: 74 additions & 79 deletions

‎democlient/democlient.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,9 @@ class CppcheckExecutor : public ErrorLogger {
7272

7373
void reportOut(const std::string & /*outmsg*/, Color /*c*/) override {}
7474
void reportErr(const ErrorMessage &msg) override {
75-
const std::string s = msg.toString(true);
75+
static const std::string templateFormat = "{bold}{file}:{line}:{column}: {red}{inconclusive:{magenta}}{severity}:{inconclusive: inconclusive:}{default} {message} [{id}]{reset}\\n{code}";
76+
static const std::string templateLocation = "{bold}{file}:{line}:{column}: {dim}note:{reset} {info}\\n{code}";
77+
const std::string s = msg.toString(true, templateFormat, templateLocation);
7678

7779
std::cout << s << std::endl;
7880

‎lib/cppcheck.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1861,6 +1861,7 @@ void CppCheck::purgedConfigurationMessage(const std::string &file, const std::st
18611861
void CppCheck::getErrorMessages(ErrorLogger &errorlogger)
18621862
{
18631863
Settings settings;
1864+
settings.templateFormat = "{callstack}: ({severity}) {inconclusive:inconclusive: }{message}"; // TODO: get rid of this
18641865
Suppressions supprs;
18651866

18661867
CppCheck cppcheck(settings, supprs, errorlogger, true, nullptr);

‎lib/errorlogger.cpp‎

Lines changed: 1 addition & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -610,30 +610,9 @@ static void replaceColors(std::string& source) {
610610
replace(source, substitutionMap);
611611
}
612612

613-
// TODO: remove default parameters
614613
std::string ErrorMessage::toString(bool verbose, const std::string &templateFormat, const std::string &templateLocation) const
615614
{
616-
// Save this ErrorMessage in plain text.
617-
618-
// TODO: should never happen - remove this
619-
// No template is given
620-
// (not 100%) equivalent templateFormat: {callstack} ({severity}{inconclusive:, inconclusive}) {message}
621-
if (templateFormat.empty()) {
622-
std::string text;
623-
if (!callStack.empty()) {
624-
text += ErrorLogger::callStackToString(callStack);
625-
text += ": ";
626-
}
627-
if (severity != Severity::none) {
628-
text += '(';
629-
text += severityToString(severity);
630-
if (certainty == Certainty::inconclusive)
631-
text += ", inconclusive";
632-
text += ") ";
633-
}
634-
text += (verbose ? mVerboseMessage : mShortMessage);
635-
return text;
636-
}
615+
assert(!templateFormat.empty());
637616

638617
// template is given. Reformat the output according to it
639618
std::string result = templateFormat;

‎lib/errorlogger.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,8 @@ class CPPCHECKLIB ErrorMessage {
158158
* @return formatted string
159159
*/
160160
std::string toString(bool verbose,
161-
const std::string &templateFormat = emptyString,
162-
const std::string &templateLocation = emptyString) const;
161+
const std::string &templateFormat,
162+
const std::string &templateLocation) const;
163163

164164
std::string serialize() const;
165165
void deserialize(const std::string &data);

‎oss-fuzz/main.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,8 @@ static Settings create_settings()
4949
{
5050
// TODO: load std.cfg
5151
Settings s;
52+
s.templateFormat = "{bold}{file}:{line}:{column}: {red}{inconclusive:{magenta}}{severity}:{inconclusive: inconclusive:}{default} {message} [{id}]{reset}\\n{code}";
53+
s.templateLocation = "{bold}{file}:{line}:{column}: {dim}note:{reset} {info}\\n{code}";
5254
s.addEnabled("all");
5355
s.certainty.setEnabled(Certainty::inconclusive, true);
5456
return s;

‎test/fixture.cpp‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,26 @@ void TestFixture::reportErr(const ErrorMessage &msg)
429429
return;
430430
if (msg.severity == Severity::information && msg.id == "normalCheckLevelMaxBranches")
431431
return;
432-
const std::string errormessage(msg.toString(mVerbose, mTemplateFormat, mTemplateLocation));
432+
std::string errormessage;
433+
if (!mTemplateFormat.empty()) {
434+
errormessage = msg.toString(mVerbose, mTemplateFormat, mTemplateLocation);
435+
}
436+
else {
437+
if (!msg.callStack.empty()) {
438+
// TODO: add column
439+
errormessage += ErrorLogger::callStackToString(msg.callStack);
440+
errormessage += ": ";
441+
}
442+
if (msg.severity != Severity::none) {
443+
errormessage += '(';
444+
errormessage += severityToString(msg.severity);
445+
if (msg.certainty == Certainty::inconclusive)
446+
errormessage += ", inconclusive";
447+
errormessage += ") ";
448+
}
449+
errormessage += msg.shortMessage();
450+
// TODO: add ID
451+
}
433452
mErrout << errormessage << std::endl;
434453
}
435454

‎test/testcppcheck.cpp‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ class TestCppcheck : public TestFixture {
3838
TestCppcheck() : TestFixture("TestCppcheck") {}
3939

4040
private:
41+
const std::string templateFormat{"{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]"};
4142

4243
class ErrorLogger2 : public ErrorLogger {
4344
public:
@@ -113,7 +114,8 @@ class TestCppcheck : public TestFixture {
113114
" return 0;\n"
114115
"}");
115116

116-
const Settings s;
117+
/*const*/ Settings s;
118+
s.templateFormat = templateFormat;
117119
Suppressions supprs;
118120
ErrorLogger2 errorLogger;
119121
CppCheck cppcheck(s, supprs, errorLogger, false, {});
@@ -135,7 +137,8 @@ class TestCppcheck : public TestFixture {
135137
" return 0;\n"
136138
"}");
137139

138-
const Settings s;
140+
/*const*/ Settings s;
141+
s.templateFormat = templateFormat;
139142
Suppressions supprs;
140143
ErrorLogger2 errorLogger;
141144
CppCheck cppcheck(s, supprs, errorLogger, false, {});
@@ -184,7 +187,9 @@ class TestCppcheck : public TestFixture {
184187
ScopedFile test_file_b("b.cpp",
185188
"#include \"inc.h\"");
186189

187-
const Settings s;
190+
/*const*/ Settings s;
191+
// this is the "simple" format
192+
s.templateFormat = templateFormat; // TODO: remove when we only longer rely on toString() in unique message handling
188193
Suppressions supprs;
189194
ErrorLogger2 errorLogger;
190195
CppCheck cppcheck(s, supprs, errorLogger, false, {});
@@ -215,9 +220,9 @@ class TestCppcheck : public TestFixture {
215220
"(void)b;\n"
216221
"}");
217222

218-
Settings s;
223+
/*const*/ Settings s;
219224
// this is the "simple" format
220-
s.templateFormat = "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]";
225+
s.templateFormat = templateFormat; // TODO: remove when we only longer rely on toString() in unique message handling?
221226
Suppressions supprs;
222227
ErrorLogger2 errorLogger;
223228
CppCheck cppcheck(s, supprs, errorLogger, false, {});

‎test/testerrorlogger.cpp‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ class TestErrorLogger : public TestFixture {
3333
TestErrorLogger() : TestFixture("TestErrorLogger") {}
3434

3535
private:
36+
const std::string templateFormat{"{callstack}: ({severity}) {inconclusive:inconclusive: }{message}"};
37+
3638
const ErrorMessage::FileLocation fooCpp5{"foo.cpp", 5, 1};
3739
const ErrorMessage::FileLocation barCpp8{"bar.cpp", 8, 1};
3840
const ErrorMessage::FileLocation barCpp8_i{"bar.cpp", "ä", 8, 1};
@@ -81,13 +83,13 @@ class TestErrorLogger : public TestFixture {
8183
ErrorMessage message;
8284
message.id = id;
8385

84-
std::string serialized = message.toString(true, idPlaceholder + plainText + idPlaceholder);
86+
std::string serialized = message.toString(true, idPlaceholder + plainText + idPlaceholder, "");
8587
ASSERT_EQUALS(id + plainText + id, serialized);
8688

87-
serialized = message.toString(true, idPlaceholder + idPlaceholder);
89+
serialized = message.toString(true, idPlaceholder + idPlaceholder, "");
8890
ASSERT_EQUALS(id + id, serialized);
8991

90-
serialized = message.toString(true, plainText + idPlaceholder + plainText);
92+
serialized = message.toString(true, plainText + idPlaceholder + plainText, "");
9193
ASSERT_EQUALS(plainText + id + plainText, serialized);
9294
}
9395

@@ -133,8 +135,8 @@ class TestErrorLogger : public TestFixture {
133135
ASSERT_EQUALS(1, msg.callStack.size());
134136
ASSERT_EQUALS("Programming error.", msg.shortMessage());
135137
ASSERT_EQUALS("Programming error.", msg.verboseMessage());
136-
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(false));
137-
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(true));
138+
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(false, templateFormat, ""));
139+
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(true, templateFormat, ""));
138140
}
139141

140142
void ErrorMessageConstructLocations() const {
@@ -143,8 +145,8 @@ class TestErrorLogger : public TestFixture {
143145
ASSERT_EQUALS(2, msg.callStack.size());
144146
ASSERT_EQUALS("Programming error.", msg.shortMessage());
145147
ASSERT_EQUALS("Programming error.", msg.verboseMessage());
146-
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(false));
147-
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(true));
148+
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(false, templateFormat, ""));
149+
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(true, templateFormat, ""));
148150
}
149151

150152
void ErrorMessageVerbose() const {
@@ -153,8 +155,8 @@ class TestErrorLogger : public TestFixture {
153155
ASSERT_EQUALS(1, msg.callStack.size());
154156
ASSERT_EQUALS("Programming error.", msg.shortMessage());
155157
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
156-
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(false));
157-
ASSERT_EQUALS("[foo.cpp:5]: (error) Verbose error", msg.toString(true));
158+
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(false, templateFormat, ""));
159+
ASSERT_EQUALS("[foo.cpp:5]: (error) Verbose error", msg.toString(true, templateFormat, ""));
158160
}
159161

160162
void ErrorMessageVerboseLocations() const {
@@ -163,8 +165,8 @@ class TestErrorLogger : public TestFixture {
163165
ASSERT_EQUALS(2, msg.callStack.size());
164166
ASSERT_EQUALS("Programming error.", msg.shortMessage());
165167
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
166-
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(false));
167-
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Verbose error", msg.toString(true));
168+
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(false, templateFormat, ""));
169+
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Verbose error", msg.toString(true, templateFormat, ""));
168170
}
169171

170172
void ErrorMessageFromInternalError() const {
@@ -179,8 +181,8 @@ class TestErrorLogger : public TestFixture {
179181
ASSERT_EQUALS(0, loc.column);
180182
ASSERT_EQUALS("message", msg.shortMessage());
181183
ASSERT_EQUALS("message", msg.verboseMessage());
182-
ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(false));
183-
ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(true));
184+
ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(false, templateFormat, ""));
185+
ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(true, templateFormat, ""));
184186
}
185187
{
186188
InternalError internalError(nullptr, "message", "details", InternalError::INTERNAL);
@@ -192,8 +194,8 @@ class TestErrorLogger : public TestFixture {
192194
ASSERT_EQUALS(0, loc.column);
193195
ASSERT_EQUALS("msg: message", msg.shortMessage());
194196
ASSERT_EQUALS("msg: message: details", msg.verboseMessage());
195-
ASSERT_EQUALS("[file.cpp:0]: (error) msg: message", msg.toString(false));
196-
ASSERT_EQUALS("[file.cpp:0]: (error) msg: message: details", msg.toString(true));
197+
ASSERT_EQUALS("[file.cpp:0]: (error) msg: message", msg.toString(false, templateFormat, ""));
198+
ASSERT_EQUALS("[file.cpp:0]: (error) msg: message: details", msg.toString(true, templateFormat, ""));
197199
}
198200
}
199201

@@ -207,7 +209,7 @@ class TestErrorLogger : public TestFixture {
207209
msg.classification = getClassification(msg.guideline, reportType);
208210
ASSERT_EQUALS("Advisory", msg.classification);
209211
ASSERT_EQUALS("2.8", msg.guideline);
210-
ASSERT_EQUALS("Advisory 2.8", msg.toString(true, format));
212+
ASSERT_EQUALS("Advisory 2.8", msg.toString(true, format, ""));
211213
}
212214

213215
void ErrorMessageReportTypeCertC() const {
@@ -220,7 +222,7 @@ class TestErrorLogger : public TestFixture {
220222
msg.classification = getClassification(msg.guideline, reportType);
221223
ASSERT_EQUALS("L3", msg.classification);
222224
ASSERT_EQUALS("FIO42-C", msg.guideline);
223-
ASSERT_EQUALS("L3 FIO42-C", msg.toString(true, format));
225+
ASSERT_EQUALS("L3 FIO42-C", msg.toString(true, format, ""));
224226
}
225227

226228
void CustomFormat() const {
@@ -229,8 +231,8 @@ class TestErrorLogger : public TestFixture {
229231
ASSERT_EQUALS(1, msg.callStack.size());
230232
ASSERT_EQUALS("Programming error.", msg.shortMessage());
231233
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
232-
ASSERT_EQUALS("foo.cpp:5,error,errorId,Programming error.", msg.toString(false, "{file}:{line},{severity},{id},{message}"));
233-
ASSERT_EQUALS("foo.cpp:5,error,errorId,Verbose error", msg.toString(true, "{file}:{line},{severity},{id},{message}"));
234+
ASSERT_EQUALS("foo.cpp:5,error,errorId,Programming error.", msg.toString(false, "{file}:{line},{severity},{id},{message}", ""));
235+
ASSERT_EQUALS("foo.cpp:5,error,errorId,Verbose error", msg.toString(true, "{file}:{line},{severity},{id},{message}", ""));
234236
}
235237

236238
void CustomFormat2() const {
@@ -239,8 +241,8 @@ class TestErrorLogger : public TestFixture {
239241
ASSERT_EQUALS(1, msg.callStack.size());
240242
ASSERT_EQUALS("Programming error.", msg.shortMessage());
241243
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
242-
ASSERT_EQUALS("Programming error. - foo.cpp(5):(error,errorId)", msg.toString(false, "{message} - {file}({line}):({severity},{id})"));
243-
ASSERT_EQUALS("Verbose error - foo.cpp(5):(error,errorId)", msg.toString(true, "{message} - {file}({line}):({severity},{id})"));
244+
ASSERT_EQUALS("Programming error. - foo.cpp(5):(error,errorId)", msg.toString(false, "{message} - {file}({line}):({severity},{id})", ""));
245+
ASSERT_EQUALS("Verbose error - foo.cpp(5):(error,errorId)", msg.toString(true, "{message} - {file}({line}):({severity},{id})", ""));
244246
}
245247

246248
void CustomFormatLocations() const {
@@ -250,8 +252,8 @@ class TestErrorLogger : public TestFixture {
250252
ASSERT_EQUALS(2, msg.callStack.size());
251253
ASSERT_EQUALS("Programming error.", msg.shortMessage());
252254
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
253-
ASSERT_EQUALS("Programming error. - bar.cpp(8):(error,errorId)", msg.toString(false, "{message} - {file}({line}):({severity},{id})"));
254-
ASSERT_EQUALS("Verbose error - bar.cpp(8):(error,errorId)", msg.toString(true, "{message} - {file}({line}):({severity},{id})"));
255+
ASSERT_EQUALS("Programming error. - bar.cpp(8):(error,errorId)", msg.toString(false, "{message} - {file}({line}):({severity},{id})", ""));
256+
ASSERT_EQUALS("Verbose error - bar.cpp(8):(error,errorId)", msg.toString(true, "{message} - {file}({line}):({severity},{id})", ""));
255257
}
256258

257259
void ToXmlV2() const {

‎test/testexecutor.cpp‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -47,34 +47,9 @@ class TestExecutor : public TestFixture {
4747

4848
private:
4949
void run() override {
50-
TEST_CASE(hasToLogDefault);
5150
TEST_CASE(hasToLogSimple);
5251
}
5352

54-
void hasToLogDefault() {
55-
const std::list<FileWithDetails> files{FileWithDetails{"test.c"}};
56-
const std::list<FileSettings> fileSettings;
57-
Suppressions supprs;
58-
DummyExecutor executor(files, fileSettings, settingsDefault, supprs, *this);
59-
60-
ErrorMessage::FileLocation loc1("test.c", 1, 2);
61-
ErrorMessage msg({std::move(loc1)}, "test.c", Severity::error, "error", "id", Certainty::normal);
62-
63-
ASSERT(executor.hasToLog_(msg));
64-
ASSERT(!executor.hasToLog_(msg));
65-
66-
ErrorMessage::FileLocation loc2("test.c", 1, 12);
67-
msg.callStack = {std::move(loc2)};
68-
69-
// TODO: the default message does not include the column
70-
TODO_ASSERT(executor.hasToLog_(msg));
71-
72-
msg.id = "id2";
73-
74-
// TODO: the default message does not include the id
75-
TODO_ASSERT(executor.hasToLog_(msg));
76-
}
77-
7853
void hasToLogSimple() {
7954
const std::list<FileWithDetails> files{FileWithDetails{"test.c"}};
8055
const std::list<FileSettings> fileSettings;

‎test/testprocessexecutor.cpp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ class TestProcessExecutorBase : public TestFixture {
9595
s.quiet = opt.quiet;
9696
if (opt.plistOutput)
9797
s.plistOutput = opt.plistOutput;
98+
s.templateFormat = "{callstack}: ({severity}) {inconclusive:inconclusive: }{message}";
9899
Suppressions supprs;
99100

100101
bool executeCommandCalled = false;

0 commit comments

Comments
 (0)