mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-25 01:02:15 +00:00
feat(lint): add LIT013 requiring pydantic models to be frozen
Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
This commit is contained in:
mateo
parent
3d26a29a1a
commit
c8cbdb3897
4 changed files with 243 additions and 2 deletions
|
|
@ -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: <reason>`
|
||||
- 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: <reason>`
|
||||
- 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: <reason>` 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
|
||||
|
|
|
|||
|
|
@ -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] # <reason>`
|
||||
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: <reason>`.
|
||||
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: <reason>` 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<reason>.*))?")
|
|||
KWARGS_OK_RE = re.compile(r"#\s*kwargs-ok(?::\s*(?P<reason>.*))?")
|
||||
REBIND_OK_RE = re.compile(r"#\s*rebind-ok(?::\s*(?P<reason>.*))?")
|
||||
WRITABLE_OK_RE = re.compile(r"#\s*writable-ok(?::\s*(?P<reason>.*))?")
|
||||
FROZEN_OK_RE = re.compile(r"#\s*frozen-ok(?::\s*(?P<reason>.*))?")
|
||||
|
||||
# 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: <reason>`)",
|
||||
)
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------- #
|
||||
# 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),
|
||||
)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
# --------------------------------------------------------------------------- #
|
||||
|
|
|
|||
|
|
@ -34,5 +34,8 @@
|
|||
},
|
||||
"LIT012": {
|
||||
"limit": 4486
|
||||
},
|
||||
"LIT013": {
|
||||
"limit": 1180
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue