From 0eb5a2edbed43c30c8e6a4a54d25d26c46646508 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Tue, 23 Jun 2026 22:20:12 -0700 Subject: [PATCH] [eric] linter: back out the naming checks (no-underscore + p-private + _models + driver); they arrive via Haik's branch, don't duplicate them here --- linter/check_naming.py | 53 ------ linter/checks/_models.py | 81 --------- linter/checks/no_underscore_names.py | 148 ---------------- linter/checks/p_private.py | 242 --------------------------- 4 files changed, 524 deletions(-) delete mode 100644 linter/check_naming.py delete mode 100644 linter/checks/_models.py delete mode 100644 linter/checks/no_underscore_names.py delete mode 100644 linter/checks/p_private.py diff --git a/linter/check_naming.py b/linter/check_naming.py deleted file mode 100644 index d60c9262..00000000 --- a/linter/check_naming.py +++ /dev/null @@ -1,53 +0,0 @@ -#!/usr/bin/env python3 -"""Naming-convention gate: runs the no-underscore-names + p-private AST checks (Haik's checks, -brought over from haik/refactor/delete-fluff) against backend/ and prints violations + counts. -This is the authoritative gate for the leading-_ -> p_/P_/public migration: green here means the -codebase follows the access-modifier convention to a tea. Run from the linter/ dir. - -Usage: python check_naming.py [--summary] [] -""" - -from __future__ import annotations - -import json -import sys -from collections import Counter -from pathlib import Path - -from checks.no_underscore_names import run_underscore_check -from checks.p_private import run_p_private_check - -ROOT = Path(__file__).resolve().parent.parent -CONFIG = json.load(open(Path(__file__).resolve().parent / "config" / "config.json")) - - -def main() -> int: - args = [a for a in sys.argv[1:] if not a.startswith("--")] - summary = "--summary" in sys.argv - prefix = args[0] if args else "" - - exceptions = CONFIG.get("exceptions", {}) - excludes = CONFIG["exclude"] - - underscore = run_underscore_check(ROOT, exceptions, excludes, None) - pprivate = run_p_private_check(ROOT, exceptions, excludes, None) - - def keep(e: str) -> bool: - return e.startswith(prefix) if prefix else True - - underscore = [e for e in underscore if keep(e)] - pprivate = [e for e in pprivate if keep(e)] - - print(f"no-underscore-names: {len(underscore)} p-private: {len(pprivate)}") - if summary: - return 1 if (underscore or pprivate) else 0 - - for e in underscore: - print(e) - for e in pprivate: - print(e) - return 1 if (underscore or pprivate) else 0 - - -if __name__ == "__main__": - sys.exit(main()) diff --git a/linter/checks/_models.py b/linter/checks/_models.py deleted file mode 100644 index fb23da27..00000000 --- a/linter/checks/_models.py +++ /dev/null @@ -1,81 +0,0 @@ -"""Shared model metadata for the linter. - -Pydantic ``BaseModel`` field names are collected here so independent checks can -agree on which attribute names belong to a serialization schema, and are -therefore "used" even when the only *read* happens across a language boundary -(serialized to JSON over the wire and consumed by the frontend) which vulture's -Python-only analysis structurally cannot see. - -Framework detection lives in ONE place so checks/classes.py and -checks/vulture.py never drift on what counts as a model. This module owns no -section of its own and is not gated by config "enabled" flags, so any check that -imports it works regardless of which sections are turned on. -""" - -from __future__ import annotations - -import ast -from functools import lru_cache -from pathlib import Path - -FRAMEWORK_BASES = {"BaseModel"} - - -def is_framework_model(cls: ast.ClassDef) -> bool: - """True when *cls* subclasses a known framework base (e.g. pydantic BaseModel).""" - return any( - (isinstance(b, ast.Name) and b.id in FRAMEWORK_BASES) - or (isinstance(b, ast.Attribute) and b.attr in FRAMEWORK_BASES) - for b in cls.bases - ) - - -@lru_cache(maxsize=512) -def _fields_in_file_cached(filepath: str, _mtime: float) -> frozenset[str]: - """Annotated field names on framework models in *filepath*. - - Keyed on *(filepath, mtime)* so the long-lived watch process re-parses a file - after it is edited instead of returning a stale set from an earlier version. - """ - try: - tree = ast.parse(Path(filepath).read_text()) - except (OSError, SyntaxError): - return frozenset() - names: set[str] = set() - for node in ast.walk(tree): - if not isinstance(node, ast.ClassDef) or not is_framework_model(node): - continue - # Direct body only: pydantic fields are annotated assignments - # (``name: type`` / ``name: type = default``). Nested models are picked - # up on their own ClassDef pass by ast.walk. - for stmt in node.body: - if isinstance(stmt, ast.AnnAssign) and isinstance(stmt.target, ast.Name): - names.add(stmt.target.id) - return frozenset(names) - - -def _fields_in_file(filepath: Path) -> frozenset[str]: - try: - mtime = filepath.stat().st_mtime - except OSError: - return frozenset() - return _fields_in_file_cached(str(filepath), mtime) - - -def collect_pydantic_field_names(root: Path) -> set[str]: - """Every annotated field name on a ``BaseModel`` subclass under ``backend/``. - - Consumed by checks/vulture.py to treat ``obj. = ...`` writes as live - even when the only read is across the Python/TS boundary, which vulture would - otherwise report as an unused attribute. - """ - backend = root / "backend" - if not backend.is_dir(): - return set() - names: set[str] = set() - for pyfile in backend.rglob("*.py"): - parts = pyfile.parts - if ".venv" in parts or "__pycache__" in parts: - continue - names |= _fields_in_file(pyfile) - return names diff --git a/linter/checks/no_underscore_names.py b/linter/checks/no_underscore_names.py deleted file mode 100644 index f4097068..00000000 --- a/linter/checks/no_underscore_names.py +++ /dev/null @@ -1,148 +0,0 @@ -"""Ban leading-underscore names that dead-code tooling silently skips. - -Pylance's reportUnusedVariable, ruff's dummy-variable-rgx (F841/ARG0xx), and -vulture all treat a leading underscore as "intentionally private/unused" and -stop reporting it -- which makes ``_name`` a blind spot for dead-code detection. -This check bans the prefix so nothing can hide behind it. - -Exempt: dunders (``__init__`` and friends, i.e. ``__x__``) and the bare ``_`` -throwaway (``for _ in ...`` / ``a, _ = unpack()``). Everything else starting -with ``_`` is flagged, including name-mangled ``__x``. - -Covers function/method names, arguments (incl. lambda), class names, variable -bindings (assignments, annotations, walrus, loop/with/except targets, tuple -unpacking), instance/class attribute writes (``self._x = ...``), and import -aliases. Scoped to ``backend/`` Python, like checks/classes.py. -""" - -from __future__ import annotations - -import ast -from collections.abc import Callable -from pathlib import Path - -from . import is_excepted, is_excluded, is_lintignored - -RULE = "no-underscore-names" - -Report = Callable[[str, int, int, str], None] - - -def _is_dunder(name: str) -> bool: - """True for ``__x__`` style names that Python requires (``__init__`` etc.).""" - return len(name) > 4 and name.startswith("__") and name.endswith("__") - - -def _flagged(name: str) -> bool: - if name == "_" or not name.startswith("_"): - return False - return not _is_dunder(name) - - -def _report_targets(target: ast.AST, report: Report) -> None: - """Walk an assignment/loop target down to the names it binds.""" - if isinstance(target, ast.Name): - report(target.id, target.lineno, target.col_offset, "variable") - elif isinstance(target, ast.Attribute): - # self._x = ... / cls._x = ...: the bound name is the attribute itself. - report(target.attr, target.end_lineno or target.lineno, _attr_col(target), "attribute") - elif isinstance(target, ast.Starred): - _report_targets(target.value, report) - elif isinstance(target, (ast.Tuple, ast.List)): - for elt in target.elts: - _report_targets(elt, report) - # ast.Subscript (``d["_x"] = ...``) is not a name binding and is skipped. - - -def _attr_col(node: ast.Attribute) -> int: - """Best-effort column for the attribute name (after the dot).""" - # value.end_col_offset points just past ``value``; +1 skips the dot. - end = getattr(node.value, "end_col_offset", None) - return (end + 1) if end is not None else node.col_offset - - -def _args(a: ast.arguments) -> list[ast.arg | None]: - return [*a.posonlyargs, *a.args, *a.kwonlyargs, a.vararg, a.kwarg] - - -def _check_tree(tree: ast.AST, rel: str) -> list[str]: - errors: list[str] = [] - seen: set[tuple[int, int, str]] = set() - - def report(name: str, lineno: int, col: int, kind: str) -> None: - if not _flagged(name): - return - key = (lineno, col, name) - if key in seen: - return - seen.add(key) - errors.append( - f"{rel}:{lineno}:{col + 1}: error: " - f"[{RULE}] {kind} '{name}' has a leading underscore" - ) - - for node in ast.walk(tree): - if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): - report(node.name, node.lineno, node.col_offset, "function") - for arg in _args(node.args): - if arg is not None: - report(arg.arg, arg.lineno, arg.col_offset, "argument") - elif isinstance(node, ast.Lambda): - for arg in _args(node.args): - if arg is not None: - report(arg.arg, arg.lineno, arg.col_offset, "argument") - elif isinstance(node, ast.ClassDef): - report(node.name, node.lineno, node.col_offset, "class") - elif isinstance(node, (ast.Assign, ast.AnnAssign, ast.NamedExpr)): - targets = node.targets if isinstance(node, ast.Assign) else [node.target] - for t in targets: - _report_targets(t, report) - elif isinstance(node, (ast.For, ast.AsyncFor, ast.comprehension)): - _report_targets(node.target, report) - elif isinstance(node, (ast.With, ast.AsyncWith)): - for item in node.items: - if item.optional_vars is not None: - _report_targets(item.optional_vars, report) - elif isinstance(node, ast.ExceptHandler) and node.name: - report(node.name, node.lineno, node.col_offset, "exception") - elif isinstance(node, ast.Import): - for alias in node.names: - report( - alias.asname or alias.name.split(".")[0], - node.lineno, node.col_offset, "import", - ) - elif isinstance(node, ast.ImportFrom): - for alias in node.names: - if alias.name != "*": - report(alias.asname or alias.name, node.lineno, node.col_offset, "import") - - return errors - - -def run_underscore_check( - root: Path, - exceptions: dict[str, list[str]], - excludes: list[str], - ignores: dict[Path, set[str]] | None = None, -) -> list[str]: - """Flag leading-underscore names in backend Python files.""" - errors: list[str] = [] - backend = root / "backend" - if not backend.is_dir(): - return errors - - for pyfile in sorted(backend.rglob("*.py")): - if is_excluded(pyfile, root, excludes): - continue - rel = str(pyfile.relative_to(root)) - if is_excepted(rel, RULE, exceptions): - continue - if ignores and is_lintignored(pyfile, root, RULE, ignores): - continue - try: - tree = ast.parse(pyfile.read_text(), filename=rel) - except (OSError, SyntaxError): - continue - errors.extend(_check_tree(tree, rel)) - - return errors diff --git a/linter/checks/p_private.py b/linter/checks/p_private.py deleted file mode 100644 index 6f60be78..00000000 --- a/linter/checks/p_private.py +++ /dev/null @@ -1,242 +0,0 @@ -"""Cross-file/class privacy for the ``p_`` naming convention (Java ``private``). - -A name prefixed with ``p_`` (or ``P_`` -- the leading p is case-insensitive, so -UPPER_SNAKE constants like ``P_SECRET`` count too) is private to the scope that -owns it, enforced the way Java enforces ``private``: - - * **Module-level** ``p_`` symbols (top-level ``def`` / ``class`` / assignment) - are **file-private** -- usable anywhere in their own file, nowhere else. - * **Class members** (``def p_m``, ``self.p_x = ...``, and class-body fields - ``p_x: T``) are **class-private** -- any ``recv.p_x`` is legal only when it - appears lexically inside the owning class (or a class nested within it), - matching Java's type-scoped ``private`` while side-stepping type inference. - -Strict: a subclass in another file reaching a base's ``p_`` member is a -violation (Java ``private``, not ``protected``). Nested/inner classes may reach -the enclosing class's members and vice versa (the whole enclosing-class stack is -checked). No exemptions -- tests and ``__init__.py`` re-exports are enforced. - -How access is detected (no type inference needed): Python reaches class members -only through attribute access (``self.p_x`` / ``obj.p_x``) and module-level -symbols only by bare name (legal in-file) or ``module.p_x`` / ``from m import -p_x`` across files. So the access *form* selects the scoping rule: - - * ``recv.p_x`` (attribute load) -> class rule, else module rule - * ``from m import p_x`` -> module rule (cross-file import = leak) - -Two passes: phase 1 records ownership, phase 2 checks references against it. -Scoped to ``backend/`` Python, like checks/classes.py. -""" - -from __future__ import annotations - -import ast -import re -from pathlib import Path - -from . import is_excepted, is_excluded, is_lintignored - -RULE = "p-private" - -# Inline suppression, mirroring vulture's: `# p-private-ignore` (bare) silences -# any p-private finding on the line; `# p-private-ignore: p_x, p_y` only silences -# when the finding names one of the listed symbols (the noqa-with-codes form). -# The comment goes on the *reference* line (where the error is reported), e.g. -# `legacy.p_state # p-private-ignore: p_state`. -_INLINE_IGNORE_RE = re.compile(r"#\s*p-private-ignore\b(?::\s*(?P.*))?") - -# name -> set of relative file paths that define it at module level -ModuleOwners = dict[str, set[str]] -# name -> set of (relative file path, qualified class name) that define it as a member -ClassOwners = dict[str, set[tuple[str, str]]] - - -def _is_p(name: str) -> bool: - # Case-insensitive on the leading p so UPPER_SNAKE constants (P_SECRET) count - # as private too, not just lowercase p_ functions/vars. - return len(name) > 2 and name[:2] in ("p_", "P_") - - -# --------------------------------------------------------------------------- # -# Phase 1: ownership collection # -# --------------------------------------------------------------------------- # - -def _own(name: str, rel: str, stack: list[str], container: str, mp: ModuleOwners, cp: ClassOwners) -> None: - if container == "class": - cp.setdefault(name, set()).add((rel, stack[-1])) - elif container == "module": - mp.setdefault(name, set()).add(rel) - # container == "function" -> local definition, not externally reachable - - -def _own_target(t: ast.AST, rel: str, stack: list[str], container: str, mp: ModuleOwners, cp: ClassOwners) -> None: - if isinstance(t, ast.Name) and _is_p(t.id): - _own(t.id, rel, stack, container, mp, cp) - elif isinstance(t, ast.Attribute) and _is_p(t.attr): - # self._x = ... / cls._x = ...: owned by the lexically enclosing class. - if stack: - cp.setdefault(t.attr, set()).add((rel, stack[-1])) - elif isinstance(t, ast.Starred): - _own_target(t.value, rel, stack, container, mp, cp) - elif isinstance(t, (ast.Tuple, ast.List)): - for elt in t.elts: - _own_target(elt, rel, stack, container, mp, cp) - - -def _collect(node: ast.AST, rel: str, stack: list[str], container: str, mp: ModuleOwners, cp: ClassOwners) -> None: - if isinstance(node, ast.ClassDef): - if _is_p(node.name): - _own(node.name, rel, stack, container, mp, cp) - qual = f"{stack[-1]}.{node.name}" if stack else node.name - for child in node.body: - _collect(child, rel, [*stack, qual], "class", mp, cp) - return - if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): - if _is_p(node.name): - _own(node.name, rel, stack, container, mp, cp) - for child in node.body: - _collect(child, rel, stack, "function", mp, cp) - return - if isinstance(node, (ast.Assign, ast.AnnAssign)): - targets = node.targets if isinstance(node, ast.Assign) else [node.target] - for t in targets: - _own_target(t, rel, stack, container, mp, cp) - return - # Plain compound statements (if/for/while/with/try/...) keep the container so - # a conditionally-defined member is still attributed to its real scope. - for child in ast.iter_child_nodes(node): - _collect(child, rel, stack, container, mp, cp) - - -# --------------------------------------------------------------------------- # -# Phase 2: reference checking # -# --------------------------------------------------------------------------- # - -def _inline_ignored(lines: tuple[str, ...], lineno: int, name: str) -> bool: - """True when the reported line carries a matching ``# p-private-ignore``.""" - if not (1 <= lineno <= len(lines)): - return False - m = _INLINE_IGNORE_RE.search(lines[lineno - 1]) - if not m: - return False - names = m.group("names") - if not names: - return True # bare form silences any p-private finding on the line - wanted = {n.strip() for n in names.replace(",", " ").split() if n.strip()} - return name in wanted - - -def _emit( - out: list[str], seen: set[tuple[int, int, str]], rel: str, node: ast.AST, - name: str, msg: str, lines: tuple[str, ...], -) -> None: - if _inline_ignored(lines, node.lineno, name): - return - key = (node.lineno, node.col_offset, msg) - if key in seen: - return - seen.add(key) - out.append(f"{rel}:{node.lineno}:{node.col_offset + 1}: error: [{RULE}] {msg}") - - -def _check_attr( - node: ast.Attribute, rel: str, stack: list[str], - mp: ModuleOwners, cp: ClassOwners, out: list[str], seen: set[tuple[int, int, str]], - lines: tuple[str, ...], -) -> None: - name = node.attr - if name in cp: - if cp[name] & {(rel, q) for q in stack}: - return # lexically inside an owning class -> legal - owners = ", ".join(sorted(f"{q} ({r})" for r, q in cp[name])) - _emit(out, seen, rel, node, name, f"class-private '{name}' accessed outside its class (owner: {owners})", lines) - return - if name in mp: - if rel in mp[name]: - return - owners = ", ".join(sorted(mp[name])) - _emit(out, seen, rel, node, name, f"module-private '{name}' accessed outside its file (defined in {owners})", lines) - - -def _check_import( - name: str, node: ast.AST, rel: str, - mp: ModuleOwners, out: list[str], seen: set[tuple[int, int, str]], - lines: tuple[str, ...], -) -> None: - if name in mp and rel not in mp[name]: - owners = ", ".join(sorted(mp[name])) - _emit(out, seen, rel, node, name, f"module-private '{name}' imported outside its file (defined in {owners})", lines) - - -def _check_refs( - node: ast.AST, rel: str, stack: list[str], - mp: ModuleOwners, cp: ClassOwners, out: list[str], seen: set[tuple[int, int, str]], - lines: tuple[str, ...], -) -> None: - if isinstance(node, ast.ClassDef): - qual = f"{stack[-1]}.{node.name}" if stack else node.name - # Decorators/bases are evaluated in the enclosing scope, not inside the class. - for outer in (*node.decorator_list, *node.bases, *node.keywords): - _check_refs(outer, rel, stack, mp, cp, out, seen, lines) - for child in node.body: - _check_refs(child, rel, [*stack, qual], mp, cp, out, seen, lines) - return - if isinstance(node, ast.Attribute) and isinstance(node.ctx, ast.Load) and _is_p(node.attr): - _check_attr(node, rel, stack, mp, cp, out, seen, lines) - elif isinstance(node, ast.ImportFrom): - for alias in node.names: - if alias.name != "*" and _is_p(alias.name): - _check_import(alias.name, node, rel, mp, out, seen, lines) - # A function does not open a new *class* scope, so the class stack is carried - # through unchanged; that's why generic descent (not an early return) is right. - for child in ast.iter_child_nodes(node): - _check_refs(child, rel, stack, mp, cp, out, seen, lines) - - -# --------------------------------------------------------------------------- # -# Runner # -# --------------------------------------------------------------------------- # - -def run_p_private_check( - root: Path, - exceptions: dict[str, list[str]], - excludes: list[str], - ignores: dict[Path, set[str]] | None = None, -) -> list[str]: - """Flag ``p_`` symbols accessed outside their owning file/class.""" - backend = root / "backend" - if not backend.is_dir(): - return [] - - # Parse once; phase 1 must see *every* file (even exempted ones) so ownership - # is known when a non-exempt file reaches into them. The source lines are kept - # so phase 2 can honor inline `# p-private-ignore` comments. - trees: list[tuple[Path, str, ast.AST, tuple[str, ...]]] = [] - mp: ModuleOwners = {} - cp: ClassOwners = {} - for pyfile in sorted(backend.rglob("*.py")): - if is_excluded(pyfile, root, excludes): - continue - rel = str(pyfile.relative_to(root)) - try: - source = pyfile.read_text() - tree = ast.parse(source, filename=rel) - except (OSError, SyntaxError): - continue - trees.append((pyfile, rel, tree, tuple(source.splitlines()))) - for child in ast.iter_child_nodes(tree): - _collect(child, rel, [], "module", mp, cp) - - # Phase 2 reports only for files that aren't exempted/lintignored. - errors: list[str] = [] - for pyfile, rel, tree, lines in trees: - if is_excepted(rel, RULE, exceptions): - continue - if ignores and is_lintignored(pyfile, root, RULE, ignores): - continue - seen: set[tuple[int, int, str]] = set() - out: list[str] = [] - _check_refs(tree, rel, [], mp, cp, out, seen, lines) - errors.extend(out) - - return errors