added ruff code reivew
This commit is contained in:
ykcheungab
@@ -0,0 +1,193 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Merge gate for lint on a change — the narrow rule set a change must pass.
|
||||
|
||||
This is deliberately *not* the repo's full `ruff.toml` selection. The policy, decided from the
|
||||
measured findings rather than from taste:
|
||||
|
||||
* ``BLE001`` (blind except) is **allowed**. A broad ``except Exception`` is the right choice
|
||||
around device I/O, GL teardown and camera probing, and this codebase already records that
|
||||
choice inline as ``# noqa: BLE001 - <reason>``. Gating it would produce ~100 findings whose
|
||||
only correct fix is "add a reasoned suppression".
|
||||
* ``S110``/``S112`` (a handler that is only ``pass``/``continue``) and ``E722`` (bare
|
||||
``except:``) **are** gated: the complaint is not the breadth of the catch, it is that the
|
||||
failure becomes invisible. Silence is acceptable only with a reasoned suppression or a
|
||||
handler that reports/re-raises.
|
||||
* ``RUF100`` (unused suppression) is reported for triage but is configured ``unfixable``: a
|
||||
machine must never delete a suppression whose comment records why it exists.
|
||||
|
||||
Run over the files a change touches, never the whole tree, so an AI-authored diff is not
|
||||
drowned in pre-existing style churn.
|
||||
|
||||
Usage::
|
||||
|
||||
tools/lint_gate.py # files changed vs HEAD (staged + unstaged)
|
||||
tools/lint_gate.py --base origin/main # files changed vs a ref
|
||||
tools/lint_gate.py --files a.py b.py # explicit files
|
||||
tools/lint_gate.py --all # gate rules over the whole repo
|
||||
tools/lint_gate.py --json # machine-readable result on stdout
|
||||
|
||||
Exit codes: 0 clean, 1 findings, 2 a usage or environment problem (e.g. ruff missing).
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import argparse
|
||||
import json
|
||||
import os
|
||||
import shutil
|
||||
import subprocess
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
#: The gate. Explicit, so a rule added to `ruff.toml` cannot silently start failing builds.
|
||||
#: `E4`/`E7` are deliberately absent: `E402` fires on the intentional "set the environment before
|
||||
#: importing Qt" pattern in scripts, and `E701`/`E702` are style, which `ruff format` handles.
|
||||
GATE_SELECT = "E9,E722,F,S110,S112,B023,PLW1510"
|
||||
|
||||
#: Rules we report but never let a tool auto-fix (RUF100 is `unfixable` in ruff.toml too).
|
||||
REPORT_ONLY = "RUF100"
|
||||
|
||||
REPO = Path(__file__).resolve().parents[1]
|
||||
|
||||
|
||||
def find_ruff() -> str:
|
||||
"""Locate ruff: $RUFF, then PATH, then the interpreter environment."""
|
||||
for candidate in (os.environ.get("RUFF"), shutil.which("ruff")):
|
||||
if candidate and Path(candidate).is_file():
|
||||
return candidate
|
||||
beside = Path(sys.executable).parent / ("ruff.exe" if os.name == "nt" else "ruff")
|
||||
if beside.is_file():
|
||||
return str(beside)
|
||||
print(
|
||||
"lint_gate: ruff not found. Set $RUFF, install it beside the interpreter, or put it on "
|
||||
"PATH. The pin matters: findings differ between ruff versions, so the gate names its "
|
||||
"version below when it runs.",
|
||||
file=sys.stderr,
|
||||
)
|
||||
raise SystemExit(2)
|
||||
|
||||
|
||||
def git(*args: str) -> list[str]:
|
||||
result = subprocess.run(
|
||||
["git", "-C", str(REPO), *args],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
print(f"lint_gate: git {' '.join(args)} failed: {result.stderr.strip()}", file=sys.stderr)
|
||||
raise SystemExit(2)
|
||||
return [line for line in result.stdout.splitlines() if line.strip()]
|
||||
|
||||
|
||||
def changed_python_files(base: str | None) -> list[str]:
|
||||
"""The .py files this change touches: working tree, index, and new files."""
|
||||
paths: set[str] = set()
|
||||
if base:
|
||||
paths.update(git("diff", "--name-only", "--diff-filter=ACMR", f"{base}...HEAD"))
|
||||
paths.update(git("diff", "--name-only", "--diff-filter=ACMR"))
|
||||
paths.update(git("diff", "--name-only", "--cached", "--diff-filter=ACMR"))
|
||||
paths.update(git("ls-files", "--others", "--exclude-standard"))
|
||||
return sorted(p for p in paths if p.endswith(".py") and (REPO / p).is_file())
|
||||
|
||||
|
||||
def run_ruff(ruff: str, args: list[str]) -> tuple[list[dict], str]:
|
||||
version = subprocess.run(
|
||||
[ruff, "--version"], capture_output=True, text=True, check=False
|
||||
).stdout.strip()
|
||||
result = subprocess.run(
|
||||
[ruff, "check", "--no-cache", "--output-format", "json", *args],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
if result.returncode not in (0, 1):
|
||||
print(f"lint_gate: ruff failed ({result.returncode}): {result.stderr.strip()}", file=sys.stderr)
|
||||
raise SystemExit(2)
|
||||
try:
|
||||
findings = json.loads(result.stdout or "[]")
|
||||
except json.JSONDecodeError:
|
||||
print(f"lint_gate: could not parse ruff output: {result.stdout[:400]}", file=sys.stderr)
|
||||
raise SystemExit(2)
|
||||
return findings, version
|
||||
|
||||
|
||||
def describe(finding: dict) -> str:
|
||||
code = finding.get("code") or "?"
|
||||
location = finding.get("location") or {}
|
||||
path = str(finding.get("filename", "")).replace(str(REPO) + os.sep, "")
|
||||
return (
|
||||
f"{path}:{location.get('row', '?')}:{location.get('column', '?')}: {code}: "
|
||||
f"{finding.get('message', '')}"
|
||||
)
|
||||
|
||||
|
||||
def main(argv: list[str] | None = None) -> int:
|
||||
parser = argparse.ArgumentParser(description=__doc__.splitlines()[0])
|
||||
parser.add_argument("--base", default=None, help="compare against this ref (e.g. origin/main)")
|
||||
parser.add_argument("--files", nargs="*", default=None, help="explicit files to check")
|
||||
parser.add_argument("--all", action="store_true", help="run the gate rules over the whole repo")
|
||||
parser.add_argument("--json", action="store_true", help="machine-readable result on stdout")
|
||||
args = parser.parse_args(argv)
|
||||
|
||||
ruff = find_ruff()
|
||||
|
||||
if args.all:
|
||||
targets = [str(REPO)]
|
||||
else:
|
||||
targets = args.files or changed_python_files(args.base)
|
||||
if not targets:
|
||||
print("lint_gate: no changed Python files; nothing to check.")
|
||||
if args.json:
|
||||
print(json.dumps({"findings": [], "checked": [], "status": "clean"}))
|
||||
return 0
|
||||
|
||||
findings, version = run_ruff(ruff, ["--select", GATE_SELECT, *targets])
|
||||
report_only, _ = run_ruff(ruff, ["--select", REPORT_ONLY, *targets])
|
||||
|
||||
if args.json:
|
||||
print(
|
||||
json.dumps(
|
||||
{
|
||||
"ruff": version,
|
||||
"select": GATE_SELECT,
|
||||
"checked": targets,
|
||||
"findings": [
|
||||
{
|
||||
"file": str(f.get("filename", "")).replace(str(REPO) + os.sep, ""),
|
||||
"row": (f.get("location") or {}).get("row"),
|
||||
"column": (f.get("location") or {}).get("column"),
|
||||
"code": f.get("code"),
|
||||
"message": f.get("message"),
|
||||
"fixable": bool((f.get("fix") or {}).get("applicability") == "safe"),
|
||||
}
|
||||
for f in findings
|
||||
],
|
||||
"unused_suppressions": [describe(f) for f in report_only],
|
||||
"status": "findings" if findings else "clean",
|
||||
},
|
||||
indent=2,
|
||||
)
|
||||
)
|
||||
return 1 if findings else 0
|
||||
|
||||
print(f"lint_gate: {version}")
|
||||
print(f" select {GATE_SELECT} (BLE001 is allowed by policy; see ruff.toml)")
|
||||
print(f" checked {len(targets)} target(s)")
|
||||
if findings:
|
||||
print(f"\n{len(findings)} finding(s):")
|
||||
for finding in sorted(findings, key=describe):
|
||||
hint = " [safe fix: ruff check --fix]" if (finding.get("fix") or {}).get("applicability") == "safe" else ""
|
||||
print(f" {describe(finding)}{hint}")
|
||||
else:
|
||||
print(" no findings")
|
||||
if report_only:
|
||||
print(f"\n{len(report_only)} unused suppression(s) to triage by hand (never auto-removed):")
|
||||
for finding in report_only:
|
||||
print(f" {describe(finding)}")
|
||||
print(f"\nstatus: {'FINDINGS' if findings else 'clean'}")
|
||||
return 1 if findings else 0
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
raise SystemExit(main())
|
||||
Reference in New Issue
Block a user