Unit Testing: Test that suppressions work

Makefile

@@ -52,6 +52,7 @@ TESTOBJ =     test/testautovariables.o \
               test/testpreprocessor.o \
               test/testredundantif.o \
               test/testrunner.o \
+              test/testsettings.o \
               test/testsimplifytokens.o \
               test/teststl.o \
               test/testsuite.o \
@@ -213,6 +214,9 @@ test/testredundantif.o: test/testredundantif.cpp lib/tokenize.h lib/classinfo.h
 test/testrunner.o: test/testrunner.cpp test/testsuite.h lib/errorlogger.h lib/settings.h
 	$(CXX) $(CXXFLAGS) -Ilib -Icli -c -o test/testrunner.o test/testrunner.cpp
 
+test/testsettings.o: test/testsettings.cpp lib/settings.h test/testsuite.h lib/errorlogger.h
+	$(CXX) $(CXXFLAGS) -Ilib -Icli -c -o test/testsettings.o test/testsettings.cpp
+
 test/testsimplifytokens.o: test/testsimplifytokens.cpp test/testsuite.h lib/errorlogger.h lib/settings.h lib/tokenize.h lib/classinfo.h lib/token.h
 	$(CXX) $(CXXFLAGS) -Ilib -Icli -c -o test/testsimplifytokens.o test/testsimplifytokens.cpp
 

lib/settings.cpp

@@ -53,6 +53,8 @@ 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))
@@ -74,36 +76,38 @@ bool Settings::Suppressions::parseFile(std::istream &istr)
         }
 
         // We could perhaps check if the id is valid and return error if it is not
-        addSuppression(id, file, lineNumber);
+        ret &= addSuppression(id, file, lineNumber);
     }
 
-    return true;
+    return ret;
 }
 
-void Settings::Suppressions::addSuppression(const std::string &errorId, const std::string &file, unsigned int line)
+bool 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;
+        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;
+            return false;
         }
         if (pos == 0 && std::isdigit(errorId[pos]))
         {
             std::cerr << "Failed to add suppression. Invalid id \"" << errorId << "\"" << std::endl;
-            return;
+            return false;
         }
     }
 
     _suppressions[errorId][file].push_back(line);
     _suppressions[errorId][file].sort();
+
+    return true;
 }
 
 bool Settings::Suppressions::isSuppressed(const std::string &errorId, const std::string &file, unsigned int line)

lib/settings.h

@@ -145,8 +145,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.
          */
-        void addSuppression(const std::string &errorId, const std::string &file = "", unsigned int line = 0);
+        bool 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

@@ -0,0 +1,57 @@
+/*
+ * Cppcheck - A tool for static C/C++ code analysis
+ * Copyright (C) 2007-2010 Daniel Marjamäki and Cppcheck team.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "settings.h"
+#include "testsuite.h"
+
+#include <sstream>
+
+extern std::ostringstream errout;
+
+class TestSettings : public TestFixture
+{
+public:
+    TestSettings() : TestFixture("TestSettings")
+    { }
+
+private:
+
+    void run()
+    {
+        TEST_CASE(suppressionsBadId1);
+        TEST_CASE(suppressionsDosFormat);     // Ticket #1836
+    }
+
+    void suppressionsBadId1()
+    {
+        Settings::Suppressions suppressions;
+        std::istringstream s("123");
+        ASSERT_EQUALS(false, suppressions.parseFile(s));
+    }
+
+    void suppressionsDosFormat()
+    {
+        Settings::Suppressions suppressions;
+        std::istringstream s("abc\r\ndef\r\n");
+        ASSERT_EQUALS(true, suppressions.parseFile(s));
+        ASSERT_EQUALS(true, suppressions.isSuppressed("abc", "test.cpp", 1));
+        ASSERT_EQUALS(true, suppressions.isSuppressed("def", "test.cpp", 1));
+    }
+};
+
+REGISTER_TEST(TestSettings)