Refactoring: Settings::addSuppression return error message and callers make sure it's reported properly. Ticket: #1839

lib/cppcheck.cpp

@@ -285,7 +285,12 @@ bool CppCheck::parseFromArgs(int argc, const char* const argv[])
                 reportOut("cppcheck: Couldn't open the file \"" + std::string(argv[i]) + "\"");
                 return false;
             }
-            _settings.nomsg.parseFile(f);
+            const std::string errmsg(_settings.nomsg.parseFile(f));
+            if (!errmsg.empty())
+            {
+                reportOut(errmsg);
+                return false;
+            }
         }
 
         // Filter errors
@@ -305,7 +310,12 @@ bool CppCheck::parseFromArgs(int argc, const char* const argv[])
                 reportOut("cppcheck: Couldn't open the file \"" + std::string(argv[i]) + "\"");
                 return false;
             }
-            _settings.nofail.parseFile(f);
+            const std::string errmsg(_settings.nofail.parseFile(f));
+            if (!errmsg.empty())
+            {
+                reportOut(errmsg);
+                return false;
+            }
         }
 
         // Enables inline suppressions.

lib/preprocessor.cpp

@@ -238,7 +238,13 @@ std::string Preprocessor::removeComments(const std::string &str, const std::stri
         {
             // Add the suppressions.
             for (size_t j(0); j < suppressionIDs.size(); ++j)
-                settings->nomsg.addSuppression(suppressionIDs[j], filename, lineno);
+            {
+                const std::string errmsg(settings->nomsg.addSuppression(suppressionIDs[j], filename, lineno));
+                if (!errmsg.empty())
+                {
+                    writeError(filename, lineno, _errorLogger, "cppcheckError", errmsg);
+                }
+            }
             suppressionIDs.clear();
         }
 

lib/settings.cpp

@@ -43,7 +43,7 @@ Settings::Settings()
     test_2_pass = false;
 }
 
-bool Settings::Suppressions::parseFile(std::istream &istr)
+std::string Settings::Suppressions::parseFile(std::istream &istr)
 {
     // Change '\r' to '\n' in the istr
     std::string filedata;
@@ -53,8 +53,6 @@ bool Settings::Suppressions::parseFile(std::istream &istr)
     while (filedata.find("\r") != std::string::npos)
         filedata[filedata.find("\r")] = '\n';
 
-    bool ret = true;
-
     // Parse filedata..
     std::istringstream istr2(filedata);
     while (std::getline(istr2, line))
@@ -76,38 +74,37 @@ bool Settings::Suppressions::parseFile(std::istream &istr)
         }
 
         // We could perhaps check if the id is valid and return error if it is not
-        ret &= addSuppression(id, file, lineNumber);
+        const std::string errmsg(addSuppression(id, file, lineNumber));
+        if (!errmsg.empty())
+            return errmsg;
     }
 
-    return ret;
+    return "";
 }
 
-bool Settings::Suppressions::addSuppression(const std::string &errorId, const std::string &file, unsigned int line)
+std::string Settings::Suppressions::addSuppression(const std::string &errorId, const std::string &file, unsigned int line)
 {
     // Check that errorId is valid..
     if (errorId.empty())
     {
-        std::cerr << "Failed to add suppression. No id." << std::endl;
+        return "Failed to add suppression. No id.";
-        return false;
     }
     for (std::string::size_type pos = 0; pos < errorId.length(); ++pos)
     {
         if (errorId[pos] < 0 || !std::isalnum(errorId[pos]))
         {
-            std::cerr << "Failed to add suppression. Invalid id \"" << errorId << "\"" << std::endl;
+            return "Failed to add suppression. Invalid id \"" + errorId + "\"";
-            return false;
         }
         if (pos == 0 && std::isdigit(errorId[pos]))
         {
-            std::cerr << "Failed to add suppression. Invalid id \"" << errorId << "\"" << std::endl;
+            return "Failed to add suppression. Invalid id \"" + errorId + "\"";
-            return false;
         }
     }
 
     _suppressions[errorId][file].push_back(line);
     _suppressions[errorId][file].sort();
 
-    return true;
+    return "";
 }
 
 bool Settings::Suppressions::isSuppressed(const std::string &errorId, const std::string &file, unsigned int line)

lib/settings.h

@@ -136,9 +136,9 @@ public:
         /**
          * @brief Don't show errors listed in the file.
          * @param istr Open file stream where errors can be read.
-         * @return true on success, false in syntax error is noticed.
+         * @return error message. empty upon success
          */
-        bool parseFile(std::istream &istr);
+        std::string parseFile(std::istream &istr);
 
         /**
          * @brief Don't show this error. If file and/or line are optional. In which case
@@ -146,9 +146,9 @@ public:
          * @param errorId the id for the error, e.g. "arrayIndexOutOfBounds"
          * @param file File name with the path, e.g. "src/main.cpp"
          * @param line number, e.g. "123"
-         * @return true on success, false in syntax error is noticed.
+         * @return error message. empty upon success
          */
-        bool addSuppression(const std::string &errorId, const std::string &file = "", unsigned int line = 0);
+        std::string addSuppression(const std::string &errorId, const std::string &file = "", unsigned int line = 0);
 
         /**
          * @brief Returns true if this message should not be shown to the user.

test/testsettings.cpp

@@ -41,14 +41,14 @@ private:
     {
         Settings::Suppressions suppressions;
         std::istringstream s("123");
-        ASSERT_EQUALS(false, suppressions.parseFile(s));
+        ASSERT_EQUALS("Failed to add suppression. Invalid id \"123\"", suppressions.parseFile(s));
     }
 
     void suppressionsDosFormat()
     {
         Settings::Suppressions suppressions;
         std::istringstream s("abc\r\ndef\r\n");
-        ASSERT_EQUALS(true, suppressions.parseFile(s));
+        ASSERT_EQUALS("", suppressions.parseFile(s));
         ASSERT_EQUALS(true, suppressions.isSuppressed("abc", "test.cpp", 1));
         ASSERT_EQUALS(true, suppressions.isSuppressed("def", "test.cpp", 1));
     }