Skip to content

Commit

Permalink
[fix] Enable and disable checkers
Browse files Browse the repository at this point in the history
This PR fixes the enabling and disabling checker options, focusing on resolving ambiguities in option processing.

Key updates:
	Ambiguity handling:
		When ambiguous checker options are provided with `-e` or `-d` flags, an error is now raised. The user receives suggestions for specifying the option by using namespaces (checker, prefix, guideline, etc.).
		If a checker name matches multiple checkers as a prefix, it triggers an ambiguity error. Suggestions are provided to help users choose between the checker name or the prefix.

	Additional namespaces:
		A new `checker` namespace is created. All label types can be a namespace as well, specified with a `:` separator before the checker option.

	Default profile settings improvements:
		Only those checkers enabled that has profile:default label in the label files. Prefix-based configuration is no longer applied for the default profile, ensuring precise and predictable behavior.

	Getting rid of `W` prefix:
		It is no longer available to enable or disable a warrning with W.
  • Loading branch information
cservakt committed Dec 3, 2024
1 parent b0555b9 commit e6a8c5d
Show file tree
Hide file tree
Showing 14 changed files with 319 additions and 131 deletions.
3 changes: 3 additions & 0 deletions analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@ def get_analyzer_checkers(cls):
("clang-diagnostic-" + warning, "")
for warning in get_warnings())

checker_description.append(("clang-diagnostic-error",
"Indicates compiler errors."))

cls.__analyzer_checkers = checker_description

