Skip to content

dropped internal default template format not necessary for production code #7342

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 2, 2025
Merged

dropped internal default template format not necessary for production code #7342

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion democlient/democlient.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,9 @@ class CppcheckExecutor : public ErrorLogger {

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

std::cout << s << std::endl;

Expand Down
1 change: 1 addition & 0 deletions lib/cppcheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1867,6 +1867,7 @@ void CppCheck::purgedConfigurationMessage(const std::string &file, const std::st
void CppCheck::getErrorMessages(ErrorLogger &errorlogger)
{
Settings settings;
settings.templateFormat = "{callstack}: ({severity}) {inconclusive:inconclusive: }{message}"; // TODO: get rid of this
Suppressions supprs;

CppCheck cppcheck(settings, supprs, errorlogger, true, nullptr);
Expand Down
23 changes: 1 addition & 22 deletions lib/errorlogger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -610,30 +610,9 @@ static void replaceColors(std::string& source) {
replace(source, substitutionMap);
}

// TODO: remove default parameters
std::string ErrorMessage::toString(bool verbose, const std::string &templateFormat, const std::string &templateLocation) const
{
// Save this ErrorMessage in plain text.

// TODO: should never happen - remove this
// No template is given
// (not 100%) equivalent templateFormat: {callstack} ({severity}{inconclusive:, inconclusive}) {message}
if (templateFormat.empty()) {
std::string text;
if (!callStack.empty()) {
text += ErrorLogger::callStackToString(callStack);
text += ": ";
}
if (severity != Severity::none) {
text += '(';
text += severityToString(severity);
if (certainty == Certainty::inconclusive)
text += ", inconclusive";
text += ") ";
}
text += (verbose ? mVerboseMessage : mShortMessage);
return text;
}
assert(!templateFormat.empty());

// template is given. Reformat the output according to it
std::string result = templateFormat;
Expand Down
4 changes: 2 additions & 2 deletions lib/errorlogger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,8 @@ class CPPCHECKLIB ErrorMessage {
* @return formatted string
*/
std::string toString(bool verbose,
const std::string &templateFormat = emptyString,
const std::string &templateLocation = emptyString) const;
const std::string &templateFormat,
const std::string &templateLocation) const;

std::string serialize() const;
void deserialize(const std::string &data);
Expand Down
2 changes: 2 additions & 0 deletions oss-fuzz/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ static Settings create_settings()
{
// TODO: load std.cfg
Settings s;
s.templateFormat = "{bold}{file}:{line}:{column}: {red}{inconclusive:{magenta}}{severity}:{inconclusive: inconclusive:}{default} {message} [{id}]{reset}\\n{code}";
s.templateLocation = "{bold}{file}:{line}:{column}: {dim}note:{reset} {info}\\n{code}";
s.addEnabled("all");
s.certainty.setEnabled(Certainty::inconclusive, true);
return s;
Expand Down
21 changes: 20 additions & 1 deletion test/fixture.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,26 @@ void TestFixture::reportErr(const ErrorMessage &msg)
return;
if (msg.severity == Severity::information && msg.id == "normalCheckLevelMaxBranches")
return;
const std::string errormessage(msg.toString(mVerbose, mTemplateFormat, mTemplateLocation));
std::string errormessage;
if (!mTemplateFormat.empty()) {
errormessage = msg.toString(mVerbose, mTemplateFormat, mTemplateLocation);
}
else {
if (!msg.callStack.empty()) {
// TODO: add column
errormessage += ErrorLogger::callStackToString(msg.callStack);
errormessage += ": ";
}
if (msg.severity != Severity::none) {
errormessage += '(';
errormessage += severityToString(msg.severity);
if (msg.certainty == Certainty::inconclusive)
errormessage += ", inconclusive";
errormessage += ") ";
}
errormessage += msg.shortMessage();
// TODO: add ID
}
mErrout << errormessage << std::endl;
}

Expand Down
15 changes: 10 additions & 5 deletions test/testcppcheck.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ class TestCppcheck : public TestFixture {
TestCppcheck() : TestFixture("TestCppcheck") {}

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

class ErrorLogger2 : public ErrorLogger {
public:
Expand Down Expand Up @@ -113,7 +114,8 @@ class TestCppcheck : public TestFixture {
" return 0;\n"
"}");

const Settings s;
/*const*/ Settings s;
s.templateFormat = templateFormat;
Suppressions supprs;
ErrorLogger2 errorLogger;
CppCheck cppcheck(s, supprs, errorLogger, false, {});
Expand All @@ -135,7 +137,8 @@ class TestCppcheck : public TestFixture {
" return 0;\n"
"}");

const Settings s;
/*const*/ Settings s;
s.templateFormat = templateFormat;
Suppressions supprs;
ErrorLogger2 errorLogger;
CppCheck cppcheck(s, supprs, errorLogger, false, {});
Expand Down Expand Up @@ -184,7 +187,9 @@ class TestCppcheck : public TestFixture {
ScopedFile test_file_b("b.cpp",
"#include \"inc.h\"");

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

Settings s;
/*const*/ Settings s;
// this is the "simple" format
s.templateFormat = "{file}:{line}:{column}: {severity}:{inconclusive:inconclusive:} {message} [{id}]";
s.templateFormat = templateFormat; // TODO: remove when we only longer rely on toString() in unique message handling?
Suppressions supprs;
ErrorLogger2 errorLogger;
CppCheck cppcheck(s, supprs, errorLogger, false, {});
Expand Down
48 changes: 25 additions & 23 deletions test/testerrorlogger.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class TestErrorLogger : public TestFixture {
TestErrorLogger() : TestFixture("TestErrorLogger") {}

private:
const std::string templateFormat{"{callstack}: ({severity}) {inconclusive:inconclusive: }{message}"};

const ErrorMessage::FileLocation fooCpp5{"foo.cpp", 5, 1};
const ErrorMessage::FileLocation barCpp8{"bar.cpp", 8, 1};
const ErrorMessage::FileLocation barCpp8_i{"bar.cpp", "ä", 8, 1};
Expand Down Expand Up @@ -81,13 +83,13 @@ class TestErrorLogger : public TestFixture {
ErrorMessage message;
message.id = id;

std::string serialized = message.toString(true, idPlaceholder + plainText + idPlaceholder);
std::string serialized = message.toString(true, idPlaceholder + plainText + idPlaceholder, "");
ASSERT_EQUALS(id + plainText + id, serialized);

serialized = message.toString(true, idPlaceholder + idPlaceholder);
serialized = message.toString(true, idPlaceholder + idPlaceholder, "");
ASSERT_EQUALS(id + id, serialized);

serialized = message.toString(true, plainText + idPlaceholder + plainText);
serialized = message.toString(true, plainText + idPlaceholder + plainText, "");
ASSERT_EQUALS(plainText + id + plainText, serialized);
}

Expand Down Expand Up @@ -133,8 +135,8 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS(1, msg.callStack.size());
ASSERT_EQUALS("Programming error.", msg.shortMessage());
ASSERT_EQUALS("Programming error.", msg.verboseMessage());
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(false));
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(true));
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(false, templateFormat, ""));
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(true, templateFormat, ""));
}

void ErrorMessageConstructLocations() const {
Expand All @@ -143,8 +145,8 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS(2, msg.callStack.size());
ASSERT_EQUALS("Programming error.", msg.shortMessage());
ASSERT_EQUALS("Programming error.", msg.verboseMessage());
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(false));
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(true));
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(false, templateFormat, ""));
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(true, templateFormat, ""));
}

