fixed #14276 - report inline suppressions with invalid error IDs

Author: firewave

lib/preprocessor.cpp

@@ -179,6 +179,15 @@ static std::string getRelativeFilename(const simplecpp::Token* tok, const Settin
     return Path::simplifyPath(std::move(relativeFilename));
 }
 
+static void addInlineSuppression(SuppressionList::Suppression suppr, SuppressionList &suppressions, std::list<BadInlineSuppression> &bad)
+{
+    const std::string file = suppr.fileName;
+    const int line = suppr.lineNumber;
+    const std::string errmsg = suppressions.addSuppression(std::move(suppr));
+    if (!errmsg.empty())
+        bad.emplace_back(file, line, 0, errmsg); // TODO: set column
+}
+
 static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Settings &settings, SuppressionList &suppressions, std::list<BadInlineSuppression> &bad)
 {
     std::list<SuppressionList::Suppression> inlineSuppressionsBlockBegin;
@@ -257,7 +266,7 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
                             suppr.lineNumber = supprBegin->lineNumber;
                             suppr.type = SuppressionList::Type::block;
                             inlineSuppressionsBlockBegin.erase(supprBegin);
-                            suppressions.addSuppression(std::move(suppr)); // TODO: check result
+                            addInlineSuppression(std::move(suppr), suppressions, bad);
                             throwError = false;
                             break;
                         }
@@ -282,10 +291,10 @@ static void addInlineSuppressions(const simplecpp::TokenList &tokens, const Sett
                 suppr.thisAndNextLine = thisAndNextLine;
                 suppr.lineNumber = tok->location.line;
                 suppr.macroName = macroName;
-                suppressions.addSuppression(std::move(suppr)); // TODO: check result
+                addInlineSuppression(std::move(suppr), suppressions, bad);
             } else if (SuppressionList::Type::file == suppr.type) {
                 if (onlyComments)
-                    suppressions.addSuppression(std::move(suppr)); // TODO: check result
+                    addInlineSuppression(std::move(suppr), suppressions, bad);
                 else
                     bad.emplace_back(suppr.fileName, suppr.lineNumber, 0, "File suppression should be at the top of the file"); // TODO: set column
             }

test/cli/inline-suppress_test.py

@@ -36,7 +36,9 @@ def test_1():
         'proj-inline-suppress'
     ]
     ret, stdout, stderr = cppcheck(args, cwd=__script_dir)
-    assert stderr == ''
+    assert stderr.splitlines() == [
+        "{}duplicate.cpp:3:0: error: suppression 'unreadVariable:proj-inline-suppress/duplicate.cpp:3' already exists [invalidSuppression]".format(__proj_inline_suppres_path)
+    ]
     assert stdout == ''
     assert ret == 0, stdout
 

test/testsuppressions.cpp

@@ -703,6 +703,39 @@ class TestSuppressions : public TestFixture {
                       "[test.cpp:3:5]: (error) Uninitialized variable: a [uninitvar]\n"
                       "[test.cpp:5:5]: (error) Uninitialized variable: b [uninitvar]\n", errout_str());
 
+        ASSERT_EQUALS(1, (this->*check)("// cppcheck-suppress-file :id0\n"
+                                        "// cppcheck-suppress :id\n"
+                                        "// cppcheck-suppress [:id1,id2]\n"
+                                        "// cppcheck-suppress-begin :id3\n"
+                                        "void f() {}\n"
+                                        "// cppcheck-suppress-end :id3\n",
+                                        ""));
+        ASSERT_EQUALS("[test.cpp:1:0]: (error) Failed to add suppression. Invalid id \":id0\" [invalidSuppression]\n"
+                      "[test.cpp:5:0]: (error) Failed to add suppression. Invalid id \":id\" [invalidSuppression]\n" // TODO: should we report the location of the suppression instead?
+                      "[test.cpp:5:0]: (error) Failed to add suppression. Invalid id \":id1\" [invalidSuppression]\n" // TODO: should we report the location of the suppression instead?
+                      "[test.cpp:4:0]: (error) Failed to add suppression. Invalid id \":id3\" [invalidSuppression]\n"
+                      "[test.cpp:5:0]: (information) Unmatched suppression: id2 [unmatchedSuppression]\n", errout_str()); // TODO: should we report the location of the suppression instead?
+
+        ASSERT_EQUALS(1, (this->*check)("// cppcheck-suppress-file id\n"
+                                        "// cppcheck-suppress-file [id,id0,id0]\n" // TODO: duplicated suppression "id" not detected
+                                        "// cppcheck-suppress id1\n"
+                                        "// cppcheck-suppress id1\n"
+                                        "// cppcheck-suppress [id1,id2,id2]\n"
+                                        "// cppcheck-suppress-begin [id3,id3]\n"
+                                        "void f() {}\n"
+                                        "// cppcheck-suppress-end [id3,id3]\n",
+                                        ""));
+        ASSERT_EQUALS("[test.cpp:2:0]: (error) suppression 'id0:test.cpp:2' already exists [invalidSuppression]\n"
+                      "[test.cpp:7:0]: (error) suppression 'id1:test.cpp:7' already exists [invalidSuppression]\n" // TODO: should we report the location of the suppression instead?
+                      "[test.cpp:7:0]: (error) suppression 'id2:test.cpp:7' already exists [invalidSuppression]\n" // TODO: should we report the location of the suppression instead?
+                      "[test.cpp:6:0]: (error) suppression 'id3:test.cpp:6' already exists [invalidSuppression]\n"
+                      "[test.cpp:1:0]: (information) Unmatched suppression: id [unmatchedSuppression]\n"
+                      "[test.cpp:2:0]: (information) Unmatched suppression: id [unmatchedSuppression]\n" // TODO: should not be reported
+                      "[test.cpp:2:0]: (information) Unmatched suppression: id0 [unmatchedSuppression]\n" // TODO: should we report the location of the suppression instead?
+                      "[test.cpp:7:0]: (information) Unmatched suppression: id1 [unmatchedSuppression]\n" // TODO: should we report the location of the suppression instead?
+                      "[test.cpp:7:0]: (information) Unmatched suppression: id2 [unmatchedSuppression]\n" // TODO: should we report the location of the suppression instead?
+                      "[test.cpp:6:0]: (information) Unmatched suppression: id3 [unmatchedSuppression]\n", errout_str());
+
         ASSERT_EQUALS(1, (this->*check)("void f() {\n"
                                         "    int a;\n"
                                         "    // cppcheck-suppress-begin uninitvar\n"