Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy] Allow to override checker list #3203

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,8 @@ def invoke_binary_checked(binary_path, args=None, environ=None):
encoding="utf-8",
errors="ignore")
except (subprocess.CalledProcessError, OSError) as e:
LOG.debug(
'Command invocation failed because of non-zero exit code!'
'Details: %s', str(e))
LOG.debug('Command invocation failed because of non-zero exit code!'
'Details: %s', str(e))
return False
return output

Expand Down
110 changes: 75 additions & 35 deletions analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,13 @@
"""
"""


import ast
import json
import os
import re
import shlex
import subprocess
from typing import List, Tuple

from codechecker_common.logger import get_logger

Expand Down Expand Up @@ -127,51 +128,88 @@ def get_analyzer_config(cls, cfg_handler, environ):
except (subprocess.CalledProcessError, OSError):
return []

def construct_analyzer_cmd(self, result_handler):
def get_checker_list(self, config) -> Tuple[List[str], List[str]]:
"""
Return a list of checkers and warnings what needs to be enabled during
analysis.

If the file specified by the '--tidy-config' option contains a 'Checks'
key or the 'Checks' option is specified through the '--analyzer-config'
the return value will be a tuple of empty lists which means do not turn
checkers explicitly.
"""
try:
config = self.config_handler
checkers = []
compiler_warnings = []

analyzer_cmd = [config.analyzer_binary]
if config.analyzer_config.get('take-config-from-directory') == 'true':
return checkers, compiler_warnings

# Do not disable any clang-tidy checks explicitly, but don't run
# ClangSA checkers. ClangSA checkers are driven by an other
# analyzer in CodeChecker.
# For clang compiler warnings a correspoding
# clang-diagnostic error is generated by Clang tidy.
# They can be disabled by this glob -clang-diagnostic-*
checkers_cmdline = ['-clang-analyzer-*', 'clang-diagnostic-*']
has_checker_config = \
config.checker_config and config.checker_config != '{}'

compiler_warnings = []
if has_checker_config:
try:
# Is is possible that the value of config.checker_config
# is not a valid JSON string because the keys/values are
# quoted with single quotes instead of double quotes. For
# this reason we can't use the json.loads function to parse
# this string but we need to use 'literal_eval' function to
# safely evaluate the expression which will be a
# valid dictionary.
checker_cfg = ast.literal_eval(config.checker_config.strip())
if checker_cfg.get('Checks'):
return checkers, compiler_warnings
except SyntaxError as ex:
LOG.debug("Invalid checker configuration: %s. Error: %s",
config.checker_config, ex)

# Do not disable any clang-tidy checks explicitly, but don't run
# ClangSA checkers. ClangSA checkers are driven by an other
# analyzer in CodeChecker.
# For clang compiler warnings a correspoding
# clang-diagnostic error is generated by Clang tidy.
# They can be disabled by this glob -clang-diagnostic-*
checkers = ['-clang-analyzer-*', 'clang-diagnostic-*']

# Config handler stores which checkers are enabled or disabled.
for checker_name, value in config.checks().items():
state, _ = value

# Checker name is a compiler warning.
if checker_name.startswith('W'):
warning_name = checker_name[4:] if \
checker_name.startswith('Wno-') else checker_name[1:]

# Config handler stores which checkers are enabled or disabled.
for checker_name, value in config.checks().items():
state, _ = value
if state == CheckerState.enabled:
compiler_warnings.append('-W' + warning_name)
elif state == CheckerState.disabled:
compiler_warnings.append('-Wno-' + warning_name)

# Checker name is a compiler warning.
if checker_name.startswith('W'):
warning_name = checker_name[4:] if \
checker_name.startswith('Wno-') else checker_name[1:]
continue
elif checker_name.startswith('clang-diagnostic'):
checker_name += '*'

if state == CheckerState.enabled:
compiler_warnings.append('-W' + warning_name)
elif state == CheckerState.disabled:
compiler_warnings.append('-Wno-' + warning_name)
if state == CheckerState.enabled:
checkers.append(checker_name)
elif state == CheckerState.disabled:
checkers.append('-' + checker_name)

continue
elif checker_name.startswith('clang-diagnostic'):
checker_name += '*'
return checkers, compiler_warnings

if state == CheckerState.enabled:
checkers_cmdline.append(checker_name)
elif state == CheckerState.disabled:
checkers_cmdline.append('-' + checker_name)
def construct_analyzer_cmd(self, result_handler):
""" Contruct command which will be executed on analysis. """
try:
config = self.config_handler

