diff --git a/.cargo/config.toml b/.cargo/config.toml deleted file mode 100644 index e9e6b7002..000000000 --- a/.cargo/config.toml +++ /dev/null @@ -1,12 +0,0 @@ -# Enable the Polonius alpha borrow-checking analysis for every Cargo -# invocation, including rust-analyzer, so editors and verification tooling -# agree with CI about what borrows are legal. -# -# Note: an inherited `RUSTFLAGS` environment variable overrides this table. -# Wrappers that set `RUSTFLAGS` (see the Makefile) must re-state -# `-Zpolonius=next` themselves. `cargo kani` sets `CARGO_ENCODED_RUSTFLAGS` -# itself, which also bypasses this table, so the `kani-full` recipe passes -# the flag via `RUSTFLAGS`. See -# docs/adr-006-adopt-polonius-nightly-toolchain.md. -[build] -rustflags = ["-Zpolonius=next"] diff --git a/.github/workflows/build-and-package.yml b/.github/workflows/build-and-package.yml index d9673a6ac..cae1dc5af 100644 --- a/.github/workflows/build-and-package.yml +++ b/.github/workflows/build-and-package.yml @@ -94,9 +94,6 @@ jobs: bin-name: ${{ env.BIN_NAME }} project-dir: . manifest-path: Cargo.toml - # Preserve the Polonius requirement through nested toolchain setup - # (see docs/adr-006-adopt-polonius-nightly-toolchain.md). - rustflags: -Zpolonius=next skip-man-page-discovery: 'true' - name: Generate release help diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 898c4a7ef..71c8ca3b9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,10 +20,10 @@ jobs: env: CARGO_TERM_COLOR: always BUILD_PROFILE: debug - # The tree requires -Zpolonius=next (see - # docs/adr-006-adopt-polonius-nightly-toolchain.md), so CI builds with - # the dated nightly pinned in rust-toolchain.toml. - NETSUKE_RUST_TOOLCHAIN: nightly-2026-06-25 + # The tree requires the Polonius borrow checker, which nightly enables by + # default (see docs/adr-006-adopt-polonius-nightly-toolchain.md), so CI + # builds with the dated nightly pinned in rust-toolchain.toml. + NETSUKE_RUST_TOOLCHAIN: nightly-2026-08-23 WHITAKER_INSTALLER_VERSION: '0.2.7' steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -52,8 +52,8 @@ jobs: uses: leynos/shared-actions/.github/actions/setup-rust@8add2d99854a5b77548eae98cca59202e68fefc8 with: toolchain: ${{ env.NETSUKE_RUST_TOOLCHAIN }} - # Preserve warnings-as-errors and Polonius through toolchain setup. - rustflags: -D warnings -Zpolonius=next + # Preserve warnings-as-errors through toolchain setup. + rustflags: -D warnings - name: Install cargo-nextest uses: taiki-e/install-action@18b1216eba7f8039b0f8d131d5473787f0edce68 # v2.85.3 with: @@ -145,10 +145,10 @@ jobs: env: CARGO_TERM_COLOR: always BUILD_PROFILE: debug - # The tree requires -Zpolonius=next (see - # docs/adr-006-adopt-polonius-nightly-toolchain.md), so CI builds with - # the dated nightly pinned in rust-toolchain.toml. - NETSUKE_RUST_TOOLCHAIN: nightly-2026-06-25 + # The tree requires the Polonius borrow checker, which nightly enables by + # default (see docs/adr-006-adopt-polonius-nightly-toolchain.md), so CI + # builds with the dated nightly pinned in rust-toolchain.toml. + NETSUKE_RUST_TOOLCHAIN: nightly-2026-08-23 WHITAKER_INSTALLER_VERSION: '0.2.7' defaults: run: @@ -166,8 +166,8 @@ jobs: uses: leynos/shared-actions/.github/actions/setup-rust@8add2d99854a5b77548eae98cca59202e68fefc8 with: toolchain: ${{ env.NETSUKE_RUST_TOOLCHAIN }} - # Preserve warnings-as-errors and Polonius through toolchain setup. - rustflags: -D warnings -Zpolonius=next + # Preserve warnings-as-errors through toolchain setup. + rustflags: -D warnings - name: Install Ninja uses: seanmiddleditch/gha-setup-ninja@3b1f8f94a2f8254bd26914c4ab9474d4f0015f67 # v6 - name: Install cargo-nextest @@ -223,7 +223,7 @@ jobs: # the `#[cfg(windows)]` arms. A failure blocks the merge. run: make SHELL=bash lint-whitaker - name: Test - # cargo-nextest plus doctests under `-D warnings -Zpolonius=next`, + # cargo-nextest plus doctests under `-D warnings`, # compiling and running the `#[cfg(windows)]` test tree. A failure # blocks the merge. run: make SHELL=bash test diff --git a/.github/workflows/coverage-main.yml b/.github/workflows/coverage-main.yml index 8c86175e1..5b2f0eb1c 100644 --- a/.github/workflows/coverage-main.yml +++ b/.github/workflows/coverage-main.yml @@ -26,12 +26,13 @@ jobs: - name: Setup Rust uses: leynos/shared-actions/.github/actions/setup-rust@8add2d99854a5b77548eae98cca59202e68fefc8 with: - # Match rust-toolchain.toml: the tree needs -Zpolonius=next (see + # Match rust-toolchain.toml: the tree needs the nightly-default + # Polonius borrow checker (see # docs/adr-006-adopt-polonius-nightly-toolchain.md). - toolchain: nightly-2026-06-25 - # Preserve warnings-as-errors and Polonius through toolchain setup; + toolchain: nightly-2026-08-23 + # Preserve warnings-as-errors through toolchain setup; # cargo-llvm-cov appends its instrumentation flags to this value. - rustflags: -D warnings -Zpolonius=next + rustflags: -D warnings - name: Test and Measure Coverage uses: leynos/shared-actions/.github/actions/generate-coverage@8add2d99854a5b77548eae98cca59202e68fefc8 with: diff --git a/.github/workflows/netsukefile-test.yml b/.github/workflows/netsukefile-test.yml index 75676aa74..08440287b 100644 --- a/.github/workflows/netsukefile-test.yml +++ b/.github/workflows/netsukefile-test.yml @@ -12,9 +12,9 @@ jobs: permissions: contents: read env: - # Match rust-toolchain.toml: the tree needs -Zpolonius=next (see - # docs/adr-006-adopt-polonius-nightly-toolchain.md). - NETSUKE_RUST_TOOLCHAIN: nightly-2026-06-25 + # Match rust-toolchain.toml: the tree needs the nightly-default Polonius + # borrow checker (see docs/adr-006-adopt-polonius-nightly-toolchain.md). + NETSUKE_RUST_TOOLCHAIN: nightly-2026-08-23 steps: - name: Checkout repository uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -24,8 +24,6 @@ jobs: uses: leynos/shared-actions/.github/actions/setup-rust@8add2d99854a5b77548eae98cca59202e68fefc8 with: toolchain: ${{ env.NETSUKE_RUST_TOOLCHAIN }} - # Preserve the Polonius requirement through toolchain setup. - rustflags: -Zpolonius=next - name: Show rustc version run: | rustup show diff --git a/AGENTS.md b/AGENTS.md index 365022efb..f7fdee5f5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -135,11 +135,12 @@ project: ### Borrow checker: Polonius, not NLL -Netsuke compiles with the Polonius alpha analysis (`-Zpolonius=next`) on the -dated nightly pinned in `rust-toolchain.toml` (see -`docs/adr-006-adopt-polonius-nightly-toolchain.md` and `docs/polonius.md`). -Internal APIs are borrow-centric: lookups and get-or-create accessors return -references, clone keys only on insertion, and build error context lazily. +Netsuke compiles with the Polonius alpha analysis, which the dated nightly +pinned in `rust-toolchain.toml` enables by default (see +`docs/adr-006-adopt-polonius-nightly-toolchain.md` and `docs/polonius.md`). No +`-Z` directive is needed, or wanted: do not add one. Internal APIs are +borrow-centric: lookups and get-or-create accessors return references, clone +keys only on insertion, and build error context lazily. - **Never** rewrite a site tagged `POLONIUS(...)` into a double lookup (`contains_key` + `get_mut`), an `entry(key.clone())` call, or an id/index @@ -152,8 +153,17 @@ references, clone keys only on insertion, and build error context lazily. - Respect `POLONIUS-REFUSED(...)` tags: the named constraint (persistent identity, lock boundaries, aliasing, suspension points, thread boundaries) is permanent. Do not convert those sites to reference-returning forms. -- When adding a new borrow-centric API, verify it with and without - `-Zpolonius=next` and record the classification in `docs/polonius.md`. +- When adding a new borrow-centric API, record the classification in + `docs/polonius.md`. + +### Trait solver: next-generation, enabled by the pin + +The same pinned nightly enables the next-generation trait solver, and Netsuke +assumes it. Write to what the solver accepts: do not contort a design around an +old-solver limitation, and do not add explicit turbofish, redundant bounds, or +intermediate bindings to work around inference that already succeeds. As with +Polonius, the pin is the whole mechanism — do not add a `-Znext-solver` +directive anywhere. - Run `make check-fmt`, `make lint`, `make doc-coverage`, and `make test` before committing. These targets wrap the following commands, so contributors @@ -168,9 +178,9 @@ references, clone keys only on insertion, and build error context lazily. - `make lint` executes: ```sh - RUSTFLAGS="${RUSTFLAGS:+$RUSTFLAGS }-D warnings -Zpolonius=next" \ + RUSTFLAGS="${RUSTFLAGS:+$RUSTFLAGS }-D warnings" \ RUSTDOCFLAGS="--cfg docsrs -D warnings" cargo doc --workspace --no-deps - RUSTFLAGS="${RUSTFLAGS:+$RUSTFLAGS }-D warnings -Zpolonius=next" \ + RUSTFLAGS="${RUSTFLAGS:+$RUSTFLAGS }-D warnings" \ cargo clippy --workspace --all-targets --all-features -- -D warnings whitaker --all -- --all-targets --all-features ``` @@ -183,9 +193,9 @@ references, clone keys only on insertion, and build error context lazily. - `make test` executes: ```sh - RUSTFLAGS="${RUSTFLAGS:+$RUSTFLAGS }-D warnings -Zpolonius=next" \ + RUSTFLAGS="${RUSTFLAGS:+$RUSTFLAGS }-D warnings" \ cargo nextest run --workspace --all-targets --all-features - RUSTFLAGS="${RUSTFLAGS:+$RUSTFLAGS }-D warnings -Zpolonius=next" \ + RUSTFLAGS="${RUSTFLAGS:+$RUSTFLAGS }-D warnings" \ cargo test --workspace --doc --all-features ``` @@ -198,7 +208,6 @@ references, clone keys only on insertion, and build error context lazily. - `make doc-coverage` executes: ```sh - RUSTFLAGS="${RUSTFLAGS:+$RUSTFLAGS }-Zpolonius=next" \ RUSTDOCFLAGS="--cfg docsrs -D warnings" \ python3 scripts/doc-coverage.py --threshold 80 ``` diff --git a/Makefile b/Makefile index 70d8df357..81c3d08b8 100644 --- a/Makefile +++ b/Makefile @@ -22,9 +22,6 @@ CARGO ?= $(shell command -v cargo 2>/dev/null || printf '%s' "$$HOME/.cargo/bin/ # CARGO is resolved above before it is exported: `export` alone would define # the variable empty and shadow the `?=` fallback for every recipe. export CARGO -# The Polonius borrow-checker flag normally flows from .cargo/config.toml, but -# any recipe that sets RUSTFLAGS overrides that table and must re-state it. -POLONIUS_FLAGS ?= -Zpolonius=next # Extra build-parallelism flags for plain Cargo invocations, e.g. `-j 4`. BUILD_JOBS ?= # The same concept for cargo-nextest, which spells build parallelism @@ -38,12 +35,11 @@ KANI_FLAGS ?= KANI_INSTALL_FLAGS ?= KANI_CHECK_FLAGS ?= KANI_VERSION_FILE ?= tools/kani/VERSION -# Opt-in local build acceleration. The Cargo fragment is deliberately separate -# from `.cargo/config.toml`: that file is auto-discovered and carries the -# repository-wide Polonius flag, whereas Cranelift and mold must stay opt-in so -# release, packaging, coverage, and formal-verification paths keep the -# supported LLVM backend and platform linker. The toolchain is not pinned -# separately — dev-fast uses the repository's own nightly. +# Opt-in local build acceleration. The Cargo fragment is deliberately kept out +# of an auto-discovered `.cargo/config.toml`, because Cranelift and mold must +# stay opt-in so release, packaging, coverage, and formal-verification paths +# keep the supported LLVM backend and platform linker. The toolchain is not +# pinned separately — dev-fast uses the repository's own nightly. MOLD_VERSION_FILE ?= tools/mold/VERSION MOLD_SHA256SUMS_FILE ?= tools/mold/SHA256SUMS DEV_FAST_CONFIG ?= tools/dev-fast/config.toml @@ -84,7 +80,7 @@ MD_FILES_FIND = find . -type f -name '*.md' \ PROVER_TOOLS_SOURCE ?= git+https://github.com/leynos/rust-prover-tools@b07ef696f8373d54ae68e517d39d47a5d27a5bd5 PROVER_TOOLS ?= uv tool run --from $(PROVER_TOOLS_SOURCE) prover-tools RUSTDOC_FLAGS ?= --cfg docsrs -D warnings -export PYTHON POLONIUS_FLAGS RUSTDOC_FLAGS +export PYTHON RUSTDOC_FLAGS VERUS_FLAGS ?= VERUS_INSTALL_FLAGS ?= WHITAKER ?= whitaker @@ -102,10 +98,10 @@ clean: ## Remove build artefacts test: test-nextest doctest ## Run every Rust test with warnings treated as errors test-nextest: ## Run all non-doctest Rust tests through cargo-nextest - RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)" $(CARGO) nextest run --workspace --all-targets --all-features $(NEXTEST_BUILD_JOBS) + RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings" $(CARGO) nextest run --workspace --all-targets --all-features $(NEXTEST_BUILD_JOBS) doctest: ## Run doctests, which cargo-nextest cannot execute - RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)" $(CARGO) test --workspace --doc --all-features $(BUILD_JOBS) + RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings" $(CARGO) test --workspace --doc --all-features $(BUILD_JOBS) test-workflow-contracts: ## Validate the mutation-testing caller contract uv run --with 'pytest>=8' --with 'pyyaml>=6' --with 'hypothesis>=6' pytest tests/workflow_contracts -q @@ -113,29 +109,34 @@ test-workflow-contracts: ## Validate the mutation-testing caller contract test-typos-config: spelling-helper-test ## Verify the shared spelling-policy integration target/%/$(APP): ## Build binary in debug or release mode - RUSTFLAGS="$${RUSTFLAGS-} $(POLONIUS_FLAGS)" $(CARGO) build $(BUILD_JOBS) $(if $(findstring release,$(@)),--release) --bin $(APP) + $(CARGO) build $(BUILD_JOBS) $(if $(findstring release,$(@)),--release) --bin $(APP) lint: lint-clippy lint-whitaker ## Run Clippy and the Whitaker Dylint suite with warnings denied lint-clippy: ## Run rustdoc and Clippy with warnings denied - RUSTDOCFLAGS="$(RUSTDOC_FLAGS)" RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)" $(CARGO) doc --workspace --no-deps - RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)" $(CARGO) clippy $(CLIPPY_FLAGS) + RUSTDOCFLAGS="$(RUSTDOC_FLAGS)" RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings" $(CARGO) doc --workspace --no-deps + RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings" $(CARGO) clippy $(CLIPPY_FLAGS) lint-whitaker: ## Run the Whitaker Dylint suite with warnings denied - DYLINT_TOML="$$(cat dylint.toml)" RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)" $(WHITAKER) --all --no-deps --package netsuke-build -- --all-targets --all-features + DYLINT_TOML="$$(cat dylint.toml)" RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings" $(WHITAKER) --all --no-deps --package netsuke-build -- --all-targets --all-features # Run from the crate directory as well so Whitaker loads the narrow # `test_support::fs` exemption from test_support/dylint.toml. - cd test_support && DYLINT_TOML="$$(cat dylint.toml)" RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)" $(WHITAKER) --all --no-deps --package test_support -- --all-targets --all-features + cd test_support && DYLINT_TOML="$$(cat dylint.toml)" RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings" $(WHITAKER) --all --no-deps --package test_support -- --all-targets --all-features doc-coverage: doc-coverage-test ## Verify aggregate Rustdoc doc-comment coverage meets the threshold - @RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }$${POLONIUS_FLAGS}" RUSTDOCFLAGS="$${RUSTDOC_FLAGS}" \ + @RUSTDOCFLAGS="$${RUSTDOC_FLAGS}" \ "$${PYTHON}" scripts/doc-coverage.py --toolchain "$$DOC_COVERAGE_TOOLCHAIN" --threshold "$$DOC_COVERAGE_THRESHOLD" -doc-coverage-test: ## Run the pytest suite for scripts/doc-coverage.py +doc-coverage-test: ## Run documentation-coverage pytest modules @PYTHONPATH=scripts $(UV_ENV) $(UV) run --no-project --python 3.13 \ --with pytest==9.0.2 --with pytest-cov==7.0.0 \ - python -m pytest scripts/tests/test_doc_coverage.py -c /dev/null \ - --rootdir=. -p no:cacheprovider --cov=doc_coverage_module + python -m pytest scripts/tests/test_doc_coverage_model.py \ + scripts/tests/test_doc_coverage_cargo.py \ + scripts/tests/test_doc_coverage_cargo_payload.py \ + scripts/tests/test_doc_coverage_runner.py \ + scripts/tests/test_doc_coverage.py -c /dev/null --rootdir=. \ + -p no:cacheprovider --cov=doc_coverage_model --cov=doc_coverage_cargo \ + --cov=doc_coverage_runner --cov=doc_coverage_module fmt: ## Format Rust and Markdown sources $(CARGO) fmt --all @@ -145,7 +146,7 @@ check-fmt: ## Verify formatting $(CARGO) fmt --all -- --check typecheck: ## Typecheck all targets and features - RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)" $(CARGO) check --all-targets --all-features $(BUILD_JOBS) + RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings" $(CARGO) check --all-targets --all-features $(BUILD_JOBS) markdownlint: spelling ## Lint Markdown and enforce en-GB-oxendict spelling $(MDLINT) "**/*.md" @@ -181,7 +182,7 @@ kani-check: ## Check the installed Kani verifier version @$(PROVER_TOOLS) kani check-version --kani-command "$(KANI)" $(KANI_CHECK_FLAGS) || { status=$$?; printf 'prover-tools: target=kani-check failed exit=%s\n' "$$status" >&2; exit "$$status"; } kani-full: ## Run the full Kani verification suite - RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }$(POLONIUS_FLAGS)" $(KANI) $(KANI_FLAGS) + $(KANI) $(KANI_FLAGS) kani-ir: kani-full ## Run the IR Kani verification suite @@ -223,7 +224,7 @@ bench-build: dev-fast-check ## Time clean and incremental debug builds for both @CARGO="$(CARGO)" scripts/bench-build.sh bench-config-load: ## Benchmark cached configuration loading without layer copies - RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }$(POLONIUS_FLAGS)" $(CARGO) bench --bench config_load_cached_merge + $(CARGO) bench --bench config_load_cached_merge help: ## Show available targets @grep -E '^[a-zA-Z_-]+:.*?##' $(MAKEFILE_LIST) | \ diff --git a/README.md b/README.md index 248f10d1e..51ec60cd5 100644 --- a/README.md +++ b/README.md @@ -37,8 +37,9 @@ Netsuke currently requires: - [Ninja](https://ninja-build.org/) on `PATH`; - when installing from source, the dated Rust nightly toolchain pinned in [`rust-toolchain.toml`](rust-toolchain.toml) (`rustup` installs it - automatically in a checkout). Netsuke builds with the Polonius borrow checker - (`-Zpolonius=next`), which is nightly-only until it stabilizes; see + automatically in a checkout). Netsuke builds with the Polonius borrow + checker, which nightly enables by default and which stays nightly-only until + it stabilizes; see [ADR-006](docs/adr-006-adopt-polonius-nightly-toolchain.md). ### Installation @@ -54,15 +55,14 @@ requirement below. cargo binstall netsuke-build ``` -Building from the registry instead runs outside a repository checkout, so -neither the pinned toolchain nor the Polonius flag is picked up automatically; -supply both explicitly: +Building from the registry instead runs outside a repository checkout, so the +pinned toolchain is not picked up automatically; select it explicitly: ```sh -rustup toolchain install nightly-2026-06-25 -RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke-build +rustup toolchain install nightly-2026-08-23 +cargo +nightly-2026-08-23 install netsuke-build ``` Pre-built installers are available from the diff --git a/docs/adr-006-adopt-polonius-nightly-toolchain.md b/docs/adr-006-adopt-polonius-nightly-toolchain.md index cb9a6a288..3f57a7eb3 100644 --- a/docs/adr-006-adopt-polonius-nightly-toolchain.md +++ b/docs/adr-006-adopt-polonius-nightly-toolchain.md @@ -99,3 +99,28 @@ remains correct. - `cargo +stable` invocations fail on `-Zpolonius=next`. This is intentional: the failure is loud and immediate rather than a confusing borrowck error later. + +## Addendum — 2026-08-27: nightly-default Polonius and toolchain boundaries + +The repository pin has since moved to `nightly-2026-08-23`. Nightlies dated +2026-08-04 and later enable the Polonius alpha analysis by default, so the pin +now carries the borrow-checker requirement without an explicit directive. + +The explicit `-Zpolonius` plumbing from the original decision has been retired: +the `.cargo/config.toml` rustflags entry, the `POLONIUS_FLAGS` Makefile +variable, and the CI `with.rustflags` inputs were removed. The pin is now the +sole repository mechanism for this compiler behaviour. + +Kani is outside that boundary. Kani 0.67.0 installs and uses its own bundled +`nightly-2025-11-21` toolchain through `cargo kani setup`. That toolchain +predates nightly-default Polonius, so Kani currently uses NLL. Moving the +repository Rust pin does not upgrade Kani. Do not claim that Kani verifies +`POLONIUS(...)` APIs with Polonius until a Kani release bundles a sufficiently +recent nightly, or Kani is rebuilt from source against that nightly. + +Registry installs likewise do not inherit the checkout's toolchain file. They +must select the repository's pinned nightly explicitly, for example: + +```sh +cargo +nightly-2026-08-23 install netsuke-build +``` diff --git a/docs/adr-007-publish-as-netsuke-build.md b/docs/adr-007-publish-as-netsuke-build.md index 3f98965bd..99a1eac60 100644 --- a/docs/adr-007-publish-as-netsuke-build.md +++ b/docs/adr-007-publish-as-netsuke-build.md @@ -53,9 +53,9 @@ Publish as `netsuke-build`, and keep every user-facing name as `netsuke`. covers every released target because `stage-release-artefacts` names each target's archive to the same shape. Without the template `cargo binstall` would probe its default asset-name patterns, which place the target before - the version, fail to find any matching asset, and fall back to a source - build that needs the pinned nightly and the Polonius flag — the very - fallback the documented command exists to avoid. + the version, fail to find any matching asset, and fall back to a source build + that needs the pinned nightly — the very fallback the documented command + exists to avoid. - Update the crates.io installation guidance in the README, the users' guide, and the quickstart to install `netsuke-build`. @@ -87,8 +87,8 @@ Publish as `netsuke-build`, and keep every user-facing name as `netsuke`. script that stages those archives to the release root. - The `pkg-url` template encodes the staged archive name's shape. Changing `staging_dir_template`, `bin_name`, or the workflow artefact names without - keeping the staged archive name in step with the template breaks `cargo - binstall`; the contract tests fail first for the parts they can derive. + keeping the staged archive name in step with the template breaks + `cargo binstall`; the contract tests fail first for the parts they can derive. - Documentation and contract tests refer to `netsuke-build` only for registry installation. Everywhere else — prose, examples, help output, packaging — the project remains Netsuke. @@ -104,3 +104,10 @@ Publish as `netsuke-build`, and keep every user-facing name as `netsuke`. rule. - [Developer guide](developers-guide.md): the day-to-day naming guidance and the contract tests that enforce it. + +## Addendum (2026-08-28) + +A source-build fallback from `cargo binstall` requires the repository's pinned +nightly toolchain. This requirement follows the repository toolchain policy; +the fallback must not restore the retired `RUSTFLAGS=-Zpolonius=next` +instruction. diff --git a/docs/debugging/debugging-plan-20260825-doc-coverage.md b/docs/debugging/debugging-plan-20260825-doc-coverage.md new file mode 100644 index 000000000..2bacc2399 --- /dev/null +++ b/docs/debugging/debugging-plan-20260825-doc-coverage.md @@ -0,0 +1,112 @@ +# Debugging plan: restore Rustdoc coverage parsing + +**Generated**: 2026-08-25 **Issue ID**: rebase follow-up for PR #577 +**Severity**: high **Falsification sub-agent**: `alchemist` **Planning agent +boundary**: This document was prepared by the planning agent. Falsification +must be executed by the named sub-agent, not by the planning agent. + +## Problem statement + +`make doc-coverage` passes its Python unit tests but fails while measuring the +`test_support` library on the pinned nightly. Cargo exits successfully, yet the +coverage script receives empty standard output where it expects Rustdoc JSON. +The gate must parse the compiler's actual output channel without weakening the +coverage threshold or skipping the workspace member. + +## Context summary + +Table: Context and initial observations for the coverage failure. + +| Aspect | Details | +| ------------------- | ------------------------------------------------------------ | +| First observed | 2026-08-25, after rebasing PR #577 onto `origin/main` | +| Reproduction rate | Deterministic through `make doc-coverage` | +| Affected components | `scripts/doc-coverage.py`, `test_support` Rustdoc invocation | +| Recent changes | Pin moved to `nightly-2026-08-23`; `main` added doc coverage | + +### Error artefacts + +```text +error: cargo rustdoc for test_support lib (lib) did not emit coverage JSON: +Expecting value: line 1 column 1 (char 0) +``` + +### Information gaps + +- Whether the nightly now writes the coverage JSON to standard error. +- Whether the output differs only for workspace member libraries. + +______________________________________________________________________ + +## Hypotheses + +### H1: Rustdoc moved coverage JSON to standard error + +**Claim**: The pinned nightly returns a successful `cargo rustdoc` invocation +for `test_support`, but writes `--show-coverage --output-format json` output to +standard error rather than standard output. + +**Plausibility**: Falsified — the exact command wrote a generated-file notice +to standard output and progress to standard error; neither stream held JSON. + +**Prediction**: Running the exact target invocation while capturing both output +streams finds a non-empty, parseable JSON document on standard error and an +empty standard output stream. + +#### H1 falsification plan + +Table: Falsification steps for the standard-error output hypothesis. + +| Step | Action | Expected Negative Result | +| ---- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------- | +| 1 | Run the exact `cargo rustdoc` command built by `rustdoc_args` for `test_support --lib`, capture standard output and standard error separately, then inspect their byte counts and leading bytes. | Parseable coverage JSON appears on standard output, or neither stream contains it. | + +**Tooling**: `cargo`, the pinned toolchain, `wc`, and `head`. + +**Confidence on falsification**: Decisive for the output-channel hypothesis; +the command is exactly the one the gate constructs. + +______________________________________________________________________ + +### H2: Rustdoc writes coverage JSON to its generated output file + +**Claim**: Rustdoc writes the requested coverage JSON to the file named in its +successful standard-output notice, `target/doc/test_support.json`, rather than +to either captured stream. + +**Plausibility**: High — H1's experiment reported exactly that generated path. + +**Prediction**: The named file exists after the invocation and its contents +parse as the per-file coverage object that `aggregate_coverage_payload` accepts. + +#### H2 falsification plan + +Table: Falsification steps for the generated-file output hypothesis. + +| Step | Action | Expected Negative Result | +| ---- | ------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------- | +| 1 | Read `target/doc/test_support.json` after the exact `test_support --lib` command and parse it with Python's JSON decoder. | The file is absent, invalid JSON, or has a shape the current aggregator rejects. | + +**Tooling**: `cargo`, the pinned toolchain, and Python's standard `json` module. + +**Confidence on falsification**: Decisive for the generated-file contract and +for whether the existing aggregator can consume it unchanged. + +______________________________________________________________________ + +## Recommended execution order + +1. **H1** — falsified: neither output stream contains JSON. +2. **H2** — verify the file Rustdoc announced before changing the collector. + +## Termination criteria + +- **Root cause identified**: H2 survives its falsification test. +- **Escalation trigger**: H2 is falsified; revise this plan before inspecting + a third cause. + +## Notes for executing agent + +Do not edit tracked files or run the full repository gates. Return the exact +stream observed and a verdict of `falsified`, `not-falsified`, or +`inconclusive`. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 7eee9d316..1a830826e 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -396,19 +396,28 @@ names diverge. ## Toolchain and borrow checker Netsuke builds on the dated nightly toolchain pinned in `rust-toolchain.toml` -with the Polonius alpha borrow-checking analysis (`-Zpolonius=next`) enabled. -`rustup` provisions the toolchain automatically, and `.cargo/config.toml` -supplies the flag by default, covering Cargo invocations such as rust-analyzer -that run without `RUSTFLAGS` in the environment. Makefile recipes that set -`RUSTFLAGS` re-state the flag through the `POLONIUS_FLAGS` variable because an -inherited `RUSTFLAGS` environment variable overrides `.cargo/config.toml`. The -recipes that add `-D warnings` and `$(POLONIUS_FLAGS)` build the value as -`RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)"`; the +with the Polonius alpha borrow-checking analysis enabled. Nightly toolchains +dated 2026-08-04 and later run Polonius by default, so the pin carries the +requirement on its own: **no `-Zpolonius` directive is passed anywhere, and +none should be added.** The directive is being retired upstream, and a build +that restates it is a build that can silently drop it. A contract test +(described below) fails if one reappears. + +`rustup` provisions the toolchain automatically inside a checkout, which covers +every checkout consumer — plain Cargo invocations, rust-analyzer, Clippy, and +Whitaker — without any Cargo configuration. `cargo kani setup` is a separate +boundary: Kani 0.67.0 installs and uses its bundled `nightly-2025-11-21` +toolchain rather than the checkout toolchain. The repository has no +`.cargo/config.toml`; carrying the flag was that file's only purpose, and it +was deleted when the pin moved past 2026-08-04. + +Makefile recipes still set `RUSTFLAGS`, but only to deny warnings. Each builds +the value as `RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings"`; the `$${RUSTFLAGS:+$$RUSTFLAGS }` expansion prepends any `RUSTFLAGS` already set by the caller (for example a CI wrapper), so those flags survive rather than being -silently discarded. `cargo kani` sets `CARGO_ENCODED_RUSTFLAGS` itself, which -also overrides the table, so `make kani-full` passes the flag through -`RUSTFLAGS` as well. +silently discarded. `make kani-full` and the binary-build recipe set no +`RUSTFLAGS` at all: Kani compiles third-party crates the workspace lint policy +does not govern, and a plain binary build is not a lint gate. [ADR-006](adr-006-adopt-polonius-nightly-toolchain.md) records the policy decision, and the [polonius migration notes](polonius.md) track every site @@ -420,36 +429,47 @@ fails to compile, consult the migration notes before restructuring. ### Polonius CI shared-action contract -GitHub Actions jobs do not read `.cargo/config.toml` for every build they -launch, and the shared Rust setup actions export their own `RUSTFLAGS`. The -Polonius flags therefore travel as *action inputs*, not as job environment -variables: each affected workflow passes them through the relevant shared -action's `with.rustflags` input, and none of them may set a job-level -`env.RUSTFLAGS`. A job-level override would win over the action's exported -value and silently drop the flag, so the tree would fail to borrow-check with a -confusing `E0499` rather than an obvious configuration error. +Continuous integration gets Polonius the same way a checkout does: from the +pinned toolchain. What the shared-action contract still governs is the +*toolchain selection* and the warning policy that travels beside it. + +The shared Rust setup actions export their own `RUSTFLAGS`, so anything a job +needs travels as an *action input*, not as a job environment variable: each +affected workflow passes it through the relevant shared action's +`with.rustflags` input, and none of them may set a job-level `env.RUSTFLAGS`. A +job-level override would win over the action's exported value and silently drop +whatever the action set. Five CI jobs across four workflows carry the contract: -| Workflow | Job | Shared action | `with.rustflags` | -| --------------------------------------------------------------------- | -------------------- | -------------------- | ----------------------------- | -| [`ci.yml`](../.github/workflows/ci.yml) | `build-test` | `setup-rust` | `-D warnings -Zpolonius=next` | -| [`ci.yml`](../.github/workflows/ci.yml) | `build-test-windows` | `setup-rust` | `-D warnings -Zpolonius=next` | -| [`coverage-main.yml`](../.github/workflows/coverage-main.yml) | `coverage-upload` | `setup-rust` | `-D warnings -Zpolonius=next` | -| [`netsukefile-test.yml`](../.github/workflows/netsukefile-test.yml) | `netsukefile` | `setup-rust` | `-Zpolonius=next` | -| [`build-and-package.yml`](../.github/workflows/build-and-package.yml) | `build` | `rust-build-release` | `-Zpolonius=next` | - -CI and coverage add `-D warnings` because those jobs gate on a warning-free -build; the Netsukefile and packaging jobs carry the Polonius flag alone, so a -new upstream warning cannot break a release build. The coverage action's -`cargo-llvm-cov` invocation inherits the flags `setup-rust` exports and appends -its own instrumentation flags. - -`NETSUKE_RUST_TOOLCHAIN` follows a separate rule. CI and Netsukefile pin it to -the channel in `rust-toolchain.toml` so those jobs provision the dated nightly -explicitly; coverage and packaging must leave it unset, because they select -their toolchain through the action's own `toolchain` input and a second, -independently edited pin would let the two disagree. +| Workflow | Job | Shared action | `with.rustflags` | +| --------------------------------------------------------------------- | -------------------- | -------------------- | --------------------------- | +| [`ci.yml`](../.github/workflows/ci.yml) | `build-test` | `setup-rust` | `-D warnings` | +| [`ci.yml`](../.github/workflows/ci.yml) | `build-test-windows` | `setup-rust` | `-D warnings` | +| [`coverage-main.yml`](../.github/workflows/coverage-main.yml) | `coverage-upload` | `setup-rust` | `-D warnings` | +| [`netsukefile-test.yml`](../.github/workflows/netsukefile-test.yml) | `netsukefile` | `setup-rust` | *(omitted; action default)* | +| [`build-and-package.yml`](../.github/workflows/build-and-package.yml) | `build` | `rust-build-release` | *(omitted; action default)* | + +The CI jobs and coverage pass `-D warnings` explicitly because those jobs gate +on a warning-free build — on Windows that is what surfaces findings in the +`#[cfg(windows)]` tree at all. The pinned shared actions also apply +`-D warnings` by default when `with.rustflags` is omitted, so an upstream +compiler warning can fail both the Netsukefile and packaging jobs. Their +omitted inputs are intentional and remain distinct from jobs that explicitly +pass `with.rustflags: -D warnings`; neither job supplies an explicit empty +value. The coverage action's `cargo-llvm-cov` invocation inherits the flags +`setup-rust` exports and appends its own instrumentation flags. + +No `setup-rust` call passes a `components` input. The shared action is not +declared to accept one and installs rustfmt and clippy itself, so passing it +only emitted an "Unexpected input(s)" warning on every run; +`tests/workflow_contracts/ci_lint_test.py` holds that. + +`NETSUKE_RUST_TOOLCHAIN` follows a separate rule. The CI jobs and Netsukefile +pin it to the channel in `rust-toolchain.toml` so those jobs provision the +dated nightly explicitly; coverage and packaging must leave it unset, because +they select their toolchain through the action's own `toolchain` input and a +second, independently edited pin would let the two disagree. [`tests/polonius_toolchain_contract.rs`](../tests/polonius_toolchain_contract.rs) enforces all five callers. For each one it asserts: @@ -458,12 +478,18 @@ enforces all five callers. For each one it asserts: revision, the latter derived from the checked workflows themselves rather than restated in the test (see "Workflow pins and Dependabot" below for why the revision is asserted here); -- the `with.rustflags` value matches the table above in full, not merely that - it contains `-Zpolonius=next`, so a dropped `-D warnings` is caught too; +- the `with.rustflags` value matches the table above exactly, including the + two jobs that must pass no `rustflags` input at all; - the job declares no `env.RUSTFLAGS`; - the `NETSUKE_RUST_TOOLCHAIN` policy above — pinned to the - `rust-toolchain.toml` channel for CI and Netsukefile, absent for coverage and - packaging. + `rust-toolchain.toml` channel for the CI jobs and Netsukefile, absent for + coverage and packaging. + +The same test carries the two toolchain-level assertions: that the pinned +channel is a dated nightly at or after 2026-08-04, the first nightly on which +Polonius is the default analysis, and that no build configuration — the +Makefile, a Cargo configuration fragment, a workflow, or a recreated +`.cargo/config.toml` — passes a `-Zpolonius` directive. Run it with: @@ -472,8 +498,8 @@ cargo nextest run --test polonius_toolchain_contract ``` Keep this section and the [Polonius migration notes](polonius.md) in step: both -describe the same contract, and the notes list every remaining harness that -must propagate the flag. +describe the same no-directive, pinned-toolchain contract, and the notes record +the remaining harness consequences of that policy. ## Quality gates @@ -498,7 +524,13 @@ The toolchain the metric measures with is `DOC_COVERAGE_TOOLCHAIN`, defaulting to the channel pinned in `rust-toolchain.toml`. See *Doc-comment coverage* in `AGENTS.md` for the counting rules and the exemptions (Rustdoc excludes trait-implementation overrides, and `cfg(test)` items are not compiled into the -doc build). +doc build). Rustdoc writes the coverage JSON to its reported generated file, +which the script reads immediately after each successful invocation. That +path-extraction helper belongs only to the +`--show-coverage --output-format json` collector; do not reuse it for general +Rustdoc output. The pure `Coverage`/`DocTarget` model and payload validation +belong to `scripts/doc_coverage_model.py`; only the coverage gate may import +it. The executable retains Cargo invocation and user-facing error translation. `make test` runs the non-doctest suite through [cargo-nextest](https://nexte.st/) and then runs the doctests separately. CI @@ -599,15 +631,15 @@ The root Whitaker invocation selects only the `netsuke-build` package (the Cargo package name behind the `netsuke` targets; see ADR-007) and disables Dylint dependency checks. It supplies the root `dylint.toml` contents explicitly through `DYLINT_TOML`, so every invocation receives the same -capability-boundary policy regardless of how Dylint resolves the current -crate. `test_support` is a workspace member with one sanctioned ambient -boundary configured per crate. Its second, scoped invocation supplies -`test_support/dylint.toml` through `DYLINT_TOML`, and uses `--package -test_support` and `--no-deps`, because running from a member directory alone -would otherwise check the parent workspace. That configuration names only -`test_support::fs` in `excluded_paths`. The root `excluded_crates` must not -contain `test_support`: every other module in the crate remains subject to the -filesystem policy. +capability-boundary policy regardless of how Dylint resolves the current crate. +`test_support` is a workspace member with one sanctioned ambient boundary +configured per crate. Its second, scoped invocation supplies +`test_support/dylint.toml` through `DYLINT_TOML`, and uses +`--package test_support` and `--no-deps`, because running from a member +directory alone would otherwise check the parent workspace. That configuration +names only `test_support::fs` in `excluded_paths`. The root `excluded_crates` +must not contain `test_support`: every other module in the crate remains +subject to the filesystem policy. Permanent exceptions belong in `dylint.toml`, scoped as narrowly as the lint allows. Do not use Rust `#[allow]` or `#[expect]` for `no_std_fs_operations`: @@ -932,11 +964,10 @@ missing or empty file is reported as `dev-fast: missing version pin: ` rather than silently becoming an empty version. - `rust-toolchain.toml` supplies the toolchain. dev-fast deliberately shares - the repository's own dated nightly rather than pinning a second one: the tree - borrow-checks only under Polonius on that nightly (ADR-006), so a separate - pin would let the accelerated loop and the gates disagree about which borrows - are legal. `make install-dev-fast` adds `rustc-codegen-cranelift-preview` to - that toolchain. + the repository's own dated nightly rather than pinning a second one, keeping + the accelerated loop and the gates on the same toolchain. The + `make install-dev-fast` target adds `rustc-codegen-cranelift-preview` to that + toolchain. - `tools/mold/VERSION` holds the `mold` release tag. - `tools/mold/SHA256SUMS` holds the SHA-256 checksum of each supported `mold` release artefact. `make install-dev-fast` refuses to install an artefact that @@ -1007,17 +1038,16 @@ and formal-verification builds. The fragment is instead passed explicitly with `cargo --config tools/dev-fast/config.toml` from the `make dev-*` targets, and must not be sourced from any target that CI invokes. -A repository-root `.cargo/config.toml` does exist, and legitimately so: it -carries the Polonius flag (`[build] rustflags = ["-Zpolonius=next"]`, see -Polonius under Composition rules) needed by every build in the repository. The -rule is about what belongs in that file, not about whether it may exist: -settings needed everywhere may go there; settings that are only safe for the -accelerated dev loop must not. +No repository-root `.cargo/config.toml` exists any more. It once carried the +Polonius flag, and was deleted when the pinned nightly began enabling the +analysis by default. The rule is about what would belong in that file if it +returned, not about whether it may exist: settings needed everywhere may go +there; settings that are only safe for the accelerated dev loop must not. The fragment sets the `codegen-backend` unstable flag, `codegen-backend = "cranelift"` on the `dev` profile, and a -`cfg(target_os = "linux")`-gated rustflags list carrying both `-Zpolonius=next` -and `-Clink-arg=-fuse-ld=mold`. +`cfg(target_os = "linux")`-gated rustflags list carrying +`-Clink-arg=-fuse-ld=mold`. ### Composition rules @@ -1029,11 +1059,11 @@ and `-Clink-arg=-fuse-ld=mold`. described below. Run the ordinary gates before proposing a change; `make dev-test` is a faster inner-loop proxy, not a substitute. - **`RUSTFLAGS`.** `make test-nextest`, `make doctest`, `make typecheck`, and - the rustdoc stage of `make lint` append `-D warnings` and `$(POLONIUS_FLAGS)` - to any flags inherited from the caller. An externally set `RUSTFLAGS` - overrides the `[target.*]` `rustflags` in a Cargo configuration file, so the - `dev-*` targets deliberately do not set it. Exporting `RUSTFLAGS` in the - shell silently disables `mold` for these targets. + the rustdoc stage of `make lint` append `-D warnings` to any flags inherited + from the caller. An externally set `RUSTFLAGS` overrides the `[target.*]` + `rustflags` in a Cargo configuration file, so the `dev-*` targets + deliberately do not set it. Exporting `RUSTFLAGS` in the shell silently + disables `mold` for these targets. - **Release and packaging.** `make release` and everything under `.github/workflows/build-and-package.yml` use the release profile, the LLVM backend, and the platform linker. Cranelift is applied to the `dev` profile @@ -1064,13 +1094,12 @@ and `-Clink-arg=-fuse-ld=mold`. rust-analyzer into Cranelift is a personal, machine-local choice; it needs a separate target directory to avoid thrashing the cache shared with `make test`. -- **Polonius.** `.cargo/config.toml` applies `-Zpolonius=next` to every Cargo - invocation, and the tree does not borrow-check without it (ADR-006). Cargo - picks a single rustflags source rather than merging them, and a `[target.*]` - table outranks that `[build]` table, so the dev-fast fragment restates the - flag alongside the `mold` link argument. Anything that adds a rustflag there - must restate it too: omitting it does not merely diverge from the gate, it - stops the tree compiling. +- **Polonius.** The analysis comes from the pinned nightly (ADR-006), and the + `dev-*` targets use that same toolchain, so the fragment needs no + Polonius-specific cooperation and must not add a `-Zpolonius` directive. + Cargo does still pick a single rustflags source rather than merging them, so + anything the fragment's `[target.*]` table must carry has to be named there + in full. ### Fallback behaviour @@ -1227,11 +1256,11 @@ run ends, including on interrupt. To benchmark two things at once, override directory behind, remove it. Results below were recorded on a 24-core x86_64 Linux host, with both variants -on the repository's own `nightly-2026-06-25` supplying Cranelift 0.132.0, and -`mold` 2.41.0. Regenerate the table verbatim with `make bench-build`. Absolute -figures move with machine load, so the ratio between the two rows is the -durable signal, not the seconds; the run below is representative of three -consecutive runs that agreed to within 0.4 s. +on the repository's then-pinned `nightly-2026-06-25` supplying Cranelift +0.132.0, and `mold` 2.41.0. Regenerate the table verbatim with +`make bench-build`. Absolute figures move with machine load, so the ratio +between the two rows is the durable signal, not the seconds; the run below is +representative of three consecutive runs that agreed to within 0.4 s. | Variant | Clean build (s) | Incremental build (s) | | ------------------------------- | --------------- | --------------------- | @@ -1269,9 +1298,13 @@ make install-kani `cargo kani setup`, and verifies that `cargo kani` is callable. Kani may manage its own supporting Rust nightly toolchain during setup. That toolchain must not replace the repository's pinned nightly workflow (see -[ADR-006](adr-006-adopt-polonius-nightly-toolchain.md)). Kani builds pick up -`-Zpolonius=next` from `.cargo/config.toml`, so Polonius-dependent code -verifies unchanged. +[ADR-006](adr-006-adopt-polonius-nightly-toolchain.md)). Kani 0.67.0's +supporting nightly is `nightly-2025-11-21`, which predates the Polonius +default, so Kani borrow-checks under NLL. That is currently harmless — the tree +has no `POLONIUS(...)`-tagged sites — but a future tagged site could fail to +verify under Kani while compiling everywhere else. If that happens, move Kani +to a build whose nightly is 2026-08-04 or later rather than reinstating a +`-Zpolonius` directive. Delegated prover targets print maintainer diagnostics to standard error before invoking `rust-prover-tools`. Expect `prover-tools:` lines containing the @@ -1389,16 +1422,15 @@ Cargo home plus Kani support-file home. - `make test-nextest` — `cargo nextest run --workspace --all-targets --all-features`, with - `RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)"` (the - Makefile re-states the Polonius flag because a set `RUSTFLAGS` overrides - `.cargo/config.toml`, and the `$${RUSTFLAGS:+$$RUSTFLAGS }` prefix preserves - any `RUSTFLAGS` inherited from the caller). This runs every unit, integration, - `rstest`, and `rstest-bdd` test. + `RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings"` (the + `$${RUSTFLAGS:+$$RUSTFLAGS }` prefix preserves any `RUSTFLAGS` inherited from + the caller). This runs every unit, integration, `rstest`, and `rstest-bdd` + test. - `make doctest` — `cargo test --workspace --doc --all-features`, with - `RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)"`. This - preserves flags inherited from the caller and restores Polonius while denying - warnings. nextest cannot execute doctests, so they need their own pass; the - separate target is what makes a broken documentation example fail the gate. + `RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings"`. This preserves flags + inherited from the caller while denying warnings. nextest cannot execute + doctests, so they need their own pass; the separate target is what makes a + broken documentation example fail the gate. If either pass fails, `make test` fails. Run the individual targets when iterating, but treat `make test` as the gate. @@ -1428,8 +1460,8 @@ governs the non-doctest pass only, and deliberately stays small: nextest runs each test in its own process, but the codebase does not rely on that isolation for environment safety. Tests pass environment values through explicit configuration seams or configure a child with `env_clear()` followed by -`Command::env`. The BDD suite carries no environment or CWD lock: its steps run -inside the generated test-harness process, not inside an `assert_cmd` child. +`Command::env`. The BDD suite carries no environment or CWD lock: its steps +run inside the generated test-harness process, not inside an `assert_cmd` child. `EnvLock` and `CwdGuard` remain only for direct tests that deliberately exercise process working-directory behaviour outside that suite. @@ -1520,30 +1552,29 @@ The config-precedence ladder and display-policy domain are covered by three modules under `tests/cli_tests/`: - `config_precedence_ladder.rs` pins the closed selector model (`--config` > - `NETSUKE_CONFIG` > automatic discovery) end to end and checks that the - merged scalar fields follow CLI > environment > project > discovered - (user/system) > defaults. It includes an explicit guard that the removed - `NETSUKE_CONFIG_PATH` alias is not a selector, even when it names an - existing file with distinct values. + `NETSUKE_CONFIG` > automatic discovery) end to end and checks that the merged + scalar fields follow CLI > environment > project > discovered (user/system) > + defaults. It includes an explicit guard that the removed + `NETSUKE_CONFIG_PATH` alias is not a selector, even when it names an existing + file with distinct values. - `display_policy_domain.rs` exhaustively verifies the consolidated - display-policy resolution (`EmojiPolicy`, `ColourPolicy`, - `ProgressPolicy`, `AccessibilityPolicy`, `json`, `NO_COLOR`, and - `TERM`/output mode) against a handwritten truth model, using one flat - Cartesian-product sweep plus a proptest. It adds coverage only; the - production resolution in `src/theme.rs` and `src/output_prefs.rs` is not - changed. + display-policy resolution (`EmojiPolicy`, `ColourPolicy`, `ProgressPolicy`, + `AccessibilityPolicy`, `json`, `NO_COLOR`, and `TERM`/output mode) against a + handwritten truth model, using one flat Cartesian-product sweep plus a + proptest. It adds coverage only; the production resolution in `src/theme.rs` + and `src/output_prefs.rs` is not changed. - `merge_targets_proptests.rs` holds the handwritten proptest strategies (no `#[derive(Arbitrary)]`) for the `default_targets` append-in-discovery-order invariant and scalar merge ordering (defaults → file → environment → CLI). These tests drive a re-executed worker process through -`tests/cli_tests/merge_probe.rs`. `merge_probe` builds an isolated -environment (`HOME`, `XDG_CONFIG_HOME`, `XDG_CONFIG_DIRS`, and, for the -system-scope variants, a redirectable `XDG_CONFIG_DIRS`) and `merge_in_child` -runs the real ambient adapters in a child process, so the parent harness never -mutates the process environment. The XDG system/user scope scenarios are -Unix-only: Windows discovers configuration through `APPDATA`/`LOCALAPPDATA` -rather than the XDG variables these tests inject. +`tests/cli_tests/merge_probe.rs`. `merge_probe` builds an isolated environment +(`HOME`, `XDG_CONFIG_HOME`, `XDG_CONFIG_DIRS`, and, for the system-scope +variants, a redirectable `XDG_CONFIG_DIRS`) and `merge_in_child` runs the real +ambient adapters in a child process, so the parent harness never mutates the +process environment. The XDG system/user scope scenarios are Unix-only: Windows +discovers configuration through `APPDATA`/`LOCALAPPDATA` rather than the XDG +variables these tests inject. ### Temporary executable test helpers @@ -2066,8 +2097,8 @@ of the contract. BDD steps must not change either process-global value: use an injected environment and absolute paths for in-process library assertions, or an isolated `assert_cmd` child for end-to-end behaviour. `CwdGuard` is the RAII utility for the few direct CWD tests that deliberately exercise it. For -locale-sensitive snapshot tests, use the `EnLocalizer` scoped pattern documented -in the +locale-sensitive snapshot tests, use the `EnLocalizer` scoped pattern +documented in the [snapshot testing guide](snapshot-testing-in-netsuke-using-insta.md#locale-pinned-snapshot-tests). `src/snapshot_test_support.rs` owns output-oriented unit-test fixtures; @@ -2100,7 +2131,7 @@ is not obvious from the name: boolean predicates: `Ok(true)` when the path is a regular file, `Ok(false)` when it is absent (`NotFound` is folded into the boolean result), and `Err` for any other metadata failure, so callers can distinguish absence from - inaccessibility. The binary locator in `test_support/src/netsuke.rs` + inaccessibility. The binary locator in `test_support/src/netsuke/locator.rs` (`netsuke_executable_from`, see [Locating the netsuke binary](#locating-the-netsuke-binary)) relies on it to surface unexpected filesystem errors while probing candidate paths. @@ -2225,52 +2256,36 @@ runs Make, runs Cargo, or writes anything. `tests/makefile_test_target.rs` is the crate root for the Makefile contract tests. It includes `tests/support/makefile.rs` for the capability-scoped read and recipe-lookup helpers, pins the `make test` runner contract, and declares -two child modules that together own the `RUSTFLAGS` contract: - -- `tests/makefile_test_target/rustflags.rs` models every recipe line that - assigns `RUSTFLAGS` as a `RustflagsCase` — the Make target, the substring - selecting the line, and the warning and inheritance policies the line must - uphold. -- `tests/makefile_test_target/rustflags_polonius_tests.rs` covers the - `POLONIUS_FLAGS` resolution guard directly, against synthetic Makefile text. +one child module that owns the `RUSTFLAGS` contract: +`tests/makefile_test_target/rustflags.rs` models every recipe line that assigns +`RUSTFLAGS` as a `RustflagsCase` — the Make target and the substring selecting +the line. + +Every recipe that sets `RUSTFLAGS` now does so for the same reason: to deny +warnings while conditionally preserving an inherited value. Both contracts are +therefore asserted for every case rather than being carried as per-case policy +fields. A recipe needing a different policy fails the assertions instead of +slipping through, which is the signal to reintroduce a policy field rather than +to relax the test. The tests assert on what a shell would produce, not on recipe text. For each -case, `rustflags.rs` extracts the double-quoted assignment, resolves -`$(POLONIUS_FLAGS)`, reduces Make's `$$` escape to the single `$` the shell -receives, and — on Unix only — expands the resulting expression with -`printf '%s'` under `sh`. Only the assignment is expanded; the command the -recipe would run is never executed, so no test here invokes Cargo, Kani, -nextest, or Dylint. Expansion needs a shell, so the behavioural tests and the -policy fields they read are gated on `#[cfg(unix)]`; the parsing tests are not. -Two guards keep the model honest: `shell_expression` refuses an expression -still naming an unresolved Make variable or embedding a shell command -substitution, and a completeness test walks the Makefile and fails when a line -sets `RUSTFLAGS` without a matching case, so a new recipe joins the contract or +case, `rustflags.rs` extracts the double-quoted assignment, reduces Make's `$$` +escape to the single `$` the shell receives, and — on Unix only — expands the +resulting expression with `printf '%s'` under `sh`. Only the assignment is +expanded; the command the recipe would run is never executed, so no test here +invokes Cargo, Kani, nextest, or Dylint. Expansion needs a shell, so the +behavioural tests are gated on `#[cfg(unix)]`; the parsing tests are not. Two +guards keep the model honest: `shell_expression` refuses an expression still +naming an unresolved Make variable or embedding a shell command substitution, +and a completeness test walks the Makefile and fails when a line sets +`RUSTFLAGS` without a matching case, so a new recipe joins the contract or breaks the build. -`polonius_flags(makefile) -> Result` is the crate-visible resolver both -modules share. It reads the `POLONIUS_FLAGS` variable and rejects two states: - -- **Missing** — no `POLONIUS_FLAGS ?= …` or `POLONIUS_FLAGS = …` line, reported - as "Makefile should define POLONIUS_FLAGS". -- **Empty after trimming** — a definition whose value is blank or whitespace - only, reported as "POLONIUS_FLAGS should not be empty". - -The rejection matters because every assertion built on the resolved value uses -`contains`, which an empty needle satisfies vacuously. Returning an empty -string would silently void the Polonius contract instead of failing it. On -success the resolver returns the trimmed value, and a bounded proptest in -`rustflags_polonius_tests` pins that acceptance invariant: resolution succeeds -exactly when a definition exists whose value is non-empty after trimming, and -the resolved text is the trimmed value. Its generator emits absent, -whitespace-only, and whitespace-padded flag tokens, so an untrimmed return -fails the property rather than slipping past. - -Because `rustflags.rs` and `rustflags_polonius_tests.rs` are children of the -`makefile_test_target` test binary rather than files under `tests/`, Cargo does -not compile them as separate targets. The root declares them, and they reach -the shared helpers through `use super::{read_repo_file, target_recipe}`. Keep -this shape for further Makefile contract work: general parsing helpers belong in +Because `rustflags.rs` is a child of the `makefile_test_target` test binary +rather than a file under `tests/`, Cargo does not compile it as a separate +target. The root declares it, and it reaches the shared helpers through +`use super::{read_repo_file, target_recipe}`. Keep this shape for further +Makefile contract work: general parsing helpers belong in `tests/support/makefile.rs` once a second contract test needs them, whereas model types such as `RustflagsCase` stay private to the contract they describe. @@ -2466,13 +2481,55 @@ property tests. `cargo build --message-format=json` and parses the resulting Cargo JSON messages rather than assuming its dependencies sit beside the uplifted `test_support` rlib. For every `compiler-artifact` message it records the -parent directory of each rlib the message names, and passes the whole set to -`rustc` as `-L dependency=` directories when compiling the UI fixtures. This -keeps the harness correct when Cargo's `build.build-dir` setting splits -intermediate artefacts — where dependency rlibs live — from the final, uplifted -ones; deriving the directories from what Cargo actually reports, rather than -from a single assumed location, means the harness does not need to special-case -that split. +parent directory of each loadable artefact the message names, and passes the +whole set to `rustc` as `-L dependency=` directories when compiling the UI +fixtures. This keeps the harness correct when Cargo's `build.build-dir` setting +splits intermediate artefacts — where dependency rlibs live — from the final, +uplifted ones, and when Cargo gives each crate its own build directory instead +of one shared `deps/`, as the Cargo shipped with the 1.99 nightlies does. +Deriving the directories from what Cargo actually reports, rather than from a +single assumed location, means the harness does not need to special-case either +layout. + +"Loadable artefact" means an rlib, an `.rmeta` metadata file, or a file with +the platform's dynamic-library extension. The `.rmeta` file is needed for the +metadata-only direct-`rustc` checks because Cargo builds with +`-Zembed-metadata=no`; the parser prefers it while retaining an rlib fallback +for older layouts. The dynamic-library case matters too: proc-macro crates emit +a host dynamic library rather than an rlib. A shared `deps/` directory used to +pick them up for free, so an rlib-only filter went unnoticed; once each crate +has its own directory, a filtered-out proc macro is simply absent from the +search path and its dependents fail with `E0463`. The same rule and the same +reasoning apply to `tests/command_env_ui_tests.rs`, which builds the `netsuke` +rlib and derives its search path the same way. + +The shared `tests/support/cargo_artifacts.rs` module owns parsing Cargo +`compiler-artifact` messages and extracting loadable artefact directories. It +may be included only by these direct-`rustc` UI harnesses; the callers retain +build and process-spawn orchestration. + +Those arguments reach `rustc` through a **response file**, not the command +line. One `-L dependency=` pair per crate, over the long unique roots the +split-build test creates, pushed the Windows `CreateProcessW` command line past +its 32,767-character limit; the spawn then failed with +`Os { code: 206, kind: InvalidFilename }` before `rustc` ran at all. Every +directory is required to avoid `E0463`, so the list had to move off the command +line rather than be shortened or deduplicated further. `rustc` reads arguments +from `@` — UTF-8, one argument per line, no quoting — which leaves each +harness passing exactly one argument, so command-line length no longer scales +with the dependency count. + +`tests/support/rustc_response_file.rs` owns that rendering and is included by +both harnesses through the usual `#[path = …] mod …;` pattern. Its scope is +deliberately narrow: it renders an argument vector and writes it, and knows +nothing about what a compilation needs. Reach for it from a `tests/*.rs` binary +that invokes `rustc` directly with an argument list whose length is not bounded +by the source; a harness passing a fixed handful of arguments does not need it. +Its unit tests assert the file's shape — one argument per line, spaces +preserved without quoting, newlines rejected, and every source, `--extern`, +dependency-search, and output argument retained — because the failure it +prevents is Windows-specific and cannot be reproduced on the hosts that run +most of this suite. `harness_compiles_under_a_split_build_dir` is the regression test for this: it forces a split layout with its own private `CARGO_TARGET_DIR` and @@ -2537,11 +2594,10 @@ general path-configuration API. Ownership and permitted call sites: - Production passes `None`; the sole caller is `from_path_with_registration` in - `src/manifest/query.rs`, so ambient current-directory resolution is - unchanged. + `src/manifest/query.rs`, so ambient current-directory resolution is unchanged. - Tests inject a temporary directory or a relative base such as - `Some(Path::new("."))`; the seam must not become a general - path-configuration API. + `Some(Path::new("."))`; the seam must not become a general path-configuration + API. Composition rules: @@ -2597,8 +2653,7 @@ order: `CwdGuard` restores the CWD first, and `EnvLock` releases second. These direct CWD tests are the narrow exception for exercising CWD-dependent code itself. BDD scenarios instead retain an absolute manifest path or pass -`-C/--directory` into the CLI; neither approach changes the harness process -CWD. +`-C/--directory` into the CLI; neither approach changes the harness process CWD. ### Injected and child-process environments @@ -3402,7 +3457,11 @@ any scenario that requires a hermetic child environment. #### Locating the netsuke binary Both `run_netsuke_in` and `run_netsuke_in_with_env` depend on a private locator, -`netsuke_executable()`, to find the built `netsuke` binary. +`netsuke_executable()`, to find the built `netsuke` binary. It lives in +`test_support/src/netsuke/locator.rs`, separate from the parent +`test_support::netsuke` module that runs the binary: the locator is pure path +reasoning over an injected environment, with a single existence probe as its +only filesystem contact, whereas the parent spawns processes. `netsuke_executable()` converts `std::env::current_exe()` to a `camino::Utf8PathBuf` and delegates to `netsuke_executable_from`, which takes an injected `mockable::Env` — the same injectable-environment pattern used by @@ -3412,21 +3471,32 @@ environment. The locator checks candidate paths in order: -1. beside the test executable, using its directory with any trailing `deps` - component stripped; +1. the profile directory above the test executable, derived by `profile_dir`; 2. `CARGO_TARGET_DIR//`, needed when Cargo's `build.build-dir` configuration splits intermediate artefacts — where test executables run — from the uplifted binary, which lands under the target directory; 3. `CARGO_TARGET_DIR///`, for `--target` builds where the profile directory nests under the target triple. +`profile_dir` exists because Cargo has moved integration-test executables +between two layouts, and the binary is uplifted to the profile directory in +both. It strips a trailing `deps` component, the long-standing layout; or a +trailing `build///out`, the layout used by the Cargo shipped +with the 1.99 nightlies, which has no `deps` directory at all. Any other shape +is left alone, so an unrecognized layout degrades to looking beside the +executable rather than failing outright. The same derived directory supplies the +`` component of the two fallbacks, so both layouts spell them +identically. + Filesystem errors other than "not found" are surfaced rather than treated as a missing candidate, via the [`test_support::fs`](#test_supportfs) wrapper `try_is_file`. When every candidate misses, the resulting error lists all attempted paths. -The locator's unit tests live in `test_support/src/netsuke.rs` and cover the -primary lookup, both fallback paths, and the missing-binary case. +The locator's unit tests live beside it in +`test_support/src/netsuke/locator.rs` and cover the primary lookup under both +executable layouts, an `out` directory that is *not* the Cargo build layout, +both fallback paths, and the missing-binary case. ## Digest rendering @@ -3503,13 +3573,12 @@ is what distinguishes the versions, and it is what these guards check. This guard replaced an earlier `trybuild` compile-fail harness. Trybuild always builds the host crate as a fixture dependency while discarding workspace -`build.rustflags`, so once `main` adopted the Polonius nightly toolchain it -rebuilt `netsuke` without `-Zpolonius=next`; see the "Harness consequences" -section of `docs/polonius.md`, which asks that trybuild cases depending on the -`netsuke` crate not be reintroduced while the tree is Polonius-only. The -compile-time probe is also strictly better on its own merits: no subprocess, no -scratch project, and no toolchain-sensitive `.stderr` snapshot to re-bless on -every compiler bump. +`build.rustflags`, so while Polonius was flag-gated it rebuilt `netsuke` +without the analysis; see the "Harness consequences" section of +`docs/polonius.md`. The pinned nightly now enables Polonius by default, so that +specific hazard is gone, but the compile-time probe is better on its own +merits: no subprocess, no scratch project, and no toolchain-sensitive `.stderr` +snapshot to re-bless on every compiler bump. `stdlib::path::hash_utils` unit-tests the chunked streaming loop against a one-shot digest for inputs that span more than one 8192-byte read, plus a @@ -3748,25 +3817,25 @@ background-query primitive. - **Ownership:** `runner::generation` is a private runner submodule. It owns the three read-only generation steps, the explicitly effectful build loader, - their input/output hand-offs, and the manifest and IR error contexts. It - does not own `StatusReporter` updates, command dispatch, dyndep publication, - or Ninja execution. + their input/output hand-offs, and the manifest and IR error contexts. It does + not own `StatusReporter` updates, command dispatch, dyndep publication, or + Ninja execution. - **Permitted call-sites:** `runner::generate_ninja` composes the complete build pipeline through `load_manifest_for_build` for build, clean, and generate commands. `runner::graph::handle_graph` may stop after `build_graph` to render the graph, and `runner::help_query` uses `load_manifest` for its - read-only target catalogue. Runner unit tests may compose the read-only - steps directly. New dry-run or background-generation work may use - `load_manifest`, `build_graph`, and `ninja_text` only within the runner - boundary; a public or cross-subsystem consumer requires an explicit - application boundary rather than widening these internal helpers. + read-only target catalogue. Runner unit tests may compose the read-only steps + directly. New dry-run or background-generation work may use `load_manifest`, + `build_graph`, and `ninja_text` only within the runner boundary; a public or + cross-subsystem consumer requires an explicit application boundary rather + than widening these internal helpers. - **Composition rules:** command adapters report stages before or after the relevant step and wrap `ninja_text` with runner-owned generation telemetry. Only `load_manifest_with_stage_reporting` translates `StageObserver` events into status updates and selects the effectful build loader. Consumers must not call manifest parsing, IR generation, or `ninja_gen::generate_bundle` - directly in parallel with this pipeline. Before an adapter writes or - executes a returned bundle, it must use the existing capability-injected + directly in parallel with this pipeline. Before an adapter writes or executes + a returned bundle, it must use the existing capability-injected dyndep-publication path to materialize its sidecars; the read-only steps never write files, start processes, or invoke effectful template helpers. diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 2bbcb0ce3..c27728662 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -2044,7 +2044,7 @@ This transformation involves several steps: and visitation map. Keys are cloned from the `targets` map so traversal leaves the input graph untouched. Missing dependencies encountered during traversal are logged, collected, and returned alongside any cycle to aid -diagnostics. + diagnostics. ### 5.4 Ninja file synthesis (`src/ninja_gen.rs`) @@ -2228,19 +2228,19 @@ child process's execution environment. Every invocation is described by a borrowed request bundle rather than a long parameter list: `NinjaBuildRequest` for a build and `NinjaToolRequest` for -`ninja -t `. Each names the resolved program, `NinjaProcessOptions` -(an optional UTF-8 working directory and job count), the generated build file, -the targets or tool, a `&CommandEnv` describing the child's environment, and the -`stderr_mode: StderrMode` policy field. -`run_ninja_with` and `run_ninja_tool_with` consume these; the convenience -wrappers `run_ninja` and `run_ninja_tool` live in -`runner::ninja_process_adapter`, translate `Cli` state at the runner boundary, -call them with `CommandEnv::inherit()`, and derive the `stderr_mode` policy -from the CLI via `StderrMode::from_json_enabled(cli.json)`, which is production -behaviour. Process requests never import `Cli`; callers without parser state -construct `NinjaProcessOptions` directly. -The adapter converts `Cli::directory` to the UTF-8 path at this boundary and -returns `io::ErrorKind::InvalidData` if the CLI path is not valid UTF-8. +`ninja -t `. Each names the resolved program, `NinjaProcessOptions` (an +optional UTF-8 working directory and job count), the generated build file, the +targets or tool, a `&CommandEnv` describing the child's environment, and the +`stderr_mode: StderrMode` policy field. `run_ninja_with` and +`run_ninja_tool_with` consume these; the convenience wrappers `run_ninja` and +`run_ninja_tool` live in `runner::ninja_process_adapter`, translate `Cli` state +at the runner boundary, call them with `CommandEnv::inherit()`, and derive the +`stderr_mode` policy from the CLI via +`StderrMode::from_json_enabled(cli.json)`, which is production behaviour. +Process requests never import `Cli`; callers without parser state construct +`NinjaProcessOptions` directly. The adapter converts `Cli::directory` to the +UTF-8 path at this boundary and returns `io::ErrorKind::InvalidData` if the CLI +path is not valid UTF-8. The private `run_ninja_internal` helper takes a `NinjaInternalRequest`, a clock, and a `configure` closure. The request groups the resolved program, @@ -2954,14 +2954,15 @@ Diagnostic-mode resolution uses `(OrthoResult, DiscoveryOutcome)` without emitting diagnostics. The composition boundary calls `DiscoveryOutcome::emit_diagnostics()` after tracing is configured, replaying the retained diagnostics without repeating environment -or filesystem access. `collect_file_layers_with_normalizer_and_trace(directory, -normalizer, env_source)` performs the underlying discovery scan with the path -normalizer and environment source, retaining bounded project-scope trace -metadata. The normalizer canonicalizes comparison keys so equivalent project -path spellings de-duplicate to one layer. `DiscoveryOutcome::into_layers()` -transfers the same discovered layers to `merge_with_cached_file_layers(...)`, -which consumes them for the -full merge and prevents a second discovery pass. The standalone +or filesystem access. +`collect_file_layers_with_normalizer_and_trace(directory, +normalizer, env_source)` +performs the underlying discovery scan with the path normalizer and +environment source, retaining bounded project-scope trace metadata. The +normalizer canonicalizes comparison keys so equivalent project path spellings +de-duplicate to one layer. `DiscoveryOutcome::into_layers()` transfers the same +discovered layers to `merge_with_cached_file_layers(...)`, which consumes them +for the full merge and prevents a second discovery pass. The standalone `merge_with_config_and_env(...)` path performs discovery, emits diagnostics and delegates to `merge_with_cached_file_layers(...)`. @@ -3332,16 +3333,16 @@ selected for this project and the rationale for their inclusion. | Logging | tracing | Structured, levelled diagnostic output for debugging and insight. | | Versioning | semver | The standard library for parsing and evaluating Semantic Versioning strings, essential for the `netsuke_version` field. | -Netsuke compiles with the Polonius alpha borrow-checking analysis -(`-Zpolonius=next`) on the dated nightly toolchain pinned in -`rust-toolchain.toml` ([ADR-006](adr-006-adopt-polonius-nightly-toolchain.md)). -Internal APIs follow a borrow-centric design contract: lookups and registries -return references (`&mut V` accessors with clone-on-miss keys), mutation -happens in place, and error context is built lazily on the failure path. -Owned-value style is reserved for genuine constraints — aliasing, suspension -points, thread and process boundaries, and persistent identity — and each such -refusal is recorded in the [polonius migration notes](polonius.md) alongside -the sites that depend on the analysis. +Netsuke compiles with the Polonius alpha borrow-checking analysis, which the +dated nightly toolchain pinned in `rust-toolchain.toml` enables by default +([ADR-006](adr-006-adopt-polonius-nightly-toolchain.md)). Internal APIs follow +a borrow-centric design contract: lookups and registries return references +(`&mut V` accessors with clone-on-miss keys), mutation happens in place, and +error context is built lazily on the failure path. Owned-value style is +reserved for genuine constraints — aliasing, suspension points, thread and +process boundaries, and persistent identity — and each such refusal is recorded +in the [polonius migration notes](polonius.md) alongside the sites that depend +on the analysis. ### 9.3 Future Enhancements diff --git a/docs/polonius.md b/docs/polonius.md index 6e31ac552..cf7893fad 100644 --- a/docs/polonius.md +++ b/docs/polonius.md @@ -1,12 +1,18 @@ # Polonius migration notes -Netsuke compiles with the Polonius alpha borrow-checking analysis -(`-Zpolonius=next`) on the dated nightly pinned in `rust-toolchain.toml`. +Netsuke compiles with the Polonius alpha borrow-checking analysis, which the +dated nightly pinned in `rust-toolchain.toml` enables by default. [ADR-006](adr-006-adopt-polonius-nightly-toolchain.md) records the toolchain policy; this document records the audit that motivated it, the API evolutions it enabled, and the refusals that bound it. Issue [#465](https://github.com/leynos/netsuke/issues/465) tracked the migration. +The migration originally ran against an opt-in `-Zpolonius=next` directive. +Nightly toolchains dated 2026-08-04 and later enable Polonius by default, and +the directive is being retired, so the tree passes it nowhere. Historical +references to the flag below describe how a classification was made at the +time, not a build setting that still applies. + ## Method The migration ran the `nll-to-polonius` two-pass audit with the compiler as the @@ -20,14 +26,19 @@ oracle: id/index indirection, clone-modify-writeback, snapshot-collect loops, and per-module clone hotspots. -Every change was compiled twice on `nightly-2026-06-25`: once with -`-Zpolonius=next` (must pass) and once without. The no-flag compile exists only -to classify the individual change: a failure proves the design genuinely -depends on Polonius and the site is tagged `POLONIUS(...)`; success means the -old form was habit rather than necessity and the improvement carries no -toolchain caveat. The complete behavioural test suite runs under -`-Zpolonius=next` — the tree's only supported configuration — and was required -to pass unchanged after every change. +Every change was compiled twice on `nightly-2026-06-25`, where the analysis was +still opt-in: once with `-Zpolonius=next` (must pass) and once without. The +no-flag compile existed only to classify the individual change: a failure +proves the design genuinely depends on Polonius and the site is tagged +`POLONIUS(...)`; success means the old form was habit rather than necessity and +the improvement carries no toolchain caveat. The complete behavioural test +suite runs under Polonius — the tree's only supported configuration — and was +required to pass unchanged after every change. + +Classifying a new site the same way now means compiling it against a +pre-2026-08-04 nightly, which is the last configuration that still applies NLL. +`-Zpolonius=legacy` does not restore NLL. That comparison is a one-off +diagnostic, not a build setting: the tree itself is Polonius-only. ## Polonius-dependent sites @@ -41,7 +52,7 @@ well — the owned style was habit, so they carry no toolchain caveat: - `src/graph_view/mod.rs` — `NodePathRegistry::ensure_node_mut` uses `hashbrown::HashMap::entry_ref` for a borrowed single lookup. It returns `&mut NodeKind` and allocates an owned path only for a vacant entry, while - compiling both with and without `-Zpolonius=next`. + compiling under both borrow checkers. - `src/stdlib/collections.rs` — `group_by_filter` consumed its resolved key in `entry(key_value)` instead of cloning it first. - `src/ir/cycle.rs` — `detect_targets` snapshots borrowed @@ -86,48 +97,55 @@ Scanner suspects that turned out not to be NLL residue: - Test-suite `drop()` calls (environment guards, HTTP fixture teardown) are semantic Drop effects, not borrow appeasement. -The plumbing itself is contract-tested: `tests/polonius_toolchain_contract.rs` -pins the dated-nightly channel, the `.cargo/config.toml` `build.rustflags` -entry, the `POLONIUS_FLAGS` default and every RUSTFLAGS-setting Makefile -recipe, and the shared-action `with.rustflags` and toolchain inputs in the CI, -Netsukefile, coverage, and packaging workflows. +The toolchain policy is contract-tested: `tests/polonius_toolchain_contract.rs` +requires the pinned channel to be a dated nightly at or after 2026-08-04, fails +if any build configuration reintroduces a `-Zpolonius` directive, and pins the +shared-action `with.rustflags` and toolchain inputs in the CI, Netsukefile, +coverage, and packaging workflows. The `RUSTFLAGS` shape of each Makefile +recipe is covered separately by `tests/makefile_test_target.rs`. ## Harness consequences -Tooling that rebuilds the crate with its own flags must propagate the Polonius -flag or avoid compiling the crate: +Because the analysis rides on the toolchain rather than on a flag, tooling only +has to use the pinned toolchain; nothing needs to propagate a build setting: - **trybuild** discards ambient `RUSTFLAGS` and workspace `build.rustflags`, replacing them via `--config` on its scratch project, and it always builds - the host crate as a fixture dependency. The Kani cfg policy fixture is - therefore compiled and run directly with the workspace `rustc` - (`tests/kani_cfg_ui_tests.rs`); do not reintroduce trybuild cases that depend - on the `netsuke` crate while the tree is Polonius-only. -- **Kani** and **Whitaker** run under their own toolchains but read the - workspace `.cargo/config.toml` or the Makefile `RUSTFLAGS`, so they - borrow-check with `-Zpolonius=next` and need no special handling. + the host crate as a fixture dependency. While Polonius was flag-gated that + broke every fixture depending on `netsuke`, so the Kani cfg policy fixture is + compiled and run directly with the workspace `rustc` + (`tests/kani_cfg_ui_tests.rs`). That specific hazard is gone, but trybuild + still needs a scratch project and a toolchain-sensitive `.stderr` snapshot, + so the direct-compile harnesses stay. +- **Whitaker** runs its Dylint driver on a nightly of its own and needs no + Polonius-specific handling. +- **Kani** manages a supporting nightly during `cargo kani setup`, and that + nightly can be older than the repository's. Kani 0.67.0 uses + `nightly-2025-11-21`, which predates the Polonius default, so + `make kani-full` borrow-checks under NLL. With no tagged sites in the tree + this costs nothing today; should a `POLONIUS(...)` site fail to verify, move + Kani to a build whose nightly is 2026-08-04 or later rather than reinstating a + `-Zpolonius` directive. - **CI setup actions**: the shared `setup-rust` and `rust-build-release` - actions receive the Polonius flags through their `with.rustflags` inputs; - workflows must not set a job-level `env.RUSTFLAGS`. CI and coverage pass - `-D warnings -Zpolonius=next`, while Netsukefile tests and packaging pass - `-Zpolonius=next`. The coverage action's `cargo-llvm-cov` invocation inherits - the flags exported by `setup-rust` and appends its instrumentation flags. - Makefile recipes still append `POLONIUS_FLAGS` when they set ambient - `RUSTFLAGS`. The per-workflow values, the `NETSUKE_RUST_TOOLCHAIN` policy and - the reason the contract test pins each action's exact revision are set out in - the developer guide under [Polonius CI shared-action - contract](developers-guide.md#polonius-ci-shared-action-contract). -- **Registry installs**: the crates.io package excludes - `rust-toolchain.toml` and `.cargo/config.toml`, and registry builds run - outside the checkout, so `cargo install netsuke-build` must select the pinned - nightly and pass the flag explicitly - (`RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke-build`). - The README and users' guide document the command and - `tests/documentation_examples_tests.rs` pins it. + actions export their own `RUSTFLAGS`, so anything a job needs travels through + their `with.rustflags` inputs and workflows must not set a job-level + `env.RUSTFLAGS`. CI and coverage pass `-D warnings`; Netsukefile tests and + packaging pass no `rustflags` at all. The coverage action's `cargo-llvm-cov` + invocation inherits the flags exported by `setup-rust` and appends its + instrumentation flags. The per-workflow values, the `NETSUKE_RUST_TOOLCHAIN` + policy and the reason the contract test pins each action's exact revision are + set out in the developer guide under + [Polonius CI shared-action contract](developers-guide.md#polonius-ci-shared-action-contract). +- **Registry installs**: the crates.io package excludes `rust-toolchain.toml`, + and registry builds run outside the checkout, so + `cargo install netsuke-build` must select the pinned nightly explicitly + (`cargo +nightly-2026-08-23 install netsuke-build`). The README and users' + guide document the command and `tests/documentation_installation_tests.rs` + pins it. - **cargo-mutants** (scheduled, informational) runs through the shared `mutation-cargo.yml` workflow, which controls its own environment; if those - runs regress with E0499 at tagged sites, the shared workflow needs the same - `RUSTFLAGS` treatment. + runs regress with E0499 at tagged sites, check that the shared workflow uses + the pinned toolchain. ## Clone counts @@ -148,11 +166,11 @@ projection maps and owned metadata — data ownership, not workaround shapes. ## Stabilization -When `-Zpolonius=next` (or its successor) reaches stable Rust: +The flag plumbing is already gone; the analysis is the nightly default. When it +reaches stable Rust: -1. Move `rust-toolchain.toml` to the stabilizing release and delete the - `[build] rustflags` entry in `.cargo/config.toml` plus the Makefile - `POLONIUS_FLAGS` variable. +1. Move `rust-toolchain.toml` to the stabilizing release, and relax the + dated-nightly lower bound in `tests/polonius_toolchain_contract.rs`. 2. Re-declare `rust-version` in `Cargo.toml` at that release. 3. Keep the `POLONIUS(...)` tags: they still explain why the shape exists; reword "nightly-only" phrasing in the ADR and guides. @@ -172,5 +190,5 @@ The contract for new code and reviews (also summarized in `AGENTS.md` and the locks, aliasing, suspension points, thread boundaries) is permanent, and "simplifying" those sites into reference-returning forms will not compile or will break the design. -- Classify any new borrow-centric API by compiling with and without the - flag, then record it here. +- Classify any new borrow-centric API by compiling it against a pre-2026-08-04 + nightly as well as the pinned one, then record it here. diff --git a/docs/quickstart.md b/docs/quickstart.md index 856c429c0..d6dd90055 100644 --- a/docs/quickstart.md +++ b/docs/quickstart.md @@ -14,8 +14,9 @@ Before beginning, ensure the following are available: `cargo install --path .` (which puts `netsuke` on `PATH` for the commands below), or follow the registry-install command in the [users' guide](users-guide.md#install-netsuke). A bare `cargo install` is - unsupported: registry builds need the pinned nightly toolchain and the - Polonius borrow-checker flag supplied explicitly, as the guide shows. + unsupported: registry builds need the pinned nightly toolchain — which is + what enables the Polonius borrow checker — selected explicitly, as the guide + shows. - **Ninja** build tool in the system PATH (install via the package manager, e.g., `apt install ninja-build` or `brew install ninja`) diff --git a/docs/users-guide.md b/docs/users-guide.md index 4d7b9eaf8..7182b11e8 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -11,12 +11,12 @@ may change before 1.0. Pin the Netsuke version in automated workflows. ## Install Netsuke Netsuke requires [Ninja](https://ninja-build.org/) on `PATH`. A source build -also requires the dated Rust nightly toolchain pinned in `rust-toolchain.toml` -because Netsuke builds with the Polonius borrow checker (`-Zpolonius=next`). +also requires the dated Rust nightly toolchain pinned in `rust-toolchain.toml`, +because Netsuke builds with the Polonius borrow checker, which nightly enables +by default. -Inside a checkout both settings are inherited automatically: `rustup` installs -the pinned toolchain, and the repository's `.cargo/config.toml` supplies -`RUSTFLAGS=-Zpolonius=next`. Neither has to be passed on the command line. +Inside a checkout, `rustup` automatically selects the pinned toolchain from +`rust-toolchain.toml`; no command-line argument is required. Netsuke v0.1.0-beta2 is available from crates.io. Where [`cargo binstall`](https://github.com/cargo-bins/cargo-binstall) is available, @@ -29,15 +29,14 @@ requirement below. cargo binstall netsuke-build ``` -Building from the registry instead runs outside a repository checkout, so -neither the pinned toolchain nor the Polonius flag is picked up automatically; -supply both explicitly: +Building from the registry instead runs outside a repository checkout, so the +pinned toolchain is not picked up automatically; select it explicitly: ```sh -rustup toolchain install nightly-2026-06-25 -RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke-build +rustup toolchain install nightly-2026-08-23 +cargo +nightly-2026-08-23 install netsuke-build ``` Pre-built installers are available from the @@ -86,9 +85,9 @@ the shell's documented completion mechanism. The package installation commands above do not install completion files; completion directory names and activation steps vary by shell and platform. -Install the current source checkout with Cargo. The clone supplies both the -pinned nightly toolchain and `RUSTFLAGS=-Zpolonius=next`, so neither is given -here — unlike the registry install above, which runs outside a checkout: +Install the current source checkout with Cargo. The clone supplies the pinned +nightly toolchain, so it is not given here — unlike the registry install above, +which runs outside a checkout: diff --git a/docs/v0-1-0-migration-guide.md b/docs/v0-1-0-migration-guide.md index cf4ca7713..b4ba9a59c 100644 --- a/docs/v0-1-0-migration-guide.md +++ b/docs/v0-1-0-migration-guide.md @@ -14,6 +14,18 @@ remain compatible. Callers constructing `NinjaBuildRequest` or `NinjaToolRequest` must replace `cli: &cli` with `options: &options`; every other addition is opt-in. +## Select the pinned Rust toolchain + +Source builds from a checkout require the dated nightly pinned in +`rust-toolchain.toml`. Inside the checkout, `rustup` selects that toolchain +automatically, so no command-line argument is required. Registry installs run +outside the checkout and must select the same nightly explicitly: + +```sh +rustup toolchain install nightly-2026-08-23 +cargo +nightly-2026-08-23 install netsuke-build +``` + ## Netsuke is a build tool, not a library Netsuke is intended to be used as a command-line build tool. The only surfaces diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 03ad4a7cc..ca2b2795b 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,7 +1,7 @@ -# Netsuke builds with the Polonius alpha borrow-checking analysis -# (`-Zpolonius=next`), which is nightly-only. The dated pin keeps builds -# reproducible; see docs/adr-006-adopt-polonius-nightly-toolchain.md before -# changing the channel. +# Netsuke builds with the Polonius alpha borrow-checking analysis, which is +# enabled by default on nightly toolchains from 2026-08-04 onwards and so is +# nightly-only. The dated pin keeps builds reproducible; see +# docs/adr-006-adopt-polonius-nightly-toolchain.md before changing the channel. [toolchain] -channel = "nightly-2026-06-25" +channel = "nightly-2026-08-23" components = ["rustfmt", "clippy", "rust-analyzer"] diff --git a/scripts/dev-fast-common.sh b/scripts/dev-fast-common.sh index 5ce64eea5..12bdab4bb 100644 --- a/scripts/dev-fast-common.sh +++ b/scripts/dev-fast-common.sh @@ -76,9 +76,9 @@ mold_version() { read_pin "$MOLD_VERSION_FILE"; } # The repository's toolchain, read from `rust-toolchain.toml`. # # Deliberately the same toolchain the ordinary gates use, not a second pin. -# The tree borrow-checks only under Polonius on that dated nightly (ADR-006), -# so a separate dev-fast nightly would let the fast loop and the gate disagree -# about which borrows are legal. +# The tree borrow-checks only under Polonius, which that dated nightly enables +# by default (ADR-006), so a separate dev-fast nightly could let the fast loop +# and the gate disagree about which borrows are legal. cranelift_toolchain() { local file=$RUST_TOOLCHAIN_FILE value [ -f "$file" ] || fail "missing version pin: $file" diff --git a/scripts/doc-coverage.py b/scripts/doc-coverage.py index b3828617c..9faeac204 100644 --- a/scripts/doc-coverage.py +++ b/scripts/doc-coverage.py @@ -22,274 +22,19 @@ from __future__ import annotations import argparse -import dataclasses as dc -import json -import os import pathlib -import subprocess import sys -import tomllib import typing as typ +import doc_coverage_runner as runner +from doc_coverage_model import DocTarget + if typ.TYPE_CHECKING: import collections.abc as cabc REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent -def cargo_executable() -> str: - """Return the configured Cargo executable. - - The Makefile exposes a ``CARGO`` override that every Cargo-backed target - honours, so the coverage script reads it from the environment and falls - back to ``cargo`` on ``PATH``. Hard-coding the program would run the wrong - wrapper or tool installation wherever the Makefile is invoked with a - custom executable. - """ - return os.environ.get("CARGO") or "cargo" - - -@dc.dataclass(frozen=True) -class Coverage: - """Counts of Rustdoc-measured items for one documentation run.""" - - total: int - with_docs: int - - @property - def percentage(self) -> float: - """Return the share of documented items as a percentage. - - An empty run is treated as complete rather than dividing by zero; a - crate with no doc-able targets contributes nothing either way. - """ - return 100.0 * self.with_docs / self.total if self.total else 100.0 - - def __add__(self, other: Coverage) -> Coverage: - """Return the sum of two coverage counts.""" - return Coverage(self.total + other.total, self.with_docs + other.with_docs) - - -@dc.dataclass(frozen=True) -class DocTarget: - """One target among a package's doc-able (library or binary) targets. - - Parameters - ---------- - package - Cargo package name the target belongs to. - kind - ``"lib"`` for a library target, ``"bin"`` for a binary target. - name - Binary target name, or ``None`` for a library. - """ - - package: str - kind: str - name: str | None - - -def pinned_toolchain(manifest_root: pathlib.Path) -> str: - """Return the ``channel`` pinned in the repository's toolchain file.""" - try: - with (manifest_root / "rust-toolchain.toml").open("rb") as toolchain: - return tomllib.load(toolchain)["toolchain"]["channel"] - except (OSError, tomllib.TOMLDecodeError, KeyError) as error: - detail = f"cannot read the pinned toolchain from rust-toolchain.toml: {error}" - raise RuntimeError(detail) from error - - -def doc_targets(metadata: dict) -> list[DocTarget]: - """Derive the library and binary targets of every workspace member. - - Membership is taken from ``workspace_members`` so dependency crates - outside the workspace are never measured. Build scripts, integration - tests, examples, and benches are skipped: Rustdoc coverage is defined for - the shipped library and binary surfaces, and test code is excluded by - repo convention (see AGENTS.md). - - The expected shape comes from ``cargo metadata --format-version 1``. A - response that is valid JSON but lacks the workspace keys is a broken - measurement, so it is rejected with an explicit error rather than crashing - on a ``KeyError``. An individual package record that omits its ``id`` or - ``targets`` keys simply contributes no targets to the aggregate. - """ - try: - members = set(metadata["workspace_members"]) - packages = metadata["packages"] - except (KeyError, TypeError) as error: - detail = "cargo metadata response lacks the workspace packages or members" - raise RuntimeError(detail) from error - return [ - doc - for package in packages - if "id" in package and "targets" in package - if package["id"] in members - for ordinal in package["targets"] - for doc in doc_able_targets(package, ordinal) - ] - - -def doc_able_targets(package: dict, target: dict) -> list[DocTarget]: - """Map one package ordinal target to a doc target, or none. - - Returns an empty list for targets that do not count toward Rustdoc - coverage: build scripts, tests, examples, and benches. Cargo reports a - target's kinds as a list, so both `lib` and `bin` can be matched without - guessing at the shape of a single-kind target. - """ - kinds: list[str] = target.get("kind", []) - if "lib" in kinds: - return [DocTarget(package["name"], "lib", None)] - if "bin" in kinds: - return [DocTarget(package["name"], "bin", target["name"])] - return [] - - -def rustdoc_args(target: DocTarget, toolchain: str) -> list[str]: - """Build the cargo rustdoc coverage command for one target. - - Parameters - ---------- - target - The library or binary target to measure. - toolchain - The ``+channel`` selector passed to Cargo. - - Returns - ------- - list[str] - The complete argument vector, starting with the ``cargo + - rustdoc -p `` invocation, followed by the library or binary - selector and then the Rustdoc coverage flags in their fixed order. - """ - args = [cargo_executable(), f"+{toolchain}", "rustdoc", "-p", target.package] - if target.kind == "bin": - args += ["--bin", target.name] - else: - args += ["--lib"] - args += [ - "--", - "-Z", - "unstable-options", - "--show-coverage", - "--output-format", - "json", - "--document-private-items", - ] - return args - - -def parse_coverage_output(target: DocTarget, output: str) -> Coverage: - """Decode one rustdoc coverage payload and sum its per-file counts. - - Parameters - ---------- - target - The target whose JSON payload is parsed; named only in the error - diagnostic so failures identify the measured crate. - output - The ``--show-coverage --output-format json`` document rustdoc wrote - to stdout. - - Returns - ------- - Coverage - The aggregate ``total`` and ``with_docs`` counts across every file - entry in the payload. - - Raises - ------ - RuntimeError - When ``output`` is invalid JSON or does not have Rustdoc's per-file - object shape, with the ``did not emit coverage JSON`` diagnostic - naming the target. - """ - try: - per_file = json.loads(output) - except json.JSONDecodeError as error: - raise coverage_json_error(target, str(error)) from error - try: - return aggregate_coverage_payload(per_file) - except (KeyError, TypeError, ValueError, OverflowError) as error: - detail = str(error) - if detail != "expected an object": - detail = f"each entry requires total and with_docs: {error}" - raise coverage_json_error(target, detail) from error - - -def coverage_json_error(target: DocTarget, detail: str) -> RuntimeError: - """Build a measurement error naming `target` and its invalid JSON detail.""" - message = ( - f"cargo rustdoc for {target.package} {target.kind}" - f" ({target.name or 'lib'}) did not emit coverage JSON: {detail}" - ) - return RuntimeError(message) - - -def aggregate_coverage_payload(per_file: object) -> Coverage: - """Validate and sum Rustdoc's documented and total counts.""" - match per_file: - case dict() as entries: - return sum( - (coverage_from_entry(entry) for entry in entries.values()), - Coverage(0, 0), - ) - case _: - raise TypeError("expected an object") - - -def coverage_from_entry(entry: object) -> Coverage: - """Validate one Rustdoc coverage entry and convert it to `Coverage`.""" - total = coverage_count(entry, "total") - with_docs = coverage_count(entry, "with_docs") - if with_docs > total: - raise ValueError("counts must be non-negative integers with with_docs <= total") - return Coverage(total, with_docs) - - -def coverage_count(entry: object, name: str) -> int: - """Convert and validate one Rustdoc coverage count.""" - count = entry[name] - if isinstance(count, bool) or not isinstance(count, int): - raise ValueError("counts must be non-negative integers with with_docs <= total") - if count < 0: - raise ValueError("counts must be non-negative integers with with_docs <= total") - return int(count) - - -def measure(target: DocTarget, toolchain: str, manifest_root: pathlib.Path) -> Coverage: - """Run Rustdoc's coverage meter for one target and sum its per-file counts. - - ``RUSTFLAGS`` and ``RUSTDOCFLAGS`` flow through from the environment so - the Makefile can thread the Polonius flag and the docsrs/deny-warnings - policy that the rest of the tree builds with. - """ - # With no shell involved and argv built from workspace metadata plus - # constant flags, there is no untrusted input to inject. - try: - result = subprocess.run( # noqa: S603 - rustdoc_args(target, toolchain), - cwd=manifest_root, - capture_output=True, - text=True, - check=False, - ) - except OSError as error: - detail = ( - f"cannot run cargo rustdoc for {target.package} {target.kind}" - f" ({target.name or 'lib'}): {error}" - ) - raise RuntimeError(detail) from error - if result.returncode != 0: - detail = ( - f"cargo rustdoc failed for {target.package} {target.kind}" - f" ({target.name or 'lib'}):\n{result.stderr}" - ) - raise RuntimeError(detail) - return parse_coverage_output(target, result.stdout) - - def label(target: DocTarget) -> str: """Return a human-readable name for the target in the breakdown table. @@ -301,59 +46,6 @@ def label(target: DocTarget) -> str: return f"{target.package} {target.kind} ({target.name})" -def run_measurements( - toolchain: str, manifest_root: pathlib.Path -) -> tuple[Coverage, list[tuple[str, Coverage]]]: - """Measure every doc target and return the aggregate and per-target rows. - - A failed ``cargo rustdoc`` or missing JSON output aborts the whole run — - a broken measurement is worse than an unmeasured one. - """ - totals = Coverage(0, 0) - rows: list = [] - for target in doc_targets(load_metadata(toolchain, manifest_root)): - coverage = measure(target, toolchain, manifest_root) - rows.append((label(target), coverage)) - totals += coverage - return totals, rows - - -def load_metadata(toolchain: str, manifest_root: pathlib.Path) -> dict: - """Return the ``cargo metadata`` document for the workspace.""" - args = [ - cargo_executable(), - f"+{toolchain}", - "metadata", - "--no-deps", - "--format-version", - "1", - ] - # The argv is a static command with the pinned toolchain and metadata - # flags; there is no shell or injection surface. - try: - result = subprocess.run( # noqa: S603 - args, - cwd=manifest_root, - capture_output=True, - text=True, - check=False, - ) - except OSError as error: - # Missing or non-executable cargo must surface as an explicit - # measurement error with the script's controlled exit code rather - # than escaping as a bare traceback. - detail = f"cannot run cargo metadata: {error}" - raise RuntimeError(detail) from error - if result.returncode != 0: - detail = f"cargo metadata failed: {result.stderr}" - raise RuntimeError(detail) - try: - return json.loads(result.stdout) - except json.JSONDecodeError as error: - detail = f"cargo metadata emitted invalid JSON: {error}" - raise RuntimeError(detail) from error - - def parse_threshold(value: str) -> float: """Parse a coverage threshold, rejecting NaN and out-of-range values.""" try: @@ -392,13 +84,14 @@ def main(argv: cabc.Sequence[str] | None = None) -> int: manifest_root = args.manifest_root try: - toolchain = args.toolchain or pinned_toolchain(manifest_root) - totals, rows = run_measurements(toolchain, manifest_root) + toolchain = args.toolchain or runner.pinned_toolchain(manifest_root) + totals, rows = runner.run_measurements(toolchain, manifest_root) except RuntimeError as error: print(f"error: {error}", file=sys.stderr) return 2 - for name, coverage in rows: + for target, coverage in rows: + name = label(target) print( f"{name:42s} {coverage.with_docs:5d}/{coverage.total:<5d} " f"{coverage.percentage:6.2f}%" diff --git a/scripts/doc_coverage_cargo.py b/scripts/doc_coverage_cargo.py new file mode 100644 index 000000000..7d741ae29 --- /dev/null +++ b/scripts/doc_coverage_cargo.py @@ -0,0 +1,383 @@ +"""Adapt Cargo and Rustdoc commands for the documentation-coverage gate. + +This module owns process invocation and Rustdoc's generated coverage artefact. +``doc_coverage_runner`` owns repository policy and target selection, while the +command-line entry point owns argument parsing, reporting, and exit codes. +""" + +from __future__ import annotations + +import json +import os +import pathlib +import subprocess +import dataclasses as dc + +from doc_coverage_model import Coverage, DocTarget + + +@dc.dataclass(frozen=True) +class CargoAdapter: + """Adapt one explicit Cargo executable to coverage measurements. + + The runner depends on this narrow interface instead of process globals, so + its target selection and aggregation can be tested without subprocesses. + """ + + executable: str + + def load_metadata( + self, toolchain: str, manifest_root: pathlib.Path + ) -> dict[str, object]: + """Load workspace metadata through this adapter's Cargo executable.""" + return load_metadata(toolchain, manifest_root, self.executable) + + def measure( + self, target: DocTarget, toolchain: str, manifest_root: pathlib.Path + ) -> Coverage: + """Measure one target through this adapter's Cargo executable.""" + return measure(target, toolchain, manifest_root, self.executable) + + +class CoveragePayloadShapeError(TypeError): + """Report that Rustdoc emitted a coverage payload other than an object.""" + + +def production_adapter() -> CargoAdapter: + """Create the production adapter with the configured Cargo executable.""" + return CargoAdapter(os.environ.get("CARGO") or "cargo") + + +def rustdoc_args(target: DocTarget, toolchain: str, cargo_executable: str) -> list[str]: + """Build Cargo's Rustdoc coverage command for one target. + + Parameters + ---------- + target + Workspace target Cargo will document. + toolchain + Dated nightly channel to select with Cargo's ``+`` syntax. + cargo_executable + Cargo executable selected by the adapter at the production boundary. + + Returns + ------- + list[str] + Argument vector for ``cargo rustdoc`` with its coverage options. + """ + args = [cargo_executable, f"+{toolchain}", "rustdoc", "-p", target.package] + if target.kind == "bin": + args += ["--bin", target.name] + else: + args += ["--lib"] + args += [ + "--", + "-Z", + "unstable-options", + "--show-coverage", + "--output-format", + "json", + "--document-private-items", + ] + return args + + +def parse_coverage_output(target: DocTarget, output: str) -> Coverage: + """Decode and aggregate one generated Rustdoc coverage payload. + + Parameters + ---------- + target + Workspace target whose Rustdoc output is being decoded. + output + Generated coverage JSON payload. + + Returns + ------- + Coverage + Aggregate documented and total item counts. + + Raises + ------ + RuntimeError + If the payload is malformed or does not contain valid coverage counts. + """ + try: + per_file = json.loads(output) + except json.JSONDecodeError as error: + raise coverage_json_error(target, str(error)) from error + try: + return aggregate_coverage_payload(per_file) + except CoveragePayloadShapeError as error: + raise coverage_json_error(target, str(error)) from error + except (KeyError, TypeError, ValueError, OverflowError) as error: + detail = f"each entry requires total and with_docs: {error}" + raise coverage_json_error(target, detail) from error + + +def aggregate_coverage_payload(per_file: object) -> Coverage: + """Validate and sum Rustdoc's documented and total counts. + + Parameters + ---------- + per_file + Rustdoc's mapping from source-file names to coverage entries. + + Returns + ------- + Coverage + Aggregate documented and total item counts. + + Raises + ------ + CoveragePayloadShapeError + If Rustdoc's payload is not an object. + KeyError, ValueError + If an entry omits or violates a coverage-count invariant. + """ + match per_file: + case dict() as entries: + return sum( + (coverage_from_entry(entry) for entry in entries.values()), + Coverage(0, 0), + ) + case _: + raise CoveragePayloadShapeError("expected an object") + + +def coverage_from_entry(entry: object) -> Coverage: + """Validate one Rustdoc coverage entry and convert it to ``Coverage``. + + Parameters + ---------- + entry + Rustdoc entry containing ``total`` and ``with_docs`` counts. + + Returns + ------- + Coverage + Validated counts for one source file. + + Raises + ------ + KeyError + If either required count is absent. + TypeError, ValueError + If a count is not a non-negative integer or documented items exceed + total items. + """ + total = coverage_count(entry, "total") + with_docs = coverage_count(entry, "with_docs") + if with_docs > total: + raise ValueError("counts must be non-negative integers with with_docs <= total") + return Coverage(total, with_docs) + + +def coverage_count(entry: object, name: str) -> int: + """Validate one named Rustdoc coverage count. + + Parameters + ---------- + entry + Rustdoc coverage entry containing the requested count. + name + Name of the count to retrieve. + + Returns + ------- + int + The validated non-negative count. + + Raises + ------ + KeyError + If ``name`` is absent from ``entry``. + TypeError, ValueError + If the count cannot be an integer count, including JSON booleans, or + is negative. + """ + count = entry[name] + match count: + case bool(): + raise ValueError( + "counts must be non-negative integers with with_docs <= total" + ) + case int() if count >= 0: + return count + case int(): + raise ValueError( + "counts must be non-negative integers with with_docs <= total" + ) + case _: + raise ValueError( + "counts must be non-negative integers with with_docs <= total" + ) + + +def coverage_json_error(target: DocTarget, detail: str) -> RuntimeError: + """Build a measurement error naming its target and coverage-output detail. + + Parameters + ---------- + target + Workspace target whose output Rustdoc produced. + detail + Explanation of why the generated coverage output is invalid. + + Returns + ------- + RuntimeError + Unraised error ready to identify the target at the caller boundary. + """ + message = ( + f"cargo rustdoc for {target.package} {target.kind}" + f" ({target.name or 'lib'}) did not emit coverage JSON: {detail}" + ) + return RuntimeError(message) + + +def coverage_output_path( + target: DocTarget, output: str, manifest_root: pathlib.Path +) -> pathlib.Path: + """Return the generated coverage JSON path reported by Rustdoc. + + Parameters + ---------- + target + Workspace target whose Rustdoc output is being inspected. + output + Rustdoc standard output containing its generated-file notice. + manifest_root + Workspace root used to resolve a relative generated-file path. + + Returns + ------- + pathlib.Path + Absolute path to the generated coverage JSON file. + + Raises + ------ + RuntimeError + If Rustdoc does not report a usable generated coverage JSON path. + """ + prefix = 'Generated output into "' + for line in output.splitlines(): + if line.startswith(prefix) and line.endswith('"'): + reported_path = line.removeprefix(prefix).removesuffix('"') + if reported_path: + path = pathlib.Path(reported_path) + return path if path.is_absolute() else manifest_root / path + detail = "Rustdoc did not report the generated coverage JSON path" + raise coverage_json_error(target, detail) + + +def measure( + target: DocTarget, + toolchain: str, + manifest_root: pathlib.Path, + cargo_executable: str, +) -> Coverage: + """Run Rustdoc coverage for one target and sum its per-file counts. + + Parameters + ---------- + target + Workspace target to document. + toolchain + Dated nightly channel to select for Cargo. + manifest_root + Workspace root passed to Cargo and used to resolve generated output. + cargo_executable + Explicit Cargo executable for the Rustdoc process. + + Returns + ------- + Coverage + Aggregate documented and total item counts for ``target``. + + Raises + ------ + RuntimeError + If Cargo or Rustdoc fails, omits the reported path, or emits invalid + coverage JSON. + """ + try: + result = subprocess.run( # noqa: S603 - Cargo metadata targets and pinned toolchain form argv; shell remains False. + rustdoc_args(target, toolchain, cargo_executable), + cwd=manifest_root, + capture_output=True, + text=True, + check=False, + ) + except OSError as error: + detail = ( + f"cannot run cargo rustdoc for {target.package} {target.kind}" + f" ({target.name or 'lib'}): {error}" + ) + raise RuntimeError(detail) from error + if result.returncode != 0: + detail = ( + f"cargo rustdoc failed for {target.package} {target.kind}" + f" ({target.name or 'lib'}):\n{result.stderr}" + ) + raise RuntimeError(detail) + output_path = coverage_output_path(target, result.stdout, manifest_root) + try: + output = output_path.read_text(encoding="utf-8") + except OSError as error: + detail = f"cannot read generated coverage JSON at {output_path}: {error}" + raise coverage_json_error(target, detail) from error + return parse_coverage_output(target, output) + + +def load_metadata( + toolchain: str, manifest_root: pathlib.Path, cargo_executable: str +) -> dict[str, object]: + """Return Cargo metadata for the workspace rooted at ``manifest_root``. + + Parameters + ---------- + toolchain + Dated nightly channel to select for Cargo. + manifest_root + Workspace root from which Cargo reads its manifest. + cargo_executable + Explicit Cargo executable for the metadata process. + + Returns + ------- + dict[str, object] + Decoded ``cargo metadata --format-version 1`` document. + + Raises + ------ + RuntimeError + If Cargo cannot run, returns failure, or emits invalid JSON. + """ + args = [ + cargo_executable, + f"+{toolchain}", + "metadata", + "--no-deps", + "--format-version", + "1", + ] + try: + result = subprocess.run( # noqa: S603 - Cargo metadata targets and pinned toolchain form argv; shell remains False. + args, + cwd=manifest_root, + capture_output=True, + text=True, + check=False, + ) + except OSError as error: + detail = f"cannot run cargo metadata: {error}" + raise RuntimeError(detail) from error + if result.returncode != 0: + detail = f"cargo metadata failed: {result.stderr}" + raise RuntimeError(detail) + try: + return json.loads(result.stdout) + except json.JSONDecodeError as error: + detail = f"cargo metadata emitted invalid JSON: {error}" + raise RuntimeError(detail) from error diff --git a/scripts/doc_coverage_model.py b/scripts/doc_coverage_model.py new file mode 100644 index 000000000..4a1bdefac --- /dev/null +++ b/scripts/doc_coverage_model.py @@ -0,0 +1,53 @@ +"""Represent the values shared by the Rustdoc documentation-coverage gate. + +The Cargo adapter owns Rustdoc payload decoding and validation. This module +keeps only the values shared by adapter, orchestration, and command-line code. +""" + +from __future__ import annotations + +import dataclasses as dc + + +@dc.dataclass(frozen=True) +class Coverage: + """Represent Rustdoc-measured items for one documentation run. + + Parameters + ---------- + total + Total number of Rustdoc-counted items. + with_docs + Number of those items carrying documentation. + """ + + total: int + with_docs: int + + @property + def percentage(self) -> float: + """Return the share of documented items as a percentage.""" + return 100.0 * self.with_docs / self.total if self.total else 100.0 + + def __add__(self, other: Coverage) -> Coverage: + """Return the sum of two coverage counts.""" + return Coverage(self.total + other.total, self.with_docs + other.with_docs) + + +@dc.dataclass(frozen=True) +class DocTarget: + """Describe one library or binary target measured by Rustdoc. + + Parameters + ---------- + package + Cargo package containing the target. + kind + Target category: ``"lib"`` or ``"bin"``. + name + Binary target name, or ``None`` for a library target. + """ + + package: str + kind: str + name: str | None diff --git a/scripts/doc_coverage_runner.py b/scripts/doc_coverage_runner.py new file mode 100644 index 000000000..544dc2204 --- /dev/null +++ b/scripts/doc_coverage_runner.py @@ -0,0 +1,151 @@ +"""Select documentation targets and coordinate coverage measurements. + +This module owns repository workflow policy and target discovery. +``doc_coverage_cargo`` owns Cargo and Rustdoc process handling, while the +command-line entry point owns argument parsing, reporting, and exit codes. +""" + +from __future__ import annotations + +import pathlib +import tomllib +import typing as typ + +import doc_coverage_cargo +from doc_coverage_model import Coverage, DocTarget + + +class CoverageAdapter(typ.Protocol): + """Define the Cargo boundary consumed by measurement orchestration.""" + + def load_metadata( + self, toolchain: str, manifest_root: pathlib.Path + ) -> dict[str, object]: + """Load Cargo metadata for the selected workspace.""" + + def measure( + self, target: DocTarget, toolchain: str, manifest_root: pathlib.Path + ) -> Coverage: + """Measure documentation coverage for one selected target.""" + + +def pinned_toolchain(manifest_root: pathlib.Path) -> str: + """Return the channel pinned in the repository's toolchain file. + + Parameters + ---------- + manifest_root + Workspace root containing ``rust-toolchain.toml``. + + Returns + ------- + str + The configured dated Rust toolchain channel. + + Raises + ------ + RuntimeError + If the toolchain file cannot be read or lacks a channel. + """ + try: + with (manifest_root / "rust-toolchain.toml").open("rb") as toolchain: + return tomllib.load(toolchain)["toolchain"]["channel"] + except (OSError, tomllib.TOMLDecodeError, KeyError) as error: + detail = f"cannot read the pinned toolchain from rust-toolchain.toml: {error}" + raise RuntimeError(detail) from error + + +def doc_targets(metadata: dict[str, object]) -> list[DocTarget]: + """Derive library and binary targets for every workspace member. + + Parameters + ---------- + metadata + Decoded Cargo metadata document for the workspace. + + Returns + ------- + list[DocTarget] + Every library and binary target belonging to a workspace member. + + Raises + ------ + RuntimeError + If Cargo metadata lacks the workspace package or member collections. + """ + try: + members = set(metadata["workspace_members"]) + packages = metadata["packages"] + except (KeyError, TypeError) as error: + detail = "cargo metadata response lacks the workspace packages or members" + raise RuntimeError(detail) from error + return [ + doc + for package in packages + if "id" in package and "targets" in package + if package["id"] in members + for ordinal in package["targets"] + for doc in doc_able_targets(package, ordinal) + ] + + +def doc_able_targets( + package: dict[str, object], target: dict[str, object] +) -> list[DocTarget]: + """Map one Cargo target to its measurable target, if any. + + Parameters + ---------- + package + Cargo metadata entry for the package that owns ``target``. + target + Cargo metadata entry describing one build target. + + Returns + ------- + list[DocTarget] + The library or binary target when it is measurable, otherwise empty. + """ + kinds: list[str] = target.get("kind", []) + if "lib" in kinds: + return [DocTarget(package["name"], "lib", None)] + if "bin" in kinds: + return [DocTarget(package["name"], "bin", target["name"])] + return [] + + +def run_measurements( + toolchain: str, + manifest_root: pathlib.Path, + adapter: CoverageAdapter | None = None, +) -> tuple[Coverage, list[tuple[DocTarget, Coverage]]]: + """Measure every target and return aggregate plus target-specific coverage. + + Parameters + ---------- + toolchain + Dated nightly channel to select for every Cargo invocation. + manifest_root + Workspace root passed to metadata discovery and Rustdoc. + adapter + Cargo and Rustdoc boundary to use. When omitted, the configured + production Cargo adapter is constructed. + + Returns + ------- + tuple[Coverage, list[tuple[DocTarget, Coverage]]] + Aggregate coverage followed by each target and its coverage result. + + Raises + ------ + RuntimeError + If Cargo metadata discovery or any target measurement fails. + """ + coverage_adapter = adapter or doc_coverage_cargo.production_adapter() + totals = Coverage(0, 0) + rows: list[tuple[DocTarget, Coverage]] = [] + for target in doc_targets(coverage_adapter.load_metadata(toolchain, manifest_root)): + coverage = coverage_adapter.measure(target, toolchain, manifest_root) + rows.append((target, coverage)) + totals += coverage + return totals, rows diff --git a/scripts/tests/conftest.py b/scripts/tests/conftest.py index 6e4539ec6..6f5edc211 100644 --- a/scripts/tests/conftest.py +++ b/scripts/tests/conftest.py @@ -1,23 +1,63 @@ -"""Shared fixtures for spelling rollout tests.""" +"""Provide shared dynamic-import fixtures for script test modules.""" from __future__ import annotations import importlib -import types -from pathlib import Path +import importlib.util +import pathlib +import sys +import typing as typ import pytest -SCRIPT_DIRECTORY = Path(__file__).resolve().parents[1] +if typ.TYPE_CHECKING: + import types + +SCRIPT_DIRECTORY = pathlib.Path(__file__).resolve().parents[1] @pytest.fixture(name="rollout_modules") def rollout_modules_fixture( monkeypatch: pytest.MonkeyPatch, ) -> tuple[types.ModuleType, types.ModuleType, types.ModuleType]: - """Import scripts through the top-level paths used at runtime.""" + """Import spelling-rollout scripts through their runtime module paths.""" monkeypatch.syspath_prepend(str(SCRIPT_DIRECTORY)) names = ("typos_rollout_cache", "typos_rollout", "generate_typos_config") importlib.invalidate_caches() cache, rollout, generator = (importlib.import_module(name) for name in names) return cache, rollout, generator + + +def load_script_module(module_name: str, file_name: str) -> types.ModuleType: + """Import one documentation-coverage module under its required name.""" + spec = importlib.util.spec_from_file_location( + module_name, SCRIPT_DIRECTORY / file_name + ) + if spec is None: + message = "expected import setup to produce a module spec" + raise AssertionError(message) + if spec.loader is None: + message = "expected module spec to provide a loader" + raise AssertionError(message) + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +@pytest.fixture(name="cargo") +def cargo_fixture() -> types.ModuleType: + """Import the Cargo and Rustdoc adapter under its normal module name.""" + return load_script_module("doc_coverage_cargo", "doc_coverage_cargo.py") + + +@pytest.fixture(name="runner") +def runner_fixture(cargo: types.ModuleType) -> types.ModuleType: + """Import the measurement coordinator under its normal module name.""" + return load_script_module("doc_coverage_runner", "doc_coverage_runner.py") + + +@pytest.fixture(name="script") +def script_fixture(runner: types.ModuleType) -> types.ModuleType: + """Import ``doc-coverage.py`` under a loadable module name.""" + return load_script_module("doc_coverage_module", "doc-coverage.py") diff --git a/scripts/tests/test_doc_coverage.py b/scripts/tests/test_doc_coverage.py index 08ffedbd4..1b1efe585 100644 --- a/scripts/tests/test_doc_coverage.py +++ b/scripts/tests/test_doc_coverage.py @@ -1,227 +1,79 @@ -"""Substantive tests for the workspace Rustdoc doc-comment coverage gate. - -``scripts/doc-coverage.py`` wraps ``cargo rustdoc --show-coverage`` and -``cargo metadata``; every test in this module replaces those two subprocess -boundaries with canned responses so the script's own logic — target -discovery, aggregation, threshold exits, malformed-output handling, and -command-failure translation — is exercised without invoking Cargo at all. - -The helper script cannot be imported by its file name (``doc-coverage.py`` -contains a hyphen), so a fixture loads it through ``importlib`` under a -hyphen-free module name. -""" +"""Test the documentation-coverage command-line interface.""" from __future__ import annotations import argparse import dataclasses -import importlib.util +import json +import os import pathlib +import subprocess import sys +import textwrap import typing as typ import pytest +from conftest import SCRIPT_DIRECTORY if typ.TYPE_CHECKING: import types -SCRIPT_DIRECTORY = pathlib.Path(__file__).resolve().parents[1] - - -@pytest.fixture(name="script") -def script_fixture() -> types.ModuleType: - """Import ``doc-coverage.py`` under a loadable module name.""" - spec = importlib.util.spec_from_file_location( - "doc_coverage_module", SCRIPT_DIRECTORY / "doc-coverage.py" - ) - assert spec is not None, "expected import setup to produce a module spec" - assert spec.loader is not None, "expected module spec to provide a loader" - module = importlib.util.module_from_spec(spec) - sys.modules[spec.name] = module - spec.loader.exec_module(module) - return module - - -def lib_target(name: str) -> dict: - """Return one library target as ``cargo metadata`` reports it.""" - return {"name": name, "kind": ["lib"]} - - -def bin_target(name: str) -> dict: - """Return one binary target as ``cargo metadata`` reports it.""" - return {"name": name, "kind": ["bin"]} - - -def metadata_for(packages: list[dict]) -> dict: - """Build the ``cargo metadata`` document the script consumes.""" - return { - "packages": packages, - "workspace_members": [package["id"] for package in packages], - } - - -def single_library_metadata() -> str: - """Return the ``cargo metadata`` JSON for one package with one library target. - - Returns - ------- - str - A metadata document describing the single ``x`` package with one ``lib`` - target, in the shape ``doc_targets`` consumes. - """ - return ( - '{"packages": [{"id": "pkg:x:1.0.0", "name": "x", ' - '"targets": [{"name": "x", "kind": ["lib"]}]}], ' - '"workspace_members": ["pkg:x:1.0.0"]}' - ) - - -@dataclasses.dataclass(frozen=True) -class RustdocFailureCase: - """Define one Rustdoc failure scenario for measurement integration tests. - - Parameters - ---------- - output - The mocked standard output from `cargo rustdoc`. - returncode - The mocked `cargo rustdoc` process exit code. - diagnostic - Text that must occur in the translated `RuntimeError`. - """ - - output: str - returncode: int - diagnostic: str - @dataclasses.dataclass(frozen=True) -class CoveragePayloadFailureCase: - """Define one invalid Rustdoc coverage-payload scenario. - - Parameters - ---------- - payload - The mocked output from `cargo rustdoc`. - diagnostic - Text that must occur in the translated `RuntimeError`. - """ - - payload: str - diagnostic: str - - -class FakeCargo: - """Stand-in for ``cargo metadata`` and ``cargo rustdoc`` invocations. - - Each call records its argv for later assertion and answers the metadata - call with ``metadata`` and every rustdoc call with ``rustdoc_output``. - """ - - def __init__( - self, - script: types.ModuleType, - *, - metadata: str = '{"packages": [], "workspace_members": []}', - rustdoc_output: str = "{}", - rustdoc_rc: int = 0, - ) -> None: - self._script = script - self.metadata_payload = metadata - self.rustdoc_payload = rustdoc_output - self.rustdoc_rc = rustdoc_rc - self.calls: list[list[str]] = [] - - def install(self, monkeypatch: pytest.MonkeyPatch) -> FakeCargo: - """Replace the script's ``subprocess.run`` with this fake.""" - monkeypatch.setattr(self._script.subprocess, "run", self.run) - return self - - def run(self, argv: list[str], **_kwargs: object) -> FakeResult: - """Answer one Cargo invocation from the canned payloads.""" - self.calls.append(argv) - if "metadata" in argv: - return FakeResult(0, self.metadata_payload) - return FakeResult(self.rustdoc_rc, self.rustdoc_payload) - - -class FakeResult: - """Minimal ``subprocess.CompletedProcess`` stand-in.""" - - def __init__(self, returncode: int, stdout: str, stderr: str = "") -> None: - self.returncode = returncode - self.stdout = stdout - self.stderr = stderr - - -def test_target_discovery_skips_non_doc_targets(script: types.ModuleType) -> None: - """Build scripts, tests, examples, and benches never enter the surface.""" - metadata = metadata_for( - [ - { - "id": "pkg:netsuke:0.1.0", - "name": "netsuke", - "targets": [ - lib_target("netsuke"), - bin_target("netsuke-bin"), - bin_target("extra"), - {"name": "build-main", "kind": ["custom-build"]}, - {"name": "integration", "kind": ["test"]}, - {"name": "sample", "kind": ["example"]}, - {"name": "benchmark", "kind": ["bench"]}, - ], - } - ] +class CliProcessCase: + """Define one executable documentation-coverage CLI scenario.""" + + threshold: str + fails_adapter: bool + expected_code: int + + +@pytest.fixture +def executable_cargo(tmp_path: pathlib.Path) -> tuple[pathlib.Path, pathlib.Path]: + """Create a platform-safe Cargo executable for CLI process tests.""" + program = tmp_path / "fake_cargo.py" + log_path = tmp_path / "cargo-calls.jsonl" + program.write_text( + textwrap.dedent( + """\ + #!__PYTHON__ + import json + import os + import pathlib + import sys + + args = sys.argv[1:] + with pathlib.Path(os.environ["DOC_COVERAGE_CARGO_LOG"]).open("a") as log: + print(json.dumps(args), file=log) + if os.environ.get("DOC_COVERAGE_CARGO_FAILURE"): + print("controlled cargo failure", file=sys.stderr) + raise SystemExit(1) + if args[1] == "metadata": + print( + '{"packages": [{"id": "pkg:x:1.0.0", "name": "x", ' + '"targets": [{"name": "x", "kind": ["lib"]}]}], ' + '"workspace_members": ["pkg:x:1.0.0"]}' + ) + raise SystemExit(0) + output_path = pathlib.Path.cwd() / "target" / "doc" / "x.json" + output_path.parent.mkdir(parents=True, exist_ok=True) + output_path.write_text( + '{"src/lib.rs": {"total": 10, "with_docs": 9}}', encoding="utf-8" + ) + print(f'Generated output into "{output_path}"') + """ + ).replace("__PYTHON__", sys.executable), + encoding="utf-8", ) - - targets = script.doc_targets(metadata) - - assert [target.kind for target in targets] == ["lib", "bin", "bin"] - assert {target.name for target in targets if target.kind == "bin"} == { - "netsuke-bin", - "extra", - } - - -def test_target_discovery_excludes_outside_workspace(script: types.ModuleType) -> None: - """Dependency crates outside ``workspace_members`` never get measured.""" - member = { - "id": "pkg:member:0.1.0", - "name": "member", - "targets": [lib_target("member")], - } - dependency = { - "id": "pkg:dependency:0.1.0", - "name": "dependency", - "targets": [lib_target("dependency")], - } - metadata = { - "packages": [member, dependency], - "workspace_members": ["pkg:member:0.1.0"], - } - - targets = script.doc_targets(metadata) - - assert [target.package for target in targets] == ["member"] - - -def test_aggregation_sums_targets_and_reports_percentage( - script: types.ModuleType, -) -> None: - """Aggregate totals roll per-target counts up and report the share.""" - first = script.Coverage(10, 8) - second = script.Coverage(5, 5) - - combined = first + second - - assert combined.total == 15 - assert combined.with_docs == 13 - assert combined.percentage == pytest.approx(13 / 15 * 100) - - -def test_empty_run_is_complete_not_a_division_by_zero(script: types.ModuleType) -> None: - """A crate with no doc-able targets contributes an empty, complete run.""" - assert script.Coverage(0, 0).percentage == 100.0 + if sys.platform == "win32": + executable = tmp_path / "fake-cargo.cmd" + executable.write_text( + f'@echo off\r\n"{sys.executable}" "{program}" %*\r\n', encoding="utf-8" + ) + else: + executable = program + executable.chmod(0o755) + return executable, log_path def test_threshold_flips_exit_code( @@ -229,14 +81,13 @@ def test_threshold_flips_exit_code( tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """The gate passes above the threshold and fails below it.""" - metadata = ( - '{"packages": [{"id": "pkg:x:1.0.0", "name": "x", ' - '"targets": [{"name": "x", "kind": ["lib"]}]}], ' - '"workspace_members": ["pkg:x:1.0.0"]}' + """Pass above the threshold and fail below it.""" + coverage = script.runner.Coverage(10, 6) + monkeypatch.setattr( + script.runner, + "run_measurements", + lambda _toolchain, _root: (coverage, []), ) - rustdoc = '{"src/lib.rs": {"total": 10, "with_docs": 6}}' - FakeCargo(script, metadata=metadata, rustdoc_output=rustdoc).install(monkeypatch) monkeypatch.chdir(tmp_path) passing = script.main(["--toolchain", "nightly-x", "--threshold", "50"]) @@ -246,84 +97,86 @@ def test_threshold_flips_exit_code( assert failing == 1 -def test_cargo_metadata_failure_aborts_the_run( - script: types.ModuleType, - tmp_path: pathlib.Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """A failing cargo metadata run is a measurement error, not an empty gate.""" - - def fail(_argv: list[str], **_kwargs: object) -> FakeResult: - return FakeResult(1, "", "filtered diagnostics") - - monkeypatch.setattr(script.subprocess, "run", fail) - monkeypatch.chdir(tmp_path) - - with pytest.raises(RuntimeError, match="cargo metadata failed"): - script.load_metadata("nightly-x", tmp_path) - - @pytest.mark.parametrize( "case", [ pytest.param( - RustdocFailureCase( - "not json at all", - 0, - "did not emit coverage JSON", - ), - id="malformed-output", + CliProcessCase(threshold="80", fails_adapter=False, expected_code=0), + id="passing-threshold", + ), + pytest.param( + CliProcessCase(threshold="95", fails_adapter=False, expected_code=1), + id="failing-threshold", ), pytest.param( - RustdocFailureCase( - "{}", - 1, - "cargo rustdoc failed for x", - ), - id="rustdoc-exit-failure", + CliProcessCase(threshold="80", fails_adapter=True, expected_code=2), + id="adapter-failure", ), ], ) -def test_run_measurements_propagates_rustdoc_failure( - script: types.ModuleType, +def test_cli_process_uses_configured_cargo_adapter( + executable_cargo: tuple[pathlib.Path, pathlib.Path], tmp_path: pathlib.Path, - monkeypatch: pytest.MonkeyPatch, - case: RustdocFailureCase, -) -> None: - """Propagate malformed output and non-zero rustdoc exits as measurement errors.""" - FakeCargo( - script, - metadata=single_library_metadata(), - rustdoc_output=case.output, - rustdoc_rc=case.returncode, - ).install(monkeypatch) - monkeypatch.chdir(tmp_path) - - with pytest.raises(RuntimeError, match=case.diagnostic): - script.run_measurements("nightly-x", tmp_path) - - -def test_malformed_metadata_shape_is_a_measurement_error( - script: types.ModuleType, + case: CliProcessCase, ) -> None: - """Valid JSON without workspace keys is rejected, not a KeyError crash.""" - with pytest.raises(RuntimeError, match="lacks the workspace"): - script.doc_targets({"packages": []}) - - -def test_target_without_kind_is_skipped(script: types.ModuleType) -> None: - """A target record missing its kind list simply contributes nothing.""" - metadata = metadata_for( + """Run the CLI against a controlled Cargo executable in a child process.""" + cargo_executable, log_path = executable_cargo + environment = os.environ | { + "CARGO": str(cargo_executable), + "DOC_COVERAGE_CARGO_LOG": str(log_path), + } + if case.fails_adapter: + environment["DOC_COVERAGE_CARGO_FAILURE"] = "1" + result = subprocess.run( # noqa: S603 - executes the controlled fixture with shell disabled. [ - { - "id": "pkg:x:0.1.0", - "name": "x", - "targets": [{"name": "mystery"}], - } - ] + sys.executable, + str(SCRIPT_DIRECTORY / "doc-coverage.py"), + "--manifest-root", + str(tmp_path), + "--toolchain", + "nightly-subprocess", + "--threshold", + case.threshold, + ], + cwd=tmp_path, + capture_output=True, + check=False, + env=environment, + text=True, ) - assert script.doc_targets(metadata) == [] + assert result.returncode == case.expected_code + calls = [ + json.loads(line) for line in log_path.read_text(encoding="utf-8").splitlines() + ] + assert calls[0] == [ + "+nightly-subprocess", + "metadata", + "--no-deps", + "--format-version", + "1", + ] + if case.fails_adapter: + assert ( + result.stderr + == "error: cargo metadata failed: controlled cargo failure\n\n" + ) + return + + assert calls[1][0] == "+nightly-subprocess" + assert calls[1][1:5] == ["rustdoc", "-p", "x", "--lib"] + assert "aggregate" in result.stdout + assert "9/10" in result.stdout + if case.expected_code == 0: + assert ( + "ok: doc-comment coverage 90.00% meets the 80.00% threshold." + in result.stdout + ) + else: + assert result.stderr == ( + "doc-comment coverage 90.00% is below the 95.00% threshold; document " + "the lowest-coverage targets listed above and re-run `make doc-coverage`.\n" + ) def test_missing_cargo_maps_to_measurement_error( @@ -331,46 +184,17 @@ def test_missing_cargo_maps_to_measurement_error( tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """An OSError from Cargo exits 2 with a message, not a traceback.""" + """Map a missing Cargo executable to the established CLI failure exit.""" + message = "cargo: not found" - def fail(_argv: list[str], **_kwargs: object) -> FakeResult: - message = "cargo: not found" + def fail(_argv: list[str], **_kwargs: object) -> typ.NoReturn: + """Raise the configured Cargo executable error.""" raise OSError(message) - monkeypatch.setattr(script.subprocess, "run", fail) + monkeypatch.setattr(script.runner.doc_coverage_cargo.subprocess, "run", fail) monkeypatch.chdir(tmp_path) - code = script.main(["--toolchain", "nightly-x"]) - - assert code == 2 - - -def test_measure_maps_missing_cargo_to_measurement_error( - script: types.ModuleType, - tmp_path: pathlib.Path, - monkeypatch: pytest.MonkeyPatch, -) -> None: - """An OSError from Cargo in ``measure()`` is a RuntimeError, not a traceback. - - ``test_missing_cargo_maps_to_measurement_error`` covers the same boundary - through ``main()``, where the error also surfaces at ``load_metadata()`` - time. This test isolates the ``measure()`` translation branch after - metadata loading has already succeeded, so the diagnostic is the rustdoc - one rather than the metadata one. - """ - - def fail(_argv: list[str], **_kwargs: object) -> FakeResult: - message = "cargo: not found" - raise OSError(message) - - monkeypatch.setattr(script.subprocess, "run", fail) - - target = script.DocTarget("x", "lib", None) - - with pytest.raises( - RuntimeError, match=r"cannot run cargo rustdoc for x lib \(lib\)" - ): - script.measure(target, "nightly-x", tmp_path) + assert script.main(["--toolchain", "nightly-x"]) == 2 def test_toolchain_override_reaches_every_cargo_call( @@ -378,45 +202,53 @@ def test_toolchain_override_reaches_every_cargo_call( tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """``--toolchain`` flows into the ``+channel`` selector for every call.""" + """Thread ``--toolchain`` through the ``+channel`` selector for every call.""" + calls: list[list[str]] = [] metadata = ( '{"packages": [{"id": "pkg:x:1.0.0", "name": "x", ' '"targets": [{"name": "x", "kind": ["lib"]}]}], ' '"workspace_members": ["pkg:x:1.0.0"]}' ) - fake = FakeCargo(script, metadata=metadata).install(monkeypatch) + + def run(argv: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]: + """Record a Cargo call and return a minimal successful response.""" + calls.append(argv) + if "metadata" in argv: + return subprocess.CompletedProcess(argv, 0, metadata, "") + manifest_root = pathlib.Path(typ.cast("pathlib.Path", kwargs["cwd"])) + output_path = manifest_root / "target" / "doc" / "x.json" + output_path.parent.mkdir(parents=True, exist_ok=True) + output_path.write_text( + '{"src/lib.rs": {"total": 10, "with_docs": 10}}', + encoding="utf-8", + ) + return subprocess.CompletedProcess( + argv, + 0, + f'Generated output into "{output_path}"\n', + "", + ) + + monkeypatch.setattr(script.runner.doc_coverage_cargo.subprocess, "run", run) monkeypatch.chdir(tmp_path) script.main(["--toolchain", "nightly-custom", "--threshold", "0"]) - assert fake.calls, "expected at least one cargo invocation" - assert all(argv[1] == "+nightly-custom" for argv in fake.calls), ( - f"toolchain selector not threaded through: {fake.calls!r}" - ) - - -def test_pinned_toolchain_reads_the_channel( - script: types.ModuleType, - tmp_path: pathlib.Path, -) -> None: - """The pinned channel is recollected from rust-toolchain.toml.""" - (tmp_path / "rust-toolchain.toml").write_text( - '[toolchain]\nchannel = "nightly-from-pin"\n', - encoding="utf-8", + assert calls, "expected at least one cargo invocation" + assert all(argv[1] == "+nightly-custom" for argv in calls), ( + f"toolchain selector not threaded through: {calls!r}" ) - assert script.pinned_toolchain(tmp_path) == "nightly-from-pin" - def test_parse_threshold_rejects_invalid_values(script: types.ModuleType) -> None: - """Thresholds outside [0, 100] and non-numbers are argument errors.""" + """Reject thresholds outside [0, 100] and non-numbers as argument errors.""" for value in ["101", "-1", "not-a-number"]: with pytest.raises(argparse.ArgumentTypeError): script.parse_threshold(value) def test_label_names_libraries_and_binaries(script: types.ModuleType) -> None: - """The breakdown labels distinguish lib from named binary targets.""" + """Distinguish library and named binary targets in breakdown labels.""" lib = script.DocTarget("netsuke", "lib", None) binary = script.DocTarget("netsuke", "bin", "netsuke-bin") @@ -424,142 +256,40 @@ def test_label_names_libraries_and_binaries(script: types.ModuleType) -> None: assert script.label(binary) == "netsuke bin (netsuke-bin)" -@pytest.mark.parametrize( - ("target", "selector"), - [ - pytest.param( - ("netsuke", "lib", None), - ["--lib"], - id="library", - ), - pytest.param( - ("netsuke", "bin", "netsuke-bin"), - ["--bin", "netsuke-bin"], - id="binary", - ), - ], -) -def test_rustdoc_args_for_target( +def test_main_delegates_to_runner_measurements( script: types.ModuleType, + tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch, - target: tuple[str, str, str | None], - selector: list[str], ) -> None: - """Build the complete rustdoc command, selecting lib or bin by target kind.""" - monkeypatch.setenv("CARGO", "cargo") - doc_target = script.DocTarget(*target) - - args = script.rustdoc_args(doc_target, "nightly-x") - - assert args == [ - "cargo", - "+nightly-x", - "rustdoc", - "-p", - "netsuke", - *selector, - "--", - "-Z", - "unstable-options", - "--show-coverage", - "--output-format", - "json", - "--document-private-items", - ] - - -def test_parse_coverage_output_aggregates_multiple_files( - script: types.ModuleType, -) -> None: - """Per-file totals and with_docs counts roll up across the payload.""" - target = script.DocTarget("netsuke", "lib", None) - payload = ( - '{"src/a.rs": {"total": 10, "with_docs": 8}, ' - '"src/b.rs": {"total": 5, "with_docs": 3}}' + """Delegate CLI measurement while retaining CLI reporting and exit policy.""" + expected = script.runner.Coverage(1, 1) + monkeypatch.setattr( + script.runner, + "run_measurements", + lambda _toolchain, _root: (expected, []), ) - coverage = script.parse_coverage_output(target, payload) - - assert coverage.total == 15 - assert coverage.with_docs == 11 - - -def test_parse_coverage_output_rejects_malformed_json(script: types.ModuleType) -> None: - """Non-JSON output surfaces as a RuntimeError naming the coverage gate.""" - target = script.DocTarget("netsuke", "lib", None) - - with pytest.raises(RuntimeError, match="did not emit coverage JSON"): - script.parse_coverage_output(target, "not json at all") + assert ( + script.main(["--toolchain", "nightly-x", "--manifest-root", str(tmp_path)]) == 0 + ) -@pytest.mark.parametrize( - "entry", - [ - pytest.param('{"total": 1e999, "with_docs": 0}', id="non-finite"), - pytest.param('{"total": -1, "with_docs": 0}', id="negative"), - pytest.param('{"total": 1.5, "with_docs": 0}', id="non-integer"), - pytest.param('{"total": 1, "with_docs": 2}', id="inconsistent"), - ], -) -def test_main_rejects_invalid_coverage_counts( +def test_main_translates_runner_failure_to_exit_two( script: types.ModuleType, tmp_path: pathlib.Path, monkeypatch: pytest.MonkeyPatch, - entry: str, + capsys: pytest.CaptureFixture[str], ) -> None: - """Return the controlled exit for invalid Rustdoc count invariants.""" - payload = '{"src/lib.rs": ' + entry + "}" - FakeCargo( - script, - metadata=single_library_metadata(), - rustdoc_output=payload, - ).install(monkeypatch) - monkeypatch.chdir(tmp_path) + """Translate a runner failure into the established CLI diagnostic and exit code.""" + message = "runner measurement failed" - with pytest.raises(RuntimeError, match="each entry requires total and with_docs"): - script.run_measurements("nightly-x", tmp_path) - - assert script.main(["--toolchain", "nightly-x"]) == 2 + def fail(_toolchain: str, _manifest_root: pathlib.Path) -> typ.NoReturn: + """Raise the configured runner failure.""" + raise RuntimeError(message) + monkeypatch.setattr(script.runner, "run_measurements", fail) -@pytest.mark.parametrize( - "case", - [ - pytest.param( - CoveragePayloadFailureCase("[]", "expected an object"), - id="non-object", - ), - pytest.param( - CoveragePayloadFailureCase( - '{"src/lib.rs": {"total": 1}}', - "each entry requires total and with_docs", - ), - id="missing-with-docs", - ), - pytest.param( - CoveragePayloadFailureCase( - '{"src/lib.rs": {"with_docs": 1}}', - "each entry requires total and with_docs", - ), - id="missing-total", - ), - ], -) -def test_main_maps_invalid_coverage_shape_to_measurement_error( - script: types.ModuleType, - tmp_path: pathlib.Path, - monkeypatch: pytest.MonkeyPatch, - case: CoveragePayloadFailureCase, -) -> None: - """Return the controlled measurement exit for invalid coverage JSON shapes.""" - FakeCargo( - script, - metadata=single_library_metadata(), - rustdoc_output=case.payload, - ).install(monkeypatch) - monkeypatch.chdir(tmp_path) - - with pytest.raises(RuntimeError, match=case.diagnostic): - script.run_measurements("nightly-x", tmp_path) - - assert script.main(["--toolchain", "nightly-x"]) == 2 + assert ( + script.main(["--toolchain", "nightly-x", "--manifest-root", str(tmp_path)]) == 2 + ) + assert capsys.readouterr().err == f"error: {message}\n" diff --git a/scripts/tests/test_doc_coverage_cargo.py b/scripts/tests/test_doc_coverage_cargo.py new file mode 100644 index 000000000..02cf0e6e3 --- /dev/null +++ b/scripts/tests/test_doc_coverage_cargo.py @@ -0,0 +1,357 @@ +"""Test the Cargo and Rustdoc documentation-coverage adapter.""" + +from __future__ import annotations + +import dataclasses +import pathlib +import typing as typ + +import pytest + +if typ.TYPE_CHECKING: + import types + + +def single_library_metadata() -> str: + """Return metadata JSON for one package with one library target.""" + return ( + '{"packages": [{"id": "pkg:x:1.0.0", "name": "x", ' + '"targets": [{"name": "x", "kind": ["lib"]}]}], ' + '"workspace_members": ["pkg:x:1.0.0"]}' + ) + + +@dataclasses.dataclass(frozen=True) +class RustdocFailureCase: + """Define one Rustdoc failure scenario for measurement integration tests.""" + + output: str + returncode: int + diagnostic: str + + +@dataclasses.dataclass(frozen=True) +class ReportedCoverageFileCase: + """Define one generated Rustdoc coverage-file path scenario.""" + + payload: str + output_path: pathlib.Path + reported_path: pathlib.Path | None + expected: tuple[int, int] + + +@dataclasses.dataclass(frozen=True) +class FakeRustdocResult: + """Define the simulated result of one ``cargo rustdoc`` invocation.""" + + payload: str = "{}" + returncode: int = 0 + output_path: pathlib.Path | None = None + reported_path: pathlib.Path | None = None + write_output: bool = True + + +_DEFAULT_RUSTDOC_RESULT = FakeRustdocResult() + + +class FakeResult: + """Provide a minimal ``subprocess.CompletedProcess`` stand-in.""" + + def __init__(self, returncode: int, stdout: str, stderr: str = "") -> None: + self.returncode = returncode + self.stdout = stdout + self.stderr = stderr + + +class FakeCargo: + """Stand in for ``cargo metadata`` and ``cargo rustdoc`` invocations. + + Each call records its argv, answers metadata with ``metadata``, and writes + each Rustdoc payload to the generated file path that Rustdoc reports. + """ + + def __init__( + self, + cargo: types.ModuleType, + *, + metadata: str = '{"packages": [], "workspace_members": []}', + rustdoc: FakeRustdocResult = _DEFAULT_RUSTDOC_RESULT, + ) -> None: + self._cargo = cargo + self.metadata_payload = metadata + self.rustdoc = rustdoc + self.calls: list[list[str]] = [] + + def install(self, monkeypatch: pytest.MonkeyPatch) -> FakeCargo: + """Replace the adapter's ``subprocess.run`` with this fake.""" + monkeypatch.setattr(self._cargo.subprocess, "run", self.run) + return self + + def run(self, argv: list[str], **kwargs: object) -> FakeResult: + """Answer one Cargo invocation from the canned payloads.""" + self.calls.append(argv) + if "metadata" in argv: + return FakeResult(0, self.metadata_payload) + manifest_root = pathlib.Path(typ.cast("pathlib.Path", kwargs["cwd"])) + package = argv[argv.index("-p") + 1].replace("-", "_") + configured_output_path = self.rustdoc.output_path or ( + manifest_root / "target" / "doc" / f"{package}.json" + ) + output_path = ( + configured_output_path + if configured_output_path.is_absolute() + else manifest_root / configured_output_path + ) + if self.rustdoc.write_output: + output_path.parent.mkdir(parents=True, exist_ok=True) + output_path.write_text(self.rustdoc.payload, encoding="utf-8") + reported_path = self.rustdoc.reported_path or output_path + return FakeResult( + self.rustdoc.returncode, + f'Generated output into "{reported_path}"\n', + ) + + +def test_cargo_metadata_failure_aborts_the_run( + cargo: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Treat a failing cargo metadata run as a measurement error.""" + + def fail(_argv: list[str], **_kwargs: object) -> FakeResult: + """Return the configured metadata failure.""" + return FakeResult(1, "", "filtered diagnostics") + + monkeypatch.setattr(cargo.subprocess, "run", fail) + monkeypatch.chdir(tmp_path) + + with pytest.raises(RuntimeError, match="cargo metadata failed"): + cargo.CargoAdapter("cargo").load_metadata("nightly-x", tmp_path) + + +@pytest.mark.parametrize( + "case", + [ + pytest.param( + RustdocFailureCase("not json at all", 0, "did not emit coverage JSON"), + id="malformed-output", + ), + pytest.param( + RustdocFailureCase("{}", 1, "cargo rustdoc failed for x"), + id="rustdoc-exit-failure", + ), + ], +) +def test_cargo_adapter_propagates_rustdoc_failure( + cargo: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, + case: RustdocFailureCase, +) -> None: + """Translate malformed output and process failures into measurement errors.""" + FakeCargo( + cargo, + metadata=single_library_metadata(), + rustdoc=FakeRustdocResult( + payload=case.output, + returncode=case.returncode, + ), + ).install(monkeypatch) + monkeypatch.chdir(tmp_path) + + with pytest.raises(RuntimeError, match=case.diagnostic): + cargo.CargoAdapter("cargo").measure( + cargo.DocTarget("x", "lib", None), "nightly-x", tmp_path + ) + + +def test_measure_maps_missing_cargo_to_measurement_error( + cargo: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Translate an OSError from Rustdoc execution into a RuntimeError.""" + message = "cargo: not found" + + def fail(_argv: list[str], **_kwargs: object) -> FakeResult: + """Raise the configured Cargo executable error.""" + raise OSError(message) + + monkeypatch.setattr(cargo.subprocess, "run", fail) + + with pytest.raises( + RuntimeError, match=r"cannot run cargo rustdoc for x lib \(lib\)" + ): + cargo.CargoAdapter("cargo").measure( + cargo.DocTarget("x", "lib", None), "nightly-x", tmp_path + ) + + +@pytest.mark.parametrize( + "case", + [ + pytest.param( + ReportedCoverageFileCase( + '{"src/lib.rs": {"total": 10, "with_docs": 9}}', + pathlib.Path("coverage-reports/absolute.json"), + None, + (10, 9), + ), + id="absolute-reported-path", + ), + pytest.param( + ReportedCoverageFileCase( + '{"src/lib.rs": {"total": 7, "with_docs": 6}}', + pathlib.Path("target/doc/package.json"), + pathlib.Path("target/doc/package.json"), + (7, 6), + ), + id="relative-reported-path", + ), + ], +) +def test_measure_reads_the_reported_generated_coverage_file( + cargo: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, + case: ReportedCoverageFileCase, +) -> None: + """Read absolute and manifest-relative Rustdoc coverage-file notices.""" + FakeCargo( + cargo, + rustdoc=FakeRustdocResult( + payload=case.payload, + output_path=case.output_path, + reported_path=case.reported_path, + ), + ).install(monkeypatch) + + coverage = cargo.CargoAdapter("cargo").measure( + cargo.DocTarget("x", "lib", None), "nightly-x", tmp_path + ) + + assert coverage == cargo.Coverage(*case.expected), ( + "coverage JSON was not read from the reported generated file" + ) + + +@pytest.mark.parametrize( + ("output", "expected"), + [ + pytest.param( + f'Generated output into "{pathlib.Path.cwd() / "coverage.json"}"', + pathlib.Path.cwd() / "coverage.json", + id="absolute-path", + ), + pytest.param( + 'progress\nGenerated output into "target/doc/package.json"', + pathlib.Path("target/doc/package.json"), + id="relative-path", + ), + ], +) +def test_coverage_output_path_accepts_reported_paths( + cargo: types.ModuleType, + tmp_path: pathlib.Path, + output: str, + expected: pathlib.Path, +) -> None: + """Parse absolute and relative paths from Rustdoc's generated-file notice.""" + target = cargo.DocTarget("x", "lib", None) + + path = cargo.coverage_output_path(target, output, tmp_path) + + assert path == (expected if expected.is_absolute() else tmp_path / expected), ( + "failed to resolve the reported coverage path" + ) + + +@pytest.mark.parametrize("output", ["", "progress only", 'Generated output into "']) +def test_coverage_output_path_rejects_unrelated_output( + cargo: types.ModuleType, + tmp_path: pathlib.Path, + output: str, +) -> None: + """Reject output that does not contain a complete generated-file notice.""" + target = cargo.DocTarget("x", "lib", None) + + with pytest.raises( + RuntimeError, match="did not report the generated coverage JSON path" + ): + cargo.coverage_output_path(target, output, tmp_path) + + +def test_measure_rejects_a_reported_file_that_does_not_exist( + cargo: types.ModuleType, + tmp_path: pathlib.Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Translate a missing reported JSON file into the controlled gate error.""" + FakeCargo( + cargo, + rustdoc=FakeRustdocResult( + output_path=tmp_path / "missing" / "coverage.json", + write_output=False, + ), + ).install(monkeypatch) + + with pytest.raises(RuntimeError, match="cannot read generated coverage JSON"): + cargo.CargoAdapter("cargo").measure( + cargo.DocTarget("x", "lib", None), "nightly-x", tmp_path + ) + + +@pytest.mark.parametrize( + ("target", "selector"), + [ + pytest.param(("netsuke", "lib", None), ["--lib"], id="library"), + pytest.param( + ("netsuke", "bin", "netsuke-bin"), + ["--bin", "netsuke-bin"], + id="binary", + ), + ], +) +def test_rustdoc_args_for_target( + cargo: types.ModuleType, + target: tuple[str, str, str | None], + selector: list[str], +) -> None: + """Build the complete Rustdoc command for library and binary targets.""" + doc_target = cargo.DocTarget(*target) + + args = cargo.rustdoc_args(doc_target, "nightly-x", "cargo") + + assert args == [ + "cargo", + "+nightly-x", + "rustdoc", + "-p", + "netsuke", + *selector, + "--", + "-Z", + "unstable-options", + "--show-coverage", + "--output-format", + "json", + "--document-private-items", + ] + + +def test_cargo_adapter_owns_rustdoc_arguments(cargo: types.ModuleType) -> None: + """Build the unchanged Rustdoc argv through the Cargo adapter directly.""" + assert cargo.rustdoc_args( + cargo.DocTarget("x", "lib", None), "nightly-x", "cargo-wrapper" + )[:5] == ["cargo-wrapper", "+nightly-x", "rustdoc", "-p", "x"] + + +def test_production_adapter_uses_the_configured_cargo_executable( + cargo: types.ModuleType, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Resolve the process executable once at the production adapter boundary.""" + monkeypatch.setenv("CARGO", "cargo-wrapper") + + assert cargo.production_adapter().executable == "cargo-wrapper" diff --git a/scripts/tests/test_doc_coverage_cargo_payload.py b/scripts/tests/test_doc_coverage_cargo_payload.py new file mode 100644 index 000000000..9631061a6 --- /dev/null +++ b/scripts/tests/test_doc_coverage_cargo_payload.py @@ -0,0 +1,96 @@ +"""Test Rustdoc coverage-payload decoding in the Cargo adapter.""" + +from __future__ import annotations + +import dataclasses +import typing as typ + +import pytest + +if typ.TYPE_CHECKING: + import types + + +@dataclasses.dataclass(frozen=True) +class CoveragePayloadFailureCase: + """Define one invalid Rustdoc coverage-payload scenario.""" + + payload: str + diagnostic: str + + +def test_parse_coverage_output_aggregates_multiple_files( + cargo: types.ModuleType, +) -> None: + """Roll per-file totals and documented counts up across the payload.""" + target = cargo.DocTarget("netsuke", "lib", None) + payload = ( + '{"src/a.rs": {"total": 10, "with_docs": 8}, ' + '"src/b.rs": {"total": 5, "with_docs": 3}}' + ) + + coverage = cargo.parse_coverage_output(target, payload) + + assert coverage.total == 15 + assert coverage.with_docs == 11 + + +def test_parse_coverage_output_rejects_malformed_json(cargo: types.ModuleType) -> None: + """Surface non-JSON output as a coverage-gate RuntimeError.""" + target = cargo.DocTarget("netsuke", "lib", None) + + with pytest.raises(RuntimeError, match="did not emit coverage JSON"): + cargo.parse_coverage_output(target, "not json at all") + + +@pytest.mark.parametrize( + "entry", + [ + pytest.param('{"total": true, "with_docs": 0}', id="boolean"), + pytest.param('{"total": 1e999, "with_docs": 0}', id="non-finite"), + pytest.param('{"total": -1, "with_docs": 0}', id="negative"), + pytest.param('{"total": 1.5, "with_docs": 0}', id="non-integer"), + pytest.param('{"total": 1, "with_docs": 2}', id="inconsistent"), + ], +) +def test_parse_coverage_output_rejects_invalid_counts( + cargo: types.ModuleType, + entry: str, +) -> None: + """Reject invalid Rustdoc count invariants as controlled adapter errors.""" + payload = '{"src/lib.rs": ' + entry + "}" + + with pytest.raises(RuntimeError, match="each entry requires total and with_docs"): + cargo.parse_coverage_output(cargo.DocTarget("x", "lib", None), payload) + + +@pytest.mark.parametrize( + "case", + [ + pytest.param( + CoveragePayloadFailureCase("[]", "expected an object"), + id="non-object", + ), + pytest.param( + CoveragePayloadFailureCase( + '{"src/lib.rs": {"total": 1}}', + "each entry requires total and with_docs", + ), + id="missing-with-docs", + ), + pytest.param( + CoveragePayloadFailureCase( + '{"src/lib.rs": {"with_docs": 1}}', + "each entry requires total and with_docs", + ), + id="missing-total", + ), + ], +) +def test_parse_coverage_output_rejects_invalid_shape( + cargo: types.ModuleType, + case: CoveragePayloadFailureCase, +) -> None: + """Reject malformed Rustdoc coverage structures as controlled errors.""" + with pytest.raises(RuntimeError, match=case.diagnostic): + cargo.parse_coverage_output(cargo.DocTarget("x", "lib", None), case.payload) diff --git a/scripts/tests/test_doc_coverage_model.py b/scripts/tests/test_doc_coverage_model.py new file mode 100644 index 000000000..9252287e2 --- /dev/null +++ b/scripts/tests/test_doc_coverage_model.py @@ -0,0 +1,21 @@ +"""Test documentation-coverage value objects.""" + +import pytest +from doc_coverage_model import Coverage + + +def test_aggregation_sums_targets_and_reports_percentage() -> None: + """Aggregate totals roll per-target counts up and report the share.""" + first = Coverage(10, 8) + second = Coverage(5, 5) + + combined = first + second + + assert combined.total == 15 + assert combined.with_docs == 13 + assert combined.percentage == pytest.approx(13 / 15 * 100) + + +def test_empty_run_is_complete_not_a_division_by_zero() -> None: + """Treat a crate with no doc-able targets as a complete, empty run.""" + assert Coverage(0, 0).percentage == 100.0 diff --git a/scripts/tests/test_doc_coverage_runner.py b/scripts/tests/test_doc_coverage_runner.py new file mode 100644 index 000000000..502b2aa8d --- /dev/null +++ b/scripts/tests/test_doc_coverage_runner.py @@ -0,0 +1,168 @@ +"""Test documentation-coverage target selection and measurement orchestration.""" + +from __future__ import annotations + +import typing as typ + +import pytest + +if typ.TYPE_CHECKING: + import pathlib + import types + + +def lib_target(name: str) -> dict[str, object]: + """Return one library target as ``cargo metadata`` reports it.""" + return {"name": name, "kind": ["lib"]} + + +def bin_target(name: str) -> dict[str, object]: + """Return one binary target as ``cargo metadata`` reports it.""" + return {"name": name, "kind": ["bin"]} + + +def metadata_for(packages: list[dict[str, object]]) -> dict[str, object]: + """Build the ``cargo metadata`` document the runner consumes.""" + return { + "packages": packages, + "workspace_members": [package["id"] for package in packages], + } + + +class FakeCoverageAdapter: + """Record runner calls while returning one target and its coverage result.""" + + def __init__(self, calls: list[object], coverage: object) -> None: + self._calls = calls + self._coverage = coverage + + def load_metadata( + self, toolchain: str, manifest_root: pathlib.Path + ) -> dict[str, object]: + """Return metadata containing the one target under test.""" + self._calls.append(("metadata", toolchain, manifest_root)) + return { + "packages": [ + { + "id": "pkg:x:1.0.0", + "name": "x", + "targets": [{"name": "x", "kind": ["lib"]}], + } + ], + "workspace_members": ["pkg:x:1.0.0"], + } + + def measure( + self, target: object, toolchain: str, manifest_root: pathlib.Path + ) -> object: + """Record the selected target and return fixed coverage.""" + self._calls.append((target, toolchain, manifest_root)) + return self._coverage + + +def test_target_discovery_skips_non_doc_targets(runner: types.ModuleType) -> None: + """Exclude build scripts, tests, examples, and benches from the surface.""" + metadata = metadata_for( + [ + { + "id": "pkg:netsuke:0.1.0", + "name": "netsuke", + "targets": [ + lib_target("netsuke"), + bin_target("netsuke-bin"), + bin_target("extra"), + {"name": "build-main", "kind": ["custom-build"]}, + {"name": "integration", "kind": ["test"]}, + {"name": "sample", "kind": ["example"]}, + {"name": "benchmark", "kind": ["bench"]}, + ], + } + ] + ) + + targets = runner.doc_targets(metadata) + + assert [target.kind for target in targets] == ["lib", "bin", "bin"] + assert {target.name for target in targets if target.kind == "bin"} == { + "netsuke-bin", + "extra", + } + + +def test_target_discovery_excludes_outside_workspace(runner: types.ModuleType) -> None: + """Exclude dependency crates outside ``workspace_members`` from measurement.""" + member = { + "id": "pkg:member:0.1.0", + "name": "member", + "targets": [lib_target("member")], + } + dependency = { + "id": "pkg:dependency:0.1.0", + "name": "dependency", + "targets": [lib_target("dependency")], + } + metadata = { + "packages": [member, dependency], + "workspace_members": ["pkg:member:0.1.0"], + } + + targets = runner.doc_targets(metadata) + + assert [target.package for target in targets] == ["member"] + + +def test_malformed_metadata_shape_is_a_measurement_error( + runner: types.ModuleType, +) -> None: + """Reject valid JSON without workspace keys rather than leaking a KeyError.""" + with pytest.raises(RuntimeError, match="lacks the workspace"): + runner.doc_targets({"packages": []}) + + +def test_target_without_kind_is_skipped(runner: types.ModuleType) -> None: + """Ignore a target record that lacks its kind list.""" + metadata = metadata_for( + [ + { + "id": "pkg:x:0.1.0", + "name": "x", + "targets": [{"name": "mystery"}], + } + ] + ) + + assert runner.doc_targets(metadata) == [] + + +def test_pinned_toolchain_reads_the_channel( + runner: types.ModuleType, + tmp_path: pathlib.Path, +) -> None: + """Read the pinned channel from rust-toolchain.toml.""" + (tmp_path / "rust-toolchain.toml").write_text( + '[toolchain]\nchannel = "nightly-from-pin"\n', + encoding="utf-8", + ) + + assert runner.pinned_toolchain(tmp_path) == "nightly-from-pin" + + +def test_runner_delegates_to_cargo_adapter( + runner: types.ModuleType, + tmp_path: pathlib.Path, +) -> None: + """Delegate target work to the adapter while retaining aggregation.""" + target = runner.DocTarget("x", "lib", None) + coverage = runner.Coverage(3, 2) + calls: list[object] = [] + + adapter = FakeCoverageAdapter(calls, coverage) + + totals, rows = runner.run_measurements("nightly-x", tmp_path, adapter) + + assert totals == coverage + assert rows == [(target, coverage)] + assert calls == [ + ("metadata", "nightly-x", tmp_path), + (target, "nightly-x", tmp_path), + ] diff --git a/src/graph_view/tests.rs b/src/graph_view/tests.rs index b62a6de1a..3413639dc 100644 --- a/src/graph_view/tests.rs +++ b/src/graph_view/tests.rs @@ -22,9 +22,9 @@ use support::{EdgeFixture, add_edge, make_action, p, render_dot, render_html}; fn empty_graph_yields_empty_view() { let graph = BuildGraph::default(); let view = GraphView::from_build_graph(&graph); - assert!(view.nodes.is_empty()); - assert!(view.edges.is_empty()); - assert!(view.default_targets.is_empty()); + assert_eq!(view.nodes, Vec::new()); + assert_eq!(view.edges, Vec::new()); + assert_eq!(view.default_targets, Vec::::new()); assert!(view.limit.is_none()); } diff --git a/src/hex_property_tests.rs b/src/hex_property_tests.rs index bce5c2dfb..a11ef9f06 100644 --- a/src/hex_property_tests.rs +++ b/src/hex_property_tests.rs @@ -25,8 +25,12 @@ fn decode_lower_hex(hex: &str) -> Option> { return None; } let digits = hex.as_bytes(); + // The length is a multiple of two, checked above, so the remainder chunk + // `as_chunks` returns alongside the pairs is always empty. digits - .chunks_exact(2) + .as_chunks::<2>() + .0 + .iter() .map(|pair| { let text = std::str::from_utf8(pair).ok()?; u8::from_str_radix(text, 16).ok() diff --git a/src/status_timing_format_tests.rs b/src/status_timing_format_tests.rs index bbcb6d123..cea99a9ab 100644 --- a/src/status_timing_format_tests.rs +++ b/src/status_timing_format_tests.rs @@ -88,7 +88,7 @@ fn timing_recorder_incomplete_flow_has_no_summary_lines(test_prefs: OutputPrefs) ); let lines = render_summary_lines(test_prefs, state.completed_stages()); - assert!(lines.is_empty()); + assert_eq!(lines, Vec::::new()); } #[rstest] diff --git a/test_support/src/dev_fast/sandbox/mod.rs b/test_support/src/dev_fast/sandbox/mod.rs index 06d2ad7cf..4c9d4a7c5 100644 --- a/test_support/src/dev_fast/sandbox/mod.rs +++ b/test_support/src/dev_fast/sandbox/mod.rs @@ -353,7 +353,8 @@ pub fn pinned_mold_version() -> Result { /// The repository's toolchain, read from `rust-toolchain.toml`. /// /// dev-fast deliberately shares it rather than pinning a second nightly, so the -/// accelerated loop and the gates borrow-check identically under Polonius. +/// accelerated loop and the gates borrow-check identically; the pinned nightly +/// is what enables Polonius. /// /// # Errors /// diff --git a/test_support/src/netsuke.rs b/test_support/src/netsuke/locator.rs similarity index 77% rename from test_support/src/netsuke.rs rename to test_support/src/netsuke/locator.rs index 58c7fff53..720f68b68 100644 --- a/test_support/src/netsuke.rs +++ b/test_support/src/netsuke/locator.rs @@ -1,13 +1,18 @@ -//! Helpers for invoking the built `netsuke` binary in tests. +//! Locating the built `netsuke` executable from a running test. //! -//! These utilities use `assert_cmd` to locate the current workspace's -//! `netsuke` executable and run it in a controlled working directory, -//! capturing stdout/stderr for assertions. +//! Split from the parent `netsuke` module, which owns running the binary once +//! it is found. The two concerns are independent: everything here is pure path +//! reasoning over an injected environment, with the only filesystem contact +//! being the existence probe, whereas the parent module spawns processes. +//! Keeping them apart also keeps each within the module line cap. +//! +//! Reuse policy: private to `test_support::netsuke`. Tests reach the binary +//! through `run_netsuke_in` or `run_netsuke_in_with_env`, never through the +//! locator directly, so nothing here is exported from the crate. use anyhow::{Context, Result, bail}; use camino::{Utf8Path, Utf8PathBuf}; use mockable::{DefaultEnv, Env}; -use std::path::Path; /// Locate the built `netsuke` executable for integration-style tests. /// @@ -15,17 +20,46 @@ use std::path::Path; /// fall back to `CARGO_TARGET_DIR` when Cargo's `build.build-dir` splits /// intermediate artefacts from final ones: test executables then run from the /// build dir while the uplifted binary lands under the target dir. -fn netsuke_executable() -> Result { +pub(super) fn netsuke_executable() -> Result { let raw_exe = std::env::current_exe().context("locate current test executable")?; let current_exe = Utf8PathBuf::from_path_buf(raw_exe) .map_err(|path| anyhow::anyhow!("test executable path {} is not UTF-8", path.display()))?; netsuke_executable_from(&DefaultEnv, ¤t_exe) } +/// Reduces a test executable's directory to the profile directory above it. +/// +/// Cargo has placed integration-test executables in two different places, and +/// the final binary is uplifted to the profile directory in both cases: +/// +/// - `/deps/`, the long-standing layout; +/// - `/build///out/`, used by the Cargo shipped with +/// the 1.99 nightlies, which has no `deps` directory at all. +/// +/// Anything else is returned unchanged, so an unrecognized layout degrades to +/// looking beside the executable rather than failing here. The result also +/// supplies the profile name for the `CARGO_TARGET_DIR` fallbacks, so both +/// layouts derive the same name. +fn profile_dir(exe_dir: &Utf8Path) -> &Utf8Path { + match exe_dir.file_name() { + Some("deps") => exe_dir.parent().unwrap_or(exe_dir), + Some("out") => { + let build = exe_dir + .parent() + .and_then(Utf8Path::parent) + .and_then(Utf8Path::parent); + match build { + Some(dir) if dir.file_name() == Some("build") => dir.parent().unwrap_or(exe_dir), + _ => exe_dir, + } + } + _ => exe_dir, + } +} /// Locate the `netsuke` binary from an injected environment and test path. /// /// Candidates are checked in order: -/// 1. beside the test executable (its directory, minus a trailing `deps`); +/// 1. the profile directory above the test executable (see [`profile_dir`]); /// 2. `CARGO_TARGET_DIR//` for split `build.build-dir` layouts; /// 3. `CARGO_TARGET_DIR///` for `--target` builds, where the /// profile directory nests under the target triple. @@ -33,14 +67,11 @@ fn netsuke_executable() -> Result { /// Filesystem errors other than "not found" are surfaced rather than treated /// as a missing candidate. fn netsuke_executable_from(env: &impl Env, current_exe: &Utf8Path) -> Result { - let mut exe_dir = current_exe - .parent() - .context("test executable should have a parent directory")?; - if exe_dir.file_name() == Some("deps") { - exe_dir = exe_dir + let exe_dir = profile_dir( + current_exe .parent() - .context("deps directory should have a parent")?; - } + .context("test executable should have a parent directory")?, + ); let binary_name = format!("netsuke{}", std::env::consts::EXE_SUFFIX); let candidates = candidate_paths(env, exe_dir, &binary_name); @@ -78,86 +109,6 @@ fn candidate_paths(env: &impl Env, exe_dir: &Utf8Path, binary_name: &str) -> Vec } candidates } - -/// Captured output from a `netsuke` invocation. -#[derive(Debug)] -pub struct NetsukeRun { - /// Captured stdout (lossy UTF-8). - pub stdout: String, - /// Captured stderr (lossy UTF-8). - pub stderr: String, - /// Whether the command exited successfully. - pub success: bool, -} - -/// Run `netsuke` in `current_dir` with the supplied args. -/// -/// The function clears `PATH` so tests don't accidentally execute a host -/// dependency. Other process environment variables are inherited, except for -/// configuration selectors that this helper removes explicitly. -/// -/// # Errors -/// -/// Returns an error when `netsuke` cannot be located or the process cannot be -/// spawned. -pub fn run_netsuke_in(current_dir: &Path, args: &[&str]) -> Result { - let isolated_config_home = current_dir.join(".config"); - let executable = netsuke_executable()?; - let mut cmd = assert_cmd::Command::new(executable); - let output = cmd - .current_dir(current_dir) - .env("PATH", "") - .env_remove("NETSUKE_CONFIG_PATH") - .env_remove("NETSUKE_OUTPUT_FORMAT") - .env("HOME", current_dir) - .env("XDG_CONFIG_HOME", &isolated_config_home) - .args(args) - .output() - .context("run netsuke command")?; - Ok(NetsukeRun { - stdout: String::from_utf8_lossy(&output.stdout).into_owned(), - stderr: String::from_utf8_lossy(&output.stderr).into_owned(), - success: output.status.success(), - }) -} - -/// Run `netsuke` in `current_dir` with an isolated environment. -/// -/// Unlike [`run_netsuke_in`], this variant uses `env_clear()` so the child -/// inherits no process environment variables. The child receives only an -/// isolated `PATH`, `HOME`, `XDG_CONFIG_HOME`, and the variables supplied in -/// `extra_env`. This prevents process-level environment races when tests run -/// in parallel. -/// -/// # Errors -/// -/// Returns an error when `netsuke` cannot be located or the process cannot be -/// spawned. -pub fn run_netsuke_in_with_env( - current_dir: &Path, - args: &[&str], - extra_env: &[(&str, &str)], -) -> Result { - let executable = netsuke_executable()?; - let mut cmd = assert_cmd::Command::new(executable); - let isolated_config_home = current_dir.join(".config"); - let isolated_path = tempfile::tempdir().context("create isolated executable directory")?; - cmd.current_dir(current_dir) - .env_clear() - .env("PATH", isolated_path.path()) - .env("HOME", current_dir) - .env("XDG_CONFIG_HOME", isolated_config_home); - for &(key, value) in extra_env { - cmd.env(key, value); - } - let output = cmd.args(args).output().context("run netsuke command")?; - Ok(NetsukeRun { - stdout: String::from_utf8_lossy(&output.stdout).into_owned(), - stderr: String::from_utf8_lossy(&output.stderr).into_owned(), - success: output.status.success(), - }) -} - #[cfg(test)] mod tests { //! Unit tests for the netsuke binary locator. @@ -268,6 +219,28 @@ mod tests { "target/x86_64-unknown-linux-gnu/debug", "triple fallback should resolve" ))] + // Cargo 1.99 nightlies drop `deps/` and run integration tests from a + // build directory under the profile, uplifting the binary as before. + #[case::build_out_primary(LocatorScenario::new( + "target/debug/build/netsuke-build/abc123/out/test-exe", + None, + "target/debug", + "build-out layout should resolve beside the profile" + ))] + #[case::build_out_profile_fallback(LocatorScenario::new( + "build/debug/build/netsuke-build/abc123/out/test-exe", + Some("target"), + "target/debug", + "build-out layout should reach the profile fallback" + ))] + // A directory merely named `out` is not the build layout, so the locator + // must look beside the executable rather than four levels up. + #[case::unrecognized_out_directory(LocatorScenario::new( + "somewhere/out/test-exe", + None, + "somewhere/out", + "an unrecognized `out` directory should not be stripped" + ))] fn resolves_each_candidate_layout( temp_root: TempRoot, #[case] scenario: LocatorScenario, diff --git a/test_support/src/netsuke/mod.rs b/test_support/src/netsuke/mod.rs new file mode 100644 index 000000000..4167ac137 --- /dev/null +++ b/test_support/src/netsuke/mod.rs @@ -0,0 +1,90 @@ +//! Helpers for invoking the built `netsuke` binary in tests. +//! +//! These utilities use `assert_cmd` to run the current workspace's `netsuke` +//! executable in a controlled working directory, capturing stdout/stderr for +//! assertions. Finding that executable is the `locator` submodule's job. + +mod locator; + +use anyhow::{Context, Result}; +use locator::netsuke_executable; +use std::path::Path; + +/// Captured output from a `netsuke` invocation. +#[derive(Debug)] +pub struct NetsukeRun { + /// Captured stdout (lossy UTF-8). + pub stdout: String, + /// Captured stderr (lossy UTF-8). + pub stderr: String, + /// Whether the command exited successfully. + pub success: bool, +} + +/// Run `netsuke` in `current_dir` with the supplied args. +/// +/// The function clears `PATH` so tests don't accidentally execute a host +/// dependency. Other process environment variables are inherited, except for +/// configuration selectors that this helper removes explicitly. +/// +/// # Errors +/// +/// Returns an error when `netsuke` cannot be located or the process cannot be +/// spawned. +pub fn run_netsuke_in(current_dir: &Path, args: &[&str]) -> Result { + let isolated_config_home = current_dir.join(".config"); + let executable = netsuke_executable()?; + let mut cmd = assert_cmd::Command::new(executable); + let output = cmd + .current_dir(current_dir) + .env("PATH", "") + .env_remove("NETSUKE_CONFIG_PATH") + .env_remove("NETSUKE_OUTPUT_FORMAT") + .env("HOME", current_dir) + .env("XDG_CONFIG_HOME", &isolated_config_home) + .args(args) + .output() + .context("run netsuke command")?; + Ok(NetsukeRun { + stdout: String::from_utf8_lossy(&output.stdout).into_owned(), + stderr: String::from_utf8_lossy(&output.stderr).into_owned(), + success: output.status.success(), + }) +} + +/// Run `netsuke` in `current_dir` with an isolated environment. +/// +/// Unlike [`run_netsuke_in`], this variant uses `env_clear()` so the child +/// inherits no process environment variables. The child receives only an +/// isolated `PATH`, `HOME`, `XDG_CONFIG_HOME`, and the variables supplied in +/// `extra_env`. This prevents process-level environment races when tests run +/// in parallel. +/// +/// # Errors +/// +/// Returns an error when `netsuke` cannot be located or the process cannot be +/// spawned. +pub fn run_netsuke_in_with_env( + current_dir: &Path, + args: &[&str], + extra_env: &[(&str, &str)], +) -> Result { + let executable = netsuke_executable()?; + let mut cmd = assert_cmd::Command::new(executable); + let isolated_config_home = current_dir.join(".config"); + let isolated_path = tempfile::tempdir().context("create isolated executable directory")?; + cmd.current_dir(current_dir) + .env_clear() + .env("PATH", isolated_path.path()) + .env("HOME", current_dir) + .env("XDG_CONFIG_HOME", isolated_config_home); + for &(key, value) in extra_env { + cmd.env(key, value); + } + let output = cmd.args(args).output().context("run netsuke command")?; + Ok(NetsukeRun { + stdout: String::from_utf8_lossy(&output.stdout).into_owned(), + stderr: String::from_utf8_lossy(&output.stderr).into_owned(), + success: output.status.success(), + }) +} diff --git a/tests/command_env_ui_tests.rs b/tests/command_env_ui_tests.rs index d64e802e0..014783f30 100644 --- a/tests/command_env_ui_tests.rs +++ b/tests/command_env_ui_tests.rs @@ -15,12 +15,20 @@ //! diagnostic wording for a missing item would make the suite fail on //! compiler upgrades without guarding anything extra. //! -//! Trybuild cannot drive this: it removes ambient `RUSTFLAGS` and overrides -//! workspace `build.rustflags` outright, so it would rebuild the `netsuke` -//! dependency without `-Zpolonius=next` and reject the crate's `POLONIUS(...)` -//! sites (see docs/polonius.md). Instead the `netsuke` rlib is built by Cargo -//! — which does inherit the ambient flags — and the fixture is compiled -//! directly with the workspace `rustc` against that rlib. +//! Trybuild drove this during the Polonius migration and could not: it removes +//! ambient `RUSTFLAGS` and overrides workspace `build.rustflags` outright, so +//! while Polonius was flag-gated it rebuilt the `netsuke` dependency without +//! the analysis and rejected the crate's `POLONIUS(...)` sites (see +//! docs/polonius.md). The pinned nightly now enables Polonius by default, so +//! that hazard is gone, but the direct-compile harness is kept: it needs no +//! scratch project and no toolchain-sensitive `.stderr` snapshot. The `netsuke` +//! rlib is built by Cargo, and the fixture is compiled directly with the +//! workspace `rustc` against it. + +#[path = "support/cargo_artifacts.rs"] +mod cargo_artifacts; +#[path = "support/rustc_response_file.rs"] +mod rustc_response_file; use std::{ io, @@ -127,18 +135,18 @@ fn compile_public_api_fixture(source: &str, failure_message: &str) -> io::Result Ok(()) } -/// The `netsuke` rlib and the deps directory holding its dependencies. +/// The `netsuke` rlib and every directory holding its dependencies. struct NetsukeRlib { rlib: PathBuf, - deps_dir: PathBuf, + deps_dirs: Vec, } impl NetsukeRlib { /// Build the `netsuke` library with Cargo and locate the resulting rlib. /// - /// Cargo inherits the ambient `RUSTFLAGS`, so the rlib is borrow-checked - /// with the same Polonius flags as the rest of the suite. The package is - /// named `netsuke-build`, but the lib target — and therefore the + /// Cargo inherits the ambient `RUSTFLAGS` and the pinned toolchain, so the + /// rlib is borrow-checked exactly as the rest of the suite is. The package + /// is named `netsuke-build`, but the lib target — and therefore the /// `--extern` name — is `netsuke`. fn build() -> io::Result { let output = Command::new(cargo()) @@ -158,21 +166,30 @@ impl NetsukeRlib { let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); let rlib = stdout .lines() - .filter_map(netsuke_rlib_in_message) + .filter_map(|line| cargo_artifacts::library_path_in_message(line, "netsuke")) .next_back() .ok_or_else(|| io::Error::other("cargo reported no netsuke rlib artefact"))?; - // Cargo uplifts the top-level package's artefacts out of `deps/` into - // the profile directory, so the rlib's own parent is not necessarily - // where the dependency rlibs live. - let parent = rlib - .parent() - .ok_or_else(|| io::Error::other("the rlib path should have a parent"))?; - let deps_dir = if parent.file_name() == Some(std::ffi::OsStr::new("deps")) { - parent.to_path_buf() - } else { - parent.join("deps") - }; - Ok(Self { rlib, deps_dir }) + // Dependencies do not sit in one predictable directory. Cargo uplifts + // the top-level package's artefacts into the profile directory, and + // the Cargo shipped with the 1.99 nightlies gives every crate its own + // build directory rather than one shared `deps/`. Each + // compiler-artifact message names where its own artefacts really + // landed, so derive the search path from what Cargo reports. + let mut deps_dirs: Vec = Vec::new(); + for parent in stdout + .lines() + .flat_map(cargo_artifacts::dependency_dirs_in_message) + { + if !deps_dirs.contains(&parent) { + deps_dirs.push(parent); + } + } + if deps_dirs.is_empty() { + return Err(io::Error::other( + "cargo reported no library artefacts to derive dependency dirs from", + )); + } + Ok(Self { rlib, deps_dirs }) } /// Type-check `source` with or without the `netsuke` rlib. @@ -182,55 +199,50 @@ impl NetsukeRlib { /// dependency tree. When `include_netsuke_extern` is `false`, the fixture /// must fail because it imports `netsuke`; this control proves the normal /// compile path receives an effective `--extern` argument. + /// + /// The arguments travel in a `rustc` response file rather than on the + /// command line. Cargo 1.99 gives every crate its own artefact directory, + /// so `deps_dirs` holds one entry per dependency; passed directly, a list + /// that long can exceed the Windows `CreateProcess` command-line limit and + /// fail the spawn with `Os { code: 206 }` before `rustc` runs. Every + /// directory is required to avoid `E0463`, so the list moves off the + /// command line rather than being shortened. fn compile(&self, source: &str, include_netsuke_extern: bool) -> io::Result { let output_dir = tempfile::tempdir()?; - let mut command = Command::new(rustc()); - command - .arg("--edition=2024") - .arg("--crate-type=bin") - .arg("--emit=metadata") - .arg(manifest_dir().join(source)); + let mut args = vec![ + String::from("--edition=2024"), + String::from("--crate-type=bin"), + String::from("--emit=metadata"), + manifest_dir().join(source).to_string_lossy().into_owned(), + ]; if include_netsuke_extern { - command - .arg("--extern") - .arg(format!("netsuke={}", self.rlib.display())); + args.extend([ + String::from("--extern"), + format!("netsuke={}", self.rlib.display()), + ]); } - command - .arg("-L") - .arg(format!("dependency={}", self.deps_dir.display())) - .arg("-o") - .arg(output_dir.path().join("command-env-ui.rmeta")) - .output() - } -} + args.extend( + self.deps_dirs + .iter() + .flat_map(|dir| [String::from("-L"), format!("dependency={}", dir.display())]), + ); + args.push(String::from("-o")); + args.push( + output_dir + .path() + .join("command-env-ui.rmeta") + .to_string_lossy() + .into_owned(), + ); -/// Extract the `netsuke` lib rlib path from one Cargo JSON message, if any. -/// -/// The package also ships a `netsuke` bin target; requiring an `.rlib` -/// filename keeps the filter on the library artefact. -fn netsuke_rlib_in_message(line: &str) -> Option { - let message: serde_json::Value = serde_json::from_str(line).ok()?; - if message.get("reason")? != "compiler-artifact" - || message.get("target")?.get("name")? != "netsuke" - { - return None; + let response = rustc_response_file::write(output_dir.path(), "command-env-ui.args", &args)?; + // `output_dir` owns the response file and stays in scope across the + // call below, so the file still exists when `rustc` opens it at spawn. + Command::new(rustc()).arg(response).output() } - message - .get("filenames")? - .as_array()? - .iter() - .filter_map(|filename| filename.as_str()) - .filter(|filename| { - Path::new(filename) - .extension() - .is_some_and(|extension| extension.eq_ignore_ascii_case("rlib")) - }) - .map(PathBuf::from) - .next() } - fn manifest_dir() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")) } diff --git a/tests/dev_fast_make_target_tests.rs b/tests/dev_fast_make_target_tests.rs index 4ba7efe56..83a66e68e 100644 --- a/tests/dev_fast_make_target_tests.rs +++ b/tests/dev_fast_make_target_tests.rs @@ -182,10 +182,7 @@ fn a_drifting_mold_invokes_cargo_not_at_all(#[case] target: &str) -> Result<()> #[case::unstable_flag("unstable.codegen-backend", "unstable.codegen-backend = true")] #[case::linux_rustflags( "target", - concat!( - "target.'cfg(target_os = \"linux\")'.rustflags = ", - "[\"-Zpolonius=next\", \"-Clink-arg=-fuse-ld=mold\"]", - ) + "target.'cfg(target_os = \"linux\")'.rustflags = [\"-Clink-arg=-fuse-ld=mold\"]" )] fn cargo_resolves_the_fragment_to_the_intended_settings( #[case] query: &str, @@ -258,17 +255,6 @@ fn the_cargo_fragment_selects_cranelift_and_mold() -> Result<()> { .any(|flag| flag.contains("-fuse-ld=mold")), "the Linux target must select mold, got `{linux:?}`" ); - // Cargo picks one rustflags source rather than merging, and this table - // outranks `.cargo/config.toml`'s `[build]` table. Dropping the Polonius - // flag here does not merely diverge from the gate: the tree does not - // borrow-check without it, so `make dev-build` stops compiling. - ensure!( - linux - .iter() - .filter_map(toml::Value::as_str) - .any(|flag| flag == "-Zpolonius=next"), - "the Linux target must restate the Polonius flag it shadows, got `{linux:?}`" - ); Ok(()) } diff --git a/tests/documentation_installation_tests.rs b/tests/documentation_installation_tests.rs index 05bebc7b8..b167c597d 100644 --- a/tests/documentation_installation_tests.rs +++ b/tests/documentation_installation_tests.rs @@ -28,13 +28,12 @@ fn installation_examples_match_source_and_release_contracts() -> Result<()> { } #[test] -fn registry_install_examples_pin_toolchain_and_polonius() -> Result<()> { - // Registry installs build outside a checkout, where neither - // rust-toolchain.toml nor .cargo/config.toml applies, so every tagged - // example installing from crates.io must select the pinned nightly and - // pass the Polonius flag itself. `cargo binstall` fetches a prebuilt - // binary and `cargo install --path .` runs inside a checkout, so both - // are exempt. +fn registry_install_examples_pin_the_toolchain() -> Result<()> { + // Registry installs build outside a checkout, where rust-toolchain.toml + // does not apply, so every tagged example installing from crates.io must + // select the pinned nightly itself; that nightly is what enables Polonius. + // `cargo binstall` fetches a prebuilt binary and `cargo install --path .` + // runs inside a checkout, so both are exempt. let mut registry_install_ids = Vec::new(); for example in load_documented_examples()? { let mut example_matches = false; @@ -43,15 +42,10 @@ fn registry_install_examples_pin_toolchain_and_polonius() -> Result<()> { continue; } ensure!( - line.contains("cargo +nightly-2026-06-25 install netsuke-build"), + line.contains("cargo +nightly-2026-08-23 install netsuke-build"), "{id} must install with the pinned nightly toolchain: {line}", id = example.id ); - ensure!( - line.contains("RUSTFLAGS=-Zpolonius=next"), - "{id} must pass the Polonius borrow-checker flag: {line}", - id = example.id - ); example_matches = true; } if example_matches { @@ -88,12 +82,12 @@ fn assert_release_installation_contract() -> Result<()> { ); let readme_release = documented_example("readme-crates-io-install")?; let guide_release = documented_example("guide-crates-io-install")?; - // Registry installs run outside a checkout, so the packaged source sees - // neither rust-toolchain.toml nor .cargo/config.toml; the documented - // command must select the pinned nightly and the Polonius flag itself. + // Registry installs run outside a checkout, so the packaged source does + // not see rust-toolchain.toml; the documented command must select the + // pinned nightly itself, which is what enables Polonius. let expected_release = concat!( - "rustup toolchain install nightly-2026-06-25\n", - "RUSTFLAGS=-Zpolonius=next cargo +nightly-2026-06-25 install netsuke-build\n" + "rustup toolchain install nightly-2026-08-23\n", + "cargo +nightly-2026-08-23 install netsuke-build\n" ); ensure!(readme_release.body == expected_release, "README drifted"); ensure!(guide_release.body == expected_release, "user guide drifted"); diff --git a/tests/ir_tests.rs b/tests/ir_tests.rs index beba6a676..bc9507e5d 100644 --- a/tests/ir_tests.rs +++ b/tests/ir_tests.rs @@ -15,7 +15,7 @@ fn build_graph_default_is_empty() { let graph = BuildGraph::default(); assert!(graph.actions.is_empty()); assert!(graph.targets.is_empty()); - assert!(graph.default_targets.is_empty()); + assert_eq!(graph.default_targets, Vec::::new()); } #[rstest] diff --git a/tests/kani_cfg_ui_tests.rs b/tests/kani_cfg_ui_tests.rs index 69705f062..73b40af3f 100644 --- a/tests/kani_cfg_ui_tests.rs +++ b/tests/kani_cfg_ui_tests.rs @@ -14,12 +14,13 @@ use std::{ /// Verify repository policy files used by the `cfg(kani)` compile contract. /// /// The fixture is compiled with the workspace `rustc` and then executed, so -/// its assertions run against the checked-in policy files. Trybuild -/// previously drove this case, but trybuild discards ambient `RUSTFLAGS` -/// and workspace `build.rustflags`, so it rebuilt the `netsuke` dependency -/// without `-Zpolonius=next` and rejected the crate's `POLONIUS(...)` -/// sites; the fixture needs no dependencies, so a direct compile preserves -/// the contract (see docs/polonius.md). +/// its assertions run against the checked-in policy files. Trybuild previously +/// drove this case, but it discards ambient `RUSTFLAGS` and workspace +/// `build.rustflags`, so while Polonius was flag-gated it rebuilt the `netsuke` +/// dependency without the analysis and rejected the crate's `POLONIUS(...)` +/// sites (see docs/polonius.md). The pinned nightly now enables Polonius by +/// default, but the fixture needs no dependencies at all, so the direct +/// compile remains the simpler harness. #[test] fn compiled_fixture_validates_kani_cfg_policy_sources() -> io::Result<()> { let output_dir = tempfile::tempdir()?; diff --git a/tests/locale_stub_ui_tests.rs b/tests/locale_stub_ui_tests.rs index b6c582042..8d8294309 100644 --- a/tests/locale_stub_ui_tests.rs +++ b/tests/locale_stub_ui_tests.rs @@ -5,16 +5,22 @@ //! then panic at run time for the common "no locale set" case. These tests //! keep that a compile-time contract rather than a doc-comment promise. //! -//! Trybuild cannot drive them: it removes ambient `RUSTFLAGS` and overrides -//! workspace `build.rustflags` outright (`env_remove("RUSTFLAGS")` plus -//! `--config=build.rustflags=…` in its cargo invocations), so it would -//! rebuild the `netsuke` dependency without `-Zpolonius=next` and reject the -//! crate's `POLONIUS(...)` sites (see docs/polonius.md). Instead the -//! `test_support` rlib is built by Cargo — which does inherit the ambient -//! flags — and the fixtures are compiled directly with the workspace `rustc` -//! against that rlib. +//! Trybuild drove these during the Polonius migration and could not: it +//! removes ambient `RUSTFLAGS` and overrides workspace `build.rustflags` +//! outright (`env_remove("RUSTFLAGS")` plus `--config=build.rustflags=…` in +//! its cargo invocations), so while Polonius was flag-gated it rebuilt the +//! `test_support` dependency without the analysis and rejected the crate's +//! `POLONIUS(...)` sites (see docs/polonius.md). The pinned nightly now +//! enables Polonius by default, so that hazard is gone, but the direct-compile +//! harness is kept: it needs no scratch project and no toolchain-sensitive +//! `.stderr` snapshot. The `test_support` rlib is built by Cargo, and the +//! fixtures are compiled directly with the workspace `rustc` against it. + +#[path = "support/cargo_artifacts.rs"] +mod cargo_artifacts; +#[path = "support/rustc_response_file.rs"] +mod rustc_response_file; -use camino::{Utf8Path, Utf8PathBuf}; use rstest::{fixture, rstest}; use std::{ io, @@ -79,16 +85,15 @@ fn stub_env_builders_compile_under_the_same_harness( /// The `test_support` rlib and the directories holding its dependencies. struct TestSupportRlib { - rlib: Utf8PathBuf, - deps_dirs: Vec, + rlib: PathBuf, + deps_dirs: Vec, } impl TestSupportRlib { /// Build `test_support` with Cargo and locate the resulting rlib. /// - /// Cargo inherits the ambient `RUSTFLAGS`, so the rlib is borrow-checked - /// with the same Polonius flags as the rest of the suite — the property - /// trybuild could not preserve. + /// Cargo inherits the ambient `RUSTFLAGS` and the pinned toolchain, so the + /// rlib is borrow-checked exactly as the rest of the suite is. fn build() -> io::Result { Self::build_with(&[]) } @@ -118,16 +123,21 @@ impl TestSupportRlib { let stdout = String::from_utf8_lossy(&output.stdout).into_owned(); let rlib = stdout .lines() - .filter_map(test_support_rlib_in_message) + .filter_map(|line| cargo_artifacts::library_path_in_message(line, "test_support")) .next_back() .ok_or_else(|| io::Error::other("cargo reported no test_support rlib artefact"))?; - // Dependency rlibs do not necessarily sit beside the uplifted + // Dependencies do not necessarily sit beside the uplifted // `test_support` rlib: Cargo's `build.build-dir` setting splits - // intermediate artefacts (where dependencies live) from final ones. - // Every compiler-artifact message names its rlib's real location, so - // collect each artefact's parent directory for `-L dependency=`. - let mut deps_dirs: Vec = Vec::new(); - for parent in stdout.lines().flat_map(rlib_parents_in_message) { + // intermediate artefacts (where dependencies live) from final ones, + // and the Cargo shipped with the 1.99 nightlies gives every crate its + // own directory rather than one shared `deps/`. Every + // compiler-artifact message names where its own artefacts really + // landed, so collect each parent directory for `-L dependency=`. + let mut deps_dirs = Vec::new(); + for parent in stdout + .lines() + .flat_map(cargo_artifacts::dependency_dirs_in_message) + { if !deps_dirs.contains(&parent) { deps_dirs.push(parent); } @@ -144,75 +154,48 @@ impl TestSupportRlib { /// /// `--emit=metadata` is enough to surface the missing-item error while /// sparing the harness a full link of `test_support`'s dependency tree. + /// + /// The arguments travel in a `rustc` response file rather than on the + /// command line. Cargo 1.99 gives every crate its own artefact directory, + /// so `deps_dirs` holds one entry per dependency, and the split-build test + /// adds long temporary roots on top; passed directly, the result exceeds + /// the Windows `CreateProcess` command-line limit and the spawn fails with + /// `Os { code: 206 }` before `rustc` runs. Every directory is required to + /// avoid `E0463`, so the list moves off the command line rather than being + /// shortened. fn compile(&self, source: &str) -> io::Result { let output_dir = tempfile::tempdir()?; - Command::new(rustc()) - .arg("--edition=2024") - .arg("--crate-type=bin") - .arg("--emit=metadata") - .arg(manifest_dir().join(source)) - .arg("--extern") - .arg(format!("test_support={}", self.rlib)) - .args( - self.deps_dirs - .iter() - .flat_map(|dir| [String::from("-L"), format!("dependency={dir}")]), - ) - .arg("-o") - .arg(output_dir.path().join("stub-env-ui.rmeta")) - .output() - } -} - -/// Extract a compiler-artifact message's target name and rlib paths. -/// -/// Returns `None` for lines that are not valid JSON, not compiler-artifact -/// messages, or that lack a target name; the rlib list may be empty for -/// artefacts that emit no rlib. -fn compiler_artifact_rlibs(line: &str) -> Option<(String, Vec)> { - let message: serde_json::Value = serde_json::from_str(line).ok()?; - if message.get("reason")? != "compiler-artifact" { - return None; - } - let name = message.get("target")?.get("name")?.as_str()?.to_owned(); - let rlibs = message - .get("filenames") - .and_then(serde_json::Value::as_array) - .into_iter() - .flatten() - .filter_map(serde_json::Value::as_str) - .filter(|filename| { - Utf8Path::new(filename) - .extension() - .is_some_and(|extension| extension.eq_ignore_ascii_case("rlib")) - }) - .map(Utf8PathBuf::from) - .collect(); - Some((name, rlibs)) -} - -/// Extract the parent directories of every rlib in one Cargo JSON message. -fn rlib_parents_in_message(line: &str) -> Vec { - compiler_artifact_rlibs(line) - .map(|(_name, rlibs)| { - rlibs + let mut args = vec![ + String::from("--edition=2024"), + String::from("--crate-type=bin"), + String::from("--emit=metadata"), + manifest_dir().join(source).to_string_lossy().into_owned(), + String::from("--extern"), + format!("test_support={}", self.rlib.display()), + ]; + args.extend( + self.deps_dirs .iter() - .filter_map(|rlib| rlib.parent().map(Utf8Path::to_path_buf)) - .collect() - }) - .unwrap_or_default() -} + .flat_map(|dir| [String::from("-L"), format!("dependency={}", dir.display())]), + ); + args.push(String::from("-o")); + args.push( + output_dir + .path() + .join("stub-env-ui.rmeta") + .to_string_lossy() + .into_owned(), + ); -/// Extract the `test_support` rlib path from one Cargo JSON message, if any. -fn test_support_rlib_in_message(line: &str) -> Option { - let (name, rlibs) = compiler_artifact_rlibs(line)?; - (name == "test_support") - .then(|| rlibs.into_iter().next_back()) - .flatten() + let response = rustc_response_file::write(output_dir.path(), "stub-env-ui.args", &args)?; + // `output_dir` owns the response file and stays in scope across the + // call below, so the file still exists when `rustc` opens it at spawn. + Command::new(rustc()).arg(response).output() + } } -fn manifest_dir() -> Utf8PathBuf { - Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR")) +fn manifest_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) } #[expect( @@ -235,52 +218,6 @@ fn stderr(output: &Output) -> String { String::from_utf8_lossy(&output.stderr).into_owned() } -/// A synthetic Cargo message with two rlibs in different directories, -/// mirroring a split `build.build-dir` layout. -const SPLIT_LAYOUT_MESSAGE: &str = r#"{"reason":"compiler-artifact","target":{"name":"anyhow"},"filenames":["/build/debug/deps/libanyhow-1.rlib","/target/debug/libanyhow-1.rlib"]}"#; - -#[rstest] -fn parser_collects_every_rlib_directory_from_a_message() { - let parents = rlib_parents_in_message(SPLIT_LAYOUT_MESSAGE); - assert_eq!( - parents, - vec![ - Utf8PathBuf::from("/build/debug/deps"), - Utf8PathBuf::from("/target/debug"), - ], - "both rlib directories should be collected in message order" - ); -} - -#[rstest] -#[case::malformed_json("not json at all")] -#[case::other_reason(r#"{"reason":"build-script-executed","target":{"name":"anyhow"}}"#)] -#[case::missing_target(r#"{"reason":"compiler-artifact","filenames":["/a/lib.rlib"]}"#)] -fn parser_ignores_non_artifact_messages(#[case] line: &str) { - assert!( - rlib_parents_in_message(line).is_empty(), - "non-artifact input should yield no directories: {line:?}" - ); - assert!( - test_support_rlib_in_message(line).is_none(), - "non-artifact input should yield no test_support rlib: {line:?}" - ); -} - -#[rstest] -fn parser_selects_the_test_support_rlib_by_target_name() { - let message = r#"{"reason":"compiler-artifact","target":{"name":"test_support"},"filenames":["/deps/libtest_support-1.rlib","/final/libtest_support.rlib"]}"#; - assert_eq!( - test_support_rlib_in_message(message), - Some(Utf8PathBuf::from("/final/libtest_support.rlib")), - "the last-listed rlib should win, matching Cargo's uplift ordering" - ); - assert!( - test_support_rlib_in_message(SPLIT_LAYOUT_MESSAGE).is_none(), - "other targets' artefacts should not be mistaken for test_support" - ); -} - /// Forcing a split `build.build-dir` must still yield a working harness: /// the dependency rlibs land apart from the uplifted `test_support` rlib, so /// the collected `-L dependency=` set has to span the split for the control diff --git a/tests/makefile_test_target.rs b/tests/makefile_test_target.rs index a3b566cbc..37a3333be 100644 --- a/tests/makefile_test_target.rs +++ b/tests/makefile_test_target.rs @@ -7,23 +7,17 @@ //! because nextest cannot execute them. //! //! They also pin the `RUSTFLAGS` contract shared by every recipe that sets the -//! variable. Setting `RUSTFLAGS` at all overrides the `[build] rustflags` -//! table in `.cargo/config.toml`, so each such recipe re-states the Polonius -//! flag, and each prepends any value the caller already exported rather than -//! discarding it. `test-nextest`, `lint-clippy`'s Clippy line, `lint-whitaker`, -//! and `typecheck` additionally add `-D warnings`; -//! `kani-full` adds only the Polonius flag, because Kani compiles third-party -//! crates the workspace lint policy does not govern. The binary-build recipe -//! preserves the caller's value through a different expansion and adds -//! Polonius, but not `-D warnings`. Rustdoc and doctests inherit caller flags, -//! deny warnings, and explicitly restore Polonius like the other checked -//! recipes. +//! variable. Each such recipe adds `-D warnings` and prepends any value the +//! caller already exported rather than discarding it. `kani-full`, +//! `bench-config-load`, and the binary-build recipe deliberately set nothing: +//! Kani compiles third-party crates the workspace lint policy does not govern, +//! and neither a benchmark nor a plain binary build is a lint gate. //! -//! The `RUSTFLAGS` tests extract each assignment from the Makefile, resolve -//! the Make variables it names, and expand the result in a shell. They assert -//! on the flags that expansion yields rather than on recipe text, so they stay -//! valid when the command a recipe runs changes. A guard test fails if a -//! recipe starts setting `RUSTFLAGS` without joining the covered set. +//! The `RUSTFLAGS` tests extract each assignment from the Makefile and expand +//! it in a shell. They assert on the flags that expansion yields rather than +//! on recipe text, so they stay valid when the command a recipe runs changes. +//! A guard test fails if a recipe starts setting `RUSTFLAGS` without joining +//! the covered set. #[path = "support/makefile.rs"] mod makefile; @@ -65,9 +59,8 @@ fn behavioural_make_test_composes_the_nextest_and_doctest_passes() -> Result<()> "test-nextest should cover the workspace, found {nextest_recipe:?}" ); ensure!( - nextest_recipe - .contains(r#"RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)""#), - "test-nextest should deny warnings and enable Polonius, found {nextest_recipe:?}" + nextest_recipe.contains(r#"RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings""#), + "test-nextest should preserve inherited flags and deny warnings, found {nextest_recipe:?}" ); let doctest_recipe = @@ -81,9 +74,8 @@ fn behavioural_make_test_composes_the_nextest_and_doctest_passes() -> Result<()> "doctests cannot run under nextest, found {doctest_recipe:?}" ); ensure!( - doctest_recipe - .contains(r#"RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings $(POLONIUS_FLAGS)""#,), - "doctest should preserve inherited flags, deny warnings, and enable Polonius; found {doctest_recipe:?}" + doctest_recipe.contains(r#"RUSTFLAGS="$${RUSTFLAGS:+$$RUSTFLAGS }-D warnings""#), + "doctest should preserve inherited flags and deny warnings; found {doctest_recipe:?}" ); ensure!( doctest_recipe.contains("--workspace"), @@ -94,8 +86,6 @@ fn behavioural_make_test_composes_the_nextest_and_doctest_passes() -> Result<()> #[path = "makefile_test_target/rustflags.rs"] mod rustflags; -#[path = "makefile_test_target/rustflags_polonius_tests.rs"] -mod rustflags_polonius_tests; /// Returns every nextest profile override. fn all_profile_overrides(config: &Value) -> impl Iterator { diff --git a/tests/makefile_test_target/rustflags.rs b/tests/makefile_test_target/rustflags.rs index 3925c4706..de003354a 100644 --- a/tests/makefile_test_target/rustflags.rs +++ b/tests/makefile_test_target/rustflags.rs @@ -1,19 +1,18 @@ //! Contract model for Makefile recipes that set `RUSTFLAGS`. //! //! Each recipe line that assigns `RUSTFLAGS` is described by a -//! [`RustflagsCase`] naming its target, the substring selecting the line, and -//! the warning and inheritance contracts it must uphold. The module extracts -//! the double-quoted assignment from the recipe, resolves the -//! `$(POLONIUS_FLAGS)` Make variable, and (on Unix) expands the result in a -//! real shell — without running the recipe's command — so the tests assert -//! what Cargo would actually receive: inherited caller flags preserved, -//! Polonius enabled, and `-D warnings` applied exactly where the contract -//! says. A completeness test walks the Makefile and fails when any +//! [`RustflagsCase`] naming its target and the substring selecting the line. +//! The module extracts the double-quoted assignment from the recipe and (on +//! Unix) expands it in a real shell — without running the recipe's command — +//! so the tests assert what Cargo would actually receive: inherited caller +//! flags preserved, and `-D warnings` applied. Every recipe that sets +//! `RUSTFLAGS` at all does so to deny warnings while conditionally preserving +//! an inherited value, so both contracts are asserted unconditionally; a +//! recipe needing a different policy would fail these assertions rather than +//! slip through. A completeness test walks the Makefile and fails when any //! `RUSTFLAGS`-setting line lacks a case, so new recipes join the contract or //! break the build. The parent `makefile_test_target` module supplies the -//! repository-file and recipe-lookup helpers; the sibling -//! `rustflags_polonius_tests` module covers the `POLONIUS_FLAGS` resolution -//! guard directly. +//! repository-file and recipe-lookup helpers. use super::{read_repo_file, target_recipe}; use anyhow::{Context, Result, ensure}; @@ -31,19 +30,6 @@ const CALLER_RUSTFLAGS: &str = "-C target-cpu=native"; #[cfg(unix)] const DENY_WARNINGS: &str = "-D warnings"; - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum WarningPolicy { - Deny, - Default, -} - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -enum InheritancePolicy { - Conditional, - Plain, -} - /// A recipe line that overrides `RUSTFLAGS`, and the contract it must meet. #[derive(Clone, Copy, Debug)] struct RustflagsCase { @@ -51,122 +37,69 @@ struct RustflagsCase { target: &'static str, /// Substring selecting the recipe line. line_marker: &'static str, - /// Whether the recipe adds `-D warnings`. - /// - /// Only the Unix behavioural tests read the policy fields (expansion - /// needs a shell), so non-Unix builds would otherwise flag them dead. - #[cfg_attr( - not(unix), - expect(dead_code, reason = "read only by Unix expansion tests") - )] - warning_policy: WarningPolicy, - /// How the recipe handles a caller-supplied value. - #[cfg_attr( - not(unix), - expect(dead_code, reason = "read only by Unix expansion tests") - )] - inheritance_policy: InheritancePolicy, } -/// Every `RUSTFLAGS`-setting recipe line under contract. -/// -/// The cases are inline literals rather than constructor helpers so the file -/// stays within the repository's 400-line module cap while covering every -/// recipe that assigns `RUSTFLAGS`. -const RUSTFLAGS_CASES: [RustflagsCase; 11] = [ - RustflagsCase { - target: "test-nextest", - line_marker: "nextest run", - warning_policy: WarningPolicy::Deny, - inheritance_policy: InheritancePolicy::Conditional, - }, - RustflagsCase { - target: "doctest", - line_marker: "--doc", - warning_policy: WarningPolicy::Deny, - inheritance_policy: InheritancePolicy::Conditional, - }, - RustflagsCase { - target: "target/%/$(APP)", - line_marker: "build", - warning_policy: WarningPolicy::Default, - inheritance_policy: InheritancePolicy::Plain, - }, - RustflagsCase { - target: "lint-clippy", - line_marker: "doc --workspace", - warning_policy: WarningPolicy::Deny, - inheritance_policy: InheritancePolicy::Conditional, - }, - RustflagsCase { - target: "lint-clippy", - line_marker: "clippy", - warning_policy: WarningPolicy::Deny, - inheritance_policy: InheritancePolicy::Conditional, - }, - RustflagsCase { - target: "lint-whitaker", - line_marker: "$(WHITAKER)", - warning_policy: WarningPolicy::Deny, - inheritance_policy: InheritancePolicy::Conditional, - }, - RustflagsCase { - target: "lint-whitaker", - line_marker: "cd test_support", - warning_policy: WarningPolicy::Deny, - inheritance_policy: InheritancePolicy::Conditional, - }, - RustflagsCase { - target: "typecheck", - line_marker: "check", - warning_policy: WarningPolicy::Deny, - inheritance_policy: InheritancePolicy::Conditional, - }, - RustflagsCase { - target: "kani-full", - line_marker: "$(KANI)", - warning_policy: WarningPolicy::Default, - inheritance_policy: InheritancePolicy::Conditional, - }, - RustflagsCase { - target: "doc-coverage", - line_marker: "RUSTDOCFLAGS", - warning_policy: WarningPolicy::Default, - inheritance_policy: InheritancePolicy::Conditional, - }, - RustflagsCase { - target: "bench-config-load", - line_marker: "bench --bench config_load_cached_merge", - warning_policy: WarningPolicy::Default, - inheritance_policy: InheritancePolicy::Conditional, - }, -]; -/// Resolves `POLONIUS_FLAGS`, rejecting a missing or empty definition. -/// -/// Every assertion built on the resolved value uses `contains`, which an -/// empty string satisfies vacuously, so an empty definition would silently -/// void the Polonius contract rather than fail it. -pub(crate) fn polonius_flags(makefile: &str) -> Result { - let value = make_variable(makefile, "POLONIUS_FLAGS") - .context("Makefile should define POLONIUS_FLAGS")?; - ensure!( - !value.is_empty(), - "POLONIUS_FLAGS should not be empty; the contract cannot assert a vacuous flag" - ); - Ok(value) -} +impl RustflagsCase { + const fn test_nextest() -> Self { + Self { + target: "test-nextest", + line_marker: "nextest run", + } + } + + const fn doctest() -> Self { + Self { + target: "doctest", + line_marker: "--doc", + } + } -/// Returns the value of a simple `NAME ?= value` or `NAME = value` variable. -fn make_variable(contents: &str, name: &str) -> Option { - contents.lines().find_map(|line| { - let rest = line.strip_prefix(name)?; - let value = rest - .strip_prefix(" ?= ") - .or_else(|| rest.strip_prefix(" = "))?; - Some(value.trim().to_owned()) - }) + const fn lint_rustdoc() -> Self { + Self { + target: "lint-clippy", + line_marker: "doc --workspace", + } + } + + const fn lint_clippy() -> Self { + Self { + target: "lint-clippy", + line_marker: "clippy", + } + } + + const fn lint_whitaker() -> Self { + Self { + target: "lint-whitaker", + line_marker: "$(WHITAKER)", + } + } + + const fn lint_whitaker_test_support() -> Self { + Self { + target: "lint-whitaker", + line_marker: "cd test_support", + } + } + + const fn typecheck() -> Self { + Self { + target: "typecheck", + line_marker: "check", + } + } } +/// Every `RUSTFLAGS`-setting recipe line under contract. +const RUSTFLAGS_CASES: [RustflagsCase; 7] = [ + RustflagsCase::test_nextest(), + RustflagsCase::doctest(), + RustflagsCase::lint_rustdoc(), + RustflagsCase::lint_clippy(), + RustflagsCase::lint_whitaker(), + RustflagsCase::lint_whitaker_test_support(), + RustflagsCase::typecheck(), +]; /// Extracts the double-quoted `RUSTFLAGS` assignment from a recipe line. /// /// `RUSTDOCFLAGS="…"` does not contain `RUSTFLAGS="`, so a line setting both @@ -197,8 +130,8 @@ fn recipe_line(makefile: &str, case: RustflagsCase) -> Result { /// Returns `case`'s `RUSTFLAGS` assignment as a shell expression. /// -/// Make variable references are resolved, and Make's `$$` escape is reduced to -/// the single `$` the shell receives. +/// Make's `$$` escape is reduced to the single `$` the shell receives. No +/// assignment may name a Make variable, since this test cannot resolve one. fn shell_expression(makefile: &str, case: RustflagsCase) -> Result { let line = recipe_line(makefile, case)?; let assignment = rustflags_assignment(&line).with_context(|| { @@ -207,10 +140,7 @@ fn shell_expression(makefile: &str, case: RustflagsCase) -> Result { case.target ) })?; - let polonius = polonius_flags(makefile)?; - let resolved = assignment - .replace("$(POLONIUS_FLAGS)", &polonius) - .replace("$$", "$"); + let resolved = assignment.replace("$$", "$"); ensure!( !resolved.contains("$("), "{}: RUSTFLAGS assignment {resolved:?} names a Make variable this test cannot resolve", @@ -219,12 +149,12 @@ fn shell_expression(makefile: &str, case: RustflagsCase) -> Result { Ok(resolved) } -/// Expands `expression` in a shell with Make's exported flag variables. +/// Expands `expression` in a shell, exporting `inherited` as `RUSTFLAGS`. /// /// Only the assignment is expanded; the command the recipe would run is never /// executed, so no test here invokes Cargo, Kani, nextest, or Dylint. #[cfg(unix)] -fn expand(expression: &str, inherited: Option<&str>, polonius: &str) -> Result { +fn expand(expression: &str, inherited: Option<&str>) -> Result { ensure!( !expression.contains('"') && !expression.contains('`'), "the expansion helper cannot safely embed {expression:?}" @@ -233,8 +163,7 @@ fn expand(expression: &str, inherited: Option<&str>, polonius: &str) -> Result Result<()> { let makefile = read_repo_file(Utf8Path::new("Makefile"))?; - let polonius = polonius_flags(&makefile)?; for case in RUSTFLAGS_CASES { - let expanded = expand( - &shell_expression(&makefile, case)?, - Some(CALLER_RUSTFLAGS), - &polonius, - )?; + let expanded = expand(&shell_expression(&makefile, case)?, Some(CALLER_RUSTFLAGS))?; ensure!( expanded.contains(CALLER_RUSTFLAGS), @@ -302,20 +220,10 @@ fn behavioural_rustflags_recipes_preserve_inherited_flags() -> Result<()> { case.target ); ensure!( - expanded.contains(&polonius), - "{} should enable Polonius with {polonius}, expanded to {expanded:?}", + expanded.contains(DENY_WARNINGS), + "{} should deny warnings, expanded to {expanded:?}", case.target ); - ensure!( - expanded.contains(DENY_WARNINGS) == (case.warning_policy == WarningPolicy::Deny), - "{} should {}deny warnings, expanded to {expanded:?}", - case.target, - if case.warning_policy == WarningPolicy::Deny { - "" - } else { - "not " - } - ); } Ok(()) } @@ -324,32 +232,23 @@ fn behavioural_rustflags_recipes_preserve_inherited_flags() -> Result<()> { #[test] fn behavioural_rustflags_recipes_are_well_formed_without_inherited_flags() -> Result<()> { let makefile = read_repo_file(Utf8Path::new("Makefile"))?; - let polonius = polonius_flags(&makefile)?; for case in RUSTFLAGS_CASES { let expression = shell_expression(&makefile, case)?; - let expanded = expand(&expression, None, &polonius)?; + let expanded = expand(&expression, None)?; - ensure!( - expanded.contains(&polonius), - "{} should enable Polonius with {polonius}, expanded to {expanded:?}", - case.target - ); ensure!( !expanded.contains(CALLER_RUSTFLAGS), "{} should not invent flags the caller never set, expanded to {expanded:?}", case.target ); // `${VAR:+VAR }` contributes its separator only alongside a value, so - // an unset RUSTFLAGS must not leave a leading space. Recipes spelling - // `${VAR-}` tolerate one, so the case declares which contract applies. - if case.inheritance_policy == InheritancePolicy::Conditional { - ensure!( - !expanded.starts_with(' '), - "{} should not emit a leading separator when RUSTFLAGS is unset, \ - expanded to {expanded:?}", - case.target - ); - } + // an unset RUSTFLAGS must not leave a leading space. + ensure!( + !expanded.starts_with(' '), + "{} should not emit a leading separator when RUSTFLAGS is unset, \ + expanded to {expanded:?}", + case.target + ); } Ok(()) } diff --git a/tests/makefile_test_target/rustflags_polonius_tests.rs b/tests/makefile_test_target/rustflags_polonius_tests.rs deleted file mode 100644 index 516d5c5db..000000000 --- a/tests/makefile_test_target/rustflags_polonius_tests.rs +++ /dev/null @@ -1,98 +0,0 @@ -//! Direct coverage for the `POLONIUS_FLAGS` resolution guard. -//! -//! The `RUSTFLAGS` contract assertions all use `contains`, which an empty -//! string satisfies vacuously, so `rustflags::polonius_flags` must reject a -//! missing or empty definition rather than hand back a value that voids the -//! Polonius contract. These tests exercise the guard against synthetic -//! Makefile text — the behavioural tests only ever see the real, non-empty -//! definition — and a bounded property test pins the acceptance invariant: -//! resolution succeeds exactly when a definition exists whose value is -//! non-empty after trimming. - -use super::rustflags::polonius_flags; -use anyhow::{Result, ensure}; -use proptest::prelude::*; -use rstest::rstest; - -#[rstest] -#[case::defined_with_flag("POLONIUS_FLAGS ?= -Zpolonius=next\n", Some("-Zpolonius=next"))] -#[case::defined_with_plain_assignment( - "POLONIUS_FLAGS = -Zpolonius=next\n", - Some("-Zpolonius=next") -)] -#[case::missing_definition("OTHER_VAR ?= value\n", None)] -#[case::empty_value("POLONIUS_FLAGS ?= \n", None)] -#[case::whitespace_only_value("POLONIUS_FLAGS ?= \t \n", None)] -fn polonius_flags_accepts_only_non_empty_definitions( - #[case] makefile: &str, - #[case] expected: Option<&str>, -) -> Result<()> { - let outcome = polonius_flags(makefile); - if let Some(value) = expected { - let resolved = outcome?; - ensure!( - resolved == value, - "expected {value:?} from {makefile:?}; got {resolved:?}" - ); - } else { - let error = outcome.err().map(|error| error.to_string()); - let message = error.as_deref().unwrap_or("(unexpectedly succeeded)"); - ensure!( - message.contains("POLONIUS_FLAGS"), - "rejection for {makefile:?} should name the variable; got {message:?}" - ); - } - Ok(()) -} - -#[rstest] -#[case::missing_variable("A ?= b\n", "should define POLONIUS_FLAGS")] -#[case::empty_value("POLONIUS_FLAGS ?= \n", "should not be empty")] -fn polonius_flags_names_the_rejection_cause(#[case] makefile: &str, #[case] expected: &str) { - let error = polonius_flags(makefile).expect_err("definition should be rejected"); - assert!( - error.to_string().contains(expected), - "rejection of {makefile:?} should report {expected:?}; got {error:?}" - ); -} - -/// Strategy for the `POLONIUS_FLAGS` value slot: absent, whitespace-only, or -/// a non-empty flag-like token that may be padded with whitespace, so the -/// property also pins the trimming the resolver owes its callers. -fn arb_polonius_value() -> impl Strategy> { - prop_oneof![ - Just(None), - "[ \t]{0,4}".prop_map(Some), - ("[ \t]{0,4}", "[-A-Za-z0-9=+.]{1,24}", "[ \t]{0,4}") - .prop_map(|(prefix, flag, suffix)| Some(format!("{prefix}{flag}{suffix}"))), - ] -} - -proptest! { - /// Resolution succeeds exactly when a definition exists whose value is - /// non-empty after trimming, and the resolved text is the trimmed value. - #[test] - fn prop_polonius_flags_acceptance_matches_the_definition( - value in arb_polonius_value(), - filler in "[A-Z_]{1,8}", - ) { - let makefile = value.as_ref().map_or_else( - || format!("{filler} ?= x\n"), - |text| format!("{filler} ?= x\nPOLONIUS_FLAGS ?= {text}\n"), - ); - let outcome = polonius_flags(&makefile); - let expected = value - .as_deref() - .map(str::trim) - .filter(|trimmed| !trimmed.is_empty()); - if let Some(trimmed) = expected { - let resolved = outcome.map_err(|error| TestCaseError::fail(error.to_string()))?; - prop_assert_eq!(resolved, trimmed, "resolved value should be the trimmed text"); - } else { - prop_assert!( - outcome.is_err(), - "missing or blank definitions must be rejected: {makefile:?}" - ); - } - } -} diff --git a/tests/polonius_toolchain_contract.rs b/tests/polonius_toolchain_contract.rs index f70e0f8e1..a8c9b9cfa 100644 --- a/tests/polonius_toolchain_contract.rs +++ b/tests/polonius_toolchain_contract.rs @@ -1,14 +1,13 @@ //! Contract tests pinning the Polonius toolchain plumbing. //! -//! The tree only borrow-checks under `-Zpolonius=next` on the dated nightly -//! pinned in `rust-toolchain.toml` (see ADR-006 and docs/polonius.md). An -//! inherited `RUSTFLAGS` environment variable overrides the -//! `.cargo/config.toml` `build.rustflags` table, so Makefile recipes that -//! compile borrow-checked targets must restate the flag, while workflows must -//! pass it through the shared action's `with.rustflags` input and reject a -//! job-level `env.RUSTFLAGS` override. These tests fail when any layer drops -//! the required policy, so a regression cannot reach CI as a confusing -//! borrow-check error. +//! The tree only borrow-checks under the Polonius alpha analysis (see ADR-006 +//! and docs/polonius.md). Nightly toolchains dated 2026-08-04 and later enable +//! Polonius by default, so the requirement is carried entirely by the dated +//! pin in `rust-toolchain.toml`: no build configuration passes a `-Zpolonius` +//! directive any more, and reintroducing one would pin the tree to an +//! interface that is on its way out. These tests hold both halves — the pin is +//! recent enough to enable Polonius, and nothing restates the retired flag — +//! plus the workflow contract that keeps CI on the same pinned channel. #[path = "support/makefile.rs"] mod makefile; @@ -17,23 +16,44 @@ pub mod shared_actions; use anyhow::{Context, Result, ensure}; use camino::Utf8Path; -use makefile::{read_repo_file, target_recipe}; +use makefile::{read_repo_file, repo_root}; use rstest::rstest; use serde_yaml::Value as YamlValue; +use std::io::ErrorKind; use toml::Value as TomlValue; -const POLONIUS_FLAG: &str = "-Zpolonius=next"; -const POLONIUS_VAR: &str = "$(POLONIUS_FLAGS)"; +/// The retired directive that must not reappear in build configuration. +const POLONIUS_FLAG: &str = "-Zpolonius"; +/// The first nightly on which Polonius is the default borrow-check analysis. +const POLONIUS_DEFAULT_SINCE: &str = "2026-08-04"; const SETUP_RUST_ACTION: &str = "leynos/shared-actions/.github/actions/setup-rust"; const RUST_BUILD_RELEASE_ACTION: &str = "leynos/shared-actions/.github/actions/rust-build-release"; -const WARNINGS_POLONIUS_RUSTFLAGS: &str = "-D warnings -Zpolonius=next"; +const DENY_WARNINGS_RUSTFLAGS: &str = "-D warnings"; + +/// Build-configuration surfaces that could reintroduce the retired directive. +/// +/// `.cargo/config.toml` is listed even though it no longer exists: it is the +/// path Cargo auto-discovers, so recreating it to carry the flag is the most +/// likely regression and a missing file is simply skipped. +const BUILD_CONFIGURATION_FILES: [&str; 8] = [ + "Makefile", + ".cargo/config.toml", + "tools/dev-fast/config.toml", + "scripts/dev-fast-common.sh", + ".github/workflows/ci.yml", + ".github/workflows/netsukefile-test.yml", + ".github/workflows/coverage-main.yml", + ".github/workflows/build-and-package.yml", +]; /// Describes one workflow's shared-action and toolchain contract. struct WorkflowExpectation { path: &'static str, job: &'static str, action: &'static str, - rustflags: &'static str, + /// The `with.rustflags` input the job must pass, or `None` when it must + /// pass none at all and inherit the action's default. + rustflags: Option<&'static str>, pins_toolchain_env: bool, } @@ -41,35 +61,35 @@ const CI_WORKFLOW: WorkflowExpectation = WorkflowExpectation { path: ".github/workflows/ci.yml", job: "build-test", action: SETUP_RUST_ACTION, - rustflags: WARNINGS_POLONIUS_RUSTFLAGS, + rustflags: Some(DENY_WARNINGS_RUSTFLAGS), pins_toolchain_env: true, }; const CI_WINDOWS_WORKFLOW: WorkflowExpectation = WorkflowExpectation { path: ".github/workflows/ci.yml", job: "build-test-windows", action: SETUP_RUST_ACTION, - rustflags: WARNINGS_POLONIUS_RUSTFLAGS, + rustflags: Some(DENY_WARNINGS_RUSTFLAGS), pins_toolchain_env: true, }; const NETSUKEFILE_WORKFLOW: WorkflowExpectation = WorkflowExpectation { path: ".github/workflows/netsukefile-test.yml", job: "netsukefile", action: SETUP_RUST_ACTION, - rustflags: POLONIUS_FLAG, + rustflags: None, pins_toolchain_env: true, }; const COVERAGE_WORKFLOW: WorkflowExpectation = WorkflowExpectation { path: ".github/workflows/coverage-main.yml", job: "coverage-upload", action: SETUP_RUST_ACTION, - rustflags: WARNINGS_POLONIUS_RUSTFLAGS, + rustflags: Some(DENY_WARNINGS_RUSTFLAGS), pins_toolchain_env: false, }; const PACKAGING_WORKFLOW: WorkflowExpectation = WorkflowExpectation { path: ".github/workflows/build-and-package.yml", job: "build", action: RUST_BUILD_RELEASE_ACTION, - rustflags: POLONIUS_FLAG, + rustflags: None, pins_toolchain_env: false, }; @@ -133,43 +153,40 @@ fn yaml_str<'a>(value: &'a YamlValue, keys: &[&str]) -> Option<&'a str> { } #[test] -fn rust_toolchain_pins_dated_nightly() -> Result<()> { +fn rust_toolchain_pins_a_nightly_that_enables_polonius_by_default() -> Result<()> { let channel = pinned_toolchain()?; + let date = channel + .strip_prefix("nightly-") + .context("rust-toolchain.toml should pin a dated nightly")?; + // ISO-8601 dates sort lexicographically, so a string comparison is a date + // comparison here and needs no calendar parsing. ensure!( - channel.starts_with("nightly-20"), - "rust-toolchain.toml should pin a dated nightly, found {channel:?}" + date >= POLONIUS_DEFAULT_SINCE, + "the pinned nightly {channel:?} predates {POLONIUS_DEFAULT_SINCE}, \ + when Polonius became the default borrow-check analysis" ); Ok(()) } #[test] -fn cargo_config_enables_polonius_by_default() -> Result<()> { - let config: TomlValue = read_repo_file(Utf8Path::new(".cargo/config.toml"))? - .parse() - .context("parse .cargo/config.toml")?; - let rustflags = config - .get("build") - .and_then(|build| build.get("rustflags")) - .and_then(TomlValue::as_array) - .context(".cargo/config.toml should declare build.rustflags")?; - ensure!( - rustflags - .iter() - .any(|flag| flag.as_str() == Some(POLONIUS_FLAG)), - "build.rustflags should enable {POLONIUS_FLAG}, found {rustflags:?}" - ); - Ok(()) -} - -#[test] -fn makefile_declares_the_polonius_flags_variable() -> Result<()> { - let makefile = read_repo_file(Utf8Path::new("Makefile"))?; - ensure!( - makefile - .lines() - .any(|line| line.trim() == format!("POLONIUS_FLAGS ?= {POLONIUS_FLAG}")), - "the Makefile should default POLONIUS_FLAGS to {POLONIUS_FLAG}" - ); +fn build_configuration_does_not_restate_the_retired_polonius_flag() -> Result<()> { + let root = repo_root()?; + for path in BUILD_CONFIGURATION_FILES { + let contents = match root.read_to_string(path) { + Ok(contents) => contents, + // `.cargo/config.toml` is intentionally absent but remains under + // contract because recreating it is the likeliest flag regression. + Err(error) if path == ".cargo/config.toml" && error.kind() == ErrorKind::NotFound => { + continue; + } + Err(error) => return Err(error).with_context(|| format!("read {path}")), + }; + ensure!( + !contents.contains(POLONIUS_FLAG), + "{path} passes {POLONIUS_FLAG}; the pinned nightly enables Polonius by default \ + and the directive is being retired" + ); + } Ok(()) } @@ -179,7 +196,7 @@ fn makefile_declares_the_polonius_flags_variable() -> Result<()> { #[case::netsukefile(NETSUKEFILE_WORKFLOW)] #[case::coverage(COVERAGE_WORKFLOW)] #[case::packaging(PACKAGING_WORKFLOW)] -fn workflows_pass_polonius_rustflags_to_shared_actions( +fn workflows_agree_with_the_pinned_toolchain( #[case] expectation: WorkflowExpectation, ) -> Result<()> { let WorkflowExpectation { @@ -206,8 +223,7 @@ fn workflows_pass_polonius_rustflags_to_shared_actions( .iter() .find(|step| yaml_str(step, &["uses"]) == Some(expected_action.as_str())) .with_context(|| format!("{path} job {job} should use {expected_action}"))?; - let rustflags = yaml_str(shared_action, &["with", "rustflags"]) - .with_context(|| format!("{path} {expected_action} should pass rustflags"))?; + let rustflags = yaml_str(shared_action, &["with", "rustflags"]); ensure!( rustflags == expected_rustflags, "{path} {expected_action} passes {rustflags:?}, expected {expected_rustflags:?}" @@ -230,38 +246,6 @@ fn workflows_pass_polonius_rustflags_to_shared_actions( Ok(()) } -#[rstest] -#[case::test_nextest("test-nextest", true)] -#[case::doctest("doctest", true)] -#[case::typecheck("typecheck", true)] -#[case::lint_clippy("lint-clippy", true)] -#[case::lint_whitaker("lint-whitaker", true)] -#[case::build_binary("target/%/$(APP)", true)] -fn rustflags_setting_recipes_apply_polonius_policy( - #[case] target: &str, - #[case] expects_polonius: bool, -) -> Result<()> { - let makefile = read_repo_file(Utf8Path::new("Makefile"))?; - let recipe = target_recipe(&makefile, target) - .with_context(|| format!("the Makefile should declare a {target} target"))?; - let rustflags_lines: Vec<&str> = recipe - .lines() - .filter(|line| line.contains("RUSTFLAGS=")) - .collect(); - ensure!( - !rustflags_lines.is_empty(), - "{target} should set RUSTFLAGS, found {recipe:?}" - ); - for line in rustflags_lines { - let contains_polonius = line.contains(POLONIUS_VAR) || line.contains(POLONIUS_FLAG); - ensure!( - contains_polonius == expects_polonius, - "{target} has the wrong Polonius policy in {line:?}" - ); - } - Ok(()) -} - #[test] fn coverage_workflow_setup_matches_the_pinned_toolchain() -> Result<()> { let path = ".github/workflows/coverage-main.yml"; diff --git a/tests/sha2_migration_guard_tests.rs b/tests/sha2_migration_guard_tests.rs index 1718edcdb..183d5625d 100644 --- a/tests/sha2_migration_guard_tests.rs +++ b/tests/sha2_migration_guard_tests.rs @@ -29,10 +29,11 @@ //! //! This replaced a `trybuild` harness during the Polonius migration. Trybuild //! always builds the host crate as a fixture dependency while discarding -//! workspace `build.rustflags`, so it rebuilt `netsuke` without -//! `-Zpolonius=next`; see the "Harness consequences" section of -//! `docs/polonius.md`. These probes need no subprocess, no scratch project, and -//! no toolchain-sensitive `.stderr` snapshot. +//! workspace `build.rustflags`, so while Polonius was flag-gated it rebuilt +//! `netsuke` without the analysis; see the "Harness consequences" section of +//! `docs/polonius.md`. The pinned nightly now enables Polonius by default, but +//! these probes still need no subprocess, no scratch project, and no +//! toolchain-sensitive `.stderr` snapshot. /// Define a compile-time probe module for one trait bound. /// diff --git a/tests/support/cargo_artifacts.rs b/tests/support/cargo_artifacts.rs new file mode 100644 index 000000000..40dfaa7fd --- /dev/null +++ b/tests/support/cargo_artifacts.rs @@ -0,0 +1,240 @@ +//! Parses Cargo compiler-artifact messages for direct-rustc UI harnesses. +//! +//! Cargo 1.99 gives every crate its own artefact directory, so a harness that +//! invokes `rustc` directly must derive its dependency search paths from Cargo +//! JSON rather than assume a shared `target//deps` directory. +//! +//! Scope: this module owns parsing and selecting loadable artefact paths from +//! `compiler-artifact` messages. It does not build Cargo packages, deduplicate +//! directories, assemble rustc arguments, or spawn rustc; each harness retains +//! those responsibilities. +//! +//! Reuse policy: include this module only from `tests/*.rs` direct-rustc UI +//! harnesses. Ordinary Cargo-driven tests neither parse these messages nor need +//! Cargo's private artefact layout. + +use std::path::{Path, PathBuf}; + +/// Return loadable artefact parent directories from one Cargo JSON message. +/// +/// The directories preserve Cargo's message order. The caller deduplicates +/// across messages because each direct-rustc harness owns its search-path +/// ordering policy. +pub fn dependency_dirs_in_message(line: &str) -> Vec { + compiler_artifact_paths(line) + .map(|(_target, paths)| { + paths + .into_iter() + .filter(|path| is_dependency_artefact(path)) + .filter_map(|path| path.parent().map(Path::to_path_buf)) + .collect() + }) + .unwrap_or_default() +} + +/// Return the preferred metadata path for `target_name` from one Cargo message. +/// +/// Rustc type-checks the UI fixtures with `--emit=metadata`. Cargo builds with +/// `-Zembed-metadata=no`, so the matching `.rmeta` is preferred and the rlib +/// remains a compatibility fallback for older Cargo layouts. +pub fn library_path_in_message(line: &str, target_name: &str) -> Option { + let (name, paths) = compiler_artifact_paths(line)?; + (name == target_name) + .then_some(paths) + .and_then(|artifact_paths| { + last_with_extension(&artifact_paths, "rmeta") + .or_else(|| last_with_extension(&artifact_paths, "rlib")) + }) +} + +/// Return the target name and filenames in one compiler-artifact message. +fn compiler_artifact_paths(line: &str) -> Option<(String, Vec)> { + let message: serde_json::Value = serde_json::from_str(line).ok()?; + if message.get("reason")?.as_str()? != "compiler-artifact" { + return None; + } + let name = message.get("target")?.get("name")?.as_str()?.to_owned(); + let paths = message + .get("filenames") + .and_then(serde_json::Value::as_array)? + .iter() + .filter_map(serde_json::Value::as_str) + .map(PathBuf::from) + .collect(); + Some((name, paths)) +} + +/// Return whether `path` names an artefact rustc can load through `-L`. +fn is_dependency_artefact(path: &Path) -> bool { + path.extension().is_some_and(|extension| { + extension.eq_ignore_ascii_case("rmeta") + || extension.eq_ignore_ascii_case("rlib") + || extension.eq_ignore_ascii_case(std::env::consts::DLL_EXTENSION) + }) +} + +/// Return the last path with `extension`, retaining Cargo's uplift ordering. +fn last_with_extension(paths: &[PathBuf], extension: &str) -> Option { + paths + .iter() + .rfind(|path| { + path.extension() + .is_some_and(|candidate| candidate.eq_ignore_ascii_case(extension)) + }) + .cloned() +} + +#[cfg(test)] +mod tests { + //! Property and example tests for the Cargo artefact parser. + + use super::{compiler_artifact_paths, dependency_dirs_in_message, library_path_in_message}; + use proptest::prelude::*; + use rstest::rstest; + use std::path::{Path, PathBuf}; + + /// Generate a newline-free Cargo artefact path and whether rustc can load it. + fn artefact_path() -> impl Strategy { + ( + "[\\p{L}\\p{N} _-]{1,12}", + "[\\p{L}\\p{N}_-]{1,12}", + prop_oneof![ + Just("rmeta"), + Just("rlib"), + Just(std::env::consts::DLL_EXTENSION), + Just("txt") + ], + ) + .prop_map(|(directory, name, extension)| { + let path = format!("/tmp/{directory}/lib{name}.{extension}"); + let loadable = matches!(extension, "rmeta" | "rlib") + || extension.eq_ignore_ascii_case(std::env::consts::DLL_EXTENSION); + (path, loadable) + }) + } + + /// Generate an ordered path whose extension can select a library artefact. + fn library_artefact_path() -> impl Strategy { + ( + "[a-z]{1,12}", + prop_oneof![Just("rmeta"), Just("rlib"), Just("txt"), Just("dylib")], + ) + .prop_map(|(name, extension)| format!("/tmp/build/lib{name}.{extension}")) + } + + /// Derive the metadata-first library selection from an ordered path list. + fn expected_library_path(paths: &[String]) -> Option { + ["rmeta", "rlib"].into_iter().find_map(|extension| { + paths + .iter() + .rfind(|path| { + Path::new(path) + .extension() + .is_some_and(|value| value == extension) + }) + .map(PathBuf::from) + }) + } + + proptest! { + /// Preserve every ordered parent directory rustc needs across varied paths. + #[test] + fn parser_preserves_loadable_artefact_parent_order( + artefacts in proptest::collection::vec(artefact_path(), 0..16), + ) { + let filenames: Vec<&str> = artefacts.iter().map(|(path, _)| path.as_str()).collect(); + let message = serde_json::json!({ + "reason": "compiler-artifact", + "target": {"name": "fixture"}, + "filenames": filenames, + }); + let expected: Vec = artefacts + .iter() + .filter(|(_, loadable)| *loadable) + .filter_map(|(path, _)| Path::new(path).parent().map(Path::to_path_buf)) + .collect(); + + prop_assert_eq!(dependency_dirs_in_message(&message.to_string()), expected); + } + + /// Select the last metadata artefact, or the last library artefact when needed. + #[test] + fn library_parser_prefers_last_metadata_then_library_and_rejects_mismatches( + artefacts in proptest::collection::vec(library_artefact_path(), 0..16), + ) { + let message = serde_json::json!({ + "reason": "compiler-artifact", + "target": {"name": "fixture"}, + "filenames": &artefacts, + }); + let expected = expected_library_path(&artefacts); + + prop_assert_eq!(library_path_in_message(&message.to_string(), "fixture"), expected); + prop_assert_eq!(library_path_in_message(&message.to_string(), "other"), None); + } + } + + /// Prefer metadata and fall back to the library artefact. + #[rstest] + #[case( + r#"{"reason":"compiler-artifact","target":{"name":"fixture"},"filenames":["/final/libfixture.rlib","/build/libfixture.rmeta"]}"#, + "/build/libfixture.rmeta", + "metadata" + )] + #[case( + r#"{"reason":"compiler-artifact","target":{"name":"fixture"},"filenames":["/final/libfixture.rlib"]}"#, + "/final/libfixture.rlib", + "library fallback" + )] + #[case( + r#"{"reason":"compiler-artifact","target":{"name":"fixture"},"filenames":["/first/libfixture.rmeta","/first/libfixture.rlib","/last/libfixture.rmeta","/last/libfixture.rlib"]}"#, + "/last/libfixture.rmeta", + "last metadata" + )] + #[case( + r#"{"reason":"compiler-artifact","target":{"name":"fixture"},"filenames":["/first/libfixture.rlib","/last/libfixture.rlib"]}"#, + "/last/libfixture.rlib", + "last library fallback" + )] + fn parser_prefers_metadata_then_falls_back_to_library( + #[case] message: &str, + #[case] expected: &str, + #[case] selection: &str, + ) { + assert_eq!( + library_path_in_message(message, "fixture"), + Some(PathBuf::from(expected)), + "{selection} should be selected when Cargo reports it" + ); + } + + /// Reject malformed and irrelevant messages before reading artefact paths. + #[rstest] + #[case("not JSON")] + #[case(r#"{"reason":"build-script-executed"}"#)] + #[case(r#"{"reason":"compiler-artifact","filenames":[]}"#)] + #[case(r#"{"reason":"compiler-artifact","target":{},"filenames":[]}"#)] + #[case(r#"{"reason":"compiler-artifact","target":{"name":"fixture"}}"#)] + #[case(r#"{"reason":"compiler-artifact","target":{"name":"fixture"},"filenames":{}}"#)] + fn compiler_artifact_parser_rejects_invalid_messages(#[case] message: &str) { + assert_eq!(compiler_artifact_paths(message), None); + } + + /// Reject malformed, irrelevant, and mismatched messages for a named library. + #[rstest] + #[case("not JSON")] + #[case(r#"{"reason":"build-script-executed"}"#)] + #[case(r#"{"reason":"compiler-artifact","filenames":[]}"#)] + #[case(r#"{"reason":"compiler-artifact","target":{},"filenames":[]}"#)] + #[case(r#"{"reason":"compiler-artifact","target":{"name":"fixture"}}"#)] + #[case(r#"{"reason":"compiler-artifact","target":{"name":"fixture"},"filenames":{}}"#)] + #[case( + r#"{"reason":"compiler-artifact","target":{"name":"fixture"},"filenames":["/build/libfixture.a","/build/libfixture.txt"]}"# + )] + #[case( + r#"{"reason":"compiler-artifact","target":{"name":"other"},"filenames":["/build/libother.rmeta"]}"# + )] + fn library_path_parser_rejects_unmatched_messages(#[case] message: &str) { + assert_eq!(library_path_in_message(message, "fixture"), None); + } +} diff --git a/tests/support/rustc_response_file.rs b/tests/support/rustc_response_file.rs new file mode 100644 index 000000000..6f4f42dfc --- /dev/null +++ b/tests/support/rustc_response_file.rs @@ -0,0 +1,237 @@ +//! Builds `rustc` response files for the direct-compile UI harnesses. +//! +//! Why this exists: Cargo 1.99 gives every crate its own artefact directory +//! rather than one shared `deps/`, so a harness that must reach every +//! dependency passes one `-L dependency=` pair per crate. The +//! split-build regression test compounds that with long, unique temporary +//! roots. On Windows the resulting `CreateProcessW` command line exceeds the +//! 32,767-character limit and the spawn fails before `rustc` runs at all, with +//! `Os { code: 206, kind: InvalidFilename }`. Every one of those directories is +//! required — dropping any of them reintroduces `E0463` — so the fix is to stop +//! putting them on the command line, not to shorten the list. +//! +//! `rustc` reads arguments from a file named `@`, one argument per line, +//! UTF-8 encoded. That moves the whole argument vector off the command line +//! and leaves the spawn well under any platform limit. +//! +//! Scope, deliberately narrow: rendering an argument vector into response-file +//! text and writing it. Nothing here spawns a process, chooses arguments, or +//! knows what a compilation needs. +//! +//! Reuse policy: include this module from a `tests/*.rs` binary that invokes +//! `rustc` directly with an argument list whose length is not bounded by the +//! source. A harness passing a fixed handful of arguments does not need it. +//! +//! Integration tests under `tests/` compile as independent crates, so there is +//! no library to share through. The module lives in a subdirectory, which Cargo +//! does not auto-discover as a test target, and each consumer includes it with +//! `#[path = "support/rustc_response_file.rs"] mod rustc_response_file;`. +//! +//! Every helper is exercised by this module's own unit tests, which run once +//! per including crate. That is what keeps a consumer using only part of the +//! surface from tripping `dead_code`. + +use std::io; +use std::path::{Path, PathBuf}; + +/// Renders `args` as response-file text: one argument per line. +/// +/// `rustc` splits a response file on line boundaries and performs no quoting or +/// escaping, so an argument containing a newline would silently become two +/// arguments. That cannot arise from the paths these harnesses pass, but it +/// would corrupt the compilation invisibly if it ever did, so it is rejected +/// here rather than diagnosed later as a baffling `rustc` error. +/// +/// # Errors +/// +/// Returns an error when any argument contains a newline. +/// +/// # Examples +/// +/// ``` +/// let text = render(&["--edition=2024".to_owned(), "-L".to_owned()]) +/// .expect("newline-free arguments"); +/// assert_eq!(text, "--edition=2024\n-L\n"); +/// ``` +pub fn render(args: &[String]) -> io::Result { + if let Some(bad) = args.iter().find(|arg| arg.contains('\n')) { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + format!("a response-file argument cannot contain a newline: {bad:?}"), + )); + } + let mut text = String::new(); + for arg in args { + text.push_str(arg); + text.push('\n'); + } + Ok(text) +} + +/// Writes `args` to `/` and returns the `@path` argument. +/// +/// The returned string is the single argument to hand `rustc`. `dir` is +/// normally the harness's existing output `TempDir`, which must outlive the +/// `rustc` invocation: the response file is read at spawn time, so dropping the +/// directory first would delete the file before `rustc` opens it. +/// +/// # Errors +/// +/// Returns an error when an argument contains a newline, when the path is not +/// valid UTF-8 (`rustc` requires a UTF-8 response file path), or when the write +/// fails. +/// +/// # Examples +/// +/// ```no_run +/// let dir = tempfile::tempdir().expect("temp dir"); +/// let arg = write(dir.path(), "ui.args", &["--edition=2024".to_owned()]) +/// .expect("write the response file"); +/// assert!(arg.starts_with('@')); +/// ``` +pub fn write(dir: &Path, file_name: &str, args: &[String]) -> io::Result { + let path: PathBuf = dir.join(file_name); + let text = render(args)?; + test_support::fs::write(&path, text.as_bytes()).map_err(|error| { + io::Error::other(format!("write response file {}: {error}", path.display())) + })?; + let path_str = path.to_str().ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidInput, + format!("response file path is not UTF-8: {}", path.display()), + ) + })?; + Ok(format!("@{path_str}")) +} + +#[cfg(test)] +mod tests { + //! Unit tests for the response-file builder. + //! + //! These also keep every helper used from each including crate, so a + //! consumer needing only part of the surface does not trip `dead_code`. + //! + //! The command-line-length failure these helpers exist to prevent is + //! Windows-specific and cannot be reproduced on the machines that run most + //! of this suite, so the contract asserted here is the file's *shape* — + //! which holds everywhere — rather than a host-specific spawn. + + use super::{render, write}; + use proptest::prelude::*; + + /// Convert borrowed test arguments to owned compiler arguments. + fn owned(args: &[&str]) -> Vec { + args.iter().map(|arg| (*arg).to_owned()).collect() + } + + /// Render each compiler argument on its own response-file line. + #[test] + fn each_argument_occupies_its_own_line() { + let args = owned(&["--edition=2024", "--crate-type=bin", "--emit=metadata"]); + let text = render(&args).expect("newline-free arguments render"); + assert_eq!( + text.lines().collect::>(), + ["--edition=2024", "--crate-type=bin", "--emit=metadata"], + "rustc splits a response file on line boundaries, one argument per line" + ); + assert!( + text.ends_with('\n'), + "the final argument needs its terminator too, got {text:?}" + ); + } + + /// Preserve arguments containing spaces on one response-file line. + #[test] + fn arguments_containing_spaces_stay_on_one_line() { + // A path with a space must not be split; the line boundary is the only + // separator, so no quoting is applied or needed. + let args = owned(&["-o", "/tmp/a directory/out.rmeta"]); + let text = render(&args).expect("spaces are legal in a response file"); + assert_eq!( + text.lines().collect::>(), + ["-o", "/tmp/a directory/out.rmeta"] + ); + } + + /// Reject an argument whose newline would become a second argument. + #[test] + fn a_newline_in_an_argument_is_rejected() { + let error = render(&owned(&["-L", "dependency=/tmp/a\nb"])) + .expect_err("a newline would silently become an argument boundary"); + assert!( + error.to_string().contains("newline"), + "the error should name the cause, got {error}" + ); + } + + /// Render an empty compiler-argument list as empty text. + #[test] + fn an_empty_argument_list_renders_empty() { + assert_eq!(render(&[]).expect("no arguments render"), ""); + } + + proptest! { + /// Preserve every newline-free compiler argument in ordered UTF-8 form. + #[test] + fn render_preserves_newline_free_argument_vectors( + args in proptest::collection::vec("[\\p{L}\\p{N}\\p{P}\\p{Zs}]{0,32}", 0..32), + ) { + let expected = args.iter().fold(String::new(), |mut text, argument| { + text.push_str(argument); + text.push('\n'); + text + }); + let actual = render(&args).map_err(|error| error.to_string()); + + prop_assert_eq!(actual, Ok(expected)); + } + } + + /// Retain every compiler argument when writing the response file. + #[test] + fn the_written_file_retains_every_compiler_argument() { + let dir = tempfile::tempdir().expect("create temp dir"); + let args = owned(&[ + "--edition=2024", + "--crate-type=bin", + "--emit=metadata", + "/repo/tests/ui/fixture.rs", + "--extern", + "test_support=/target/debug/libtest_support.rmeta", + "-L", + "dependency=/target/debug/build/anyhow/1/out", + "-L", + "dependency=/target/debug/build/serde/2/out", + "-o", + "/tmp/out.rmeta", + ]); + let arg = write(dir.path(), "ui.args", &args).expect("write the response file"); + + let path = arg + .strip_prefix('@') + .expect("the argument passed to rustc is @"); + let text = test_support::fs::read_to_string(path).expect("read the response file back"); + assert_eq!( + text.lines().collect::>(), + args, + "every argument should survive the round trip, in order" + ); + // Spot-check the categories the harnesses depend on, so a future + // refactor that drops one fails here rather than as an E0463 or a + // missing-output error from rustc. + assert!( + text.contains("\n/repo/tests/ui/fixture.rs\n"), + "source path retained" + ); + assert!(text.contains("\n--extern\n"), "extern flag retained"); + assert_eq!( + text.matches("\ndependency=").count(), + 2, + "every dependency search directory retained" + ); + assert!( + text.ends_with("-o\n/tmp/out.rmeta\n"), + "output path retained last" + ); + } +} diff --git a/tests/workflow_contracts/ci_lint_test.py b/tests/workflow_contracts/ci_lint_test.py index 27d164062..dce3135d8 100644 --- a/tests/workflow_contracts/ci_lint_test.py +++ b/tests/workflow_contracts/ci_lint_test.py @@ -382,12 +382,14 @@ def test_windows_job_uses_git_bash_for_recipes() -> None: ) -def test_windows_setup_rust_keeps_warnings_and_polonius() -> None: - """The Windows toolchain setup preserves -D warnings and -Zpolonius=next. +def test_windows_setup_rust_keeps_warnings() -> None: + """The Windows toolchain setup preserves -D warnings. The `#[cfg(windows)]` tree must be compiled under `-D warnings` to surface - findings, and the tree requires the Polonius analysis, so the shared - setup-rust action must receive both flags through its `rustflags` input. + findings, so the shared setup-rust action must receive that flag through + its `rustflags` input. Polonius does not appear here: the pinned nightly + enables it by default, and restating a `-Zpolonius` directive is exactly + the fragility that retiring it removed. """ step = _windows_step("Setup Rust") assert "setup-rust" in step.get("uses", ""), ( @@ -402,9 +404,9 @@ def test_windows_setup_rust_keeps_warnings_and_polonius() -> None: "Setup Rust must use the pinned NETSUKE_RUST_TOOLCHAIN, " f"got {with_.get('toolchain')!r}" ) - assert with_.get("rustflags") == "-D warnings -Zpolonius=next", ( - "Setup Rust must pass -D warnings -Zpolonius=next through rustflags " - f"so the #[cfg(windows)] tree compiles under warnings-as-errors, " + assert with_.get("rustflags") == "-D warnings", ( + "Setup Rust must pass -D warnings through rustflags so the " + f"#[cfg(windows)] tree compiles under warnings-as-errors, " f"got {with_.get('rustflags')!r}" ) diff --git a/tools/dev-fast/config.toml b/tools/dev-fast/config.toml index ea3f851e5..0bc4ec1f9 100644 --- a/tools/dev-fast/config.toml +++ b/tools/dev-fast/config.toml @@ -1,13 +1,11 @@ # Opt-in Cargo configuration fragment for accelerated local debug builds. # -# This file is deliberately kept out of `.cargo/config.toml`. Cargo -# auto-discovers that path, and the repository uses it for settings every build -# must have — the Polonius flag. Cranelift and a Linux-only linker are not in -# that category: putting them there would apply them to release packaging, -# coverage, and the formal-verification toolchains, which must keep the -# supported LLVM backend and platform linker. This fragment is instead passed -# explicitly with `cargo --config tools/dev-fast/config.toml ...` from the -# `make dev-*` targets. +# This file is deliberately kept out of `.cargo/config.toml`, which Cargo +# auto-discovers. Cranelift and a Linux-only linker must not apply to release +# packaging, coverage, and the formal-verification toolchains, which must keep +# the supported LLVM backend and platform linker. This fragment is instead +# passed explicitly with `cargo --config tools/dev-fast/config.toml ...` from +# the `make dev-*` targets. # # The toolchain is the repository's own, from `rust-toolchain.toml`; only the # mold release is pinned here, in `tools/mold/VERSION`. See @@ -26,10 +24,7 @@ codegen-backend = "cranelift" # linker is used; `make dev-fast-check` reports that fallback explicitly rather # than leaving it implicit. # -# `-Zpolonius=next` is restated here, not inherited. Cargo picks a single -# rustflags source rather than merging them, and a `[target.*]` table outranks -# the `[build]` table in `.cargo/config.toml`, so naming any flag here drops -# Polonius. The tree only borrow-checks under Polonius (ADR-006), so omitting -# it does not merely diverge from the gate — it fails to compile. +# Cargo picks a single rustflags source rather than merging them, so anything +# this table must carry has to be named here in full; nothing is inherited. [target.'cfg(target_os = "linux")'] -rustflags = ["-Zpolonius=next", "-Clink-arg=-fuse-ld=mold"] +rustflags = ["-Clink-arg=-fuse-ld=mold"]