Fix identification of inline noqa skips (#2300)

Fixes: #2271

.ansible-lint

@@ -39,7 +39,6 @@ use_default_rules: true
 # This makes linter to fully ignore rules/tags listed below
 skip_list:
   - skip_this_tag
-  - latest[git]
 
 # Any rule that has the 'opt-in' tag will not be loaded unless its 'id' is
 # mentioned in the enable_list:
@@ -57,7 +56,6 @@ enable_list:
 # This makes the linter display but not fail for rules/tags listed below:
 warn_list:
   - skip_this_tag
-  - latest[git]
   - experimental # experimental is included in the implicit list
   # - role-name
   # - yaml[document-start]  # you can also use sub-rule matches

examples/playbooks/test_skip_inside_yaml.yml

@@ -2,9 +2,9 @@
 - name: Fixture
   hosts: all
   tags:
-    - skip_ansible_lint
+    - skip_ansible_lint # should disable error at playbook level
   tasks:
-    - name: Test latest[hg]
+    - name: Test # <-- 0 latest[hg]
       action: ansible.builtin.hg
     - name: Test latest[hg] (skipped) # noqa latest[hg]
       action: ansible.builtin.hg
@@ -16,20 +16,20 @@
       become_user: alice
       action: ansible.builtin.git
 
-    - name: Test YAML and jinja[spacing]
+    - name: Test YAML # <-- 1 jinja[spacing]
       ansible.builtin.get_url:
         # noqa: risky-file-permissions
-        url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf
+        url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf # <-- 2 yaml[line-length]
         dest: "{{dest_proj_path}}/foo.conf"
     - name: Test YAML and jinja[spacing] (skipped)
       ansible.builtin.get_url:
         # noqa: risky-file-permissions
-        url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf # noqa yaml
+        url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf # noqa yaml[line-length]
         dest: "{{dest_proj_path}}/foo.conf" # noqa jinja[spacing]
 
-    - name: Test deprecated-command-syntax
+    - name: Test deprecated-command-syntax # <-- 3 deprecated-command-syntax
       ansible.builtin.command: creates=B chmod 644 A
-    - name: Test deprecated-command-syntax
+    - name: Test deprecated-command-syntax # <-- 4 deprecated-command-syntax
       ansible.builtin.command: warn=yes creates=B chmod 644 A
     - name: Test deprecated-command-syntax (skipped via no warn)
       ansible.builtin.command: warn=no creates=B chmod 644 A

src/ansiblelint/file_utils.py

@@ -8,7 +8,7 @@
 import subprocess
 import sys
 from argparse import Namespace
-from collections import OrderedDict
+from collections import OrderedDict, defaultdict
 from contextlib import contextmanager
 from pathlib import Path
 from tempfile import NamedTemporaryFile
@@ -148,6 +148,7 @@ def __init__(
         self.kind: FileType | None = None
         self.stop_processing = False  # Set to stop other rules from running
         self._data: Any = None
+        self.line_skips: dict[int, set[str]] = defaultdict(set)
 
         if isinstance(name, str):
             self.name = normpath(name)

src/ansiblelint/rules/__init__.py

@@ -166,7 +166,11 @@ def matchtasks(self, file: Lintable) -> list[MatchError]:
                 # normalize_task converts AnsibleParserError to MatchError
                 return [error]
 
-            if self.id in skipped_tags or ("action" not in task):
+            if (
+                self.id in skipped_tags
+                or ("action" not in task)
+                or "skip_ansible_lint" in task.get("tags", [])
+            ):
                 continue
 
             if self.needs_raw_task:
@@ -223,6 +227,9 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
             if self.id in play.get("skipped_rules", ()):
                 continue
 
+            if "skip_ansible_lint" in play.get("tags", []):
+                continue
+
             matches.extend(self.matchplay(file, play))
 
         return matches

src/ansiblelint/rules/yaml_rule.py

@@ -3,16 +3,18 @@
 
 import logging
 import sys
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Iterable
 
 from yamllint.linter import run as run_yamllint
 
 from ansiblelint.file_utils import Lintable
 from ansiblelint.rules import AnsibleLintRule
-from ansiblelint.skip_utils import get_rule_skips_from_line
+from ansiblelint.utils import LINE_NUMBER_KEY
 from ansiblelint.yaml_utils import load_yamllint_config
 
 if TYPE_CHECKING:
+    from typing import Any, Generator
+
     from ansiblelint.errors import MatchError
 
 _logger = logging.getLogger(__name__)
@@ -56,17 +58,55 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
                 )
             )
 
