Skip to content

Commit

Permalink
[cmd] Eliminate default checker status
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bruntib committed Jul 20, 2023
1 parent 0cb679e commit 34143c0
Show file tree
Hide file tree
Showing 8 changed files with 44 additions and 70 deletions.
4 changes: 2 additions & 2 deletions analyzer/codechecker_analyzer/analysis_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
7 changes: 7 additions & 0 deletions analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
31 changes: 11 additions & 20 deletions analyzer/codechecker_analyzer/analyzers/config_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

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

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
5 changes: 1 addition & 4 deletions analyzer/codechecker_analyzer/cmd/checkers.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,10 +385,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
Expand Down
4 changes: 3 additions & 1 deletion analyzer/tests/functional/analyze/test_analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
30 changes: 18 additions & 12 deletions analyzer/tests/unit/test_checker_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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):
"""
Expand Down

0 comments on commit 34143c0

Please sign in to comment.