From b1a8a011dc089d9524c40c4341e0e7ec30aac033 Mon Sep 17 00:00:00 2001 From: mateo Date: Sat, 8 Aug 2026 19:30:12 +0000 Subject: [PATCH] fix(ci): read spread config mappings when detecting extra="allow" Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../ban_pydantic_extra_allow.py | 12 +++++++++++- .../test_ban_pydantic_extra_allow.py | 16 ++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/code_coverage_tests/ban_pydantic_extra_allow.py b/tests/code_coverage_tests/ban_pydantic_extra_allow.py index 37c9d1d76b4..b83722fcd7f 100644 --- a/tests/code_coverage_tests/ban_pydantic_extra_allow.py +++ b/tests/code_coverage_tests/ban_pydantic_extra_allow.py @@ -7,6 +7,10 @@ anything new must declare its fields. That list is a budget like the others, so ``scripts/budget_ratchet_check.py`` is what reds when it grows, and its ``limit`` must equal the number of models it lists. + +What it reads is the opt-in a module spells out, so it says nothing about a model that +inherits ``extra="allow"`` from a base declared elsewhere. Subclassing a listed model +still widens it, and only review catches that. """ import ast @@ -138,12 +142,18 @@ def _is_allow_literal(node: ast.expr, scope: Scope) -> bool: def _is_extra_allow_keyword(keyword: ast.keyword, scope: Scope) -> bool: + """A keyword with no ``arg`` is a ``**`` spread, so what it spreads is read as a config itself.""" + if keyword.arg is None: + return _is_extra_allow_value(keyword.value, scope) return keyword.arg == "extra" and _is_allow_literal(keyword.value, scope) def _mapping_sets_extra_allow(node: ast.Dict, scope: Scope) -> bool: + """A ``None`` key is a ``**`` spread, so what it spreads is read as a config itself.""" return any( - isinstance(key, ast.Constant) and key.value == "extra" and _is_allow_literal(value, scope) + _is_extra_allow_value(value, scope) + if key is None + else isinstance(key, ast.Constant) and key.value == "extra" and _is_allow_literal(value, scope) for key, value in zip(node.keys, node.values) ) diff --git a/tests/test_litellm/test_ban_pydantic_extra_allow.py b/tests/test_litellm/test_ban_pydantic_extra_allow.py index 1bfaa3c07de..3a843ad73a7 100644 --- a/tests/test_litellm/test_ban_pydantic_extra_allow.py +++ b/tests/test_litellm/test_ban_pydantic_extra_allow.py @@ -137,6 +137,18 @@ _ratchet_spec.loader.exec_module(ratchet) 'class Foo(BaseModel):\n class Config:\n _EXTRA = "allow"\n extra = _EXTRA\n', id="legacy_inner_config_reading_its_own_constant", ), + pytest.param( + 'class Foo(BaseModel):\n model_config = ConfigDict(**{"extra": "allow"})\n', + id="config_dict_kwargs_spread", + ), + pytest.param( + 'ALLOW = ConfigDict(extra="allow")\n\n\nclass Foo(BaseModel):\n model_config = {**ALLOW}\n', + id="permissive_constant_spread_into_a_dict", + ), + pytest.param( + 'ALLOW = {"extra": "allow"}\n\n\nclass Foo(BaseModel, **ALLOW):\n pass\n', + id="permissive_constant_spread_into_the_class_keywords", + ), ], ) def test_detects_extra_allow(source): @@ -203,6 +215,10 @@ def test_detects_extra_allow(source): 'class Foo(BaseModel):\n model_config = LATER\n LATER = ConfigDict(extra="allow")\n', id="class_local_constant_bound_below_the_line_reading_it", ), + pytest.param( + 'FORBID = ConfigDict(extra="forbid")\n\n\nclass Foo(BaseModel):\n model_config = {**FORBID}\n', + id="harmless_constant_spread_into_a_dict", + ), ], ) def test_ignores_non_violations(source):