[cmd] Parse command exits with error in case of duplicated suppress c… (

#3253)

Parse command exits with error in case of duplicated suppress commands

analyzer/codechecker_analyzer/cmd/parse.py

@@ -220,6 +220,12 @@ def write(self,
                                 self.src_comment_handler,
                                 self.src_comment_status_filter)
 
+                if self.src_comment_handler and source_code_comments:
+                    self.src_comment_handler.store_suppress_bug_id(
+                        report_hash, os.path.basename(source_file),
+                        source_code_comments[0]['message'],
+                        source_code_comments[0]['status'])
+
                 if skip:
                     continue
 
@@ -353,28 +359,23 @@ def skip_report(report_hash, source_file, report_line, checker_name,
         skip = True if src_comment_status_filter and \
             'unreviewed' not in src_comment_status_filter else False
         return skip, src_comment_data
-    elif len(src_comment_data) == 1:
+
+    num_of_suppress_comments = len(src_comment_data)
+    if num_of_suppress_comments == 1:
         status = src_comment_data[0]['status']
 
         LOG.debug("Suppressed by source code comment.")
-        if src_comment_handler:
-            file_name = os.path.basename(source_file)
-            message = src_comment_data[0]['message']
-            src_comment_handler.store_suppress_bug_id(
-                report_hash,
-                file_name,
-                message,
-                status)
 
         if src_comment_status_filter and \
                 status not in src_comment_status_filter:
             return True, src_comment_data
 
-    elif len(src_comment_data) > 1:
-        LOG.warning("Multiple source code comment can be found "
-                    "for '%s' checker in '%s' at line %d. "
-                    "This bug will not be suppressed!",
-                    checker_name, source_file, report_line)
+    if num_of_suppress_comments > 1:
+        LOG.error("Multiple source code comment can be found "
+                  "for '%s' checker in '%s' at line %d.",
+                  checker_name, source_file, report_line)
+        sys.exit(1)
+
     return False, src_comment_data
 
 
@@ -713,7 +714,7 @@ def main(args):
     elif 'create_suppress' in args:
         LOG.error("Can't use '--export-source-suppress' unless '--suppress "
                   "SUPPRESS_FILE' is also given.")
-        sys.exit(2)
+        sys.exit(1)
 
     processed_path_hashes = set()
 
@@ -793,6 +794,12 @@ def skip_html_report_data_handler(report_hash, source_file, report_line,
                                                  suppr_handler,
                                                  src_comment_status_filter)
 
+        if suppr_handler and source_code_comments:
+            suppr_handler.store_suppress_bug_id(
+                report_hash, os.path.basename(source_file),
+                source_code_comments[0]['message'],
+                source_code_comments[0]['status'])
+
         if skip_handler:
             skip |= skip_handler.should_skip(source_file)
 

analyzer/tests/functional/suppress/__init__.py

@@ -13,7 +13,6 @@
 import os
 import shutil
 import sys
-import uuid
 
 from libtest import codechecker
 from libtest import env

analyzer/tests/functional/suppress/duplicated_suppress.skip

@@ -0,0 +1,2 @@
++*duplicated_suppress.cpp
+-*

analyzer/tests/functional/suppress/suppress.expected

@@ -0,0 +1,8 @@
+0c07579523063acece2d7aebd4357cac||suppress_export.cpp||foo1 simple||false_positive
+58b0efedaf67e081290e27c6caabce3f||suppress_export.cpp||foo2 simple||false_positive
+0b4a424e23295c1f133bf643006a8c1e||suppress_export.cpp||foo2 simple||false_positive
+4c3ca4953cad12c05eebfe6a71833880||suppress_export.cpp||foo3 simple||intentional
+d1551d7a120c30fadec6ac2e416705f5||suppress_export.cpp||foo3 simple||intentional
+26806cbc2fd851b977e4915eba75289e||suppress_export.cpp||foo4 simple||confirmed
+b14c948e6ae8e27e3a33b0ca898e98f1||suppress_export.cpp||foo4 simple||confirmed
+e5f1b57c0f5d41e95b1eb5bf2032aada||suppress_export.cpp||substring of the checker name||confirmed

analyzer/tests/functional/suppress/suppress_already_confirmed.skip

@@ -0,0 +1,2 @@
++*suppress_already_confirmed.cpp
+-*

analyzer/tests/functional/suppress/suppress_by_all.skip

@@ -0,0 +1,2 @@
++*suppress_duplicated_by_all.cpp
+-*

analyzer/tests/functional/suppress/suppress_by_all_in_two_lines.skip

@@ -0,0 +1,2 @@
++*suppress_duplicated_in_two_lines.cpp
+-*

analyzer/tests/functional/suppress/suppress_export.skip

@@ -0,0 +1,2 @@
++*suppress_export.cpp
+-*

analyzer/tests/functional/suppress/suppress_without_bug.skip

@@ -0,0 +1,2 @@
++*suppress_without_bug.cpp
+-*

analyzer/tests/functional/suppress/test_suppress.py

@@ -0,0 +1,165 @@
+# -------------------------------------------------------------------------
+#
+#  Part of the CodeChecker project, under the Apache License v2.0 with
+#  LLVM Exceptions. See LICENSE for license information.
+#  SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+#
+# -------------------------------------------------------------------------
+"""
+Test source-code level suppression data writing to suppress file.
+"""
+
+
+import os
+import inspect
+import unittest
+
+from libtest import env, codechecker
+
+
+class TestSuppress(unittest.TestCase):
+    """
+    Test source-code level suppression data writing to suppress file.
+    """
+
+    def setUp(self):
+        self._test_workspace = os.environ['TEST_WORKSPACE']
+
+        self._testproject_data = env.setup_test_proj_cfg(self._test_workspace)
+        self.assertIsNotNone(self._testproject_data)
+
+        self._test_project_path = self._testproject_data['project_path']
+        self._test_directory = os.path.dirname(os.path.abspath(inspect.getfile(
+            inspect.currentframe())))
+
+    def test_source_suppress_export(self):
+        """
+        Test exporting a source suppress comment automatically to file.
+        """
+
+        generated_file = os.path.join(self._test_workspace,
+                                      "generated.suppress")
+        skip_file = os.path.join(self._test_directory, "suppress_export.skip")
+
+        extract_cmd = [env.codechecker_cmd(), 'parse',
+                       os.path.join(self._test_workspace, "reports"),
+                       "--suppress", generated_file,
+                       "--export-source-suppress", "--ignore", skip_file]
+
+        _, _, ret = codechecker.call_command(
+            extract_cmd, self._test_project_path,
+            env.test_env(self._test_directory))
+        self.assertEqual(ret, 2, "Failed to generate suppress file.")
+
+        with open(generated_file, 'r',
+                  encoding='utf-8', errors='ignore') as generated:
+            expected_file = os.path.join(self._test_directory,
+                                         "suppress.expected")
+            with open(expected_file, 'r', encoding='utf-8',
+                      errors='ignore') as expected:
+                generated_content = generated.read()
+                expected_content = expected.read()
+                print("generated")
+                print(generated_content)
+                print("expected")
+                print(expected_content)
+
+                diff = set(expected_content).symmetric_difference(
+                           generated_content)
+                print("difference")
+                {print(elem) for elem in diff}
+                self.assertEqual(len(diff),
+                                 0,
+                                 "The generated suppress file does not "
+                                 "look like what was expected")
+
+    def test_doubled_suppress(self):
+        """
+        Test to catch repeated suppress comments with same bug.
+        """
+
+        skip_file = os.path.join(self._test_directory,
+                                 "duplicated_suppress.skip")
+
+        cmd = [env.codechecker_cmd(), 'parse',
+               os.path.join(self._test_workspace, "reports"),
+               "--ignore", skip_file]
+
+        _, _, ret = codechecker.call_command(
+            cmd, self._test_project_path,
+            env.test_env(self._test_workspace))
+        self.assertEqual(ret, 1, "Repeated suppress comment not recognized.")
+
+    def test_doubled_suppress_by_all(self):
+        """
+        Test to catch multiple suppress comments in a line when "all"
+        is one of them.
+        """
+
+        skip_file = os.path.join(self._test_directory, "suppress_by_all.skip")
+
+        cmd = [env.codechecker_cmd(), 'parse',
+               os.path.join(self._test_workspace, "reports"),
+               "--ignore", skip_file]
+
+        _, _, ret = codechecker.call_command(
+            cmd, self._test_project_path,
+            env.test_env(self._test_workspace))
+        self.assertEqual(ret, 1, "Already covered suppress comment not "
+                         "recognized.")
+
+    def test_doubled_suppress_by_all_in_two_lines(self):
+        """
+        Test to catch unnecessary suppress comment that was covered by a
+        suppress all comment in the previous line.
+        """
+
+        skip_file = os.path.join(self._test_directory,
+                                 "suppress_by_all_in_two_lines.skip")
+
+        cmd = [env.codechecker_cmd(), 'parse',
+               os.path.join(self._test_workspace, "reports"),
+               "--ignore", skip_file]
+
+        _, _, ret = codechecker.call_command(
+            cmd, self._test_project_path,
+            env.test_env(self._test_workspace))
+        self.assertEqual(ret, 1, "Already covered suppress comment not "
+                         "recognized.")
+
+    def test_confirmed_already_suppressed(self):
+        """
+        Test to catch unnecessary confirmed comment that was covered by a
+        suppress all comment in the previous line.
+        """
+
+        skip_file = os.path.join(self._test_directory,
+                                 "suppress_already_confirmed.skip")
+
+        cmd = [env.codechecker_cmd(), 'parse',
+               os.path.join(self._test_workspace, "reports"),
+               "--ignore", skip_file]
+
+        _, _, ret = codechecker.call_command(
+            cmd, self._test_project_path,
+            env.test_env(self._test_workspace))
+        self.assertEqual(ret, 1, "Already suppressed comment must not be "
+                         "confirmed.")
+
+    def test_suppress_with_no_bug_is_ok(self):
+        """
+        Test that the suppress comment that suppresses non existent bug does
+        not cause fail.
+        """
+
+        skip_file = os.path.join(self._test_directory,
+                                 "suppress_without_bug.skip")
+
+        cmd = [env.codechecker_cmd(), 'parse',
+               os.path.join(self._test_workspace, "reports"),
+               "--ignore", skip_file]
+
+        _, _, ret = codechecker.call_command(
+            cmd, self._test_project_path,
+            env.test_env(self._test_workspace))
+        self.assertEqual(ret, 0, "Suppress without existent bug causes error.")

analyzer/tests/projects/suppress/Makefile

@@ -1,4 +1,7 @@
 all:
-	$(CXX) suppress.cpp
+	$(CXX) suppress_export.cpp duplicated_suppress.cpp \
+		suppress_duplicated_by_all.cpp suppress_duplicated_in_two_lines.cpp \
+		suppress_already_confirmed.cpp suppress_without_bug.cpp
+
 clean:
 	rm a.out

analyzer/tests/projects/suppress/duplicated_suppress.cpp

@@ -0,0 +1,9 @@
+int duplicatedSuppress();
+
+int duplicatedSuppress()
+{
+    // codechecker_suppress [bugprone-sizeof-expression] Same suppress comment twice.
+    // codechecker_suppress [core.DivideZero] Unmatching suppress comment.
+    // codechecker_suppress [bugprone-sizeof-expression] Same suppress comment twice.
+    return sizeof(42);
+}

analyzer/tests/projects/suppress/suppress_already_confirmed.cpp

@@ -0,0 +1,6 @@
+void suppressAlreadyDoneWithAnotherSuppressionType()
+{
+  // codechecker_suppress [clang-diagnostic-unused-value, bugprone-sizeof-expression] Suppress two bugs in the next C/C++ statement
+  // codechecker_confirmed [bugprone-sizeof-expression] Already suppressed by the previous comment above
+  sizeof(48);
+}

analyzer/tests/projects/suppress/suppress_duplicated_by_all.cpp

@@ -0,0 +1,5 @@
+void suppressDuplicatedByAll()
+{
+  // codechecker_suppress [all, bugprone-sizeof-expression] Comment already have included "all". The "bugprone sizeof expression" is an unnecessary duplication.
+  sizeof(45);
+}