Skip to content

Commit 4f14816

Browse files
dkruppdkrupp
authored
Merge pull request #4392 from cservakt/ambigous-checker-options
[fix] Resolve checker enable/disable ambiguity
2 parents 2f1aa32 + fe19dc0 commit 4f14816

File tree

15 files changed

+397
-163
lines changed

15 files changed

+397
-163
lines changed

‎analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py

+3
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,9 @@ def get_analyzer_checkers(cls):
288288
("clang-diagnostic-" + warning, "")
289289
for warning in get_warnings())
290290

291+
checker_description.append(("clang-diagnostic-error",
292+
"Indicates compiler errors."))
293+
291294
cls.__analyzer_checkers = checker_description
292295

293296
return checker_description

‎analyzer/codechecker_analyzer/analyzers/clangtidy/config_handler.py

-10
Original file line numberDiff line numberDiff line change
@@ -41,13 +41,3 @@ def add_checker(self, checker_name, description='',
4141
return
4242

4343
super().add_checker(checker_name, description, state)
44-
45-
def set_checker_enabled(self, checker_name, enabled=True):
46-
"""
47-
Enable checker, keep description if already set.
48-
"""
49-
if checker_name.startswith('W') or \
50-
checker_name.startswith('clang-diagnostic'):
51-
self.add_checker(checker_name)
52-
53-
super().set_checker_enabled(checker_name, enabled)

‎analyzer/codechecker_analyzer/analyzers/config_handler.py

+79-45
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212

1313
from abc import ABCMeta
1414
from enum import Enum
15-
from string import Template
1615
import collections
1716
import platform
1817
import sys
@@ -86,15 +85,16 @@ def add_checker(self, checker_name, description='',
8685
"""
8786
self.__available_checkers[checker_name] = (state, description)
8887

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

9695
for ch_name, values in self.__available_checkers.items():
97-
if re.match(regex, ch_name):
96+
if (is_strict and ch_name == checker_name) \
97+
or (not is_strict and re.match(regex, ch_name)):
9898
_, description = values
9999
state = CheckerState.ENABLED if enabled \
100100
else CheckerState.DISABLED
@@ -118,7 +118,7 @@ def checks(self):
118118
"""
119119
return self.__available_checkers
120120

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

139-
return reserved_names
139+
return list(set(reserved_names))
140140

141141
def initialize_checkers(self,
142142
checkers,
@@ -184,7 +184,7 @@ def initialize_checkers(self,
184184
else:
185185
# Turn default checkers on.
186186
for checker in default_profile_checkers:
187-
self.set_checker_enabled(checker)
187+
self.set_checker_enabled(checker, is_strict=True)
188188

189189
self.enable_all = enable_all
190190
# If enable_all is given, almost all checkers should be enabled.
@@ -207,48 +207,82 @@ def initialize_checkers(self,
207207
self.set_checker_enabled(checker_name)
208208

209209
# Set user defined enabled or disabled checkers from the command line.
210+
for identifier, enabled in cmdline_enable:
211+
labels = checker_labels.labels() \
212+
if callable(getattr(checker_labels, 'labels', None)) \
213+
else ["guideline", "profile", "severity", "sei-cert"]
210214

211-
# Construct a list of reserved checker names.
212-
# (It is used to check if a profile name is valid.)
213-
reserved_names = self.__gen_name_variations()
214-
profiles = checker_labels.get_description('profile')
215-
guidelines = checker_labels.occurring_values('guideline')
215+
all_namespaces = ["checker", "prefix"] + labels
216216

217-
templ = Template("The ${entity} name '${identifier}' conflicts with a "
218-
"checker name prefix '${identifier}'. Please use -e "
219-
"${entity}:${identifier} to enable checkers of the "
220-
"${identifier} ${entity} or use -e "
221-
"prefix:${identifier} to select checkers which have "
222-
"a name starting with '${identifier}'.")
217+
all_options = dict(zip(labels, map(
218+
checker_labels.occurring_values, labels)))
223219

224-
for identifier, enabled in cmdline_enable:
225-
if "prefix:" in identifier:
226-
identifier = identifier.replace("prefix:", "")
227-
self.set_checker_enabled(identifier, enabled)
228-
229-
elif ':' in identifier:
230-
for checker in checker_labels.checkers_by_labels([identifier]):
231-
self.set_checker_enabled(checker, enabled)
232-
233-
elif identifier in profiles:
234-
if identifier in reserved_names:
235-
LOG.error(templ.substitute(entity="profile",
236-
identifier=identifier))
220+
all_options["prefix"] = list(set(self.__gen_name_variations(
221+
only_prefix=True)))
222+
223+
all_options["checker"] = self.__available_checkers
224+
225+
if ":" in identifier:
226+
identifier_namespace = identifier.split(":")[0]
227+
identifier = identifier.split(":", 1)[1]
228+
229+
if identifier_namespace not in all_namespaces:
230+
LOG.error("The %s namespace is not known. Please select"
231+
"one of these existing namespace options: %s.",
232+
identifier_namespace, ", ".join(all_namespaces))
237233
sys.exit(1)
238-
else:
239-
for checker in checker_labels.checkers_by_labels(
240-
[f'profile:{identifier}']):
241-
self.set_checker_enabled(checker, enabled)
242-
243-
elif identifier in guidelines:
244-
if identifier in reserved_names:
245-
LOG.error(templ.substitute(entity="guideline",
246-
identifier=identifier))
234+
235+
# TODO: Each analyzer has its own config handler and is unaware
236+
# of other checkers. To avoid not reliable error, we pass
237+
# checker:'any_options' and prefix:'any_options'. This ensures
238+
# enabling a checker doesn't inadvertently cause an error in a
239+
# different analyzer. Ideally, there should be a centralized
240+
# main configuration accessible to all analyzers.
241+
if identifier not in all_options[identifier_namespace] \
242+
and identifier_namespace not in ("checker", "prefix"):
243+
LOG.error("The %s identifier does not exist in the %s "
244+
"namespace. Please select one of these "
245+
"existing options: %s.", identifier,
246+
identifier_namespace, ", ".join(
247+
all_options[identifier_namespace]))
247248
sys.exit(1)
248-
else:
249-
for checker in checker_labels.checkers_by_labels(
250-
[f'guideline:{identifier}']):
251-
self.set_checker_enabled(checker, enabled)
249+
250+
self.initialize_checkers_by_namespace(
251+
identifier_namespace, identifier, enabled)
252252

253253
else:
254-
self.set_checker_enabled(identifier, enabled)
254+
possible_options = {}
255+
for label, options in all_options.items():
256+
if identifier in options:
257+
possible_options[label] = identifier
258+
259+
if len(possible_options) == 1:
260+
self.initialize_checkers_by_namespace(
261+
*list(possible_options.items())[0], enabled)
262+
elif len(possible_options) > 1:
263+
error_options = ", ".join(f"{label}:{option}"
264+
for label, option
265+
in possible_options.items())
266+
267+
LOG.error("The %s is ambigous. Please select one of these"
268+
" options to clarify the checker list: %s.",
269+
identifier, error_options)
270+
sys.exit(1)
271+
else:
272+
# The identifier is not known but we just pass it
273+
# and handle it in a different section.
274+
continue
275+
276+
def initialize_checkers_by_namespace(self,
277+
identifier_namespace,
278+
identifier,
279+
enabled):
280+
if identifier_namespace == "checker":
281+
self.set_checker_enabled(identifier, enabled, is_strict=True)
282+
elif identifier_namespace == "prefix":
283+
self.set_checker_enabled(identifier, enabled)
284+
else:
285+
checker_labels = analyzer_context.get_context().checker_labels
286+
for checker in checker_labels.checkers_by_labels(
287+
[f"{identifier_namespace}:{identifier}"]):
288+
self.set_checker_enabled(checker, enabled, is_strict=True)

‎analyzer/codechecker_analyzer/checkers.py

+1-6
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,7 @@ def available(ordered_checkers, available_checkers):
1919
"""
2020
missing_checkers = set()
2121
for checker_name, _ in ordered_checkers:
22-
# TODO: This label list shouldn't be hard-coded here.
23-
if checker_name.startswith('profile:') or \
24-
checker_name.startswith('guideline:') or \
25-
checker_name.startswith('severity:') or \
26-
checker_name.startswith('sei-cert:') or \
27-
checker_name.startswith('prefix:'):
22+
if ":" in checker_name:
2823
continue
2924

3025
name_match = False

‎analyzer/codechecker_analyzer/cmd/analyze.py

+10-4
Original file line numberDiff line numberDiff line change
@@ -773,8 +773,11 @@ def add_arguments_to_parser(parser):
773773
"between the checker prefix "
774774
"group/profile/guideline name, the use of "
775775
"one of the following labels is "
776-
"mandatory: 'prefix:', 'profile:', "
777-
"'guideline:'.")
776+
"mandatory: 'checker:', 'prefix:', "
777+
"'profile:', 'guideline:'. If a checker "
778+
"name matches multiple checkers as a "
779+
"prefix, 'checker:' or 'prefix:' "
780+
"namespace is required")
778781

779782
checkers_opts.add_argument('-d', '--disable',
780783
dest="disable",
@@ -792,8 +795,11 @@ def add_arguments_to_parser(parser):
792795
"between the checker prefix "
793796
"group/profile/guideline name, the use of "
794797
"one of the following labels is "
795-
"mandatory: 'prefix:', 'profile:', "
796-
"'guideline:'.")
798+
"mandatory: 'checker:', 'prefix:', "
799+
"'profile:', 'guideline:'. If a checker "
800+
"name matches multiple checkers as a "
801+
"prefix, 'checker:' or 'prefix:' "
802+
"namespace is required")
797803

798804
checkers_opts.add_argument('--enable-all',
799805
dest="enable_all",

‎analyzer/codechecker_analyzer/cmd/check.py

+23-15
Original file line numberDiff line numberDiff line change
@@ -707,35 +707,43 @@ def add_arguments_to_parser(parser):
707707
metavar='checker/group/profile',
708708
default=argparse.SUPPRESS,
709709
action=OrderedCheckersAction,
710-
help="Set a checker (or checker group), "
711-
"profile or guideline "
712-
"to BE USED in the analysis. In case of "
713-
"ambiguity the priority order is profile, "
714-
"guideline, checker name (e.g. security "
715-
"means the profile, not the checker "
716-
"group). Moreover, labels can also be "
710+
help="Set a checker (or checker prefix group), "
711+
"profile or guideline to BE USED in the "
712+
"analysis. Labels can also be "
717713
"used for selecting checkers, for example "
718714
"profile:extreme or severity:STYLE. See "
719715
"'CodeChecker checkers --label' for "
720-
"further details.")
716+
"further details. In case of a name clash "
717+
"between the checker prefix "
718+
"group/profile/guideline name, the use of "
719+
"one of the following labels is "
720+
"mandatory: 'checker:', 'prefix:', "
721+
"'profile:', 'guideline:'. If a checker "
722+
"name matches multiple checkers as a "
723+
"prefix, 'checker:' or 'prefix:' "
724+
"namespace is required")
721725

722726
checkers_opts.add_argument('-d', '--disable',
723727
dest="disable",
724728
metavar='checker/group/profile',
725729
default=argparse.SUPPRESS,
726730
action=OrderedCheckersAction,
727-
help="Set a checker (or checker group), "
731+
help="Set a checker (or checker prefix group), "
728732
"profile or guideline "
729733
"to BE PROHIBITED from use in the "
730-
"analysis. In case of "
731-
"ambiguity the priority order is profile, "
732-
"guideline, checker name (e.g. security "
733-
"means the profile, not the checker "
734-
"group). Moreover, labels can also be "
734+
"analysis. Labels can also be "
735735
"used for selecting checkers, for example "
736736
"profile:extreme or severity:STYLE. See "
737737
"'CodeChecker checkers --label' for "
738-
"further details.")
738+
"further details. In case of a name clash "
739+
"between the checker prefix "
740+
"group/profile/guideline name, the use of "
741+
"one of the following labels is "
742+
"mandatory: 'checker:', 'prefix:', "
743+
"'profile:', 'guideline:'. If a checker "
744+
"name matches multiple checkers as a "
745+
"prefix, 'checker:' or 'prefix:' "
746+
"namespace is required")
739747

740748
checkers_opts.add_argument('--enable-all',
741749
dest="enable_all",

‎analyzer/tests/functional/analyze/test_analyze.py

+1-1
Original file line numberDiff line numberDiff line change
@@ -886,7 +886,7 @@ def test_disable_all_warnings(self):
886886
analyze_cmd = [self._codechecker_cmd, "check", "-l", build_json,
887887
"--analyzers", "clang-tidy",
888888
"-d", "clang-diagnostic",
889-
"-e", "clang-diagnostic-unused"]
889+
"-e", "prefix:clang-diagnostic-unused"]
890890

891891
source_file = os.path.join(self.test_dir, "compiler_warning.c")
892892
build_log = [{"directory": self.test_workspace,

‎analyzer/tests/functional/analyze_and_parse/test_files/compiler_warning_wno_group.output

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
NORMAL#CodeChecker log --output $LOGFILE$ --build "make compiler_warning_wno_group" --quiet
2-
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -e clang-diagnostic-unused
2+
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -e prefix:clang-diagnostic-unused
33
NORMAL#CodeChecker parse $OUTPUT$
4-
CHECK#CodeChecker check --build "make compiler_warning_wno_group" --output $OUTPUT$ --quiet --analyzers clang-tidy -e clang-diagnostic-unused
4+
CHECK#CodeChecker check --build "make compiler_warning_wno_group" --output $OUTPUT$ --quiet --analyzers clang-tidy -e prefix:clang-diagnostic-unused
55
--------------------------------------------------------------------------------
66
[] - Starting build...
77
[] - Using CodeChecker ld-logger.

‎analyzer/tests/functional/analyze_and_parse/test_files/compiler_warning_wno_simple2.output

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
NORMAL#CodeChecker log --output $LOGFILE$ --build "make compiler_warning_unused" --quiet
2-
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -d clang-diagnostic-unused
2+
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -d prefix:clang-diagnostic-unused
33
NORMAL#CodeChecker parse $OUTPUT$
4-
CHECK#CodeChecker check --build "make compiler_warning_unused" --output $OUTPUT$ --quiet --analyzers clang-tidy -d clang-diagnostic-unused
4+
CHECK#CodeChecker check --build "make compiler_warning_unused" --output $OUTPUT$ --quiet --analyzers clang-tidy -d prefix:clang-diagnostic-unused
55
--------------------------------------------------------------------------------
66
[] - Starting build...
77
[] - Using CodeChecker ld-logger.

‎analyzer/tests/functional/analyze_and_parse/test_files/compiler_warning_wunused.output

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
NORMAL#CodeChecker log --output $LOGFILE$ --build "make compiler_warning_unused" --quiet
2-
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -d clang-diagnostic-unused
2+
NORMAL#CodeChecker analyze $LOGFILE$ --output $OUTPUT$ --analyzers clang-tidy -d prefix:clang-diagnostic-unused
33
NORMAL#CodeChecker parse $OUTPUT$
4-
CHECK#CodeChecker check --build "make compiler_warning_unused" --output $OUTPUT$ --quiet --analyzers clang-tidy -d clang-diagnostic-unused
4+
CHECK#CodeChecker check --build "make compiler_warning_unused" --output $OUTPUT$ --quiet --analyzers clang-tidy -d prefix:clang-diagnostic-unused
55
--------------------------------------------------------------------------------
66
[] - Starting build...
77
[] - Using CodeChecker ld-logger.

0 commit comments

Comments
 (0)