mirror of
https://github.com/kuhyx/diet-guard.git
synced 2026-11-10 19:27:14 +01:00
Enforce the shared 250-line file cap
Replace this repo's stale file-length hook with the shared gate every other
repo will use. The old hook capped at 500 lines, called a repo-local
scripts/check_file_length.py, and filtered to types_or: [python, shell] --
so it never looked at the Flutter app in app/ or at any prose. Two caps that
disagree is worse than one, so the old one is deleted rather than kept.
The new hook checks only the files in the commit and exits 1; CI runs --all
as the backstop, since a fresh clone that never ran `pre-commit install` has
no hooks at all. The exemption list lives with the checker in
~/utils/file_length/config.py so the gate and the cross-repo survey cannot
disagree about what "too long" means.
Two things this repo has that ~/todo (the reference implementation) did not:
- A pre-commit CI job running --all-files, where $HOME/utils does not exist.
The shared gate is checked out to .utils and reached via UTILS_ROOT, or the
hook fails on a missing checker rather than on anything in this repo.
- Tests that isolate live user state with dotted-string patches
(`patch("diet_guard._state.FOOD_LOG_FILE", ...)`). unittest.mock.patch fails
OPEN: once a refactor moves a name, the old path still imports, the patch
redirects nothing, the test still passes -- and the suite writes to the real
~/.local/share/diet_guard, which diet-guard-sync.timer publishes within 15
minutes. check_patch_targets.py resolves all 30 patch targets statically so
that failure is loud instead of silent. It is landed before any split for
exactly that reason.
The gate is red at 52 files; the splits that follow drive it to zero.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MK3HCZrM75sRHL1gL8usve
This commit is contained in:
parent
7a491d9d91
commit
c7538f691b
33
.github/workflows/file-length.yml
vendored
Normal file
33
.github/workflows/file-length.yml
vendored
Normal file
@ -0,0 +1,33 @@
|
|||||||
|
name: file-length
|
||||||
|
|
||||||
|
# The 250-line cap, enforced on push as well as at commit time. The local
|
||||||
|
# pre-commit hook can be skipped (a fresh clone that never ran `pre-commit
|
||||||
|
# install` has no hooks at all), so the cap needs a second gate that no one
|
||||||
|
# can forget to install.
|
||||||
|
#
|
||||||
|
# Deliberately a SEPARATE workflow from release-apk.yml: a file that grew too
|
||||||
|
# long should fail lint, not block publishing an APK.
|
||||||
|
on:
|
||||||
|
push:
|
||||||
|
branches: [main]
|
||||||
|
pull_request:
|
||||||
|
workflow_dispatch:
|
||||||
|
|
||||||
|
jobs:
|
||||||
|
file-length:
|
||||||
|
name: file length <= 250 lines
|
||||||
|
runs-on: ubuntu-latest
|
||||||
|
steps:
|
||||||
|
- uses: actions/checkout@v4
|
||||||
|
|
||||||
|
# The gate and its exemption list live in the shared utils repo so every
|
||||||
|
# repo enforces the same cap. Checking it out beats vendoring a copy
|
||||||
|
# here, which is exactly how a gate and a survey start disagreeing.
|
||||||
|
- name: Check out the shared gate
|
||||||
|
uses: actions/checkout@v4
|
||||||
|
with:
|
||||||
|
repository: kuhyx/utils
|
||||||
|
path: .utils
|
||||||
|
|
||||||
|
- name: Enforce the cap
|
||||||
|
run: bash .utils/scripts/check_file_length.sh --all
|
||||||
14
.github/workflows/pre-commit.yml
vendored
14
.github/workflows/pre-commit.yml
vendored
@ -16,4 +16,18 @@ jobs:
|
|||||||
python-version: "3.12"
|
python-version: "3.12"
|
||||||
- name: Install dependencies
|
- name: Install dependencies
|
||||||
run: pip install -r requirements.txt
|
run: pip install -r requirements.txt
|
||||||
|
|
||||||
|
# The file-length hook delegates to the shared gate in kuhyx/utils, which
|
||||||
|
# a runner does not have at $HOME/utils. Check it out and point the shim
|
||||||
|
# at it via UTILS_ROOT, or `pre-commit run --all-files` fails on a
|
||||||
|
# missing checker rather than on anything in this repo. `.utils` is in
|
||||||
|
# the gate's own EXCLUDED_DIRS, so it does not lint itself.
|
||||||
|
- name: Check out the shared gate
|
||||||
|
uses: actions/checkout@v4
|
||||||
|
with:
|
||||||
|
repository: kuhyx/utils
|
||||||
|
path: .utils
|
||||||
|
|
||||||
- uses: pre-commit/action@v3.0.1
|
- uses: pre-commit/action@v3.0.1
|
||||||
|
env:
|
||||||
|
UTILS_ROOT: ${{ github.workspace }}/.utils
|
||||||
|
|||||||
@ -27,7 +27,7 @@ repos:
|
|||||||
- id: name-tests-test
|
- id: name-tests-test
|
||||||
args: [--pytest-test-first]
|
args: [--pytest-test-first]
|
||||||
# Shared fake widgets, not a test module. Split out of conftest.py only
|
# Shared fake widgets, not a test module. Split out of conftest.py only
|
||||||
# because that file passed the 500-line cap.
|
# because that file passed the 250-line cap.
|
||||||
exclude: ^diet_guard/tests/_(tk_fakes|gate_layout_probe|gate_keyboard_probe)\.py$
|
exclude: ^diet_guard/tests/_(tk_fakes|gate_layout_probe|gate_keyboard_probe)\.py$
|
||||||
- id: check-ast
|
- id: check-ast
|
||||||
- id: mixed-line-ending
|
- id: mixed-line-ending
|
||||||
@ -170,8 +170,28 @@ repos:
|
|||||||
# shebang (SC2148) and its variables are read by makepkg rather than by
|
# shebang (SC2148) and its variables are read by makepkg rather than by
|
||||||
# the file itself (SC2034). namcap is what actually lints these.
|
# the file itself (SC2034). namcap is what actually lints these.
|
||||||
exclude: '^packaging/aur/PKGBUILD$'
|
exclude: '^packaging/aur/PKGBUILD$'
|
||||||
- id: max-file-length
|
# The shared 250-line cap. Checks only the files in the commit, so an
|
||||||
name: Max file length (500 lines)
|
# unrelated commit never breaks; exits 1 rather than warning. No
|
||||||
entry: python3 scripts/check_file_length.py
|
# baseline and no allowlist -- those are suppressions. The exemption
|
||||||
|
# list (generated/vendored/data files) lives with the checker in
|
||||||
|
# ~/utils/file_length/config.py so the gate and the survey cannot
|
||||||
|
# disagree.
|
||||||
|
#
|
||||||
|
# Deliberately unfiltered: no types_or and no files. The cap applies to
|
||||||
|
# Dart and prose too, and a type filter here is what would silently
|
||||||
|
# exempt the whole Flutter app and every .md.
|
||||||
|
- id: file-length
|
||||||
|
name: file length <= 250 lines
|
||||||
|
entry: scripts/check_file_length.sh
|
||||||
language: system
|
language: system
|
||||||
types_or: [python, shell]
|
|
||||||
|
# unittest.mock.patch fails OPEN: once a refactor moves a name, a patch
|
||||||
|
# target that still imports redirects nothing, the test passes, and the
|
||||||
|
# suite writes to the real ~/.local/share/diet_guard. This resolves every
|
||||||
|
# patch target statically so a stale path fails the commit instead.
|
||||||
|
- id: patch-targets
|
||||||
|
name: test patch targets resolve
|
||||||
|
entry: python3 scripts/check_patch_targets.py
|
||||||
|
language: system
|
||||||
|
pass_filenames: false
|
||||||
|
files: ^diet_guard/
|
||||||
|
|||||||
@ -2,7 +2,7 @@ name: diet_guard_app
|
|||||||
description: "Companion phone app for diet_guard: log meals on the go."
|
description: "Companion phone app for diet_guard: log meals on the go."
|
||||||
publish_to: 'none'
|
publish_to: 'none'
|
||||||
|
|
||||||
version: 1.0.11+1
|
version: 1.0.12+1
|
||||||
|
|
||||||
environment:
|
environment:
|
||||||
sdk: ^3.12.2
|
sdk: ^3.12.2
|
||||||
|
|||||||
@ -1,26 +0,0 @@
|
|||||||
#!/usr/bin/env python3
|
|
||||||
"""Pre-commit hook: fail if any file exceeds MAX_LINES lines."""
|
|
||||||
|
|
||||||
from pathlib import Path
|
|
||||||
import sys
|
|
||||||
|
|
||||||
MAX_LINES = 500
|
|
||||||
|
|
||||||
|
|
||||||
def main() -> int:
|
|
||||||
"""Return 1 if any file exceeds the line limit, else 0."""
|
|
||||||
failed = False
|
|
||||||
for filepath in sys.argv[1:]:
|
|
||||||
try:
|
|
||||||
with Path(filepath).open(encoding="utf-8", errors="replace") as fh:
|
|
||||||
count = sum(1 for _ in fh)
|
|
||||||
except OSError:
|
|
||||||
failed = True
|
|
||||||
continue
|
|
||||||
if count > MAX_LINES:
|
|
||||||
failed = True
|
|
||||||
return 1 if failed else 0
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
|
||||||
sys.exit(main())
|
|
||||||
31
scripts/check_file_length.sh
Executable file
31
scripts/check_file_length.sh
Executable file
@ -0,0 +1,31 @@
|
|||||||
|
#!/bin/bash
|
||||||
|
|
||||||
|
# ============================================================================
|
||||||
|
# Fail if any file in the commit exceeds the shared 250-line cap.
|
||||||
|
#
|
||||||
|
# Thin delegate to the shared gate in ~/utils, which owns the cap and the
|
||||||
|
# exemption list (generated / vendored / data files). Copying that logic here
|
||||||
|
# is what lets one repo's idea of "too long" drift from every other repo's --
|
||||||
|
# so this script only locates the shared checker and forwards its arguments.
|
||||||
|
#
|
||||||
|
# Usage:
|
||||||
|
# scripts/check_file_length.sh <file> [<file> ...] # pre-commit passes these
|
||||||
|
# scripts/check_file_length.sh --all # whole tree, from cwd
|
||||||
|
# ============================================================================
|
||||||
|
|
||||||
|
set -euo pipefail
|
||||||
|
|
||||||
|
readonly SHARED_GATE="${UTILS_ROOT:-$HOME/utils}/scripts/check_file_length.sh"
|
||||||
|
|
||||||
|
main() {
|
||||||
|
if [[ ! -x "$SHARED_GATE" ]]; then
|
||||||
|
echo "Error: shared file-length gate not found at $SHARED_GATE" >&2
|
||||||
|
echo " Clone github.com/kuhyx/utils to ~/utils, or set" >&2
|
||||||
|
echo " UTILS_ROOT to where it lives." >&2
|
||||||
|
exit 1
|
||||||
|
fi
|
||||||
|
|
||||||
|
exec bash "$SHARED_GATE" "$@"
|
||||||
|
}
|
||||||
|
|
||||||
|
main "$@"
|
||||||
101
scripts/check_patch_targets.py
Executable file
101
scripts/check_patch_targets.py
Executable file
@ -0,0 +1,101 @@
|
|||||||
|
#!/usr/bin/env python3
|
||||||
|
"""Fail if a test patches a dotted path that no longer resolves.
|
||||||
|
|
||||||
|
``unittest.mock.patch("diet_guard._state.FOOD_LOG_FILE", ...)`` fails *open*.
|
||||||
|
Once a refactor moves ``FOOD_LOG_FILE`` to another module, the old path still
|
||||||
|
imports, the patch redirects nothing, and the test keeps passing -- while the
|
||||||
|
suite writes to the real ``~/.local/share/diet_guard``, which
|
||||||
|
``diet-guard-sync.timer`` then publishes to every other device.
|
||||||
|
|
||||||
|
That is why this gate exists rather than a note in CLAUDE.md: the failure it
|
||||||
|
guards against is silent and green, so only a static check catches it.
|
||||||
|
|
||||||
|
Usage:
|
||||||
|
python3 scripts/check_patch_targets.py
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import ast
|
||||||
|
import importlib
|
||||||
|
import pathlib
|
||||||
|
import sys
|
||||||
|
from typing import TYPE_CHECKING
|
||||||
|
|
||||||
|
if TYPE_CHECKING:
|
||||||
|
from collections.abc import Iterator
|
||||||
|
|
||||||
|
#: Only paths into this package are checked; a patch of `requests` or `tk` is
|
||||||
|
#: not ours to validate.
|
||||||
|
PACKAGE = "diet_guard"
|
||||||
|
|
||||||
|
TESTS_ROOT = pathlib.Path(PACKAGE) / "tests"
|
||||||
|
|
||||||
|
|
||||||
|
def patch_targets(root: pathlib.Path) -> Iterator[tuple[pathlib.Path, int, str]]:
|
||||||
|
"""Yield ``(file, lineno, dotted_target)`` for every literal patch target."""
|
||||||
|
for path in sorted(root.rglob("*.py")):
|
||||||
|
tree = ast.parse(path.read_text(), str(path))
|
||||||
|
for node in ast.walk(tree):
|
||||||
|
if not isinstance(node, ast.Call):
|
||||||
|
continue
|
||||||
|
name = getattr(node.func, "attr", None) or getattr(node.func, "id", None)
|
||||||
|
if name != "patch" or not node.args:
|
||||||
|
continue
|
||||||
|
first = node.args[0]
|
||||||
|
if (
|
||||||
|
isinstance(first, ast.Constant)
|
||||||
|
and isinstance(first.value, str)
|
||||||
|
and first.value.startswith(PACKAGE)
|
||||||
|
):
|
||||||
|
yield path, node.lineno, first.value
|
||||||
|
|
||||||
|
|
||||||
|
def resolves(dotted: str) -> bool:
|
||||||
|
"""Whether ``dotted`` names a real attribute reachable from a real module.
|
||||||
|
|
||||||
|
Tries the longest importable module prefix first, then walks the remaining
|
||||||
|
segments as attributes -- the same resolution order ``mock.patch`` uses.
|
||||||
|
"""
|
||||||
|
parts = dotted.split(".")
|
||||||
|
for split in range(len(parts), 0, -1):
|
||||||
|
try:
|
||||||
|
module = importlib.import_module(".".join(parts[:split]))
|
||||||
|
except ImportError:
|
||||||
|
continue
|
||||||
|
obj: object = module
|
||||||
|
for attr in parts[split:]:
|
||||||
|
if not hasattr(obj, attr):
|
||||||
|
return False
|
||||||
|
obj = getattr(obj, attr)
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def main() -> int:
|
||||||
|
"""Report every unresolvable patch target; return 1 if any were found."""
|
||||||
|
if not TESTS_ROOT.is_dir():
|
||||||
|
sys.stderr.write(f"Error: {TESTS_ROOT} not found; run from the repo root.\n")
|
||||||
|
return 1
|
||||||
|
|
||||||
|
stale = [
|
||||||
|
(path, lineno, target)
|
||||||
|
for path, lineno, target in patch_targets(TESTS_ROOT)
|
||||||
|
if not resolves(target)
|
||||||
|
]
|
||||||
|
for path, lineno, target in stale:
|
||||||
|
sys.stderr.write(f"{path}:{lineno}: patch target does not resolve: {target}\n")
|
||||||
|
|
||||||
|
if stale:
|
||||||
|
plural = "" if len(stale) == 1 else "s"
|
||||||
|
sys.stderr.write(
|
||||||
|
f"\n{len(stale)} stale patch target{plural}. A patch that does not "
|
||||||
|
f"resolve silently redirects nothing -- the test passes while writing "
|
||||||
|
f"to real user state. Update the path to where the name now lives.\n"
|
||||||
|
)
|
||||||
|
return 1
|
||||||
|
return 0
|
||||||
|
|
||||||
|
|
||||||
|
if __name__ == "__main__":
|
||||||
|
raise SystemExit(main())
|
||||||
Loading…
Reference in New Issue
Block a user