mirror of
https://github.com/BerriAI/litellm.git
synced 2026-08-28 05:25:59 +00:00
Merge pull request #35325 from BerriAI/litellm_lit002_exempt_freezing_wrappers
fix(type-discipline): exempt values frozen in place by tuple/frozenset/MappingProxyType from LIT002
This commit is contained in:
commit
9a09104dc3
3 changed files with 58 additions and 3 deletions
|
|
@ -20,7 +20,10 @@ LIT002 Mutable-collection *construction*: a list/dict/set literal or comprehens
|
|||
generator (`tuple(f(x) for x in xs)`), a tuple literal, or a frozen dataclass /
|
||||
NamedTuple / ReadOnly TypedDict. Generator expressions and `tuple`/`frozenset`
|
||||
calls are not construction and pass. Annotation-internal lists (`Callable[[int],
|
||||
str]`) are exempt. Suppress with `# mutable-ok: <reason>`.
|
||||
str]`) are exempt, as is a value passed directly to a freezing wrapper
|
||||
(`tuple(...)`, `frozenset(...)`, `MappingProxyType(...)`): it is frozen before
|
||||
it can escape, though anything mutable nested inside it still counts.
|
||||
Suppress with `# mutable-ok: <reason>`.
|
||||
LIT003 noqa suppression without rule codes or without a reason.
|
||||
Required shape: `# noqa: TID251 # <reason>`
|
||||
LIT004 pyright/mypy ignore without bracketed codes or without a reason.
|
||||
|
|
@ -90,6 +93,7 @@ MUTABLE_CONSTRUCTORS = frozenset((
|
|||
# are common methods (e.g. pydantic's `model.dict()`), not collection construction. A
|
||||
# qualified `collections.deque(...)` still counts.
|
||||
QUALIFIED_CONSTRUCTORS = MUTABLE_CONSTRUCTORS - frozenset(("dict", "list", "set"))
|
||||
FREEZING_WRAPPERS = frozenset(("tuple", "frozenset", "MappingProxyType"))
|
||||
UNSAFE_GUARDS = frozenset(("TypeGuard", "TypeIs"))
|
||||
MIN_REASON_LEN = 3
|
||||
|
||||
|
|
@ -382,6 +386,34 @@ def _annotation_node_ids(tree: ast.AST) -> frozenset[int]:
|
|||
)
|
||||
|
||||
|
||||
def _is_freezing_wrapper(func: ast.expr) -> bool:
|
||||
if isinstance(func, ast.Name):
|
||||
return func.id in FREEZING_WRAPPERS
|
||||
return (
|
||||
isinstance(func, ast.Attribute)
|
||||
and func.attr == "MappingProxyType"
|
||||
and isinstance(func.value, ast.Name)
|
||||
and func.value.id == "types"
|
||||
)
|
||||
|
||||
|
||||
def _frozen_argument_ids(tree: ast.AST) -> frozenset[int]:
|
||||
"""ids() of every expression passed directly to a freezing wrapper.
|
||||
|
||||
`MappingProxyType({...})`, `frozenset({...})`, and `tuple([...])` freeze their
|
||||
argument before it can escape, so the literal inside is a one-shot build, not a
|
||||
mutable value anyone can grow later. Only the argument itself is exempt; a
|
||||
mutable collection nested inside it still trips LIT002. Only bare names (plus
|
||||
`types.MappingProxyType`) qualify, so an unrelated method that happens to share
|
||||
a wrapper's name cannot exempt its argument.
|
||||
"""
|
||||
return frozenset(
|
||||
id(node.args[0])
|
||||
for node in ast.walk(tree)
|
||||
if isinstance(node, ast.Call) and len(node.args) == 1 and _is_freezing_wrapper(node.func)
|
||||
)
|
||||
|
||||
|
||||
def _construction_kind(node: ast.expr) -> str | None:
|
||||
"""Human label if `node` builds a mutable collection, else None."""
|
||||
if isinstance(node, ast.List):
|
||||
|
|
@ -407,8 +439,9 @@ def _construction_kind(node: ast.expr) -> str | None:
|
|||
|
||||
def iter_construction_violations(path: Path, tree: ast.AST, comments: Comments) -> Iterator[Violation]:
|
||||
in_annotation = _annotation_node_ids(tree)
|
||||
frozen_arguments = _frozen_argument_ids(tree)
|
||||
for node in ast.walk(tree):
|
||||
if not isinstance(node, ast.expr) or id(node) in in_annotation:
|
||||
if not isinstance(node, ast.expr) or id(node) in in_annotation or id(node) in frozen_arguments:
|
||||
continue
|
||||
kind = _construction_kind(node)
|
||||
if kind is None or node.lineno in comments.mutable_ok_lines:
|
||||
|
|
|
|||
|
|
@ -152,6 +152,28 @@ def test_qualified_collections_constructors_still_count(tmp_path):
|
|||
assert "LIT002" in _codes(tmp_path, "import collections\nm = collections.defaultdict(list)\n")
|
||||
|
||||
|
||||
def test_value_frozen_by_wrapper_is_exempt(tmp_path):
|
||||
assert "LIT002" not in _codes(tmp_path, "from types import MappingProxyType\nm = MappingProxyType({'a': 1})\n")
|
||||
assert "LIT002" not in _codes(tmp_path, "import types\nm = types.MappingProxyType({'a': 1})\n")
|
||||
assert "LIT002" not in _codes(tmp_path, "from types import MappingProxyType\nm = MappingProxyType(dict(a=1))\n")
|
||||
assert "LIT002" not in _codes(tmp_path, "f = frozenset({1, 2})\n")
|
||||
assert "LIT002" not in _codes(tmp_path, "t = tuple([1, 2])\n")
|
||||
|
||||
|
||||
def test_same_named_method_does_not_exempt_its_argument(tmp_path):
|
||||
assert "LIT002" in _codes(tmp_path, "t = obj.tuple([1, 2])\n")
|
||||
assert "LIT002" in _codes(tmp_path, "f = obj.frozenset({1, 2})\n")
|
||||
assert "LIT002" in _codes(tmp_path, "m = obj.MappingProxyType({'a': 1})\n")
|
||||
|
||||
|
||||
def test_mutable_nested_inside_frozen_wrapper_still_counts(tmp_path):
|
||||
assert "LIT002" in _codes(tmp_path, "from types import MappingProxyType\nm = MappingProxyType({'a': []})\n")
|
||||
|
||||
|
||||
def test_unfrozen_literal_still_counts(tmp_path):
|
||||
assert "LIT002" in _codes(tmp_path, "from types import MappingProxyType\nd = {'a': 1}\nm = MappingProxyType(d)\n")
|
||||
|
||||
|
||||
def test_mutable_ok_with_reason_suppresses_both_rules(tmp_path):
|
||||
codes = _codes(tmp_path, "x: dict[str, int] = {} # mutable-ok: in-place buffer mutated hot path\n")
|
||||
assert "LIT001" not in codes
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@
|
|||
"limit": 23253
|
||||
},
|
||||
"LIT002": {
|
||||
"limit": 27427
|
||||
"limit": 27280
|
||||
},
|
||||
"LIT003": {
|
||||
"limit": 292
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue