Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Jul 6, 2021
37 changes: 22 additions & 15 deletions analyzer/codechecker_analyzer/cmd/parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -712,7 +713,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()

Expand Down Expand Up @@ -792,6 +793,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)

Expand Down
1 change: 0 additions & 1 deletion analyzer/tests/functional/suppress/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import os
import shutil
import sys
import uuid

from libtest import codechecker
from libtest import env
Expand Down
2 changes: 2 additions & 0 deletions analyzer/tests/functional/suppress/duplicated_suppress.skip
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
+*duplicated_suppress.cpp
-*
8 changes: 8 additions & 0 deletions analyzer/tests/functional/suppress/suppress.expected
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
0c07579523063acece2d7aebd4357cac||suppress_export.cpp||foo1 simple||false_positive
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this file is hardly connected to source file: https://github.com/zomen2/codechecker/blob/fix_doubled_report_error/analyzer/tests/projects/suppress/suppress_export.cpp
If we change the source file we need to change this file too. So it would be better to move this file back to the original location beside the source file.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this moment the test source does not allow this. A large refactor should be performed on function test code before your suggestion can be done.

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
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
+*suppress_already_confirmed.cpp
-*
2 changes: 2 additions & 0 deletions analyzer/tests/functional/suppress/suppress_by_all.skip
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
+*suppress_duplicated_by_all.cpp
-*
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
+*suppress_duplicated_in_two_lines.cpp
-*
2 changes: 2 additions & 0 deletions analyzer/tests/functional/suppress/suppress_export.skip
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
+*suppress_export.cpp
-*
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
+*suppress_without_bug.cpp
-*
165 changes: 165 additions & 0 deletions analyzer/tests/functional/suppress/test_suppress.py
Original file line number Diff line number Diff line change
@@ -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.")
88 changes: 0 additions & 88 deletions analyzer/tests/functional/suppress/test_suppress_export.py

This file was deleted.

5 changes: 4 additions & 1 deletion analyzer/tests/projects/suppress/Makefile
Original file line number Diff line number Diff line change
@@ -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
9 changes: 9 additions & 0 deletions analyzer/tests/projects/suppress/duplicated_suppress.cpp
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -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);
}
Loading