diff --git a/AGENTS.md b/AGENTS.md index d0269b7f628..ce3bfa94a0d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -97,7 +97,7 @@ Follow these coding conventions for new/updated code (a three-line fix in a lega - Annotate every variable with `: Final` (LIT010). Unpacking and walrus targets cannot carry the annotation, so they are implicitly final. Don't rebind them. Never rebind or mutate function parameters (LIT011); `self`/`cls` attribute stores are the exception. If rebinding or in-place mutation is truly unavoidable, suppress with `# rebind-ok: ` - Qualify every TypedDict field with `ReadOnly[...]` (LIT012), which nests freely with `Required` / `NotRequired` / `Annotated` in any order. If making the key writable is truly unavoidable, suppress with `# writable-ok: ` - Comprehensions take at most one `for` clause and one `if` clause (LIT014); split stacked clauses into a helper generator, a named intermediate, or a plain loop. Suppress with `# comprehension-ok: ` only when unavoidable - - Every pydantic model must be frozen (LIT015), set via `model_config = ConfigDict(frozen=True)`, a dict-literal `model_config`, an inner `class Config`, or the class keywords. Subclasses inherit it unless they override it. If making the model mutable is truly unavoidable, suppress with `# frozen-ok: ` on the `class` line + - Every pydantic model must be frozen (LIT015), set via `model_config = ConfigDict(frozen=True)`, a dict-literal `model_config`, an inner `class Config`, or the class keywords. Subclasses inherit it unless they override it. Replace in-place field writes with `model_copy(update=...)`. If making the model mutable is truly unavoidable, suppress with `# frozen-ok: ` on the `class` line - Use dependency injection - Fully typed; no `Any` or coarse types like `dict[str, Any]` or just `dict`. Every function parameter must be strongly typed - Use tagged unions + match diff --git a/scripts/check_type_discipline.py b/scripts/check_type_discipline.py index 1d63e8f347b..d210c7849d8 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -92,13 +92,15 @@ LIT014 Comprehension with more than one `for` clause or more than one `if` clau marker belongs to the innermost violating comprehension spanning that line, and also to any single-line violating comprehension on that line. LIT015 Pydantic model class that is not frozen. Set `frozen=True` in - `model_config = ConfigDict(...)`, a dict-literal `model_config`, an inner - `class Config`, or the class keywords. Classes inherit the frozen setting - from in-module model bases, unless their own configuration overrides it. - Detection is name-based: `BaseModel`, `pydantic.BaseModel`, - `LiteLLMPydanticObjectBase`, and `RootModel` identify models, while - `TypedDict` classes are exempt. Suppress with `# frozen-ok: ` on - the `class` line. + `model_config = ConfigDict(...)`, `SettingsConfigDict(...)`, a dict-literal + `model_config`, an inner `class Config`, or the class keywords. Classes + inherit the frozen setting from in-module model bases, unless their own + configuration overrides it. Detection is name-based: `BaseModel`, + `pydantic.BaseModel`, `LiteLLMBaseModel`, `BaseLiteLLMOpenAIResponseObject`, + `LiteLLMPydanticObjectBase`, `RootModel`, and `BaseSettings` identify + models, while `TypedDict` classes are exempt. Replace in-place field writes + with `model_copy(update=...)`. Suppress with `# frozen-ok: ` on the + `class` line. LIT000 Setup failure: a target file could not be read, or contains a syntax error. Reported as a violation rather than crashing the run. @@ -158,7 +160,17 @@ READONLY_QUALIFIER = "ReadOnly" FIELD_QUALIFIER_WRAPPERS = frozenset(("Required", "NotRequired", "Annotated")) TYPEDDICT_BASE = "TypedDict" # Base names that mark a class as a pydantic model (LIT015). -PYDANTIC_BASES: Final = frozenset(("BaseModel", "LiteLLMPydanticObjectBase", "RootModel")) +PYDANTIC_BASES: Final = frozenset( + ( + "BaseModel", + "LiteLLMBaseModel", + "BaseLiteLLMOpenAIResponseObject", + "LiteLLMPydanticObjectBase", + "RootModel", + "BaseSettings", + ) +) +PYDANTIC_CONFIG_FACTORIES: Final = frozenset(("ConfigDict", "SettingsConfigDict")) MIN_REASON_LEN = 3 NOQA_RE = re.compile( @@ -868,7 +880,7 @@ def _bool_constant(value: ast.expr) -> bool | None: def _model_config_frozen(value: ast.expr) -> bool | None: - if isinstance(value, ast.Call) and _head_name(value.func) == "ConfigDict": + if isinstance(value, ast.Call) and _head_name(value.func) in PYDANTIC_CONFIG_FACTORIES: flags: Final = tuple(_bool_constant(kw.value) for kw in value.keywords if kw.arg == "frozen") return flags[-1] if flags else None if isinstance(value, ast.Dict): @@ -916,27 +928,28 @@ def _class_frozen_override(cls: ast.ClassDef) -> bool | None: def iter_pydantic_violations(path: Path, tree: ast.AST) -> Iterator[Violation]: models: Final = _pydantic_classes(tree) - bases_of: Final = {cls.name: _base_names(cls) for cls in models} - override_of: Final = {cls.name: _class_frozen_override(cls) for cls in models} + bases_of: Final = {cls: _base_names(cls) for cls in models} + override_of: Final = {cls: _class_frozen_override(cls) for cls in models} def frozen(known: frozenset[str]) -> frozenset[str]: grown: Final = known | frozenset( cls.name for cls in models - if override_of[cls.name] is True or (override_of[cls.name] is None and bases_of[cls.name] & known) + if override_of[cls] is True or (override_of[cls] is None and bases_of[cls] & known) ) return grown if grown == known else frozen(grown) frozen_names: Final = frozen(frozenset()) for cls in models: - if cls.name in frozen_names: + if override_of[cls] is True or (override_of[cls] is None and bases_of[cls] & frozen_names): continue yield Violation( path, cls.lineno, "LIT015", f"pydantic model `{cls.name}` is not frozen: set `frozen=True` in " - f"`model_config`, an inner `class Config`, or the class keywords " + f"`model_config`, an inner `class Config`, or the class keywords, and " + f"replace in-place field writes with `model_copy(update=...)` " f"(suppress: `# frozen-ok: `)", ) diff --git a/tests/unit/test_check_type_discipline.py b/tests/unit/test_check_type_discipline.py index bc7c564cbe0..9ea4508ebc3 100644 --- a/tests/unit/test_check_type_discipline.py +++ b/tests/unit/test_check_type_discipline.py @@ -735,6 +735,41 @@ def test_extra_allow_does_not_exempt(tmp_path): assert "LIT015" in _codes(tmp_path, src) +def test_litellm_base_model_is_flagged(tmp_path): + src = "class Foo(LiteLLMBaseModel):\n x: int\n" + assert "LIT015" in _codes(tmp_path, src) + + +def test_litellm_openai_response_base_is_flagged(tmp_path): + src = "class R(BaseLiteLLMOpenAIResponseObject):\n x: int\n" + assert "LIT015" in _codes(tmp_path, src) + + +def test_base_settings_is_flagged(tmp_path): + src = "class S(BaseSettings):\n x: int\n" + assert "LIT015" in _codes(tmp_path, src) + + +def test_settings_config_dict_frozen_true_is_clean(tmp_path): + src = ( + "from pydantic_settings import BaseSettings, SettingsConfigDict\n" + "class S(BaseSettings):\n" + " model_config = SettingsConfigDict(frozen=True)\n" + ) + assert "LIT015" not in _codes(tmp_path, src) + + +def test_same_named_models_use_each_classes_own_override(tmp_path): + src = ( + "from pydantic import BaseModel\n" + "class Dup(BaseModel, frozen=True):\n" + " x: int\n" + "class Dup(BaseModel):\n" + " x: int\n" + ) + assert _codes(tmp_path, src) == ["LIT015"] + + # --------------------------------------------------------------------------- # # Stacked comprehension clauses (LIT014) # --------------------------------------------------------------------------- # diff --git a/type-discipline-budget.json b/type-discipline-budget.json index 40752398386..47b303c41e8 100644 --- a/type-discipline-budget.json +++ b/type-discipline-budget.json @@ -39,6 +39,6 @@ "limit": 369 }, "LIT015": { - "limit": 230 + "limit": 1424 } }