diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a02cf585f..502995322 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -613,6 +613,19 @@ jobs: run: python3 utils/check-diagnostic-prefix.py - name: check-diagnostic-prefix self-tests (explicit) run: python3 -m unittest -q utils/check-diagnostic-prefix-test.py + # Defensive twin for the safety-doc pin gate (#1057): the + # module doc of big-code-analysis-py/src/node.rs is the + # canonical soundness argument for this workspace's only + # sanctioned `unsafe` block, and it reasons about a named + # tree-sitter release. A bump that leaves the literal behind + # leaves an argument that reads as verified against a crate + # nobody compiles. Its self-tests run as their own step for the + # usual reason: a source-scanning gate that stops matching + # reports a clean tree. + - name: check-safety-doc-pin (explicit) + run: python3 utils/check-safety-doc-pin.py + - name: check-safety-doc-pin self-tests (explicit) + run: python3 -m unittest -q utils/check-safety-doc-pin-test.py # Defensive twin for the grammar-marker-sync gate (#400): bumping # the notification-only marker in tree-sitter-{mozjs,mozcpp}/ # Cargo.toml without re-running the matching generate-*.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6fac50cbd..bf3287928 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -411,6 +411,31 @@ repos: entry: python3 -m unittest -q utils/check-diagnostic-prefix-test.py pass_filenames: false + # Safety-doc pin gate — the module doc of + # big-code-analysis-py/src/node.rs is the canonical soundness + # argument for this workspace's only sanctioned `unsafe` block, + # and it reasons about a named tree-sitter release. #1057: the + # pin moved twice while the literal stayed at `=0.26.9`, leaving + # an argument that read as verified against a crate nobody was + # compiling. Fires on the root manifest as well as the file, so + # a bump cannot land without the citation being re-checked. + - id: check-safety-doc-pin + name: check-safety-doc-pin + language: system + files: '^(Cargo\.toml|big-code-analysis-py/src/node\.rs|utils/check-safety-doc-pin\.py)$' + entry: python3 utils/check-safety-doc-pin.py + pass_filenames: false + + # Self-tests for the safety-doc-pin gate, for the same reason as + # the diagnostic-prefix pair above: a source-scanning gate that + # stops matching reports a clean tree. + - id: check-safety-doc-pin-test + name: check-safety-doc-pin-test + language: system + files: '^utils/check-safety-doc-pin(-test)?\.py$' + entry: python3 -m unittest -q utils/check-safety-doc-pin-test.py + pass_filenames: false + # The `enums/` crate is workspace-excluded, so the workspace # clippy/test hooks above never touch it. Add a dedicated check # so warnings here cannot drift (see #164). diff --git a/AGENTS.md b/AGENTS.md index fd0b7e5f5..6f7c3441f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -62,7 +62,7 @@ and `cargo run -p big-code-analysis-web --`. by `make pre-commit` / `make ci`: `check-versions.py`, `check-snapshot-anchors.py`, `check-rustfmt-bail.py`, `check-manpage-assets.py`, `check-manpage-drift.py`, - `check-diagnostic-prefix.py`, + `check-diagnostic-prefix.py`, `check-safety-doc-pin.py`, `check-grammar-marker-sync.py`, `check-enums-codegen-drift.sh`, `check-grammar-crate.py`, `check-grammars-crates.sh`, `check-excluded-manifests.py`, `check-ruff-lockstep.py`, @@ -200,7 +200,12 @@ and `cargo run -p big-code-analysis-web --`. keeping the owning tree alive through a strong `Py<...>` handle. The canonical soundness argument lives at `big-code-analysis-py/src/node.rs` (the `# Safety` module doc and - `detach`). Any `unsafe` outside this exact pattern remains banned and + `detach`), and the node/`Ast` pairing that argument depends on is + enforced by the `owned` module boundary there — private fields, so a + handle can only be built by `wrap` / `rewrap` / `rooted_at` and not by + a struct literal that pairs a node with the wrong tree (#1057). The + version the doc names is gated by `make check-safety-doc-pin`. Any + `unsafe` outside this exact pattern remains banned and needs a deliberate amendment to this rule. (The PyO3-macro-generated FFI shims under `#![allow(unsafe_op_in_unsafe_fn)]` in `src/lib.rs` are source-level `unsafe`-free and not covered by this exception.) @@ -274,7 +279,12 @@ modified, deleted, **and** newly added pages, the last of which `git diff` alone cannot see, #1249), the diagnostic-prefix gate (`make check-diagnostic-prefix`, which blocks a capitalised `Warning:` / `Error:` / `Note:` string literal — see "Rust -conventions"), the bca self-scan threshold gate at both +conventions"), the safety-doc pin gate +(`make check-safety-doc-pin`, which fails when the `tree-sitter` +version cited by the `unsafe` soundness argument in +`big-code-analysis-py/src/node.rs` is not the version +`[workspace.dependencies]` pins, or when the citation is dropped +altogether — #1057), the bca self-scan threshold gate at both tiers (`make self-scan` mirroring the `Threshold gate` step in `.github/workflows/pages.yml`, plus `make self-scan-headroom` which scales every limit by `BCA_HEADROOM` — default `0.95` — so @@ -722,6 +732,15 @@ pins it at `=0.26.12` with the workspace resolving. Its ABI version is also what each vendored `parser.c` was generated against, so an accidental bump is precisely the drift the gate exists to catch. +A runtime bump also has to carry the `unsafe` soundness argument with +it. `big-code-analysis-py/src/node.rs` reasons about a *named* +tree-sitter release — `Tree(NonNull)`, +`Node<'tree>(ffi::TSNode, PhantomData<&'tree ()>)`, +`Tree::edit(&mut self)`, `Send + Sync` — and `make +check-safety-doc-pin` fails until the literal in that doc matches the +new pin. Re-read the argument against the new release before editing +the line; the forced diff is the prompt, not the fix (#1057). + Treat the pinned version as fixed: - Do not loosen pins to a range without explicit user approval. diff --git a/CHANGELOG.md b/CHANGELOG.md index 72af0e069..b9a1adbfe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -89,6 +89,18 @@ for historical reference. by a comment alone and had already drifted silently once (`v0.15.14` against a lockfile resolving 0.15.22), which stays invisible until the two versions disagree and then presents as "works locally, red in CI" (#1230). +- A `check-safety-doc-pin` gate (`make check-safety-doc-pin`, wired into + `make lint` / `pre-commit` / `ci`, the pre-commit hooks, and its own CI + step) holds the `tree-sitter` version cited by the `unsafe` soundness + argument in `big-code-analysis-py/src/node.rs` equal to the version + `[workspace.dependencies]` pins. That module doc is the canonical + justification for the workspace's only sanctioned `unsafe` block and + reasons about a *named* release — the `Node<'tree>` layout, `Tree::edit` + taking `&mut self`, `Send + Sync` — so a pin that moves while the literal + does not leaves an argument reading as verified against a crate nobody + compiles. The gate also fails when the literal is dropped altogether, + since a version-free phrasing hides the staleness rather than fixing it. + No library behaviour changes (#1057). - **web:** `error_kind` token `vcs_invalid_author_hash_key`. The `STABILITY.md` vocabulary list also gains `not_acceptable` and diff --git a/Makefile b/Makefile index 933e4613f..1c6d4b058 100644 --- a/Makefile +++ b/Makefile @@ -88,7 +88,7 @@ find-by-ext = $(if $(FD),$(FD) --extension $(1) $(FD_EXCLUDE) $(2),find . -name NEXTEST := $(shell command -v cargo-nextest 2>/dev/null) TEST_CMD = $(if $(NEXTEST),$(NEXTEST) nextest run --workspace --all-features,cargo test --workspace --all-features --lib --bins --tests) -.PHONY: help check-tools worktree-setup worktree-setup-test build build-release check test test-doc chain-audit fmt fmt-check markdown-fmt markdown-lint shellcheck sh-fmt sh-fmt-check toml-fmt toml-fmt-check toml-lint makefile-check actionlint snapshot-anchors snapshot-anchors-test rustfmt-bail rustfmt-bail-test grammar-marker-sync grammar-marker-sync-test check-versions check-excluded-manifests check-excluded-manifests-test check-ruff-lockstep check-ruff-lockstep-test check-publish-metadata check-publish-metadata-test check-manpage-assets check-manpage-drift-test check-diagnostic-prefix check-diagnostic-prefix-test gate-status-test check-tools-test enums-check enums-codegen-drift enums-codegen-drift-test self-scan self-scan-headroom self-scan-write-baseline self-scan-write-baseline-headroom vcs lint clippy udeps insta-review insta-accept clean distclean install install-cli install-web doc doc-open doc-check doc-check-docsrs book book-serve book-pot book-po-update book-ja book-deploy all pre-commit ci release-check verify-changelog pkg-deb-local pkg-rpm-local dev-env-build dev-env-run dev-env-shell dev-env-rm py-bootstrap py-sync py-relock py-clean py-fmt py-fmt-check py-lint py-typecheck py-test py-stubtest smoke smoke-cli smoke-lib bench bench-scaling bench-walk fuzz-check fuzz-smoke fuzz-replay fuzz-run fuzz-tmin _check-find _pc-all _pc-fmt _pc-clippy _pc-test _pc-doc-check _pc-udeps _pc-shellcheck _pc-markdown-lint _pc-toml-lint _pc-makefile-check _pc-actionlint _pc-snapshot-anchors _pc-snapshot-anchors-test _pc-rustfmt-bail _pc-rustfmt-bail-test _pc-grammar-marker-sync _pc-grammar-marker-sync-test _pc-check-versions _pc-check-versions-test _pc-check-grammar-crate-test _pc-check-excluded-manifests _pc-check-excluded-manifests-test _pc-check-ruff-lockstep _pc-check-ruff-lockstep-test _pc-check-publish-metadata _pc-check-publish-metadata-test _pc-check-manpage-assets _pc-check-manpage-drift-test _pc-check-diagnostic-prefix _pc-check-diagnostic-prefix-test _pc-worktree-setup-test _pc-gate-status-test _pc-check-tools-test _pc-enums-check _pc-enums-codegen-drift _pc-enums-codegen-drift-test _pc-self-scan _pc-self-scan-headroom _pc-py-fmt _pc-py-typecheck _pc-py-test _pc-py-stubtest _ci-all _ci-fmt-check _ci-clippy _ci-test _ci-doc-check _ci-build _ci-udeps _ci-shellcheck _ci-markdown-lint _ci-toml-lint _ci-makefile-check _ci-actionlint _ci-snapshot-anchors _ci-snapshot-anchors-test _ci-rustfmt-bail _ci-rustfmt-bail-test _ci-grammar-marker-sync _ci-grammar-marker-sync-test _ci-check-versions _ci-check-versions-test _ci-check-grammar-crate-test _ci-check-excluded-manifests _ci-check-excluded-manifests-test _ci-check-ruff-lockstep _ci-check-ruff-lockstep-test _ci-check-publish-metadata _ci-check-publish-metadata-test _ci-check-manpage-assets _ci-check-manpage-drift-test _ci-check-diagnostic-prefix _ci-check-diagnostic-prefix-test _ci-worktree-setup-test _ci-gate-status-test _ci-check-tools-test _ci-enums-check _ci-enums-codegen-drift _ci-enums-codegen-drift-test _ci-enums-codegen-drift-test _ci-self-scan _ci-self-scan-headroom _ci-cargo-pipeline _ci-py-fmt-check _ci-py-lint _ci-py-typecheck _ci-py-test _ci-py-stubtest +.PHONY: help check-tools worktree-setup worktree-setup-test build build-release check test test-doc chain-audit fmt fmt-check markdown-fmt markdown-lint shellcheck sh-fmt sh-fmt-check toml-fmt toml-fmt-check toml-lint makefile-check actionlint snapshot-anchors snapshot-anchors-test rustfmt-bail rustfmt-bail-test grammar-marker-sync grammar-marker-sync-test check-versions check-excluded-manifests check-excluded-manifests-test check-ruff-lockstep check-ruff-lockstep-test check-publish-metadata check-publish-metadata-test check-manpage-assets check-manpage-drift-test check-diagnostic-prefix check-diagnostic-prefix-test check-safety-doc-pin check-safety-doc-pin-test gate-status-test check-tools-test enums-check enums-codegen-drift enums-codegen-drift-test self-scan self-scan-headroom self-scan-write-baseline self-scan-write-baseline-headroom vcs lint clippy udeps insta-review insta-accept clean distclean install install-cli install-web doc doc-open doc-check doc-check-docsrs book book-serve book-pot book-po-update book-ja book-deploy all pre-commit ci release-check verify-changelog pkg-deb-local pkg-rpm-local dev-env-build dev-env-run dev-env-shell dev-env-rm py-bootstrap py-sync py-relock py-clean py-fmt py-fmt-check py-lint py-typecheck py-test py-stubtest smoke smoke-cli smoke-lib bench bench-scaling bench-walk fuzz-check fuzz-smoke fuzz-replay fuzz-run fuzz-tmin _check-find _pc-all _pc-fmt _pc-clippy _pc-test _pc-doc-check _pc-udeps _pc-shellcheck _pc-markdown-lint _pc-toml-lint _pc-makefile-check _pc-actionlint _pc-snapshot-anchors _pc-snapshot-anchors-test _pc-rustfmt-bail _pc-rustfmt-bail-test _pc-grammar-marker-sync _pc-grammar-marker-sync-test _pc-check-versions _pc-check-versions-test _pc-check-grammar-crate-test _pc-check-excluded-manifests _pc-check-excluded-manifests-test _pc-check-ruff-lockstep _pc-check-ruff-lockstep-test _pc-check-publish-metadata _pc-check-publish-metadata-test _pc-check-manpage-assets _pc-check-manpage-drift-test _pc-check-diagnostic-prefix _pc-check-diagnostic-prefix-test _pc-check-safety-doc-pin _pc-check-safety-doc-pin-test _pc-worktree-setup-test _pc-gate-status-test _pc-check-tools-test _pc-enums-check _pc-enums-codegen-drift _pc-enums-codegen-drift-test _pc-self-scan _pc-self-scan-headroom _pc-py-fmt _pc-py-typecheck _pc-py-test _pc-py-stubtest _ci-all _ci-fmt-check _ci-clippy _ci-test _ci-doc-check _ci-build _ci-udeps _ci-shellcheck _ci-markdown-lint _ci-toml-lint _ci-makefile-check _ci-actionlint _ci-snapshot-anchors _ci-snapshot-anchors-test _ci-rustfmt-bail _ci-rustfmt-bail-test _ci-grammar-marker-sync _ci-grammar-marker-sync-test _ci-check-versions _ci-check-versions-test _ci-check-grammar-crate-test _ci-check-excluded-manifests _ci-check-excluded-manifests-test _ci-check-ruff-lockstep _ci-check-ruff-lockstep-test _ci-check-publish-metadata _ci-check-publish-metadata-test _ci-check-manpage-assets _ci-check-manpage-drift-test _ci-check-diagnostic-prefix _ci-check-diagnostic-prefix-test _ci-check-safety-doc-pin _ci-check-safety-doc-pin-test _ci-worktree-setup-test _ci-gate-status-test _ci-check-tools-test _ci-enums-check _ci-enums-codegen-drift _ci-enums-codegen-drift-test _ci-enums-codegen-drift-test _ci-self-scan _ci-self-scan-headroom _ci-cargo-pipeline _ci-py-fmt-check _ci-py-lint _ci-py-typecheck _ci-py-test _ci-py-stubtest # Default target help: @@ -146,6 +146,8 @@ help: @echo " check-manpage-drift-test Self-tests for the man-page drift gate" @echo " check-diagnostic-prefix Block capitalised Warning:/Error:/Note: literals" @echo " check-diagnostic-prefix-test Self-tests for the diagnostic-prefix gate" + @echo " check-safety-doc-pin Assert node.rs's unsafe soundness doc cites the live tree-sitter pin" + @echo " check-safety-doc-pin-test Self-tests for the safety-doc-pin gate" @echo " worktree-setup-test Self-tests for the worktree-setup submodule classifier" @echo " gate-status-test Self-tests for the pre-commit/ci BCA_GATE verdict line" @echo " check-tools-test Self-tests for the check-tools tool probes and hints" @@ -632,6 +634,23 @@ check-diagnostic-prefix-test: @echo "Running check-diagnostic-prefix self-tests..." @(cd $(BASE_DIR) && python3 -m unittest -q utils/check-diagnostic-prefix-test.py) +# Safety-doc pin gate (#1057). The module doc of +# big-code-analysis-py/src/node.rs is the canonical soundness argument +# for this workspace's only sanctioned `unsafe` block, and it reasons +# about a named tree-sitter release. When the pin moves and that literal +# does not, the argument keeps reading as verified while describing a +# crate nobody compiles against. Static lint - no network, no cargo. +check-safety-doc-pin: + @echo "Checking the unsafe soundness doc's tree-sitter citation..." + @python3 $(BASE_DIR)utils/check-safety-doc-pin.py + +# Self-tests for the safety-doc-pin gate. Separate target for the same +# reason as the other gate self-tests: a source-scanning gate that stops +# matching reports a clean tree. +check-safety-doc-pin-test: + @echo "Running check-safety-doc-pin self-tests..." + @(cd $(BASE_DIR) && python3 -m unittest -q utils/check-safety-doc-pin-test.py) + # Sync gate for check-grammar-crate.py's EXTENSIONS table. Re-derives # the grammar -> extension mapping from src/langs.rs `mk_langs!` and # fails if the hand-maintained table has drifted (#869). Static — no @@ -1330,7 +1349,7 @@ lint: $(MAKE) -j --output-sync=target \ _ci-clippy \ _ci-shellcheck _ci-markdown-lint _ci-toml-lint _ci-makefile-check \ - _ci-actionlint _ci-snapshot-anchors _ci-snapshot-anchors-test _ci-rustfmt-bail _ci-rustfmt-bail-test _ci-grammar-marker-sync _ci-grammar-marker-sync-test _ci-check-versions _ci-check-versions-test _ci-check-grammar-crate-test _ci-check-excluded-manifests _ci-check-excluded-manifests-test _ci-check-ruff-lockstep _ci-check-ruff-lockstep-test _ci-check-publish-metadata _ci-check-publish-metadata-test _ci-check-manpage-assets _ci-check-manpage-drift-test _ci-check-diagnostic-prefix _ci-check-diagnostic-prefix-test _ci-worktree-setup-test _ci-gate-status-test _ci-check-tools-test _ci-enums-check _ci-enums-codegen-drift _ci-enums-codegen-drift-test + _ci-actionlint _ci-snapshot-anchors _ci-snapshot-anchors-test _ci-rustfmt-bail _ci-rustfmt-bail-test _ci-grammar-marker-sync _ci-grammar-marker-sync-test _ci-check-versions _ci-check-versions-test _ci-check-grammar-crate-test _ci-check-excluded-manifests _ci-check-excluded-manifests-test _ci-check-ruff-lockstep _ci-check-ruff-lockstep-test _ci-check-publish-metadata _ci-check-publish-metadata-test _ci-check-manpage-assets _ci-check-manpage-drift-test _ci-check-diagnostic-prefix _ci-check-diagnostic-prefix-test _ci-check-safety-doc-pin _ci-check-safety-doc-pin-test _ci-worktree-setup-test _ci-gate-status-test _ci-check-tools-test _ci-enums-check _ci-enums-codegen-drift _ci-enums-codegen-drift-test # --------------------------------------------------------------------------- # Maintenance @@ -1502,7 +1521,7 @@ _pc-all: $(MAKE) -j --output-sync=target \ _pc-test \ _pc-shellcheck _pc-markdown-lint _pc-toml-lint _pc-makefile-check \ - _pc-actionlint _pc-snapshot-anchors _pc-snapshot-anchors-test _pc-rustfmt-bail _pc-rustfmt-bail-test _pc-grammar-marker-sync _pc-grammar-marker-sync-test _pc-check-versions _pc-check-versions-test _pc-check-grammar-crate-test _pc-check-excluded-manifests _pc-check-excluded-manifests-test _pc-check-ruff-lockstep _pc-check-ruff-lockstep-test _pc-check-publish-metadata _pc-check-publish-metadata-test _pc-check-manpage-assets _pc-check-manpage-drift-test _pc-check-diagnostic-prefix _pc-check-diagnostic-prefix-test _pc-worktree-setup-test _pc-gate-status-test _pc-check-tools-test _pc-enums-check _pc-enums-codegen-drift _pc-enums-codegen-drift-test \ + _pc-actionlint _pc-snapshot-anchors _pc-snapshot-anchors-test _pc-rustfmt-bail _pc-rustfmt-bail-test _pc-grammar-marker-sync _pc-grammar-marker-sync-test _pc-check-versions _pc-check-versions-test _pc-check-grammar-crate-test _pc-check-excluded-manifests _pc-check-excluded-manifests-test _pc-check-ruff-lockstep _pc-check-ruff-lockstep-test _pc-check-publish-metadata _pc-check-publish-metadata-test _pc-check-manpage-assets _pc-check-manpage-drift-test _pc-check-diagnostic-prefix _pc-check-diagnostic-prefix-test _pc-check-safety-doc-pin _pc-check-safety-doc-pin-test _pc-worktree-setup-test _pc-gate-status-test _pc-check-tools-test _pc-enums-check _pc-enums-codegen-drift _pc-enums-codegen-drift-test \ _pc-manpages \ _pc-self-scan _pc-self-scan-headroom \ _pc-py-fmt _pc-py-typecheck _pc-py-test _pc-py-stubtest @@ -1512,7 +1531,7 @@ _ci-all: $(MAKE) -j --output-sync=target \ _ci-cargo-pipeline \ _ci-shellcheck _ci-markdown-lint _ci-toml-lint _ci-makefile-check \ - _ci-actionlint _ci-snapshot-anchors _ci-snapshot-anchors-test _ci-rustfmt-bail _ci-rustfmt-bail-test _ci-grammar-marker-sync _ci-grammar-marker-sync-test _ci-check-versions _ci-check-versions-test _ci-check-grammar-crate-test _ci-check-excluded-manifests _ci-check-excluded-manifests-test _ci-check-ruff-lockstep _ci-check-ruff-lockstep-test _ci-check-publish-metadata _ci-check-publish-metadata-test _ci-check-manpage-assets _ci-check-manpage-drift-test _ci-check-diagnostic-prefix _ci-check-diagnostic-prefix-test _ci-worktree-setup-test _ci-gate-status-test _ci-check-tools-test _ci-enums-check _ci-enums-codegen-drift _ci-enums-codegen-drift-test \ + _ci-actionlint _ci-snapshot-anchors _ci-snapshot-anchors-test _ci-rustfmt-bail _ci-rustfmt-bail-test _ci-grammar-marker-sync _ci-grammar-marker-sync-test _ci-check-versions _ci-check-versions-test _ci-check-grammar-crate-test _ci-check-excluded-manifests _ci-check-excluded-manifests-test _ci-check-ruff-lockstep _ci-check-ruff-lockstep-test _ci-check-publish-metadata _ci-check-publish-metadata-test _ci-check-manpage-assets _ci-check-manpage-drift-test _ci-check-diagnostic-prefix _ci-check-diagnostic-prefix-test _ci-check-safety-doc-pin _ci-check-safety-doc-pin-test _ci-worktree-setup-test _ci-gate-status-test _ci-check-tools-test _ci-enums-check _ci-enums-codegen-drift _ci-enums-codegen-drift-test \ _ci-py-fmt-check _ci-py-lint _ci-py-typecheck _ci-py-test _ci-py-stubtest # --------------------------------------------------------------------------- @@ -1552,6 +1571,8 @@ _ci-all: # ├── _pc-check-publish-metadata-test # ├── _pc-check-diagnostic-prefix # ├── _pc-check-diagnostic-prefix-test +# ├── _pc-check-safety-doc-pin +# ├── _pc-check-safety-doc-pin-test # ├── _pc-worktree-setup-test # ├── _pc-gate-status-test # ├── _pc-check-tools-test @@ -1675,6 +1696,12 @@ _pc-check-diagnostic-prefix: _pc-fmt _pc-check-diagnostic-prefix-test: _pc-fmt $(MAKE) check-diagnostic-prefix-test +_pc-check-safety-doc-pin: _pc-fmt + $(MAKE) check-safety-doc-pin + +_pc-check-safety-doc-pin-test: _pc-fmt + $(MAKE) check-safety-doc-pin-test + _pc-worktree-setup-test: _pc-fmt $(MAKE) worktree-setup-test @@ -1872,6 +1899,12 @@ _ci-check-diagnostic-prefix: _ci-check-diagnostic-prefix-test: $(MAKE) check-diagnostic-prefix-test +_ci-check-safety-doc-pin: + $(MAKE) check-safety-doc-pin + +_ci-check-safety-doc-pin-test: + $(MAKE) check-safety-doc-pin-test + _ci-worktree-setup-test: $(MAKE) worktree-setup-test diff --git a/big-code-analysis-py/src/node.rs b/big-code-analysis-py/src/node.rs index bd9f11539..4e8ba76b1 100644 --- a/big-code-analysis-py/src/node.rs +++ b/big-code-analysis-py/src/node.rs @@ -57,6 +57,38 @@ //! `tree_sitter::Tree`, `Node`, and `TreeCursor` are `Send + Sync` under //! the pinned `=0.26.12`, so the pyclasses are sendable (no `unsendable`) //! and compose with `ThreadPoolExecutor` fan-out like [`PyAst`] itself. +//! That version literal is a premise, not decoration — the layouts and +//! the `&mut self` on `Tree::edit` are what make the argument above +//! checkable, and only at a named release. `utils/check-safety-doc-pin.py` +//! fails the build when the pin moves and this line does not (#1057). +//! +//! ## What has been checked +//! +//! The invariants above were probed against a built `_native.abi3.so`, +//! not argued from the source alone. Nothing below broke; it is recorded +//! here because a future auditor of this `unsafe` block cannot cheaply +//! re-derive it (#1057). +//! +//! * `Node` is not GC-tracked. It declares no `__traverse__`, so +//! `Py_TPFLAGS_HAVE_GC` is unset and the collector has no mechanism to +//! clear a keep-alive; `PyAst` holds no Python-object fields, so a +//! cycle through one is not constructible either. +//! * No Python-reachable constructor exists — `Node.__new__` rejects, and +//! copy / deepcopy / pickle all raise — so a `Node` cannot exist +//! without an `Ast`. A mispaired handle is therefore unconstructible +//! from Python, which is why the pairing is enforced at the Rust module +//! boundary ([`owned`]) rather than by a runtime test that could only +//! pass vacuously. +//! * [`PyNodeWalk::next_node`] bumps the refcount per yielded node rather +//! than sharing the iterator's own handle, so a yielded node safely +//! outlives the walk that produced it. +//! * 3,000 parse-and-drop cycles under `gc.set_threshold(1, 1, 1)` with a +//! survivor pool, and 8 threads x 4,000 iterations plus 6 threads +//! racing `next()` on a shared `NodeWalk`: no crash, no +//! `PyBorrowMutError`. +//! +//! The residual risk is source-level — a future Rust change — rather than +//! input-level, which is what invariant 3 says from the other direction. use big_code_analysis::tree_sitter::{Node as TsNode, TreeCursor}; use pyo3::prelude::*; @@ -105,46 +137,154 @@ fn push_children_for_preorder( stack[first_child..].reverse(); } -/// A lazy handle to one node of a parsed [`Ast`](big_code_analysis::Ast). +/// Ownership boundary for the erased-lifetime pair (#1057). /// -/// Reached from [`Ast.root_node`](PyAst) or [`Ast.find`](PyAst), then walked -/// with `children` / `parent` / `walk()` / `descendants_by_kind()`. Holds a -/// strong reference to its `Ast`, so it stays valid even after the caller -/// drops every other reference to the parse. -#[pyclass(name = "Node", module = "big_code_analysis._native", frozen)] -pub(crate) struct PyNode { - // Keep-alive: owns the `Ast` (Tree + source) the erased node borrows. - ast: Py, - node: TsNode<'static>, -} +/// Both pyclasses pair an erased `Node<'static>` / `TreeCursor<'static>` +/// with the `Py` that keeps the owning tree alive, and that +/// pairing *is* [`detach`]'s precondition: a handle paired with the +/// wrong `Ast` outlives its tree and dereferences freed memory. Rust +/// field privacy alone does not enforce it — `PyNode { ast, node }` +/// compiles anywhere in the defining module, which is how three +/// construction sites accumulated on one precondition. Fields private +/// to *this* module make the struct literal unavailable to the +/// `#[pymethods]` blocks outside it, so every handle is built by one of +/// the constructors below and the pairing is structural rather than +/// reviewed. The accessors are the price. +/// +/// Everything except [`PyNode::wrap`] is `pub(super)`, not +/// `pub(crate)`: `wrap` is the one entry point another module needs +/// (`ast.rs` builds the root handle), while `node` + `rewrap` together +/// *are* the mispairing primitive — `a.rewrap(py, b.node())` pairs one +/// tree's node with another tree's keep-alive. Keeping them file-local +/// is what makes the boundary above true beyond this file. +mod owned { + use pyo3::prelude::*; + + use super::{PyAst, TreeCursor, TsNode, detach, push_children_for_preorder}; + + /// A lazy handle to one node of a parsed [`Ast`](big_code_analysis::Ast). + /// + /// Reached from [`Ast.root_node`](PyAst) or [`Ast.find`](PyAst), then walked + /// with `children` / `parent` / `walk()` / `descendants_by_kind()`. Holds a + /// strong reference to its `Ast`, so it stays valid even after the caller + /// drops every other reference to the parse. + #[pyclass(name = "Node", module = "big_code_analysis._native", frozen)] + pub(crate) struct PyNode { + // Keep-alive: owns the `Ast` (Tree + source) the erased node borrows. + ast: Py, + node: TsNode<'static>, + } + + impl PyNode { + /// Wrap `node`, erasing its lifetime brand and keeping its owning + /// `Ast` alive through `ast`. + /// + /// The one place the pairing is a *caller* obligation: `node` must + /// have come from the tree held by `ast`. Both callers are + /// [`PyAst::root_node`] / [`PyAst::find`], which pass a node from + /// the very `Ast` they are wrapping. Every other handle in this + /// module is derived from an existing one by [`rewrap`](Self::rewrap) + /// or [`PyNodeWalk::next_node`], which carry the pairing over + /// instead of restating it. + pub(crate) fn wrap(ast: Py, node: TsNode<'_>) -> Self { + // SAFETY: `ast` is the very `Ast` whose `Tree` produced `node`, and + // it is stored here for the whole life of the returned `PyNode`, so + // the erased node never outlives its tree. + let node = unsafe { detach(node) }; + Self { ast, node } + } -impl PyNode { - /// Wrap `node`, erasing its lifetime brand and keeping its owning `Ast` - /// alive through `ast`. + /// Re-wrap a sibling/child node discovered during navigation, sharing + /// this node's keep-alive `Ast`. The input already carries the erased + /// `'static` brand (it came from `self.node`), so no further `detach` + /// is needed — only a refcount bump on the `Ast`. + /// + /// Callers pass a node reached from `self.node` by a `tree_sitter` + /// navigation call, so it belongs to the same tree by construction. + pub(super) fn rewrap(&self, py: Python<'_>, node: TsNode<'static>) -> Self { + Self { + ast: self.ast.clone_ref(py), + node, + } + } + + /// The keep-alive handle to the `Ast` this node borrows from. + pub(super) fn ast(&self) -> &Py { + &self.ast + } + + /// The erased node. `Node<'static>` is `Copy`, so this hands out a + /// value rather than a borrow — it is only valid while `self` (and + /// therefore [`ast`](Self::ast)) is alive. + pub(super) fn node(&self) -> TsNode<'static> { + self.node + } + } + + /// Lazy pre-order iterator over a node and its descendants, returned by + /// [`PyNode::walk`]. + /// + /// Holds a work stack of not-yet-visited erased nodes plus the keep-alive + /// `Py`. Each `__next__` pops the next node, pushes its children + /// (leftmost on top), and yields the popped node — pre-order, one node at a + /// time, so traversal never materialises the whole subtree at once. + /// + /// The `TreeCursor` is held for the whole walk rather than built per + /// step, for [`push_children_for_preorder`]'s reason. It is branded + /// `'static` by the same erasure as the nodes it enumerates, and stays + /// valid for the same reason: `ast` keeps the owning parse alive. /// - /// `node` must have come from the tree held by `ast` (the only callers - /// are [`PyAst::root_node`] / [`PyAst::find`] and this module's own - /// navigation, which always pass a node from `self.ast`'s tree). - pub(crate) fn wrap(ast: Py, node: TsNode<'_>) -> Self { - // SAFETY: `ast` is the very `Ast` whose `Tree` produced `node`, and - // it is stored here for the whole life of the returned `PyNode`, so - // the erased node never outlives its tree. - let node = unsafe { detach(node) }; - Self { ast, node } - } - - /// Re-wrap a sibling/child node discovered during navigation, sharing - /// this node's keep-alive `Ast`. The input already carries the erased - /// `'static` brand (it came from `self.node`), so no further `detach` - /// is needed — only a refcount bump on the `Ast`. - fn rewrap(&self, py: Python<'_>, node: TsNode<'static>) -> Self { - Self { - ast: self.ast.clone_ref(py), - node, + /// Field order is load-bearing. Rust drops fields in declaration order, + /// so `cursor` is declared *before* the `ast` that keeps its tree alive + /// — otherwise the keep-alive would be released first and the doc claim + /// above would be false by construction. Today's `Drop for TreeCursor` + /// only frees the cursor's own stack and never reads the tree, so the + /// current order is not unsound; this makes the stated invariant hold + /// regardless, and survives a future tree-sitter destructor that does + /// touch it. (`Vec` has no drop glue, so `cursor` is the only + /// field that raises the question.) + #[pyclass(name = "NodeWalk", module = "big_code_analysis._native")] + pub(crate) struct PyNodeWalk { + cursor: TreeCursor<'static>, + stack: Vec>, + ast: Py, + } + + impl PyNodeWalk { + /// Root a walk at `node`, sharing its keep-alive `Ast`. + /// + /// Takes the `PyNode` rather than a `(Ast, node)` pair, so the + /// cursor, the seed node, and the keep-alive all come from one + /// already-valid handle: there is no argument to get wrong. + pub(super) fn rooted_at(node: &PyNode, py: Python<'_>) -> Self { + Self { + cursor: node.node.walk(), + stack: vec![node.node], + ast: node.ast.clone_ref(py), + } + } + + /// Pop the next node in pre-order, pushing its children first. + /// + /// Lives here rather than in `__next__` because it is the walk's + /// only reader of `stack` / `cursor` / `ast`, and the node it + /// yields is paired with the `Ast` the walk was rooted at. + pub(super) fn next_node(&mut self, py: Python<'_>) -> Option { + let node = self.stack.pop()?; + push_children_for_preorder(&mut self.stack, &mut self.cursor, node); + // Each yielded node carries its own keep-alive `Py`: it may + // outlive this iterator, so the per-node refcount bump is required + // for soundness, not an optimisation to hoist out of the loop. + Some(PyNode { + ast: self.ast.clone_ref(py), + node, + }) } } } +pub(crate) use owned::{PyNode, PyNodeWalk}; + #[pymethods] impl PyNode { /// The raw grammar kind of this node (e.g. `"function_item"`). @@ -154,7 +294,7 @@ impl PyNode { /// altered nodes (string literals, etc.). #[getter] fn kind(&self) -> &'static str { - self.node.kind() + self.node().kind() } /// py-tree-sitter-compatible alias for [`kind`](PyNode::kind). @@ -164,71 +304,71 @@ impl PyNode { /// py-tree-sitter spelling so an existing matcher ports over unchanged. #[getter] fn r#type(&self) -> &'static str { - self.node.kind() + self.node().kind() } /// The numeric grammar id behind [`kind`](PyNode::kind). #[getter] fn kind_id(&self) -> u16 { - self.node.kind_id() + self.node().kind_id() } /// Whether this node is a *named* grammar production (as opposed to an /// anonymous token such as punctuation or a keyword literal). #[getter] fn is_named(&self) -> bool { - self.node.is_named() + self.node().is_named() } /// Whether this node is an `ERROR` node produced by the parser. #[getter] fn is_error(&self) -> bool { - self.node.is_error() + self.node().is_error() } /// Whether this node is a zero-width `MISSING` node the parser inserted /// to recover from a syntax error. #[getter] fn is_missing(&self) -> bool { - self.node.is_missing() + self.node().is_missing() } /// Whether this node is an `extra` (a node that may appear anywhere, /// such as a comment in most grammars). #[getter] fn is_extra(&self) -> bool { - self.node.is_extra() + self.node().is_extra() } /// Whether this node or any node beneath it is an error or missing node. #[getter] fn has_error(&self) -> bool { - self.node.has_error() + self.node().has_error() } /// Start byte offset (inclusive) into [`PyAst::source`]. #[getter] fn start_byte(&self) -> usize { - self.node.start_byte() + self.node().start_byte() } /// End byte offset (exclusive) into [`PyAst::source`]. #[getter] fn end_byte(&self) -> usize { - self.node.end_byte() + self.node().end_byte() } /// 0-based `(row, column)` of the node's start — py-tree-sitter parity. #[getter] fn start_point(&self) -> (usize, usize) { - let p = self.node.start_position(); + let p = self.node().start_position(); (p.row, p.column) } /// 0-based `(row, column)` of the node's end — py-tree-sitter parity. #[getter] fn end_point(&self) -> (usize, usize) { - let p = self.node.end_position(); + let p = self.node().end_position(); (p.row, p.column) } @@ -236,28 +376,28 @@ impl PyNode { /// vocabulary (`start_line == start_point[0] + 1`). #[getter] fn start_line(&self) -> usize { - self.node.start_position().row + 1 + self.node().start_position().row + 1 } /// 1-based end line (`end_line == end_point[0] + 1`). #[getter] fn end_line(&self) -> usize { - self.node.end_position().row + 1 + self.node().end_position().row + 1 } /// The node's span as the same 1-based dict `dump()` emits: /// `{start_line, start_col, end_line, end_col, start_byte, end_byte}`. #[getter] fn span<'py>(&self, py: Python<'py>) -> PyResult> { - let start = self.node.start_position(); - let end = self.node.end_position(); + let start = self.node().start_position(); + let end = self.node().end_position(); let dict = PyDict::new(py); dict.set_item("start_line", start.row + 1)?; dict.set_item("start_col", start.column + 1)?; dict.set_item("end_line", end.row + 1)?; dict.set_item("end_col", end.column + 1)?; - dict.set_item("start_byte", self.node.start_byte())?; - dict.set_item("end_byte", self.node.end_byte())?; + dict.set_item("start_byte", self.node().start_byte())?; + dict.set_item("end_byte", self.node().end_byte())?; Ok(dict) } @@ -266,8 +406,8 @@ impl PyNode { /// parent reaches without a field. #[getter] fn field_name(&self) -> Option<&'static str> { - let parent = self.node.parent()?; - let id = self.node.id(); + let parent = self.node().parent()?; + let id = self.node().id(); let mut cursor = parent.walk(); if !cursor.goto_first_child() { return None; @@ -285,20 +425,20 @@ impl PyNode { /// The number of direct children (named and anonymous). #[getter] fn child_count(&self) -> usize { - self.node.child_count() + self.node().child_count() } /// The number of direct *named* children. #[getter] fn named_child_count(&self) -> usize { - self.node.named_child_count() + self.node().named_child_count() } /// All direct children (named and anonymous), in document order. #[getter] fn children(&self, py: Python<'_>) -> Vec { - let mut cursor = self.node.walk(); - self.node + let mut cursor = self.node().walk(); + self.node() .children(&mut cursor) .map(|c| self.rewrap(py, c)) .collect() @@ -307,8 +447,8 @@ impl PyNode { /// The direct *named* children, in document order. #[getter] fn named_children(&self, py: Python<'_>) -> Vec { - let mut cursor = self.node.walk(); - self.node + let mut cursor = self.node().walk(); + self.node() .named_children(&mut cursor) .map(|c| self.rewrap(py, c)) .collect() @@ -317,31 +457,31 @@ impl PyNode { /// This node's parent, or `None` at the root. #[getter] fn parent(&self, py: Python<'_>) -> Option { - self.node.parent().map(|p| self.rewrap(py, p)) + self.node().parent().map(|p| self.rewrap(py, p)) } /// The next sibling (named or anonymous), or `None`. #[getter] fn next_sibling(&self, py: Python<'_>) -> Option { - self.node.next_sibling().map(|n| self.rewrap(py, n)) + self.node().next_sibling().map(|n| self.rewrap(py, n)) } /// The previous sibling (named or anonymous), or `None`. #[getter] fn prev_sibling(&self, py: Python<'_>) -> Option { - self.node.prev_sibling().map(|n| self.rewrap(py, n)) + self.node().prev_sibling().map(|n| self.rewrap(py, n)) } /// The next *named* sibling, or `None`. #[getter] fn next_named_sibling(&self, py: Python<'_>) -> Option { - self.node.next_named_sibling().map(|n| self.rewrap(py, n)) + self.node().next_named_sibling().map(|n| self.rewrap(py, n)) } /// The previous *named* sibling, or `None`. #[getter] fn prev_named_sibling(&self, py: Python<'_>) -> Option { - self.node.prev_named_sibling().map(|n| self.rewrap(py, n)) + self.node().prev_named_sibling().map(|n| self.rewrap(py, n)) } /// The child at `index` (named and anonymous children counted), or @@ -350,7 +490,7 @@ impl PyNode { fn child(&self, py: Python<'_>, index: usize) -> Option { u32::try_from(index) .ok() - .and_then(|i| self.node.child(i)) + .and_then(|i| self.node().child(i)) .map(|c| self.rewrap(py, c)) } @@ -359,14 +499,14 @@ impl PyNode { fn named_child(&self, py: Python<'_>, index: usize) -> Option { u32::try_from(index) .ok() - .and_then(|i| self.node.named_child(i)) + .and_then(|i| self.node().named_child(i)) .map(|c| self.rewrap(py, c)) } /// The first child reached through the grammar field `name`, or `None`. #[pyo3(signature = (name, /))] fn child_by_field_name(&self, py: Python<'_>, name: &str) -> Option { - self.node + self.node() .child_by_field_name(name) .map(|c| self.rewrap(py, c)) } @@ -374,8 +514,8 @@ impl PyNode { /// Every child reached through the grammar field `name`, in order. #[pyo3(signature = (name, /))] fn children_by_field_name(&self, py: Python<'_>, name: &str) -> Vec { - let mut cursor = self.node.walk(); - self.node + let mut cursor = self.node().walk(); + self.node() .children_by_field_name(name, &mut cursor) .map(|c| self.rewrap(py, c)) .collect() @@ -387,7 +527,7 @@ impl PyNode { fn field_name_for_child(&self, index: usize) -> Option<&'static str> { u32::try_from(index) .ok() - .and_then(|i| self.node.field_name_for_child(i)) + .and_then(|i| self.node().field_name_for_child(i)) } /// This node's source text, the `source[start_byte:end_byte]` slice of @@ -398,11 +538,11 @@ impl PyNode { /// is hot. #[getter] fn text<'py>(&self, py: Python<'py>) -> Bound<'py, PyBytes> { - let source = self.ast.bind(py).get().ast_ref().source(); + let source = self.ast().bind(py).get().ast_ref().source(); // Tree-sitter guarantees the span lies within the source, but slice // defensively rather than risk a panic on a malformed input. let slice = source - .get(self.node.start_byte()..self.node.end_byte()) + .get(self.node().start_byte()..self.node().end_byte()) .unwrap_or(&[]); PyBytes::new(py, slice) } @@ -414,11 +554,7 @@ impl PyNode { /// early without materialising the whole subtree — the memory payoff of /// the lazy surface. Mirrors the Rust `Node::preorder` (#728). fn walk(&self, py: Python<'_>) -> PyNodeWalk { - PyNodeWalk { - cursor: self.node.walk(), - stack: vec![self.node], - ast: self.ast.clone_ref(py), - } + PyNodeWalk::rooted_at(self, py) } /// Collect every node in this subtree (this node included) whose @@ -433,9 +569,9 @@ impl PyNode { #[allow(clippy::needless_pass_by_value)] fn descendants_by_kind(&self, py: Python<'_>, kinds: Vec) -> Vec { let mut out = Vec::new(); - let mut stack = vec![self.node]; + let mut stack = vec![self.node()]; // One cursor for the whole subtree, not one per visited node. - let mut cursor = self.node.walk(); + let mut cursor = self.node().walk(); while let Some(node) = stack.pop() { // Only matches pay the `rewrap` (a keep-alive refcount bump), so // a selective filter does not allocate a handle per visited node. @@ -455,9 +591,9 @@ impl PyNode { /// deallocated, so its address cannot be recycled for a *different* /// `Ast`. Pointer equality therefore implies the same `Ast` instance. fn __eq__(&self, other: &Bound<'_, PyAny>) -> bool { - other - .extract::>() - .is_ok_and(|o| self.ast.as_ptr() == o.ast.as_ptr() && self.node.id() == o.node.id()) + other.extract::>().is_ok_and(|o| { + self.ast().as_ptr() == o.ast().as_ptr() && self.node().id() == o.node().id() + }) } /// Hash consistent with [`__eq__`](PyNode::__eq__): the `(Ast object @@ -468,53 +604,24 @@ impl PyNode { fn __hash__(&self) -> isize { use std::hash::{Hash, Hasher}; let mut hasher = std::collections::hash_map::DefaultHasher::new(); - (self.ast.as_ptr() as usize).hash(&mut hasher); - self.node.id().hash(&mut hasher); + (self.ast().as_ptr() as usize).hash(&mut hasher); + self.node().id().hash(&mut hasher); hasher.finish() as isize } fn __repr__(&self) -> String { - let start = self.node.start_position(); + let start = self.node().start_position(); format!( "Node(kind='{}', start_point=({}, {}), byte_range={}..{})", - self.node.kind(), + self.node().kind(), start.row, start.column, - self.node.start_byte(), - self.node.end_byte(), + self.node().start_byte(), + self.node().end_byte(), ) } } -/// Lazy pre-order iterator over a node and its descendants, returned by -/// [`PyNode::walk`]. -/// -/// Holds a work stack of not-yet-visited erased nodes plus the keep-alive -/// `Py`. Each `__next__` pops the next node, pushes its children -/// (leftmost on top), and yields the popped node — pre-order, one node at a -/// time, so traversal never materialises the whole subtree at once. -/// -/// The `TreeCursor` is held for the whole walk rather than built per -/// step, for [`push_children_for_preorder`]'s reason. It is branded -/// `'static` by the same erasure as the nodes it enumerates, and stays -/// valid for the same reason: `ast` keeps the owning parse alive. -/// -/// Field order is load-bearing. Rust drops fields in declaration order, -/// so `cursor` is declared *before* the `ast` that keeps its tree alive -/// — otherwise the keep-alive would be released first and the doc claim -/// above would be false by construction. Today's `Drop for TreeCursor` -/// only frees the cursor's own stack and never reads the tree, so the -/// current order is not unsound; this makes the stated invariant hold -/// regardless, and survives a future tree-sitter destructor that does -/// touch it. (`Vec` has no drop glue, so `cursor` is the only -/// field that raises the question.) -#[pyclass(name = "NodeWalk", module = "big_code_analysis._native")] -pub(crate) struct PyNodeWalk { - cursor: TreeCursor<'static>, - stack: Vec>, - ast: Py, -} - #[pymethods] impl PyNodeWalk { fn __iter__(slf: PyRef<'_, Self>) -> PyRef<'_, Self> { @@ -522,14 +629,6 @@ impl PyNodeWalk { } fn __next__(&mut self, py: Python<'_>) -> Option { - let node = self.stack.pop()?; - push_children_for_preorder(&mut self.stack, &mut self.cursor, node); - // Each yielded node carries its own keep-alive `Py`: it may - // outlive this iterator, so the per-node refcount bump is required - // for soundness, not an optimisation to hoist out of the loop. - Some(PyNode { - ast: self.ast.clone_ref(py), - node, - }) + self.next_node(py) } } diff --git a/utils/check-safety-doc-pin-test.py b/utils/check-safety-doc-pin-test.py new file mode 100644 index 000000000..1d79533cd --- /dev/null +++ b/utils/check-safety-doc-pin-test.py @@ -0,0 +1,216 @@ +#!/usr/bin/env python3 +"""Tests for check-safety-doc-pin.py. + +Three kinds of test, matching the check-diagnostic-prefix-test.py +pattern: + +* Unit tests over the two extractors, weighted toward the false-clean + direction: a module doc the scanner fails to read reports no citations + and, if the "no citation" branch were ever softened, would report a + clean tree. The stop condition (where the ``//!`` block ends) is what + decides how much of the file the gate can see, so it is pinned in both + directions — a line that must not end the block, and one that must. +* ``main()`` tests over a synthetic repository root, covering the clean, + stale, and citation-dropped branches plus the unreadable-manifest exit. +* A smoke test running the real gate against the real repository, + asserting a clean tree reports OK. + +Run with: + python3 -m unittest -q utils/check-safety-doc-pin-test.py +""" + +from __future__ import annotations + +import contextlib +import importlib.util +import io +import pathlib +import subprocess +import sys +import tempfile +import unittest +from unittest import mock + +UTILS_DIR = pathlib.Path(__file__).resolve().parent +REPO_ROOT = UTILS_DIR.parent +SCRIPT_SRC = UTILS_DIR / "check-safety-doc-pin.py" + + +def _load_module(): # type: ignore[no-untyped-def] + spec = importlib.util.spec_from_file_location("check_safety_doc_pin", SCRIPT_SRC) + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +GATE = _load_module() + +# A module doc shaped like node.rs's: prose, a blank `//!` line, and the +# sendability claim carrying the version literal. +DOC_SOURCE = """\ +//! Lazy `Node` handle over the tree retained by [`PyAst`]. +//! +//! `tree_sitter::Tree`, `Node`, and `TreeCursor` are `Send + Sync` under +//! the pinned `=0.26.12`, so the pyclasses are sendable. + +use pyo3::prelude::*; + +/// Item doc citing `=9.9.9`, deliberately out of scope. +fn f() {} +""" + + +class ModuleDocLines(unittest.TestCase): + """What the gate can and cannot see.""" + + def test_collects_the_leading_block_only(self) -> None: + lines = GATE.module_doc_lines(DOC_SOURCE) + self.assertEqual([n for n, _ in lines], [1, 2, 3, 4]) + self.assertTrue(lines[3][1].endswith("the pyclasses are sendable.")) + + def test_item_doc_below_is_out_of_scope(self) -> None: + # The `///` citation must not be collected: the gate's contract + # is the module-level soundness argument, and an item doc may + # legitimately name another crate's version. + bodies = [body for _, body in GATE.module_doc_lines(DOC_SOURCE)] + self.assertFalse(any("9.9.9" in body for body in bodies)) + + def test_inner_attribute_does_not_end_the_block(self) -> None: + # A `#![allow(...)]` between doc paragraphs must not truncate the + # scan: everything after it would silently stop being gated. + source = "//! first\n#![allow(clippy::pedantic)]\n//! `=1.2.3`\n\nfn f() {}\n" + self.assertEqual( + GATE.cited_versions(GATE.module_doc_lines(source)), [(3, "1.2.3")] + ) + + def test_code_ends_the_block(self) -> None: + source = "//! `=1.2.3`\nuse pyo3::prelude::*;\n//! `=4.5.6`\n" + self.assertEqual( + GATE.cited_versions(GATE.module_doc_lines(source)), [(1, "1.2.3")] + ) + + +class CitedVersions(unittest.TestCase): + def test_reads_the_backticked_literal(self) -> None: + self.assertEqual( + GATE.cited_versions(GATE.module_doc_lines(DOC_SOURCE)), [(4, "0.26.12")] + ) + + def test_unbackticked_version_is_ignored(self) -> None: + # Prose such as "since 0.26.0 the cursor …" is not a citation of + # the pin and must not fail the gate. + source = "//! behaviour changed in =0.26.0 upstream\n" + self.assertEqual(GATE.cited_versions(GATE.module_doc_lines(source)), []) + + def test_every_citation_is_reported(self) -> None: + source = "//! `=1.2.3` and later `=1.2.4`\n" + self.assertEqual( + GATE.cited_versions(GATE.module_doc_lines(source)), + [(1, "1.2.3"), (1, "1.2.4")], + ) + + +class PinnedVersion(unittest.TestCase): + def test_string_form(self) -> None: + manifest = '[workspace.dependencies]\ntree-sitter = "=0.26.12"\n' + self.assertEqual(GATE.pinned_version(manifest), "0.26.12") + + def test_table_form(self) -> None: + manifest = ( + "[workspace.dependencies]\n" + 'tree-sitter = { version = "=0.26.12", default-features = false }\n' + ) + self.assertEqual(GATE.pinned_version(manifest), "0.26.12") + + def test_caret_range_is_rejected(self) -> None: + manifest = '[workspace.dependencies]\ntree-sitter = "0.26"\n' + with self.assertRaises(GATE.ManifestError): + GATE.pinned_version(manifest) + + def test_missing_entry_is_rejected(self) -> None: + manifest = '[workspace.dependencies]\ntree-sitter-c = "=0.24.1"\n' + with self.assertRaises(GATE.ManifestError): + GATE.pinned_version(manifest) + + +@contextlib.contextmanager +def _synthetic_repo(manifest: str, doc_source: str): # type: ignore[no-untyped-def] + """Point the gate at a throwaway root and capture its stderr/stdout.""" + with tempfile.TemporaryDirectory() as tmp: + root = pathlib.Path(tmp) + (root / "Cargo.toml").write_text(manifest, encoding="utf-8") + doc = root / "big-code-analysis-py" / "src" / "node.rs" + doc.parent.mkdir(parents=True) + doc.write_text(doc_source, encoding="utf-8") + out, err = io.StringIO(), io.StringIO() + with ( + mock.patch.object(GATE, "REPO_ROOT", root), + mock.patch.object(GATE, "ROOT_MANIFEST", root / "Cargo.toml"), + mock.patch.object(GATE, "SAFETY_DOC", doc), + contextlib.redirect_stdout(out), + contextlib.redirect_stderr(err), + ): + yield lambda: (GATE.main(), out.getvalue(), err.getvalue()) + + +PINNED = '[workspace.dependencies]\ntree-sitter = "=0.26.12"\n' + + +class Main(unittest.TestCase): + def test_matching_citation_passes(self) -> None: + with _synthetic_repo(PINNED, DOC_SOURCE) as run: + code, out, err = run() + self.assertEqual(code, 0) + self.assertIn("safety-doc-pin: OK", out) + self.assertEqual(err, "") + + def test_stale_citation_fails_and_names_the_line(self) -> None: + stale = DOC_SOURCE.replace("=0.26.12", "=0.26.9") + with _synthetic_repo(PINNED, stale) as run: + code, _out, err = run() + self.assertEqual(code, 1) + self.assertIn("node.rs:4: cites `=0.26.9`, pin is `=0.26.12`", err) + # The remediation must send the reader back to the argument, not + # only to the literal — re-reading it is the point of the gate. + self.assertIn("Re-read that", err) + + def test_dropping_the_citation_fails(self) -> None: + symbolic = DOC_SOURCE.replace("the pinned `=0.26.12`", "the pinned version") + with _synthetic_repo(PINNED, symbolic) as run: + code, _out, err = run() + self.assertEqual(code, 1) + # Spelled out rather than matched loosely: the crate name is + # not itself a version requirement, so `=tree-sitter` would be + # a nonsense literal for the reader to go looking for. + self.assertIn("no longer cites a `tree-sitter` version", err) + self.assertIn("`=0.26.12`", err) + + def test_unreadable_pin_exits_two(self) -> None: + with _synthetic_repo("[workspace.dependencies]\n", DOC_SOURCE) as run: + code, _out, err = run() + self.assertEqual(code, 2) + self.assertIn("could not read the tree-sitter pin", err) + + def test_missing_safety_doc_exits_two(self) -> None: + with _synthetic_repo(PINNED, DOC_SOURCE) as run: + GATE.SAFETY_DOC.unlink() + code, _out, err = run() + self.assertEqual(code, 2) + self.assertIn("could not read", err) + + +class RealRepository(unittest.TestCase): + def test_gate_passes_on_the_checked_in_tree(self) -> None: + result = subprocess.run( + [sys.executable, str(SCRIPT_SRC)], + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("safety-doc-pin: OK", result.stdout) + + +if __name__ == "__main__": + unittest.main() diff --git a/utils/check-safety-doc-pin.py b/utils/check-safety-doc-pin.py new file mode 100755 index 000000000..25fcec1ea --- /dev/null +++ b/utils/check-safety-doc-pin.py @@ -0,0 +1,182 @@ +#!/usr/bin/env python3 +"""check-safety-doc-pin + +Guard that the ``tree-sitter`` version cited by the soundness argument +in ``big-code-analysis-py/src/node.rs`` is the version the workspace +actually pins in the root ``Cargo.toml``. + +That module doc is the canonical justification for the workspace's only +sanctioned ``unsafe`` block (AGENTS.md, "Rust conventions"), and part of +its argument — that ``Tree``/``Node``/``TreeCursor`` are ``Send + Sync``, +that ``Node<'tree>`` is ``(ffi::TSNode, PhantomData<&'tree ()>)``, that +``Tree::edit`` takes ``&mut self`` — is a claim about a *specific* +release. The version literal is what makes the claim checkable: a reader +can open that version of the crate and confirm it. When the pin moves and +the literal does not, the argument keeps reading as verified while +silently describing a crate nobody is compiling against. That is #1057: +the doc said ``=0.26.9`` two patch releases after the pin had moved. + +The gate does not verify the *argument* — no gate can. It forces a diff +on that line at bump time, which is the prompt to re-read the argument. + +Two directions are enforced: + +* every backticked ``=X.Y.Z`` literal in the module doc equals the pin; +* at least one such literal is present. Dropping the literal in favour of + a version-free phrase ("under the pinned version") was considered and + declined in #1057: it removes the stale number by making the claim + unfalsifiable at read time, which moves the staleness from visible to + invisible rather than fixing it. + +Scope is deliberately one file. Two other places name this pin in +prose — the runtime-exemption rationale in ``AGENTS.md`` and the same +rationale echoed in ``utils/check-excluded-manifests.py`` — but both sit +in text that also cites *grammar* pins illustratively (``=0.23.5``, +``=0.26.10``), so a literal-scanning rule there would fail on versions +that are supposed to differ. In ``node.rs`` an ``=X.Y.Z`` literal can +only mean the tree-sitter pin, which is what makes the rule crisp here. + +See AGENTS.md "Validation gates" for the policy this enforces. +""" + +from __future__ import annotations + +import pathlib +import re +import sys +import tomllib + +# `parents[1]`, not `parent`: these gates live in `utils/` but every +# path they read is anchored at the repository root. +REPO_ROOT = pathlib.Path(__file__).resolve().parents[1] +ROOT_MANIFEST = REPO_ROOT / "Cargo.toml" +SAFETY_DOC = REPO_ROOT / "big-code-analysis-py" / "src" / "node.rs" + +# The dependency whose pin the safety argument is about. +PINNED_CRATE = "tree-sitter" + +# A backticked exact-version requirement, e.g. `=0.26.12`. Backticks are +# required so ordinary prose containing a version-shaped substring is not +# swept in. +CITED_VERSION = re.compile(r"`=(\d+\.\d+\.\d+)`") + + +class ManifestError(Exception): + """The root manifest did not carry a readable ``tree-sitter`` pin.""" + + +def module_doc_lines(text: str) -> list[tuple[int, str]]: + """Return the file's leading ``//!`` block as ``(line_no, body)``. + + Line numbers are 1-based and file-relative, so a failure can name the + offending line directly. Inner attributes (``#![…]``) and blank lines + are skipped rather than terminating the block, since either may be + interleaved with a module doc; the first other non-empty line ends + it, which keeps ``///`` item docs and code out of scope. + """ + out: list[tuple[int, str]] = [] + for line_no, raw in enumerate(text.splitlines(), start=1): + line = raw.strip() + if line.startswith("//!"): + out.append((line_no, line[len("//!") :].strip())) + elif not line or line.startswith("#!["): + continue + else: + break + return out + + +def cited_versions(doc: list[tuple[int, str]]) -> list[tuple[int, str]]: + """Return every ``(line_no, version)`` the module doc cites.""" + return [ + (line_no, match.group(1)) + for line_no, body in doc + for match in CITED_VERSION.finditer(body) + ] + + +def pinned_version(manifest_text: str) -> str: + """Return the exact version ``[workspace.dependencies]`` pins. + + Accepts both the string form (``tree-sitter = "=0.26.12"``) and the + table form (``tree-sitter = { version = "=0.26.12", … }``), so a + future manifest restructure does not read as a missing pin. + + Raises :class:`ManifestError` if the entry is absent or is not an + exact ``=X.Y.Z`` requirement — a caret range would make the cited + literal unverifiable, which is the same defect from the other end. + """ + data = tomllib.loads(manifest_text) + entry = data.get("workspace", {}).get("dependencies", {}).get(PINNED_CRATE) + if isinstance(entry, dict): + entry = entry.get("version") + if not isinstance(entry, str): + raise ManifestError( + f"[workspace.dependencies].{PINNED_CRATE} is missing or is not " + f"a version string (got {entry!r})" + ) + match = re.fullmatch(r"=\s*(\d+\.\d+\.\d+)", entry.strip()) + if match is None: + raise ManifestError( + f"[workspace.dependencies].{PINNED_CRATE} = {entry!r} is not an " + f"exact `=X.Y.Z` pin" + ) + return match.group(1) + + +def main() -> int: + try: + pin = pinned_version(ROOT_MANIFEST.read_text(encoding="utf-8")) + except (OSError, ManifestError) as exc: + sys.stderr.write( + f"error: could not read the {PINNED_CRATE} pin from " + f"{ROOT_MANIFEST}: {exc}\n" + ) + return 2 + + try: + doc = module_doc_lines(SAFETY_DOC.read_text(encoding="utf-8")) + except OSError as exc: + sys.stderr.write(f"error: could not read {SAFETY_DOC}: {exc}\n") + return 2 + + rel = SAFETY_DOC.relative_to(REPO_ROOT) + cited = cited_versions(doc) + + if not cited: + sys.stderr.write( + f"error: {rel} no longer cites a `{PINNED_CRATE}` version in " + f"its module doc\n" + f" The soundness argument for this workspace's only\n" + f" sanctioned `unsafe` block makes claims about a\n" + f" specific release; naming it is what lets a reader\n" + f" check them. Restore the literal as `={pin}` (see\n" + f" #1057 — the version-free phrasing was declined).\n" + ) + return 1 + + stale = [(line_no, version) for line_no, version in cited if version != pin] + if stale: + sys.stderr.write( + f"error: {rel} cites a {PINNED_CRATE} version the workspace " + f"does not pin\n" + ) + for line_no, version in stale: + sys.stderr.write(f" {rel}:{line_no}: cites `={version}`, pin is `={pin}`\n") + sys.stderr.write( + f"\nThe module doc is the canonical soundness argument for the\n" + f"only `unsafe` block in this workspace, and it reasons about\n" + f"the pinned {PINNED_CRATE} release specifically. Re-read that\n" + f"argument against {pin} — `Tree(NonNull)`,\n" + f"`Node<'tree>(ffi::TSNode, PhantomData<&'tree ()>)`,\n" + f"`Tree::edit(&mut self)`, `Send + Sync` — and then update the\n" + f"literal. See #1057.\n" + ) + return 1 + + print(f"safety-doc-pin: OK ({PINNED_CRATE} `={pin}`, {len(cited)} citation(s))") + return 0 + + +if __name__ == "__main__": + sys.exit(main())