From 4e7d3232810f733e1a9f4fdb2ff27918716f5e9e Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Mar 2023 00:33:11 +0100 Subject: [PATCH 01/13] errorlogger.cpp: optimized `replaceColors()` by only processing the string once --- lib/errorlogger.cpp | 43 +++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 16 deletions(-) diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index e1f15520efa..82c042c9f6e 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -513,22 +513,33 @@ static std::string readCode(const std::string &file, int linenr, int column, con static void replaceColors(std::string& source) { - static const std::string reset_str = ::toString(Color::Reset); - findAndReplace(source, "{reset}", reset_str); - static const std::string bold_str = ::toString(Color::Bold); - findAndReplace(source, "{bold}", bold_str); - static const std::string dim_str = ::toString(Color::Dim); - findAndReplace(source, "{dim}", dim_str); - static const std::string red_str = ::toString(Color::FgRed); - findAndReplace(source, "{red}", red_str); - static const std::string green_str = ::toString(Color::FgGreen); - findAndReplace(source, "{green}", green_str); - static const std::string blue_str = ::toString(Color::FgBlue); - findAndReplace(source, "{blue}", blue_str); - static const std::string magenta_str = ::toString(Color::FgMagenta); - findAndReplace(source, "{magenta}", magenta_str); - static const std::string default_str = ::toString(Color::FgDefault); - findAndReplace(source, "{default}", default_str); + static const std::unordered_map substitutionMap = + { + {"{reset}", ::toString(Color::Reset)}, + {"{bold}", ::toString(Color::Bold)}, + {"{dim}", ::toString(Color::Dim)}, + {"{red}", ::toString(Color::FgRed)}, + {"{green}", ::toString(Color::FgGreen)}, + {"{blue}", ::toString(Color::FgBlue)}, + {"{magenta}", ::toString(Color::FgMagenta)}, + {"{default}", ::toString(Color::FgDefault)}, + }; + + std::string::size_type index = 0; + while ((index = source.find('{', index)) != std::string::npos) { + const std::string::size_type end = source.find('}', index); + if (end == std::string::npos) + break; + const std::string searchFor = source.substr(index, end-index+1); + const auto it = substitutionMap.find(searchFor); + if (it == substitutionMap.end()) { + index += 1; + continue; + } + const std::string& replaceWith = it->second; + source.replace(index, searchFor.length(), replaceWith); + index += replaceWith.length(); + } } std::string ErrorMessage::toString(bool verbose, const std::string &templateFormat, const std::string &templateLocation) const From df368f953131f63d6c90445a2bce75a852aca55d Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Mar 2023 00:34:04 +0100 Subject: [PATCH 02/13] color.cpp: optimized `toString()` for Windows --- lib/color.cpp | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/color.cpp b/lib/color.cpp index 5fec2edec9e..4e827531786 100644 --- a/lib/color.cpp +++ b/lib/color.cpp @@ -20,9 +20,9 @@ #ifndef _WIN32 #include -#endif #include #include // IWYU pragma: keep +#endif bool gDisableColors = false; @@ -32,18 +32,26 @@ std::ostream& operator<<(std::ostream& os, const Color& /*c*/) #else std::ostream& operator<<(std::ostream & os, const Color& c) { +#ifndef _WIN32 // TODO: handle piping into file as well as other pipes like stderr static const bool s_is_tty = isatty(STDOUT_FILENO); if (!gDisableColors && s_is_tty) return os << "\033[" << static_cast(c) << "m"; +#else + (void)c; #endif return os; } std::string toString(const Color& c) { +#ifndef _WIN32 std::stringstream ss; ss << c; return ss.str(); +#else + (void)c; + return ""; +#endif } From 8eb5e072832db25fd93b6d3ab407bf837f5317f4 Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Mar 2023 00:56:36 +0100 Subject: [PATCH 03/13] errorlogger.cpp: extracted replacing of special characters into function which only processes the string once --- lib/errorlogger.cpp | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 82c042c9f6e..709e76cc070 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -511,6 +511,31 @@ static std::string readCode(const std::string &file, int linenr, int column, con return line + endl + std::string((column>0 ? column-1 : 0), ' ') + '^'; } +static void replaceSpecialChars(std::string& source) +{ + // Support a few special characters to allow to specific formatting, see http://sourceforge.net/apps/phpbb/cppcheck/viewtopic.php?f=4&t=494&sid=21715d362c0dbafd3791da4d9522f814 + // Substitution should be done first so messages from cppcheck never get translated. + static const std::unordered_map substitutionMap = { + {'b', "\b"}, + {'n', "\n"}, + {'r', "\r"}, + {'t', "\t"} + }; + + std::string::size_type index = 0; + while ((index = source.find('\\', index)) != std::string::npos) { + const char searchFor = source[index+1]; + const auto it = substitutionMap.find(searchFor); + if (it == substitutionMap.end()) { + index += 1; + continue; + } + const std::string& replaceWith = it->second; + source.replace(index, 2, replaceWith); + index += replaceWith.length(); + } +} + static void replaceColors(std::string& source) { static const std::unordered_map substitutionMap = @@ -566,12 +591,7 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm // template is given. Reformat the output according to it std::string result = templateFormat; - // Support a few special characters to allow to specific formatting, see http://sourceforge.net/apps/phpbb/cppcheck/viewtopic.php?f=4&t=494&sid=21715d362c0dbafd3791da4d9522f814 - // Substitution should be done first so messages from cppcheck never get translated. - findAndReplace(result, "\\b", "\b"); - findAndReplace(result, "\\n", "\n"); - findAndReplace(result, "\\r", "\r"); - findAndReplace(result, "\\t", "\t"); + replaceSpecialChars(result); replaceColors(result); findAndReplace(result, "{id}", id); @@ -616,10 +636,7 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm for (const FileLocation &fileLocation : callStack) { std::string text = templateLocation; - findAndReplace(text, "\\b", "\b"); - findAndReplace(text, "\\n", "\n"); - findAndReplace(text, "\\r", "\r"); - findAndReplace(text, "\\t", "\t"); + replaceSpecialChars(text); replaceColors(text); findAndReplace(text, "{file}", fileLocation.getfile()); From 8e44ebdd53405c4608078c708d7ef7e20c5949b7 Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Mar 2023 01:40:30 +0100 Subject: [PATCH 04/13] substitute static parts of the template strings only once --- cli/cmdlineparser.cpp | 3 ++ lib/color.cpp | 15 +++++---- lib/errorlogger.cpp | 17 +++++++--- lib/errorlogger.h | 6 ++++ test/testcmdlineparser.cpp | 3 +- test/testerrorlogger.cpp | 66 ++++++++++++++++++++++++++++++++++++++ 6 files changed, 98 insertions(+), 12 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 5f2cf23c527..28f8bdd44e6 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -1015,6 +1015,9 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) if (mSettings.templateLocation.empty()) mSettings.templateLocation = "{bold}{file}:{line}:{column}: {dim}note:{reset} {info}\\n{code}"; } + // replace static parts of the templates + substituteTemplateFormatStatic(mSettings.templateFormat); + substituteTemplateLocationStatic(mSettings.templateLocation); mSettings.project.ignorePaths(mIgnoredPaths); diff --git a/lib/color.cpp b/lib/color.cpp index 4e827531786..8971ab7bd31 100644 --- a/lib/color.cpp +++ b/lib/color.cpp @@ -26,16 +26,19 @@ bool gDisableColors = false; -#ifdef _WIN32 -std::ostream& operator<<(std::ostream& os, const Color& /*c*/) +#ifndef _WIN32 +static bool isStdOutATty() { -#else + // TODO: handle piping into file as well as other pipes like stderr + static const bool stdout_tty = isatty(STDOUT_FILENO); + return stdout_tty; +} +#endif + std::ostream& operator<<(std::ostream & os, const Color& c) { #ifndef _WIN32 - // TODO: handle piping into file as well as other pipes like stderr - static const bool s_is_tty = isatty(STDOUT_FILENO); - if (!gDisableColors && s_is_tty) + if (!gDisableColors && isStdOutATty()) return os << "\033[" << static_cast(c) << "m"; #else (void)c; diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 709e76cc070..64cdf061acf 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -591,9 +591,7 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm // template is given. Reformat the output according to it std::string result = templateFormat; - replaceSpecialChars(result); - replaceColors(result); findAndReplace(result, "{id}", id); std::string::size_type pos1 = result.find("{inconclusive:"); @@ -636,9 +634,6 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm for (const FileLocation &fileLocation : callStack) { std::string text = templateLocation; - replaceSpecialChars(text); - - replaceColors(text); findAndReplace(text, "{file}", fileLocation.getfile()); findAndReplace(text, "{line}", MathLib::toString(fileLocation.line)); findAndReplace(text, "{column}", MathLib::toString(fileLocation.column)); @@ -905,3 +900,15 @@ std::string replaceStr(std::string s, const std::string &from, const std::string } return s; } + +void substituteTemplateFormatStatic(std::string& templateFormat) +{ + replaceSpecialChars(templateFormat); + replaceColors(templateFormat); +} + +void substituteTemplateLocationStatic(std::string& templateLocation) +{ + replaceSpecialChars(templateLocation); + replaceColors(templateLocation); +} diff --git a/lib/errorlogger.h b/lib/errorlogger.h index 6fff2b7e630..b47baff77ec 100644 --- a/lib/errorlogger.h +++ b/lib/errorlogger.h @@ -278,6 +278,12 @@ class CPPCHECKLIB ErrorLogger { /** Replace substring. Example replaceStr("1,NR,3", "NR", "2") => "1,2,3" */ std::string replaceStr(std::string s, const std::string &from, const std::string &to); +/** replaces the static parts of the location template **/ +CPPCHECKLIB void substituteTemplateFormatStatic(std::string& templateFormat); + +/** replaces the static parts of the location template **/ +CPPCHECKLIB void substituteTemplateLocationStatic(std::string& templateLocation); + /// @} //--------------------------------------------------------------------------- #endif // errorloggerH diff --git a/test/testcmdlineparser.cpp b/test/testcmdlineparser.cpp index 36776694044..4f3f88cac8b 100644 --- a/test/testcmdlineparser.cpp +++ b/test/testcmdlineparser.cpp @@ -17,6 +17,7 @@ */ #include "cmdlineparser.h" +#include "color.h" #include "cppcheckexecutor.h" #include "errortypes.h" #include "platform.h" @@ -1264,7 +1265,7 @@ class TestCmdlineParser : public TestFixture { const char * const argv[] = {"cppcheck", "--template", "gcc", "file.cpp"}; settings.templateFormat.clear(); ASSERT(defParser.parseFromArgs(4, argv)); - ASSERT_EQUALS("{bold}{file}:{line}:{column}: {magenta}warning:{default} {message} [{id}]{reset}\\n{code}", settings.templateFormat); + ASSERT_EQUALS("{file}:{line}:{column}: warning: {message} [{id}]\n{code}", settings.templateFormat); ASSERT_EQUALS("", GET_REDIRECT_OUTPUT); } diff --git a/test/testerrorlogger.cpp b/test/testerrorlogger.cpp index c6ad97857fb..368fb91dcdf 100644 --- a/test/testerrorlogger.cpp +++ b/test/testerrorlogger.cpp @@ -16,6 +16,7 @@ * along with this program. If not, see . */ +#include "color.h" #include "config.h" #include "cppcheck.h" #include "errorlogger.h" @@ -63,6 +64,8 @@ class TestErrorLogger : public TestFixture { TEST_CASE(SerializeFileLocation); TEST_CASE(suppressUnmatchedSuppressions); + TEST_CASE(substituteTemplateFormatStatic); + TEST_CASE(substituteTemplateLocationStatic); } void TestPatternSearchReplace(const std::string& idPlaceholder, const std::string& id) const { @@ -481,6 +484,69 @@ class TestErrorLogger : public TestFixture { reportUnmatchedSuppressions(suppressions); ASSERT_EQUALS("[a.c:10]: (information) Unmatched suppression: abc\n", errout.str()); } + + void substituteTemplateFormatStatic() + { + { + std::string s; + ::substituteTemplateFormatStatic(s); + ASSERT_EQUALS("", s); + } + { + std::string s = "template{black}\\z"; + ::substituteTemplateFormatStatic(s); + ASSERT_EQUALS("template{black}\\z", s); + } + { + std::string s = "{reset}{bold}{dim}{red}{blue}{magenta}{default}\\b\\n\\r\\t"; + ::substituteTemplateFormatStatic(s); + ASSERT_EQUALS("\b\n\r\t", s); + } + { + std::string s = "\\\\n"; + ::substituteTemplateFormatStatic(s); + ASSERT_EQUALS("\\\n", s); + } + { + std::string s = "{{red}"; + ::substituteTemplateFormatStatic(s); + ASSERT_EQUALS("{", s); + } + } + + void substituteTemplateLocationStatic() + { + { + std::string s; + ::substituteTemplateLocationStatic(s); + ASSERT_EQUALS("", s); + } + { + std::string s = "template"; + ::substituteTemplateLocationStatic(s); + ASSERT_EQUALS("template", s); + } + { + std::string s = "template{black}\\z"; + ::substituteTemplateFormatStatic(s); + ASSERT_EQUALS("template{black}\\z", s); + } + { + std::string s = "{reset}{bold}{dim}{red}{blue}{magenta}{default}\\b\\n\\r\\t"; + ::substituteTemplateFormatStatic(s); + ASSERT_EQUALS("\b\n\r\t", s); + } + { + std::string s = "\\\\n"; + ::substituteTemplateFormatStatic(s); + ASSERT_EQUALS("\\\n", s); + } + { + std::string s = "{{red}"; + ::substituteTemplateFormatStatic(s); + ASSERT_EQUALS("{", s); + } + } }; REGISTER_TEST(TestErrorLogger) From af88d89568dff3a1e3deeb3e81b40c7141aa7145 Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Mar 2023 11:57:41 +0100 Subject: [PATCH 05/13] library.cpp: avoid redundant `tinyxml2::XMLElement::GetText()` calls as they perform a lookup --- lib/library.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/library.cpp b/lib/library.cpp index e5b08e463a6..c33cdc97be9 100644 --- a/lib/library.cpp +++ b/lib/library.cpp @@ -678,9 +678,10 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co for (const tinyxml2::XMLElement *functionnode = node->FirstChildElement(); functionnode; functionnode = functionnode->NextSiblingElement()) { const std::string functionnodename = functionnode->Name(); if (functionnodename == "noreturn") { - if (strcmp(functionnode->GetText(), "false") == 0) + const char * const text = functionnode->GetText(); + if (strcmp(text, "false") == 0) mNoReturn[name] = FalseTrueMaybe::False; - else if (strcmp(functionnode->GetText(), "maybe") == 0) + else if (strcmp(text, "maybe") == 0) mNoReturn[name] = FalseTrueMaybe::Maybe; else mNoReturn[name] = FalseTrueMaybe::True; // Safe @@ -757,9 +758,9 @@ Library::Error Library::loadFunction(const tinyxml2::XMLElement * const node, co // Validate the validation expression const char *p = argnode->GetText(); if (!isCompliantValidationExpression(p)) - return Error(ErrorCode::BAD_ATTRIBUTE_VALUE, (!p ? "\"\"" : argnode->GetText())); + return Error(ErrorCode::BAD_ATTRIBUTE_VALUE, (!p ? "\"\"" : p)); // Set validation expression - ac.valid = argnode->GetText(); + ac.valid = p; } else if (argnodename == "minsize") { const char *typeattr = argnode->Attribute("type"); From e888aaf38af4031120711b5a381ef62c19c9088f Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Mar 2023 12:07:43 +0100 Subject: [PATCH 06/13] errorlogger.cpp: extracted replacement logic into separate function --- lib/errorlogger.cpp | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 64cdf061acf..344789cd6aa 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -536,20 +536,8 @@ static void replaceSpecialChars(std::string& source) } } -static void replaceColors(std::string& source) +static void replace(std::string& source, const std::unordered_map &substitutionMap) { - static const std::unordered_map substitutionMap = - { - {"{reset}", ::toString(Color::Reset)}, - {"{bold}", ::toString(Color::Bold)}, - {"{dim}", ::toString(Color::Dim)}, - {"{red}", ::toString(Color::FgRed)}, - {"{green}", ::toString(Color::FgGreen)}, - {"{blue}", ::toString(Color::FgBlue)}, - {"{magenta}", ::toString(Color::FgMagenta)}, - {"{default}", ::toString(Color::FgDefault)}, - }; - std::string::size_type index = 0; while ((index = source.find('{', index)) != std::string::npos) { const std::string::size_type end = source.find('}', index); @@ -567,6 +555,21 @@ static void replaceColors(std::string& source) } } +static void replaceColors(std::string& source) { + static const std::unordered_map substitutionMap = + { + {"{reset}", ::toString(Color::Reset)}, + {"{bold}", ::toString(Color::Bold)}, + {"{dim}", ::toString(Color::Dim)}, + {"{red}", ::toString(Color::FgRed)}, + {"{green}", ::toString(Color::FgGreen)}, + {"{blue}", ::toString(Color::FgBlue)}, + {"{magenta}", ::toString(Color::FgMagenta)}, + {"{default}", ::toString(Color::FgDefault)}, + }; + replace(source, substitutionMap); +} + std::string ErrorMessage::toString(bool verbose, const std::string &templateFormat, const std::string &templateLocation) const { // Save this ErrorMessage in plain text. From a09098e626ebe4d8986687fbb88e6835f2f12029 Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Mar 2023 12:24:01 +0100 Subject: [PATCH 07/13] errorlogger.cpp: use substitution map when no callstack exists --- lib/errorlogger.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/errorlogger.cpp b/lib/errorlogger.cpp index 344789cd6aa..7cc26c58bf2 100644 --- a/lib/errorlogger.cpp +++ b/lib/errorlogger.cpp @@ -626,11 +626,15 @@ std::string ErrorMessage::toString(bool verbose, const std::string &templateForm findAndReplace(result, "{code}", readCode(callStack.back().getOrigFile(), callStack.back().line, callStack.back().column, endl)); } } else { - findAndReplace(result, "{callstack}", emptyString); - findAndReplace(result, "{file}", "nofile"); - findAndReplace(result, "{line}", "0"); - findAndReplace(result, "{column}", "0"); - findAndReplace(result, "{code}", emptyString); + static const std::unordered_map callStackSubstitutionMap = + { + {"{callstack}", ""}, + {"{file}", "nofile"}, + {"{line}", "0"}, + {"{column}", "0"}, + {"{code}", ""} + }; + replace(result, callStackSubstitutionMap); } if (!templateLocation.empty() && callStack.size() >= 2U) { From 8c8f23092d788947a24d3235309220d9e5a2af23 Mon Sep 17 00:00:00 2001 From: firewave Date: Thu, 2 Mar 2023 14:33:41 +0100 Subject: [PATCH 08/13] improved testing of `--template*` options / added TODOs --- cli/cmdlineparser.cpp | 6 +- test/testcmdlineparser.cpp | 130 ++++++++++++++++++++++++++++++++++++- 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/cli/cmdlineparser.cpp b/cli/cmdlineparser.cpp index 28f8bdd44e6..1366f1d0613 100644 --- a/cli/cmdlineparser.cpp +++ b/cli/cmdlineparser.cpp @@ -888,6 +888,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[]) } } + // TODO: deprecate "--template