return checker_description
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,3 @@ def add_checker(self, checker_name, description='',
return

super().add_checker(checker_name, description, state)

def set_checker_enabled(self, checker_name, enabled=True):
"""
Enable checker, keep description if already set.
"""
if checker_name.startswith('W') or \
checker_name.startswith('clang-diagnostic'):
self.add_checker(checker_name)

super().set_checker_enabled(checker_name, enabled)
114 changes: 66 additions & 48 deletions analyzer/codechecker_analyzer/analyzers/config_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@

from abc import ABCMeta
from enum import Enum
from string import Template
import collections
import platform
import sys
Expand Down Expand Up @@ -86,15 +85,16 @@ def add_checker(self, checker_name, description='',
"""
self.__available_checkers[checker_name] = (state, description)

def set_checker_enabled(self, checker_name, enabled=True):
def set_checker_enabled(self, checker_name, enabled=True, is_strict=False):
"""
Explicitly handle checker state, keep description if already set.
"""
changed_states = []
regex = "^" + re.escape(str(checker_name)) + "\\b.*$"

for ch_name, values in self.__available_checkers.items():
if re.match(regex, ch_name):
if (is_strict and ch_name == checker_name) \
or (not is_strict and re.match(regex, ch_name)):
_, description = values
state = CheckerState.ENABLED if enabled \
else CheckerState.DISABLED
Expand All @@ -118,7 +118,7 @@ def checks(self):
"""
return self.__available_checkers

def __gen_name_variations(self):
def __gen_name_variations(self, only_prefix=False):
"""
Generate all applicable name variations from the given checker list.
"""
Expand All @@ -134,9 +134,9 @@ def __gen_name_variations(self):
# ['misc', 'misc-dangling', 'misc-dangling-handle']
# from 'misc-dangling-handle'.
v = [delim.join(parts[:(i + 1)]) for i in range(len(parts))]
reserved_names += v
reserved_names += v[:-1] if only_prefix else v

return reserved_names
return list(set(reserved_names))

def initialize_checkers(self,
checkers,
Expand Down Expand Up @@ -184,7 +184,7 @@ def initialize_checkers(self,
else:
# Turn default checkers on.
for checker in default_profile_checkers:
self.set_checker_enabled(checker)
self.set_checker_enabled(checker, is_strict=True)

self.enable_all = enable_all
# If enable_all is given, almost all checkers should be enabled.
Expand All @@ -207,48 +207,66 @@ def initialize_checkers(self,
self.set_checker_enabled(checker_name)

# Set user defined enabled or disabled checkers from the command line.

# Construct a list of reserved checker names.
# (It is used to check if a profile name is valid.)
reserved_names = self.__gen_name_variations()
profiles = checker_labels.get_description('profile')
guidelines = checker_labels.occurring_values('guideline')

templ = Template("The ${entity} name '${identifier}' conflicts with a "
"checker name prefix '${identifier}'. Please use -e "
"${entity}:${identifier} to enable checkers of the "
"${identifier} ${entity} or use -e "
"prefix:${identifier} to select checkers which have "
"a name starting with '${identifier}'.")

for identifier, enabled in cmdline_enable:
if "prefix:" in identifier:
identifier = identifier.replace("prefix:", "")
self.set_checker_enabled(identifier, enabled)

elif ':' in identifier:
for checker in checker_labels.checkers_by_labels([identifier]):
self.set_checker_enabled(checker, enabled)

elif identifier in profiles:
if identifier in reserved_names:
LOG.error(templ.substitute(entity="profile",
identifier=identifier))
sys.exit(1)
else:
for checker in checker_labels.checkers_by_labels(
[f'profile:{identifier}']):
self.set_checker_enabled(checker, enabled)

elif identifier in guidelines:
if identifier in reserved_names:
LOG.error(templ.substitute(entity="guideline",
identifier=identifier))
labels = checker_labels.labels() \
if callable(getattr(checker_labels, 'labels', None)) \
else ["guideline", "profile", "severity", "sei-cert"]

if ":" in identifier:
identifier_namespace = identifier.split(":")[0]
all_namespaces = ["checker", "prefix"] + labels

if identifier_namespace not in all_namespaces:
LOG.error("The %s namespace is not known. Please select"
"one of these existing namespace options: %s",
identifier_namespace, ", ".join(all_namespaces))
sys.exit(1)
else:
for checker in checker_labels.checkers_by_labels(
[f'guideline:{identifier}']):
self.set_checker_enabled(checker, enabled)

identifier = identifier.split(":", 1)[1]
self.initialize_checkers_by_namespace(
identifier_namespace, identifier, enabled)

else:
self.set_checker_enabled(identifier, enabled)
all_options = dict(zip(labels, map(
checker_labels.occurring_values, labels)))

all_options["prefix"] = list(set(self.__gen_name_variations(
only_prefix=True)))

all_options["checker"] = self.__available_checkers

possible_options = {}
for label, options in all_options.items():
if identifier in options:
possible_options[label] = identifier

if len(possible_options) == 1:
self.initialize_checkers_by_namespace(
*list(possible_options.items())[0], enabled)
elif len(possible_options) > 1:
error_options = ", ".join(f"{label}:{option}"
for label, option
in possible_options.items())

LOG.error("The %s is ambigous. Please select one of these"
" options to clarify the checker list: %s.",
identifier, error_options)
sys.exit(1)
else:
# The identifier is not known but we just pass it
# and handle it in a different section.
continue

def initialize_checkers_by_namespace(self,
identifier_namespace,
identifier,
enabled):
if identifier_namespace == "checker":
self.set_checker_enabled(identifier, enabled, is_strict=True)
elif identifier_namespace == "prefix":
self.set_checker_enabled(identifier, enabled)
else:
checker_labels = analyzer_context.get_context().checker_labels
for checker in checker_labels.checkers_by_labels(
[f"{identifier_namespace}:{identifier}"]):
self.set_checker_enabled(checker, enabled, is_strict=True)
7 changes: 1 addition & 6 deletions analyzer/codechecker_analyzer/checkers.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,7 @@ def available(ordered_checkers, available_checkers):
"""
missing_checkers = set()
for checker_name, _ in ordered_checkers:
# TODO: This label list shouldn't be hard-coded here.
if checker_name.startswith('profile:') or \
checker_name.startswith('guideline:') or \
checker_name.startswith('severity:') or \
checker_name.startswith('sei-cert:') or \
checker_name.startswith('prefix:'):
if ":" in checker_name:
continue

name_match = False
Expand Down
14 changes: 10 additions & 4 deletions analyzer/codechecker_analyzer/cmd/analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -773,8 +773,11 @@ def add_arguments_to_parser(parser):
"between the checker prefix "
"group/profile/guideline name, the use of "
"one of the following labels is "
"mandatory: 'prefix:', 'profile:', "
"'guideline:'.")
"mandatory: 'checker:', 'prefix:', "
"'profile:', 'guideline:'. If a checker "
"name matches multiple checkers as a "
"prefix, 'checker:' or 'prefix:' "
"namespace is required")

checkers_opts.add_argument('-d', '--disable',
dest="disable",
Expand All @@ -792,8 +795,11 @@ def add_arguments_to_parser(parser):
"between the checker prefix "
"group/profile/guideline name, the use of "
"one of the following labels is "
"mandatory: 'prefix:', 'profile:', "
"'guideline:'.")
"mandatory: 'checker:', 'prefix:', "
"'profile:', 'guideline:'. If a checker "
"name matches multiple checkers as a "
"prefix, 'checker:' or 'prefix:' "
"namespace is required")

checkers_opts.add_argument('--enable-all',
dest="enable_all",
Expand Down
38 changes: 23 additions & 15 deletions analyzer/codechecker_analyzer/cmd/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -707,35 +707,43 @@ def add_arguments_to_parser(parser):
metavar='checker/group/profile',
default=argparse.SUPPRESS,
action=OrderedCheckersAction,
help="Set a checker (or checker group), "
"profile or guideline "
"to BE USED in the analysis. In case of "
"ambiguity the priority order is profile, "
"guideline, checker name (e.g. security "
"means the profile, not the checker "
"group). Moreover, labels can also be "
help="Set a checker (or checker prefix group), "
"profile or guideline to BE USED in the "
"analysis. Labels can also be "
"used for selecting checkers, for example "
"profile:extreme or severity:STYLE. See "
"'CodeChecker checkers --label' for "
"further details.")
"further details. In case of a name clash "
"between the checker prefix "
"group/profile/guideline name, the use of "
"one of the following labels is "
"mandatory: 'checker:', 'prefix:', "
"'profile:', 'guideline:'. If a checker "
"name matches multiple checkers as a "
"prefix, 'checker:' or 'prefix:' "
"namespace is required")

checkers_opts.add_argument('-d', '--disable',
dest="disable",
metavar='checker/group/profile',
default=argparse.SUPPRESS,
action=OrderedCheckersAction,
help="Set a checker (or checker group), "
help="Set a checker (or checker prefix group), "
"profile or guideline "
"to BE PROHIBITED from use in the "
"analysis. In case of "
"ambiguity the priority order is profile, "
"guideline, checker name (e.g. security "
"means the profile, not the checker "
"group). Moreover, labels can also be "
"analysis. Labels can also be "
"used for selecting checkers, for example "
"profile:extreme or severity:STYLE. See "
"'CodeChecker checkers --label' for "
"further details.")
"further details. In case of a name clash "
"between the checker prefix "
"group/profile/guideline name, the use of "
"one of the following labels is "
"mandatory: 'checker:', 'prefix:', "
"'profile:', 'guideline:'. If a checker "
"name matches multiple checkers as a "
"prefix, 'checker:' or 'prefix:' "
"namespace is required")

checkers_opts.add_argument('--enable-all',
dest="enable_all",
Expand Down
2 changes: 1 addition & 1 deletion analyzer/tests/functional/analyze/test_analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@ def test_disable_all_warnings(self):
analyze_cmd = [self._codechecker_cmd, "check", "-l", build_json,
"--analyzers", "clang-tidy",
"-d", "clang-diagnostic",
"-e", "clang-diagnostic-unused"]
"-e", "prefix:clang-diagnostic-unused"]

source_file = os.path.join(self.test_dir, "compiler_warning.c")
build_log = [{"directory": self.test_workspace,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
NORMAL#CodeChecker log --output $LOGFILE$ --build "make compiler_warning_wno_group" --quiet
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -e clang-diagnostic-unused
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -e prefix:clang-diagnostic-unused
NORMAL#CodeChecker parse $OUTPUT$
CHECK#CodeChecker check --build "make compiler_warning_wno_group" --output $OUTPUT$ --quiet --analyzers clang-tidy -e clang-diagnostic-unused
CHECK#CodeChecker check --build "make compiler_warning_wno_group" --output $OUTPUT$ --quiet --analyzers clang-tidy -e prefix:clang-diagnostic-unused
--------------------------------------------------------------------------------
[] - Starting build...
[] - Using CodeChecker ld-logger.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
NORMAL#CodeChecker log --output $LOGFILE$ --build "make compiler_warning_unused" --quiet
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -d clang-diagnostic-unused
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -d prefix:clang-diagnostic-unused
NORMAL#CodeChecker parse $OUTPUT$
CHECK#CodeChecker check --build "make compiler_warning_unused" --output $OUTPUT$ --quiet --analyzers clang-tidy -d clang-diagnostic-unused
CHECK#CodeChecker check --build "make compiler_warning_unused" --output $OUTPUT$ --quiet --analyzers clang-tidy -d prefix:clang-diagnostic-unused
--------------------------------------------------------------------------------
[] - Starting build...
[] - Using CodeChecker ld-logger.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
NORMAL#CodeChecker log --output $LOGFILE$ --build "make compiler_warning_unused" --quiet
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -d clang-diagnostic-unused
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -d prefix:clang-diagnostic-unused
NORMAL#CodeChecker parse $OUTPUT$
CHECK#CodeChecker check --build "make compiler_warning_unused" --output $OUTPUT$ --quiet --analyzers clang-tidy -d clang-diagnostic-unused
CHECK#CodeChecker check --build "make compiler_warning_unused" --output $OUTPUT$ --quiet --analyzers clang-tidy -d prefix:clang-diagnostic-unused
--------------------------------------------------------------------------------
[] - Starting build...
[] - Using CodeChecker ld-logger.
Expand Down
Loading

0 comments on commit e6a8c5d

Please sign in to comment.