# The invocation should end in a Popen call with shell=False, so
# no globbing should occur even if the checks argument contains
# characters that would trigger globbing in the shell.
analyzer_cmd.append("-checks=%s" % ','.join(checkers_cmdline))
analyzer_cmd = [config.analyzer_binary]

checks, compiler_warnings = self.get_checker_list(config)

if checks:
# The invocation should end in a Popen call with shell=False,
# so no globbing should occur even if the checks argument
# contains characters that would trigger globbing in the shell.
analyzer_cmd.append("-checks=%s" % ','.join(checks))

analyzer_cmd.extend(config.analyzer_extra_arguments)

Expand Down Expand Up @@ -375,6 +413,8 @@ def construct_config_handler(cls, args, context):
# No clang tidy config file was given in the command line.
LOG.debug_analyzer(aerr)

handler.analyzer_config = analyzer_config

# 'take-config-from-directory' is a special option which let the user
# to use the '.clang-tidy' config files. It will disable analyzer and
# checker configuration options.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,32 @@

from codechecker_common.logger import get_logger

from .. import config_handler
from ..config_handler import AnalyzerConfigHandler, CheckerState

LOG = get_logger('analyzer.tidy')


class ClangTidyConfigHandler(config_handler.AnalyzerConfigHandler):
class ClangTidyConfigHandler(AnalyzerConfigHandler):
"""
Configuration handler for Clang-tidy analyzer.
"""

def __init__(self):
super(ClangTidyConfigHandler, self).__init__()

def add_checker(self, checker_name, description='',
state=CheckerState.default):
"""
Add additional checker if the 'take-config-from-directory'
analyzer configuration option is not set.
"""
if self.analyzer_config and \
self.analyzer_config.get('take-config-from-directory') == 'true':
return

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

