From a10a8da6d1acff17e510ce5f92ae376c84cd2b7a Mon Sep 17 00:00:00 2001 From: Quinten Steenhuis Date: Mon, 21 Sep 2026 12:30:35 -0400 Subject: [PATCH] Fix EG308 false positives with Python AST reference analysis --- src/dayamlchecker/yaml_structure.py | 29 ++++++++- tests/test_issue_87.py | 92 +++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 1 deletion(-) create mode 100644 tests/test_issue_87.py diff --git a/src/dayamlchecker/yaml_structure.py b/src/dayamlchecker/yaml_structure.py index 8b818eb..042843f 100644 --- a/src/dayamlchecker/yaml_structure.py +++ b/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,6 +1989,7 @@ 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: @@ -1974,7 +1997,11 @@ def _find_unmatched_interview_order_references( 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 diff --git a/tests/test_issue_87.py b/tests/test_issue_87.py new file mode 100644 index 0000000..08f223f --- /dev/null +++ b/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)