From 4f23852736709ed8b00573c1173a43fdcc5838e5 Mon Sep 17 00:00:00 2001 From: Nora Zn Date: Thu, 11 Apr 2024 10:04:14 +0200 Subject: [PATCH] Add functionality to validate analyzer and checker options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- analyzer/codechecker_analyzer/cmd/analyze.py | 139 ++++++++++-------- analyzer/codechecker_analyzer/cmd/check.py | 5 +- .../tests/functional/cmdline/test_cmdline.py | 24 +++ analyzer/tests/unit/test_checker_handling.py | 39 ++++- 4 files changed, 141 insertions(+), 66 deletions(-) diff --git a/analyzer/codechecker_analyzer/cmd/analyze.py b/analyzer/codechecker_analyzer/cmd/analyze.py index 98d970f731..d730517706 100644 --- a/analyzer/codechecker_analyzer/cmd/analyze.py +++ b/analyzer/codechecker_analyzer/cmd/analyze.py @@ -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 @@ -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( @@ -800,37 +803,43 @@ 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: @@ -838,34 +847,40 @@ 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( @@ -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( diff --git a/analyzer/codechecker_analyzer/cmd/check.py b/analyzer/codechecker_analyzer/cmd/check.py index abbb7add6d..7456307a36 100644 --- a/analyzer/codechecker_analyzer/cmd/check.py +++ b/analyzer/codechecker_analyzer/cmd/check.py @@ -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") @@ -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', diff --git a/analyzer/tests/functional/cmdline/test_cmdline.py b/analyzer/tests/functional/cmdline/test_cmdline.py index a3408dd131..c721da7b34 100644 --- a/analyzer/tests/functional/cmdline/test_cmdline.py +++ b/analyzer/tests/functional/cmdline/test_cmdline.py @@ -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. diff --git a/analyzer/tests/unit/test_checker_handling.py b/analyzer/tests/unit/test_checker_handling.py index 8f127127ae..d2c97615f7 100644 --- a/analyzer/tests/unit/test_checker_handling.py +++ b/analyzer/tests/unit/test_checker_handling.py @@ -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 @@ -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): """