optimized template string substitutions a bit

Makefile

@@ -281,6 +281,7 @@ TESTOBJ =     test/fixture.o \
               test/testclangimport.o \
               test/testclass.o \
               test/testcmdlineparser.o \
+              test/testcolor.o \
               test/testcondition.o \
               test/testconstructors.o \
               test/testcppcheck.o \
@@ -714,6 +715,9 @@ test/testclass.o: test/testclass.cpp externals/simplecpp/simplecpp.h lib/check.h
 test/testcmdlineparser.o: test/testcmdlineparser.cpp cli/cmdlineparser.h cli/cppcheckexecutor.h lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/settings.h lib/standards.h lib/suppressions.h lib/timer.h lib/utils.h test/fixture.h test/redirect.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testcmdlineparser.cpp
 
+test/testcolor.o: test/testcolor.cpp lib/check.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/suppressions.h test/fixture.h
+	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testcolor.cpp
+
 test/testcondition.o: test/testcondition.cpp externals/simplecpp/simplecpp.h lib/check.h lib/checkcondition.h lib/color.h lib/config.h lib/errorlogger.h lib/errortypes.h lib/importproject.h lib/library.h lib/mathlib.h lib/platform.h lib/preprocessor.h lib/settings.h lib/standards.h lib/suppressions.h lib/templatesimplifier.h lib/token.h lib/tokenize.h lib/tokenlist.h lib/utils.h lib/vfvalue.h test/fixture.h
 	$(CXX) ${INCLUDE_FOR_TEST} $(CPPFLAGS) $(CXXFLAGS) -c -o $@ test/testcondition.cpp
 

cli/cmdlineparser.cpp

@@ -888,6 +888,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
                 }
             }
 
+            // TODO: deprecate "--template <template>"
             // Output formatter
             else if (std::strcmp(argv[i], "--template") == 0 ||
                      std::strncmp(argv[i], "--template=", 11) == 0) {
@@ -901,6 +902,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
                     printError("argument to '--template' is missing.");
                     return false;
                 }
+                // TODO: bail out when no placeholders are found?
 
                 if (mSettings.templateFormat == "gcc") {
                     mSettings.templateFormat = "{bold}{file}:{line}:{column}: {magenta}warning:{default} {message} [{id}]{reset}\\n{code}";
@@ -922,6 +924,7 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
                 }
             }
 
+            // TODO: deprecate "--template-location <template>"
             else if (std::strcmp(argv[i], "--template-location") == 0 ||
                      std::strncmp(argv[i], "--template-location=", 20) == 0) {
                 // "--template-location format"
@@ -931,9 +934,10 @@ bool CmdLineParser::parseFromArgs(int argc, const char* const argv[])
                     ++i;
                     mSettings.templateLocation = argv[i];
                 } else {
-                    printError("argument to '--template' is missing.");
+                    printError("argument to '--template-location' is missing.");
                     return false;
                 }
+                // TODO: bail out when no placeholders are found?
             }
 
             else if (std::strncmp(argv[i], "--template-max-time=", 20) == 0) {
@@ -1015,6 +1019,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);
 

cli/cppcheckexecutor.cpp

@@ -383,7 +383,7 @@ void CppCheckExecutor::reportOut(const std::string &outmsg, Color c)
     if (c == Color::Reset)
         std::cout << ansiToOEM(outmsg, true) << std::endl;
     else
-        std::cout << toString(c) << ansiToOEM(outmsg, true) << toString(Color::Reset) << std::endl;
+        std::cout << c << ansiToOEM(outmsg, true) << Color::Reset << std::endl;
 }
 
 void CppCheckExecutor::reportProgress(const std::string &filename, const char stage[], const std::size_t value)

lib/color.cpp

@@ -20,30 +20,46 @@
 
 #ifndef _WIN32
 #include <unistd.h>
-#endif
 #include <cstddef>
 #include <sstream> // IWYU pragma: keep
+#include <iostream>
+#endif
 
 bool gDisableColors = false;
 
-#ifdef _WIN32
-std::ostream& operator<<(std::ostream& os, const Color& /*c*/)
+#ifndef _WIN32
+static bool isStreamATty(const std::ostream & os)
 {
-#else
+    static const bool stdout_tty = isatty(STDOUT_FILENO);
+    static const bool stderr_tty = isatty(STDERR_FILENO);
+    if (&os == &std::cout)
+        return stdout_tty;
+    if (&os == &std::cerr)
+        return stderr_tty;
+    return (stdout_tty && stderr_tty);
+}
+#endif
+
 std::ostream& operator<<(std::ostream & os, const Color& c)
 {
-    // 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)
+#ifndef _WIN32
+    if (!gDisableColors && isStreamATty(os))
         return os << "\033[" << static_cast<std::size_t>(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
 }
 

lib/errorlogger.cpp

@@ -511,24 +511,64 @@ 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 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 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<char, std::string> 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 replace(std::string& source, const std::unordered_map<std::string, std::string> &substitutionMap)
+{
+    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();
+    }
+}
+
+static void replaceColors(std::string& source) {
+    // TODO: colors are not applied when either stdout or stderr is not a TTY because we resolve them before the stream usage
+    static const std::unordered_map<std::string, std::string> 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
@@ -555,14 +595,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");
 
-    replaceColors(result);
     findAndReplace(result, "{id}", id);
 
     std::string::size_type pos1 = result.find("{inconclusive:");
@@ -594,23 +627,21 @@ 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<std::string, std::string> callStackSubstitutionMap =
+        {
+            {"{callstack}",   ""},
+            {"{file}",    "nofile"},
+            {"{line}",     "0"},
+            {"{column}",     "0"},
+            {"{code}",     ""}
+        };
+        replace(result, callStackSubstitutionMap);
     }
 
     if (!templateLocation.empty() && callStack.size() >= 2U) {
         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");
-
-            replaceColors(text);
             findAndReplace(text, "{file}", fileLocation.getfile());
             findAndReplace(text, "{line}", MathLib::toString(fileLocation.line));
             findAndReplace(text, "{column}", MathLib::toString(fileLocation.column));
@@ -877,3 +908,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);
+}

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

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");