Merge pull request Ericsson#3655 from Szelethus/fix_analyzer_config_c…

…onflict

[config][CLI] Merge, and don't override when multiple --analyzer-configs are specified

analyzer/codechecker_analyzer/analyzers/clangtidy/analyzer.py

@@ -362,7 +362,7 @@ def construct_config_handler(cls, args, context):
             LOG.debug_analyzer(aerr)
 
         analyzer_config = {}
-        # TODO: This extra "isinsrance" check is needed for
+        # TODO: This extra "isinstance" check is needed for
         # CodeChecker analyzers --analyzer-config. This command also
         # runs this function in order to construct a config handler.
         if 'analyzer_config' in args and \
@@ -394,7 +394,7 @@ def construct_config_handler(cls, args, context):
             del handler.analyzer_extra_arguments[i:i + arg_num]
             break
 
-        # TODO: This extra "isinsrance" check is needed for
+        # TODO: This extra "isinstance" check is needed for
         # CodeChecker checkers --checker-config. This command also
         # runs this function in order to construct a config handler.
         if 'checker_config' in args and isinstance(args.checker_config, list):

analyzer/codechecker_analyzer/arg.py

@@ -36,6 +36,38 @@ def __call__(self, parser, namespace, value, option_string=None):
         if 'ordered_checkers' not in namespace:
             namespace.ordered_checkers = []
         ordered_checkers = namespace.ordered_checkers
+        # Map each checker to whether its enabled or not.
         ordered_checkers.append((value, self.dest == 'enable'))
 
         namespace.ordered_checkers = ordered_checkers
+
+
+class OrderedConfigAction(argparse.Action):
+    """
+    Action to store --analyzer-config and --checker-config values. These may
+    come from many sources, including the CLI, the CodeChecker config file,
+    the saargs file and the tidyargs file, or some other places.
+    """
+
+    def __init__(self, option_strings, dest, nargs=None, **kwargs):
+        if nargs is not '*':
+            raise ValueError("nargs must be '*' for backward compatibility "
+                             "reasons!")
+        super(OrderedConfigAction, self).__init__(option_strings, dest,
+                                                  nargs, **kwargs)
+
+    def __call__(self, parser, namespace, value, option_string=None):
+
+        assert isinstance(value, list), \
+               f"--analyzer-config or --checker-config value ({value}) is " \
+               "not a list, but should be if nargs is not None!"
+
+        if not hasattr(namespace, self.dest):
+            setattr(namespace, self.dest, [])
+
+        dest = getattr(namespace, self.dest)
+
+        for flag in value:
+            if flag in dest:
+                dest.remove(flag)
+            dest.append(flag)

analyzer/codechecker_analyzer/cmd/analyze.py

@@ -26,7 +26,8 @@
 
 from codechecker_analyzer import analyzer, analyzer_context, env
 from codechecker_analyzer.analyzers import analyzer_types, clangsa
-from codechecker_analyzer.arg import OrderedCheckersAction
+from codechecker_analyzer.arg import \
+        OrderedCheckersAction, OrderedConfigAction
 from codechecker_analyzer.buildlog import log_parser
 
 from codechecker_common import arg, logger, cmd_config
