Update how comments are added to config files

2 files changed, 139 insertions, 64 deletions

diff --git a/pylint/config.py b/pylint/config.py
index 95a4e3c24..699f789a0 100644
--- a/pylint/config.py
+++ b/pylint/config.py
@@ -376,7 +376,7 @@ class ConfigurationStore(object):
 
 class ConfigParser(metaclass=abc.ABCMeta):
     def __init__(self):
-        self._option_definitions = {}
+        self._option_definitions = collections.OrderedDict()
         self._option_groups = set()
 
     def add_option_definitions(self, option_definitions):
@@ -535,29 +535,6 @@ class FileParser(ConfigParser, metaclass=abc.ABCMeta):
         pass
 
 
-def make_commenter(description):
-    """Make a function to add comments from re.sub."""
-
-    def make_comment(match):
-        """Make the replacement string including commented desciption."""
-        comment = "\n".join("# {}".format(line) for line in textwrap.wrap(description))
-        return match.expand(r"{}\n\g<0>".format(comment))
-
-    return make_comment
-
-
-def make_commented_config_text(option_definitions, config_text):
-    """Make config ini text with descriptions added as comments."""
-    for name, definition in option_definitions.items():
-        config_text = re.sub(
-            r"^{}[ =]".format(re.escape(name)),
-            make_commenter(definition["help"]),
-            config_text,
-            flags=re.MULTILINE,
-        )
-    return config_text
-
-
 class IniFileParser(FileParser):
     """Parses a config files into config objects."""
 
@@ -604,9 +581,7 @@ class IniFileParser(FileParser):
         group = definition.get("group", "MASTER").upper()
         return group, default
 
-    def parse(self, to_parse, config):
-        self._parser.read(to_parse)
-
+    def apply_to_configuration(self, configuration):
         for section in self._parser.sections():
             # Normalise the section titles
             if not section.isupper():
@@ -622,14 +597,65 @@ class IniFileParser(FileParser):
                     type_ = definition.get("type")
                     validator = VALIDATORS.get(type_, lambda x: x)
                     value = validator(value)
-                config.set_option(option, value)
+                configuration.set_option(option, value)
+
+    def parse(self, to_parse, config):
+        self._parser.read(to_parse)
+        self.apply_to_configuration(config)
 
     def write(self, stream=sys.stdout):
-        with six.StringIO() as temp_stream:
-            self._parser.write(temp_stream)
-            config_text = temp_stream.getvalue()
-        config_text = make_commented_config_text(self._option_definitions, config_text)
-        stream.write(config_text)
+        """
+        Write out config to stream.
+
+        Includes option help and options with default values as
+        comments.
+        """
+        # We can't reuse self._parser because it doesn't have the help
+        # comments. allow_no_value lets us add comments as keys.
+        write_parser = configparser.ConfigParser(
+            default_section="MASTER", allow_no_value=True
+        )
+
+        # This makes keys case sensitive, needed for preserving comment
+        # formatting.
+        write_parser.optionxform = str
+
+        configuration = Configuration()
+        self.apply_to_configuration(configuration)
+        config_set_args = self.make_set_args(self._option_definitions, configuration)
+        for args in config_set_args:
+            section = args[0]
+            if section != "MASTER" and not write_parser.has_section(section):
+                write_parser.add_section(section)
+            write_parser.set(*args)
+        write_parser.write(stream)
+
+    @staticmethod
+    def make_set_args(option_definitions, configuration):
+        """Make args for configparser.ConfigParser.set."""
+        for option, definition in option_definitions.items():
+            section = definition.get("group", "MASTER").upper()
+            help_lines = [
+                "# {}".format(line)
+                for line in textwrap.wrap(definition.get("help", ""))
+            ]
+            for help_line in help_lines:
+                # Calling set with only two args does not leave an =
+                # symbol at the end of the line.
+                yield (section, help_line)
+            default = definition.get("default")
+            option_type = definition.get("type", "string")
+            value = getattr(configuration, option)
+            if option_type == "csv":
+                str_value = ",\n".join(value)
+            else:
+                str_value = UNVALIDATORS[option_type](value)
+            if value == default:
+                key = "# {}".format(option)
+                str_value = "\n# ".join(str_value.split("\n"))
+            else:
+                key = option
+            yield (section, key, str_value)
 
 
 class LongHelpFormatter(argparse.HelpFormatter):
