Fix EG308 false positives and validate issue #87 against interview corpus

src/dayamlchecker/yaml_structure.py

@@ -1804,6 +1804,28 @@ def _find_variable_reference_lines(code: str, variable_expr: str) -> list[int]:
     return [i + 1 for i, line in enumerate(lines) if pattern.search(line)]
 
 
+def _python_reference_lines(code: str) -> dict[str, list[int]]:
+    """Index real Python variable reads, excluding comments and literal text."""
+    references: dict[str, list[int]] = {}
+    try:
+        tree = ast.parse(code)
+    except SyntaxError:
+        # Invalid Python is reported separately; do not interpret it as code.
+        return references
+    for node in ast.walk(tree):
+        if isinstance(node, (ast.Name, ast.Attribute, ast.Subscript)) and isinstance(
+            node.ctx, ast.Load
+        ):
+            references.setdefault(ast.unparse(node), []).append(node.lineno)
+        elif isinstance(node, ast.AugAssign):
+            # Augmented assignment reads its target before writing it, although
+            # Python marks that target with Store context.
+            references.setdefault(ast.unparse(node.target), []).append(
+                node.target.lineno
+            )
+    return references
+
+
 def _statement_span(stmts: list[ast.stmt]) -> Optional[tuple[int, int]]:
     if not stmts:
         return None
@@ -1967,14 +1989,19 @@ def _find_unmatched_interview_order_references(
         return []
 
     guards_by_line = _extract_branch_guards_by_line(code)
+    references = _python_reference_lines(code)
     unmatched: list[tuple[str, int]] = []
     seen_fields: set[str] = set()
     for conditional in conditional_fields:
         field_var = conditional["field_var"]
         if field_var in seen_fields:
             continue
         expected_guards = conditional["guards"]
-        for ref_line in _find_variable_reference_lines(code, field_var):
+        try:
+            reference_key = ast.unparse(ast.parse(field_var, mode="eval").body)
+        except SyntaxError:
+            continue
+        for ref_line in sorted(set(references.get(reference_key, []))):
             active_guards = guards_by_line.get(ref_line, [])
             if _has_showifdef_guard(active_guards, field_var):
                 continue

tests/test_issue_87.py

@@ -0,0 +1,92 @@
+"""Regression coverage for issue #87's conditional-field reports."""
+
+import textwrap
+
+import pytest
+
+from dayamlchecker.yaml_structure import find_errors_from_string
+
+
+@pytest.mark.parametrize("field", ["bar", "person.bar", "people[i].bar", "data['bar']"])
+@pytest.mark.parametrize(
+    "code, expected",
+    [
+        ("foo\n# {field}", False),
+        ('print("{field}")', False),
+        ('print("""literal\n{field}\ntext""")', False),
+        ("{field} = 1", False),
+        ("{field}", True),
+        ("print({field})", True),
+        ("{field} += 1", True),
+        ('print(f"{{{field}}}")', True),
+        ("if foo:\n    {field}", False),
+        ("# {field}\nif foo:\n    {field}\n{field}", True),
+    ],
+)
+def test_python_references(field, code, expected):
+    source = (
+        "question: Example\nfields:\n"
+        f"  - Value: {field}\n    show if: foo\n"
+        "---\nmandatory: True\ncode: |\n"
+        + textwrap.indent(code.format(field=field), "  ")
+        + "\n"
+    )
+    findings = find_errors_from_string(source)
+    assert any(f.code == "EG308" for f in findings) is expected
+
+
+@pytest.mark.parametrize("code", ["person.bar", "bar_extra", "other.person.bar"])
+def test_unrelated_python_reference(code):
+    source = (
+        "question: Example\nfields:\n  - Value: bar\n    show if: foo\n"
+        "---\nmandatory: True\ncode: |\n  " + code + "\n"
+    )
+    assert not any(f.code == "EG308" for f in find_errors_from_string(source))
+
+
+@pytest.mark.parametrize("condition", ["if: foo", "hide if: not foo", ""])
+@pytest.mark.parametrize("key", ["attachment", "attachments"])
+def test_unsupported_attachment_conditions_do_not_suppress_warning(condition, key):
+    # docassemble's process_attachment() does not consume these keys. They do
+    # not guarantee that the field is defined when the content is evaluated.
+    source = (
+        "question: Example\nfields:\n  - Value: bar\n    show if: foo\n"
+        f"---\n{key}:\n  - name: Example\n    filename: example\n"
+        f"    {condition}\n    content: |\n      ${{ bar }}\n"
+    )
+    assert any(f.code == "EG416" for f in find_errors_from_string(source))
+
+
+def test_python_reference_line_ignores_earlier_comment():
+    source = (
+        "question: Example\nfields:\n  - Value: bar\n    show if: foo\n"
+        "---\nmandatory: True\ncode: |\n  # bar\n  bar\n"
+    )
+    finding = next(f for f in find_errors_from_string(source) if f.code == "EG308")
+    assert finding.line_number == 9
+
+
+@pytest.mark.parametrize(
+    "code",
+    [
+        'print(data["bar"])',
+        "print(data [ 'bar' ])",
+        "print(data[\n    'bar'\n])",
+    ],
+)
+def test_subscript_reference_normalizes_quotes_and_whitespace(code):
+    source = (
+        "question: Example\nfields:\n  - Value: data['bar']\n    show if: foo\n"
+        "---\nmandatory: True\ncode: |\n" + textwrap.indent(code, "  ") + "\n"
+    )
+    assert any(f.code == "EG308" for f in find_errors_from_string(source))
+
+
+def test_invalid_python_still_reports_syntax_error():
+    source = (
+        "question: Example\nfields:\n  - Value: bar\n    show if: foo\n"
+        "---\nmandatory: True\ncode: |\n  if:\n    bar\n"
+    )
+    findings = find_errors_from_string(source)
+    assert any("syntax" in f.message.lower() for f in findings)
+    assert not any(f.code == "EG308" for f in findings)