From d290f48fddd47372680bdcc3f089e1e15b718fe6 Mon Sep 17 00:00:00 2001 From: Gyorgy Orban Date: Mon, 29 Jan 2018 15:30:58 +0100 Subject: [PATCH] refactor plist to plaintext formatting (parse) Plist parsing and formatting used by the old architecture is not the way it should be handled now. It was not flexible and it was hard to improve it. It should be easier to test and setup the output formatting after the changes. Skip lists were not supported by the parse command, this commit will add the skip files support for parse too. --- .../analyze/analyzers/analyzer_types.py | 20 -- .../analyzers/result_handler_clang_tidy.py | 18 -- .../result_handler_plist_to_stdout.py | 210 ------------------ libcodechecker/analyze/plist_parser.py | 190 +++++++++++++++- libcodechecker/libhandlers/parse.py | 40 ++-- 5 files changed, 213 insertions(+), 265 deletions(-) delete mode 100644 libcodechecker/analyze/analyzers/result_handler_plist_to_stdout.py diff --git a/libcodechecker/analyze/analyzers/analyzer_types.py b/libcodechecker/analyze/analyzers/analyzer_types.py index 8e60b68182..24276385ba 100644 --- a/libcodechecker/analyze/analyzers/analyzer_types.py +++ b/libcodechecker/analyze/analyzers/analyzer_types.py @@ -20,7 +20,6 @@ from libcodechecker.analyze.analyzers import config_handler_clangsa from libcodechecker.analyze.analyzers import result_handler_base from libcodechecker.analyze.analyzers import result_handler_clang_tidy -from libcodechecker.analyze.analyzers import result_handler_plist_to_stdout from libcodechecker.logger import get_logger LOG = get_logger('analyzer') @@ -456,22 +455,3 @@ def construct_analyze_handler(buildaction, res_handler.severity_map = severity_map res_handler.skiplist_handler = skiplist_handler return res_handler - - -def construct_parse_handler(buildaction, - output, - severity_map, - suppress_handler, - print_steps): - """ - Construct a result handler for parsing results in a human-readable format. - """ - res_handler = result_handler_plist_to_stdout.PlistToStdout( - buildaction, - output, - None) - - res_handler.print_steps = print_steps - res_handler.severity_map = severity_map - res_handler.suppress_handler = suppress_handler - return res_handler diff --git a/libcodechecker/analyze/analyzers/result_handler_clang_tidy.py b/libcodechecker/analyze/analyzers/result_handler_clang_tidy.py index 723775867d..58dc3c9212 100644 --- a/libcodechecker/analyze/analyzers/result_handler_clang_tidy.py +++ b/libcodechecker/analyze/analyzers/result_handler_clang_tidy.py @@ -7,8 +7,6 @@ from libcodechecker.analyze import tidy_output_converter from libcodechecker.analyze.analyzers.result_handler_base \ import ResultHandler -from libcodechecker.analyze.analyzers.result_handler_plist_to_stdout \ - import PlistToStdout from libcodechecker.logger import get_logger LOG = get_logger('report') @@ -42,19 +40,3 @@ def postprocess_result(self): LOG.debug_analyzer(self.analyzer_stdout) tidy_stdout = self.analyzer_stdout.splitlines() generate_plist_from_tidy_result(output_file, tidy_stdout) - - -class ClangTidyPlistToStdout(PlistToStdout): - """ - Print the clang tidy results to the standard output. - """ - - def postprocess_result(self): - """ - Clang-tidy results are post processed to have the same format as the - clang static analyzer result files. - """ - - output_file = self.analyzer_result_file - tidy_stdout = self.analyzer_stdout.splitlines() - generate_plist_from_tidy_result(output_file, tidy_stdout) diff --git a/libcodechecker/analyze/analyzers/result_handler_plist_to_stdout.py b/libcodechecker/analyze/analyzers/result_handler_plist_to_stdout.py deleted file mode 100644 index aadcce285d..0000000000 --- a/libcodechecker/analyze/analyzers/result_handler_plist_to_stdout.py +++ /dev/null @@ -1,210 +0,0 @@ -# ------------------------------------------------------------------------- -# The CodeChecker Infrastructure -# This file is distributed under the University of Illinois Open Source -# License. See LICENSE.TXT for details. -# ------------------------------------------------------------------------- - -from abc import ABCMeta -from collections import defaultdict -import math -import os -import sys - -from libcodechecker import suppress_handler -from libcodechecker import util -from libcodechecker.analyze import plist_parser -from libcodechecker.analyze.analyzers.result_handler_base import ResultHandler -from libcodechecker.logger import get_logger - -LOG = get_logger('report') - - -class PlistToStdout(ResultHandler): - """ - Result handler for processing a plist file with the - analysis results and print them to the standard output. - """ - - __metaclass__ = ABCMeta - - def __init__(self, buildaction, workspace, lock): - super(PlistToStdout, self).__init__(buildaction, workspace) - self.__print_steps = False - self.__output = sys.stdout - self.__lock = lock - self.suppress_handler = None - - @property - def print_steps(self): - """ - Print the multiple steps for a bug if there is any. - """ - return self.__print_steps - - @print_steps.setter - def print_steps(self, value): - """ - Print the multiple steps for a bug if there is any. - """ - self.__print_steps = value - - @staticmethod - def __format_location(event, source_file): - loc = event['location'] - line = util.get_line(source_file, loc['line']) - if line == '': - return line - - marker_line = line[0:(loc['col'] - 1)] - marker_line = ' ' * (len(marker_line) + marker_line.count('\t')) - return '%s%s^' % (line.replace('\t', ' '), marker_line) - - @staticmethod - def __format_bug_event(name, severity, event, source_file): - - loc = event['location'] - fname = os.path.basename(source_file) - if name: - return '[%s] %s:%d:%d: %s [%s]' % (severity, - fname, - loc['line'], - loc['col'], - event['message'], - name) - else: - return '%s:%d:%d: %s' % (fname, - loc['line'], - loc['col'], - event['message']) - - def __print_bugs(self, reports, files, report_stats): - - severity_stats = defaultdict(int) - file_stats = defaultdict(int) - report_count = defaultdict(int) - - report_num = len(reports) - if report_num > 0: - index_format = ' %%%dd, ' % \ - int(math.floor(math.log10(report_num)) + 1) - - non_suppressed = 0 - for report in reports: - events = [i for i in report.bug_path if i.get('kind') == 'event'] - f_path = files[events[-1]['location']['file']] - if self.skiplist_handler and \ - self.skiplist_handler.should_skip(f_path): - LOG.debug(report + ' is skipped (in ' + f_path + ")") - continue - hash_value = report.main['issue_hash_content_of_line_in_context'] - bug = {'hash_value': hash_value, - 'file_path': f_path - } - if self.suppress_handler and \ - self.suppress_handler.get_suppressed(bug): - LOG.debug("Suppressed by suppress file: {0}".format(report)) - continue - - last_report_event = report.bug_path[-1] - source_file = files[last_report_event['location']['file']] - report_line = last_report_event['location']['line'] - report_hash = report.main['issue_hash_content_of_line_in_context'] - checker_name = report.main['check_name'] - sp_handler = suppress_handler.SourceSuppressHandler(source_file, - report_line, - report_hash, - checker_name) - - # Check for suppress comment. - suppress_data = sp_handler.get_suppressed() - if suppress_data: - if self.suppress_handler: - LOG.info("Writing source-code suppress at '{0}:{1}' to " - "suppress file".format(source_file, report_line)) - hash_value, file_name, comment = suppress_data - self.suppress_handler.store_suppress_bug_id(hash_value, - file_name, - comment) - - continue - - file_stats[f_path] += 1 - severity = self.severity_map.get(checker_name, - 'UNSPECIFIED') - severity_stats[severity] += 1 - report_count["report_count"] += 1 - - self.__output.write(self.__format_bug_event(checker_name, - severity, - last_report_event, - source_file)) - self.__output.write('\n') - self.__output.write(self.__format_location(last_report_event, - source_file)) - self.__output.write('\n') - if self.__print_steps: - self.__output.write(' Report hash: ' + report_hash + '\n') - self.__output.write(' Steps:\n') - for index, event in enumerate(events): - self.__output.write(index_format % (index + 1)) - source_file = files[event['location']['file']] - self.__output.write(self.__format_bug_event(None, - None, - event, - source_file)) - self.__output.write('\n') - self.__output.write('\n') - - non_suppressed += 1 - - basefile_print = (' ' + - os.path.basename(self.analyzed_source_file)) \ - if self.analyzed_source_file and \ - len(self.analyzed_source_file) > 0 else '' - - if non_suppressed == 0: - self.__output.write('Found no defects while analyzing%s\n' % - (basefile_print)) - else: - self.__output.write( - 'Found %d defect(s) while analyzing%s\n\n' % - (non_suppressed, basefile_print)) - - report_stats["severity"] = severity_stats - report_stats["files"] = file_stats - report_stats["reports"] = report_count - - def handle_results(self, client=None): - plist = self.analyzer_result_file - - report_stats = {} - - try: - files, reports = plist_parser.parse_plist(plist) - except Exception as ex: - LOG.error('The generated plist is not valid!') - LOG.error(ex) - return 1 - - err_code = self.analyzer_returncode - - if err_code == 0: - try: - # No lock when consuming plist. - if self.__lock: - self.__lock.acquire() - self.__print_bugs(reports, files, report_stats) - finally: - if self.__lock: - self.__lock.release() - else: - self.__output.write('Analyzing %s with %s failed.\n' % - (os.path.basename(self.analyzed_source_file), - self.buildaction.analyzer_type)) - return report_stats - - def postprocess_result(self): - """ - No postprocessing required for plists. - """ - pass diff --git a/libcodechecker/analyze/plist_parser.py b/libcodechecker/analyze/plist_parser.py index 0bfa45271b..7a5dfcad4a 100644 --- a/libcodechecker/analyze/plist_parser.py +++ b/libcodechecker/analyze/plist_parser.py @@ -30,14 +30,17 @@ """ +from collections import defaultdict +import math import os import plistlib import sys import traceback from xml.parsers.expat import ExpatError +from libcodechecker import suppress_handler +from libcodechecker import util from libcodechecker.logger import get_logger - from libcodechecker.report import Report from libcodechecker.report import generate_report_hash @@ -268,3 +271,188 @@ def skip_report_from_plist(plist_file, skip_handler): plist.seek(0) plist.write(new_plist_content) plist.truncate() + + +class PlistToPlaintextFormatter(object): + """ + Parse and format plist reports to a more human readable format. + """ + + def __init__(self, + suppress_handler, + skip_handler, + severity_map, + analyzer_type="clangsa"): + + self.__analyzer_type = analyzer_type + self.__severity_map = severity_map + self.suppress_handler = suppress_handler + + self.__print_steps = False + + # FIXME do we need a skip list handler for parse here??? + self.skiplist_handler = skip_handler + + @property + def print_steps(self): + """ + Print the multiple steps for a bug if there is any. + """ + return self.__print_steps + + @print_steps.setter + def print_steps(self, value): + """ + Print the multiple steps for a bug if there is any. + """ + self.__print_steps = value + + @staticmethod + def __format_location(event, source_file): + loc = event['location'] + line = util.get_line(source_file, loc['line']) + if line == '': + return line + + marker_line = line[0:(loc['col'] - 1)] + marker_line = ' ' * (len(marker_line) + marker_line.count('\t')) + return '%s%s^' % (line.replace('\t', ' '), marker_line) + + @staticmethod + def __format_bug_event(name, severity, event, source_file): + + loc = event['location'] + fname = os.path.basename(source_file) + if name: + return '[%s] %s:%d:%d: %s [%s]' % (severity, + fname, + loc['line'], + loc['col'], + event['message'], + name) + else: + return '%s:%d:%d: %s' % (fname, + loc['line'], + loc['col'], + event['message']) + + def __write_bugs(self, output, reports, files, analyzed_source_file, + report_stats): + """ + Write out the bugs to the output and collect report statistics. + """ + + severity_stats = defaultdict(int) + file_stats = defaultdict(int) + report_count = defaultdict(int) + + report_num = len(reports) + if report_num > 0: + index_format = ' %%%dd, ' % \ + int(math.floor(math.log10(report_num)) + 1) + + non_suppressed = 0 + for report in reports: + events = [i for i in report.bug_path if i.get('kind') == 'event'] + f_path = files[events[-1]['location']['file']] + if self.skiplist_handler and \ + self.skiplist_handler.should_skip(f_path): + LOG.debug(report + ' is skipped (in ' + f_path + ")") + continue + hash_value = report.main['issue_hash_content_of_line_in_context'] + bug = {'hash_value': hash_value, + 'file_path': f_path + } + if self.suppress_handler and \ + self.suppress_handler.get_suppressed(bug): + LOG.debug("Suppressed by suppress file: {0}".format(report)) + continue + + last_report_event = report.bug_path[-1] + source_file = files[last_report_event['location']['file']] + report_line = last_report_event['location']['line'] + report_hash = report.main['issue_hash_content_of_line_in_context'] + checker_name = report.main['check_name'] + sp_handler = suppress_handler.SourceSuppressHandler(source_file, + report_line, + report_hash, + checker_name) + + # Check for suppress comment. + suppress_data = sp_handler.get_suppressed() + if suppress_data: + if self.suppress_handler: + LOG.info("Writing source-code suppress at '{0}:{1}' to " + "suppress file".format(source_file, report_line)) + hash_value, file_name, comment = suppress_data + self.suppress_handler.store_suppress_bug_id(hash_value, + file_name, + comment) + + continue + + file_stats[f_path] += 1 + severity = self.__severity_map.get(checker_name, + 'UNSPECIFIED') + severity_stats[severity] += 1 + report_count["report_count"] += 1 + + output.write(self.__format_bug_event(checker_name, + severity, + last_report_event, + source_file)) + output.write('\n') + output.write(self.__format_location(last_report_event, + source_file)) + output.write('\n') + + if self.print_steps: + output.write(' Report hash: ' + report_hash + '\n') + output.write(' Steps:\n') + for index, event in enumerate(events): + output.write(index_format % (index + 1)) + source_file = files[event['location']['file']] + output.write(self.__format_bug_event(None, + None, + event, + source_file)) + output.write('\n') + output.write('\n') + + non_suppressed += 1 + + basefile_print = (' ' + + os.path.basename(analyzed_source_file)) \ + if analyzed_source_file and \ + len(analyzed_source_file) > 0 else '' + + if non_suppressed == 0: + output.write('Found no defects while analyzing%s\n' % + (basefile_print)) + else: + output.write( + 'Found %d defect(s) while analyzing%s\n\n' % + (non_suppressed, basefile_print)) + + report_stats["severity"] = severity_stats + report_stats["files"] = file_stats + report_stats["reports"] = report_count + + def parse_and_write(self, plist_file, analyzed_source_file, + output=sys.stdout): + """ + Parse a plist report file format it to a more human readable format. + """ + report_stats = {} + try: + files, reports = parse_plist(plist_file) + except Exception as ex: + traceback.print_stack() + LOG.error('The generated plist is not valid!') + LOG.error(ex) + return 1 + + self.__write_bugs(output, reports, files, analyzed_source_file, + report_stats) + + return report_stats diff --git a/libcodechecker/libhandlers/parse.py b/libcodechecker/libhandlers/parse.py index 17ca50142e..ca49da2b30 100644 --- a/libcodechecker/libhandlers/parse.py +++ b/libcodechecker/libhandlers/parse.py @@ -20,9 +20,9 @@ from libcodechecker import generic_package_suppress_handler from libcodechecker import logger from libcodechecker import util -from libcodechecker.analyze.analyzers import analyzer_types +from libcodechecker.analyze import plist_parser +from libcodechecker.analyze.skiplist_handler import SkipListHandler # TODO: This is a cross-subpackage reference... -from libcodechecker.log import build_action from libcodechecker.output_formatters import twodim_to_str LOG = logger.get_logger('system') @@ -129,6 +129,15 @@ def add_arguments_to_parser(parser): help="Print the steps the analyzers took in finding " "the reported defect.") + parser.add_argument('-i', '--ignore', '--skip', + dest="skipfile", + required=False, + default=argparse.SUPPRESS, + help="Path to the Skipfile dictating which project " + "files should be omitted from analysis. Please " + "consult the User guide on how a Skipfile " + "should be laid out.") + logger.add_verbose_arguments(parser) def __handle(args): @@ -151,7 +160,7 @@ def arg_match(options): parser.set_defaults(func=__handle) -def parse(f, context, metadata_dict, suppress_handler, steps): +def parse(f, context, metadata_dict, suppress_handler, skip_handler, steps): """ Prints the results in the given file to the standard output in a human- readable format. @@ -165,27 +174,21 @@ def parse(f, context, metadata_dict, suppress_handler, steps): LOG.debug("Parsing input file '" + f + "'") - buildaction = build_action.BuildAction() + rh = plist_parser.PlistToPlaintextFormatter(suppress_handler, + skip_handler, + context.severity_map) - rh = analyzer_types.construct_parse_handler(buildaction, - f, - context.severity_map, - suppress_handler, - steps) + rh.print_steps = steps # Set some variables of the result handler to use the saved file. - rh.analyzer_returncode = 0 - rh.analyzer_result_file = f - rh.analyzer_cmd = "" + analyzed_source_file = "UNKNOWN" if 'result_source_files' in metadata_dict and \ f in metadata_dict['result_source_files']: - rh.analyzed_source_file = \ + analyzed_source_file = \ metadata_dict['result_source_files'][f] - else: - rh.analyzed_source_file = "UNKNOWN" - return rh.handle_results() + return rh.parse_and_write(f, analyzed_source_file) def main(args): @@ -231,6 +234,10 @@ def main(args): "SUPPRESS_FILE' is also given.") sys.exit(2) + skip_handler = None + if 'skipfile' in args: + skip_handler = SkipListHandler(args.skipfile) + for input_path in args.input: input_path = os.path.abspath(input_path) @@ -276,6 +283,7 @@ def main(args): context, metadata_dict, suppress_handler, + skip_handler, 'print_steps' in args) severity_stats.update(Counter(report_stats.get('severity',