@@ -401,6 +402,7 @@ def add_arguments_to_parser(parser):
     analyzer_opts.add_argument('--analyzer-config',
                                dest='analyzer_config',
                                nargs='*',
+                               action=OrderedConfigAction,
                                default=["clang-tidy:HeaderFilterRegex=.*"],
                                help="Analyzer configuration options in the "
                                     "following format: analyzer:key=value. "
@@ -423,6 +425,7 @@ def add_arguments_to_parser(parser):
     analyzer_opts.add_argument('--checker-config',
                                dest='checker_config',
                                nargs='*',
+                               action=OrderedConfigAction,
                                default=argparse.SUPPRESS,
                                help="Checker configuration options in the "
                                     "following format: analyzer:key=value. "

analyzer/codechecker_analyzer/cmd/check.py

@@ -20,7 +20,8 @@
 
 from codechecker_analyzer import analyzer_context
 from codechecker_analyzer.analyzers import analyzer_types
-from codechecker_analyzer.arg import OrderedCheckersAction
+from codechecker_analyzer.arg import \
+        OrderedCheckersAction, OrderedConfigAction
 
 from codechecker_common import arg, cmd_config, logger
 from codechecker_report_converter.source_code_comment_handler import \
@@ -349,6 +350,7 @@ def add_arguments_to_parser(parser):
     analyzer_opts.add_argument('--analyzer-config',
                                dest='analyzer_config',
                                nargs='*',
+                               action=OrderedConfigAction,
                                default=["clang-tidy:HeaderFilterRegex=.*"],
                                help="Analyzer configuration options in the "
                                     "following format: analyzer:key=value. "
@@ -371,6 +373,7 @@ def add_arguments_to_parser(parser):
     analyzer_opts.add_argument('--checker-config',
                                dest='checker_config',
                                nargs='*',
+                               action=OrderedConfigAction,
                                default=argparse.SUPPRESS,
                                help="Checker configuration options in the "
                                     "following format: analyzer:key=value. "

analyzer/tests/functional/config/test_config.py

@@ -118,6 +118,129 @@ def test_only_clangsa_config(self):
         self.assertIn("clangsa analyzed simple.cpp", out)
         self.assertNotIn("clang-tidy analyzed simple.cpp", out)
 
+    def test_config_file_multiple_analyzer_config_resolution(self):
+        """
+        Test whether multiple --analyzer-config arguments from a CodeChecker
+        config file are merged, and don't overwrite one another.
+        """
+        with open(self.config_file_json, 'w+',
+                  encoding="utf-8", errors="ignore") as config_f:
+            json.dump({
+                'analyzer': [
+                    "--analyzer-config",
+                    "clangsa:track-conditions=false",
+                    "--analyzer-config",
+                    "clang-tidy:HeaderFilterRegex=.*"
+                ]}, config_f)
+
+        out, returncode = self.__run_analyze(self.config_file_json,
+                                             ["--verbose", "debug_analyzer"])
+
+        self.assertNotEqual(returncode, 1)
+        self.assertIn("track-conditions=false", out)
+        self.assertIn("{\"HeaderFilterRegex\": \".*\"}", out)
+
+    def test_config_file_and_cmd_resolution(self):
+        """
+        Test whether multiple --analyzer-config arguments from *both* a
+        CodeChecker config file and the CLI are merged, and don't overwrite one
+        another.
+        """
+        with open(self.config_file_json, 'w+',
+                  encoding="utf-8", errors="ignore") as config_f:
+            json.dump({
+                'analyzer': [
+                    "--analyzer-config",
+                    "clangsa:track-conditions=false"
+                ]}, config_f)
+
+        out, returncode = self.__run_analyze(self.config_file_json,
+                                             ["--analyzer-config",
+                                              "clang-tidy:"
+                                              "HeaderFilterRegex=.*",
+                                              "--verbose", "debug_analyzer"])
+
+        self.assertNotEqual(returncode, 1)
+        self.assertIn("track-conditions=false", out)
+        self.assertIn("{\"HeaderFilterRegex\": \".*\"}", out)
+
+    def test_cmd_multiple_analyzer_config_resolution(self):
+        """
+        Test whether multiple --analyzer-config arguments from the command line
+        are merged, and don't overwrite one another.
+        """
+
+        with open(self.config_file_json, 'w+',
+                  encoding="utf-8", errors="ignore") as config_f:
+            config_f.write("")
+
+        out, returncode = self.__run_analyze(self.config_file_json,
+                                             ["--analyzer-config",
+                                              "clang-tidy:"
+                                              "HeaderFilterRegex=.*",
+                                              "--analyzer-config",
+                                              "clangsa:track-conditions=false",
+                                              "--verbose", "debug_analyzer"])
+
+        self.assertNotEqual(returncode, 1)
+        self.assertIn("track-conditions=false", out)
+        self.assertIn("{\"HeaderFilterRegex\": \".*\"}", out)
+
+    def test_cmd_multiple_checker_config_resolution(self):
+        """
+        Test whether multiple --checker-config arguments from the command line
+        are merged, and don't overwrite one another.
+        """
+
+        with open(self.config_file_json, 'w+',
+                  encoding="utf-8", errors="ignore") as config_f:
+            config_f.write("")
+
+        out, returncode = self.__run_analyze(self.config_file_json,
+                                             ["--checker-config",
+                                              "clangsa:"
+                                              "core.CallAndMessage:"
+                                              "CXXDeallocationArg=true",
+                                              "--checker-config",
+                                              "clangsa:"
+                                              "core.CallAndMessage:"
+                                              "ParameterCount=true",
+                                              "--verbose", "debug_analyzer"])
+
+        self.assertNotEqual(returncode, 1)
+        self.assertIn("core.CallAndMessage:CXXDeallocationArg=true", out)
+        self.assertIn("core.CallAndMessage:ParameterCount=true", out)
+
+    def test_cmd_overrides_config_file(self):
+        """
+        Test the precedence of multiple --analyzer-config arguments specify the
+        same option from both a CodeChecker config file and the CLI, but with a
+        different value. CLI arguments should in effect override the config
+        file (is the later argument in the invocation).
+        """
+        with open(self.config_file_json, 'w+',
+                  encoding="utf-8", errors="ignore") as config_f:
+            json.dump({
+                'analyze': [
+                    '--analyzers', 'clangsa',
+                    "--analyzer-config",
+                    "clangsa:track-conditions=false"
+                ]}, config_f)
+
+        out, returncode = self.__run_analyze(self.config_file_json,
+                                             ["--analyzer-config",
+                                              "clangsa:track-conditions=true",
+                                              "--verbose", "debug_analyzer"])
+
+        self.assertNotEqual(returncode, 1)
+
+        # If the config from the config file is positioned behind the config
+        # from the CLI, it will override the config file. As intended.
+        cli_idx = out.rfind("track-conditions=true")
+        conf_file_idx = out.rfind("track-conditions=false")
+        self.assertLess(conf_file_idx, cli_idx)
+        self.assertEqual(out.count("track-conditions=true"), 1)
+
     def test_only_clangsa_config_backward_compatible_mixed(self):
         """
         Test the 'analyzer' configuration option backward compatibility.