From 50b2a415ddc5e384671eb8a349211d7b89e7d489 Mon Sep 17 00:00:00 2001 From: bruntib Date: Thu, 21 Jan 2021 12:16:55 +0100 Subject: [PATCH] [CLI] Fix double clang-tidy config flags `clang-tidy` can be given config options via --analyzer-config and -config in --tidyargs. --analyzer-config also has a default value, so in case --tidyargs also contains a -config flag then both is given to clang-tidy which thus fails. --- .../analyzers/clangtidy/analyzer.py | 21 ++++++++++++ analyzer/codechecker_analyzer/cmd/analyze.py | 5 ++- analyzer/codechecker_analyzer/cmd/check.py | 5 ++- .../tests/functional/analyze/test_analyze.py | 16 ++++++---- .../analyze/test_files/extra_args.c | 11 ------- .../analyze/test_files/extra_args.cpp | 20 ++++++++++++ .../functional/analyze/test_files/tidyargs | 2 +- docs/analyzer/user_guide.md | 32 +++++++++++-------- 8 files changed, 78 insertions(+), 34 deletions(-) delete mode 100644 analyzer/tests/functional/analyze/test_files/extra_args.c create mode 100644 analyzer/tests/functional/analyze/test_files/extra_args.cpp diff --git a/analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py b/analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py index 950603be91..23107678e9 100644 --- a/analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py +++ b/analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py @@ -330,6 +330,27 @@ def construct_config_handler(cls, args, context): if m.group('analyzer') == cls.ANALYZER_NAME: analyzer_config[m.group('key')] = m.group('value') + # If both --analyzer-config and -config (in --tidyargs) is given then + # these need to be merged. Since "HeaderFilterRegex" has a default + # value in --analyzer-config, we take --tidyargs stronger so user can + # overwrite its value. + for i, extra_arg in enumerate(handler.analyzer_extra_arguments): + if not extra_arg.startswith('-config'): + continue + + # -config flag can be together or separate from its argument: + # "-config blabla" vs. "-config=blabla" + if extra_arg == '-config': + arg = handler.analyzer_extra_arguments[i + 1] + arg_num = 2 + else: + arg = extra_arg[len('-config='):] + arg_num = 1 + + analyzer_config.update(json.loads(arg)) + del handler.analyzer_extra_arguments[i:i + arg_num] + break + # TODO: This extra "isinsrance" check is needed for # CodeChecker checkers --checker-config. This command also # runs this function in order to construct a config handler. diff --git a/analyzer/codechecker_analyzer/cmd/analyze.py b/analyzer/codechecker_analyzer/cmd/analyze.py index 0b209bb70f..0429afee8f 100644 --- a/analyzer/codechecker_analyzer/cmd/analyze.py +++ b/analyzer/codechecker_analyzer/cmd/analyze.py @@ -393,7 +393,10 @@ def add_arguments_to_parser(parser): "default behaviour of this option you can " "use the " "'clang-tidy:take-config-from-directory=" - "true' option.") + "true' option. If the file at --tidyargs " + "contains a -config flag then those " + "options extend these and override " + "\"HeaderFilterRegex\" if any.") analyzer_opts.add_argument('--checker-config', dest='checker_config', diff --git a/analyzer/codechecker_analyzer/cmd/check.py b/analyzer/codechecker_analyzer/cmd/check.py index f182392d02..b851fbf948 100644 --- a/analyzer/codechecker_analyzer/cmd/check.py +++ b/analyzer/codechecker_analyzer/cmd/check.py @@ -394,7 +394,10 @@ def add_arguments_to_parser(parser): "default behaviour of this option you can " "use the " "'clang-tidy:take-config-from-directory=" - "true' option.") + "true' option. If the file at --tidyargs " + "contains a -config flag then those " + "options extend these and override " + "\"HeaderFilterRegex\" if any.") analyzer_opts.add_argument('--checker-config', dest='checker_config', diff --git a/analyzer/tests/functional/analyze/test_analyze.py b/analyzer/tests/functional/analyze/test_analyze.py index 2cd0e6aad1..d7c8d39a43 100644 --- a/analyzer/tests/functional/analyze/test_analyze.py +++ b/analyzer/tests/functional/analyze/test_analyze.py @@ -474,12 +474,12 @@ def test_tidyargs_saargs(self): """ build_json = os.path.join(self.test_workspace, "build_extra_args.json") report_dir = os.path.join(self.test_workspace, "reports_extra_args") - source_file = os.path.join(self.test_dir, "extra_args.c") + source_file = os.path.join(self.test_dir, "extra_args.cpp") tidyargs_file = os.path.join(self.test_dir, "tidyargs") saargs_file = os.path.join(self.test_dir, "saargs") build_log = [{"directory": self.test_dir, - "command": "cc -c " + source_file, + "command": "g++ -c " + source_file, "file": source_file }] @@ -488,7 +488,9 @@ def test_tidyargs_saargs(self): json.dump(build_log, outfile) analyze_cmd = [self._codechecker_cmd, "analyze", build_json, - "-o", report_dir, "--tidyargs", tidyargs_file] + "-o", report_dir, "--tidyargs", tidyargs_file, + "--analyzer-config", 'clang-tidy:HeaderFilterRegex=.*', + 'clang-tidy:Checks=modernize-use-bool-literals'] process = subprocess.Popen( analyze_cmd, @@ -509,6 +511,8 @@ def test_tidyargs_saargs(self): out, _ = process.communicate() self.assertIn("division by zero", out) + self.assertIn("modernize-avoid-bind", out) + self.assertNotIn("performance-for-range-copy", out) analyze_cmd = [self._codechecker_cmd, "analyze", build_json, "-o", report_dir, "--saargs", saargs_file] @@ -821,13 +825,13 @@ def test_makefile_generation(self): analyze_cmd = [self._codechecker_cmd, "analyze", build_json, "-o", self.report_dir, '--makefile'] - source_file = os.path.join(self.test_dir, "extra_args.c") + source_file = os.path.join(self.test_dir, "extra_args.cpp") build_log = [{"directory": self.test_workspace, - "command": "gcc -DTIDYARGS -c " + source_file, + "command": "g++ -DTIDYARGS -c " + source_file, "file": source_file }, {"directory": self.test_workspace, - "command": "gcc -DSAARGS -DTIDYARGS -c " + source_file, + "command": "g++ -DSAARGS -DTIDYARGS -c " + source_file, "file": source_file }] diff --git a/analyzer/tests/functional/analyze/test_files/extra_args.c b/analyzer/tests/functional/analyze/test_files/extra_args.c deleted file mode 100644 index e12f0f6867..0000000000 --- a/analyzer/tests/functional/analyze/test_files/extra_args.c +++ /dev/null @@ -1,11 +0,0 @@ -int main() -{ - #ifdef TIDYARGS - int i = 1 / 0; - #endif - - #ifdef SAARGS - int* p = 0; - *p = 42; - #endif -} diff --git a/analyzer/tests/functional/analyze/test_files/extra_args.cpp b/analyzer/tests/functional/analyze/test_files/extra_args.cpp new file mode 100644 index 0000000000..95bf24001e --- /dev/null +++ b/analyzer/tests/functional/analyze/test_files/extra_args.cpp @@ -0,0 +1,20 @@ +#include + +int add(int x, int y) { return x + y; } + +int main() +{ + #ifdef TIDYARGS + int i = 1 / 0; + #endif + + #ifdef SAARGS + int* p = 0; + *p = 42; + #endif + + int x = 2; + auto clj = std::bind(add, x, std::placeholders::_1); + + bool b = 1; +} diff --git a/analyzer/tests/functional/analyze/test_files/tidyargs b/analyzer/tests/functional/analyze/test_files/tidyargs index e5c888ebe0..dcff6e4462 100644 --- a/analyzer/tests/functional/analyze/test_files/tidyargs +++ b/analyzer/tests/functional/analyze/test_files/tidyargs @@ -1 +1 @@ ---extra-arg=-DTIDYARGS +--extra-arg=-DTIDYARGS '-config={"Checks": "modernize-avoid-bind"}' diff --git a/docs/analyzer/user_guide.md b/docs/analyzer/user_guide.md index b2047b915d..cbf0e12334 100644 --- a/docs/analyzer/user_guide.md +++ b/docs/analyzer/user_guide.md @@ -161,7 +161,7 @@ optional arguments: Set verbosity level. log arguments: - + Specify how the build information database should be obtained. You need to specify either an already existing log file, or a build command which will be used to generate a log file on the fly. @@ -248,8 +248,10 @@ analyzer arguments: 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. (default: ['clang- - tidy:HeaderFilterRegex=.*']) + 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=.*']) --checker-config [CHECKER_CONFIG [CHECKER_CONFIG ...]] Checker configuration options in the following format: analyzer:key=value. The collection of the options can @@ -273,7 +275,7 @@ analyzer arguments: solver only. (default: on) cross translation unit analysis arguments: - + These arguments are only available if the Clang Static Analyzer supports Cross-TU analysis. By default, no CTU analysis is run when 'CodeChecker check' is called. @@ -307,7 +309,7 @@ cross translation unit analysis arguments: second phase of the analysis. (default: parse-on-demand) checker configuration: - + Checkers ------------------------------------------------ The analyzer performs checks that are categorized into families or "checkers". @@ -319,7 +321,7 @@ checker configuration: 'core.uninitialized' group. Please consult the manual for details. Disabling certain checkers - such as the 'core' group - is unsupported by the LLVM/Clang community, and thus discouraged. - + Compiler warnings and errors ------------------------------------------------ Compiler warnings are diagnostic messages that report constructions that are @@ -899,13 +901,13 @@ one explicitly specified to **be analyzed** (`/dir/do.check.this.file`). ``` In the above example, `important_file.cpp` will be analyzed even if every file -where the path matches to `/my_project/my_lib_to_skip` will be skiped. -Every other file where the path contains `/myproject` except the files in the +where the path matches to `/my_project/my_lib_to_skip` will be skiped. +Every other file where the path contains `/myproject` except the files in the `my_project/3pplib` will be analyzed. The provided *shell-style* pattern is converted to a regex with the [fnmatch.translate](https://docs.python.org/2/library/fnmatch.html#fnmatch.translate). -Please note that when `-i SKIPFILE` is used along with `--stats` or +Please note that when `-i SKIPFILE` is used along with `--stats` or `--ctu` the skip list will be ignored in the pre-analysis phase. This means that statistics and ctu-pre-analysis will be created for *all* files in the *compilation database*. @@ -919,7 +921,7 @@ analyzer arguments: Currently supported analyzers are: clangsa, clang- tidy. --add-compiler-defaults - DEPRECATED. Always True. + DEPRECATED. Always True. Retrieve compiler-specific configuration from the compilers themselves, and use them with Clang. This is used when the compiler on the system is special, e.g. @@ -961,8 +963,10 @@ analyzer arguments: 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. (default: ['clang- - tidy:HeaderFilterRegex=.*']) + 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=.*']) --checker-config [CHECKER_CONFIG [CHECKER_CONFIG ...]] Checker configuration options in the following format: analyzer:key=value. The collection of the options can @@ -1034,7 +1038,7 @@ Lets assume you have a configuration file ``` This configuration file example contains configuration options for multiple codechecker subcommands (analyze, parse, server, store) so not just the -`analyze` subcommand can be configured like this. +`analyze` subcommand can be configured like this. The focus is on the `analyze` subcommand configuration in the next examples. If you run the following command: @@ -1469,7 +1473,7 @@ Statistics analysis feature arguments: function (calculated as calls with a property/all calls). CodeChecker will warn for calls of f that do not have that property.(default: 0.85) - + ``` ## `parse`