Skip to content

Commit

Permalink
Add functionality to validate analyzer and checker options
Browse files Browse the repository at this point in the history
When specifying invalid checker and analyzer config,
the CodeChecker analyze command used to give a warning
but continued running.
This functionality throws an error and prevents the
analysis from running in this case.

Co-authored-by: Kristóf Umann <dkszelethus@gmail.com>
  • Loading branch information
2 people authored and Nora Zinaeddin committed Apr 19, 2024
1 parent 11dc7da commit 4f23852
Show file tree
Hide file tree
Showing 4 changed files with 141 additions and 66 deletions.
139 changes: 81 additions & 58 deletions analyzer/codechecker_analyzer/cmd/analyze.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
from codechecker_analyzer.analyzers import analyzer_types, clangsa
from codechecker_analyzer.arg import \
OrderedCheckersAction, OrderedConfigAction, existing_abspath, \
analyzer_config, checker_config
analyzer_config, checker_config, AnalyzerConfig, CheckerConfig

from codechecker_analyzer.buildlog import log_parser

from codechecker_common import arg, logger, cmd_config, review_status_handler
Expand Down Expand Up @@ -788,7 +789,9 @@ def add_arguments_to_parser(parser):
default=argparse.SUPPRESS,
help="Emit a warning instead of an error when "
"an unknown checker name is given to "
"either --enable or --disable.")
"either --enable, --disable,"
"--analyzer-config and/or "
"--checker-config.")