diff --git a/pylint/test/unittest_config.py b/pylint/test/unittest_config.py
index 0ef6c331e..a52d5e6b5 100644
--- a/pylint/test/unittest_config.py
+++ b/pylint/test/unittest_config.py
@@ -9,6 +9,7 @@
 
 """Unit tests for the config module."""
 
+from argparse import Namespace
 import re
 import sre_constants
 
@@ -64,52 +65,100 @@ def test__regexp_csv_validator_invalid():
 
 
 @pytest.mark.comments
-def test_make_commenter():
-    """Test make_commenter."""
-    commenter = config.make_commenter(
+def test_make_set_args_default():
+    """Test IniFileParser.make_set_args comments default values."""
+    assert list(
+        config.IniFileParser.make_set_args(
+            {
+                "validate": {
+                    "help": "Validate something.",
+                    "group": "checks",
+                    "default": "strictly",
+                }
+            },
+            Namespace(validate="strictly"),
+        )
+    ) == [("CHECKS", "# Validate something."), ("CHECKS", "# validate", "strictly")]
+
+
+@pytest.mark.comments
+def test_make_set_args_long():
+    """Test IniFileParser.make_set_args wraps long help comments."""
+    assert list(
+        config.IniFileParser.make_set_args(
+            {
+                "check": {
+                    "help": (
+                        "Check something. I am continuing to type "
+                        "because I want a long line to test multiline "
+                        "comments."
+                    ),
+                    "group": "checks",
+                }
+            },
+            Namespace(check="1"),
+        )
+    ) == [
         (
-            "This is a long description, I am trying to test wrapping "
-            "works OK. I hope this test passes."
+            "CHECKS",
+            (
+                "# Check something. I am continuing to type "
+                "because I want a long line to"
+            ),
+        ),
+        ("CHECKS", "# test multiline comments."),
+        ("CHECKS", "check", "1"),
+    ]
+
+
+@pytest.mark.comments
+def test_make_set_args_csv():
+    """Test IniFileParser.make_set_args writes multiline csv values."""
+    assert list(
+        config.IniFileParser.make_set_args(
+            {"assess": {"help": "Assess something.", "group": "checks", "type": "csv"}},
+            Namespace(assess=["somewhat", "completely"]),
         )
-    )
-    match = re.match("^check ", "check = hey")
-    assert commenter(match) == (
-        "# This is a long description, I am trying to test "
-        "wrapping works OK. I\n"
-        "# hope this test passes.\n"
-        "check "
-    )
+    ) == [
+        ("CHECKS", "# Assess something."),
+        ("CHECKS", "assess", "somewhat,\ncompletely"),
+    ]
 
 
 @pytest.mark.comments
-def test_make_commented_config_text():
-    """Test make_commented_config_text."""
-    assert config.make_commented_config_text(
-        {
-            "check": {"help": "Check something."},
-            "validate": {"help": "Validate a thing."},
-        },
-        "check = 1\nvalidate=2\nother = 3",
-    ) == (
-        "# Check something.\n"
-        "check = 1\n"
-        "# Validate a thing.\n"
-        "validate=2\n"
-        "other = 3"
-    )
+def test_make_set_args_comment_csv():
+    """Test IniFileParser.make_set_args comments out every line."""
+    assert list(
+        config.IniFileParser.make_set_args(
+            {
+                "confirm": {
+                    "help": "Confirm something.",
+                    "group": "checks",
+                    "type": "csv",
+                    "default": ["present", "accurate"],
+                }
+            },
+            Namespace(confirm=["present", "accurate"]),
+        )
+    ) == [
+        ("CHECKS", "# Confirm something."),
+        ("CHECKS", "# confirm", "present,\n# accurate"),
+    ]
 
 
 @pytest.mark.comments
 def test_write(tmpdir):
-    """Test IniFileParser.write."""
+    """Test IniFileParser.write writes and INI file with comments."""
     parser = config.IniFileParser()
     parser.add_option_definitions(
         (("check", {"help": "Check something.", "group": "checks"}),)
     )
     config_path = tmpdir.join("config.cfg")
-    config_path.write("[checks]\ncheck = 1")
+    config_path.write("[CHECKS]\n" "check = 1")
     parser.parse(str(config_path), config.Configuration())
     output_path = tmpdir.join("output.cfg")
     with output_path.open("w") as output:
         parser.write(stream=output)
-    assert output_path.read() == "[CHECKS]\n# Check something.\ncheck = 1\n\n"
+    assert output_path.read() == (
+        "[CHECKS]\n" "# Check something.\n" "check = 1\n" "\n"
+    )