void ErrorMessageVerbose() const {
Expand All @@ -153,8 +155,8 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS(1, msg.callStack.size());
ASSERT_EQUALS("Programming error.", msg.shortMessage());
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(false));
ASSERT_EQUALS("[foo.cpp:5]: (error) Verbose error", msg.toString(true));
ASSERT_EQUALS("[foo.cpp:5]: (error) Programming error.", msg.toString(false, templateFormat, ""));
ASSERT_EQUALS("[foo.cpp:5]: (error) Verbose error", msg.toString(true, templateFormat, ""));
}

void ErrorMessageVerboseLocations() const {
Expand All @@ -163,8 +165,8 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS(2, msg.callStack.size());
ASSERT_EQUALS("Programming error.", msg.shortMessage());
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(false));
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Verbose error", msg.toString(true));
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Programming error.", msg.toString(false, templateFormat, ""));
ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Verbose error", msg.toString(true, templateFormat, ""));
}

void ErrorMessageFromInternalError() const {
Expand All @@ -179,8 +181,8 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS(0, loc.column);
ASSERT_EQUALS("message", msg.shortMessage());
ASSERT_EQUALS("message", msg.verboseMessage());
ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(false));
ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(true));
ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(false, templateFormat, ""));
ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(true, templateFormat, ""));
}
{
InternalError internalError(nullptr, "message", "details", InternalError::INTERNAL);
Expand All @@ -192,8 +194,8 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS(0, loc.column);
ASSERT_EQUALS("msg: message", msg.shortMessage());
ASSERT_EQUALS("msg: message: details", msg.verboseMessage());
ASSERT_EQUALS("[file.cpp:0]: (error) msg: message", msg.toString(false));
ASSERT_EQUALS("[file.cpp:0]: (error) msg: message: details", msg.toString(true));
ASSERT_EQUALS("[file.cpp:0]: (error) msg: message", msg.toString(false, templateFormat, ""));
ASSERT_EQUALS("[file.cpp:0]: (error) msg: message: details", msg.toString(true, templateFormat, ""));
}
}