def set_checker_enabled(self, checker_name, enabled=True):
"""
Enable checker, keep description if already set.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ def __init__(self):
self.analyzer_plugins_dir = None
self.analyzer_extra_arguments = []
self.checker_config = ''
self.analyzer_config = None
self.report_hash = None

# The key is the checker name, the value is a tuple.
Expand Down
16 changes: 10 additions & 6 deletions analyzer/codechecker_analyzer/cmd/analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,14 +412,18 @@ def add_arguments_to_parser(parser):
"The collection of the options can be "
"printed with "
"'CodeChecker analyzers "
"--analyzer-config'. To disable the "
"default behaviour of this option you can "
"use the "
"'clang-tidy:take-config-from-directory="
"true' option. If the file at --tidyargs "
"--analyzer-config'.\n"
"If the file at --tidyargs "
"contains a -config flag then those "
"options extend these and override "
"\"HeaderFilterRegex\" if any.")
"\"HeaderFilterRegex\" if any.\n"
"To use analyzer configuration file "
"in case of Clang Tidy (.clang-tidy) use "
"the "
"'clang-tidy:take-config-from-directory="
"true' option. It will skip setting the "
"'-checks' parameter of the clang-tidy "
"binary.")

analyzer_opts.add_argument('--checker-config',
dest='checker_config',
Expand Down
16 changes: 10 additions & 6 deletions analyzer/codechecker_analyzer/cmd/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -369,14 +369,18 @@ def add_arguments_to_parser(parser):
"The collection of the options can be "
"printed with "
"'CodeChecker analyzers "
"--analyzer-config'. To disable the "
"default behaviour of this option you can "
"use the "
"'clang-tidy:take-config-from-directory="
"true' option. If the file at --tidyargs "
"--analyzer-config'.\n"
"If the file at --tidyargs "
"contains a -config flag then those "
"options extend these and override "
"\"HeaderFilterRegex\" if any.")
"\"HeaderFilterRegex\" if any.\n"
"To use analyzer configuration file "
"in case of Clang Tidy (.clang-tidy) use "
"the "
"'clang-tidy:take-config-from-directory="
"true' option. It will skip setting the "
"'-checks' parameter of the clang-tidy "
"binary.")

analyzer_opts.add_argument('--checker-config',
dest='checker_config',
Expand Down
28 changes: 16 additions & 12 deletions docs/analyzer/user_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -258,12 +258,14 @@ analyzer arguments:
Analyzer configuration options in the following
format: analyzer:key=value. The collection of the
options can be printed with 'CodeChecker analyzers
--analyzer-config'. To disable the default behaviour
of this option you can use the 'clang-tidy:take-
config-from-directory=true' option. If the file at
--tidyargs contains a -config flag then those options
extend these and override "HeaderFilterRegex" if any.
(default: ['clang-tidy:HeaderFilterRegex=.*'])
--analyzer-config'. If the file at --tidyargs contains
a -config flag then those options extend these and
override "HeaderFilterRegex" if any. To use analyzer
configuration file in case of Clang Tidy (.clang-tidy)
use the 'clang-tidy:take-config-from-directory=true'
option. It will skip setting the '-checks' parameter
of the clang-tidy binary. (default: ['clang-
tidy:HeaderFilterRegex=.*'])
--checker-config [CHECKER_CONFIG [CHECKER_CONFIG ...]]
Checker configuration options in the following format:
analyzer:key=value. The collection of the options can
Expand Down Expand Up @@ -1050,12 +1052,14 @@ analyzer arguments:
Analyzer configuration options in the following
format: analyzer:key=value. The collection of the
options can be printed with 'CodeChecker analyzers
--analyzer-config'. To disable the default behaviour
of this option you can use the 'clang-tidy:take-
config-from-directory=true' option. If the file at
--tidyargs contains a -config flag then those options
extend these and override "HeaderFilterRegex" if any.
(default: ['clang-tidy:HeaderFilterRegex=.*'])
--analyzer-config'. If the file at --tidyargs contains
a -config flag then those options extend these and
override "HeaderFilterRegex" if any. To use analyzer
configuration file in case of Clang Tidy (.clang-tidy)
use the 'clang-tidy:take-config-from-directory=true'
option. It will skip setting the '-checks' parameter
of the clang-tidy binary. (default: ['clang-
tidy:HeaderFilterRegex=.*'])
--checker-config [CHECKER_CONFIG [CHECKER_CONFIG ...]]
Checker configuration options in the following format:
analyzer:key=value. The collection of the options can
Expand Down
46 changes: 7 additions & 39 deletions web/tests/functional/detection_status/test_detection_status.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
""" detection_status function test. """


import glob
import json
import os
import shutil
Expand Down Expand Up @@ -373,12 +372,12 @@ def test_detection_status_off(self):

def test_detection_status_off_with_cfg(self):
""" Test detection status with .clang-tidy config file. """
# Explicitly disable all modernize checkers from the command line but
# Explicitly disable all hicpp checkers from the command line but
# enable all hicpp and modernize checkers from the .clang-tidy file.
# If we store the results to the server than no reports will be marked
# as OFF.
cfg = dict(self._codechecker_cfg)
cfg['checkers'] = ['-d', 'modernize']
cfg['checkers'] = ['-d', 'hicpp']
cfg['analyzer_config'] = ['clang-tidy:take-config-from-directory=true']

self._create_source_file(1)
Expand All @@ -390,7 +389,11 @@ def test_detection_status_off_with_cfg(self):

hicpp_results = [r for r in reports
if r.checkerId.startswith('hicpp')]
self.assertEqual(len(hicpp_results), 1)
self.assertTrue(hicpp_results)

modernize_results = [r for r in reports
if r.checkerId.startswith('modernize')]
self.assertTrue(modernize_results)

offed_reports = [r for r in reports
if r.detectionStatus == DetectionStatus.OFF]
Expand All @@ -405,41 +408,6 @@ def test_detection_status_off_with_cfg(self):
self.assertTrue([r for r in reports
if r.detectionStatus == DetectionStatus.UNRESOLVED])

# We turn off all the 'hicpp' checkers too. If we store the results to
# the server see that 'hicpp' results will be marked as 'OFF' now.
cfg['checkers'] = ['-d', 'modernize', '-d', 'hicpp']
self._check_source_file(cfg)

reports = self._cc_client.getRunResults(None, 100, 0, [], None, None,
False)

offed_reports = [r for r in reports
if r.detectionStatus == DetectionStatus.OFF]
self.assertEqual(len(offed_reports), 1)

# We do not disable hicpp checkers from the command line, so it will
# be enabled by the .clang-tidy file. We analyze our test project and
# remove the plist files to see that every reports will be marked as
# Resolved.
# FIXME: Unfortunately it will not work because hicpp checkers will be
# disabled by the metadata.json config file so the server will mark
# these reports as OFF.
# A solution for this problem can be if we mark reports as OFF only and
# only if the user explicitly disabled a checker in the command line.
cfg['checkers'] = ['-d', 'modernize']
codechecker.analyze(cfg, self._test_dir)

# Remove all plist files.
report_dir = self._codechecker_cfg['reportdir']
for pfile in glob.glob(os.path.join(report_dir, '*.plist')):
os.remove(pfile)

codechecker.store(cfg, self._run_name)

offed_reports = [r for r in reports
if r.detectionStatus == DetectionStatus.OFF]
self.assertEqual(len(offed_reports), 1)

# Remove .clang-tidy configuration file.
os.remove(self.clang_tidy_cfg)

Expand Down