-        if matches:
-            lines = file.content.splitlines()
-            for match in matches:
-                # rule.linenumber starts with 1, not zero
-                skip_list = get_rule_skips_from_line(lines[match.linenumber - 1])
-                # print(skip_list)
-                if match.rule.id not in skip_list and match.tag not in skip_list:
-                    filtered_matches.append(match)
+        # Now we save inside the file the skips, so they can be removed later,
+        # especially as these skips can be about other rules than yaml one.
+        _fetch_skips(file.data, file.line_skips)
+
+        for match in matches:
+            last_skips = set()
+
+            for line, skips in file.line_skips.items():
+                if line > match.linenumber:
+                    break
+                last_skips = skips
+            if last_skips.intersection({"skip_ansible_lint", match.rule.id, match.tag}):
+                continue
+            filtered_matches.append(match)
+
         return filtered_matches
 
 
+def _combine_skip_rules(data: Any) -> set[str]:
+    """Return a consolidated list of skipped rules."""
+    result = set(data.get("skipped_rules", []))
+    if "skip_ansible_lint" in data.get("tags", []):
+        result.add("skip_ansible_lint")
+    return result
+
+
+def _fetch_skips(data: Any, collector: dict[int, set[str]]) -> dict[int, set[str]]:
+    """Retrieve a dictionary with line: skips by looking recursively in given JSON structure."""
+    if hasattr(data, "get") and data.get(LINE_NUMBER_KEY):
+        rules = _combine_skip_rules(data)
+        if rules:
+            collector[data.get(LINE_NUMBER_KEY)].update(rules)
+    if isinstance(data, Iterable) and not isinstance(data, str):
+        if isinstance(data, dict):
+            for entry, value in data.items():
+                _fetch_skips(value, collector)
+        else:
+            for entry in data:
+                if (
+                    entry
+                    and LINE_NUMBER_KEY in entry
+                    and "skipped_rules" in entry
+                    and entry["skipped_rules"]
+                ):
+                    collector[entry[LINE_NUMBER_KEY]].update(entry["skipped_rules"])
+                _fetch_skips(entry, collector)
+    return collector
+
+
 # testing code to be loaded only with pytest or when executed the rule file
 if "pytest" in sys.modules:
     import pytest

src/ansiblelint/yaml_utils.py

@@ -136,7 +136,7 @@ def iter_tasks_in_file(
             yield raw_task, raw_task, skip_tags, err
             return
 
-        if "skip_ansible_lint" in (raw_task.get("tags") or []):
+        if "skip_ansible_lint" in raw_task.get("tags", []):
             skip_tags.append("skip_ansible_lint")
         if skip_tags:
             yield raw_task, normalized_task, skip_tags, err

test/test_skip_inside_yaml.py

@@ -1,4 +1,6 @@
 """Tests related to use of inline noqa."""
+import pytest
+
 from ansiblelint.rules import RulesCollection
 from ansiblelint.runner import Runner
 from ansiblelint.testing import RunFromText, run_ansible_lint
@@ -51,13 +53,17 @@ def test_role_tasks_with_block(default_text_runner: RunFromText) -> None:
     assert len(results) == 4
 
 
-def test_skip_playbook(default_rules_collection: RulesCollection) -> None:
+@pytest.mark.parametrize(
+    ("lintable", "expected"),
+    (pytest.param("examples/playbooks/test_skip_inside_yaml.yml", 6, id="yaml"),),
+)
+def test_inline_skips(
+    default_rules_collection: RulesCollection, lintable: str, expected: int
+) -> None:
     """Check that playbooks can contain skips."""
-    results = Runner(
-        "examples/playbooks/test_skip_inside_yaml.yml", rules=default_rules_collection
-    ).run()
+    results = Runner(lintable, rules=default_rules_collection).run()
 
-    assert len(results) == 8
+    assert len(results) == expected
 
 
 def test_role_meta() -> None: