diff --git a/analyzer/codechecker_analyzer/cmd/analyze.py b/analyzer/codechecker_analyzer/cmd/analyze.py index 320a8a8426..932fc889b1 100644 --- a/analyzer/codechecker_analyzer/cmd/analyze.py +++ b/analyzer/codechecker_analyzer/cmd/analyze.py @@ -198,6 +198,15 @@ def add_arguments_to_parser(parser): "Example: '/path/to/main.cpp', 'lib/*.cpp', " "*/test*'.") + parser.add_argument('--review-status-config', + dest="review_status_config", + required=False, + type=existing_abspath, + default=argparse.SUPPRESS, + help="Path of review_status.yaml config file which " + "contains review status settings assigned to " + "specific directories, checkers, bug hashes.") + parser.add_argument('-o', '--output', dest="output_path", required=True, @@ -858,6 +867,18 @@ def __update_skip_file(args): shutil.copyfile(args.skipfile, skip_file_to_send) +def __update_review_status_config(args): + rs_config_to_send = os.path.join(args.output_path, 'review_status.yaml') + + if os.path.exists(rs_config_to_send): + os.remove(rs_config_to_send) + + if 'review_status_config' in args: + LOG.debug("Copying review status config file %s to %s", + args.review_status_config, rs_config_to_send) + shutil.copyfile(args.review_status_config, rs_config_to_send) + + def __cleanup_metadata(metadata_prev, metadata): """ Cleanup metadata. @@ -1103,6 +1124,7 @@ def main(args): compile_cmd_count) __update_skip_file(args) + __update_review_status_config(args) LOG.debug("Cleanup metadata file started.") __cleanup_metadata(metadata_prev, metadata) diff --git a/analyzer/codechecker_analyzer/cmd/check.py b/analyzer/codechecker_analyzer/cmd/check.py index ced027255e..2ac50a8e97 100644 --- a/analyzer/codechecker_analyzer/cmd/check.py +++ b/analyzer/codechecker_analyzer/cmd/check.py @@ -21,10 +21,10 @@ from codechecker_analyzer.analyzers import analyzer_types from codechecker_analyzer.arg import \ OrderedCheckersAction, OrderedConfigAction, \ - analyzer_config, checker_config + analyzer_config, checker_config, existing_abspath from codechecker_common import arg, cmd_config, logger -from codechecker_report_converter.source_code_comment_handler import \ +from codechecker_common.source_code_comment_handler import \ REVIEW_STATUS_VALUES from codechecker_analyzer.cmd.analyze import \ @@ -400,6 +400,16 @@ def add_arguments_to_parser(parser): "the analysis is considered as a failed " "one.") + analyzer_opts.add_argument('--review-status-config', + dest="review_status_config", + required=False, + type=existing_abspath, + default=argparse.SUPPRESS, + help="Path of review_status.yaml config file " + "which contains review status settings " + "assigned to specific directories, " + "checkers, bug hashes.") + clang_has_z3 = analyzer_types.is_z3_capable() if clang_has_z3: @@ -872,6 +882,7 @@ def __update_if_key_exists(source, target, key): 'disable_all', 'ordered_checkers', # --enable and --disable. 'timeout', + 'review_status_config', 'compile_uniqueing', 'report_hash', 'enable_z3', diff --git a/analyzer/codechecker_analyzer/cmd/parse.py b/analyzer/codechecker_analyzer/cmd/parse.py index 3574e95f41..89eae4b91a 100644 --- a/analyzer/codechecker_analyzer/cmd/parse.py +++ b/analyzer/codechecker_analyzer/cmd/parse.py @@ -25,15 +25,16 @@ from codechecker_report_converter.report.output.html import \ html as report_to_html from codechecker_report_converter.report.statistics import Statistics -from codechecker_report_converter.source_code_comment_handler import \ - REVIEW_STATUS_VALUES from codechecker_analyzer import analyzer_context, suppress_handler from codechecker_common import arg, logger, cmd_config +from codechecker_common.review_status_handler import ReviewStatusHandler from codechecker_common.skiplist_handler import SkipListHandler, \ SkipListHandlers +from codechecker_common.source_code_comment_handler import \ + REVIEW_STATUS_VALUES from codechecker_common.util import load_json @@ -384,6 +385,7 @@ def get_output_file_path(default_file_name: str) -> Optional[str]: processed_path_hashes = set() processed_file_paths = set() print_steps = 'print_steps' in args + review_status_handler = ReviewStatusHandler() html_builder: Optional[report_to_html.HtmlBuilder] = None if export == 'html': @@ -392,6 +394,15 @@ def get_output_file_path(default_file_name: str) -> Optional[str]: context.checker_labels) for dir_path, file_paths in report_file.analyzer_result_files(args.input): + review_status_cfg = os.path.join(dir_path, 'review_status.yaml') + if os.path.isfile(review_status_cfg): + try: + review_status_handler.set_review_status_config( + review_status_cfg) + except ValueError as err: + LOG.error(err) + sys.exit(1) + metadata = get_metadata(dir_path) if metadata and 'files' in args: @@ -413,6 +424,20 @@ def get_output_file_path(default_file_name: str) -> Optional[str]: reports = report_file.get_reports( file_path, context.checker_labels, file_cache) + for report in reports: + try: + # TODO: skip_handler is used later in reports_helper.skip() + # too. However, skipped reports shouldn't check source code + # comments because they potentially raise an exception. + # Skipped files shouldn't raise an exception, also, "skip" + # shouldn't be checked twice. + if not report.skip(skip_handlers): + report.review_status = \ + review_status_handler.get_review_status(report) + except ValueError as err: + LOG.error(err) + sys.exit(1) + reports = reports_helper.skip( reports, processed_path_hashes, skip_handlers, suppr_handler, src_comment_status_filter) @@ -434,6 +459,7 @@ def get_output_file_path(default_file_name: str) -> Optional[str]: file_report_map = plaintext.get_file_report_map( reports, file_path, metadata) plaintext.convert( + review_status_handler, file_report_map, processed_file_paths, print_steps) elif export == 'html': print(f"Parsing input file '{file_path}'.") @@ -441,6 +467,9 @@ def get_output_file_path(default_file_name: str) -> Optional[str]: file_path, reports, output_dir_path, html_builder) + for warning in review_status_handler.source_comment_warnings(): + LOG.warning(warning) + if export is None: # Plain text output statistics.write() elif export == 'html': diff --git a/analyzer/codechecker_analyzer/suppress_file_handler.py b/analyzer/codechecker_analyzer/suppress_file_handler.py index 54fcbd09b5..1a8f606f68 100644 --- a/analyzer/codechecker_analyzer/suppress_file_handler.py +++ b/analyzer/codechecker_analyzer/suppress_file_handler.py @@ -24,7 +24,7 @@ import re from codechecker_common.logger import get_logger -from codechecker_report_converter.source_code_comment_handler import \ +from codechecker_common.source_code_comment_handler import \ SourceCodeCommentHandler LOG = get_logger('system') diff --git a/analyzer/tests/functional/analyze_and_parse/test_analyze_and_parse.py b/analyzer/tests/functional/analyze_and_parse/test_analyze_and_parse.py index 98fabd73ce..042695570c 100644 --- a/analyzer/tests/functional/analyze_and_parse/test_analyze_and_parse.py +++ b/analyzer/tests/functional/analyze_and_parse/test_analyze_and_parse.py @@ -232,13 +232,21 @@ def check_one_file(self, path, mode): # Replace full path only to file name on the following # formatted lines: + # 1. # [severity] /a/b/x.cpp:line:col: message [checker] # The replacement on this line will be the following: # [severity] x.cpp:line:col: message [checker] + # 2. + # [] - /a/b/x.cpp contains misspelled ... + # The replacement on this line will be the following: + # [] - x.cpp contains misspelled ... sep = re.escape(os.sep) line = re.sub(r'^(\[\w+\]\s)(?P.+{0})' r'(.+\:\d+\:\d+\:\s.*\s\[.*\])$'.format(sep), r'\1\3', line) + line = re.sub(r'^\[\] - (?P.+{0})' + r'(.+ contains misspelled.+)'.format(sep), + r'[] - \2', line) if not any([line.startswith(prefix) for prefix in skip_prefixes]): diff --git a/analyzer/tests/functional/analyze_and_parse/test_files/multi_error_suppress_typo.output b/analyzer/tests/functional/analyze_and_parse/test_files/multi_error_suppress_typo.output index 0578d528d1..8536e1bf4e 100644 --- a/analyzer/tests/functional/analyze_and_parse/test_files/multi_error_suppress_typo.output +++ b/analyzer/tests/functional/analyze_and_parse/test_files/multi_error_suppress_typo.output @@ -22,13 +22,13 @@ CHECK#CodeChecker check --build "make multi_error_suppress_typo" --output $OUTPU return x/0; ^ -[] - multi_error_suppress_typo.cpp contains misspelled review status comment @9: // codechecker_suppressssss [all] some comment [LOW] multi_error_suppress_typo.cpp:10:3: Value stored to 'y' is never read [deadcode.DeadStores] y = 7; ^ Found 2 defect(s) in multi_error_suppress_typo.cpp +[] - multi_error_suppress_typo.cpp contains misspelled review status comment @9: // codechecker_suppressssss [all] some comment ----==== Severity Statistics ====---- ---------------------------- diff --git a/tools/report-converter/tests/unit/source_code_comment/source_code_comment_test_files/test_file_1 b/analyzer/tests/unit/source_code_comment_test_files/test_file_1 similarity index 100% rename from tools/report-converter/tests/unit/source_code_comment/source_code_comment_test_files/test_file_1 rename to analyzer/tests/unit/source_code_comment_test_files/test_file_1 diff --git a/tools/report-converter/tests/unit/source_code_comment/source_code_comment_test_files/test_file_2 b/analyzer/tests/unit/source_code_comment_test_files/test_file_2 similarity index 100% rename from tools/report-converter/tests/unit/source_code_comment/source_code_comment_test_files/test_file_2 rename to analyzer/tests/unit/source_code_comment_test_files/test_file_2 diff --git a/tools/report-converter/tests/unit/source_code_comment/source_code_comment_test_files/test_file_3 b/analyzer/tests/unit/source_code_comment_test_files/test_file_3 similarity index 100% rename from tools/report-converter/tests/unit/source_code_comment/source_code_comment_test_files/test_file_3 rename to analyzer/tests/unit/source_code_comment_test_files/test_file_3 diff --git a/tools/report-converter/tests/unit/source_code_comment/test_source_code_comment.py b/analyzer/tests/unit/test_source_code_comment.py similarity index 99% rename from tools/report-converter/tests/unit/source_code_comment/test_source_code_comment.py rename to analyzer/tests/unit/test_source_code_comment.py index 7609e810a8..a225fbf856 100644 --- a/tools/report-converter/tests/unit/source_code_comment/test_source_code_comment.py +++ b/analyzer/tests/unit/test_source_code_comment.py @@ -14,7 +14,7 @@ import os import unittest -from codechecker_report_converter.source_code_comment_handler import \ +from codechecker_common.source_code_comment_handler import \ SourceCodeComment, SourceCodeCommentHandler diff --git a/codechecker_common/review_status_handler.py b/codechecker_common/review_status_handler.py new file mode 100644 index 0000000000..a6b71bec38 --- /dev/null +++ b/codechecker_common/review_status_handler.py @@ -0,0 +1,283 @@ +# ------------------------------------------------------------------------- +# +# 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 +# +# ------------------------------------------------------------------------- + + +import fnmatch +import os +from typing import List, Optional +import yaml + +from codechecker_report_converter.report import Report, SourceReviewStatus +from codechecker_common.logger import get_logger +from codechecker_common.source_code_comment_handler import \ + SourceCodeCommentHandler, SpellException, contains_codechecker_comment, \ + SourceCodeComment, SourceCodeComments + + +LOG = get_logger('system') + + +class ReviewStatusHandler: + """ + This class helps to determine the review status of a report. The review + status may come either from several sources: + + 1. Source code comment: + // codechecker_suppress [core.DivideZero] This is a false positive. + 2. Review status configuration file: + /review_status.yaml contains review status settings on + file path or bug has granularity. + 3. Review status rule: + A mapping between bug hashes and review statuses set in the GUI. + + TODO: Option 3. should also be covered by this handler class. + """ + + REVIEW_STATUS_OPTIONS = [ + 'false_positive', + 'suppress', + 'confirmed', + 'intentional'] + + ALLOWED_FIELDS = [ + 'filepath_filter', + 'checker_filter', + 'report_hash_filter', + 'review_status', + 'message'] + + def __init__(self, source_root=''): + """ + TODO: What happens if multiple report directories are stored? + """ + self.__review_status_yaml = None + self.__source_root = source_root + self.__source_comment_warnings = [] + self.__source_commets = {} + self.__data = None + + def __parse_codechecker_review_comment( + self, + source_file_name: str, + report_line: int, + checker_name: str + ) -> SourceCodeComments: + """Parse the CodeChecker review comments from a source file at a given + position. Returns an empty list if there are no comments. + """ + src_comment_data = [] + with open(source_file_name, encoding='utf-8', errors='ignore') as f: + if contains_codechecker_comment(f): + sc_handler = SourceCodeCommentHandler() + try: + src_comment_data = sc_handler.filter_source_line_comments( + f, report_line, checker_name) + except SpellException as ex: + self.__source_comment_warnings.append( + f"{source_file_name} contains {ex}") + + return src_comment_data + + def __validate_review_status_yaml_data(self): + """ + This function validates the data read from review_status.yaml file and + raises ValueError with the description of the error if the format is + invalid. + """ + if not isinstance(self.__data, list): + raise ValueError( + f"{self.__review_status_yaml} should be a list of review " + "status descriptor objects.") + + for item in self.__data: + if not isinstance(item, dict): + raise ValueError( + f"Format error in {self.__review_status_yaml}: {item} " + "should be a review status descriptor object.") + + for field in item: + if field not in ReviewStatusHandler.ALLOWED_FIELDS: + raise ValueError( + f"Format error in {self.__review_status_yaml}: field " + f"'{field}' is not allowed. Available fields are: " + f"{', '.join(ReviewStatusHandler.ALLOWED_FIELDS)}") + + if 'review_status' not in item: + raise ValueError( + f"Format error in {self.__review_status_yaml}: " + f"'review_status' field is missing from {item}.") + + if item['review_status'] \ + not in ReviewStatusHandler.REVIEW_STATUS_OPTIONS: + raise ValueError( + f"Invalid review status field: {item['review_status']} at " + f"{item} in {self.__review_status_yaml}. Available " + f"options are: " + f"{', '.join(ReviewStatusHandler.REVIEW_STATUS_OPTIONS)}.") + + if item['review_status'] == 'suppress': + item['review_status'] = 'false_positive' + + def get_review_status(self, report: Report) -> SourceReviewStatus: + """ + Return the review status of the report based on source code comments. + + If the review status is not set in the source code then "unreviewed" + review status returns. This function throws ValueError if the review + status is ambiguous (see get_review_status_from_source()). + """ + # 1. Check if the report has in-source review status setting. + review_status = self.get_review_status_from_source(report) + + if review_status: + return review_status + + # 2. Check if the report has review status setting in the yaml config. + if self.__data: + review_status = self.get_review_status_from_config(report) + if review_status: + return review_status + + # 3. Return "unreviewed" otherwise. + return SourceReviewStatus(bug_hash=report.report_hash) + + def set_review_status_config(self, config_file): + """ + Set the location of the review_status.yaml config file. + + When the content of multiple report directories are parsed then they + may contain separate config files. These need to be given before + parsing each report folders. + """ + self.__review_status_yaml = config_file + + with open(self.__review_status_yaml, + encoding='utf-8', errors='ignore') as f: + # TODO: Validate format. + # - Can filepath be a list? + # TODO: May throw yaml.scanner.ScannerError. + try: + self.__data = yaml.safe_load(f) + except yaml.scanner.ScannerError as err: + raise ValueError( + f"Invalid YAML format in {self.__review_status_yaml}:\n" + f"{err}") + + self.__validate_review_status_yaml_data() + + def get_review_status_from_config( + self, + report: Report + ) -> Optional[SourceReviewStatus]: + """ + Return the review status of the given report based on the config file + set by set_review_status_config(). If not config file set, or no + setting matches the report then None returns. + """ + assert self.__data is not None, \ + "Review status config file has to be set with " \ + "set_review_status_config()." + + # TODO: Document "in_source". + for item in self.__data: + if 'filepath_filter' in item and not fnmatch.fnmatch( + report.file.original_path, item['filepath_filter']): + continue + if 'checker_filter' in item and \ + report.checker_name != item['checker_filter']: + continue + if 'report_hash_filter' in item and \ + not report.report_hash.startswith( + item['report_hash_filter']): + continue + + if any(filt in item for filt in + ['filepath_filter', 'checker_filter', + 'report_hash_filter']): + return SourceReviewStatus( + status=item['review_status'], + message=item['message'] + .encode(encoding='utf-8', errors='ignore') + if 'message' in item else b'', + bug_hash=report.report_hash, + in_source=True) + + return None + + def get_review_status_from_source( + self, + report: Report + ) -> Optional[SourceReviewStatus]: + """ + Return the review status based on the source code comment belonging to + the given report. + + - Return the review status if it is set in the source code. + - If the review status is ambiguous (i.e. multiple source code + comments belong to it) then a ValueError exception is raised which + contains information about the problem in a string. + - If the soure file changed (which means that the source comments may + have replaced, changed or removed) then None returns. + - If no review status belongs to the report in the source code, then + return None. + TODO: This function either returns a SourceReviewStatus, or raises an + exception or returns None. This is too many things that a caller needs + to handle. The reason is that according to the current behaviors, + ambiguous comment results a hard error, and changed files result a + warning with "unreviewed" status. In the future we could implement a + logic where we take the first comment into account in case of ambiguity + or return "unreviewed" with a warning, like changed files. + """ + # The original file path is needed here, not the trimmed, because + # the source files are extracted as the original file path. + # TODO: Should original_path be strip('/') at store? + source_file_name = os.path.realpath(os.path.join( + self.__source_root, report.file.original_path)) + + if source_file_name in report.changed_files: + return None + + if os.path.isfile(source_file_name): + src_comment_data = self.__parse_codechecker_review_comment( + source_file_name, report.line, report.checker_name) + + if len(src_comment_data) == 1: + data = src_comment_data[0] + status, message = data.status, data.message + self.__source_commets[report] = data + + return SourceReviewStatus( + status=status, + message=message.encode('utf-8'), + bug_hash=report.report_hash, + in_source=True) + elif len(src_comment_data) > 1: + raise ValueError( + f"Multiple source code comments can be found for " + f"'{report.checker_name}' checker in '{source_file_name}' " + f"at line {report.line}.") + + return None + + def source_comment_warnings(self) -> List[str]: + """ + Sometimes it is not intuitive why the given review status is determined + for a report. For example, if an in-source suppression is misspelled, + then the report remains unreviewed: + // codechecker_suppresssss [] comment + This function returns a list of warning messages on these unintuitive + behaviors. + """ + return self.__source_comment_warnings + + def source_comment(self, report: Report) -> Optional[SourceCodeComment]: + """ + This ReviewStatusHandler class is caching source comments so they are + read and parsed only once for each report. + """ + return self.__source_commets.get(report) diff --git a/tools/report-converter/codechecker_report_converter/source_code_comment_handler.py b/codechecker_common/source_code_comment_handler.py similarity index 100% rename from tools/report-converter/codechecker_report_converter/source_code_comment_handler.py rename to codechecker_common/source_code_comment_handler.py diff --git a/codechecker_common/util.py b/codechecker_common/util.py index 3465681e72..08743f8f35 100644 --- a/codechecker_common/util.py +++ b/codechecker_common/util.py @@ -12,6 +12,7 @@ import itertools import json +from typing import TextIO import portalocker from codechecker_common.logger import get_logger @@ -84,3 +85,16 @@ def load_json(path: str, default=None, lock=False, display_warning=True): LOG.warning(ex) return ret + + +def get_linef(fp: TextIO, line_no: int) -> str: + """'fp' should be (readable) file object. + Return the line content at line_no or an empty line + if there is less lines than line_no. + """ + fp.seek(0) + for line in fp: + line_no -= 1 + if line_no == 0: + return line + return '' diff --git a/docs/analyzer/user_guide.md b/docs/analyzer/user_guide.md index 8c59b088f0..21aff22911 100644 --- a/docs/analyzer/user_guide.md +++ b/docs/analyzer/user_guide.md @@ -47,15 +47,17 @@ - [`checkers`](#checkers) - [`analyzers`](#analyzers) - [Configuring Clang version](#configuring-clang-version) - - [Suppressing False positives (source code comments for review status)](#suppressing-false-positives-source-code-comments-for-review-status) - - [Supported formats](#supported-formats) - - [Change review status of a specific checker result](#change-review-status-of-a-specific-checker-result) - - [Change review status of a specific checker result by using a substring of the checker name](#change-review-status-of-a-specific-checker-result-by-using-a-substring-of-the-checker-name) - - [Change review status of all checker result](#change-review-status-of-all-checker-result) - - [Change review status of all checker result with C style comment](#change-review-status-of-all-checker-result-with-c-style-comment) - - [Multi line comments](#multi-line-comments) - - [Multi line C style comments](#multi-line-c-style-comments) - - [Change review status for multiple checker results in the same line](#change-review-status-for-multiple-checker-results-in-the-same-line) + - [Review status handling](#review-status-handling) + - [Setting with source code comments](#setting-with-source-code-comments) + - [Supported formats](#supported-formats) + - [Change review status of a specific checker result](#change-review-status-of-a-specific-checker-result) + - [Change review status of a specific checker result by using a substring of the checker name](#change-review-status-of-a-specific-checker-result-by-using-a-substring-of-the-checker-name) + - [Change review status of all checker results](#change-review-status-of-all-checker-results) + - [Change review status of all checker results with C style comment](#change-review-status-of-all-checker-results-with-c-style-comment) + - [Multi line comments](#multi-line-comments) + - [Multi line C style comments](#multi-line-c-style-comments) + - [Change review status for multiple checker results in the same line](#change-review-status-for-multiple-checker-results-in-the-same-line) + - [Setting with config file](#setting-with-config-file) ## Overview CodeChecker command line tooling provides sub-commands to perform **static code analysis** and to store reports into a **web-based storage**. @@ -68,7 +70,7 @@ The analysis related use-cases can be fully performed without a web-server. * [Execuing Static Analysis](#analyze) * [Showing analysis results in the command line](#parse) * [Applying code fixes](#fixit) -* [Suppressing false positives with source-code comments](#suppressing-false-positives-source-code-comments-for-review-status) +* [Suppressing false positives with source-code comments](#setting-with-source-code-comments) ## Quick Analysis @@ -2429,41 +2431,53 @@ Clang based tools search by default for in a path relative to the tool binary. `$(dirname /path/to/tool)/../lib/clang/8.0.0/include` -## Suppressing False positives (source code comments for review status) +## Review status handling + +Users can categorize CodeChecker reports by setting their _review status_. A +report can be _false positive_ or it can also indicate a _confirmed_ bug. +Sometimes a bug is _intentional_ in a test code. Developers can review the +reports and assign such a status accordingly with a short comment message. A +report is _unreviewed_ by default. + +The review status is not only a stateless indication of a report, but takes +effect in situations when CodeChecker needs to determine the set of outstanding +bugs. For example, a report with _intentional_ or _false positive_ detection +status is not considered outstanding, because the analyzed project's developers +don't need to worry about them. + +### Setting with source code comments Source code comments can be used in the source files to change the review -status of a specific or all checker results found in a particular line of code. -Source code comment should be above the line where the defect was found, and +status of a specific report found in a particular line of code. +Source code comments should be above the line where the defect was found, and __no__ empty lines are allowed between the line with the bug and the source code comment. -Comment lines staring with `//` or C style `/**/` comments are supported. +Comment lines staring with `//` or C style `/**/` comments are both supported. Watch out for the comment format! -### Supported formats +#### Supported formats The source code comment has the following format: -```sh -// codechecker comment type [checker name] comment +```c +// codechecker_ [] comment ``` -Multiple source code comment types are allowed: +Several source code comment types are allowed: - * `codechecker_suppress` - * `codechecker_false_positive` - * `codechecker_intentional` - * `codechecker_confirmed` - -Source code comment change the `review status` of a bug in the following form: - - * `codechecker_suppress` and `codechecker_false_positive` to `False positive` - * `codechecker_intentional` to `Intentional` - * `codechecker_confirmed` to `Confirmed`. - -Note: `codechecker_suppress` does the same as `codechecker_false_positive`. + * `codechecker_suppress`: Sets the review status to _false positive_. These + reports are not considered outstanding. The limitations of analyzers may + cause false positive reports, these can be categorized by this status value. + * `codechecker_false_positive`: Same as `codechecker_suppress`. + * `codechecker_intentional`: Sets the review status to _intentional_. These + reports are not considered outstanding. For example a bug may be intentional + in a test code. + * `codechecker_confirmed`: Sets the review status to _confirmed_. This is an + indication for developers to deal with this report. Such a report is + considered outstanding. You can read more about review status [here](https://github.com/Ericsson/codechecker/blob/master/www/userguide/userguide.md#userguide-review-status) -### Change review status of a specific checker result +#### Change review status of a specific checker result ```cpp void test() { int x; @@ -2472,7 +2486,7 @@ void test() { } ``` -### Change review status of a specific checker result by using a substring of the checker name +#### Change review status of a specific checker result by using a substring of the checker name There is no need to specify the whole checker name in the source code comment like `deadcode.DeadStores`, because it will not be resilient to package name changes. You are able to specify only a substring of the checker name for the @@ -2486,7 +2500,7 @@ void test() { ``` -### Change review status of all checker result +#### Change review status of all checker results ```cpp void test() { int x; @@ -2495,7 +2509,7 @@ void test() { } ``` -### Change review status of all checker result with C style comment +#### Change review status of all checker results with C style comment ```cpp void test() { int x; @@ -2504,7 +2518,7 @@ void test() { } ``` -### Multi line comments +#### Multi line comments ```cpp void test() { int x; @@ -2517,7 +2531,7 @@ void test() { } ``` -### Multi line C style comments +#### Multi line C style comments ```cpp void test() { int x; @@ -2544,7 +2558,7 @@ void test() { } ``` -### Change review status for multiple checker results in the same line +#### Change review status for multiple checker results in the same line You can change multiple checker reports with a single source code comment: ```cpp @@ -2555,7 +2569,7 @@ void test() { ``` The limitation of this format is that you can't use separate status or message -for checkers. To solve this problem you can use one of the following format: +for checkers. To solve this problem you can use one of the following formats: ```cpp void test_simple() { @@ -2589,3 +2603,45 @@ void testError2() { int x = 1 / 0; } ``` + +### Setting with config file + +Review status can be configured by a config file in YAML format. This config +file has to represent a list of review status settings: + +```yaml +- filepath_filter: /path/to/project/test/* + checker_filter: core.DivideZero + message: Division by zero in test files is automatically intentional. + review_status: intentional +- filepath_filter: /path/to/project/important/module/* + message: All reports in this module should be investigated. + review_status: confirmed +- filepath_filter: "*/project/test/*" + message: If a filter starts with asterix, then it should be quoted due to YAML format. + review_status: suppress +- report_hash_filter: b85851b34789e35c6acfa1a4aaf65382 + message: This report is false positive. + review_status: false_positive +``` + +The fields of a review status settings are: + +- `filepath_filter` (optional): A glob to a path where the given review status + is applied. A [https://docs.python.org/3/library/glob.html](glob) is a path + that may contain shell-style wildcards: `*` substitutes zero or more + characters, `?` substitutes exactly one character. This filter option is + applied on the full path of a source file, even if `--trim-path-prefix` flag + is used later. +- `checker_filter` (optional): Set the review status for only these checkers' + reports. +- `report_hash_filter` (optional): Set the review status for only the checkers + having this report hash. A prefix match is applied on report hashes, so it is + enough to provide the beginning of a hash. Make sure to use a quite long + prefix so it covers one specific report. +- `message` (optional): A comment message that describes the reason of the + setting. +- `review_status`: The review status to set. + +If none of the `_filter` options is provided, then that setting is not applied +on any report. diff --git a/docs/usage.md b/docs/usage.md index 38a52b8e6f..4e2c7634be 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -12,7 +12,7 @@ in your source code and integrate the static analysis into your CI flow to preve - [Step in the docs/examples directory](#step-in-the-docsexamples-directory) - [Clean the workspace](#clean-the-workspace) - [Log your build](#log-your-build) - - [Check the contents of compile_commands.json file](#check-the-contents-of-compile_commandsjson-file) + - [Check the contents of compile\_commands.json file](#check-the-contents-of-compile_commandsjson-file) - [Step 2: Analyze your code](#step-2-analyze-your-code) - [Run the analysis](#run-the-analysis) - [View the analysis results in the command line](#view-the-analysis-results-in-the-command-line) @@ -37,7 +37,7 @@ in your source code and integrate the static analysis into your CI flow to preve - [Configure Checkers](#configure-checkers) - [Identify files that failed analysis](#identify-files-that-failed-analysis) - [Step 8: Integrate CodeChecker into your CI loop](#step-8-integrate-codechecker-into-your-ci-loop) - - [Storing & Updating runs](#storing--updating-runs) + - [Storing \& Updating runs](#storing--updating-runs) - [Alternative 1 (RECOMMENDED): Store the results of each commit in the same run](#alternative-1-recommended-store-the-results-of-each-commit-in-the-same-run) - [Alternative 2: Store each analysis in a new run](#alternative-2-store-each-analysis-in-a-new-run) - [Use CodeChecker in the CI without a server](#use-codechecker-in-the-ci-without-a-server) @@ -556,6 +556,10 @@ recommended to suppress false positives on the Web UI only, because this way the suppression will be stored in a database that is unrelated to the source code. +It is also possible to suppress multiple reports located in a specific folder +or reported by a given checker. For further details see +(this guide)[analyzer/user_guide.md#setting-with-config-file]. + ### Ignore modules from your analysis You can ignore analysis results for certain files for example 3rd party modules. For that use the `--ignore` parameter of the analyze command: diff --git a/tools/report-converter/codechecker_report_converter/report/__init__.py b/tools/report-converter/codechecker_report_converter/report/__init__.py index 52cbf772e7..fc3512577b 100644 --- a/tools/report-converter/codechecker_report_converter/report/__init__.py +++ b/tools/report-converter/codechecker_report_converter/report/__init__.py @@ -7,16 +7,16 @@ # ------------------------------------------------------------------------- import builtins +from dataclasses import dataclass +from datetime import datetime import itertools import json import logging import os -from typing import Callable, Dict, Iterable, List, Optional, Set +from typing import Callable, Dict, List, Optional, Set from .. import util -from ..source_code_comment_handler import SourceCodeCommentHandler, \ - SourceCodeComments, SpellException LOG = logging.getLogger('report-converter') @@ -271,6 +271,26 @@ def __repr__(self): return json.dumps(self.to_json()) +@dataclass +class SourceReviewStatus: + """ + Helper class for handling in source review statuses. + Collect the same info as a review status rule. + + TODO: rename this class, because this not only represents review statuses + in source code comments but in review status yaml too. + """ + status: str = "unreviewed" + message: bytes = b"" + bug_hash: str = "" + in_source: bool = False + author: Optional[str] = None + date: Optional[datetime] = None + + def formatted_status(self): + return self.status.lower().replace('_', ' ').capitalize() + + class Report: """ Represents a report object. """ @@ -293,7 +313,8 @@ def __init__( notes: Optional[List[BugPathEvent]] = None, macro_expansions: Optional[List[MacroExpansion]] = None, annotations: Optional[Dict[str, str]] = None, - static_message: Optional[str] = None + static_message: Optional[str] = None, + review_status: Optional[SourceReviewStatus] = SourceReviewStatus() ): """ This constructor populates the members of the Report object. @@ -333,9 +354,7 @@ def __init__( self.macro_expansions = macro_expansions \ if macro_expansions is not None else [] - self.__source_code_comments: Optional[SourceCodeComments] = None - self.__source_code_comment_warnings: List[str] = [] - self.__sc_handler = SourceCodeCommentHandler() + self.review_status = review_status self.__source_line: Optional[str] = source_line self.__files: Optional[Set[File]] = None @@ -449,92 +468,6 @@ def changed_files(self, changed_files: Set[str]): if self.__changed_files is None: self.__changed_files = changed_files - def __init_source_code_comments(self): - """ - Initialize source code comments and warnings if it is not parsed yet. - """ - if self.__source_code_comments is not None: - return None - - self.__source_code_comments = [] - - if self.file.original_path in self.changed_files: - return None - - if not os.path.exists(self.file.original_path): - return None - - with open(self.file.original_path, - encoding='utf-8', errors='ignore') as f: - try: - self.__source_code_comments = \ - self.__sc_handler.filter_source_line_comments( - f, self.line, self.checker_name) - except SpellException as ex: - self.__source_code_comment_warnings.append( - f"{self.file.name} contains {str(ex)}") - - if len(self.__source_code_comments) == 1: - LOG.debug("Found source code comment for report '%s' in file " - "'%s': %s", - self.report_hash, self.file.path, - self.__source_code_comments) - elif len(self.__source_code_comments) > 1: - self.__source_code_comment_warnings.append( - f"Multiple source code comment can be found for " - f"'{self.checker_name}' checker in '{self.file.path}' at " - f"line {self.line}. This bug will not be suppressed!") - - @property - def source_code_comment_warnings(self) -> List[str]: - """ Get source code comment warnings. """ - self.__init_source_code_comments() - return self.__source_code_comment_warnings - - def dump_source_code_comment_warnings(self): - """ Dump source code comments warnings. """ - for warning in self.source_code_comment_warnings: - LOG.warning(warning) - - @property - def source_code_comments(self) -> SourceCodeComments: - """ - Get source code comments for the report. - It will read the source file only once. - """ - self.__init_source_code_comments() - - if self.__source_code_comments is None: - self.__source_code_comments = [] - - return self.__source_code_comments - - @source_code_comments.setter - def source_code_comments(self, source_code_comments: SourceCodeComments): - """ Sets the source code comments manually if it's not set yet. """ - if self.__source_code_comments is None: - self.__source_code_comments = source_code_comments - - def check_source_code_comments(self, comment_types: Iterable[str]) -> bool: - """ - True if it doesn't have a source code comment or if every comments have - specified comment types. - """ - if not self.source_code_comments: - return True - - return all(c.status in comment_types - for c in self.source_code_comments) - - @property - def review_status(self) -> str: - """ Return review status for the given report. """ - if len(self.source_code_comments) == 1: - return self.source_code_comments[0].status \ - .lower().replace('_', ' ') - - return 'unreviewed' - def skip(self, skip_handlers: Optional[SkipListHandlers]) -> bool: """ True if the report should be skipped. """ if not skip_handlers: @@ -556,9 +489,8 @@ def to_json(self) -> Dict: "analyzer_name": self.analyzer_name, "category": self.category, "type": self.type, - "source_code_comments": [ - s.to_json() for s in self.source_code_comments], - "review_status": self.review_status, + "review_status": self.review_status.status + if self.review_status else '', "bug_path_events": [e.to_json() for e in self.bug_path_events], "bug_path_positions": [ p.to_json() for p in self.bug_path_positions], @@ -578,5 +510,19 @@ def __eq__(self, other): raise NotImplementedError( f"Comparison Range object with '{type(other)}' is not supported") + def __hash__(self): + """ + We consider two report objects equivalent if these members are the + same. This way a report object can be used as key in a dict. + WARNING! Make sure that the same members are compared by __eq__(). + """ + return builtins.hash(( + self.file, + self.line, + self.column, + self.message, + self.checker_name, + self.report_hash)) + def __repr__(self): return json.dumps(self.to_json()) diff --git a/tools/report-converter/codechecker_report_converter/report/output/html/html.py b/tools/report-converter/codechecker_report_converter/report/output/html/html.py index fc1e2c0561..fd4ef21342 100644 --- a/tools/report-converter/codechecker_report_converter/report/output/html/html.py +++ b/tools/report-converter/codechecker_report_converter/report/output/html/html.py @@ -249,7 +249,8 @@ def to_macro_expansions( 'events': to_bug_path_events(report.bug_path_events), 'macros': to_macro_expansions(report.macro_expansions), 'notes': to_bug_path_events(report.notes), - 'reviewStatus': report.review_status, + 'reviewStatus': report.review_status.formatted_status() + if report.review_status else '', 'severity': self.get_severity(report.checker_name) }) diff --git a/tools/report-converter/codechecker_report_converter/report/output/plaintext.py b/tools/report-converter/codechecker_report_converter/report/output/plaintext.py index 86d1ef683c..c013e13314 100644 --- a/tools/report-converter/codechecker_report_converter/report/output/plaintext.py +++ b/tools/report-converter/codechecker_report_converter/report/output/plaintext.py @@ -64,8 +64,9 @@ def format_report(report: Report, content_is_not_changed: bool) -> str: out = f"[{report.severity}] {file_path}:{report.line}:{report.column}: " \ f"{report.message} [{report.checker_name}]" - if content_is_not_changed and report.source_code_comments: - out += f" [{report.review_status.capitalize()}]" + if content_is_not_changed and report.review_status and \ + report.review_status.status != 'unreviewed': + out += f" [{report.review_status.formatted_status()}]" return out @@ -144,6 +145,7 @@ def get_file_report_map( def convert( + review_status_handler, source_file_report_map: Dict[str, List[Report]], processed_file_paths: Optional[Set[str]] = None, print_steps: bool = False, @@ -164,16 +166,12 @@ def convert( content_is_not_changed = \ report.file.original_path not in report.changed_files - if content_is_not_changed: - report.dump_source_code_comment_warnings() - output.write(f"{format_report(report, content_is_not_changed)}\n") if content_is_not_changed: - # Print source code comments. - for source_code_comment in report.source_code_comments: - if source_code_comment.line: - output.write(f"{source_code_comment.line.rstrip()}\n") + source_comment = review_status_handler.source_comment(report) + if source_comment and source_comment.line: + output.write(f"{source_comment.line.rstrip()}\n") output.write(f"{format_main_report(report)}") else: diff --git a/tools/report-converter/codechecker_report_converter/report/reports.py b/tools/report-converter/codechecker_report_converter/report/reports.py index 6db7e99112..7ace456d9e 100644 --- a/tools/report-converter/codechecker_report_converter/report/reports.py +++ b/tools/report-converter/codechecker_report_converter/report/reports.py @@ -7,7 +7,6 @@ # ------------------------------------------------------------------------- import logging -import sys from typing import Any, Callable, Iterable, List, Optional, Set @@ -58,7 +57,7 @@ def skip( processed_path_hashes: Optional[Set[str]] = None, skip_handlers: Optional[SkipListHandlers] = None, suppr_handler: Optional[GenericSuppressHandler] = None, - src_comment_status_filter: Optional[Iterable[str]] = None + review_status_filter: Optional[Iterable[str]] = None ) -> List[Report]: """ Skip reports. """ kept_reports = [] @@ -74,29 +73,22 @@ def skip( report.checker_name, report.report_hash) continue - if report.source_code_comments: - if len(report.source_code_comments) > 1: - LOG.error("Multiple source code comment can be found for '%s' " - "checker in '%s' at line %d.", report.checker_name, - report.file.original_path, report.line) - sys.exit(1) - + if report.review_status: if suppr_handler: if not report.report_hash: LOG.warning("Can't store suppress information for report " "because no report hash is set: %s", report) continue - source_code_comment = report.source_code_comments[0] - suppr_handler.store_suppress_bug_id( - report.report_hash, - report.file.name, - source_code_comment.message, - source_code_comment.status) + if report.review_status.status != 'unreviewed': + suppr_handler.store_suppress_bug_id( + report.report_hash, + report.file.name, + report.review_status.message.decode(), + report.review_status.status) - if src_comment_status_filter and \ - not report.check_source_code_comments( - src_comment_status_filter): + if review_status_filter and \ + report.review_status.status not in review_status_filter: LOG.debug("Filtered out by --review-status filter option: " "%s:%s [%s] %s [%s]", report.file.original_path, report.line, @@ -104,8 +96,8 @@ def skip( report.review_status) continue else: - if src_comment_status_filter and \ - 'unreviewed' not in src_comment_status_filter: + if review_status_filter and \ + 'unreviewed' not in review_status_filter: LOG.debug("Filtered out by --review-status filter option: " "%s:%s [%s] %s [unreviewed]", report.file.original_path, report.line, diff --git a/tools/report-converter/tests/unit/source_code_comment/__init__.py b/tools/report-converter/tests/unit/source_code_comment/__init__.py deleted file mode 100644 index 4259749345..0000000000 --- a/tools/report-converter/tests/unit/source_code_comment/__init__.py +++ /dev/null @@ -1,7 +0,0 @@ -# ------------------------------------------------------------------------- -# -# 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 -# -# ------------------------------------------------------------------------- diff --git a/web/client/codechecker_client/cmd/store.py b/web/client/codechecker_client/cmd/store.py index f1ed65f19b..668cc75adc 100644 --- a/web/client/codechecker_client/cmd/store.py +++ b/web/client/codechecker_client/cmd/store.py @@ -38,13 +38,13 @@ reports as reports_helper, statistics as report_statistics from codechecker_report_converter.report.hash import HashType, \ get_report_path_hash -from codechecker_report_converter.source_code_comment_handler import \ - SourceCodeCommentHandler from codechecker_client import client as libclient from codechecker_client import product from codechecker_common import arg, logger, cmd_config from codechecker_common.checker_labels import CheckerLabels +from codechecker_common.source_code_comment_handler import \ + SourceCodeCommentHandler from codechecker_common.util import load_json from codechecker_web.shared import webserver_context, host_check @@ -449,6 +449,10 @@ def assemble_zip(inputs, files_to_compress.add(skip_file_path) + review_status_file_path = os.path.join(dir_path, 'review_status.yaml') + if os.path.exists(review_status_file_path): + files_to_compress.add(review_status_file_path) + LOG.debug("Processing report files ...") # Currently ProcessPoolExecutor fails completely in windows. diff --git a/web/client/codechecker_client/cmd_line_client.py b/web/client/codechecker_client/cmd_line_client.py index fba88dfc0d..78857207f0 100644 --- a/web/client/codechecker_client/cmd_line_client.py +++ b/web/client/codechecker_client/cmd_line_client.py @@ -35,6 +35,7 @@ from codechecker_common import logger from codechecker_common.checker_labels import CheckerLabels +from codechecker_common.review_status_handler import ReviewStatusHandler from codechecker_common.util import load_json from codechecker_web.shared import convert, webserver_context @@ -181,10 +182,13 @@ def process_run_args(client, run_args_with_tag: Iterable[str]): def get_suppressed_reports(reports: List[Report], review_st: List[ttypes.ReviewStatus]) -> List[str]: """Returns a list of suppressed report hashes.""" - return [report.report_hash for report in reports - if not report.check_source_code_comments( - [ttypes.ReviewStatus._VALUES_TO_NAMES[x] - for x in review_st])] + statuses_str = [ttypes.ReviewStatus._VALUES_TO_NAMES[x].lower() + for x in review_st] + + return [ + report.report_hash for report in reports + if report.review_status.status != 'unreviewed' and + report.review_status not in statuses_str] def get_report_dir_results( @@ -199,11 +203,30 @@ def get_report_dir_results( all_reports = [] processed_path_hashes = set() - for _, file_paths in report_file.analyzer_result_files(report_dirs): + for report_dir, file_paths in \ + report_file.analyzer_result_files(report_dirs): + + review_status_handler = ReviewStatusHandler() + review_status_cfg = os.path.join(report_dir, 'review_status.yaml') + if os.path.isfile(review_status_cfg): + try: + review_status_handler.set_review_status_config( + review_status_cfg) + except ValueError as err: + LOG.error(err) + sys.exit(1) + for file_path in file_paths: # Get reports. reports = report_file.get_reports(file_path, checker_labels) + try: + for report in reports: + report.review_status = \ + review_status_handler.get_review_status(report) + except ValueError as err: + LOG.error(err) + # Skip duplicated reports. reports = reports_helper.skip(reports, processed_path_hashes) @@ -835,9 +858,9 @@ def get_diff_local_dir_remote_run( report_dir_results = [ r for r in report_dir_results if - r.review_status not in ['false positive', 'intentional'] and + r.review_status.status not in ['false_positive', 'intentional'] and (r.report_hash not in closed_hashes or - r.review_status == 'confirmed')] + r.review_status.status == 'confirmed')] local_report_hashes = set(r.report_hash for r in report_dir_results) local_report_hashes.update( baseline.get_report_hashes(baseline_files) - closed_hashes) @@ -932,9 +955,10 @@ def get_diff_remote_run_local_dir( report_dir_results = [ r for r in report_dir_results if - r.review_status not in ['false positive', 'intentional'] and + r.review_status.status not in ['false_positive', 'intentional'] and (r.report_hash not in closed_hashes or - r.review_status == 'confirmed')] + r.review_status.status == 'confirmed')] + local_report_hashes = set(r.report_hash for r in report_dir_results) local_report_hashes.update( baseline.get_report_hashes(baseline_files) - closed_hashes) @@ -1044,16 +1068,17 @@ def get_diff_local_dirs( context = webserver_context.get_context() statuses_str = [ttypes.ReviewStatus._VALUES_TO_NAMES[x].lower() for x in report_filter.reviewStatus] + statuses_str.append('unreviewed') base_results = get_report_dir_results( report_dirs, report_filter, context.checker_labels) base_results = [res for res in base_results - if res.check_source_code_comments(statuses_str)] + if res.review_status.status in statuses_str] new_results = get_report_dir_results( new_report_dirs, report_filter, context.checker_labels) new_results = [res for res in new_results - if res.check_source_code_comments(statuses_str)] + if res.review_status.status in statuses_str] base_hashes = set([res.report_hash for res in base_results]) new_hashes = set([res.report_hash for res in new_results]) @@ -1116,6 +1141,7 @@ def print_reports( if output_format == 'plaintext': file_report_map = plaintext.get_file_report_map(reports) plaintext.convert( + ReviewStatusHandler(), file_report_map, processed_file_paths, print_steps) context = webserver_context.get_context() diff --git a/web/client/codechecker_client/suppress_file_handler.py b/web/client/codechecker_client/suppress_file_handler.py index 9133fc039d..89d1fcca5d 100644 --- a/web/client/codechecker_client/suppress_file_handler.py +++ b/web/client/codechecker_client/suppress_file_handler.py @@ -25,7 +25,7 @@ import re from codechecker_common.logger import get_logger -from codechecker_report_converter.source_code_comment_handler import \ +from codechecker_common.source_code_comment_handler import \ SourceCodeCommentHandler LOG = get_logger('system') diff --git a/web/server/codechecker_server/api/mass_store_run.py b/web/server/codechecker_server/api/mass_store_run.py index 75a6a98569..f311f9dc69 100644 --- a/web/server/codechecker_server/api/mass_store_run.py +++ b/web/server/codechecker_server/api/mass_store_run.py @@ -26,13 +26,13 @@ from codechecker_common import skiplist_handler from codechecker_common.logger import get_logger +from codechecker_common.review_status_handler import ReviewStatusHandler, \ + SourceReviewStatus from codechecker_common.util import load_json from codechecker_report_converter.util import trim_path_prefixes from codechecker_report_converter.report import report_file, Report from codechecker_report_converter.report.hash import get_report_path_hash -from codechecker_report_converter.source_code_comment_handler import \ - SourceCodeCommentHandler, SpellException, contains_codechecker_comment from ..database import db_cleanup from ..database.config_db_model import Product @@ -65,10 +65,6 @@ def __exit__(self, *args): self.__msg, round(time.time() - self.__start_time, 2)) -# FIXME: when these types are introduced we need to use those. -SourceLineComments = List[Any] - - def unzip(b64zip: str, output_dir: str) -> int: """ This function unzips the base64 encoded zip file. This zip is extracted @@ -101,27 +97,6 @@ def get_file_content(file_path: str) -> bytes: return f.read() -def parse_codechecker_review_comment( - source_file_name: str, - report_line: int, - checker_name: str -) -> SourceLineComments: - """Parse the CodeChecker review comments from a source file at a given - position. Returns an empty list if there are no comments. - """ - src_comment_data = [] - with open(source_file_name, encoding='utf-8', errors='ignore') as f: - if contains_codechecker_comment(f): - sc_handler = SourceCodeCommentHandler() - try: - src_comment_data = sc_handler.filter_source_line_comments( - f, report_line, checker_name) - except SpellException as ex: - LOG.warning("File %s contains %s", source_file_name, ex) - - return src_comment_data - - def add_file_record( session: DBSession, file_path: str, @@ -216,29 +191,6 @@ def get_blame_file_data( return blame_info, remote_url, tracking_branch -class SourceReviewStatus: - """ - Helper class for handling in source review statuses. - Collect the same info as a review status rule. - FIXME Change this to dataclass when available - """ - def __init__( - self, - status: str, - message: bytes, - bug_hash: Any, - author: str, - date: datetime, - in_source: bool, - ): - self.status = status - self.message = message - self.bug_hash = bug_hash - self.author = author - self.date = date - self.in_source = in_source - - class MassStoreRun: def __init__( self, @@ -876,11 +828,11 @@ def __process_report_file( self, report_file_path: str, session: DBSession, - source_root: str, run_id: int, file_path_to_id: Dict[str, int], run_history_time: datetime, skip_handler: skiplist_handler.SkipListHandler, + review_status_handler: ReviewStatusHandler, hash_map_reports: Dict[str, List[Any]] ) -> bool: """ @@ -891,63 +843,6 @@ def __process_report_file( if not reports: return True - def get_review_status_from_source( - report: Report) -> SourceReviewStatus: - """ - Return the review status set in the source code belonging to - the given report. - - - Return the review status if it is set in the source code. - - If the review status is ambiguous (i.e. multiple source code - comments belong to it) then (unreviewed, in_source=False) - returns. - """ - # The original file path is needed here, not the trimmed, because - # the source files are extracted as the original file path. - source_file_name = os.path.realpath(os.path.join( - source_root, report.file.original_path.strip("/"))) - - if os.path.isfile(source_file_name): - src_comment_data = parse_codechecker_review_comment( - source_file_name, report.line, report.checker_name) - - if len(src_comment_data) == 1: - data = src_comment_data[0] - status, message = data.status, bytes(data.message, 'utf-8') - - review_status = SourceReviewStatus( - status=status, - message=message, - bug_hash=report.report_hash, - author=self.user_name, - date=run_history_time, - in_source=True - ) - - return review_status - elif len(src_comment_data) > 1: - LOG.warning( - "Multiple source code comment can be found " - "for '%s' checker in '%s' at line %s. " - "This suppression will not be taken into account!", - report.checker_name, source_file_name, report.line) - - self.__wrong_src_code_comments.append( - f"{source_file_name}|{report.line}|" - f"{report.checker_name}") - - # A better way to handle reports where the review status is not - # set in the source is to return None, and set the reviews status - # and set the review status info at report addition time. - return SourceReviewStatus( - status="unreviewed", - message=b'', - bug_hash=report.report_hash, - author=self.user_name, - date=run_history_time, - in_source=False - ) - def get_missing_file_ids(report: Report) -> List[str]: """ Returns file paths which database file id is missing. """ missing_ids_for_files = [] @@ -1000,12 +895,20 @@ def get_missing_file_ids(report: Report) -> List[str]: analyzer_name = mip.checker_to_analyzer.get( report.checker_name, report.analyzer_name) - rs_from_source = get_review_status_from_source(report) + review_status = SourceReviewStatus() + + try: + review_status = review_status_handler.get_review_status(report) + except ValueError as err: + self.__wrong_src_code_comments.append(str(err)) + + review_status.author = self.user_name + review_status.date = run_history_time # False positive and intentional reports are considered as closed # reports which is indicated with non-null "fixed_at" date. fixed_at = None - if rs_from_source.status in ['false_positive', 'intentional']: + if review_status.status in ['false_positive', 'intentional']: # Keep in mind that now this is not handling review status # rules, only review status source code comments if old_report and old_report.review_status in \ @@ -1017,11 +920,11 @@ def get_missing_file_ids(report: Report) -> List[str]: self.__check_report_count() report_id = self.__add_report( session, run_id, report, file_path_to_id, - rs_from_source, detection_status, detected_at, + review_status, detection_status, detected_at, run_history_time, analysis_info, analyzer_name, fixed_at) self.__new_report_hashes[report.report_hash] = \ - rs_from_source.status + review_status.status self.__already_added_report_hashes.add(report_path_hash) LOG.debug("Storing report done. ID=%d", report_id) @@ -1164,6 +1067,14 @@ def get_skip_handler( skip_handler = get_skip_handler(root_dir_path) + review_status_handler = ReviewStatusHandler(source_root) + + review_status_cfg = \ + os.path.join(root_dir_path, 'review_status.yaml') + if os.path.isfile(review_status_cfg): + review_status_handler.set_review_status_config( + review_status_cfg) + mip = self.__mips[root_dir_path] enabled_checkers.update(mip.enabled_checkers) disabled_checkers.update(mip.disabled_checkers) @@ -1176,9 +1087,9 @@ def get_skip_handler( report_file_path = os.path.join(root_dir_path, f) self.__process_report_file( - report_file_path, session, source_root, run_id, + report_file_path, session, run_id, file_path_to_id, run_history_time, - skip_handler, report_to_report_id) + skip_handler, review_status_handler, report_to_report_id) processed_result_file_count += 1 session.flush() @@ -1189,9 +1100,10 @@ def get_skip_handler( # but before first check the performance reports_to_rs_rules = session.query(ReviewStatusRule, DBReport) \ .join(DBReport, DBReport.bug_id == ReviewStatusRule.bug_hash) \ - .filter(sqlalchemy.and_(DBReport.run_id == run_id, - ReviewStatusRule.bug_hash. - in_(self.__new_report_hashes))) + .filter(sqlalchemy.and_( + DBReport.run_id == run_id, + DBReport.review_status_is_in_source.is_(False), + ReviewStatusRule.bug_hash.in_(self.__new_report_hashes))) # Set the newly stored reports for review_status, db_report in reports_to_rs_rules: @@ -1449,6 +1361,4 @@ def store(self) -> int: if self.__wrong_src_code_comments: raise codechecker_api_shared.ttypes.RequestFailed( codechecker_api_shared.ttypes.ErrorCode.SOURCE_FILE, - "Multiple source code comment can be found with the same " - "checker name for same bug!", self.__wrong_src_code_comments) diff --git a/web/server/codechecker_server/migrations/report/versions/75ae226b5d88_review_status_for_each_report.py b/web/server/codechecker_server/migrations/report/versions/75ae226b5d88_review_status_for_each_report.py index f73f1ea310..663dd73591 100644 --- a/web/server/codechecker_server/migrations/report/versions/75ae226b5d88_review_status_for_each_report.py +++ b/web/server/codechecker_server/migrations/report/versions/75ae226b5d88_review_status_for_each_report.py @@ -18,8 +18,7 @@ import zlib from io import StringIO -from codechecker_common import util -from codechecker_report_converter.source_code_comment_handler import \ +from codechecker_common.source_code_comment_handler import \ SourceCodeCommentHandler, SpellException diff --git a/web/server/vue-cli/src/components/Report/SelectReviewStatus.vue b/web/server/vue-cli/src/components/Report/SelectReviewStatus.vue index b6edd3f9ae..6ba5c1f19f 100644 --- a/web/server/vue-cli/src/components/Report/SelectReviewStatus.vue +++ b/web/server/vue-cli/src/components/Report/SelectReviewStatus.vue @@ -71,12 +71,12 @@ >

Review status of the following reports are given as source code - comment. Their review status will not change. However, this option - sets the review status of all reports without source code comment - even in the future. This may result that a bug is assigned two - different review statuses. We don't recommend setting review - status in this interface for reports which already have one as a - source code comment! + comment or review status config YAML file. Their review status + will not change. However, this option sets the review status of + all reports without source code comment even in the future. This + may result that a bug is assigned two different review statuses. + We don't recommend setting review status in this interface for + reports which already have one as a source code comment!