diff --git a/AGENTS.md b/AGENTS.md index cade08bdd02..b5652770bdd 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,6 +96,7 @@ Follow these coding conventions for new/updated code (a three-line fix in a lega - No mutation; don't reassign variables, global or local. Instead of mutable lists and dicts, prefer tuples, frozen dataclasses (with slots=True), `MappingProxyType`, etc. - 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: ` + - Every pydantic model must be `frozen=True` (LIT013), set via `model_config = ConfigDict(frozen=True)` which subclasses inherit. 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 a2ab4760c4f..359ace379ef 100644 --- a/scripts/check_type_discipline.py +++ b/scripts/check_type_discipline.py @@ -40,7 +40,7 @@ LIT003 noqa suppression without rule codes or without a reason. LIT004 pyright/mypy ignore without bracketed codes or without a reason. Required shape: `# pyright: ignore[reportArgumentType] # ` LIT005 A `# mutable-ok` / `# cast-ok` / `# guard-ok` / `# kwargs-ok` / - `# rebind-ok` / `# writable-ok` suppression without a reason. + `# rebind-ok` / `# writable-ok` / `# frozen-ok` suppression without a reason. LIT006 `cast(...)` call. typing.cast is an unchecked assertion (the moral equivalent of TypeScript's `as`); it lies to the type checker with zero runtime guarantee. Validate into a concrete frozen type at the boundary instead. @@ -99,6 +99,17 @@ LIT012 TypedDict field without a `ReadOnly[...]` qualifier. A writable key lets the functional form (`X = TypedDict("X", {...})`) is checked too. A base imported from another module is out of reach without import resolution. Suppress with `# writable-ok: `. +LIT013 Pydantic model class that is not frozen. A writable model lets any holder + rewrite its fields after validation; set `model_config = ConfigDict(frozen=True)` + (or `frozen = True` in an inner `class Config`), which subclasses inherit, or + a dict-literal `model_config = {"frozen": True, ...}`. Detection is name-based, + like the TypedDict check: a class is a pydantic model when `BaseModel`, + `pydantic.BaseModel`, `LiteLLMPydanticObjectBase`, or `RootModel` is among its + bases, or when it inherits, transitively within the same module, from a class + already determined to be one; a base defined in another module is out of reach. + Classes whose bases include `TypedDict` are not pydantic models and are exempt. + A body that sets `frozen=False` explicitly is a violation. 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. @@ -164,6 +175,11 @@ READONLY_QUALIFIER = "ReadOnly" # first argument is type syntax, the rest is metadata and never qualifies the field. FIELD_QUALIFIER_WRAPPERS = frozenset(("Required", "NotRequired", "Annotated")) TYPEDDICT_BASE = "TypedDict" +# Base names that mark a class as a pydantic model (LIT013). Dotted access resolves +# to the same head, so `pydantic.BaseModel` is covered by `BaseModel`; subclasses +# join transitively within the same module, and an in-file frozen ancestor makes +# the subclass frozen too (pydantic v2 inherits model_config). +PYDANTIC_BASES = frozenset(("BaseModel", "LiteLLMPydanticObjectBase", "RootModel")) MIN_REASON_LEN = 3 NOQA_RE = re.compile( @@ -182,6 +198,7 @@ GUARD_OK_RE = re.compile(r"#\s*guard-ok(?::\s*(?P.*))?") KWARGS_OK_RE = re.compile(r"#\s*kwargs-ok(?::\s*(?P.*))?") REBIND_OK_RE = re.compile(r"#\s*rebind-ok(?::\s*(?P.*))?") WRITABLE_OK_RE = re.compile(r"#\s*writable-ok(?::\s*(?P.*))?") +FROZEN_OK_RE = re.compile(r"#\s*frozen-ok(?::\s*(?P.*))?") # Suppression tokens that must each carry a reason (LIT005). OK_SUPPRESSIONS: tuple[tuple[str, re.Pattern[str]], ...] = ( @@ -191,6 +208,7 @@ OK_SUPPRESSIONS: tuple[tuple[str, re.Pattern[str]], ...] = ( ("kwargs-ok", KWARGS_OK_RE), ("rebind-ok", REBIND_OK_RE), ("writable-ok", WRITABLE_OK_RE), + ("frozen-ok", FROZEN_OK_RE), ) @@ -214,6 +232,7 @@ class Comments: kwargs_ok_lines: frozenset[int] rebind_ok_lines: frozenset[int] writable_ok_lines: frozenset[int] + frozen_ok_lines: frozenset[int] # --------------------------------------------------------------------------- # @@ -269,7 +288,7 @@ def scan_comments(path: Path, source: str) -> tuple[Comments, tuple[Violation, . # tokenize raises TokenError (EOF mid-construct) or a SyntaxError subclass # (IndentationError / TabError) on malformed source; defer to ast.parse below, # which re-raises and is reported as LIT000 rather than crashing the run. - return Comments(frozenset(), frozenset(), frozenset(), frozenset(), frozenset(), frozenset()), () + return Comments(*(frozenset() for _ in Comments.__dataclass_fields__)), () def _lines_with(regex: re.Pattern[str]) -> frozenset[int]: return frozenset(line for line, text in comment_toks if _valid_ok(regex, text)) @@ -282,6 +301,7 @@ def scan_comments(path: Path, source: str) -> tuple[Comments, tuple[Violation, . kwargs_ok_lines=_lines_with(KWARGS_OK_RE), rebind_ok_lines=_lines_with(REBIND_OK_RE), writable_ok_lines=_lines_with(WRITABLE_OK_RE), + frozen_ok_lines=_lines_with(FROZEN_OK_RE), ), tuple(v for line, text in comment_toks for v in _comment_violations(path, line, text)), ) @@ -1033,6 +1053,98 @@ def iter_typeddict_violations(path: Path, tree: ast.AST, comments: Comments) -> ) +# --------------------------------------------------------------------------- # +# Unfrozen pydantic models (LIT013) +# --------------------------------------------------------------------------- # + + +def _pydantic_classes(tree: ast.AST) -> tuple[ast.ClassDef, ...]: + """ClassDefs that are pydantic models: a PYDANTIC_BASES name among the bases, or + -- transitively, within this module -- a base that is itself one of these classes. + TypedDict classes are excluded: their bases name a form that is not pydantic.""" + classes = tuple(node for node in ast.walk(tree) if isinstance(node, ast.ClassDef)) + bases_of = {cls.name: _base_names(cls) for cls in classes} + + def expand(known: frozenset[str]) -> frozenset[str]: + grown = known | frozenset(name for name, bases in bases_of.items() if bases & known) + return grown if grown == known else expand(grown) + + names = expand(PYDANTIC_BASES) - expand(frozenset((TYPEDDICT_BASE,))) + return tuple(cls for cls in classes if cls.name in names) + + +def _model_config_is_frozen(value: ast.expr) -> bool: + """`ConfigDict(frozen=True)` in any kwarg position, or a dict literal carrying + `"frozen": True`.""" + if isinstance(value, ast.Call) and _head_name(value.func) == "ConfigDict": + return any( + kw.arg == "frozen" + and isinstance(kw.value, ast.Constant) + and kw.value.value is True + for kw in value.keywords + ) + if isinstance(value, ast.Dict): + return any( + isinstance(key, ast.Constant) + and key.value == "frozen" + and isinstance(item, ast.Constant) + and item.value is True + for key, item in zip(value.keys, value.values) + ) + return False + + +def _assigns_name(stmt: ast.stmt, name: str) -> ast.expr | None: + """The value a simple `name = ...` / `name: T = ...` body statement binds.""" + value = stmt.value if isinstance(stmt, (ast.Assign, ast.AnnAssign)) else None + targets = stmt.targets if isinstance(stmt, ast.Assign) else (stmt.target,) if isinstance(stmt, ast.AnnAssign) else () + if value is not None and any(isinstance(t, ast.Name) and t.id == name for t in targets): + return value + return None + + +def _config_class_is_frozen(node: ast.ClassDef) -> bool: + """An inner `class Config:` counts only when it binds `frozen = True`.""" + return any( + isinstance(value, ast.Constant) and value.value is True + for stmt in node.body + for value in (_assigns_name(stmt, "frozen"),) + if value is not None + ) + + +def _class_is_frozen(cls: ast.ClassDef) -> bool: + for stmt in cls.body: + config_value = _assigns_name(stmt, "model_config") + if config_value is not None and _model_config_is_frozen(config_value): + return True + if isinstance(stmt, ast.ClassDef) and stmt.name == "Config" and _config_class_is_frozen(stmt): + return True + return False + + +def iter_pydantic_violations(path: Path, tree: ast.AST, comments: Comments) -> Iterator[Violation]: + models = _pydantic_classes(tree) + bases_of = {cls.name: _base_names(cls) for cls in models} + + def frozen(known: frozenset[str]) -> frozenset[str]: + grown = known | frozenset( + cls.name for cls in models if _class_is_frozen(cls) or bases_of[cls.name] & known + ) + return grown if grown == known else frozen(grown) + + frozen_names = frozen(frozenset()) + for cls in models: + if cls.name in frozen_names or cls.lineno in comments.frozen_ok_lines: + continue + yield Violation( + path, cls.lineno, "LIT013", + f"pydantic model `{cls.name}` is not frozen: any holder can rewrite its " + f"fields after validation. Set `model_config = ConfigDict(frozen=True)` " + f"(inherited by subclasses) (suppress: `# frozen-ok: `)", + ) + + # --------------------------------------------------------------------------- # # Driver # --------------------------------------------------------------------------- # @@ -1060,6 +1172,7 @@ def check_file(path: Path) -> tuple[Violation, ...]: *iter_final_violations(path, tree, comments), *iter_param_violations(path, tree, comments), *iter_typeddict_violations(path, tree, comments), + *iter_pydantic_violations(path, tree, comments), ) diff --git a/tests/test_litellm/test_check_type_discipline.py b/tests/test_litellm/test_check_type_discipline.py index 2d49332e687..35fc807234b 100644 --- a/tests/test_litellm/test_check_type_discipline.py +++ b/tests/test_litellm/test_check_type_discipline.py @@ -687,6 +687,130 @@ def test_writable_ok_without_reason_is_lit005_and_does_not_suppress(tmp_path): assert "LIT012" in codes +# --------------------------------------------------------------------------- # +# Unfrozen pydantic models (LIT013) +# --------------------------------------------------------------------------- # + + +def test_unfrozen_basemodel_is_flagged(tmp_path): + src = "from pydantic import BaseModel\nclass P(BaseModel):\n a: int\n" + assert "LIT013" in _codes(tmp_path, src) + + +def test_configdict_frozen_true_is_clean(tmp_path): + src = ( + "from pydantic import BaseModel, ConfigDict\n" + "class P(BaseModel):\n" + " model_config = ConfigDict(extra='allow', frozen=True)\n" + ) + assert "LIT013" not in _codes(tmp_path, src) + + +def test_dict_literal_model_config_frozen_true_is_clean(tmp_path): + src = ( + "from pydantic import BaseModel\n" + "class P(BaseModel):\n" + " model_config = {'frozen': True, 'extra': 'allow'}\n" + ) + assert "LIT013" not in _codes(tmp_path, src) + + +def test_subclass_of_in_file_frozen_model_is_clean(tmp_path): + src = ( + "from pydantic import BaseModel, ConfigDict\n" + "class Base(BaseModel):\n" + " model_config = ConfigDict(frozen=True)\n" + "class Child(Base):\n" + " a: int\n" + ) + assert "LIT013" not in _codes(tmp_path, src) + + +def test_subclass_of_in_file_unfrozen_model_flags_both(tmp_path): + src = ( + "from pydantic import BaseModel\n" + "class Base(BaseModel):\n" + " pass\n" + "class Child(Base):\n" + " a: int\n" + ) + assert _codes(tmp_path, src).count("LIT013") == 2 + + +def test_litellm_pydantic_object_base_without_frozen_is_flagged(tmp_path): + src = "class P(LiteLLMPydanticObjectBase):\n a: int\n" + assert "LIT013" in _codes(tmp_path, src) + + +def test_inner_config_class_frozen_true_is_clean(tmp_path): + src = ( + "from pydantic import BaseModel\n" + "class P(BaseModel):\n" + " class Config:\n" + " frozen = True\n" + ) + assert "LIT013" not in _codes(tmp_path, src) + + +def test_frozen_false_is_flagged(tmp_path): + src = ( + "from pydantic import BaseModel, ConfigDict\n" + "class P(BaseModel):\n" + " model_config = ConfigDict(frozen=False)\n" + ) + assert "LIT013" in _codes(tmp_path, src) + + +def test_root_model_without_frozen_is_flagged(tmp_path): + src = "from pydantic import RootModel\nclass P(RootModel):\n root: int\n" + assert "LIT013" in _codes(tmp_path, src) + + +def test_qualified_pydantic_basemodel_is_flagged(tmp_path): + src = "import pydantic\nclass P(pydantic.BaseModel):\n a: int\n" + assert "LIT013" in _codes(tmp_path, src) + + +def test_frozen_ok_with_reason_suppresses_lit013(tmp_path): + src = ( + "from pydantic import BaseModel\n" + "class P(BaseModel): # frozen-ok: mutated during build before handoff\n" + " a: int\n" + ) + assert "LIT013" not in _codes(tmp_path, src) + + +def test_frozen_ok_without_reason_is_lit005_and_does_not_suppress(tmp_path): + src = ( + "from pydantic import BaseModel\n" + "class P(BaseModel): # frozen-ok\n" + " a: int\n" + ) + codes = _codes(tmp_path, src) + assert "LIT005" in codes + assert "LIT013" in codes + + +def test_typeddict_and_plain_classes_are_not_models(tmp_path): + src = ( + "from typing import TypedDict\n" + "class T(TypedDict):\n" + " a: int\n" + "class C:\n" + " a: int\n" + ) + assert "LIT013" not in _codes(tmp_path, src) + + +def test_extra_allow_does_not_exempt(tmp_path): + src = ( + "from pydantic import BaseModel, ConfigDict\n" + "class P(BaseModel):\n" + " model_config = ConfigDict(extra='allow')\n" + ) + assert "LIT013" in _codes(tmp_path, src) + + # --------------------------------------------------------------------------- # # Budget integrity: every emittable LIT rule (bar the LIT000 read/parse error) is gated # --------------------------------------------------------------------------- # diff --git a/type-discipline-budget.json b/type-discipline-budget.json index 0c0952289e2..cc1950dd16c 100644 --- a/type-discipline-budget.json +++ b/type-discipline-budget.json @@ -34,5 +34,8 @@ }, "LIT012": { "limit": 4486 + }, + "LIT013": { + "limit": 1180 } }