Expand All @@ -207,7 +209,7 @@ class TestErrorLogger : public TestFixture {
msg.classification = getClassification(msg.guideline, reportType);
ASSERT_EQUALS("Advisory", msg.classification);
ASSERT_EQUALS("2.8", msg.guideline);
ASSERT_EQUALS("Advisory 2.8", msg.toString(true, format));
ASSERT_EQUALS("Advisory 2.8", msg.toString(true, format, ""));
}

void ErrorMessageReportTypeCertC() const {
Expand All @@ -220,7 +222,7 @@ class TestErrorLogger : public TestFixture {
msg.classification = getClassification(msg.guideline, reportType);
ASSERT_EQUALS("L3", msg.classification);
ASSERT_EQUALS("FIO42-C", msg.guideline);
ASSERT_EQUALS("L3 FIO42-C", msg.toString(true, format));
ASSERT_EQUALS("L3 FIO42-C", msg.toString(true, format, ""));
}

void CustomFormat() const {
Expand All @@ -229,8 +231,8 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS(1, msg.callStack.size());
ASSERT_EQUALS("Programming error.", msg.shortMessage());
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
ASSERT_EQUALS("foo.cpp:5,error,errorId,Programming error.", msg.toString(false, "{file}:{line},{severity},{id},{message}"));
ASSERT_EQUALS("foo.cpp:5,error,errorId,Verbose error", msg.toString(true, "{file}:{line},{severity},{id},{message}"));
ASSERT_EQUALS("foo.cpp:5,error,errorId,Programming error.", msg.toString(false, "{file}:{line},{severity},{id},{message}", ""));
ASSERT_EQUALS("foo.cpp:5,error,errorId,Verbose error", msg.toString(true, "{file}:{line},{severity},{id},{message}", ""));
}

void CustomFormat2() const {
Expand All @@ -239,8 +241,8 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS(1, msg.callStack.size());
ASSERT_EQUALS("Programming error.", msg.shortMessage());
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
ASSERT_EQUALS("Programming error. - foo.cpp(5):(error,errorId)", msg.toString(false, "{message} - {file}({line}):({severity},{id})"));
ASSERT_EQUALS("Verbose error - foo.cpp(5):(error,errorId)", msg.toString(true, "{message} - {file}({line}):({severity},{id})"));
ASSERT_EQUALS("Programming error. - foo.cpp(5):(error,errorId)", msg.toString(false, "{message} - {file}({line}):({severity},{id})", ""));
ASSERT_EQUALS("Verbose error - foo.cpp(5):(error,errorId)", msg.toString(true, "{message} - {file}({line}):({severity},{id})", ""));
}

void CustomFormatLocations() const {
Expand All @@ -250,8 +252,8 @@ class TestErrorLogger : public TestFixture {
ASSERT_EQUALS(2, msg.callStack.size());
ASSERT_EQUALS("Programming error.", msg.shortMessage());
ASSERT_EQUALS("Verbose error", msg.verboseMessage());
ASSERT_EQUALS("Programming error. - bar.cpp(8):(error,errorId)", msg.toString(false, "{message} - {file}({line}):({severity},{id})"));
ASSERT_EQUALS("Verbose error - bar.cpp(8):(error,errorId)", msg.toString(true, "{message} - {file}({line}):({severity},{id})"));
ASSERT_EQUALS("Programming error. - bar.cpp(8):(error,errorId)", msg.toString(false, "{message} - {file}({line}):({severity},{id})", ""));
ASSERT_EQUALS("Verbose error - bar.cpp(8):(error,errorId)", msg.toString(true, "{message} - {file}({line}):({severity},{id})", ""));
}

void ToXmlV2() const {
Expand Down
25 changes: 0 additions & 25 deletions test/testexecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,34 +47,9 @@ class TestExecutor : public TestFixture {

private:
void run() override {
TEST_CASE(hasToLogDefault);
TEST_CASE(hasToLogSimple);
}

void hasToLogDefault() {
const std::list<FileWithDetails> files{FileWithDetails{"test.c"}};
const std::list<FileSettings> fileSettings;
Suppressions supprs;
DummyExecutor executor(files, fileSettings, settingsDefault, supprs, *this);

ErrorMessage::FileLocation loc1("test.c", 1, 2);
ErrorMessage msg({std::move(loc1)}, "test.c", Severity::error, "error", "id", Certainty::normal);

ASSERT(executor.hasToLog_(msg));
ASSERT(!executor.hasToLog_(msg));

ErrorMessage::FileLocation loc2("test.c", 1, 12);
msg.callStack = {std::move(loc2)};

// TODO: the default message does not include the column
TODO_ASSERT(executor.hasToLog_(msg));

msg.id = "id2";

// TODO: the default message does not include the id
TODO_ASSERT(executor.hasToLog_(msg));
}

void hasToLogSimple() {
const std::list<FileWithDetails> files{FileWithDetails{"test.c"}};
const std::list<FileSettings> fileSettings;
Expand Down
1 change: 1 addition & 0 deletions test/testprocessexecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class TestProcessExecutorBase : public TestFixture {
s.quiet = opt.quiet;
if (opt.plistOutput)
s.plistOutput = opt.plistOutput;
s.templateFormat = "{callstack}: ({severity}) {inconclusive:inconclusive: }{message}";
Suppressions supprs;

bool executeCommandCalled = false;
Expand Down
1 change: 1 addition & 0 deletions test/testsingleexecutor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ class TestSingleExecutorBase : public TestFixture {
if (opt.plistOutput)
s.plistOutput = opt.plistOutput;
s.clangTidy = opt.clangTidy;
s.templateFormat = "{callstack}: ({severity}) {inconclusive:inconclusive: }{message}"; // TODO: remove when we only longer rely on toString() in unique message handling?

Suppressions supprs;

Expand Down
7 changes: 7 additions & 0 deletions test/testsuppressions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class TestSuppressions : public TestFixture {

private:

const std::string templateFormat{"{callstack}: ({severity}) {inconclusive:inconclusive: }{message}"};

void run() override {
TEST_CASE(suppressionsBadId1);
TEST_CASE(suppressionsDosFormat); // Ticket #1836
Expand Down Expand Up @@ -253,6 +255,7 @@ class TestSuppressions : public TestFixture {
settings.severity.enable(Severity::information);
if (suppression == "unusedFunction")
settings.checks.setEnabled(Checks::unusedFunction, true);
settings.templateFormat = templateFormat;

std::vector<std::unique_ptr<ScopedFile>> scopedfiles;
scopedfiles.reserve(filelist.size());
Expand Down Expand Up @@ -294,6 +297,7 @@ class TestSuppressions : public TestFixture {
$.quiet = true,
$.inlineSuppressions = true);
settings.severity.enable(Severity::information);
settings.templateFormat = templateFormat;

Suppressions supprs;
if (!suppression.empty()) {
Expand Down Expand Up @@ -340,6 +344,7 @@ class TestSuppressions : public TestFixture {
$.quiet = true,
$.inlineSuppressions = true);
settings.severity.enable(Severity::information);
settings.templateFormat = templateFormat;

Suppressions supprs;
if (!suppression.empty()) {
Expand Down Expand Up @@ -1198,6 +1203,7 @@ class TestSuppressions : public TestFixture {
Settings settings;
settings.quiet = true;
settings.exitCode = 1;
settings.templateFormat = templateFormat;

Suppressions supprs;
ASSERT_EQUALS("", supprs.nomsg.addSuppressionLine("uninitvar"));
Expand Down Expand Up @@ -1238,6 +1244,7 @@ class TestSuppressions : public TestFixture {
settings.inlineSuppressions = true;
settings.relativePaths = true;
settings.basePaths.emplace_back("/somewhere");
settings.templateFormat = templateFormat;
const char code[] =
"struct Point\n"
"{\n"
Expand Down
Loading
Loading