From bd0383f54c3bab55cb5a1fe625087a59abcc02de Mon Sep 17 00:00:00 2001 From: bruntib Date: Fri, 30 Jun 2023 16:19:33 +0200 Subject: [PATCH] [cmd] Eliminate default checker status Currently there are 3 kind of checkers regarding whether they are running during analysis: enabled, disabled, unknown. Checkers can be enabled if they are explicitly given to --enable flag, or they are the member of "default" profile. Checkers can also be explicitly disabled. The 3rd status is tricky: CodeChecker doesn't explicitly enable or disable them, but gives the choice to the analyzer tool. This behavior is bad, because a user cannot tell exactly which checkers were executed during analysis. It is impossible to determine in case of no error if a checker was disabled or just simply didn't report any issue. The goal in this commit is that every checker is either enabled or disabled. It is the analyzer module's responsibility to assemble an analysis command which executes at least the enabled checkers and turns off or hides the disabled ones. --- .../codechecker_analyzer/analysis_manager.py | 4 +-- .../analyzers/clangtidy/analyzer.py | 7 +++++ .../analyzers/clangtidy/config_handler.py | 2 +- .../analyzers/config_handler.py | 31 +++++++------------ .../analyzers/cppcheck/config_handler.py | 31 +------------------ analyzer/codechecker_analyzer/cmd/checkers.py | 13 +++----- .../tests/functional/analyze/test_analyze.py | 4 ++- analyzer/tests/unit/test_checker_handling.py | 30 +++++++++++------- 8 files changed, 48 insertions(+), 74 deletions(-) diff --git a/analyzer/codechecker_analyzer/analysis_manager.py b/analyzer/codechecker_analyzer/analysis_manager.py index 7ad29087e7..a41dc17644 100644 --- a/analyzer/codechecker_analyzer/analysis_manager.py +++ b/analyzer/codechecker_analyzer/analysis_manager.py @@ -329,8 +329,8 @@ def handle_failure( # from the standard output by this postprocess phase so we can present them # as CodeChecker reports. checks = source_analyzer.config_handler.checks() - state = checks.get('clang-diagnostic-error', (CheckerState.default, ''))[0] - if state != CheckerState.disabled: + state = checks.get('clang-diagnostic-error', (CheckerState.enabled, ''))[0] + if state == CheckerState.enabled: rh.postprocess_result(skip_handlers) # Remove files that successfully analyzed earlier on. diff --git a/analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py b/analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py index 7bc5d92b95..284649cfad 100644 --- a/analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py +++ b/analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py @@ -290,6 +290,13 @@ def get_checker_list(self, config) -> Tuple[List[str], List[str]]: warning_name, warning_type = \ get_compiler_warning_name_and_type(checker_name) + + # This warning must be given a parameter separated by either '=' or + # space. This warning is not supported as a checker name so its + # special usage is avoided. + if warning_name and warning_name.startswith('frame-larger-than'): + continue + if warning_name is not None: # -W and clang-diagnostic- are added as compiler warnings. if warning_type == CheckerType.compiler: diff --git a/analyzer/codechecker_analyzer/analyzers/clangtidy/config_handler.py b/analyzer/codechecker_analyzer/analyzers/clangtidy/config_handler.py index 27200d9452..35a0d445ff 100644 --- a/analyzer/codechecker_analyzer/analyzers/clangtidy/config_handler.py +++ b/analyzer/codechecker_analyzer/analyzers/clangtidy/config_handler.py @@ -33,7 +33,7 @@ def __init__(self): super(ClangTidyConfigHandler, self).__init__() def add_checker(self, checker_name, description='', - state=CheckerState.default): + state=CheckerState.disabled): """ Add additional checker if the 'take-config-from-directory' analyzer configuration option is not set. diff --git a/analyzer/codechecker_analyzer/analyzers/config_handler.py b/analyzer/codechecker_analyzer/analyzers/config_handler.py index b346f8b41d..e575c5ea57 100644 --- a/analyzer/codechecker_analyzer/analyzers/config_handler.py +++ b/analyzer/codechecker_analyzer/analyzers/config_handler.py @@ -21,18 +21,13 @@ LOG = get_logger('system') -# The baseline handling of checks in every analyzer is to let the analysis -# engine decide which checks are worthwhile run. Checks handled this way -# (implicitly by the analyzer) are considered to have a CheckerState of -# default. If the check however appears in profiles, and such a profile is -# enabled explicitly on the command-line or implicitly as in case of the -# default profile, then they are considered to have a CheckerState of enabled. -# Likewise for individually enabled checks. If a check is however explicitly -# disabled on the command-line, or belongs to a profile explicitly disabled -# on the command-line, then it is considered to have a CheckerState of -# disabled. +# If the check appears in profiles, and such a profile is enabled explicitly on +# the command-line or implicitly as in case of the default profile, then they +# are considered to have a CheckerState of enabled. Likewise for individually +# enabled checks. If a check is however explicitly disabled on the +# command-line, or belongs to a profile explicitly disabled on the +# command-line, then it is considered to have a CheckerState of disabled. class CheckerState(Enum): - default = 0 disabled = 1 enabled = 2 @@ -77,17 +72,14 @@ def __init__(self): self.report_hash = None self.enable_all = None - # The key is the checker name, the value is a tuple. - # False if disabled (should be by default). - # True if checker is enabled. - # (False/True, 'checker_description') + # The key is the checker name, the value is a tuple of CheckerState and + # checker description. self.__available_checkers = collections.OrderedDict() def add_checker(self, checker_name, description='', - state=CheckerState.default): + state=CheckerState.disabled): """ - Add additional checker. If no state argument is given, the actual usage - of the checker is handled by the analyzer. + Add a checker to the available checkers' list. """ self.__available_checkers[checker_name] = (state, description) @@ -171,8 +163,7 @@ def initialize_checkers(self, checker_labels = analyzer_context.get_context().checker_labels - # Add all checkers marked as default. This means the analyzer should - # manage whether it is enabled or disabled. + # Add all checkers marked as disabled. for checker_name, description in checkers: self.add_checker(checker_name, description) diff --git a/analyzer/codechecker_analyzer/analyzers/cppcheck/config_handler.py b/analyzer/codechecker_analyzer/analyzers/cppcheck/config_handler.py index b99a19c856..11c41ce19d 100644 --- a/analyzer/codechecker_analyzer/analyzers/cppcheck/config_handler.py +++ b/analyzer/codechecker_analyzer/analyzers/cppcheck/config_handler.py @@ -9,39 +9,10 @@ Config handler for Cppcheck analyzer. """ from .. import config_handler -from ..config_handler import CheckerState class CppcheckConfigHandler(config_handler.AnalyzerConfigHandler): """ Configuration handler for Cppcheck analyzer. """ - def initialize_checkers(self, - checkers, - cmdline_enable=None, - enable_all=False): - if not cmdline_enable: - cmdline_enable = list() - """ - Set all the default checkers to disabled. This will ensure that - --enable=all will not run with all the possible checkers - """ - super().initialize_checkers( - checkers, - cmdline_enable, - enable_all) - - # Set all the checkers with default CheckerState checkers to - # disabled. This will ensure that --enable=all will not run with - # all the possible checkers. All the checkers that are in the default - # profile (or configured othewise, eg.: from the cli) should be - # already enabled at this point. - # This happens in two phases in order to avoid iterator invalidation. - # (self.set_checker_enabled() removes elements, so we can't use it - # while iterating over the checker list.) - default_state_checkers = [ - checker_name for checker_name, data in - self.checks().items() if data[0] == CheckerState.default] - - for checker_name in default_state_checkers: - self.set_checker_enabled(checker_name, enabled=False) + pass diff --git a/analyzer/codechecker_analyzer/cmd/checkers.py b/analyzer/codechecker_analyzer/cmd/checkers.py index cd28e3b430..273ac632e3 100644 --- a/analyzer/codechecker_analyzer/cmd/checkers.py +++ b/analyzer/codechecker_analyzer/cmd/checkers.py @@ -260,9 +260,11 @@ def __get_detailed_checker_info( config_handler.initialize_checkers(checkers, profile_checkers) for checker, (state, description) in config_handler.checks().items(): + labels = cl.labels_of_checker(checker, analyzer) + state = CheckerState.enabled if ('profile', 'default') in labels \ + else CheckerState.disabled checker_info[analyzer].append( - (state, checker, analyzer, description, - sorted(cl.labels_of_checker(checker, analyzer)))) + (state, checker, analyzer, description, sorted(labels))) return checker_info @@ -385,10 +387,7 @@ def __format_row(row: Tuple) -> Tuple: row -- A tuple with detailed checker info coming from __get_detailed_checker_info() function. """ - state = '+' if row[0] == CheckerState.enabled else \ - '-' if row[0] == CheckerState.disabled else \ - '?' - + state = '+' if row[0] == CheckerState.enabled else '-' labels = ', '.join(f'{k}:{v}' for k, v in row[4]) return state, row[1], row[2], row[3], labels @@ -404,8 +403,6 @@ def __print_checkers_custom_format(checkers: Iterable): status = 'enabled' elif checker[0] == CheckerState.disabled: status = 'disabled' - else: - status = 'unknown' print(checker[1]) print(' Status:', status) diff --git a/analyzer/tests/functional/analyze/test_analyze.py b/analyzer/tests/functional/analyze/test_analyze.py index 0f19ac8659..af37563e9a 100644 --- a/analyzer/tests/functional/analyze/test_analyze.py +++ b/analyzer/tests/functional/analyze/test_analyze.py @@ -1046,7 +1046,9 @@ def test_makefile_generation(self): """ Test makefile generation. """ build_json = os.path.join(self.test_workspace, "build_extra_args.json") analyze_cmd = [self._codechecker_cmd, "analyze", build_json, - "-o", self.report_dir, '--makefile'] + "-o", self.report_dir, + "-e", "clang-diagnostic", + '--makefile'] source_file = os.path.join(self.test_dir, "extra_args.cpp") build_log = [{"directory": self.test_workspace, diff --git a/analyzer/tests/unit/test_checker_handling.py b/analyzer/tests/unit/test_checker_handling.py index fa02da5bac..9ca2fb3424 100644 --- a/analyzer/tests/unit/test_checker_handling.py +++ b/analyzer/tests/unit/test_checker_handling.py @@ -108,9 +108,15 @@ def test_no_disabled_checks(self): """ Test that ClangSA only uses enable lists. """ - self.assertFalse( - any(arg.startswith('-analyzer-disable-checker') - for arg in self.__class__.cmd)) + # TODO: This test is currently removed, because checkers that are not + # enabled are explicitly disabled. In a next commit ClangSA reports + # will be hidden instead of disabled. In that commit this test could be + # re-enabled. + pass + + # self.assertFalse( + # any(arg.startswith('-analyzer-disable-checker') + # for arg in self.__class__.cmd)) def test_checker_initializer(self): """ @@ -154,19 +160,19 @@ def f(checks, checkers): checkers.extend(map(add_description, cert_guideline)) # "default" profile checkers are enabled explicitly. Others are in - # "default" state. + # "disabled" state. cfg_handler = ClangSA.construct_config_handler(args) cfg_handler.initialize_checkers(checkers) self.assertTrue(all_with_status(CheckerState.enabled) (cfg_handler.checks(), default_profile)) - self.assertTrue(all_with_status(CheckerState.default) + self.assertTrue(all_with_status(CheckerState.disabled) (cfg_handler.checks(), security_profile_alpha)) - # "--enable-all" leaves alpha checkers in "default" state. Others + # "--enable-all" leaves alpha checkers in "disabled" state. Others # become enabled. cfg_handler = ClangSA.construct_config_handler(args) cfg_handler.initialize_checkers(checkers, enable_all=True) - self.assertTrue(all_with_status(CheckerState.default) + self.assertTrue(all_with_status(CheckerState.disabled) (cfg_handler.checks(), security_profile_alpha)) self.assertTrue(all_with_status(CheckerState.enabled) (cfg_handler.checks(), default_profile)) @@ -307,15 +313,15 @@ def test_default_checkers_are_not_disabled(self): self.assertFalse('-*' in self.__class__.checks_list) - def test_only_clangsa_analyzer_checks_are_disabled(self): + def test_clangsa_analyzer_checks_are_disabled(self): """ - Test that exactly the clang-analyzer group is disabled in Clang Tidy. + Test that the clang-analyzer group is disabled in Clang Tidy. """ self.assertTrue('-clang-analyzer-*' in self.__class__.checks_list) - self.assertFalse( - any(check.startswith('-') and check != '-clang-analyzer-*' - for check in self.__class__.checks_list)) + # self.assertFalse( + # any(check.startswith('-') and check != '-clang-analyzer-*' + # for check in self.__class__.checks_list)) def test_clang_diags_as_compiler_warnings(self): """