Skip to content

Commit

Permalink
[feat] Introduce review status config file
Browse files Browse the repository at this point in the history
The review statuses can be provided by a yaml config file. This file can
be used for setting the review status of reports in a given directory or
a checker name.
  • Loading branch information
bruntib committed Oct 23, 2023
1 parent 88eff4b commit c7c6879
Show file tree
Hide file tree
Showing 8 changed files with 235 additions and 54 deletions.
21 changes: 21 additions & 0 deletions analyzer/codechecker_analyzer/cmd/analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,14 @@ 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,
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,
Expand Down Expand Up @@ -853,6 +861,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.
Expand Down Expand Up @@ -1098,6 +1118,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)
Expand Down
10 changes: 10 additions & 0 deletions analyzer/codechecker_analyzer/cmd/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -400,6 +400,15 @@ 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,
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:
Expand Down Expand Up @@ -872,6 +881,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',
Expand Down
4 changes: 4 additions & 0 deletions analyzer/codechecker_analyzer/cmd/parse.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,6 +394,10 @@ 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):
review_status_handler.set_review_status_config(review_status_cfg)

metadata = get_metadata(dir_path)

if metadata and 'files' in args:
Expand Down
98 changes: 93 additions & 5 deletions codechecker_common/review_status_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,10 @@
# -------------------------------------------------------------------------


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
Expand All @@ -33,13 +35,16 @@ class ReviewStatusHandler:
3. Review status rule:
A mapping between bug hashes and review statuses set in the GUI.
TODO: Option 2. will be handled in a future commit.
TODO: Option 3. should also be covered by this handler class.
"""
def __init__(self, source_root=''):
"""
TODO: What happens if multiple report directories are stored?
"""
self.__source_root = source_root
self.__source_comment_warnings = []
self.__source_commets = {}
self.__data = None

def __parse_codechecker_review_comment(
self,
Expand All @@ -63,6 +68,28 @@ def __parse_codechecker_review_comment(

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.")

if 'review_status' not in item:
raise ValueError(
f"Format error in {self.__review_status_yaml}: "
f"'review_status' field is missing from {item}.")

def get_review_status(self, report: Report) -> SourceReviewStatus:
"""
Return the review status of the report based on source code comments.
Expand All @@ -71,13 +98,72 @@ def get_review_status(self, report: Report) -> SourceReviewStatus:
review status returns. This function throws ValueError if the review
status is ambiguous (see get_review_status_from_source()).
"""
rs_from_source = self.get_review_status_from_source(report)
# 1. Check if the report has in-source review status setting.
review_status = self.get_review_status_from_source(report)

if rs_from_source:
return rs_from_source
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.
self.__data = yaml.safe_load(f)

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' in item and not fnmatch.fnmatch(
report.file.original_path, item['filepath']):
continue
if 'checker' in item and \
report.checker_name != item['checker']:
continue

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
Expand All @@ -92,6 +178,8 @@ def get_review_status_from_source(
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,
Expand Down Expand Up @@ -147,4 +235,4 @@ 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)
return self.__source_commets.get(report)
Loading

0 comments on commit c7c6879

Please sign in to comment.