diff --git a/.github/workflows/full_test.yml b/.github/workflows/full_test.yml index 341923a71..550449d88 100644 --- a/.github/workflows/full_test.yml +++ b/.github/workflows/full_test.yml @@ -14,10 +14,20 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - - uses: astral-sh/ruff-action@v2 + + - uses: extractions/setup-just@v3 + + - name: Set up uv + uses: astral-sh/setup-uv@v6 with: - version: 0.6.2 - src: src + python-version: "3.12" + enable-cache: true + + - name: Lint + run: just lint + + - name: Check for private attribute access on third-party objects + run: just private-access build: runs-on: ubuntu-latest diff --git a/justfile b/justfile index d027887a5..68aa46b22 100644 --- a/justfile +++ b/justfile @@ -1,7 +1,7 @@ set windows-shell := ["powershell.exe", "-NoLogo", "-Command"] -# Run all checks: lint, typecheck, test, doctest -check: lint typecheck test doctest +# Run all checks: lint, private-access, typecheck, test, doctest +check: lint private-access typecheck test doctest # Build package (after typecheck and test) build: typecheck test @@ -9,16 +9,20 @@ build: typecheck test # Lint with ruff lint: - uv run ruff check src + uv run ruff check src tools # Auto-fix formatting format: - uv run ruff format src + uv run ruff format src tools # Run tests test: uv run pytest --disable-warnings +# Report private attribute access on third-party objects +private-access: + uv run python tools/check_third_party_private_access.py + # Type check with mypy typecheck: uv run mypy src/ --config-file pyproject.toml diff --git a/pyproject.toml b/pyproject.toml index 890fc7574..ed7a9e936 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -71,10 +71,17 @@ networks = ["mikeio1d>=1.2.1", "networkx"] [tool.ruff] extend-exclude = ["notebooks"] +# PLC2701 (no private imports from other packages) is still a preview rule +preview = true [tool.ruff.lint] ignore = ["E501"] -select = ["E4", "E7", "E9", "F", "D200", "D205"] +select = ["E4", "E7", "E9", "F", "D200", "D205", "PLC2701"] + +[tool.ruff.lint.per-file-ignores] +# Tests import modelskill by absolute path, so its own private modules count as +# "external" to this rule. Reaching into them from tests is a separate problem. +"tests/**" = ["PLC2701"] [tool.mypy] python_version = "3.12" diff --git a/tools/check_third_party_private_access.py b/tools/check_third_party_private_access.py new file mode 100644 index 000000000..d687d6704 --- /dev/null +++ b/tools/check_third_party_private_access.py @@ -0,0 +1,123 @@ +"""Report access to private attributes on objects from other packages. + +ModelSkill reaches into its own private attributes freely -- that is ordinary +intra-package access. Reaching into a *third-party* object's private attribute +(for example ``mikeio_dataset._zn``) is different: nothing stops the other +package from renaming or removing it in a patch release, and nothing in CI +would notice until a user hits it. + +Ruff's ``SLF001`` finds every private attribute access but cannot tell whose +object it is, because it does not infer types. This script narrows ``SLF001`` +by name: a member that modelskill defines somewhere in ``src/modelskill`` is +assumed to be ours; anything else is assumed to belong to another package. + +That is a heuristic, not type inference. It misses an access whose attribute +name we happen to use ourselves, and it can misreport an attribute we only ever +read (never assign) as third-party. It is meant to make these accesses visible, +not to prove their absence. + +Run with ``just private-access``. Exits non-zero if anything is reported. +""" + +from __future__ import annotations + +import ast +import json +import subprocess +import sys +from pathlib import Path + +SRC = Path(__file__).resolve().parent.parent / "src" / "modelskill" + + +def own_names(src: Path) -> set[str]: + """Collect every attribute or name modelskill itself defines. + + Parameters + ---------- + src : Path + Root of the modelskill package. + + Returns + ------- + set of str + Names bound anywhere in the package: functions, classes, assigned + attributes, class-body and annotated assignments, and arguments. + """ + names: set[str] = set() + for path in sorted(src.rglob("*.py")): + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + for node in ast.walk(tree): + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef, ast.ClassDef)): + names.add(node.name) + elif isinstance(node, ast.arg): + names.add(node.arg) + continue + + if isinstance(node, ast.Assign): + targets: list[ast.expr] = list(node.targets) + elif isinstance(node, (ast.AnnAssign, ast.AugAssign)): + targets = [node.target] + else: + continue + for target in targets: + if isinstance(target, ast.Name): + names.add(target.id) + elif isinstance(target, ast.Attribute): + names.add(target.attr) + return names + + +def private_accesses(src: Path) -> list[dict]: + """Run ruff's SLF001 over the package and return its findings as JSON.""" + result = subprocess.run( + [ + "ruff", + "check", + "--no-cache", + "--isolated", + "--select", + "SLF001", + "--output-format", + "json", + str(src), + ], + capture_output=True, + text=True, + ) + if result.returncode not in (0, 1): + sys.exit(f"ruff failed:\n{result.stderr}") + return json.loads(result.stdout or "[]") + + +def main() -> int: + ours = own_names(SRC) + root = SRC.parent.parent + + reported = [] + for hit in private_accesses(SRC): + member = hit["message"].split("`")[1] + if member in ours: + continue + path = Path(hit["filename"]) + if path.is_relative_to(root): + path = path.relative_to(root) + reported.append((path, hit["location"]["row"], member)) + + if not reported: + print("No private attribute access on third-party objects.") + return 0 + + print("Private attribute access on objects from other packages:\n") + for path, row, member in reported: + print(f" {path}:{row}: {member}") + print( + "\nThese attributes are not part of any public API and can disappear " + "without a deprecation. Ask the upstream package for a public accessor, " + "or add the name to the docstring's list of known exceptions." + ) + return 1 + + +if __name__ == "__main__": + raise SystemExit(main())