logger.add_verbose_arguments(parser)
parser.set_defaults(
Expand All @@ -800,72 +803,84 @@ def is_analyzer_config_valid(analyzer_conf: List[AnalyzerConfig]) -> bool:
Ensure that the analyzer_config parameter is set to a valid value
by verifying if it belongs to the set of allowed values.
"""
analyzer = analyzer_conf.__getitem__(0).analyzer
wrong_configs = []
supported_analyzers = analyzer_types.supported_analyzers

if analyzer in analyzer_types.supported_analyzers:
analyzer_class = analyzer_types.supported_analyzers[analyzer]
else:
LOG.error(f"Invalid --analyzer-config parameter: {analyzer} is not "
f"a supported analyzer. The supported analyzers are "
f"{', '.join(a for a in analyzer_types.supported_analyzers)}"
f".")
return False
for cfg in analyzer_conf:
if cfg.analyzer not in supported_analyzers:
wrong_configs.append((cfg.analyzer, None))
continue

analyzer_class = supported_analyzers[cfg.analyzer]
analyzer_name = analyzer_class.ANALYZER_NAME
analyzers = [analyzer[0] for analyzer in
analyzer_class.get_analyzer_config()]

# Make sure "[clang-tidy] Allow to override checker list #3203" works
if analyzer_name == 'clang-tidy':
analyzers.append('take-config-from-directory')

if cfg.analyzer == analyzer_name and cfg.option not in analyzers:
wrong_configs.append((analyzer_name, cfg.option))

if wrong_configs:
for analyzer_name, option in wrong_configs:
if option is None:
LOG.error(
f"Invalid argument to --analyzer-config: {analyzer_name} "
f"is not a supported analyzer. Supported analyzers are: "
f"{', '.join(a for a in supported_analyzers)}.")
else:
LOG.error(
f"Invalid argument to --analyzer-config: {analyzer_name} "
f"has no config named {option}. Use the 'CodeChecker "
f"analyzers --analyzer-config {analyzer_name}' command to "
f"see available configs.")

if not isinstance(analyzer_conf, list):
return False

analyzers = [analyzer[0] for analyzer in
analyzer_class.get_analyzer_config()]

# Make sure "[clang-tidy] Allow to override checker list #3203" works
if analyzer_class.ANALYZER_NAME == 'clang-tidy':
analyzers.append('take-config-from-directory')

for cfg in analyzer_conf:
if cfg.analyzer == analyzer_class.ANALYZER_NAME and \
cfg.option not in analyzers:
LOG.error(f"Invalid --analyzer-config parameter: "
f"{analyzer_class.ANALYZER_NAME} has no config named "
f"{cfg.option}. Use the 'CodeChecker analyzers "
f"--analyzer-config {analyzer_class.ANALYZER_NAME}' "
f"command to check available configs.")
return False
return True
return True


def is_checker_config_valid(checker_conf: List[CheckerConfig]) -> bool:
"""
Ensure that the checker_config parameter is set to a valid value
by verifying if it belongs to the set of allowed values.
"""
analyzer = checker_conf.__getitem__(0).analyzer
wrong_configs = []
supported_analyzers = analyzer_types.supported_analyzers

if analyzer in analyzer_types.supported_analyzers:
analyzer_class = analyzer_types.supported_analyzers[analyzer]
else:
LOG.error(f"Invalid --checker-config parameter: {analyzer} is not "
f"a supported analyzer. The supported analyzers are "
f"{', '.join(a for a in analyzer_types.supported_analyzers)}"
f".")
return False
for cfg in checker_conf:
if cfg.analyzer not in supported_analyzers:
wrong_configs.append((cfg.analyzer, None))
continue

analyzer_class = supported_analyzers[cfg.analyzer]
analyzer_name = analyzer_class.ANALYZER_NAME
checker_conf_opt = f"{cfg.checker}:{cfg.option}"
checkers = [checker[0] for checker in
analyzer_class.get_checker_config()]

if cfg.analyzer == analyzer_name and checker_conf_opt not in checkers:
wrong_configs.append((analyzer_name, checker_conf_opt))

if wrong_configs:
for analyzer_name, conf_opt in wrong_configs:
if conf_opt is None:
LOG.error(
f"Invalid argument to --checker-config: {analyzer_name} "
f"is not a supported analyzer. Supported analyzers are: "
f"{', '.join(a for a in supported_analyzers)}.")
else:
LOG.error(
f"Invalid argument to --checker-config: invalid checker "
f"and/or checker option '{conf_opt}' for {analyzer_name}. "
f"Use the 'CodeChecker checkers --checker-config' command "
f"to see available checkers.")

if not isinstance(checker_conf, list):
return False

checkers = [checker[0] for checker in
analyzer_class.get_analyzer_checkers()]

for cfg in checker_conf:
if cfg.analyzer == analyzer_class.ANALYZER_NAME and \
cfg.checker not in checkers:
LOG.error(
f"Invalid --checker-config parameter: "
f"{analyzer_class.ANALYZER_NAME} has no checker named "
f"{cfg.checker}. Use the 'CodeChecker checkers "
f"--checker-config' command to see available checkers."
)
return False
return True
return True


def get_affected_file_paths(
Expand Down Expand Up @@ -1007,12 +1022,20 @@ def main(args):

# Validate analyzer and checker config (if any)
config_validator = {
'analyzer_config': validate_analyzer_parameter,
'checker_config': validate_checker_parameter
'analyzer_config': is_analyzer_config_valid,
'checker_config': is_checker_config_valid
}
for conf, validate_func in config_validator.items():
if conf in args and not validate_func(getattr(args, conf)):
sys.exit(1)

config_validator_res = [validate_func(getattr(args, conf))
for conf, validate_func in config_validator.items()
if conf in args]

if False in config_validator_res \
and 'no_missing_checker_error' not in args:
LOG.info("Although it is not recommended, if you want to suppress "
"errors relating to unknown analyzer/checker configs, "
"consider using the option '--no-missing-checker-error'")
sys.exit(1)

if 'tidy_config' in args:
LOG.warning(
Expand Down
5 changes: 4 additions & 1 deletion analyzer/codechecker_analyzer/cmd/check.py
Original file line number Diff line number Diff line change
Expand Up @@ -733,7 +733,9 @@ def add_arguments_to_parser(parser):
default=argparse.SUPPRESS,
help="Emit a warning instead of an error when "
"an unknown checker name is given to "
"either --enable or --disable.")
"either --enable, --disable,"
"--analyzer-config and/or "
"--checker-config.")

output_opts = parser.add_argument_group("output arguments")

Expand Down Expand Up @@ -890,6 +892,7 @@ def __update_if_key_exists(source, target, key):
'stats_min_sample_count',
'enable_all',
'disable_all',
'no_missing_checker_error',
'ordered_checkers', # --enable and --disable.
'timeout',
'review_status_config',
Expand Down
24 changes: 24 additions & 0 deletions analyzer/tests/functional/cmdline/test_cmdline.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,30 @@ def test_analyzer_config(self):
self.assertTrue(desc)
self.assertFalse(desc[0].islower())

def test_analyze_incorrect_checker_analyzer(self):
"""
This test checks whether the analyze command stops running if a
non-existent path is specified.
"""
test_file = os.path.join(self.test_workspace, 'main.cpp')

with open(test_file, 'w', encoding="utf-8", errors="ignore") as f:
f.write("int main() {}")

cmd_err = [env.codechecker_cmd(), 'check',
'--analyzer-config', 'clang:asd=1',
'--checker-config', 'clang:asd:asd=1',
'--build', f'g++ {test_file}']

cmd_no_err = [env.codechecker_cmd(), 'check',
'--analyzer-config', 'clang:asd=1',
'--checker-config', 'clang:asd:asd=1',
'--no-missing-checker-error',
'--build', f'g++ {test_file}']

self.assertEqual(1, run_cmd(cmd_err)[0])
self.assertIn('Build finished successfully', run_cmd(cmd_no_err)[1])

def test_checker_config_format(self):
"""
Test if checker config option is meeting the reqired format.
Expand Down
39 changes: 32 additions & 7 deletions analyzer/tests/unit/test_checker_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import is_compiler_warning
from codechecker_analyzer.arg import AnalyzerConfig, CheckerConfig
from codechecker_analyzer.cmd.analyze import \
validate_analyzer_parameter, validate_checker_parameter
is_analyzer_config_valid, is_checker_config_valid

from codechecker_analyzer import analyzer_context
from codechecker_analyzer.buildlog import log_parser
Expand Down Expand Up @@ -405,13 +405,38 @@ def test_analyze_wrong_parameters(self):
--analyzer-config or --checker-config parameter is specified.
"""

analyzer_cfg = [AnalyzerConfig('asd', '123', 'value')]
checker_cfg = [CheckerConfig('clangsa', 'asd', '123', 'value')]

analyzer_func = validate_analyzer_parameter(analyzer_cfg)
checker_func = validate_checker_parameter(checker_cfg)
analyzer_cfg_valid = [AnalyzerConfig(
'clangsa', 'faux-bodies', 'false')]
checker_cfg_valid = [CheckerConfig(
'clang-tidy', 'performance-unnecessary-value-param',
'IncludeStyle', 'false')]

self.assertTrue(is_analyzer_config_valid(analyzer_cfg_valid))
self.assertTrue(is_checker_config_valid(checker_cfg_valid))

analyzer_cfg_invalid_analyzer = [AnalyzerConfig(
'asd', 'faux-bodies', 'false')]
analyzer_cfg_invalid_conf = [AnalyzerConfig(
'clangsa', 'asd', 'false')]
checker_cfg_invalid_analyzer = [CheckerConfig(
'asd', 'performance-unnecessary-value-param',
'IncludeStyle', 'false')]
checker_cfg_invalid_checker = [CheckerConfig(
'clang-tidy', 'asd',
'IncludeStyle', 'false')]
checker_cfg_invalid_checker_option = [CheckerConfig(
'clang-tidy', 'performance-unnecessary-value-param',
'asd', 'false')]

self.assertFalse(is_analyzer_config_valid(
analyzer_cfg_invalid_analyzer))
self.assertFalse(
is_analyzer_config_valid(analyzer_cfg_invalid_conf))

[self.assertFalse(func) for func in [analyzer_func, checker_func]]
self.assertFalse(is_checker_config_valid(checker_cfg_invalid_analyzer))
self.assertFalse(is_checker_config_valid(checker_cfg_invalid_checker))
self.assertFalse(is_checker_config_valid(
checker_cfg_invalid_checker_option))

def test_enable_all_disable_warning(self):
"""
Expand Down

0 comments on commit 4f23852

Please sign in to comment.