mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-10 03:28:53 +00:00
refactor(lint): graduate the 35 zero-violation strict rules into ruff.toml
Every strict-gate rule whose budget ceiling was already 0 moves into the base config's lint.extend-select, so editors and ruff check --fix surface the diagnostics directly and the budget file shrinks to rules with real debt. Graduates stay in ruff-strict.toml's select so the strict RUF100 pass keeps policing their stale noqa directives, and base external entries they made redundant (FURB, I001, RUF010, RUF022, RUF023, RUF051) are dropped so base RUF100 polices those directly. UP037 had two violations hidden behind a star import; importing Literal explicitly fixes them so UP037 can graduate too. New drift tests pin the invariants: every strict-selected rule is budgeted or hard-failed by base, every base-owned rule stays visible to exactly one RUF100 pass, and graduated rules fail the normal ruff run.
This commit is contained in:
parent
c3536c29a0
commit
f304b7b19f
4 changed files with 237 additions and 112 deletions
|
|
@ -17,7 +17,7 @@ import json
|
|||
import traceback
|
||||
from collections.abc import Mapping, Sequence
|
||||
from datetime import datetime, timezone
|
||||
from typing import Any, Final, cast
|
||||
from typing import Any, Final, Literal, cast
|
||||
|
||||
import fastapi
|
||||
from fastapi import APIRouter, Depends, Header, HTTPException, Request, status
|
||||
|
|
|
|||
|
|
@ -56,9 +56,6 @@
|
|||
"B026": {
|
||||
"limit": 3
|
||||
},
|
||||
"B033": {
|
||||
"limit": 0
|
||||
},
|
||||
"BLE001": {
|
||||
"limit": 2924
|
||||
},
|
||||
|
|
@ -113,18 +110,6 @@
|
|||
"F401": {
|
||||
"limit": 17
|
||||
},
|
||||
"FURB136": {
|
||||
"limit": 0
|
||||
},
|
||||
"FURB168": {
|
||||
"limit": 0
|
||||
},
|
||||
"FURB188": {
|
||||
"limit": 0
|
||||
},
|
||||
"I001": {
|
||||
"limit": 0
|
||||
},
|
||||
"LOG015": {
|
||||
"limit": 5
|
||||
},
|
||||
|
|
@ -137,18 +122,9 @@
|
|||
"PERF401": {
|
||||
"limit": 12
|
||||
},
|
||||
"PERF402": {
|
||||
"limit": 0
|
||||
},
|
||||
"PERF403": {
|
||||
"limit": 34
|
||||
},
|
||||
"PIE790": {
|
||||
"limit": 0
|
||||
},
|
||||
"PIE800": {
|
||||
"limit": 0
|
||||
},
|
||||
"PIE804": {
|
||||
"limit": 18
|
||||
},
|
||||
|
|
@ -158,9 +134,6 @@
|
|||
"PLC0206": {
|
||||
"limit": 26
|
||||
},
|
||||
"PLC0208": {
|
||||
"limit": 0
|
||||
},
|
||||
"PLC0414": {
|
||||
"limit": 46
|
||||
},
|
||||
|
|
@ -170,24 +143,12 @@
|
|||
"PLR0206": {
|
||||
"limit": 1
|
||||
},
|
||||
"PLR0402": {
|
||||
"limit": 0
|
||||
},
|
||||
"PLR1704": {
|
||||
"limit": 3
|
||||
},
|
||||
"PLR1711": {
|
||||
"limit": 0
|
||||
},
|
||||
"PLR1714": {
|
||||
"limit": 257
|
||||
},
|
||||
"PLR1730": {
|
||||
"limit": 0
|
||||
},
|
||||
"PLR2044": {
|
||||
"limit": 0
|
||||
},
|
||||
"PLW0127": {
|
||||
"limit": 57
|
||||
},
|
||||
|
|
@ -206,27 +167,12 @@
|
|||
"PLW1510": {
|
||||
"limit": 2
|
||||
},
|
||||
"PYI030": {
|
||||
"limit": 0
|
||||
},
|
||||
"PYI036": {
|
||||
"limit": 3
|
||||
},
|
||||
"PYI041": {
|
||||
"limit": 0
|
||||
},
|
||||
"PYI064": {
|
||||
"limit": 0
|
||||
},
|
||||
"RET501": {
|
||||
"limit": 0
|
||||
},
|
||||
"RET504": {
|
||||
"limit": 177
|
||||
},
|
||||
"RUF010": {
|
||||
"limit": 0
|
||||
},
|
||||
"RUF012": {
|
||||
"limit": 241
|
||||
},
|
||||
|
|
@ -236,18 +182,9 @@
|
|||
"RUF019": {
|
||||
"limit": 38
|
||||
},
|
||||
"RUF022": {
|
||||
"limit": 0
|
||||
},
|
||||
"RUF023": {
|
||||
"limit": 0
|
||||
},
|
||||
"RUF046": {
|
||||
"limit": 4
|
||||
},
|
||||
"RUF051": {
|
||||
"limit": 0
|
||||
},
|
||||
"RUF059": {
|
||||
"limit": 67
|
||||
},
|
||||
|
|
@ -272,18 +209,12 @@
|
|||
"SIM113": {
|
||||
"limit": 3
|
||||
},
|
||||
"SIM114": {
|
||||
"limit": 0
|
||||
},
|
||||
"SIM115": {
|
||||
"limit": 2
|
||||
},
|
||||
"SIM117": {
|
||||
"limit": 7
|
||||
},
|
||||
"SIM118": {
|
||||
"limit": 0
|
||||
},
|
||||
"SIM201": {
|
||||
"limit": 1
|
||||
},
|
||||
|
|
@ -302,9 +233,6 @@
|
|||
"TC004": {
|
||||
"limit": 5
|
||||
},
|
||||
"TC005": {
|
||||
"limit": 0
|
||||
},
|
||||
"TID251": {
|
||||
"limit": 1240
|
||||
},
|
||||
|
|
@ -323,46 +251,13 @@
|
|||
"TRY300": {
|
||||
"limit": 860
|
||||
},
|
||||
"UP006": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP007": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP008": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP012": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP018": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP024": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP028": {
|
||||
"limit": 2
|
||||
},
|
||||
"UP031": {
|
||||
"limit": 2
|
||||
},
|
||||
"UP032": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP034": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP035": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP036": {
|
||||
"limit": 1
|
||||
},
|
||||
"UP037": {
|
||||
"limit": 0
|
||||
},
|
||||
"UP045": {
|
||||
"limit": 0
|
||||
}
|
||||
}
|
||||
|
|
|
|||
22
ruff.toml
22
ruff.toml
|
|
@ -1,14 +1,26 @@
|
|||
lint.ignore = ["F405", "E402", "F403"]
|
||||
lint.extend-select = ["T20", "PGH004", "RUF008", "RUF009", "RUF100"]
|
||||
# The second group is the strict gate's graduates: rules the codebase already has zero
|
||||
# violations of, so they hard-fail here instead of being ratcheted in ruff-strict-budget.json.
|
||||
# That gives editors and `ruff check --fix` the diagnostic, which the gate script cannot.
|
||||
lint.extend-select = [
|
||||
"T20", "PGH004", "RUF008", "RUF009", "RUF100",
|
||||
"B033", "FURB136", "FURB168", "FURB188", "I001", "PERF402", "PIE790", "PIE800", "PLC0208",
|
||||
"PLR0402", "PLR1711", "PLR1730", "PLR2044", "PYI030", "PYI041", "PYI064", "RET501", "RUF010",
|
||||
"RUF022", "RUF023", "RUF051", "SIM114", "SIM118", "TC005", "UP006", "UP007", "UP008", "UP012",
|
||||
"UP018", "UP024", "UP032", "UP034", "UP035", "UP037", "UP045",
|
||||
]
|
||||
# RUF100 (unused-noqa) only knows the rules enabled in THIS config, so it would strip
|
||||
# `# noqa` directives that protect rules enforced elsewhere. List those codes as external
|
||||
# so RUF100 leaves their directives alone: the strict gate (ruff-strict.toml) and upstream
|
||||
# litellm's own ruff config both rely on suppressions this config can't see.
|
||||
lint.external = [
|
||||
# Enforced by the strict-rule gate (scripts/ruff_strict_gate.py + ruff-strict.toml)
|
||||
"ANN", "ASYNC230", "B", "C", "D419", "DTZ", "EXE", "FURB", "I001", "LOG015", "N999", "PERF",
|
||||
"PIE", "PL", "PYI", "RET", "RUF010", "RUF012", "RUF015", "RUF019", "RUF022", "RUF023",
|
||||
"RUF046", "RUF051", "RUF059", "S110", "S112", "SIM", "TC", "TID251", "TRY", "UP",
|
||||
# Enforced by the strict-rule gate (scripts/ruff_strict_gate.py + ruff-strict.toml).
|
||||
# Family entries whose every strict rule graduated into extend-select above (FURB), and
|
||||
# standalone graduated codes (I001, RUF010, RUF022, RUF023, RUF051), are dropped so this
|
||||
# config's RUF100 polices their directives itself.
|
||||
"ANN", "ASYNC230", "B", "C", "D419", "DTZ", "EXE", "LOG015", "N999", "PERF",
|
||||
"PIE", "PL", "PYI", "RET", "RUF012", "RUF015", "RUF019",
|
||||
"RUF046", "RUF059", "S110", "S112", "SIM", "TC", "TID251", "TRY", "UP",
|
||||
# Enforced by upstream litellm's ruff config, but not run in this repo's CI
|
||||
"PLC0415", "E402", "BLE001", "ARG002", "S102", "S324", "S606", "D401", "F403", "F405",
|
||||
]
|
||||
|
|
|
|||
|
|
@ -1,16 +1,24 @@
|
|||
import importlib.util
|
||||
import json
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
import tomllib
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
_MODULE_PATH = Path(__file__).resolve().parents[2] / "scripts" / "ruff_strict_gate.py"
|
||||
_REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||
_MODULE_PATH = _REPO_ROOT / "scripts" / "ruff_strict_gate.py"
|
||||
_spec = importlib.util.spec_from_file_location("ruff_strict_gate", _MODULE_PATH)
|
||||
gate = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(gate)
|
||||
|
||||
Violation = gate.Violation
|
||||
|
||||
_ENABLED_BY_RUFF_DEFAULTS = frozenset({"F401"})
|
||||
|
||||
|
||||
def rule(name, limit):
|
||||
return {name: {"limit": limit}}
|
||||
|
|
@ -151,3 +159,213 @@ def test_base_point_mid_merge_advances_to_the_merged_in_base_tip(tmp_path):
|
|||
repo, _, base_tip = _branched_repo(tmp_path)
|
||||
_git(repo, "merge", "--no-commit", "--no-ff", "main")
|
||||
assert gate.resolve_base_point("main", cwd=repo) == base_tip
|
||||
|
||||
|
||||
def _lint_section(config_name: str) -> dict:
|
||||
return tomllib.loads((_REPO_ROOT / config_name).read_text())["lint"]
|
||||
|
||||
|
||||
def _base_external() -> tuple[str, ...]:
|
||||
return tuple(_lint_section("ruff.toml")["external"])
|
||||
|
||||
|
||||
def _strict_external() -> tuple[str, ...]:
|
||||
return tuple(_lint_section("ruff-strict.toml")["external"])
|
||||
|
||||
|
||||
def _strict_selected() -> frozenset:
|
||||
return frozenset(_lint_section("ruff-strict.toml")["select"])
|
||||
|
||||
|
||||
def _prefix_covered(code: str, prefixes: tuple[str, ...]) -> bool:
|
||||
return any(code.startswith(prefix) for prefix in prefixes)
|
||||
|
||||
|
||||
def _selected_by_the_normal_config() -> frozenset:
|
||||
return frozenset(_lint_section("ruff.toml")["extend-select"]) | _ENABLED_BY_RUFF_DEFAULTS
|
||||
|
||||
|
||||
def _budgeted_rules() -> frozenset:
|
||||
return frozenset(json.loads((_REPO_ROOT / "ruff-strict-budget.json").read_text()))
|
||||
|
||||
|
||||
def _ruff_binary() -> str | None:
|
||||
beside_interpreter = Path(sys.executable).with_name("ruff")
|
||||
return str(beside_interpreter) if beside_interpreter.exists() else shutil.which("ruff")
|
||||
|
||||
|
||||
_RUFF = _ruff_binary()
|
||||
_needs_ruff = pytest.mark.skipif(_RUFF is None, reason="ruff is not installed in this environment")
|
||||
|
||||
|
||||
def _ruff_output_for_noqa(code: str, *extra_args: str) -> str:
|
||||
proc = subprocess.run(
|
||||
[
|
||||
_RUFF,
|
||||
"check",
|
||||
"--no-cache",
|
||||
"--stdin-filename",
|
||||
"litellm/types/_external_probe.py",
|
||||
*extra_args,
|
||||
"-",
|
||||
],
|
||||
cwd=_REPO_ROOT,
|
||||
input=f"def _probe(x: int): # noqa: {code}\n return x\n",
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
return proc.stdout
|
||||
|
||||
|
||||
def test_every_strict_gate_rule_is_protected_from_base_ruf100():
|
||||
unprotected = frozenset(
|
||||
selector
|
||||
for selector in _strict_selected()
|
||||
if not _prefix_covered(selector, _base_external())
|
||||
and selector not in _selected_by_the_normal_config()
|
||||
)
|
||||
assert unprotected == frozenset(), (
|
||||
f"`ruff check` deletes any `# noqa` naming {sorted(unprotected)} as unused, so suppressing "
|
||||
"one of those strict-gate rules breaks lint. Cover them in ruff.toml's lint.external or "
|
||||
"enable them in its lint.extend-select."
|
||||
)
|
||||
|
||||
|
||||
def test_every_selected_rule_keeps_stale_noqa_detection_somewhere():
|
||||
policed_by_strict = frozenset(
|
||||
selector
|
||||
for selector in _strict_selected()
|
||||
if not _prefix_covered(selector, _strict_external())
|
||||
)
|
||||
policed_by_base = frozenset(
|
||||
selector
|
||||
for selector in _selected_by_the_normal_config()
|
||||
if not _prefix_covered(selector, _base_external())
|
||||
)
|
||||
shadowed = (
|
||||
_strict_selected() | _selected_by_the_normal_config()
|
||||
) - policed_by_strict - policed_by_base
|
||||
assert shadowed == frozenset(), (
|
||||
f"no config's RUF100 can ever report a stale `# noqa` for {sorted(shadowed)}: every config "
|
||||
"that selects each of them also shadows it with an external entry. Narrow the external "
|
||||
"entry in ruff.toml or ruff-strict.toml."
|
||||
)
|
||||
|
||||
|
||||
_BASE_OWNED_FAMILY = re.compile(r"E[479]\d+|F\d+|T20\d+")
|
||||
_BASE_OWNED_SINGLES = frozenset({"PGH004", "RUF008", "RUF009", "RUF100"})
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def all_ruff_rule_codes() -> frozenset:
|
||||
listing = subprocess.run(
|
||||
[_RUFF, "rule", "--all", "--output-format", "json"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
assert listing.returncode == 0, listing.stderr
|
||||
return frozenset(
|
||||
entry["code"] for entry in json.loads(listing.stdout) if "Removed" not in entry["status"]
|
||||
)
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_every_base_owned_rule_is_external_or_selected_in_the_strict_config(all_ruff_rule_codes):
|
||||
base_owned = frozenset(
|
||||
code
|
||||
for code in all_ruff_rule_codes
|
||||
if _BASE_OWNED_FAMILY.fullmatch(code) or code in _BASE_OWNED_SINGLES
|
||||
)
|
||||
stranded = frozenset(
|
||||
code
|
||||
for code in base_owned
|
||||
if code not in _strict_selected() and not _prefix_covered(code, _strict_external())
|
||||
)
|
||||
assert stranded == frozenset(), (
|
||||
f"the strict gate's RUF100 reads a valid `# noqa` for {sorted(stranded)} as unused, the "
|
||||
"spurious-breach trap ruff-strict.toml's external override exists to prevent. Cover them "
|
||||
"there."
|
||||
)
|
||||
double_booked = frozenset(
|
||||
code
|
||||
for code in base_owned
|
||||
if code in _strict_selected() and _prefix_covered(code, _strict_external())
|
||||
)
|
||||
assert double_booked == frozenset(), (
|
||||
f"{sorted(double_booked)} are selected by the strict config yet shadowed by its external "
|
||||
"list, so their stale suppressions can never be reported. Narrow the external entry in "
|
||||
"ruff-strict.toml."
|
||||
)
|
||||
|
||||
|
||||
def test_every_budgeted_rule_is_one_the_gate_actually_measures():
|
||||
selectors = tuple(_lint_section("ruff-strict.toml")["select"])
|
||||
unmeasured = frozenset(code for code in _budgeted_rules() if not code.startswith(selectors))
|
||||
assert unmeasured == frozenset(), (
|
||||
f"the gate never counts {sorted(unmeasured)}, so their ceilings are dead config that reads "
|
||||
"as coverage. Either select them in ruff-strict.toml or drop them from the budget."
|
||||
)
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_every_strict_selected_rule_is_budgeted_or_hard_failed_by_the_base_config(all_ruff_rule_codes):
|
||||
strict_enabled = frozenset(
|
||||
code
|
||||
for code in all_ruff_rule_codes
|
||||
if code.startswith(tuple(_lint_section("ruff-strict.toml")["select"]))
|
||||
)
|
||||
base_hard_failed = tuple(_lint_section("ruff.toml")["extend-select"])
|
||||
unpoliced = frozenset(
|
||||
code
|
||||
for code in strict_enabled
|
||||
if code not in _budgeted_rules()
|
||||
and not code.startswith(base_hard_failed)
|
||||
and code not in _ENABLED_BY_RUFF_DEFAULTS
|
||||
)
|
||||
assert unpoliced == frozenset(), (
|
||||
f"nothing enforces {sorted(unpoliced)}: the gate skips rules missing from the budget, and "
|
||||
"the base config does not hard-fail them. Re-add a budget ceiling or graduate them into "
|
||||
"ruff.toml's lint.extend-select."
|
||||
)
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_a_noqa_for_a_strict_gate_rule_survives_the_normal_ruff_run():
|
||||
assert "RUF100" not in _ruff_output_for_noqa("ANN202")
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_the_external_list_is_what_saves_that_noqa():
|
||||
assert "RUF100" in _ruff_output_for_noqa("ANN202", "--config", "lint.external=[]")
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_a_stale_noqa_for_a_locally_enabled_rule_is_still_reported():
|
||||
assert "RUF100" in _ruff_output_for_noqa("F401")
|
||||
|
||||
|
||||
def _ruff_output_for_source(source: str) -> str:
|
||||
proc = subprocess.run(
|
||||
[_RUFF, "check", "--no-cache", "--stdin-filename", "litellm/types/_graduate_probe.py", "-"],
|
||||
cwd=_REPO_ROOT,
|
||||
input=source,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
return proc.stdout
|
||||
|
||||
|
||||
_DEPRECATED_TYPING_ALIAS = "from typing import List # noqa: UP035\n\n\ndef _probe(x: List[int]) -> None: ...\n"
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_a_graduated_rule_now_fails_the_normal_ruff_run_instead_of_waiting_for_the_gate():
|
||||
assert "UP006" in _ruff_output_for_source(_DEPRECATED_TYPING_ALIAS)
|
||||
|
||||
|
||||
@_needs_ruff
|
||||
def test_a_graduated_rule_can_still_be_suppressed_without_tripping_unused_noqa():
|
||||
suppressed = _DEPRECATED_TYPING_ALIAS.replace("...\n", "... # noqa: UP006\n")
|
||||
output = _ruff_output_for_source(suppressed)
|
||||
assert "UP006" not in output
|
||||
assert "RUF100" not in output
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue