Fix missing MatchErrors due to hash collisions (#4307)

Author: cavcrosby

examples/playbooks/vars/rule_var_naming_fails_files/bar.yml

@@ -0,0 +1,3 @@
+---
+CamelCaseIsBad: bar
+ALL_CAPS_ARE_BAD_TOO: bar

examples/playbooks/vars/rule_var_naming_fails_files/foo.yml

@@ -0,0 +1,3 @@
+---
+CamelCaseIsBad: foo
+ALL_CAPS_ARE_BAD_TOO: foo

src/ansiblelint/rules/var_naming.py

@@ -117,13 +117,15 @@ def get_var_naming_matcherror(
         ident: str,
         *,
         prefix: Prefix | None = None,
+        file: Lintable,
     ) -> MatchError | None:
         """Return a MatchError if the variable name is not valid, otherwise None."""
         if not isinstance(ident, str):  # pragma: no cover
             return MatchError(
                 tag="var-naming[non-string]",
                 message="Variables names must be strings.",
                 rule=self,
+                lintable=file,
             )
 
         if ident in ANNOTATION_KEYS or ident in self.allowed_special_names:
@@ -136,27 +138,31 @@ def get_var_naming_matcherror(
                 tag="var-naming[non-ascii]",
                 message=f"Variables names must be ASCII. ({ident})",
                 rule=self,
+                lintable=file,
             )
 
         if keyword.iskeyword(ident):
             return MatchError(
                 tag="var-naming[no-keyword]",
                 message=f"Variables names must not be Python keywords. ({ident})",
                 rule=self,
+                lintable=file,
             )
 
         if ident in self.reserved_names:
             return MatchError(
                 tag="var-naming[no-reserved]",
                 message=f"Variables names must not be Ansible reserved names. ({ident})",
                 rule=self,
+                lintable=file,
             )
 
         if ident in self.read_only_names:
             return MatchError(
                 tag="var-naming[read-only]",
                 message=f"This special variable is read-only. ({ident})",
                 rule=self,
+                lintable=file,
             )
 
         # We want to allow use of jinja2 templating for variable names
@@ -165,6 +171,7 @@ def get_var_naming_matcherror(
                 tag="var-naming[no-jinja]",
                 message="Variables names must not contain jinja2 templating.",
                 rule=self,
+                lintable=file,
             )
 
         if not bool(self.re_pattern.match(ident)) and (
@@ -174,6 +181,7 @@ def get_var_naming_matcherror(
                 tag="var-naming[pattern]",
                 message=f"Variables names should match {self.re_pattern_str} regex. ({ident})",
                 rule=self,
+                lintable=file,
             )
 
         if (
@@ -186,6 +194,7 @@ def get_var_naming_matcherror(
                 tag="var-naming[no-role-prefix]",
                 message=f"Variables names from within roles should use {prefix.value}_ as a prefix.",
                 rule=self,
+                lintable=file,
             )
         return None
 
@@ -199,9 +208,8 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
         # If the Play uses the 'vars' section to set variables
         our_vars = data.get("vars", {})
         for key in our_vars:
-            match_error = self.get_var_naming_matcherror(key)
+            match_error = self.get_var_naming_matcherror(key, file=file)
             if match_error:
-                match_error.filename = str(file.path)
                 match_error.lineno = (
                     key.ansible_pos[1]
                     if isinstance(key, AnsibleUnicode)
@@ -219,9 +227,9 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
                     match_error = self.get_var_naming_matcherror(
                         key,
                         prefix=prefix,
+                        file=file,
                     )
                     if match_error:
-                        match_error.filename = str(file.path)
                         match_error.message += f" (vars: {key})"
                         match_error.lineno = (
                             key.ansible_pos[1]
@@ -235,9 +243,9 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]:
                 match_error = self.get_var_naming_matcherror(
                     key,
                     prefix=prefix,
+                    file=file,
                 )
                 if match_error:
-                    match_error.filename = str(file.path)
                     match_error.message += f" (vars: {key})"
                     match_error.lineno = (
                         key.ansible_pos[1]
@@ -266,7 +274,6 @@ def matchtask(
         """Return matches for task based variables."""
         results = []
         prefix = Prefix()
-        filename = "" if file is None else str(file.path)
         if file and file.parent and file.parent.kind == "role":
             prefix = Prefix(file.parent.path.name)
         ansible_module = task["action"]["__ansible_module__"]
@@ -283,9 +290,9 @@ def matchtask(
             match_error = self.get_var_naming_matcherror(
                 key,
                 prefix=prefix,
+                file=file or Lintable(""),
             )
             if match_error:
-                match_error.filename = filename
                 match_error.lineno = our_vars[LINE_NUMBER_KEY]
                 match_error.message += f" (vars: {key})"
                 results.append(match_error)
@@ -298,9 +305,12 @@ def matchtask(
                 and x != "cacheable",
                 task["action"].keys(),
             ):
-                match_error = self.get_var_naming_matcherror(key, prefix=prefix)
+                match_error = self.get_var_naming_matcherror(
+                    key,
+                    prefix=prefix,
+                    file=file or Lintable(""),
+                )
                 if match_error:
-                    match_error.filename = filename
                     match_error.lineno = task["action"][LINE_NUMBER_KEY]
                     match_error.message += f" (set_fact: {key})"
                     results.append(match_error)
@@ -311,10 +321,10 @@ def matchtask(
             match_error = self.get_var_naming_matcherror(
                 registered_var,
                 prefix=prefix,
+                file=file or Lintable(""),
             )
             if match_error:
                 match_error.message += f" (register: {registered_var})"
-                match_error.filename = filename
                 match_error.lineno = task[LINE_NUMBER_KEY]
                 results.append(match_error)
 
@@ -325,15 +335,17 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
         results: list[MatchError] = []
         raw_results: list[MatchError] = []
         meta_data: dict[AnsibleUnicode, Any] = {}
-        filename = "" if file is None else str(file.path)
 
         if str(file.kind) == "vars" and file.data:
             meta_data = parse_yaml_from_file(str(file.path))
             for key in meta_data:
                 prefix = Prefix(file.role) if file.role else Prefix()
-                match_error = self.get_var_naming_matcherror(key, prefix=prefix)
+                match_error = self.get_var_naming_matcherror(
+                    key,
+                    prefix=prefix,
+                    file=file,
+                )
                 if match_error:
-                    match_error.filename = filename
                     match_error.lineno = key.ansible_pos[1]
                     match_error.message += f" (vars: {key})"
                     raw_results.append(match_error)
@@ -413,6 +425,25 @@ def test_invalid_var_name_varsfile(
             assert result.tag == expected_errors[idx][0]
             assert result.lineno == expected_errors[idx][1]
 
+    def test_invalid_vars_diff_files(
+        default_rules_collection: RulesCollection,
+    ) -> None:
+        """Test rule matches."""
+        results = Runner(
+            Lintable("examples/playbooks/vars/rule_var_naming_fails_files"),
+            rules=default_rules_collection,
+        ).run()
+        expected_errors = (
+            ("var-naming[pattern]", 2),
+            ("var-naming[pattern]", 3),
+            ("var-naming[pattern]", 2),
+            ("var-naming[pattern]", 3),
+        )
+        assert len(results) == len(expected_errors)
+        for idx, result in enumerate(results):
+            assert result.tag == expected_errors[idx][0]
+            assert result.lineno == expected_errors[idx][1]
+
     def test_var_naming_with_role_prefix(
         default_rules_collection: RulesCollection,
     ) -> None:

src/ansiblelint/utils.py

@@ -358,6 +358,7 @@ def taskshandlers_children(
             raise MatchError(
                 message="A malformed block was encountered while loading a block.",
                 rule=RuntimeErrorRule(),
+                lintable=lintable,
             )
         for task_handler in v:
             # ignore empty tasks, `-`
@@ -408,23 +409,21 @@ def _validate_task_handler_action_for_role(self, th_action: dict[str, Any]) -> N
         """Verify that the task handler action is valid for role include."""
         module = th_action["__ansible_module__"]
 
+        lintable = Lintable(
+            self.rules.options.lintables[0] if self.rules.options.lintables else ".",
+        )
         if "name" not in th_action:
             raise MatchError(
                 message=f"Failed to find required 'name' key in {module!s}",
                 rule=self.rules.rules[0],
-                lintable=Lintable(
-                    (
-                        self.rules.options.lintables[0]
-                        if self.rules.options.lintables
-                        else "."
-                    ),
-                ),
+                lintable=lintable,
             )
 
         if not isinstance(th_action["name"], str):
             raise MatchError(
                 message=f"Value assigned to 'name' key on '{module!s}' is not a string.",
                 rule=self.rules.rules[1],
+                lintable=lintable,
             )
 
     def roles_children(

tox.ini

@@ -74,7 +74,7 @@ setenv =
   PRE_COMMIT_COLOR = always
   # Number of expected test passes, safety measure for accidental skip of
   # tests. Update value if you add/remove tests. (tox-extra)
-  PYTEST_REQPASS = 893
+  PYTEST_REQPASS = 894
   FORCE_COLOR = 1
   pre: PIP_PRE = 1
 allowlist_externals =