--write: Move write_list processing to separate tested function

.github/workflows/tox.yml

@@ -156,7 +156,7 @@ jobs:
       WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 560
+      PYTEST_REQPASS: 599
 
     steps:
       - name: Activate WSL1

src/ansiblelint/transformer.py

@@ -31,16 +31,7 @@ class Transformer:
 
     def __init__(self, result: LintResult, options: Namespace):
         """Initialize a Transformer instance."""
-        write_list: List[str] = options.write_list
-        # "none" resets the enabled transforms
-        # So, write_list will be ["none"] or everything after "none"
-        none_indexes = [i for i, value in enumerate(write_list) if value == "none"]
-        if none_indexes:
-            index = none_indexes[-1]
-            if len(write_list) > index + 1:
-                index += 1
-            write_list = write_list[index:]
-        self.write_list = write_list
+        self.write_set = self.effective_write_set(options.write_list)
 
         self.matches: List[MatchError] = result.matches
         self.files: Set[Lintable] = result.files
@@ -61,6 +52,26 @@ def __init__(self, result: LintResult, options: Namespace):
                 self.matches_per_file[lintable] = []
             self.matches_per_file[lintable].append(match)
 
+    @staticmethod
+    def effective_write_set(write_list: List[str]) -> Set[str]:
+        """Simplify write_list based on ``"none"`` and ``"all"`` keywords.
+
+        ``"none"`` resets the enabled rule transforms.
+        This returns ``{"none"}`` or a set of everything after the last ``"none"``.
+
+        If ``"all"`` is in the ``write_list`` (after ``"none"`` if present),
+        then this will return ``{"all"}``.
+        """
+        none_indexes = [i for i, value in enumerate(write_list) if value == "none"]
+        if none_indexes:
+            index = none_indexes[-1]
+            if len(write_list) > index + 1:
+                index += 1
+            write_list = write_list[index:]
+        if "all" in write_list:
+            return {"all"}
+        return set(write_list)
+
     def run(self) -> None:
         """For each file, read it, execute transforms on it, then write it."""
         for file, matches in self.matches_per_file.items():
@@ -89,7 +100,7 @@ def run(self) -> None:
                     # Only maps and sequences can preserve comments. Skip it.
                     continue
 
-            if self.write_list != ["none"]:
+            if self.write_set != {"none"}:
                 self._do_transforms(file, ruamel_data or data, file_is_yaml, matches)
 
             if file_is_yaml:
@@ -110,11 +121,11 @@ def _do_transforms(
         for match in sorted(matches):
             if not isinstance(match.rule, TransformMixin):
                 continue
-            if "all" not in self.write_list:
+            if self.write_set != {"all"}:
                 rule = cast(AnsibleLintRule, match.rule)
                 rule_definition = set(rule.tags)
                 rule_definition.add(rule.id)
-                if rule_definition.isdisjoint(set(self.write_list)):
+                if rule_definition.isdisjoint(self.write_set):
                     # rule transform not requested. Skip it.
                     continue
             if file_is_yaml and not match.yaml_path:

test/test_transformer.py

@@ -4,7 +4,7 @@
 import pathlib
 import shutil
 from argparse import Namespace
-from typing import Iterator, Tuple
+from typing import Iterator, List, Set, Tuple
 
 import pytest
 
@@ -100,3 +100,56 @@ def test_transformer(  # pylint: disable=too-many-arguments, too-many-locals
         assert orig_playbook_content == transformed_playbook_content
 
     assert transformed_playbook_content == expected_playbook_content
+
+
+@pytest.mark.parametrize(
+    ("write_list", "expected"),
+    (
+        # 1 item
+        (["all"], {"all"}),
+        (["none"], {"none"}),
+        (["rule-id"], {"rule-id"}),
+        # 2 items
+        (["all", "all"], {"all"}),
+        (["all", "none"], {"none"}),
+        (["all", "rule-id"], {"all"}),
+        (["none", "all"], {"all"}),
+        (["none", "none"], {"none"}),
+        (["none", "rule-id"], {"rule-id"}),
+        (["rule-id", "all"], {"all"}),
+        (["rule-id", "none"], {"none"}),
+        (["rule-id", "rule-id"], {"rule-id"}),
+        # 3 items
+        (["all", "all", "all"], {"all"}),
+        (["all", "all", "none"], {"none"}),
+        (["all", "all", "rule-id"], {"all"}),
+        (["all", "none", "all"], {"all"}),
+        (["all", "none", "none"], {"none"}),
+        (["all", "none", "rule-id"], {"rule-id"}),
+        (["all", "rule-id", "all"], {"all"}),
+        (["all", "rule-id", "none"], {"none"}),
+        (["all", "rule-id", "rule-id"], {"all"}),
+        (["none", "all", "all"], {"all"}),
+        (["none", "all", "none"], {"none"}),
+        (["none", "all", "rule-id"], {"all"}),
+        (["none", "none", "all"], {"all"}),
+        (["none", "none", "none"], {"none"}),
+        (["none", "none", "rule-id"], {"rule-id"}),
+        (["none", "rule-id", "all"], {"all"}),
+        (["none", "rule-id", "none"], {"none"}),
+        (["none", "rule-id", "rule-id"], {"rule-id"}),
+        (["rule-id", "all", "all"], {"all"}),
+        (["rule-id", "all", "none"], {"none"}),
+        (["rule-id", "all", "rule-id"], {"all"}),
+        (["rule-id", "none", "all"], {"all"}),
+        (["rule-id", "none", "none"], {"none"}),
+        (["rule-id", "none", "rule-id"], {"rule-id"}),
+        (["rule-id", "rule-id", "all"], {"all"}),
+        (["rule-id", "rule-id", "none"], {"none"}),
+        (["rule-id", "rule-id", "rule-id"], {"rule-id"}),
+    ),
+)
+def test_effective_write_list(write_list: List[str], expected: Set[str]) -> None:
+    """Make sure effective_write_list handles all/none keywords correctly."""
+    actual = Transformer.effective_write_set(write_list)
+    assert actual == expected