From a5052816e99f711aaad513c4ca0c0f87132bb2cd Mon Sep 17 00:00:00 2001 From: mateo Date: Sat, 8 Aug 2026 19:38:57 +0000 Subject: [PATCH] fix(ci): keep a class-local name from hiding an opt-in read above it Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ban_pydantic_extra_allow.py | 14 +++++++------- .../test_litellm/test_ban_pydantic_extra_allow.py | 5 +++++ 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/tests/code_coverage_tests/ban_pydantic_extra_allow.py b/tests/code_coverage_tests/ban_pydantic_extra_allow.py index b83722fcd7f..e277e112d0d 100644 --- a/tests/code_coverage_tests/ban_pydantic_extra_allow.py +++ b/tests/code_coverage_tests/ban_pydantic_extra_allow.py @@ -71,7 +71,7 @@ def _shadows(later: Binding, earlier: Binding) -> bool: class Scope(NamedTuple): - """Module-level bindings, read as of ``line``. + """The bindings in view, read as of ``line``. Resolution walks a name to the values it could hold just above the line reading it, and each hop down a chain of aliases carries that alias's own line, so rebinding a name later can @@ -108,7 +108,7 @@ def _scoped_statements(node: ast.AST, branch: tuple[str, ...] = ()) -> Iterator[ for child in value if isinstance(value, list) else [value]: if not isinstance(child, ast.AST) or isinstance(child, ast.expr): continue - nested = branch if isinstance(node, ast.Module) else (*branch, f"{id(node)}.{field}") + nested = branch if isinstance(node, (ast.Module, ast.ClassDef)) else (*branch, f"{id(node)}.{field}") if isinstance(child, ast.ClassDef): yield child, nested continue @@ -177,11 +177,11 @@ def _assigns_extra_allow(statement: ast.stmt, target_names: Sequence[str], scope def _body_scope(node: ast.AST, enclosing: tuple[Binding, ...]) -> tuple[Binding, ...]: - """The bindings a statement in ``node``'s body reads, so a class-local constant is resolved - and shadows a module-level name of its own, the way the class body itself would read it.""" - local: Final = _bindings(node) - shadowed: Final = frozenset(binding.name for binding in local) - return tuple(binding for binding in enclosing if binding.name not in shadowed) + local + """The bindings a statement in ``node``'s body reads. A class-local binding joins the enclosing + ones rather than replacing them, so which one wins is decided per read line the way the body + itself decides it: a class-local constant only shadows an enclosing name below the line it is + bound on, and a read above that line still sees the enclosing value.""" + return enclosing + _bindings(node) def _legacy_config_sets_extra_allow(config: ast.ClassDef, enclosing: tuple[Binding, ...]) -> bool: diff --git a/tests/test_litellm/test_ban_pydantic_extra_allow.py b/tests/test_litellm/test_ban_pydantic_extra_allow.py index 3a843ad73a7..f79fc01db1b 100644 --- a/tests/test_litellm/test_ban_pydantic_extra_allow.py +++ b/tests/test_litellm/test_ban_pydantic_extra_allow.py @@ -149,6 +149,11 @@ _ratchet_spec.loader.exec_module(ratchet) 'ALLOW = {"extra": "allow"}\n\n\nclass Foo(BaseModel, **ALLOW):\n pass\n', id="permissive_constant_spread_into_the_class_keywords", ), + pytest.param( + 'ALLOW = ConfigDict(extra="allow")\n\n\nclass Foo(BaseModel):\n' + ' model_config = ALLOW\n ALLOW = ConfigDict(extra="forbid")\n', + id="module_constant_read_above_a_class_local_of_the_same_name", + ), ], ) def test_detects_extra_allow(source):