Repository: NextGenContributions/nitpick
Issue category: Feature Request

Issue:
Support multiline csv value in INI config
- `flake8` config typically has multiline config value such as `per-file-ignores`
- each line has value that is usually comma separated

We should support this type of config for INI format as it's frequently used to incrementally fine-tune flake8 rules




Hints:

Patch:
diff --git a/docs/nitpick_section.rst b/docs/nitpick_section.rst
index 2d3b8c56..5352e1b2 100644
--- a/docs/nitpick_section.rst
+++ b/docs/nitpick_section.rst
@@ -60,6 +60,24 @@ Multiple keys can be added.
     [nitpick.files.comma_separated_values]
     "setup.cfg" = ["flake8.ignore", "isort.some_key", "another_section.another_key"]
 
+For config values in INI file that span across multiple lines such as ``flake8.per-file-ignores``, then ``per-file-ignores`` should be added to the list of comma separated values as well as being nested under ``flake8`` section in the style file.
+
+.. code-block:: toml
+
+    [nitpick.files.comma_separated_values]
+    ".flake8" = ["flake8.per-file-ignores"]
+
+    [".flake8".flake8.per-file-ignores]
+    "tests/*.py" = "WPS116,WPS118,WPS2"
+    "tests_2/*.py" = "WPS116,WPS118,WPS218"
+
+.. code-block:: ini
+
+    per-file-ignores =
+      tests/*.py:WPS116,WPS118,WPS2
+      tests_2/*.py:WPS118,WPS218,WPS116
+
+
 [nitpick.styles]
 ----------------
 
diff --git a/src/nitpick/plugins/ini.py b/src/nitpick/plugins/ini.py
index e1fc86d2..fd94a174 100644
--- a/src/nitpick/plugins/ini.py
+++ b/src/nitpick/plugins/ini.py
@@ -2,6 +2,7 @@
 
 from __future__ import annotations
 
+import copy
 from configparser import ConfigParser, DuplicateOptionError, Error, MissingSectionHeaderError, ParsingError
 from io import StringIO
 from typing import TYPE_CHECKING, Any, ClassVar, Iterator
@@ -202,12 +203,94 @@ def enforce_section(self, section: str) -> Iterator[Fuss]:
         for diff_type, key, values in dictdiffer.diff(actual_dict, expected_dict):
             if diff_type == dictdiffer.CHANGE:
                 if f"{section}.{key}" in self.comma_separated_values:
-                    yield from self.enforce_comma_separated_values(section, key, values[0], values[1])
+                    if isinstance(values[1], dict):
+                        # The expected value is a dictionary, so it's a multiline value
+                        # make sure the actual value is also a dictionary
+                        lines = values[0].strip().split("\n")
+                        actual = {
+                            key: value
+                            for line in lines
+                            if not line.startswith((";", "#"))  # ignore INI comments
+                            for key, value in [line.split(":")]
+                        }
+                        expected = values[1]
+                        yield from self.enforce_comma_separated_values_multiline(section, key, actual, expected)
+                    else:
+                        yield from self.enforce_comma_separated_values(section, key, values[0], values[1])
                 else:
                     yield from self.compare_different_keys(section, key, values[0], values[1])
             elif diff_type == dictdiffer.ADD:
                 yield from self.show_missing_keys(section, values)
 
+    def enforce_comma_separated_values_multiline(
+        self, section, key, raw_actual: dict[str, str], raw_expected: dict[str, str]
+    ) -> Iterator[Fuss]:
+        """Enforce sections and keys with comma-separated values.
+
+        The values might contain spaces.
+        """
+        # This dict is passed around for modifying if autofix is enabled
+        raw_actual_fixed = copy.deepcopy(raw_actual)
+
+        for diff_type, option, values in dictdiffer.diff(raw_actual, raw_expected):
+            if isinstance(option, list):
+                # 'dictdiffer.diff' supports dot notation for nested keys and thus might return a list if the key has a dot '.' in it.
+                # We don't support more than two levels of nested keys in this INI plugin. This check is only precautionary for key that contains '.'
+                option = option[0]  # noqa: PLW2901
+
+            if diff_type == dictdiffer.CHANGE:
+                yield from self._add_missing_values_in_existing_option_of_multiline_config_value(
+                    key, raw_actual_fixed, section, option, values
+                )
+            elif diff_type == dictdiffer.ADD:
+                yield from self._add_missing_option_in_multiline_config_value(key, raw_actual_fixed, section, values)
+
+        if self.autofix:
+            list_of_key_value_pairs = [f"{k}:{v}" for k, v in raw_actual_fixed.items()]
+            self.updater[section][key].set_values(list_of_key_value_pairs, indent=2 * " ")
+            self.dirty = True
+
+    def _add_missing_option_in_multiline_config_value(
+        self, key: str, raw_actual_fixed: dict[str, str], section: str, missing_options: list[tuple[str, str]]
+    ):
+        """Add a missing option in a multiline configuration value."""
+        section_header = "" if section == TOP_SECTION else f"[{section}]\n"
+        for option, value in missing_options:
+            if self.autofix:
+                raw_actual_fixed[option] = value
+            yield self.reporter.make_fuss(
+                Violations.MISSING_OPTION,
+                f'{section_header}{key} = (...)\n"{option}":{value}',
+                key=f'{key}."{option}"',
+                fixed=self.autofix,
+                section=section,
+            )
+
+    def _add_missing_values_in_existing_option_of_multiline_config_value(  # pylint: disable=too-many-arguments
+        self, key: str, raw_actual_fixed: dict[str, str], section: str, option: str, values: tuple[str, str]
+    ):
+        """Add missing values in an existing option of a multiline configuration value."""
+        actual = values[0]
+        expected = values[1]
+
+        actual_set = {s.strip() for s in actual.split(",")}
+        expected_set = {s.strip() for s in expected.split(",")}
+        missing = expected_set - actual_set
+        if not missing:
+            return
+
+        joined_values = ",".join(sorted(missing))
+        value_to_append = f",{joined_values}"
+        if self.autofix:
+            raw_actual_fixed[option] += value_to_append
+        section_header = "" if section == TOP_SECTION else f"[{section}]\n"
+        yield self.reporter.make_fuss(
+            Violations.MISSING_VALUES_IN_LIST,
+            f'{section_header}{key} = (...)\n"{option}":(...){value_to_append}',
+            key=f'{key}."{option}"',
+            fixed=self.autofix,
+        )
+
     def enforce_comma_separated_values(self, section, key, raw_actual: Any, raw_expected: Any) -> Iterator[Fuss]:
         """Enforce sections and keys with comma-separated values.
 
@@ -270,9 +353,21 @@ def show_missing_keys(self, section: str, values: list[tuple[str, Any]]) -> Iter
         """Show the keys that are not present in a section."""
         parser = ConfigParser()
         missing_dict = dict(values)
-        parser[section] = missing_dict
+
+        # Create a new dictionary to store the updated options
+        updated_missing_dict = {}
+        for key, value in missing_dict.items():
+            if isinstance(value, dict):
+                # Handle multiline config value by converting a dict to a string with the format:
+                # "\n  key1:value1\n  key2:value2"  # noqa: ERA001
+                sep = "\n  "
+                updated_missing_dict[key] = sep + sep.join(f"{k}:{v}" for k, v in value.items())
+            else:
+                updated_missing_dict[key] = value
+
+        parser[section] = updated_missing_dict
         output = self.get_example_cfg(parser)
-        self.add_options_before_space(section, missing_dict)
+        self.add_options_before_space(section, updated_missing_dict)
 
         if section == TOP_SECTION:
             yield self.reporter.make_fuss(


Test patch:
diff --git a/tests/test_ini.py b/tests/test_ini.py
index f2a11a78..31c84dcc 100644
--- a/tests/test_ini.py
+++ b/tests/test_ini.py
@@ -514,3 +514,178 @@ def test_falsy_values_should_be_reported_and_fixed(tmp_path, datadir):
         ]
     ).assert_file_contents(filename, datadir / "falsy_values/expected.ini")
     project.api_check().assert_violations()
+
+
+def test_comma_separated_values_in_multiline_config_value_should_be_enforced_if_some_lines_have_missing_values(
+    tmp_path,
+):
+    """Target config value is missing some comma-separated-values in some of its lines."""
+    ProjectMock(tmp_path).save_file(
+        ".flake8",
+        """
+        [flake8]
+        per-file-ignores =
+          tests/*.py:WPS116
+          tests_2/*.py:WPS116,WPS118,WPS218
+          tests_3/*.py:WPS218
+        """,
+    ).style(
+        f"""
+        [nitpick.files.{COMMA_SEPARATED_VALUES}]
+        ".flake8" = ["flake8.per-file-ignores"]
+
+        [".flake8".flake8.per-file-ignores]
+        "tests/*.py" = "WPS116,WPS118"
+        "tests_2/*.py" = "WPS116,WPS118,WPS218"
+        "tests_3/*.py" = "WPS116,WPS118,WPS218"
+        """
+    ).api_check_then_fix(
+        Fuss(
+            True,
+            ".flake8",
+            Violations.MISSING_VALUES_IN_LIST.code,
+            " has missing values in the 'per-file-ignores.\"tests/*.py\"' key. Include those values:",
+            '[flake8]\nper-file-ignores = (...)\n"tests/*.py":(...),WPS118',
+        ),
+        Fuss(
+            True,
+            ".flake8",
+            Violations.MISSING_VALUES_IN_LIST.code,
+            " has missing values in the 'per-file-ignores.\"tests_3/*.py\"' key. Include those values:",
+            '[flake8]\nper-file-ignores = (...)\n"tests_3/*.py":(...),WPS116,WPS118',
+        ),
+    ).assert_file_contents(
+        ".flake8",
+        """
+        [flake8]
+        per-file-ignores =
+          tests/*.py:WPS116,WPS118
+          tests_2/*.py:WPS116,WPS118,WPS218
+          tests_3/*.py:WPS218,WPS116,WPS118
+        """,
+    )
+
+
+def test_comma_separated_values_in_multiline_config_value_should_be_enforced_if_some_lines_are_missing(tmp_path):
+    """Target config value exists, but is missing some lines in its multi-line value."""
+    ProjectMock(tmp_path).save_file(
+        ".flake8",
+        """
+        [flake8]
+        per-file-ignores =
+          tests/*.py:WPS116
+        """,
+    ).style(
+        f"""
+        [nitpick.files.{COMMA_SEPARATED_VALUES}]
+        ".flake8" = ["flake8.per-file-ignores"]
+
+        [".flake8".flake8.per-file-ignores]
+        "tests/*.py" = "WPS116"
+        "tests_2/*.py" = "WPS116,WPS118,WPS218"
+        "tests_3/*.py" = "WPS116,WPS118"
+        """
+    ).api_check_then_fix(
+        Fuss(
+            True,
+            ".flake8",
+            Violations.MISSING_OPTION.code,
+            ": section [flake8] has some missing key/value pairs. Use this:",
+            '[flake8]\nper-file-ignores = (...)\n"tests_2/*.py":WPS116,WPS118,WPS218',
+        ),
+        Fuss(
+            True,
+            ".flake8",
+            Violations.MISSING_OPTION.code,
+            ": section [flake8] has some missing key/value pairs. Use this:",
+            '[flake8]\nper-file-ignores = (...)\n"tests_3/*.py":WPS116,WPS118',
+        ),
+    ).assert_file_contents(
+        ".flake8",
+        """
+        [flake8]
+        per-file-ignores =
+          tests/*.py:WPS116
+          tests_2/*.py:WPS116,WPS118,WPS218
+          tests_3/*.py:WPS116,WPS118
+        """,
+    )
+
+
+def test_comma_separated_values_in_multiline_config_value_should_be_enforced_if_missing_entirely(tmp_path):
+    """Target multi-line config value is missing entirely."""
+    ProjectMock(tmp_path).save_file(
+        ".flake8",
+        """
+        [flake8]
+        woof = meow
+        """,
+    ).style(
+        f"""
+        [nitpick.files.{COMMA_SEPARATED_VALUES}]
+        ".flake8" = ["flake8.per-file-ignores"]
+
+        [".flake8".flake8.per-file-ignores]
+        "tests/*.py" = "WPS116,WPS118"
+        "tests_2/*.py" = "WPS116,WPS118,WPS218"
+        """
+    ).api_check_then_fix(
+        Fuss(
+            True,
+            ".flake8",
+            Violations.MISSING_OPTION.code,
+            ": section [flake8] has some missing key/value pairs. Use this:",
+            "[flake8]\n"
+            "per-file-ignores = \n"
+            "\t  tests/*.py:WPS116,WPS118\n"
+            "\t  tests_2/*.py:WPS116,WPS118,WPS218",
+        )
+    ).assert_file_contents(
+        ".flake8",
+        """
+        [flake8]
+        woof = meow
+        per-file-ignores =
+          tests/*.py:WPS116,WPS118
+          tests_2/*.py:WPS116,WPS118,WPS218
+        """,
+    )
+
+
+def test_comma_separated_values_in_multiline_config_value_should_be_without_violation_if_no_changes(tmp_path):
+    """Target multi-line config value has not deviated from the style.
+
+    The comment preceding the 'per-file-ignores' config should be retained.
+    While the comments preceding each option in the 'per-file-ignores' key should be removed.
+    The support to retain such comments after fix is not implemented yet.
+    """
+    ProjectMock(tmp_path).save_file(
+        ".flake8",
+        """
+        [flake8]
+        ; this comment would be retained after fix
+        per-file-ignores =
+            # this comment would be removed after fix
+          tests/*.py:WPS116,WPS118
+          ; another comment that would be removed after fix
+          tests_2/*.py:WPS116,WPS118,WPS218
+        """,
+    ).style(
+        f"""
+        [nitpick.files.{COMMA_SEPARATED_VALUES}]
+        ".flake8" = ["flake8.per-file-ignores"]
+
+        [".flake8".flake8.per-file-ignores]
+        "tests/*.py" = "WPS116,WPS118"
+        "tests_2/*.py" = "WPS116,WPS118,WPS218"
+        """
+    ).api_check_then_fix().assert_file_contents(
+        ".flake8",
+        """
+        [flake8]
+        ; this comment would be retained after fix
+        per-file-ignores =
+          tests/*.py:WPS116,WPS118
+          tests_2/*.py:WPS116,WPS118,WPS218
+        """,
+    )