From 7d4df94f7b1d9dc73c69fbedf4620d6d0e11c05c Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 18 May 2026 21:14:32 +0200 Subject: [PATCH 01/21] Draft rstest helper fixture lint plan Add the pre-implementation ExecPlan for roadmap item 8.2.1. The plan captures the scope, experimental wiring decision, validation steps, review requirements, and approval gate for creating the `rstest_helper_should_be_fixture` lint crate. --- ...2-1-create-the-rstest-helper-lint-crate.md | 684 ++++++++++++++++++ 1 file changed, 684 insertions(+) create mode 100644 docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md diff --git a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md new file mode 100644 index 00000000..b0307f87 --- /dev/null +++ b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md @@ -0,0 +1,684 @@ +# Create the `rstest_helper_should_be_fixture` lint crate + +This ExecPlan (execution plan) is a living document. The sections +`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, +`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work +proceeds. + +Status: DRAFT + +## Purpose / big picture + +Roadmap item 8.2.1 starts the first `rstest` fixture hygiene lint. After this +plan is approved and implemented, Whitaker will contain a new experimental +Dylint crate named `rstest_helper_should_be_fixture`. The crate will declare +and register the public lint `RSTEST_HELPER_SHOULD_BE_FIXTURE`, load its +configuration with sensible defaults, and be visible to the existing suite and +installer wiring without yet attempting the full call-site aggregation +algorithm. + +The observable result is structural rather than diagnostic-heavy: a maintainer +can build the new lint crate, see the lint name in Whitaker's registration +metadata, configure its defaults in `dylint.toml`, and run the unit, +behavioural, lint, and formatting gates successfully. The later roadmap items +8.2.2, 8.2.3, and 8.2.4 will add call-site collection, cross-test +aggregation, diagnostic emission, and UI pass/fail coverage. + +This plan must be approved before implementation starts. + +## Constraints + +- This plan covers roadmap item 8.2.1 only. Do not implement the full repeated + helper-call detector, cross-test grouping, final diagnostic wording, or UI + failure expectations that belong to 8.2.2 through 8.2.4. +- Keep the new lint experimental. It must not become part of the standard + default lint set until later tuning against real repositories. +- Reuse existing repository conventions for Dylint crates, suite registration, + installer resolution, configuration, `rstest` helpers, behaviour tests, and + documentation. +- Preserve the hexagonal boundary in a pragmatic Rust form: pure rule and + configuration policy should be testable without rustc or filesystem I/O, and + compiler/Dylint integration should stay in the driver adapter layer. +- Do not add external dependencies unless implementation proves there is no + reasonable existing workspace dependency or standard-library alternative. +- Do not mark roadmap item 8.2.1 done until implementation has landed, gates + pass, CodeRabbit has no unresolved concerns, and the implementation PR is + ready for review. +- Follow the repository rule that `make check-fmt`, `make lint`, and + `make test` are the commit gates for code changes. Run command suites + sequentially and capture output with `tee` into `/tmp`. +- Use `rstest` for unit tests and `rstest-bdd` for behavioural tests where + behaviour is user-meaningful. Property tests, Kani, or Verus are not required + for the 8.2.1 bootstrap because this step introduces no new algorithmic + invariant over a range of inputs. +- Keep every Rust source file at or below 400 lines and ensure each module + starts with a `//!` module-level purpose comment. + +## Tolerances + +- Scope: if the implementation requires more than 14 repository files or more + than 650 net code lines, stop and ask whether 8.2.1 should be split. +- Interface: if a public API in an existing crate must change incompatibly, + stop and present options. +- Dependencies: if a new third-party dependency is needed, stop and justify it + before adding it. +- Test strategy: if the implementation cannot provide both unit coverage and + behaviour coverage for configuration and registration, stop and explain the + missing observable behaviour. +- Diagnostics: if meaningful diagnostic emission becomes necessary to satisfy + tests, stop and confirm whether part of 8.2.2 or 8.2.3 should be pulled into + this task. +- Validation: if any of `make check-fmt`, `make lint`, or `make test` still + fails after two fix attempts, stop and document the failing command, log + path, and available choices. +- Review: if `coderabbit review --agent` reports concerns after a major + milestone, address them or record why they are out of scope before moving on. +- Ambiguity: if experimental suite wiring conflicts with the existing install + path, stop and ask whether the lint should be standalone-only for 8.2.1. + +## Risks + +- Risk: The existing suite has only a `dylint-driver` feature and derives + experimental feature names from + `installer::resolution::EXPERIMENTAL_LINT_CRATES`. Severity: medium. + Likelihood: medium. Mitigation: add a suite feature named + `experimental-rstest-helper-should-be-fixture` that enables the new optional + lint dependency, and keep the installer source of truth in sync. + +- Risk: A lint that does not yet emit diagnostics can look like dead code. + Severity: medium. Likelihood: medium. Mitigation: make the first milestone + observable through registration, configuration loading, suite metadata, + installer resolution, and tests that exercise those contracts. + +- Risk: Configuration schema drift can create user-facing documentation that + does not match runtime defaults. Severity: medium. Likelihood: low. + Mitigation: define a single `Config::default()` shape in the driver policy + layer, test deserialization with `rstest`, and copy the same defaults into + `docs/users-guide.md` and + `docs/lints-for-rstest-fixtures-and-test-hygiene.md` only if the + implementation changes the design. + +- Risk: `rstest` procedural macro expansion can hide source attributes from + late lint passes. Severity: medium. Likelihood: medium. Mitigation: for + 8.2.1, rely only on the existing `common::rstest` pure detection and + `whitaker::hir` span-recovery hooks. Defer macro-heavy call-site collection + and UI diagnostics to 8.2.2 through 8.2.4. + +- Risk: Full workspace gates are expensive and can expose unrelated failures. + Severity: medium. Likelihood: low. Mitigation: run targeted tests before full + gates, then run the required `make` gates sequentially with logs under + `/tmp`. If unrelated failures are discovered, record evidence and ask for + direction. + +## Progress + +- [x] (2026-05-18T18:36:20Z) Loaded the `leta`, `rust-router`, + `execplans`, `hexagonal-architecture`, `firecrawl-mcp`, `arch-crate-design`, + `rust-errors`, `en-gb-oxendict-style`, `commit-message`, and `pr-creation` + skills needed for planning, branch work, Rust crate boundaries, configuration + errors, documentation style, commits, and PR creation. +- [x] (2026-05-18T18:36:20Z) Created the Leta workspace for this checkout with + `leta workspace add`. +- [x] (2026-05-18T18:36:20Z) Renamed the local branch to + `8-2-1-create-the-rstest-helper-lint-crate`. +- [x] (2026-05-18T18:36:20Z) Used two Wyvern agents to inspect repository + crate/configuration patterns and roadmap/design-document requirements. +- [x] (2026-05-18T18:36:20Z) Used Firecrawl to confirm current Dylint + registration/configuration contracts and `rstest` fixture/test semantics from + upstream documentation. +- [x] (2026-05-18T18:36:20Z) Drafted this pre-implementation ExecPlan. +- [x] (2026-05-18T18:36:20Z) Ran `make fmt` to format the draft and reverted + unrelated formatter-only changes outside this plan file. +- [x] (2026-05-18T18:36:20Z) Validated the plan milestone with + `make check-fmt`, `make markdownlint`, `make lint`, and `make test`. +- [x] (2026-05-18T18:36:20Z) Reviewed and revised this plan after local + formatting and linting. +- [x] (2026-05-18T18:36:20Z) Ran `coderabbit review --agent` for the plan + milestone and cleared Markdown wrapping concerns. +- [x] (2026-05-18T18:36:20Z) Reran `coderabbit review --agent` and received + 0 findings. +- [ ] Commit this plan once documentation validation passes. +- [ ] Push the branch and create a draft PR for the ExecPlan. +- [ ] Wait for explicit approval before implementing the lint crate. + +## Surprises & discoveries + +- Observation: The installer already has an experimental lint path even though + `EXPERIMENTAL_LINT_CRATES` is currently empty. Evidence: + `installer/src/builder.rs` derives feature names as + `experimental-{lint-name-with-hyphens}` and appends them when building the + suite with `--experimental`. Impact: 8.2.1 should use that existing mechanism + instead of inventing a new feature naming scheme. + +- Observation: The roadmap prerequisites are already complete. + Evidence: `docs/roadmap.md` marks 8.1.1 and 8.1.3 as `[x]`. Impact: + implementation can use `common::rstest` detection and fingerprint APIs + directly without adding prerequisite work to this plan. + +- Observation: Current Dylint upstream docs list `dylint_linting` 6.0.0 as the + latest release, but this workspace pins `dylint_linting = "5"`. Evidence: + Firecrawl retrieved the docs.rs crate page for `dylint_linting` latest, and + `Cargo.toml` pins workspace dependency `dylint_linting = "5"`. Impact: + implementation must follow the local workspace pin and existing macro usage, + not upgrade Dylint as part of this task. + +## Decision log + +- Decision: Keep this task to scaffolding, registration, and configuration + loading, with no attempt at the full call-site detector. Rationale: Roadmap + item 8.2.1 is explicitly followed by 8.2.2 for call-site collection, 8.2.3 + for aggregation and actionable diagnostics, and 8.2.4 for UI pass/fail + coverage. Date/Author: 2026-05-18T18:36:20Z / Codex. + +- Decision: Introduce the new lint as experimental. + Rationale: `docs/developers-guide.md` states that new lints should typically + start experimental, and the lint design warns that these fixture-hygiene + lints need false-positive tuning before promotion. Date/Author: + 2026-05-18T18:36:20Z / Codex. + +- Decision: Use the existing Rust crate pattern rather than a new architectural + directory layout. Rationale: The repository already organizes lints as + independent Dylint crates under `crates/`, with pure helper code in `common/` + and driver glue inside each lint crate. This satisfies the useful part of + hexagonal architecture without transplanting a foreign package structure. + Date/Author: 2026-05-18T18:36:20Z / Codex. + +- Decision: Do not require Kani or Verus for 8.2.1. + Rationale: The bootstrap creates no substantive invariant over state spaces + or contractual business axiom. Later algorithmic milestones can revisit + property testing or bounded checking when they add grouping logic. + Date/Author: 2026-05-18T18:36:20Z / Codex. + +## Outcomes & retrospective + +No implementation outcome exists yet. This draft records the intended scope, +boundaries, validation, and review workflow for approval. + +The plan milestone has passed local validation: + +- `make check-fmt` succeeded. +- `make markdownlint` succeeded with 0 Markdown errors. +- `make lint` succeeded. +- `make test` succeeded with 1418 tests passed and 2 skipped under the default + nextest profile. +- `coderabbit review --agent` completed with 0 findings after review fixes. + +Once implementation starts, update this section after each major milestone with +what landed, what changed, and what remains. + +## Context and orientation + +Whitaker is a Rust Cargo workspace. The root `Cargo.toml` declares workspace +members `common`, `crates/*`, `installer`, and `suite`. Individual Dylint lint +crates live under `crates/`; shared pure helpers live in `common/`; the root +`whitaker` crate exposes shared compiler-facing glue; and the `suite/` crate +aggregates lint crates into one Dylint cdylib. + +The relevant roadmap entry is in `docs/roadmap.md`: + +```plaintext +8.2.1. Create the `rstest_helper_should_be_fixture` lint crate, register +`RSTEST_HELPER_SHOULD_BE_FIXTURE`, and wire configuration loading defaults. +``` + +The design source of truth is +`docs/lints-for-rstest-fixtures-and-test-hygiene.md`, especially "Lint A: +call-site fixture extraction". That document defines: + +- crate name: `rstest_helper_should_be_fixture`, +- lint name: `RSTEST_HELPER_SHOULD_BE_FIXTURE`, +- default configuration keys: + `min_calls`, `min_distinct_tests`, `require_identical_fixture_arg_names`, + `provider_param_attributes`, and `use_source_callee_fallback`, +- strict `rstest` detection for `rstest` and `rstest::rstest`, +- default provider parameter attributes: + `case`, `values`, `files`, `future`, and `context`, +- first-version fixture-local support for simple identifier bindings only. + +Roadmap prerequisites 8.1.1 and 8.1.3 are already complete. They provide the +shared `common::rstest` APIs in `common/src/rstest/`: + +- `is_rstest_test` and `is_rstest_test_with` for strict test detection, +- `is_rstest_fixture` and `is_rstest_fixture_with` for strict fixture + detection, +- `RstestDetectionOptions` and `fixture_local_names` for fixture-local + parameter policy, +- `ArgFingerprint` and `ArgAtom` for later helper-call fingerprinting. + +Existing lint crates show the local Dylint pattern. For example, +`crates/conditional_max_n_branches/src/driver.rs` declares its lint with +`dylint_linting::impl_late_lint!`, loads per-lint configuration through +`dylint_linting::config::(LINT_NAME)`, falls back to defaults on +missing or invalid configuration, and loads shared locale settings through +`whitaker::SharedConfig`. The new crate should match this style. + +The suite registration files are: + +- `suite/Cargo.toml`, +- `suite/src/lints.rs`, +- `suite/src/driver.rs`, +- `suite/tests/registration.rs`, +- `suite/tests/features/suite_registration.feature`. + +The installer registration files are: + +- `installer/src/resolution.rs`, +- `installer/src/builder.rs`, +- `installer/src/scanner.rs`, +- `installer/tests/behaviour_core.rs`, +- `installer/tests/features/installer.feature`. + +The user-facing documentation that may need updates is: + +- `docs/users-guide.md` for the new lint section and configuration example, +- `docs/developers-guide.md` for any new internal convention, +- `docs/lints-for-rstest-fixtures-and-test-hygiene.md` for implementation + decisions if runtime behaviour differs from the design, +- `docs/roadmap.md` when implementation is complete. + +Useful skills for implementation are `leta`, `rust-router`, +`arch-crate-design`, `rust-errors`, `hexagonal-architecture`, +`en-gb-oxendict-style`, `commit-message`, and `pr-creation`. If later algorithm +work introduces invariants, route again through `rust-router` and consider +`kani` or `verus`. + +External references checked during planning: + +- Dylint `dylint_linting` docs: + . The macros `impl_late_lint!` + and configuration helpers such as `config_or_default`, `config`, and + `init_config` are the relevant public contracts. +- `rstest` fixture docs: + . Fixtures are + functions annotated with `#[fixture]` and injected as `#[rstest]` test + parameters. +- `rstest` test docs: + . + `#[rstest]` supports fixture injection, cases, values, files, `future`, and + `context` parameter attributes. + +## Plan of work + +Stage A is pre-implementation review. Read this ExecPlan, compare it with +`docs/roadmap.md` and `docs/lints-for-rstest-fixtures-and-test-hygiene.md`, and +obtain explicit approval. Do not edit production code before approval. + +Stage B creates the lint crate skeleton. Add +`crates/rstest_helper_should_be_fixture/Cargo.toml`, +`crates/rstest_helper_should_be_fixture/src/lib.rs`, and a driver module such +as `src/driver.rs`. The manifest should follow existing lint crates: +`crate-type = ["cdylib", "rlib"]`, `test = false`, a `dylint-driver` feature +that enables `dylint_linting`, `rustc_*`, `serde`, `log`, `whitaker`, and +`whitaker-common`, and a `constituent` feature that enables +`dylint_linting/constituent`. The non-driver build should compile through a +small stub module so `cargo check --workspace --all-targets --all-features` and +documentation builds stay healthy. + +Stage C implements registration and configuration policy. In the driver, +declare: + +```rust +dylint_linting::impl_late_lint! { + pub RSTEST_HELPER_SHOULD_BE_FIXTURE, + Warn, + "repeated rstest helper calls should be extracted into fixtures", + RstestHelperShouldBeFixture::default() +} +``` + +Define a `Config` with `#[serde(default, deny_unknown_fields)]` and these +defaults: + +```plaintext +min_calls = 2 +min_distinct_tests = 2 +require_identical_fixture_arg_names = false +provider_param_attributes = ["case", "values", "files", "future", "context"] +use_source_callee_fallback = false +``` + +Keep pure validation and normalization near the config type. The driver should +load configuration in `check_crate`, clamp invalid numeric thresholds to at +least `1` or reject them by falling back to defaults in the same manner as +existing lints, and build `RstestDetectionOptions` from the provider +attributes. If a value is invalid enough that clamping would hide a user +mistake, use the existing "log and default" pattern rather than panic. + +Stage D adds a minimal late-lint pass body that proves wiring without claiming +8.2.2 behaviour. The pass may inspect crate or item structure enough to prove +that it can construct configuration and detection options, but it should not +emit helper-call diagnostics until the collection and aggregation milestones +exist. Any placeholder should be named as a bootstrap path, not as completed +analysis. + +Stage E wires registration outside the crate. Add +`rstest_helper_should_be_fixture` to `installer/src/resolution.rs` +`EXPERIMENTAL_LINT_CRATES`, not to `LINT_CRATES`. Add the suite optional +dependency and a feature named `experimental-rstest-helper-should-be-fixture` +to `suite/Cargo.toml`, following the feature name that +`installer/src/builder.rs` derives from the experimental crate list. Update +`suite/src/lints.rs` and `suite/src/driver.rs` so the lint is included only +when that experimental feature is enabled. Update suite and installer tests to +expect the new experimental lint when experimental mode is on and to exclude it +otherwise. + +Stage F adds tests. Unit tests should use `rstest` for `Config::default()`, +valid TOML deserialization, unknown-field rejection, provider-attribute +normalization, threshold handling, and feature-name expectations if helper +functions are added. Behaviour tests should use `rstest-bdd` where the +observable behaviour is "experimental lint resolution includes the new crate" +or "the suite metadata exposes the new lint only with experimental features". +If a crate-local behaviour file is added, keep it focused on configuration and +registration, not future diagnostic detection. + +Stage G updates documentation. Add a concise experimental lint section to +`docs/users-guide.md` that identifies the purpose, scope, and configuration +defaults. Say that this first implementation registers and configures the lint +and that diagnostic emission follows the later roadmap items if that remains +true at implementation time. Update `docs/developers-guide.md` only if a new +experimental-suite convention is introduced or clarified. Update +`docs/lints-for-rstest-fixtures-and-test-hygiene.md` with an implementation +decision note only if the implementation materially differs from the current +design. Mark `docs/roadmap.md` item 8.2.1 done only after the implementation +passes gates and review. + +Stage H validates, reviews, commits, and prepares the implementation PR. Run +targeted tests first, then the full gates. Run `coderabbit review --agent` +after the crate/configuration milestone and again before final push if the +review tool is available. Address all relevant concerns. Commit in small +logical units: crate bootstrap, registration wiring, tests, documentation, and +roadmap completion if those land separately. + +## Concrete steps + +From the repository root, confirm branch and workspace state: + +```sh +git branch --show-current +git status --short --branch +``` + +Expected output includes: + +```plaintext +8-2-1-create-the-rstest-helper-lint-crate +``` + +After approval, create the crate files and add registration wiring. Use +`apply_patch` for manual edits. Prefer existing crate files as templates: + +```sh +sed -n '1,180p' crates/conditional_max_n_branches/src/driver.rs +sed -n '1,140p' crates/conditional_max_n_branches/Cargo.toml +sed -n '1,140p' suite/src/lints.rs +sed -n '1,180p' installer/src/resolution.rs +``` + +Run targeted checks after the first code milestone: + +```sh +ACTION=check-rstest-helper +LOG="/tmp/${ACTION}-whitaker-$(git branch --show-current).out" +cargo check -p rstest_helper_should_be_fixture --all-targets --all-features 2>&1 | tee "$LOG" +``` + +Run targeted unit and behaviour tests after tests are added: + +```sh +ACTION=test-rstest-helper +LOG="/tmp/${ACTION}-whitaker-$(git branch --show-current).out" +cargo nextest run -p rstest_helper_should_be_fixture --all-targets --all-features 2>&1 | tee "$LOG" +``` + +Run suite and installer tests touched by registration: + +```sh +ACTION=test-registration +LOG="/tmp/${ACTION}-whitaker-$(git branch --show-current).out" +cargo nextest run -p whitaker_suite -p whitaker-installer --all-targets --all-features 2>&1 | tee "$LOG" +``` + +Run documentation checks after documentation changes: + +```sh +ACTION=markdownlint +LOG="/tmp/${ACTION}-whitaker-$(git branch --show-current).out" +make markdownlint 2>&1 | tee "$LOG" +``` + +If Mermaid diagrams are edited, also run: + +```sh +ACTION=nixie +LOG="/tmp/${ACTION}-whitaker-$(git branch --show-current).out" +make nixie 2>&1 | tee "$LOG" +``` + +Run final gates sequentially before committing code as complete: + +```sh +ACTION=check-fmt +LOG="/tmp/${ACTION}-whitaker-$(git branch --show-current).out" +make check-fmt 2>&1 | tee "$LOG" + +ACTION=lint +LOG="/tmp/${ACTION}-whitaker-$(git branch --show-current).out" +make lint 2>&1 | tee "$LOG" + +ACTION=test +LOG="/tmp/${ACTION}-whitaker-$(git branch --show-current).out" +make test 2>&1 | tee "$LOG" +``` + +Run CodeRabbit after each major milestone: + +```sh +coderabbit review --agent +``` + +Commit using a file-based commit message: + +```sh +git status --short +git diff --stat +git add +COMMIT_MSG_DIR=$(mktemp -d) +cat > "$COMMIT_MSG_DIR/COMMIT_MSG.md" << 'ENDOFMSG' +Create rstest helper fixture lint crate + +Add the experimental Dylint crate scaffold, registration wiring, +configuration defaults, tests, and documentation for roadmap item 8.2.1. +ENDOFMSG +git commit -F "$COMMIT_MSG_DIR/COMMIT_MSG.md" +rm -rf "$COMMIT_MSG_DIR" +``` + +Push and open a draft pull request only after validation succeeds: + +```sh +git push -u origin 8-2-1-create-the-rstest-helper-lint-crate +echo "${LODY_SESSION_ID}" +``` + +Use the Lody session URL in the PR body: + +```plaintext +https://lody.ai/leynos/sessions/${LODY_SESSION_ID} +``` + +## Validation and acceptance + +The plan is accepted for implementation when the user explicitly approves it. +Silence is not approval. + +The implementation is accepted when all of the following are true: + +- `crates/rstest_helper_should_be_fixture` exists and builds with + `--features dylint-driver`. +- The crate exposes `RSTEST_HELPER_SHOULD_BE_FIXTURE` and the pass type + `RstestHelperShouldBeFixture`. +- Configuration defaults match the lint design and are covered by `rstest` + unit tests. +- Unknown configuration fields are rejected or defaulted consistently with the + repository's existing Dylint configuration policy. +- The installer recognizes the crate as experimental and includes it only when + experimental lints are requested or when it is explicitly named. +- The suite exposes the lint only through the experimental feature + `experimental-rstest-helper-should-be-fixture`. +- Behaviour tests cover experimental registration and configuration loading in + user-observable terms. +- `docs/users-guide.md`, `docs/developers-guide.md`, and the rstest lint + design document are updated where behaviour or internal practice changed. +- `docs/roadmap.md` marks 8.2.1 done only after all implementation checks pass. +- `coderabbit review --agent` has no unresolved relevant concerns. +- `make check-fmt`, `make lint`, and `make test` all succeed. + +Expected final gate transcript shape: + +```plaintext +$ make check-fmt +... +Finished + +$ make lint +... +Finished + +$ make test +... +test result: ok +``` + +The exact test count may change as new tests are added; success means the +commands exit with status 0. + +## Idempotence and recovery + +All edit stages are ordinary file edits and can be retried from `git status`. +If a step fails part-way through, inspect `git diff` and either continue from +the incomplete file or revert only the files changed for this task. Do not +revert unrelated user changes. + +If a generated or copied lint crate scaffold is wrong, delete only +`crates/rstest_helper_should_be_fixture/` before it is committed, then recreate +it from the existing lint crate pattern. After a commit, prefer a corrective +follow-up commit over rewriting history unless the user asks for history +cleanup. + +If validation fails, inspect the matching +`/tmp/*-whitaker-8-2-1-create-the-rstest-helper-lint-crate.out` log before +changing code. Record persistent failures in `Surprises & Discoveries` or +`Decision Log` with the command and log path. + +## Artifacts and notes + +Wyvern repository-pattern findings: + +```plaintext +Existing lint crates use `dylint_linting::impl_late_lint!`, per-lint +`Config` structs, `LINT_NAME` matching the crate name, `SharedConfig` for +locale, and suite/installer registration through `suite/` and +`installer/src/resolution.rs`. +``` + +Wyvern documentation findings: + +```plaintext +8.2.1 is the bootstrap step. 8.2.2 starts call-site collection, 8.2.3 adds +aggregation and actionable diagnostics, and 8.2.4 adds UI pass/fail coverage. +``` + +Firecrawl findings: + +```plaintext +Dylint's `impl_late_lint!` macro wraps Dylint library registration, lint +declaration, lint-pass implementation, and late-pass registration for a +single-lint library. Dylint configuration is read from `dylint.toml` with +helpers such as `config`, `config_or_default`, and `init_config`. + +`rstest` fixtures are functions marked with `#[fixture]` and injected as test +arguments. `#[rstest]` parameters can also be provider-driven through +attributes such as `case`, `values`, `files`, `future`, and `context`. +``` + +## Interfaces and dependencies + +The new crate should expose these driver-facing items when `dylint-driver` is +enabled: + +```rust +pub const RSTEST_HELPER_SHOULD_BE_FIXTURE: &rustc_lint::Lint; + +pub struct RstestHelperShouldBeFixture { + config: Config, + detection_options: whitaker_common::rstest::RstestDetectionOptions, + localizer: whitaker_common::Localizer, +} +``` + +The exact field visibility may remain private. The public contract is the lint +constant and pass type. If the Dylint macro exposes a different lint constant +type than the simplified sketch above, follow the macro's actual expansion and +the pattern used by existing lint crates. + +The configuration policy should be equivalent to: + +```rust +#[derive(Clone, Debug, Deserialize, Eq, PartialEq)] +#[serde(default, deny_unknown_fields)] +struct Config { + min_calls: usize, + min_distinct_tests: usize, + require_identical_fixture_arg_names: bool, + provider_param_attributes: Vec, + use_source_callee_fallback: bool, +} +``` + +Do not export `Config` unless tests or downstream code genuinely require it. If +tests need access, prefer `pub(crate)` helpers or driver-internal tests. + +The new crate should depend on existing workspace dependencies only: + +- `dylint_linting`, +- `log`, +- `rustc_hir`, +- `rustc_lint`, +- `rustc_session`, +- `rustc_span`, +- `serde`, +- `whitaker`, +- `whitaker-common`, +- `rstest`, +- `rstest-bdd`, +- `rstest-bdd-macros`, +- `dylint_testing` if a UI harness is required for bootstrap smoke coverage. + +No new production dependency is expected for 8.2.1. + +## Revision note + +Initial draft created on 2026-05-18. It records the pre-implementation scope, +repo orientation, external references, experimental-suite decision, validation +commands, and approval gate for roadmap item 8.2.1. + +Revision on 2026-05-18 after local validation: recorded successful +`make check-fmt`, `make markdownlint`, `make lint`, and `make test` results. +This does not change the implementation scope; it only adds evidence for the +ExecPlan review milestone. + +Revision on 2026-05-18 after CodeRabbit review: wrapped one long Markdown line +reported by `coderabbit review --agent`. This is a formatting-only change with +no effect on implementation scope. + +Second revision on 2026-05-18 after CodeRabbit review: moved the `rstest` +test-documentation URL onto its own line and wrapped the following sentence. +This is a formatting-only change with no effect on implementation scope. + +Third revision on 2026-05-18 after CodeRabbit review: changed an `-isation` +spelling to the matching Oxford `-ization` spelling. This is a spelling-only +change with no effect on implementation scope. + +Fourth revision on 2026-05-18 after CodeRabbit review: recorded the clean +`coderabbit review --agent` result. This adds review evidence only and does not +change the implementation scope. From 27280ed3486dbc3937425655fb9e962b6971a506 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 18 May 2026 21:17:20 +0200 Subject: [PATCH 02/21] Record rstest helper plan PR progress Update the ExecPlan progress section after opening the draft pull request. This keeps the living plan aligned with the branch state without changing the implementation scope. --- .../8-2-1-create-the-rstest-helper-lint-crate.md | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md index b0307f87..4b414fac 100644 --- a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md +++ b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md @@ -137,8 +137,9 @@ This plan must be approved before implementation starts. milestone and cleared Markdown wrapping concerns. - [x] (2026-05-18T18:36:20Z) Reran `coderabbit review --agent` and received 0 findings. -- [ ] Commit this plan once documentation validation passes. -- [ ] Push the branch and create a draft PR for the ExecPlan. +- [x] (2026-05-18T19:15:16Z) Committed this plan after validation passed. +- [x] (2026-05-18T19:15:16Z) Pushed the branch and created draft PR + for the ExecPlan. - [ ] Wait for explicit approval before implementing the lint crate. ## Surprises & discoveries @@ -682,3 +683,7 @@ change with no effect on implementation scope. Fourth revision on 2026-05-18 after CodeRabbit review: recorded the clean `coderabbit review --agent` result. This adds review evidence only and does not change the implementation scope. + +Fifth revision on 2026-05-18 after draft PR creation: recorded the commit, +push, and draft PR URL in `Progress`. This is a process-tracking update with +no effect on implementation scope. From 813d2cc1d17e28a2d1800ce5e40ec042e11c24a7 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 20 May 2026 01:58:23 +0200 Subject: [PATCH 03/21] Add rstest helper fixture lint crate Create the experimental `rstest_helper_should_be_fixture` Dylint crate and wire it into the installer and suite behind the experimental feature. Add configuration defaults and unit coverage for the bootstrap policy, update registration behaviour coverage, and document the experimental user-facing behaviour for roadmap item 8.2.1. --- Cargo.lock | 18 ++ .../Cargo.toml | 42 +++ .../src/driver.rs | 249 ++++++++++++++++++ .../src/lib.rs | 20 ++ docs/developers-guide.md | 11 +- ...2-1-create-the-rstest-helper-lint-crate.md | 147 ++++++++--- ...ts-for-rstest-fixtures-and-test-hygiene.md | 14 + docs/roadmap.md | 2 +- docs/users-guide.md | 54 +++- installer/src/builder.rs | 7 +- installer/src/resolution.rs | 19 +- installer/tests/behaviour_core.rs | 1 + suite/Cargo.toml | 5 + suite/src/driver.rs | 28 ++ suite/src/lints.rs | 9 + 15 files changed, 580 insertions(+), 46 deletions(-) create mode 100644 crates/rstest_helper_should_be_fixture/Cargo.toml create mode 100644 crates/rstest_helper_should_be_fixture/src/driver.rs create mode 100644 crates/rstest_helper_should_be_fixture/src/lib.rs diff --git a/Cargo.lock b/Cargo.lock index e88893dd..01eea015 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2308,6 +2308,23 @@ version = "0.5.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "88f2ca584f7d9d359a09f616900be015302c959d58c2c2da6c075573352dfa0d" +[[package]] +name = "rstest_helper_should_be_fixture" +version = "0.2.5" +dependencies = [ + "dylint_linting", + "log", + "rstest", + "rstest-bdd", + "rstest-bdd-macros", + "rustc_lint", + "rustc_session", + "serde", + "toml 0.9.12+spec-1.1.0", + "whitaker", + "whitaker-common", +] + [[package]] name = "rstest_macros" version = "0.26.1" @@ -3462,6 +3479,7 @@ dependencies = [ "rstest", "rstest-bdd", "rstest-bdd-macros", + "rstest_helper_should_be_fixture", "rustc_hir", "rustc_lint", "rustc_session", diff --git a/crates/rstest_helper_should_be_fixture/Cargo.toml b/crates/rstest_helper_should_be_fixture/Cargo.toml new file mode 100644 index 00000000..0c6972b1 --- /dev/null +++ b/crates/rstest_helper_should_be_fixture/Cargo.toml @@ -0,0 +1,42 @@ +[package] +name = "rstest_helper_should_be_fixture" +version = "0.2.5" +edition = "2024" +publish = false +description = "Dylint lint that bootstraps rstest helper fixture extraction checks" +license.workspace = true +repository.workspace = true +homepage.workspace = true +documentation.workspace = true + +[lib] +crate-type = ["cdylib", "rlib"] +test = false + +[features] +default = [] +dylint-driver = [ + "dep:whitaker-common", + "dep:dylint_linting", + "dep:log", + "dep:rustc_lint", + "dep:rustc_session", + "dep:serde", + "dep:whitaker", +] +constituent = ["dylint-driver", "dylint_linting/constituent"] + +[dependencies] +whitaker-common = { workspace = true, optional = true } +dylint_linting = { workspace = true, optional = true } +log = { workspace = true, optional = true } +rustc_lint = { workspace = true, optional = true } +rustc_session = { workspace = true, optional = true } +serde = { workspace = true, optional = true } +whitaker = { workspace = true, features = ["dylint-driver"], optional = true } + +[dev-dependencies] +rstest = { workspace = true } +rstest-bdd = { workspace = true } +rstest-bdd-macros = { workspace = true } +toml = { workspace = true } diff --git a/crates/rstest_helper_should_be_fixture/src/driver.rs b/crates/rstest_helper_should_be_fixture/src/driver.rs new file mode 100644 index 00000000..6e4a0385 --- /dev/null +++ b/crates/rstest_helper_should_be_fixture/src/driver.rs @@ -0,0 +1,249 @@ +//! Dylint driver bootstrap for the `rstest` helper fixture lint. +//! +//! The driver owns compiler integration and configuration loading. Pure +//! configuration normalization stays in small helper methods so it can be +//! tested without constructing rustc contexts. + +use log::debug; +use rustc_lint::{LateContext, LateLintPass}; +use serde::Deserialize; +use whitaker::SharedConfig; +use whitaker_common::attributes::AttributePath; +use whitaker_common::i18n::{Localizer, get_localizer_for_lint}; +use whitaker_common::rstest::RstestDetectionOptions; + +const LINT_NAME: &str = "rstest_helper_should_be_fixture"; + +const DEFAULT_PROVIDER_PARAM_ATTRIBUTES: &[&str] = + &["case", "values", "files", "future", "context"]; + +dylint_linting::impl_late_lint! { + pub RSTEST_HELPER_SHOULD_BE_FIXTURE, + Warn, + "repeated rstest helper calls should be extracted into fixtures", + RstestHelperShouldBeFixture::default() +} + +/// Configuration for the `rstest_helper_should_be_fixture` lint. +/// +/// Values are loaded from `dylint.toml` and normalized so threshold settings +/// keep repeated-helper semantics. +#[derive(Clone, Debug, Deserialize, Eq, PartialEq)] +#[serde(default, deny_unknown_fields)] +struct Config { + min_calls: usize, + min_distinct_tests: usize, + require_identical_fixture_arg_names: bool, + provider_param_attributes: Vec, + use_source_callee_fallback: bool, +} + +impl Default for Config { + fn default() -> Self { + Self { + min_calls: 2, + min_distinct_tests: 2, + require_identical_fixture_arg_names: false, + provider_param_attributes: DEFAULT_PROVIDER_PARAM_ATTRIBUTES + .iter() + .map(ToString::to_string) + .collect(), + use_source_callee_fallback: false, + } + } +} + +impl Config { + fn normalized(self) -> Self { + Self { + min_calls: self.min_calls.max(2), + min_distinct_tests: self.min_distinct_tests.max(2), + require_identical_fixture_arg_names: self.require_identical_fixture_arg_names, + provider_param_attributes: normalize_provider_attributes( + self.provider_param_attributes, + ), + use_source_callee_fallback: self.use_source_callee_fallback, + } + } + + fn detection_options(&self) -> RstestDetectionOptions { + let provider_param_attributes = self + .provider_param_attributes + .iter() + .flat_map(|attribute| { + [ + AttributePath::from(attribute.as_str()), + AttributePath::from(format!("rstest::{attribute}")), + ] + }) + .collect(); + RstestDetectionOptions::new(provider_param_attributes, self.use_source_callee_fallback) + } +} + +/// Lint pass bootstrap for repeated `rstest` helper extraction. +pub struct RstestHelperShouldBeFixture { + config: Config, + detection_options: RstestDetectionOptions, + localizer: Localizer, +} + +impl Default for RstestHelperShouldBeFixture { + fn default() -> Self { + let config = Config::default(); + let detection_options = config.detection_options(); + Self { + config, + detection_options, + localizer: Localizer::new(None), + } + } +} + +impl<'tcx> LateLintPass<'tcx> for RstestHelperShouldBeFixture { + fn check_crate(&mut self, _cx: &LateContext<'tcx>) { + self.config = load_configuration(); + self.detection_options = self.config.detection_options(); + let shared_config = SharedConfig::load(); + self.localizer = get_localizer_for_lint(LINT_NAME, shared_config.locale()); + } +} + +fn normalize_provider_attributes(attributes: Vec) -> Vec { + let normalized: Vec = attributes + .into_iter() + .map(|attribute| attribute.trim().trim_start_matches("rstest::").to_owned()) + .filter(|attribute| !attribute.is_empty()) + .collect(); + + if normalized.is_empty() { + return default_provider_param_attributes(); + } + + normalized +} + +fn default_provider_param_attributes() -> Vec { + DEFAULT_PROVIDER_PARAM_ATTRIBUTES + .iter() + .map(ToString::to_string) + .collect() +} + +fn load_configuration() -> Config { + match dylint_linting::config::(LINT_NAME) { + Ok(Some(config)) => config.normalized(), + Ok(None) => Config::default(), + Err(error) => { + debug!( + target: LINT_NAME, + "failed to parse `{LINT_NAME}` configuration: {error}; using defaults" + ); + Config::default() + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use rstest::rstest; + + #[rstest] + fn default_configuration_matches_design() { + let config = Config::default(); + + assert_eq!(config.min_calls, 2); + assert_eq!(config.min_distinct_tests, 2); + assert!(!config.require_identical_fixture_arg_names); + assert_eq!( + config.provider_param_attributes, + ["case", "values", "files", "future", "context"] + ); + assert!(!config.use_source_callee_fallback); + } + + #[rstest] + fn deserializes_valid_configuration() { + let config: Config = toml::from_str::( + r#" + min_calls = 3 + min_distinct_tests = 4 + require_identical_fixture_arg_names = true + provider_param_attributes = ["case", "custom_provider"] + use_source_callee_fallback = true + "#, + ) + .expect("valid configuration should deserialize") + .normalized(); + + assert_eq!(config.min_calls, 3); + assert_eq!(config.min_distinct_tests, 4); + assert!(config.require_identical_fixture_arg_names); + assert_eq!( + config.provider_param_attributes, + ["case", "custom_provider"] + ); + assert!(config.use_source_callee_fallback); + } + + #[rstest] + fn rejects_unknown_configuration_fields() { + let result = toml::from_str::("unexpected = true"); + + assert!(result.is_err()); + } + + #[rstest] + fn normalizes_numeric_thresholds_to_two() { + let config = Config { + min_calls: 0, + min_distinct_tests: 0, + ..Config::default() + } + .normalized(); + + assert_eq!(config.min_calls, 2); + assert_eq!(config.min_distinct_tests, 2); + } + + #[rstest] + #[case::plain(vec!["case".to_string()], vec!["case"])] + #[case::qualified(vec!["rstest::values".to_string()], vec!["values"])] + #[case::blank(vec![" ".to_string()], vec!["case", "values", "files", "future", "context"])] + fn normalizes_provider_attributes(#[case] input: Vec, #[case] expected: Vec<&str>) { + let normalized = normalize_provider_attributes(input); + let expected: Vec = expected.into_iter().map(ToString::to_string).collect(); + + assert_eq!(normalized, expected); + } + + #[rstest] + fn detection_options_expand_plain_and_rstest_qualified_provider_paths() { + let config = Config { + provider_param_attributes: vec!["case".to_string(), "custom".to_string()], + use_source_callee_fallback: true, + ..Config::default() + }; + let options = config.detection_options(); + let paths: Vec = options + .provider_param_attributes() + .iter() + .map(ToString::to_string) + .collect(); + + assert_eq!(paths, ["case", "rstest::case", "custom", "rstest::custom"]); + assert!(options.use_expansion_trace_fallback()); + } + + #[rstest] + fn lint_pass_default_derives_detection_options_from_config() { + let pass = RstestHelperShouldBeFixture::default(); + + assert_eq!(pass.config, Config::default()); + assert_eq!( + pass.detection_options.provider_param_attributes().len(), + DEFAULT_PROVIDER_PARAM_ATTRIBUTES.len() * 2 + ); + } +} diff --git a/crates/rstest_helper_should_be_fixture/src/lib.rs b/crates/rstest_helper_should_be_fixture/src/lib.rs new file mode 100644 index 00000000..cf05dad2 --- /dev/null +++ b/crates/rstest_helper_should_be_fixture/src/lib.rs @@ -0,0 +1,20 @@ +//! Experimental lint crate for `rstest` helper fixture extraction. +//! +//! This crate declares the `rstest_helper_should_be_fixture` Dylint lint and +//! wires its configuration defaults. The first implementation intentionally +//! stops before helper-call collection and diagnostic emission; later roadmap +//! items add the analysis that decides when a repeated helper call should +//! become an `rstest` fixture. +#![cfg_attr(feature = "dylint-driver", feature(rustc_private))] + +#[cfg(feature = "dylint-driver")] +mod driver; + +#[cfg(feature = "dylint-driver")] +pub use driver::*; + +#[cfg(not(feature = "dylint-driver"))] +mod stub { + #[expect(dead_code, reason = "stub used when the driver feature is disabled")] + pub fn rstest_helper_should_be_fixture_disabled_stub() {} +} diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 945ba433..1f48dd97 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1445,7 +1445,9 @@ Whitaker data directory keyed by toolchain and target: - `-t, --target-dir DIR` — Staging directory for built libraries - `-l, --lint NAME` — Build specific lint (repeatable) - `--individual-lints` — Build individual crates instead of the suite -- `--experimental` — Include experimental lints in the build (none currently) +- `--experimental` — Include experimental lints in the build. In suite mode + this enables feature-gated experimental lints on `whitaker_suite`; in + `--individual-lints` mode it adds crates from `EXPERIMENTAL_LINT_CRATES`. - `--toolchain TOOLCHAIN` — Override the detected toolchain - `--cranelift` — Install `rustc-codegen-cranelift` for the selected toolchain - `-j, --jobs N` — Number of parallel build jobs @@ -1553,7 +1555,10 @@ Whitaker categorizes lints into two tiers: false positives or undergo breaking changes. They require explicit opt-in via the `--experimental` flag. -At present, all shipped Whitaker lints are standard. +The current experimental set contains `rstest_helper_should_be_fixture`. It is +feature-gated in the suite as `experimental-rstest-helper-should-be-fixture` +and listed in `installer/src/resolution.rs` so the installer can derive the +matching suite feature automatically. ### Adding a new lint @@ -1564,6 +1569,8 @@ New lints should typically start as experimental. To add a lint: 2. Add the crate name to `EXPERIMENTAL_LINT_CRATES` in `installer/src/resolution.rs` 3. Add a feature flag for the lint in `suite/Cargo.toml` under `[features]` +4. Add an optional suite dependency and gate its descriptor, lint declaration, + and combined pass entry behind that feature ### Promoting to standard diff --git a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md index 4b414fac..b76cd80e 100644 --- a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md +++ b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md @@ -1,11 +1,10 @@ # Create the `rstest_helper_should_be_fixture` lint crate -This ExecPlan (execution plan) is a living document. The sections -`Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, -`Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work -proceeds. +This ExecPlan (execution plan) is a living document. The sections `Constraints`, + `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, +and `Outcomes & Retrospective` must be kept up to date as work proceeds. -Status: DRAFT +Status: IN PROGRESS ## Purpose / big picture @@ -21,8 +20,8 @@ The observable result is structural rather than diagnostic-heavy: a maintainer can build the new lint crate, see the lint name in Whitaker's registration metadata, configure its defaults in `dylint.toml`, and run the unit, behavioural, lint, and formatting gates successfully. The later roadmap items -8.2.2, 8.2.3, and 8.2.4 will add call-site collection, cross-test -aggregation, diagnostic emission, and UI pass/fail coverage. +8.2.2, 8.2.3, and 8.2.4 will add call-site collection, cross-test aggregation, +diagnostic emission, and UI pass/fail coverage. This plan must be approved before implementation starts. @@ -140,7 +139,40 @@ This plan must be approved before implementation starts. - [x] (2026-05-18T19:15:16Z) Committed this plan after validation passed. - [x] (2026-05-18T19:15:16Z) Pushed the branch and created draft PR for the ExecPlan. -- [ ] Wait for explicit approval before implementing the lint crate. +- [x] (2026-05-20T00:00:00Z) Received explicit user approval to implement + the planned functionality. +- [x] (2026-05-20T00:00:00Z) Created the experimental lint crate and driver + bootstrap with `RSTEST_HELPER_SHOULD_BE_FIXTURE`, configuration defaults, + normalization, and `RstestDetectionOptions` construction. +- [x] (2026-05-20T00:00:00Z) Wired the crate into installer and suite + experimental registration via `EXPERIMENTAL_LINT_CRATES` and the suite feature + `experimental-rstest-helper-should-be-fixture`. +- [x] (2026-05-20T00:00:00Z) Added unit coverage for configuration defaults, + TOML deserialization, unknown fields, numeric threshold normalization, and + provider-attribute normalization in the new crate. +- [x] (2026-05-20T00:00:00Z) Confirmed behaviour coverage for experimental + registration through installer `rstest-bdd` scenarios and suite registration + scenarios. Targeted run + `cargo nextest run -p whitaker_suite -p whitaker-installer --all-targets + --all-features` passed 527 tests. +- [x] (2026-05-20T00:00:00Z) Updated user, developer, design, roadmap, and + plan documentation. `docs/roadmap.md` now marks 8.2.1 done. +- [x] (2026-05-20T00:00:00Z) Ran `coderabbit review --agent` after the + crate/bootstrap milestone and received 0 findings. +- [x] (2026-05-20T00:00:00Z) Ran final `coderabbit review --agent`. It + reported two findings: clamp thresholds to at least 2 and avoid constructing + a full default config for provider fallback. Both findings were accepted and + fixed. +- [x] (2026-05-20T00:00:00Z) Reran `coderabbit review --agent` after the + fixes. It reported one trivial Rustdoc concern for the private `Config` + type, which was accepted and fixed. +- [x] (2026-05-20T00:00:00Z) Ran final local gates. `make check-fmt`, + `make markdownlint`, `make lint`, and `make test` all passed; `make test` + reported 1428 passed and 2 skipped. +- [x] (2026-05-20T00:00:00Z) Reran final gates after the Rustdoc review fix. + `make check-fmt`, `make markdownlint`, `make lint`, and `make test` all + passed; `make test` again reported 1428 passed and 2 skipped. +- [ ] Commit, push, and update the draft PR. ## Surprises & discoveries @@ -163,6 +195,39 @@ This plan must be approved before implementation starts. implementation must follow the local workspace pin and existing macro usage, not upgrade Dylint as part of this task. +- Observation: The first implementation slice compiles without adding a new + workspace dependency. Evidence: + `cargo check -p rstest_helper_should_be_fixture --all-targets --all-features` + and `cargo check -p whitaker_suite --all-targets --all-features` both exited + with status 0. Impact: the planned crate boundary and experimental feature + wiring fit the existing workspace structure. + +- Observation: `make fmt` can introduce Markdown reference-link breakage around + issue-style links and bracketed reference text. Evidence: the first + formatting run split `[#180][issue-180]` and a + ``[`FluentLanguageLoader`]`` reference across lines, causing Markdown lint + failures. Impact: those passages were rewritten to avoid the formatter edge + case before continuing. + +- Observation: CodeRabbit caught that normalizing `min_calls` and + `min_distinct_tests` to 1 would make a "repeated" lint meaningful for a + single occurrence. Evidence: final review finding on + `crates/rstest_helper_should_be_fixture/src/driver.rs`. Impact: thresholds + now normalize to at least 2, matching the design defaults and repeated-call + semantics. + +- Observation: Even private configuration types benefit from Rustdoc when they + are the boundary between user TOML and lint policy. Evidence: final + CodeRabbit finding on `Config`. Impact: `Config` now documents that it is + loaded from `dylint.toml` and normalized before use. + +- Observation: A final confirmation rerun of `coderabbit review --agent` was + blocked by CodeRabbit rate limiting after the Rustdoc fix. Evidence: two + retry attempts returned recoverable rate-limit errors. Impact: all reported + CodeRabbit findings were fixed and local gates were rerun, but the tool could + not provide a zero-finding confirmation after the last documentation-only + code comment change. + ## Decision log - Decision: Keep this task to scaffolding, registration, and configuration @@ -192,8 +257,12 @@ This plan must be approved before implementation starts. ## Outcomes & retrospective -No implementation outcome exists yet. This draft records the intended scope, -boundaries, validation, and review workflow for approval. +Implementation has landed in the working tree and is awaiting final review, +commit, push, and PR update. The shipped behaviour is a bootstrap: the +experimental Dylint crate exists, the lint declaration and configuration +defaults are wired, suite and installer registration understand the +experimental lint, and documentation states that diagnostics follow later +roadmap items. The plan milestone has passed local validation: @@ -207,6 +276,26 @@ The plan milestone has passed local validation: Once implementation starts, update this section after each major milestone with what landed, what changed, and what remains. +The implementation milestone has passed local validation: + +- `cargo check -p rstest_helper_should_be_fixture --all-targets --all-features` + succeeded. +- `cargo check -p whitaker_suite --all-targets --all-features` succeeded. +- `cargo nextest run -p rstest_helper_should_be_fixture --all-targets + --all-features` passed 9 tests. +- `cargo nextest run -p whitaker_suite -p whitaker-installer --all-targets + --all-features` passed 527 tests. +- `make check-fmt` succeeded. +- `make markdownlint` succeeded with 0 Markdown errors. +- `make lint` succeeded. +- `make test` succeeded with 1428 tests passed and 2 skipped under the default + nextest profile. + +After addressing CodeRabbit's threshold, provider-fallback, and Rustdoc +findings, the final gates were rerun with the same successful outcomes. A +final CodeRabbit confirmation pass was attempted twice and was blocked by +recoverable rate limits. + ## Context and orientation Whitaker is a Rust Cargo workspace. The root `Cargo.toml` declares workspace @@ -277,11 +366,10 @@ The user-facing documentation that may need updates is: decisions if runtime behaviour differs from the design, - `docs/roadmap.md` when implementation is complete. -Useful skills for implementation are `leta`, `rust-router`, -`arch-crate-design`, `rust-errors`, `hexagonal-architecture`, -`en-gb-oxendict-style`, `commit-message`, and `pr-creation`. If later algorithm -work introduces invariants, route again through `rust-router` and consider -`kani` or `verus`. +Useful skills for implementation are `leta`, `rust-router`, `arch-crate-design`, + `rust-errors`, `hexagonal-architecture`, `en-gb-oxendict-style`, +`commit-message`, and `pr-creation`. If later algorithm work introduces +invariants, route again through `rust-router` and consider `kani` or `verus`. External references checked during planning: @@ -294,9 +382,9 @@ External references checked during planning: functions annotated with `#[fixture]` and injected as `#[rstest]` test parameters. - `rstest` test docs: - . - `#[rstest]` supports fixture injection, cases, values, files, `future`, and - `context` parameter attributes. + . `#[rstest]` supports + fixture injection, cases, values, files, `future`, and `context` parameter + attributes. ## Plan of work @@ -306,8 +394,8 @@ obtain explicit approval. Do not edit production code before approval. Stage B creates the lint crate skeleton. Add `crates/rstest_helper_should_be_fixture/Cargo.toml`, -`crates/rstest_helper_should_be_fixture/src/lib.rs`, and a driver module such -as `src/driver.rs`. The manifest should follow existing lint crates: +`crates/rstest_helper_should_be_fixture/src/lib.rs`, and a driver module such as + `src/driver.rs`. The manifest should follow existing lint crates: `crate-type = ["cdylib", "rlib"]`, `test = false`, a `dylint-driver` feature that enables `dylint_linting`, `rustc_*`, `serde`, `log`, `whitaker`, and `whitaker-common`, and a `constituent` feature that enables @@ -340,7 +428,7 @@ use_source_callee_fallback = false Keep pure validation and normalization near the config type. The driver should load configuration in `check_crate`, clamp invalid numeric thresholds to at -least `1` or reject them by falling back to defaults in the same manner as +least `2` or reject them by falling back to defaults in the same manner as existing lints, and build `RstestDetectionOptions` from the provider attributes. If a value is invalid enough that clamping would hide a user mistake, use the existing "log and default" pattern rather than panic. @@ -355,13 +443,12 @@ analysis. Stage E wires registration outside the crate. Add `rstest_helper_should_be_fixture` to `installer/src/resolution.rs` `EXPERIMENTAL_LINT_CRATES`, not to `LINT_CRATES`. Add the suite optional -dependency and a feature named `experimental-rstest-helper-should-be-fixture` -to `suite/Cargo.toml`, following the feature name that -`installer/src/builder.rs` derives from the experimental crate list. Update -`suite/src/lints.rs` and `suite/src/driver.rs` so the lint is included only -when that experimental feature is enabled. Update suite and installer tests to -expect the new experimental lint when experimental mode is on and to exclude it -otherwise. +dependency and a feature named `experimental-rstest-helper-should-be-fixture` to + `suite/Cargo.toml`, following the feature name that `installer/src/builder.rs` +derives from the experimental crate list. Update `suite/src/lints.rs` and +`suite/src/driver.rs` so the lint is included only when that experimental +feature is enabled. Update suite and installer tests to expect the new +experimental lint when experimental mode is on and to exclude it otherwise. Stage F adds tests. Unit tests should use `rstest` for `Config::default()`, valid TOML deserialization, unknown-field rejection, provider-attribute @@ -685,5 +772,5 @@ Fourth revision on 2026-05-18 after CodeRabbit review: recorded the clean change the implementation scope. Fifth revision on 2026-05-18 after draft PR creation: recorded the commit, -push, and draft PR URL in `Progress`. This is a process-tracking update with -no effect on implementation scope. +push, and draft PR URL in `Progress`. This is a process-tracking update with no +effect on implementation scope. diff --git a/docs/lints-for-rstest-fixtures-and-test-hygiene.md b/docs/lints-for-rstest-fixtures-and-test-hygiene.md index d148e218..2d023d01 100644 --- a/docs/lints-for-rstest-fixtures-and-test-hygiene.md +++ b/docs/lints-for-rstest-fixtures-and-test-hygiene.md @@ -407,6 +407,20 @@ standard lints. pin rather than the stale 0.2.x comment. That was documentation hygiene, not a behavioural change. +## Implementation decisions (8.2.1) + +- `rstest_helper_should_be_fixture` now exists as an experimental Dylint crate. + The first implementation declares `RSTEST_HELPER_SHOULD_BE_FIXTURE`, loads + and normalizes the Lint A configuration defaults, and builds the shared + `RstestDetectionOptions` policy. +- The lint is intentionally diagnostic-silent in 8.2.1. Call-site collection, + aggregation, and user-facing suggestions remain assigned to 8.2.2 through + 8.2.4 so this bootstrap does not claim analysis behaviour it does not yet + implement. +- The suite exposes the lint only through the experimental feature + `experimental-rstest-helper-should-be-fixture`. The installer derives that + feature from `EXPERIMENTAL_LINT_CRATES` when `--experimental` is enabled. + ## Implementation decisions (8.1.2) - Shared user-editable span recovery is now split between a pure diff --git a/docs/roadmap.md b/docs/roadmap.md index e22a1d62..9aa6cf11 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -470,7 +470,7 @@ ### 8.2. `rstest_helper_should_be_fixture` lint -- [ ] 8.2.1. Create the `rstest_helper_should_be_fixture` lint crate, register +- [x] 8.2.1. Create the `rstest_helper_should_be_fixture` lint crate, register `RSTEST_HELPER_SHOULD_BE_FIXTURE`, and wire configuration loading defaults. See [rstest fixture and test hygiene lints](lints-for-rstest-fixtures-and-test-hygiene.md) diff --git a/docs/users-guide.md b/docs/users-guide.md index 73479e6e..5988c95c 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -155,9 +155,9 @@ Whitaker lints are divided into two categories: false positives or undergo breaking changes between releases. They must be explicitly enabled. -The default `whitaker_suite` pattern includes only standard lints. At present, -all shipped Whitaker lints are standard and there are no experimental lints in -the release. +The default `whitaker_suite` pattern includes only standard lints. Whitaker +currently ships one experimental lint, `rstest_helper_should_be_fixture`, which +is available only when experimental lints are enabled. ### Enabling experimental lints @@ -167,7 +167,9 @@ the release. whitaker-installer --experimental ``` -This flag is retained for forward compatibility and currently has no effect. +This enables experimental suite features when building `whitaker_suite`. To +build experimental lints as individual libraries, combine it with +`--individual-lints`. ## Lint Configuration @@ -196,6 +198,14 @@ additional_test_attributes = ["actix_rt::test", "my_framework::test"] # Allow panics in main [no_unwrap_or_else_panic] allow_in_main = true + +# Experimental rstest fixture extraction lint +[rstest_helper_should_be_fixture] +min_calls = 2 +min_distinct_tests = 2 +require_identical_fixture_arg_names = false +provider_param_attributes = ["case", "values", "files", "future", "context"] +use_source_callee_fallback = false ``` ## Localized Diagnostics @@ -503,6 +513,42 @@ not in Whitaker's default list and is not listed in ______________________________________________________________________ +### `rstest_helper_should_be_fixture` + + +#### Purpose + +Bootstraps the experimental lint that will recommend converting repeated helper +calls inside `#[rstest]` tests into injected `#[fixture]` parameters. + + +#### Scope and behaviour + +This lint is experimental. The current implementation registers the lint and +loads configuration defaults so teams can opt into the forthcoming rule without +yet receiving helper-call diagnostics. Call-site collection, cross-test +aggregation, and actionable diagnostics are tracked by the later roadmap items +8.2.2 through 8.2.4. + + +#### Configuration + +```toml +[rstest_helper_should_be_fixture] +min_calls = 2 +min_distinct_tests = 2 +require_identical_fixture_arg_names = false +provider_param_attributes = ["case", "values", "files", "future", "context"] +use_source_callee_fallback = false +``` + +`provider_param_attributes` lists `rstest` parameter attributes that should be +treated as data providers rather than fixture-local bindings. Entries may be +written either as bare names such as `case` or qualified names such as +`rstest::case`; Whitaker normalizes them to the shared detection policy. + +______________________________________________________________________ + ### `test_must_not_have_example` Warns when test function documentation includes example headings (for example diff --git a/installer/src/builder.rs b/installer/src/builder.rs index d20d9681..c9ee8731 100644 --- a/installer/src/builder.rs +++ b/installer/src/builder.rs @@ -281,9 +281,10 @@ mod tests { fn features_for_crate_handles_suite_experimental_mode(mut builder: Builder) { builder.config.experimental = true; let result = builder.features_for_crate(&CrateName::from("whitaker_suite")); - // At present EXPERIMENTAL_LINT_CRATES is empty, so suite features remain - // unchanged even when experimental mode is enabled. - assert_eq!(result, "dylint-driver"); + assert_eq!( + result, + "dylint-driver,experimental-rstest-helper-should-be-fixture" + ); } #[test] diff --git a/installer/src/resolution.rs b/installer/src/resolution.rs index 74a9a770..ddbf793c 100644 --- a/installer/src/resolution.rs +++ b/installer/src/resolution.rs @@ -25,8 +25,8 @@ pub const LINT_CRATES: &[&str] = &[ /// Static list of experimental lint crates. /// /// These lints are not included in the default suite but can be enabled via -/// the `--experimental` flag. The list is currently empty. -pub const EXPERIMENTAL_LINT_CRATES: &[&str] = &[]; +/// the `--experimental` flag. +pub const EXPERIMENTAL_LINT_CRATES: &[&str] = &["rstest_helper_should_be_fixture"]; /// The aggregated suite crate name. pub const SUITE_CRATE: &str = "whitaker_suite"; @@ -112,14 +112,15 @@ mod tests { expect_lint: bool, expect_suite: bool, expect_bumpy_road: bool, + expect_experimental_lint: bool, } /// Parameterised tests for resolve_crates variants. #[rstest] - #[case::default_suite_only(ResolveCratesCase { individual_lints: false, experimental: false, expect_lint: false, expect_suite: true, expect_bumpy_road: false })] - #[case::individual_lints(ResolveCratesCase { individual_lints: true, experimental: false, expect_lint: true, expect_suite: false, expect_bumpy_road: true })] - #[case::individual_with_experimental(ResolveCratesCase { individual_lints: true, experimental: true, expect_lint: true, expect_suite: false, expect_bumpy_road: true })] - #[case::suite_with_experimental(ResolveCratesCase { individual_lints: false, experimental: true, expect_lint: false, expect_suite: true, expect_bumpy_road: false })] + #[case::default_suite_only(ResolveCratesCase { individual_lints: false, experimental: false, expect_lint: false, expect_suite: true, expect_bumpy_road: false, expect_experimental_lint: false })] + #[case::individual_lints(ResolveCratesCase { individual_lints: true, experimental: false, expect_lint: true, expect_suite: false, expect_bumpy_road: true, expect_experimental_lint: false })] + #[case::individual_with_experimental(ResolveCratesCase { individual_lints: true, experimental: true, expect_lint: true, expect_suite: false, expect_bumpy_road: true, expect_experimental_lint: true })] + #[case::suite_with_experimental(ResolveCratesCase { individual_lints: false, experimental: true, expect_lint: false, expect_suite: true, expect_bumpy_road: false, expect_experimental_lint: false })] fn resolve_crates_variants(#[case] case: ResolveCratesCase) { let options = CrateResolutionOptions { individual_lints: case.individual_lints, @@ -142,6 +143,11 @@ mod tests { case.expect_bumpy_road, "bumpy_road_function inclusion mismatch" ); + assert_eq!( + crates.contains(&CrateName::from("rstest_helper_should_be_fixture")), + case.expect_experimental_lint, + "rstest_helper_should_be_fixture inclusion mismatch" + ); } #[test] @@ -154,6 +160,7 @@ mod tests { #[rstest] #[case::valid(&["module_max_lines", "whitaker_suite"], true)] #[case::bumpy_road_function(&["bumpy_road_function"], true)] + #[case::experimental(&["rstest_helper_should_be_fixture"], true)] #[case::unknown(&["nonexistent_lint"], false)] fn validate_crate_names_variants(#[case] names: &[&str], #[case] expect_ok: bool) { let crate_names: Vec = names.iter().map(|&s| CrateName::from(s)).collect(); diff --git a/installer/tests/behaviour_core.rs b/installer/tests/behaviour_core.rs index 4f7daaf6..06a6f0c5 100644 --- a/installer/tests/behaviour_core.rs +++ b/installer/tests/behaviour_core.rs @@ -142,6 +142,7 @@ fn given_valid_names(validation_world: &ValidationWorld) { validation_world.names.replace(vec![ CrateName::from("module_max_lines"), CrateName::from("whitaker_suite"), + CrateName::from("rstest_helper_should_be_fixture"), ]); } diff --git a/suite/Cargo.toml b/suite/Cargo.toml index 9b0b593a..8dd0238c 100644 --- a/suite/Cargo.toml +++ b/suite/Cargo.toml @@ -26,6 +26,10 @@ dylint-driver = [ "dep:rustc_session", "dep:rustc_span", ] +experimental-rstest-helper-should-be-fixture = [ + "dylint-driver", + "dep:rstest_helper_should_be_fixture", +] [dependencies] dylint_linting = { workspace = true, optional = true } @@ -42,6 +46,7 @@ module_max_lines = { path = "../crates/module_max_lines", optional = true, featu no_unwrap_or_else_panic = { path = "../crates/no_unwrap_or_else_panic", optional = true, features = ["dylint-driver", "constituent"] } no_std_fs_operations = { path = "../crates/no_std_fs_operations", optional = true, features = ["dylint-driver", "constituent"] } bumpy_road_function = { path = "../crates/bumpy_road_function", optional = true, features = ["dylint-driver", "constituent"] } +rstest_helper_should_be_fixture = { path = "../crates/rstest_helper_should_be_fixture", optional = true, features = ["dylint-driver", "constituent"] } [dev-dependencies] rstest = { workspace = true } diff --git a/suite/src/driver.rs b/suite/src/driver.rs index 50a9cd5a..861b9b82 100644 --- a/suite/src/driver.rs +++ b/suite/src/driver.rs @@ -14,10 +14,13 @@ use module_must_have_inner_docs::ModuleMustHaveInnerDocs; use no_expect_outside_tests::NoExpectOutsideTests; use no_std_fs_operations::NoStdFsOperations; use no_unwrap_or_else_panic::NoUnwrapOrElsePanic; +#[cfg(feature = "experimental-rstest-helper-should-be-fixture")] +use rstest_helper_should_be_fixture::RstestHelperShouldBeFixture; use test_must_not_have_example::TestMustNotHaveExample; dylint_library!(); +#[cfg(not(feature = "experimental-rstest-helper-should-be-fixture"))] macro_rules! define_suite_pass { () => { rustc_lint::late_lint_methods!( @@ -37,6 +40,31 @@ macro_rules! define_suite_pass { }; } +#[cfg(feature = "experimental-rstest-helper-should-be-fixture")] +macro_rules! define_suite_pass { + () => { + rustc_lint::late_lint_methods!( + declare_combined_late_lint_pass, + [SuitePass, [ + FunctionAttrsFollowDocs: function_attrs_follow_docs::FunctionAttrsFollowDocs::default(), + NoExpectOutsideTests: no_expect_outside_tests::NoExpectOutsideTests::default(), + TestMustNotHaveExample: test_must_not_have_example::TestMustNotHaveExample::default(), + ModuleMustHaveInnerDocs: module_must_have_inner_docs::ModuleMustHaveInnerDocs::default(), + ConditionalMaxNBranches: conditional_max_n_branches::ConditionalMaxNBranches::default(), + ModuleMaxLines: module_max_lines::ModuleMaxLines::default(), + NoUnwrapOrElsePanic: no_unwrap_or_else_panic::NoUnwrapOrElsePanic::default(), + NoStdFsOperations: no_std_fs_operations::NoStdFsOperations::default(), + BumpyRoadFunction: bumpy_road_function::BumpyRoadFunction::default(), + RstestHelperShouldBeFixture: rstest_helper_should_be_fixture::RstestHelperShouldBeFixture::default(), + ]] + ); + }; +} + +#[cfg(not(feature = "experimental-rstest-helper-should-be-fixture"))] +define_suite_pass!(); + +#[cfg(feature = "experimental-rstest-helper-should-be-fixture")] define_suite_pass!(); /// Registers the suite lints into the provided lint store. diff --git a/suite/src/lints.rs b/suite/src/lints.rs index 1ba3aafe..b27c7774 100644 --- a/suite/src/lints.rs +++ b/suite/src/lints.rs @@ -47,6 +47,11 @@ pub const SUITE_LINTS: &[LintDescriptor] = &[ name: "bumpy_road_function", crate_name: "bumpy_road_function", }, + #[cfg(feature = "experimental-rstest-helper-should-be-fixture")] + LintDescriptor { + name: "rstest_helper_should_be_fixture", + crate_name: "rstest_helper_should_be_fixture", + }, ]; #[cfg(feature = "dylint-driver")] @@ -64,6 +69,8 @@ pub const SUITE_LINT_DECLS: &[&Lint] = &[ no_unwrap_or_else_panic::NO_UNWRAP_OR_ELSE_PANIC, no_std_fs_operations::NO_STD_FS_OPERATIONS, bumpy_road_function::BUMPY_ROAD_FUNCTION, + #[cfg(feature = "experimental-rstest-helper-should-be-fixture")] + rstest_helper_should_be_fixture::RSTEST_HELPER_SHOULD_BE_FIXTURE, ]; /// Returns an iterator over the canonical lint names in suite order. @@ -86,6 +93,8 @@ pub const SUITE_LINT_DECLS: &[&Lint] = &[ /// ] { /// assert!(names.contains(&expected)); /// } +/// #[cfg(feature = "experimental-rstest-helper-should-be-fixture")] +/// assert!(names.contains(&"rstest_helper_should_be_fixture")); /// ``` #[must_use = "Discarding the iterator hides suite wiring errors"] pub fn suite_lint_names() -> impl Iterator { From 00c3eab41d427ec335e89249897e7d3422ce2ec4 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 20 May 2026 01:59:33 +0200 Subject: [PATCH 04/21] Record rstest helper plan completion Update the ExecPlan status after pushing the implementation and refreshing the draft PR for roadmap item 8.2.1. --- ...2-1-create-the-rstest-helper-lint-crate.md | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md index b76cd80e..bdaa4861 100644 --- a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md +++ b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md @@ -4,7 +4,7 @@ This ExecPlan (execution plan) is a living document. The sections `Constraints`, `Tolerances`, `Risks`, `Progress`, `Surprises & Discoveries`, `Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work proceeds. -Status: IN PROGRESS +Status: COMPLETE ## Purpose / big picture @@ -172,7 +172,10 @@ This plan must be approved before implementation starts. - [x] (2026-05-20T00:00:00Z) Reran final gates after the Rustdoc review fix. `make check-fmt`, `make markdownlint`, `make lint`, and `make test` all passed; `make test` again reported 1428 passed and 2 skipped. -- [ ] Commit, push, and update the draft PR. +- [x] (2026-05-20T00:00:00Z) Committed the implementation as `813d2cc`, + pushed it to `origin/8-2-1-create-the-rstest-helper-lint-crate`, and updated + draft PR for implementation + review. ## Surprises & discoveries @@ -257,12 +260,13 @@ This plan must be approved before implementation starts. ## Outcomes & retrospective -Implementation has landed in the working tree and is awaiting final review, -commit, push, and PR update. The shipped behaviour is a bootstrap: the -experimental Dylint crate exists, the lint declaration and configuration -defaults are wired, suite and installer registration understand the -experimental lint, and documentation states that diagnostics follow later -roadmap items. +Implementation has landed on branch +`8-2-1-create-the-rstest-helper-lint-crate` and draft PR + is updated for review. The +shipped behaviour is a bootstrap: the experimental Dylint crate exists, the +lint declaration and configuration defaults are wired, suite and installer +registration understand the experimental lint, and documentation states that +diagnostics follow later roadmap items. The plan milestone has passed local validation: @@ -273,9 +277,6 @@ The plan milestone has passed local validation: nextest profile. - `coderabbit review --agent` completed with 0 findings after review fixes. -Once implementation starts, update this section after each major milestone with -what landed, what changed, and what remains. - The implementation milestone has passed local validation: - `cargo check -p rstest_helper_should_be_fixture --all-targets --all-features` From 94975451d6b3bdbd49fd585160385630ea47b369 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 20 May 2026 02:00:21 +0200 Subject: [PATCH 05/21] Record final CodeRabbit rate limit Update the ExecPlan with the final post-push CodeRabbit confirmation attempt, which remained blocked by a recoverable service rate limit. --- ...2-1-create-the-rstest-helper-lint-crate.md | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md index bdaa4861..a32626dc 100644 --- a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md +++ b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md @@ -176,6 +176,8 @@ This plan must be approved before implementation starts. pushed it to `origin/8-2-1-create-the-rstest-helper-lint-crate`, and updated draft PR for implementation review. +- [x] (2026-05-20T00:00:00Z) Attempted one final CodeRabbit confirmation on + the pushed branch. It was still blocked by a recoverable rate-limit error. ## Surprises & discoveries @@ -224,12 +226,13 @@ This plan must be approved before implementation starts. CodeRabbit finding on `Config`. Impact: `Config` now documents that it is loaded from `dylint.toml` and normalized before use. -- Observation: A final confirmation rerun of `coderabbit review --agent` was +- Observation: Final confirmation reruns of `coderabbit review --agent` were blocked by CodeRabbit rate limiting after the Rustdoc fix. Evidence: two - retry attempts returned recoverable rate-limit errors. Impact: all reported - CodeRabbit findings were fixed and local gates were rerun, but the tool could - not provide a zero-finding confirmation after the last documentation-only - code comment change. + retry attempts before the implementation commit and one retry after the final + push returned recoverable rate-limit errors. Impact: all reported CodeRabbit + findings were fixed and local gates were rerun, but the tool could not + provide a zero-finding confirmation after the last documentation-only code + comment change. ## Decision log @@ -293,9 +296,9 @@ The implementation milestone has passed local validation: nextest profile. After addressing CodeRabbit's threshold, provider-fallback, and Rustdoc -findings, the final gates were rerun with the same successful outcomes. A -final CodeRabbit confirmation pass was attempted twice and was blocked by -recoverable rate limits. +findings, the final gates were rerun with the same successful outcomes. Final +CodeRabbit confirmation passes were attempted before and after the final push, +but were blocked by recoverable rate limits. ## Context and orientation From 7e10a16402a958fc0193e19e767a282ecd9339ab Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 20 May 2026 13:38:54 +0200 Subject: [PATCH 06/21] Require opt-in for explicit experimental lint builds Validate explicit `--lint` requests with the experimental option context so experimental crates cannot bypass the documented `--experimental` gate. Add core and CLI behaviour coverage for accepted opt-in requests and rejected non-opt-in requests, and document the user-facing installer contract. --- ...2-1-create-the-rstest-helper-lint-crate.md | 33 ++++++++++++ docs/users-guide.md | 4 +- installer/src/error.rs | 10 ++++ installer/src/main.rs | 8 +-- installer/src/resolution.rs | 47 ++++++++++++---- installer/tests/behaviour_cli.rs | 15 +++++- installer/tests/behaviour_cli/scenarios.rs | 5 ++ installer/tests/behaviour_cli/support.rs | 32 +++++++++++ installer/tests/behaviour_core.rs | 53 +++++++++++++++++-- installer/tests/features/installer.feature | 17 ++++++ 10 files changed, 203 insertions(+), 21 deletions(-) diff --git a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md index a32626dc..be95b431 100644 --- a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md +++ b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md @@ -178,6 +178,12 @@ This plan must be approved before implementation starts. review. - [x] (2026-05-20T00:00:00Z) Attempted one final CodeRabbit confirmation on the pushed branch. It was still blocked by a recoverable rate-limit error. +- [x] (2026-05-20T00:00:00Z) Fixed the post-review installer policy issue + where explicit `--lint rstest_helper_should_be_fixture` requests could bypass + the documented `--experimental` opt-in gate. +- [x] (2026-05-20T00:00:00Z) Validated the opt-in fix with focused + installer tests, the required repository gates, and `coderabbit review + --agent`, which completed with 0 findings. ## Surprises & discoveries @@ -234,6 +240,13 @@ This plan must be approved before implementation starts. provide a zero-finding confirmation after the last documentation-only code comment change. +- Observation: `validate_crate_names` needs option context, not just catalogue + membership. Evidence: adding `rstest_helper_should_be_fixture` to + `EXPERIMENTAL_LINT_CRATES` made explicit + `--lint rstest_helper_should_be_fixture` valid without `--experimental`. + Impact: validation now receives `CrateResolutionOptions` and rejects + experimental explicit lint requests unless the opt-in flag is set. + ## Decision log - Decision: Keep this task to scaffolding, registration, and configuration @@ -300,6 +313,26 @@ findings, the final gates were rerun with the same successful outcomes. Final CodeRabbit confirmation passes were attempted before and after the final push, but were blocked by recoverable rate limits. +Post-review installer policy feedback was addressed by requiring +`--experimental` for explicit experimental lint builds. The resolution unit +tests and installer BDD coverage now cover both the accepted opt-in path and +the rejected non-opt-in path. + +The opt-in fix passed: + +- `cargo nextest run -p whitaker-installer --all-targets --all-features + resolution::tests:: validate_crate_names_variants + dry_run_rejects_experimental_lint_without_opt_in` with 10 selected tests. +- `cargo nextest run -p whitaker-installer --all-targets --all-features -E + 'binary(behaviour_cli) + binary(behaviour_core)'` with 12 selected core BDD + tests. +- `cargo nextest run -p whitaker-installer --all-targets --all-features + --profile ci -E + 'test(scenario_dry_run_rejects_experimental_lint_without_opt_in)'` with the + excluded CLI BDD scenario. +- `make check-fmt`, `make markdownlint`, `make lint`, and `make test`. +- `coderabbit review --agent` with 0 findings. + ## Context and orientation Whitaker is a Rust Cargo workspace. The root `Cargo.toml` declares workspace diff --git a/docs/users-guide.md b/docs/users-guide.md index 5988c95c..46f76a71 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -169,7 +169,9 @@ whitaker-installer --experimental This enables experimental suite features when building `whitaker_suite`. To build experimental lints as individual libraries, combine it with -`--individual-lints`. +`--individual-lints`. Explicit `--lint` requests for experimental lints also +require `--experimental`; without that opt-in the installer rejects the request +before building anything. ## Lint Configuration diff --git a/installer/src/error.rs b/installer/src/error.rs index fd40e964..321d67a5 100644 --- a/installer/src/error.rs +++ b/installer/src/error.rs @@ -91,6 +91,13 @@ pub enum InstallerError { name: CrateName, }, + /// An experimental lint crate was requested without explicit opt-in. + #[error("experimental lint crate {name} requires --experimental")] + ExperimentalLintRequiresFlag { + /// Name of the experimental lint crate. + name: CrateName, + }, + /// The workspace root could not be found. #[error("workspace not found: {reason}")] WorkspaceNotFound { @@ -214,6 +221,9 @@ impl Clone for InstallerError { InstallerError::LintCrateNotFound { name } => { InstallerError::LintCrateNotFound { name: name.clone() } } + InstallerError::ExperimentalLintRequiresFlag { name } => { + InstallerError::ExperimentalLintRequiresFlag { name: name.clone() } + } InstallerError::WorkspaceNotFound { reason } => InstallerError::WorkspaceNotFound { reason: reason.clone(), }, diff --git a/installer/src/main.rs b/installer/src/main.rs index 84f15137..d3d51250 100644 --- a/installer/src/main.rs +++ b/installer/src/main.rs @@ -333,14 +333,14 @@ fn resolve_requested_crates(args: &InstallArgs) -> Result> { .map(|name| CrateName::from(name.as_str())) .collect(); - if !lint_crates.is_empty() { - validate_crate_names(&lint_crates)?; - } - let options = CrateResolutionOptions { individual_lints: args.individual_lints, experimental: args.experimental, }; + if !lint_crates.is_empty() { + validate_crate_names(&lint_crates, &options)?; + } + Ok(resolve_crates(&lint_crates, &options)) } diff --git a/installer/src/resolution.rs b/installer/src/resolution.rs index ddbf793c..08b7c891 100644 --- a/installer/src/resolution.rs +++ b/installer/src/resolution.rs @@ -51,8 +51,9 @@ pub struct CrateResolutionOptions { /// `EXPERIMENTAL_LINT_CRATES` are included in the returned crate list. /// - In suite mode (default), the `experimental` flag is used by `BuildConfig` /// to enable experimental features when building the suite crate. -/// - When `specific_lints` are provided, experimental crates are accepted if -/// present, but the `experimental` flag does not affect the returned list. +/// - When `specific_lints` are provided, the returned list is exactly the +/// requested crate list after validation. Experimental crates still require +/// the `experimental` flag during validation. /// /// Note: This function assumes that `specific_lints` have been validated via /// `validate_crate_names()` prior to calling. Callers must validate inputs @@ -86,16 +87,26 @@ pub fn is_known_crate(name: &CrateName) -> bool { LINT_CRATES.contains(&s) || EXPERIMENTAL_LINT_CRATES.contains(&s) || s == SUITE_CRATE } +/// Check whether a crate name identifies an experimental lint crate. +#[must_use] +pub fn is_experimental_crate(name: &CrateName) -> bool { + EXPERIMENTAL_LINT_CRATES.contains(&name.as_str()) +} + /// Validate that all specified crate names are known lint crates. /// /// # Errors /// -/// Returns an error if any crate name is not recognised. -pub fn validate_crate_names(names: &[CrateName]) -> Result<()> { +/// Returns an error if any crate name is not recognised, or if an experimental +/// crate is requested without `--experimental`. +pub fn validate_crate_names(names: &[CrateName], options: &CrateResolutionOptions) -> Result<()> { for name in names { if !is_known_crate(name) { return Err(InstallerError::LintCrateNotFound { name: name.clone() }); } + if is_experimental_crate(name) && !options.experimental { + return Err(InstallerError::ExperimentalLintRequiresFlag { name: name.clone() }); + } } Ok(()) } @@ -158,19 +169,33 @@ mod tests { } #[rstest] - #[case::valid(&["module_max_lines", "whitaker_suite"], true)] - #[case::bumpy_road_function(&["bumpy_road_function"], true)] - #[case::experimental(&["rstest_helper_should_be_fixture"], true)] - #[case::unknown(&["nonexistent_lint"], false)] - fn validate_crate_names_variants(#[case] names: &[&str], #[case] expect_ok: bool) { + #[case::valid(&["module_max_lines", "whitaker_suite"], false, true)] + #[case::bumpy_road_function(&["bumpy_road_function"], false, true)] + #[case::experimental_with_flag(&["rstest_helper_should_be_fixture"], true, true)] + #[case::experimental_without_flag(&["rstest_helper_should_be_fixture"], false, false)] + #[case::unknown(&["nonexistent_lint"], false, false)] + fn validate_crate_names_variants( + #[case] names: &[&str], + #[case] experimental: bool, + #[case] expect_ok: bool, + ) { let crate_names: Vec = names.iter().map(|&s| CrateName::from(s)).collect(); - let res = validate_crate_names(&crate_names); + let options = CrateResolutionOptions { + experimental, + ..CrateResolutionOptions::default() + }; + let res = validate_crate_names(&crate_names, &options); if expect_ok { assert!(res.is_ok()); } else { let err = res.expect_err("expected validation failure"); assert!( - matches!(&err, InstallerError::LintCrateNotFound { name } if *name == crate_names[0]), + matches!( + &err, + InstallerError::LintCrateNotFound { name } + | InstallerError::ExperimentalLintRequiresFlag { name } + if *name == crate_names[0] + ), "unexpected error: {err:?}" ); } diff --git a/installer/tests/behaviour_cli.rs b/installer/tests/behaviour_cli.rs index 0445e07a..aaa3482a 100644 --- a/installer/tests/behaviour_cli.rs +++ b/installer/tests/behaviour_cli.rs @@ -14,8 +14,9 @@ use std::process::Command; pub(crate) use support::{CliWorld, cli_world}; use support::{ assert_cli_exits_successfully, assert_cli_exits_with_error, assert_dry_run_output_is_shown, - assert_installation_succeeds_or_is_skipped, assert_suite_library_is_staged, - assert_unknown_lint_message_is_shown, configure_dry_run_unknown_lint, + assert_experimental_lint_opt_in_message_is_shown, assert_installation_succeeds_or_is_skipped, + assert_suite_library_is_staged, assert_unknown_lint_message_is_shown, + configure_dry_run_experimental_lint, configure_dry_run_unknown_lint, configure_dry_run_with_target_dir, configure_suite_install, is_toolchain_installed, pinned_toolchain_channel, run_installer_cli, workspace_root, }; @@ -30,6 +31,11 @@ fn given_dry_run_unknown_lint(cli_world: &CliWorld) { configure_dry_run_unknown_lint(cli_world); } +#[given("the installer is invoked with dry-run and an experimental lint")] +fn given_dry_run_experimental_lint(cli_world: &CliWorld) { + configure_dry_run_experimental_lint(cli_world); +} + #[given("the installer is invoked to a temporary directory")] fn given_suite_install(cli_world: &CliWorld) { configure_suite_install(cli_world); @@ -60,6 +66,11 @@ fn then_unknown_lint_message_is_shown(cli_world: &CliWorld) { assert_unknown_lint_message_is_shown(cli_world); } +#[then("an experimental lint opt-in message is shown")] +fn then_experimental_lint_opt_in_message_is_shown(cli_world: &CliWorld) { + assert_experimental_lint_opt_in_message_is_shown(cli_world); +} + #[then("installation succeeds or is skipped")] fn then_installation_succeeds_or_is_skipped(cli_world: &CliWorld) { assert_installation_succeeds_or_is_skipped(cli_world); diff --git a/installer/tests/behaviour_cli/scenarios.rs b/installer/tests/behaviour_cli/scenarios.rs index e6c3a757..ca65531a 100644 --- a/installer/tests/behaviour_cli/scenarios.rs +++ b/installer/tests/behaviour_cli/scenarios.rs @@ -19,3 +19,8 @@ fn scenario_dry_run_rejects_unknown_lint(cli_world: CliWorld) { fn scenario_install_suite_to_temp_dir(cli_world: CliWorld) { let _ = cli_world; } + +#[scenario(path = "tests/features/installer.feature", index = 21)] +fn scenario_dry_run_rejects_experimental_lint_without_opt_in(cli_world: CliWorld) { + let _ = cli_world; +} diff --git a/installer/tests/behaviour_cli/support.rs b/installer/tests/behaviour_cli/support.rs index 5dd82f85..052e2dfb 100644 --- a/installer/tests/behaviour_cli/support.rs +++ b/installer/tests/behaviour_cli/support.rs @@ -152,6 +152,14 @@ pub(super) fn configure_dry_run_unknown_lint(cli_world: &CliWorld) { ]); } +pub(super) fn configure_dry_run_experimental_lint(cli_world: &CliWorld) { + cli_world.args.replace(vec![ + "--dry-run".to_owned(), + "--lint".to_owned(), + "rstest_helper_should_be_fixture".to_owned(), + ]); +} + pub(super) fn configure_suite_install(cli_world: &CliWorld) { let Some(_channel) = ensure_required_toolchain_available(cli_world) else { return; @@ -262,6 +270,30 @@ pub(super) fn assert_unknown_lint_message_is_shown(cli_world: &CliWorld) { ); } +pub(super) fn assert_experimental_lint_opt_in_message_is_shown(cli_world: &CliWorld) { + if cli_world.skip_assertions.get() { + return; + } + + let output = get_output(cli_world); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert!( + !stderr.contains("Dry run - no files will be modified"), + "dry-run configuration output should not be printed on experimental-lint error, stderr: {stderr}" + ); + assert!( + !stderr.contains("Crates to build:"), + "dry-run configuration output should not be printed on experimental-lint error, stderr: {stderr}" + ); + assert!( + stderr.contains( + "experimental lint crate rstest_helper_should_be_fixture requires --experimental" + ), + "unexpected stderr: {stderr}" + ); +} + pub(super) fn assert_installation_succeeds_or_is_skipped(cli_world: &CliWorld) { if cli_world.skip_assertions.get() { return; diff --git a/installer/tests/behaviour_core.rs b/installer/tests/behaviour_core.rs index 06a6f0c5..e8567d8a 100644 --- a/installer/tests/behaviour_core.rs +++ b/installer/tests/behaviour_core.rs @@ -129,7 +129,9 @@ fn then_only_requested(crate_world: &CrateResolutionWorld) { #[derive(Default)] struct ValidationWorld { names: RefCell>, + experimental: Cell, result: Cell>, + error_message: RefCell>, } #[fixture] @@ -142,10 +144,21 @@ fn given_valid_names(validation_world: &ValidationWorld) { validation_world.names.replace(vec![ CrateName::from("module_max_lines"), CrateName::from("whitaker_suite"), - CrateName::from("rstest_helper_should_be_fixture"), ]); } +#[given("a list containing an experimental crate name")] +fn given_experimental_name(validation_world: &ValidationWorld) { + validation_world + .names + .replace(vec![CrateName::from("rstest_helper_should_be_fixture")]); +} + +#[given("experimental validation is enabled")] +fn given_experimental_validation_enabled(validation_world: &ValidationWorld) { + validation_world.experimental.set(true); +} + #[given("a list containing an unknown crate name")] fn given_unknown_name(validation_world: &ValidationWorld) { validation_world @@ -156,8 +169,19 @@ fn given_unknown_name(validation_world: &ValidationWorld) { #[when("the names are validated")] fn when_names_validated(validation_world: &ValidationWorld) { let names = validation_world.names.borrow(); - let result = validate_crate_names(&names).is_ok(); - validation_world.result.set(Some(result)); + let options = CrateResolutionOptions { + experimental: validation_world.experimental.get(), + ..CrateResolutionOptions::default() + }; + match validate_crate_names(&names, &options) { + Ok(()) => validation_world.result.set(Some(true)), + Err(error) => { + validation_world + .error_message + .replace(Some(error.to_string())); + validation_world.result.set(Some(false)); + } + } } #[then("validation succeeds")] @@ -170,6 +194,19 @@ fn then_validation_fails(validation_world: &ValidationWorld) { assert_eq!(validation_world.result.get(), Some(false)); } +#[then("validation fails with an experimental opt-in error")] +fn then_validation_fails_experimental_opt_in(validation_world: &ValidationWorld) { + assert_eq!(validation_world.result.get(), Some(false)); + let error_message = validation_world.error_message.borrow(); + let error_message = error_message.as_ref().expect("error message should be set"); + assert!( + error_message.contains( + "experimental lint crate rstest_helper_should_be_fixture requires --experimental" + ), + "unexpected validation error: {error_message}" + ); +} + // --------------------------------------------------------------------------- // Toolchain detection world // --------------------------------------------------------------------------- @@ -343,3 +380,13 @@ fn scenario_reject_missing_channel(toolchain_world: ToolchainWorld) { fn scenario_generate_shell_snippets(snippet_world: SnippetWorld) { let _ = snippet_world; } + +#[scenario(path = "tests/features/installer.feature", index = 19)] +fn scenario_validate_experimental_names_with_opt_in(validation_world: ValidationWorld) { + let _ = validation_world; +} + +#[scenario(path = "tests/features/installer.feature", index = 20)] +fn scenario_reject_experimental_names_without_opt_in(validation_world: ValidationWorld) { + let _ = validation_world; +} diff --git a/installer/tests/features/installer.feature b/installer/tests/features/installer.feature index 78c4107e..68bca562 100644 --- a/installer/tests/features/installer.feature +++ b/installer/tests/features/installer.feature @@ -111,3 +111,20 @@ Feature: Whitaker lint library installer When the installer CLI is run Then installation succeeds or is skipped And output includes DYLINT_LIBRARY_PATH instructions + + Scenario: Validate experimental crate names with opt-in + Given a list containing an experimental crate name + And experimental validation is enabled + When the names are validated + Then validation succeeds + + Scenario: Reject experimental crate names without opt-in + Given a list containing an experimental crate name + When the names are validated + Then validation fails with an experimental opt-in error + + Scenario: Dry-run rejects experimental lint without opt-in + Given the installer is invoked with dry-run and an experimental lint + When the installer CLI is run + Then the CLI exits with an error + And an experimental lint opt-in message is shown From 3819acf408f21cb4d776ab061bcc950e001c3e7c Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 20 May 2026 13:54:07 +0200 Subject: [PATCH 07/21] Resolve rstest helper review comments Fix the still-valid inline review comments for the experimental rstest helper lint bootstrap. Add module documentation for the disabled-driver stub, derive the suite experimental feature expectation from `EXPERIMENTAL_LINT_CRATES`, and fill in the users-guide and design-document documentation gaps. Record the verification outcome in the ExecPlan, including the stale historical test-count comment that was intentionally left unchanged. --- .../src/lib.rs | 7 +++++ ...2-1-create-the-rstest-helper-lint-crate.md | 17 ++++++++++ ...ts-for-rstest-fixtures-and-test-hygiene.md | 28 ++++++++--------- docs/users-guide.md | 31 +++++++++++++++++++ installer/src/builder.rs | 10 ++++-- 5 files changed, 76 insertions(+), 17 deletions(-) diff --git a/crates/rstest_helper_should_be_fixture/src/lib.rs b/crates/rstest_helper_should_be_fixture/src/lib.rs index cf05dad2..1734ebd4 100644 --- a/crates/rstest_helper_should_be_fixture/src/lib.rs +++ b/crates/rstest_helper_should_be_fixture/src/lib.rs @@ -15,6 +15,13 @@ pub use driver::*; #[cfg(not(feature = "dylint-driver"))] mod stub { + //! Disabled-driver stub module. + //! + //! This module keeps non-driver builds valid when the `dylint-driver` + //! feature is off. It exposes + //! `rstest_helper_should_be_fixture_disabled_stub` as the inert public + //! symbol for that build mode. + #[expect(dead_code, reason = "stub used when the driver feature is disabled")] pub fn rstest_helper_should_be_fixture_disabled_stub() {} } diff --git a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md index be95b431..b90fc5c4 100644 --- a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md +++ b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md @@ -184,6 +184,14 @@ This plan must be approved before implementation starts. - [x] (2026-05-20T00:00:00Z) Validated the opt-in fix with focused installer tests, the required repository gates, and `coderabbit review --agent`, which completed with 0 findings. +- [x] (2026-05-20T00:00:00Z) Verified inline review comments with Wyvern and + Scribe agents, fixed the still-valid stub documentation, design-document + ordering, users-guide subsection, and builder expectation issues, and skipped + the stale historical ExecPlan count. +- [x] (2026-05-20T00:00:00Z) Validated the inline-review follow-up with + focused tests, `make check-fmt`, `make markdownlint`, `make lint`, + `make test`, and `coderabbit review --agent`, which completed with + 0 findings. ## Surprises & discoveries @@ -247,6 +255,15 @@ This plan must be approved before implementation starts. Impact: validation now receives `CrateResolutionOptions` and rejects experimental explicit lint requests unless the opt-in flag is set. +- Observation: One inline review comment targeted a stale builder test name, + but the same hard-coded expectation still existed in the nearby suite + experimental feature test. Evidence: the Wyvern verification found + `experimental_features_derives_from_experimental_lint_crates` already derives + from `EXPERIMENTAL_LINT_CRATES`, while + `features_for_crate_handles_suite_experimental_mode` still used a literal + feature string. Impact: the latter test now derives its expected value from + `EXPERIMENTAL_LINT_CRATES`. + ## Decision log - Decision: Keep this task to scaffolding, registration, and configuration diff --git a/docs/lints-for-rstest-fixtures-and-test-hygiene.md b/docs/lints-for-rstest-fixtures-and-test-hygiene.md index 2d023d01..04ee865d 100644 --- a/docs/lints-for-rstest-fixtures-and-test-hygiene.md +++ b/docs/lints-for-rstest-fixtures-and-test-hygiene.md @@ -407,20 +407,6 @@ standard lints. pin rather than the stale 0.2.x comment. That was documentation hygiene, not a behavioural change. -## Implementation decisions (8.2.1) - -- `rstest_helper_should_be_fixture` now exists as an experimental Dylint crate. - The first implementation declares `RSTEST_HELPER_SHOULD_BE_FIXTURE`, loads - and normalizes the Lint A configuration defaults, and builds the shared - `RstestDetectionOptions` policy. -- The lint is intentionally diagnostic-silent in 8.2.1. Call-site collection, - aggregation, and user-facing suggestions remain assigned to 8.2.2 through - 8.2.4 so this bootstrap does not claim analysis behaviour it does not yet - implement. -- The suite exposes the lint only through the experimental feature - `experimental-rstest-helper-should-be-fixture`. The installer derives that - feature from `EXPERIMENTAL_LINT_CRATES` when `--experimental` is enabled. - ## Implementation decisions (8.1.2) - Shared user-editable span recovery is now split between a pure @@ -443,3 +429,17 @@ standard lints. exercise the frame-selection algorithm directly, and `rstest-bdd` 0.5.x scenarios in `common/tests/rstest_span_recovery_behaviour.rs` describe the direct, recovered, first-match, and macro-only outcomes in user terms. + +## Implementation decisions (8.2.1) + +- `rstest_helper_should_be_fixture` now exists as an experimental Dylint crate. + The first implementation declares `RSTEST_HELPER_SHOULD_BE_FIXTURE`, loads + and normalizes the Lint A configuration defaults, and builds the shared + `RstestDetectionOptions` policy. +- The lint is intentionally diagnostic-silent in 8.2.1. Call-site collection, + aggregation, and user-facing suggestions remain assigned to 8.2.2 through + 8.2.4 so this bootstrap does not claim analysis behaviour it does not yet + implement. +- The suite exposes the lint only through the experimental feature + `experimental-rstest-helper-should-be-fixture`. The installer derives that + feature from `EXPERIMENTAL_LINT_CRATES` when `--experimental` is enabled. diff --git a/docs/users-guide.md b/docs/users-guide.md index 46f76a71..52040ad8 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -532,6 +532,37 @@ yet receiving helper-call diagnostics. Call-site collection, cross-test aggregation, and actionable diagnostics are tracked by the later roadmap items 8.2.2 through 8.2.4. + +#### What is allowed + +- Single-use helper calls inside `#[rstest]` tests +- Helper calls whose totals stay below `min_calls` or `min_distinct_tests` +- Parameter-provider uses covered by `provider_param_attributes`, such as + `case`, `values`, `files`, `future`, and `context` + + +#### What is denied + +When diagnostic phases are implemented, `rstest_helper_should_be_fixture` will +deny repeated non-provider helper invocations across `#[rstest]` tests when +they meet or exceed both `min_calls` and `min_distinct_tests`. The +`require_identical_fixture_arg_names` setting controls whether candidate +fixture arguments must use the same names, and `use_source_callee_fallback` +controls whether source-callsite recovery may be used for macro-expanded +callee locations. + + +#### How to fix + +- Replace repeated helper calls with a shared `#[fixture]` parameter. +- If `require_identical_fixture_arg_names` is enabled, rename helper arguments + so repeated calls use the same fixture argument names. +- Prefer provider attributes listed in `provider_param_attributes` for + parameterized data inputs rather than modelling them as fixture-local helper + calls. +- Tune `min_calls`, `min_distinct_tests`, and `use_source_callee_fallback` when + repository conventions need stricter or looser matching. + #### Configuration diff --git a/installer/src/builder.rs b/installer/src/builder.rs index c9ee8731..e84e4235 100644 --- a/installer/src/builder.rs +++ b/installer/src/builder.rs @@ -281,10 +281,14 @@ mod tests { fn features_for_crate_handles_suite_experimental_mode(mut builder: Builder) { builder.config.experimental = true; let result = builder.features_for_crate(&CrateName::from("whitaker_suite")); - assert_eq!( - result, - "dylint-driver,experimental-rstest-helper-should-be-fixture" + let mut expected_features = vec!["dylint-driver".to_owned()]; + expected_features.extend( + EXPERIMENTAL_LINT_CRATES + .iter() + .map(|&lint| format!("experimental-{}", lint.replace('_', "-"))), ); + let expected = expected_features.join(","); + assert_eq!(result, expected); } #[test] From 3e756237c4e9735f605f630ccff1bf8283621949 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 21 May 2026 22:39:15 +0200 Subject: [PATCH 08/21] Correct rstest helper plan test count Update the ExecPlan validation summary so the plan milestone uses the same 1428 passed test count as the surrounding recorded validation notes. --- docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md index b90fc5c4..23986899 100644 --- a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md +++ b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md @@ -306,7 +306,7 @@ The plan milestone has passed local validation: - `make check-fmt` succeeded. - `make markdownlint` succeeded with 0 Markdown errors. - `make lint` succeeded. -- `make test` succeeded with 1418 tests passed and 2 skipped under the default +- `make test` succeeded with 1428 tests passed and 2 skipped under the default nextest profile. - `coderabbit review --agent` completed with 0 findings after review fixes. From f3ed0ca6d81c6ae86b3bf6f99f717525cf371c0a Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 21 May 2026 22:44:13 +0200 Subject: [PATCH 09/21] Cover experimental lint dry-run opt-in Add CLI behaviour coverage for explicitly requesting the experimental rstest helper lint with `--experimental` enabled. The suite macro redefinition comment was already addressed by complementary cfg gates, so record that verification in the ExecPlan without changing the suite driver. --- ...2-1-create-the-rstest-helper-lint-crate.md | 4 ++ installer/tests/behaviour_cli.rs | 19 ++++++++-- installer/tests/behaviour_cli/scenarios.rs | 5 +++ installer/tests/behaviour_cli/support.rs | 37 +++++++++++++++++++ installer/tests/features/installer.feature | 6 +++ 5 files changed, 68 insertions(+), 3 deletions(-) diff --git a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md index 23986899..781018a9 100644 --- a/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md +++ b/docs/execplans/8-2-1-create-the-rstest-helper-lint-crate.md @@ -192,6 +192,10 @@ This plan must be approved before implementation starts. focused tests, `make check-fmt`, `make markdownlint`, `make lint`, `make test`, and `coderabbit review --agent`, which completed with 0 findings. +- [x] (2026-05-21T00:00:00Z) Verified review comments about suite macro + definition and experimental lint CLI coverage. The macro redefinition concern + was stale because the two macro definitions already use complementary `cfg` + gates. The CLI success-path coverage request was valid and was implemented. ## Surprises & discoveries diff --git a/installer/tests/behaviour_cli.rs b/installer/tests/behaviour_cli.rs index aaa3482a..0bd1f57c 100644 --- a/installer/tests/behaviour_cli.rs +++ b/installer/tests/behaviour_cli.rs @@ -14,11 +14,12 @@ use std::process::Command; pub(crate) use support::{CliWorld, cli_world}; use support::{ assert_cli_exits_successfully, assert_cli_exits_with_error, assert_dry_run_output_is_shown, + assert_experimental_lint_dry_run_output_is_shown, assert_experimental_lint_opt_in_message_is_shown, assert_installation_succeeds_or_is_skipped, assert_suite_library_is_staged, assert_unknown_lint_message_is_shown, - configure_dry_run_experimental_lint, configure_dry_run_unknown_lint, - configure_dry_run_with_target_dir, configure_suite_install, is_toolchain_installed, - pinned_toolchain_channel, run_installer_cli, workspace_root, + configure_dry_run_experimental_lint, configure_dry_run_experimental_lint_with_opt_in, + configure_dry_run_unknown_lint, configure_dry_run_with_target_dir, configure_suite_install, + is_toolchain_installed, pinned_toolchain_channel, run_installer_cli, workspace_root, }; #[given("the installer is invoked with dry-run and a target directory")] @@ -36,6 +37,13 @@ fn given_dry_run_experimental_lint(cli_world: &CliWorld) { configure_dry_run_experimental_lint(cli_world); } +#[given( + "the installer is invoked with dry-run, an experimental lint, and experimental lints enabled" +)] +fn given_dry_run_experimental_lint_with_opt_in(cli_world: &CliWorld) { + configure_dry_run_experimental_lint_with_opt_in(cli_world); +} + #[given("the installer is invoked to a temporary directory")] fn given_suite_install(cli_world: &CliWorld) { configure_suite_install(cli_world); @@ -71,6 +79,11 @@ fn then_experimental_lint_opt_in_message_is_shown(cli_world: &CliWorld) { assert_experimental_lint_opt_in_message_is_shown(cli_world); } +#[then("experimental lint dry-run output is shown")] +fn then_experimental_lint_dry_run_output_is_shown(cli_world: &CliWorld) { + assert_experimental_lint_dry_run_output_is_shown(cli_world); +} + #[then("installation succeeds or is skipped")] fn then_installation_succeeds_or_is_skipped(cli_world: &CliWorld) { assert_installation_succeeds_or_is_skipped(cli_world); diff --git a/installer/tests/behaviour_cli/scenarios.rs b/installer/tests/behaviour_cli/scenarios.rs index ca65531a..c95291d8 100644 --- a/installer/tests/behaviour_cli/scenarios.rs +++ b/installer/tests/behaviour_cli/scenarios.rs @@ -24,3 +24,8 @@ fn scenario_install_suite_to_temp_dir(cli_world: CliWorld) { fn scenario_dry_run_rejects_experimental_lint_without_opt_in(cli_world: CliWorld) { let _ = cli_world; } + +#[scenario(path = "tests/features/installer.feature", index = 22)] +fn scenario_dry_run_accepts_experimental_lint_with_opt_in(cli_world: CliWorld) { + let _ = cli_world; +} diff --git a/installer/tests/behaviour_cli/support.rs b/installer/tests/behaviour_cli/support.rs index 052e2dfb..1bae4eac 100644 --- a/installer/tests/behaviour_cli/support.rs +++ b/installer/tests/behaviour_cli/support.rs @@ -160,6 +160,24 @@ pub(super) fn configure_dry_run_experimental_lint(cli_world: &CliWorld) { ]); } +pub(super) fn configure_dry_run_experimental_lint_with_opt_in(cli_world: &CliWorld) { + let Some(channel) = ensure_required_toolchain_available(cli_world) else { + return; + }; + + let target_dir = setup_temp_dir(cli_world); + cli_world.args.replace(vec![ + "--dry-run".to_owned(), + "--experimental".to_owned(), + "--toolchain".to_owned(), + channel, + "--target-dir".to_owned(), + target_dir, + "--lint".to_owned(), + "rstest_helper_should_be_fixture".to_owned(), + ]); +} + pub(super) fn configure_suite_install(cli_world: &CliWorld) { let Some(_channel) = ensure_required_toolchain_available(cli_world) else { return; @@ -294,6 +312,25 @@ pub(super) fn assert_experimental_lint_opt_in_message_is_shown(cli_world: &CliWo ); } +pub(super) fn assert_experimental_lint_dry_run_output_is_shown(cli_world: &CliWorld) { + if cli_world.skip_assertions.get() { + return; + } + + let output = get_output(cli_world); + let stderr = String::from_utf8_lossy(&output.stderr); + + assert!(stderr.contains("Dry run - no files will be modified")); + assert!(stderr.contains("Crates to build:")); + assert!(stderr.contains("rstest_helper_should_be_fixture")); + assert!( + !stderr.contains( + "experimental lint crate rstest_helper_should_be_fixture requires --experimental" + ), + "experimental opt-in error should not be printed when --experimental is set, stderr: {stderr}" + ); +} + pub(super) fn assert_installation_succeeds_or_is_skipped(cli_world: &CliWorld) { if cli_world.skip_assertions.get() { return; diff --git a/installer/tests/features/installer.feature b/installer/tests/features/installer.feature index 68bca562..b0b3626a 100644 --- a/installer/tests/features/installer.feature +++ b/installer/tests/features/installer.feature @@ -128,3 +128,9 @@ Feature: Whitaker lint library installer When the installer CLI is run Then the CLI exits with an error And an experimental lint opt-in message is shown + + Scenario: Dry-run accepts experimental lint with opt-in + Given the installer is invoked with dry-run, an experimental lint, and experimental lints enabled + When the installer CLI is run + Then the CLI exits successfully + And experimental lint dry-run output is shown From 962ca60d19cfd7b7ca391d838505523e71c2046a Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 22 May 2026 17:19:05 +0200 Subject: [PATCH 10/21] Refactor installer error cloning Extract toolchain-specific clone handling and shared I/O error cloning from `InstallerError::clone` so the implementation satisfies the local method length threshold without changing clone behaviour. --- installer/src/error.rs | 120 ++++++++++++++++++++++------------------- 1 file changed, 64 insertions(+), 56 deletions(-) diff --git a/installer/src/error.rs b/installer/src/error.rs index 321d67a5..84e9a8f0 100644 --- a/installer/src/error.rs +++ b/installer/src/error.rs @@ -165,94 +165,102 @@ pub enum InstallerError { }, } -impl Clone for InstallerError { - fn clone(&self) -> Self { - // Helper to clone io::Error by preserving ErrorKind and formatted message. - // This is lossy: the original source chain is discarded. - fn clone_io_error(source: &std::io::Error) -> std::io::Error { - std::io::Error::new(source.kind(), source.to_string()) - } +// Helper to clone io::Error by preserving ErrorKind and formatted message. +// This is lossy: the original source chain is discarded. +fn clone_io_error(source: &std::io::Error) -> std::io::Error { + std::io::Error::new(source.kind(), source.to_string()) +} +impl InstallerError { + fn clone_toolchain_variant(&self) -> Option { match self { - InstallerError::ToolchainDetection { reason } => InstallerError::ToolchainDetection { + Self::ToolchainDetection { reason } => Some(Self::ToolchainDetection { reason: reason.clone(), - }, - InstallerError::ToolchainFileNotFound { path } => { - InstallerError::ToolchainFileNotFound { path: path.clone() } - } - InstallerError::InvalidToolchainFile { reason } => { - InstallerError::InvalidToolchainFile { - reason: reason.clone(), - } - } - InstallerError::ToolchainNotInstalled { toolchain } => { - InstallerError::ToolchainNotInstalled { - toolchain: toolchain.clone(), - } + }), + Self::ToolchainFileNotFound { path } => { + Some(Self::ToolchainFileNotFound { path: path.clone() }) } - InstallerError::ToolchainInstallFailed { toolchain, message } => { - InstallerError::ToolchainInstallFailed { + Self::InvalidToolchainFile { reason } => Some(Self::InvalidToolchainFile { + reason: reason.clone(), + }), + Self::ToolchainNotInstalled { toolchain } => Some(Self::ToolchainNotInstalled { + toolchain: toolchain.clone(), + }), + Self::ToolchainInstallFailed { toolchain, message } => { + Some(Self::ToolchainInstallFailed { toolchain: toolchain.clone(), message: message.clone(), - } + }) } - InstallerError::ToolchainComponentInstallFailed { + Self::ToolchainComponentInstallFailed { toolchain, components, message, - } => InstallerError::ToolchainComponentInstallFailed { + } => Some(Self::ToolchainComponentInstallFailed { toolchain: toolchain.clone(), components: components.clone(), message: message.clone(), - }, - InstallerError::BuildFailed { crate_name, reason } => InstallerError::BuildFailed { + }), + _ => None, + } + } +} + +impl Clone for InstallerError { + fn clone(&self) -> Self { + if let Some(value) = self.clone_toolchain_variant() { + return value; + } + + match self { + Self::BuildFailed { crate_name, reason } => Self::BuildFailed { crate_name: crate_name.clone(), reason: reason.clone(), }, - InstallerError::StagingFailed { reason } => InstallerError::StagingFailed { + Self::StagingFailed { reason } => Self::StagingFailed { reason: reason.clone(), }, - InstallerError::TargetNotWritable { path, reason } => { - InstallerError::TargetNotWritable { - path: path.clone(), - reason: reason.clone(), - } - } - InstallerError::LintCrateNotFound { name } => { - InstallerError::LintCrateNotFound { name: name.clone() } - } - InstallerError::ExperimentalLintRequiresFlag { name } => { - InstallerError::ExperimentalLintRequiresFlag { name: name.clone() } + Self::TargetNotWritable { path, reason } => Self::TargetNotWritable { + path: path.clone(), + reason: reason.clone(), + }, + Self::LintCrateNotFound { name } => Self::LintCrateNotFound { name: name.clone() }, + Self::ExperimentalLintRequiresFlag { name } => { + Self::ExperimentalLintRequiresFlag { name: name.clone() } } - InstallerError::WorkspaceNotFound { reason } => InstallerError::WorkspaceNotFound { + Self::WorkspaceNotFound { reason } => Self::WorkspaceNotFound { reason: reason.clone(), }, - InstallerError::InvalidCargoToml { path, reason } => InstallerError::InvalidCargoToml { + Self::InvalidCargoToml { path, reason } => Self::InvalidCargoToml { path: path.clone(), reason: reason.clone(), }, - InstallerError::Io(source) => InstallerError::Io(clone_io_error(source)), - InstallerError::Git { operation, message } => InstallerError::Git { + Self::Io(source) => Self::Io(clone_io_error(source)), + Self::Git { operation, message } => Self::Git { operation, message: message.clone(), }, - InstallerError::DependencyInstall { tool, message } => { - InstallerError::DependencyInstall { - tool, - message: message.clone(), - } - } - InstallerError::WrapperGeneration(message) => { - InstallerError::WrapperGeneration(message.clone()) - } - InstallerError::ScanFailed { source } => InstallerError::ScanFailed { + Self::DependencyInstall { tool, message } => Self::DependencyInstall { + tool, + message: message.clone(), + }, + Self::WrapperGeneration(message) => Self::WrapperGeneration(message.clone()), + Self::ScanFailed { source } => Self::ScanFailed { source: clone_io_error(source), }, - InstallerError::WriteFailed { source } => InstallerError::WriteFailed { + Self::WriteFailed { source } => Self::WriteFailed { source: clone_io_error(source), }, + Self::ToolchainDetection { .. } + | Self::ToolchainFileNotFound { .. } + | Self::InvalidToolchainFile { .. } + | Self::ToolchainNotInstalled { .. } + | Self::ToolchainInstallFailed { .. } + | Self::ToolchainComponentInstallFailed { .. } => { + unreachable!("handled by clone_toolchain_variant") + } #[cfg(any(test, feature = "test-support"))] - InstallerError::StubMismatch { message } => InstallerError::StubMismatch { + Self::StubMismatch { message } => Self::StubMismatch { message: message.clone(), }, } From d0d97919af35f49b13a23061b0e07d335f5a94fa Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 22 May 2026 17:27:58 +0200 Subject: [PATCH 11/21] Cover rstest helper configuration bootstrap Add focused tests around `rstest_helper_should_be_fixture` configuration loading, pass initialisation, and normalisation idempotence. Clarify the README lint count and experimental opt-in signpost so the public summary matches the current lint set. --- Cargo.lock | 1 + README.md | 11 ++- .../Cargo.toml | 1 + .../src/driver.rs | 93 ++++++++++++++++++- 4 files changed, 98 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 01eea015..271f724b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2314,6 +2314,7 @@ version = "0.2.5" dependencies = [ "dylint_linting", "log", + "proptest", "rstest", "rstest-bdd", "rstest-bdd-macros", diff --git a/README.md b/README.md index 02f42337..4d96e122 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,8 @@ For version pinning, installation details, and configuration options, see the ## The Lints -Whitaker currently ships eight lints, with more on the way: +Whitaker currently ships nine standard lints plus one experimental lint that +requires explicit opt-in. | Lint | What it does | | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------- | @@ -48,6 +49,10 @@ Whitaker currently ships eight lints, with more on the way: | `no_unwrap_or_else_panic` | Catches sneaky panics hidden inside `unwrap_or_else` closures. If you're going to panic, at least be upfront about it. | | `no_std_fs_operations` | Forbids `std::fs` operations, nudging you toward capability-based filesystem access via `cap_std`. | +Experimental lints are not enabled by default. The current experimental lint is +`rstest_helper_should_be_fixture`, which is available only when installer and +suite flows opt in with `--experimental` or the corresponding suite feature. + ## Features - **Localised diagnostics** — Messages available in English, Welsh (Cymraeg), @@ -59,8 +64,8 @@ Whitaker currently ships eight lints, with more on the way: ## Project Status -Whitaker is under active development. One additional lint -(`public_fn_must_have_docs`) is planned—see the [roadmap](docs/roadmap.md) for +Whitaker is under active development. One additional lint ( +`public_fn_must_have_docs`) is planned—see the [roadmap](docs/roadmap.md) for details. ## Verifying Release Checksums diff --git a/crates/rstest_helper_should_be_fixture/Cargo.toml b/crates/rstest_helper_should_be_fixture/Cargo.toml index 0c6972b1..75e340b9 100644 --- a/crates/rstest_helper_should_be_fixture/Cargo.toml +++ b/crates/rstest_helper_should_be_fixture/Cargo.toml @@ -36,6 +36,7 @@ serde = { workspace = true, optional = true } whitaker = { workspace = true, features = ["dylint-driver"], optional = true } [dev-dependencies] +proptest = "1" rstest = { workspace = true } rstest-bdd = { workspace = true } rstest-bdd-macros = { workspace = true } diff --git a/crates/rstest_helper_should_be_fixture/src/driver.rs b/crates/rstest_helper_should_be_fixture/src/driver.rs index 6e4a0385..95b1b7cd 100644 --- a/crates/rstest_helper_should_be_fixture/src/driver.rs +++ b/crates/rstest_helper_should_be_fixture/src/driver.rs @@ -100,15 +100,20 @@ impl Default for RstestHelperShouldBeFixture { } } -impl<'tcx> LateLintPass<'tcx> for RstestHelperShouldBeFixture { - fn check_crate(&mut self, _cx: &LateContext<'tcx>) { - self.config = load_configuration(); +impl RstestHelperShouldBeFixture { + fn apply_crate_configuration(&mut self, config: Config, shared_config: SharedConfig) { + self.config = config; self.detection_options = self.config.detection_options(); - let shared_config = SharedConfig::load(); self.localizer = get_localizer_for_lint(LINT_NAME, shared_config.locale()); } } +impl<'tcx> LateLintPass<'tcx> for RstestHelperShouldBeFixture { + fn check_crate(&mut self, _cx: &LateContext<'tcx>) { + self.apply_crate_configuration(load_configuration(), SharedConfig::load()); + } +} + fn normalize_provider_attributes(attributes: Vec) -> Vec { let normalized: Vec = attributes .into_iter() @@ -131,7 +136,14 @@ fn default_provider_param_attributes() -> Vec { } fn load_configuration() -> Config { - match dylint_linting::config::(LINT_NAME) { + loaded_configuration_or_default(dylint_linting::config::(LINT_NAME)) +} + +fn loaded_configuration_or_default(loaded: Result, E>) -> Config +where + E: std::fmt::Display, +{ + match loaded { Ok(Some(config)) => config.normalized(), Ok(None) => Config::default(), Err(error) => { @@ -147,6 +159,7 @@ fn load_configuration() -> Config { #[cfg(test)] mod tests { use super::*; + use proptest::prelude::*; use rstest::rstest; #[rstest] @@ -246,4 +259,74 @@ mod tests { DEFAULT_PROVIDER_PARAM_ATTRIBUTES.len() * 2 ); } + + #[rstest] + fn loaded_configuration_uses_default_when_config_is_absent() { + assert_eq!( + loaded_configuration_or_default::(Ok(None)), + Config::default() + ); + } + + #[rstest] + fn loaded_configuration_uses_default_when_config_errors() { + assert_eq!( + loaded_configuration_or_default(Err("invalid config")), + Config::default() + ); + } + + #[rstest] + fn loaded_configuration_normalizes_present_config() { + let config = Config { + min_calls: 1, + min_distinct_tests: 1, + provider_param_attributes: vec!["rstest::case".to_string()], + ..Config::default() + }; + + assert_eq!( + loaded_configuration_or_default::(Ok(Some(config))).provider_param_attributes, + ["case"] + ); + } + + #[rstest] + fn applying_crate_configuration_initializes_pass_state() { + let mut pass = RstestHelperShouldBeFixture::default(); + let config = Config { + provider_param_attributes: vec!["custom".to_string()], + use_source_callee_fallback: true, + ..Config::default() + }; + + pass.apply_crate_configuration(config.clone(), SharedConfig::default()); + + assert_eq!(pass.config, config); + assert!(pass.detection_options.use_expansion_trace_fallback()); + assert_eq!(pass.detection_options.provider_param_attributes().len(), 2); + } + + proptest! { + #[test] + fn normalized_configuration_is_idempotent( + min_calls in 0usize..8, + min_distinct_tests in 0usize..8, + require_identical_fixture_arg_names in any::(), + provider_param_attributes in prop::collection::vec("[ a-z:]{0,24}", 0..8), + use_source_callee_fallback in any::(), + ) { + let config = Config { + min_calls, + min_distinct_tests, + require_identical_fixture_arg_names, + provider_param_attributes, + use_source_callee_fallback, + }; + let once = config.normalized(); + let twice = once.clone().normalized(); + + prop_assert_eq!(once, twice); + } + } } From 11a0ab2740378a18574629f4d3c897fcc98a3e2f Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 22 May 2026 17:53:12 +0200 Subject: [PATCH 12/21] Consolidate suite pass macro wiring Keep a single `define_suite_pass!` macro and pass the experimental rstest helper lint entry only from the feature-gated call site. This avoids duplicate macro definitions while preserving the existing feature-gated suite contents. --- suite/src/driver.rs | 29 +++++------------------------ 1 file changed, 5 insertions(+), 24 deletions(-) diff --git a/suite/src/driver.rs b/suite/src/driver.rs index 861b9b82..daaccc52 100644 --- a/suite/src/driver.rs +++ b/suite/src/driver.rs @@ -20,9 +20,8 @@ use test_must_not_have_example::TestMustNotHaveExample; dylint_library!(); -#[cfg(not(feature = "experimental-rstest-helper-should-be-fixture"))] macro_rules! define_suite_pass { - () => { + ($($experimental_pass:tt)*) => { rustc_lint::late_lint_methods!( declare_combined_late_lint_pass, [SuitePass, [ @@ -35,27 +34,7 @@ macro_rules! define_suite_pass { NoUnwrapOrElsePanic: no_unwrap_or_else_panic::NoUnwrapOrElsePanic::default(), NoStdFsOperations: no_std_fs_operations::NoStdFsOperations::default(), BumpyRoadFunction: bumpy_road_function::BumpyRoadFunction::default(), - ]] - ); - }; -} - -#[cfg(feature = "experimental-rstest-helper-should-be-fixture")] -macro_rules! define_suite_pass { - () => { - rustc_lint::late_lint_methods!( - declare_combined_late_lint_pass, - [SuitePass, [ - FunctionAttrsFollowDocs: function_attrs_follow_docs::FunctionAttrsFollowDocs::default(), - NoExpectOutsideTests: no_expect_outside_tests::NoExpectOutsideTests::default(), - TestMustNotHaveExample: test_must_not_have_example::TestMustNotHaveExample::default(), - ModuleMustHaveInnerDocs: module_must_have_inner_docs::ModuleMustHaveInnerDocs::default(), - ConditionalMaxNBranches: conditional_max_n_branches::ConditionalMaxNBranches::default(), - ModuleMaxLines: module_max_lines::ModuleMaxLines::default(), - NoUnwrapOrElsePanic: no_unwrap_or_else_panic::NoUnwrapOrElsePanic::default(), - NoStdFsOperations: no_std_fs_operations::NoStdFsOperations::default(), - BumpyRoadFunction: bumpy_road_function::BumpyRoadFunction::default(), - RstestHelperShouldBeFixture: rstest_helper_should_be_fixture::RstestHelperShouldBeFixture::default(), + $($experimental_pass)* ]] ); }; @@ -65,7 +44,9 @@ macro_rules! define_suite_pass { define_suite_pass!(); #[cfg(feature = "experimental-rstest-helper-should-be-fixture")] -define_suite_pass!(); +define_suite_pass!( + RstestHelperShouldBeFixture: rstest_helper_should_be_fixture::RstestHelperShouldBeFixture::default(), +); /// Registers the suite lints into the provided lint store. /// From e60fec12dc9d50edbaac0de656af837df7435d44 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 22 May 2026 18:13:50 +0200 Subject: [PATCH 13/21] Resolve rstest helper review nits Move the rstest helper proptest dependency to the workspace table, tidy the README project status wording, and fix the rstest helper documentation order and grammar noted in review. --- Cargo.toml | 1 + README.md | 4 +-- .../Cargo.toml | 2 +- ...ts-for-rstest-fixtures-and-test-hygiene.md | 2 +- docs/users-guide.md | 34 +++++++++---------- 5 files changed, 22 insertions(+), 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index d4788b03..79270c2f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ zip = "2" rstest = "0.26.1" rstest-bdd = "0.5.0" rstest-bdd-macros = "0.5.0" +proptest = "1" rustc_lexer = "0.1.0" whitaker_clones_core = { path = "crates/whitaker_clones_core", version = "0.2.5" } whitaker_sarif = { path = "crates/whitaker_sarif", version = "0.2.5" } diff --git a/README.md b/README.md index 4d96e122..9df6fbda 100644 --- a/README.md +++ b/README.md @@ -64,8 +64,8 @@ suite flows opt in with `--experimental` or the corresponding suite feature. ## Project Status -Whitaker is under active development. One additional lint ( -`public_fn_must_have_docs`) is planned—see the [roadmap](docs/roadmap.md) for +Whitaker is under active development. One additional lint +(`public_fn_must_have_docs`) is planned—see the [roadmap](docs/roadmap.md) for details. ## Verifying Release Checksums diff --git a/crates/rstest_helper_should_be_fixture/Cargo.toml b/crates/rstest_helper_should_be_fixture/Cargo.toml index 75e340b9..535178d0 100644 --- a/crates/rstest_helper_should_be_fixture/Cargo.toml +++ b/crates/rstest_helper_should_be_fixture/Cargo.toml @@ -36,7 +36,7 @@ serde = { workspace = true, optional = true } whitaker = { workspace = true, features = ["dylint-driver"], optional = true } [dev-dependencies] -proptest = "1" +proptest = { workspace = true } rstest = { workspace = true } rstest-bdd = { workspace = true } rstest-bdd-macros = { workspace = true } diff --git a/docs/lints-for-rstest-fixtures-and-test-hygiene.md b/docs/lints-for-rstest-fixtures-and-test-hygiene.md index 04ee865d..3e3c52bd 100644 --- a/docs/lints-for-rstest-fixtures-and-test-hygiene.md +++ b/docs/lints-for-rstest-fixtures-and-test-hygiene.md @@ -438,7 +438,7 @@ standard lints. `RstestDetectionOptions` policy. - The lint is intentionally diagnostic-silent in 8.2.1. Call-site collection, aggregation, and user-facing suggestions remain assigned to 8.2.2 through - 8.2.4 so this bootstrap does not claim analysis behaviour it does not yet + 8.2.4, so this bootstrap does not claim analysis behaviour it does not yet implement. - The suite exposes the lint only through the experimental feature `experimental-rstest-helper-should-be-fixture`. The installer derives that diff --git a/docs/users-guide.md b/docs/users-guide.md index 52040ad8..a7752ecc 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -532,6 +532,23 @@ yet receiving helper-call diagnostics. Call-site collection, cross-test aggregation, and actionable diagnostics are tracked by the later roadmap items 8.2.2 through 8.2.4. + +#### Configuration + +```toml +[rstest_helper_should_be_fixture] +min_calls = 2 +min_distinct_tests = 2 +require_identical_fixture_arg_names = false +provider_param_attributes = ["case", "values", "files", "future", "context"] +use_source_callee_fallback = false +``` + +`provider_param_attributes` lists `rstest` parameter attributes that should be +treated as data providers rather than fixture-local bindings. Entries may be +written either as bare names such as `case` or qualified names such as +`rstest::case`; Whitaker normalizes them to the shared detection policy. + #### What is allowed @@ -563,23 +580,6 @@ callee locations. - Tune `min_calls`, `min_distinct_tests`, and `use_source_callee_fallback` when repository conventions need stricter or looser matching. - -#### Configuration - -```toml -[rstest_helper_should_be_fixture] -min_calls = 2 -min_distinct_tests = 2 -require_identical_fixture_arg_names = false -provider_param_attributes = ["case", "values", "files", "future", "context"] -use_source_callee_fallback = false -``` - -`provider_param_attributes` lists `rstest` parameter attributes that should be -treated as data providers rather than fixture-local bindings. Entries may be -written either as bare names such as `case` or qualified names such as -`rstest::case`; Whitaker normalizes them to the shared detection policy. - ______________________________________________________________________ ### `test_must_not_have_example` From 4cfe3ba6deb7df026fb927dc7b366e0687b279bb Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 22 May 2026 19:47:00 +0200 Subject: [PATCH 14/21] Expose rstest helper config load failures Return a `Result` from the rstest helper lint configuration boundary so parse failures are visible before the driver falls back to defaults. Document the driver unit-test module now that it covers configuration loading and `rstest` detection option construction. --- .../src/driver.rs | 50 ++++++++++++------- 1 file changed, 31 insertions(+), 19 deletions(-) diff --git a/crates/rstest_helper_should_be_fixture/src/driver.rs b/crates/rstest_helper_should_be_fixture/src/driver.rs index 95b1b7cd..643b304f 100644 --- a/crates/rstest_helper_should_be_fixture/src/driver.rs +++ b/crates/rstest_helper_should_be_fixture/src/driver.rs @@ -17,6 +17,8 @@ const LINT_NAME: &str = "rstest_helper_should_be_fixture"; const DEFAULT_PROVIDER_PARAM_ATTRIBUTES: &[&str] = &["case", "values", "files", "future", "context"]; +type ConfigLoadResult = Result; + dylint_linting::impl_late_lint! { pub RSTEST_HELPER_SHOULD_BE_FIXTURE, Warn, @@ -110,7 +112,18 @@ impl RstestHelperShouldBeFixture { impl<'tcx> LateLintPass<'tcx> for RstestHelperShouldBeFixture { fn check_crate(&mut self, _cx: &LateContext<'tcx>) { - self.apply_crate_configuration(load_configuration(), SharedConfig::load()); + let config = match load_configuration() { + Ok(config) => config, + Err(error) => { + debug!( + target: LINT_NAME, + "failed to parse `{LINT_NAME}` configuration: {error}; using defaults" + ); + Config::default() + } + }; + + self.apply_crate_configuration(config, SharedConfig::load()); } } @@ -135,29 +148,26 @@ fn default_provider_param_attributes() -> Vec { .collect() } -fn load_configuration() -> Config { - loaded_configuration_or_default(dylint_linting::config::(LINT_NAME)) +fn load_configuration() -> ConfigLoadResult { + loaded_configuration(dylint_linting::config::(LINT_NAME)) } -fn loaded_configuration_or_default(loaded: Result, E>) -> Config +fn loaded_configuration(loaded: Result, E>) -> ConfigLoadResult where E: std::fmt::Display, { match loaded { - Ok(Some(config)) => config.normalized(), - Ok(None) => Config::default(), - Err(error) => { - debug!( - target: LINT_NAME, - "failed to parse `{LINT_NAME}` configuration: {error}; using defaults" - ); - Config::default() - } + Ok(Some(config)) => Ok(config.normalized()), + Ok(None) => Ok(Config::default()), + Err(error) => Err(error.to_string()), } } #[cfg(test)] mod tests { + //! Unit tests for driver configuration normalization, loading boundaries, + //! and `rstest` detection option construction. + use super::*; use proptest::prelude::*; use rstest::rstest; @@ -263,16 +273,16 @@ mod tests { #[rstest] fn loaded_configuration_uses_default_when_config_is_absent() { assert_eq!( - loaded_configuration_or_default::(Ok(None)), - Config::default() + loaded_configuration::(Ok(None)).expect("missing config should default"), + Config::default(), ); } #[rstest] - fn loaded_configuration_uses_default_when_config_errors() { + fn loaded_configuration_returns_error_when_config_errors() { assert_eq!( - loaded_configuration_or_default(Err("invalid config")), - Config::default() + loaded_configuration(Err("invalid config")).expect_err("invalid config should error"), + "invalid config", ); } @@ -286,7 +296,9 @@ mod tests { }; assert_eq!( - loaded_configuration_or_default::(Ok(Some(config))).provider_param_attributes, + loaded_configuration::(Ok(Some(config))) + .expect("present config should load") + .provider_param_attributes, ["case"] ); } From f33392efffa641c75d46d6d047b6b7c903c9195f Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 22 May 2026 21:17:13 +0200 Subject: [PATCH 15/21] Name rstest helper shared config boundary Route `SharedConfig::load` through a private helper so the Dylint driver has an explicit boundary for shared configuration loading. Document that the current shared loader is treated as infallible at this call site until a follow-up makes I/O failures visible. --- crates/rstest_helper_should_be_fixture/src/driver.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/rstest_helper_should_be_fixture/src/driver.rs b/crates/rstest_helper_should_be_fixture/src/driver.rs index 643b304f..88a13b1b 100644 --- a/crates/rstest_helper_should_be_fixture/src/driver.rs +++ b/crates/rstest_helper_should_be_fixture/src/driver.rs @@ -123,7 +123,7 @@ impl<'tcx> LateLintPass<'tcx> for RstestHelperShouldBeFixture { } }; - self.apply_crate_configuration(config, SharedConfig::load()); + self.apply_crate_configuration(config, load_shared_config()); } } @@ -152,6 +152,13 @@ fn load_configuration() -> ConfigLoadResult { loaded_configuration(dylint_linting::config::(LINT_NAME)) } +fn load_shared_config() -> SharedConfig { + // SAFETY / NOTE: `SharedConfig::load` does not currently propagate I/O + // errors, so this named boundary documents the infallible call site + // pending . + SharedConfig::load() +} + fn loaded_configuration(loaded: Result, E>) -> ConfigLoadResult where E: std::fmt::Display, @@ -167,6 +174,9 @@ where mod tests { //! Unit tests for driver configuration normalization, loading boundaries, //! and `rstest` detection option construction. + //! + //! NOTE: `SharedConfig::load` is treated as infallible at the driver call + //! site pending . use super::*; use proptest::prelude::*; From 108311361409043fe306c2689aa7c0ac2ae1cf39 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 22 May 2026 22:11:08 +0200 Subject: [PATCH 16/21] Link rstest helper shared config follow-up Replace the placeholder follow-up reference in the shared configuration boundary notes with the tracked GitHub issue. --- crates/rstest_helper_should_be_fixture/src/driver.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/rstest_helper_should_be_fixture/src/driver.rs b/crates/rstest_helper_should_be_fixture/src/driver.rs index 88a13b1b..951b9146 100644 --- a/crates/rstest_helper_should_be_fixture/src/driver.rs +++ b/crates/rstest_helper_should_be_fixture/src/driver.rs @@ -155,7 +155,7 @@ fn load_configuration() -> ConfigLoadResult { fn load_shared_config() -> SharedConfig { // SAFETY / NOTE: `SharedConfig::load` does not currently propagate I/O // errors, so this named boundary documents the infallible call site - // pending . + // pending https://github.com/leynos/whitaker/issues/233. SharedConfig::load() } @@ -176,7 +176,7 @@ mod tests { //! and `rstest` detection option construction. //! //! NOTE: `SharedConfig::load` is treated as infallible at the driver call - //! site pending . + //! site pending https://github.com/leynos/whitaker/issues/233. use super::*; use proptest::prelude::*; From b44fc5b9a38b4f8a5075954e904fd3b5fdfec97a Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 23 May 2026 14:08:22 +0200 Subject: [PATCH 17/21] Cover rstest helper bootstrap observability Add a trybuild smoke harness for the experimental rstest helper lint crate so its bootstrap phase has compile-time coverage. Record debug-level decisions for lint configuration loading, shared configuration loading, state updates, and experimental crate resolution decisions. --- Cargo.lock | 46 +++++++++++++++++++ Cargo.toml | 1 + .../Cargo.toml | 1 + .../src/driver.rs | 24 +++++++++- .../tests/ui.rs | 11 +++++ .../tests/ui/bootstrap_zero_diagnostic.rs | 3 ++ installer/src/resolution.rs | 34 ++++++++++++++ 7 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 crates/rstest_helper_should_be_fixture/tests/ui.rs create mode 100644 crates/rstest_helper_should_be_fixture/tests/ui/bootstrap_zero_diagnostic.rs diff --git a/Cargo.lock b/Cargo.lock index 271f724b..248a2d1f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2322,6 +2322,7 @@ dependencies = [ "rustc_session", "serde", "toml 0.9.12+spec-1.1.0", + "trybuild", "whitaker", "whitaker-common", ] @@ -2773,6 +2774,12 @@ dependencies = [ "xattr", ] +[[package]] +name = "target-triple" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "591ef38edfb78ca4771ee32cf494cb8771944bee237a9b91fc9c1424ac4b777b" + [[package]] name = "temp-env" version = "0.3.6" @@ -2806,6 +2813,15 @@ dependencies = [ "winapi", ] +[[package]] +name = "termcolor" +version = "1.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "06794f8f6c5c898b3275aebefa6b8a1cb24cd2c6c79397ab15774837a0bc5755" +dependencies = [ + "winapi-util", +] + [[package]] name = "termtree" version = "0.5.1" @@ -2970,6 +2986,21 @@ dependencies = [ "winnow 0.7.15", ] +[[package]] +name = "toml" +version = "1.1.2+spec-1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "81f3d15e84cbcd896376e6730314d59fb5a87f31e4b038454184435cd57defee" +dependencies = [ + "indexmap", + "serde_core", + "serde_spanned", + "toml_datetime 1.1.1+spec-1.1.0", + "toml_parser", + "toml_writer", + "winnow 1.0.1", +] + [[package]] name = "toml_datetime" version = "0.7.5+spec-1.1.0" @@ -3034,6 +3065,21 @@ dependencies = [ "once_cell", ] +[[package]] +name = "trybuild" +version = "1.0.116" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "47c635f0191bd3a2941013e5062667100969f8c4e9cd787c14f977265d73616e" +dependencies = [ + "glob", + "serde", + "serde_derive", + "serde_json", + "target-triple", + "termcolor", + "toml 1.1.2+spec-1.1.0", +] + [[package]] name = "type-map" version = "0.5.1" diff --git a/Cargo.toml b/Cargo.toml index 79270c2f..33c07fbd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -28,6 +28,7 @@ flate2 = "1.1" serde_json = "1.0.149" toml = "0.9.8" thiserror = "2" +trybuild = "1.0.116" log = "0.4.28" sha2 = "0.10.9" tar = "0.4.44" diff --git a/crates/rstest_helper_should_be_fixture/Cargo.toml b/crates/rstest_helper_should_be_fixture/Cargo.toml index 535178d0..50ba7a15 100644 --- a/crates/rstest_helper_should_be_fixture/Cargo.toml +++ b/crates/rstest_helper_should_be_fixture/Cargo.toml @@ -41,3 +41,4 @@ rstest = { workspace = true } rstest-bdd = { workspace = true } rstest-bdd-macros = { workspace = true } toml = { workspace = true } +trybuild = { workspace = true } diff --git a/crates/rstest_helper_should_be_fixture/src/driver.rs b/crates/rstest_helper_should_be_fixture/src/driver.rs index 951b9146..d512df31 100644 --- a/crates/rstest_helper_should_be_fixture/src/driver.rs +++ b/crates/rstest_helper_should_be_fixture/src/driver.rs @@ -104,6 +104,18 @@ impl Default for RstestHelperShouldBeFixture { impl RstestHelperShouldBeFixture { fn apply_crate_configuration(&mut self, config: Config, shared_config: SharedConfig) { + debug!( + target: LINT_NAME, + "applying `{LINT_NAME}` configuration: min_calls={}, min_distinct_tests={}, \ + require_identical_fixture_arg_names={}, provider_param_attributes={:?}, \ + use_source_callee_fallback={}, locale={:?}", + config.min_calls, + config.min_distinct_tests, + config.require_identical_fixture_arg_names, + config.provider_param_attributes, + config.use_source_callee_fallback, + shared_config.locale(), + ); self.config = config; self.detection_options = self.config.detection_options(); self.localizer = get_localizer_for_lint(LINT_NAME, shared_config.locale()); @@ -149,6 +161,7 @@ fn default_provider_param_attributes() -> Vec { } fn load_configuration() -> ConfigLoadResult { + debug!(target: LINT_NAME, "loading `{LINT_NAME}` configuration"); loaded_configuration(dylint_linting::config::(LINT_NAME)) } @@ -156,6 +169,7 @@ fn load_shared_config() -> SharedConfig { // SAFETY / NOTE: `SharedConfig::load` does not currently propagate I/O // errors, so this named boundary documents the infallible call site // pending https://github.com/leynos/whitaker/issues/233. + debug!(target: LINT_NAME, "loading shared Whitaker configuration"); SharedConfig::load() } @@ -164,8 +178,14 @@ where E: std::fmt::Display, { match loaded { - Ok(Some(config)) => Ok(config.normalized()), - Ok(None) => Ok(Config::default()), + Ok(Some(config)) => { + debug!(target: LINT_NAME, "loaded explicit `{LINT_NAME}` configuration"); + Ok(config.normalized()) + } + Ok(None) => { + debug!(target: LINT_NAME, "no `{LINT_NAME}` configuration found; using defaults"); + Ok(Config::default()) + } Err(error) => Err(error.to_string()), } } diff --git a/crates/rstest_helper_should_be_fixture/tests/ui.rs b/crates/rstest_helper_should_be_fixture/tests/ui.rs new file mode 100644 index 00000000..277bd698 --- /dev/null +++ b/crates/rstest_helper_should_be_fixture/tests/ui.rs @@ -0,0 +1,11 @@ +//! Compile-time UI smoke tests for the `rstest_helper_should_be_fixture` +//! bootstrap crate. +//! +//! These trybuild cases intentionally expect no diagnostics while the lint is +//! in its registration and configuration-loading phase. + +#[test] +fn bootstrap_fixtures_compile_without_diagnostics() { + let cases = trybuild::TestCases::new(); + cases.pass("tests/ui/bootstrap_zero_diagnostic.rs"); +} diff --git a/crates/rstest_helper_should_be_fixture/tests/ui/bootstrap_zero_diagnostic.rs b/crates/rstest_helper_should_be_fixture/tests/ui/bootstrap_zero_diagnostic.rs new file mode 100644 index 00000000..0935f41a --- /dev/null +++ b/crates/rstest_helper_should_be_fixture/tests/ui/bootstrap_zero_diagnostic.rs @@ -0,0 +1,3 @@ +//! Zero-diagnostic fixture for the `rstest_helper_should_be_fixture` bootstrap. + +fn main() {} diff --git a/installer/src/resolution.rs b/installer/src/resolution.rs index 08b7c891..077ad59a 100644 --- a/installer/src/resolution.rs +++ b/installer/src/resolution.rs @@ -5,6 +5,7 @@ use crate::crate_name::CrateName; use crate::error::{InstallerError, Result}; +use log::debug; /// Static list of lint crates available for building. /// @@ -65,18 +66,36 @@ pub fn resolve_crates( ) -> Vec { if !specific_lints.is_empty() { // Assumes names have been validated via validate_crate_names(). + debug!( + target: "whitaker_installer::resolution", + "using explicit lint crate selection: {:?}", + specific_lints + ); return specific_lints.to_vec(); } if options.individual_lints { let mut crates: Vec = LINT_CRATES.iter().map(|&c| CrateName::from(c)).collect(); if options.experimental { + debug!( + target: "whitaker_installer::resolution", + "including experimental lint crates because --experimental is enabled" + ); crates.extend(EXPERIMENTAL_LINT_CRATES.iter().map(|&c| CrateName::from(c))); + } else { + debug!( + target: "whitaker_installer::resolution", + "excluding experimental lint crates because --experimental is disabled" + ); } return crates; } // Default: suite only (experimental feature is handled by BuildConfig) + debug!( + target: "whitaker_installer::resolution", + "using suite crate selection; experimental feature handling is deferred to build config" + ); vec![CrateName::from(SUITE_CRATE)] } @@ -102,11 +121,26 @@ pub fn is_experimental_crate(name: &CrateName) -> bool { pub fn validate_crate_names(names: &[CrateName], options: &CrateResolutionOptions) -> Result<()> { for name in names { if !is_known_crate(name) { + debug!( + target: "whitaker_installer::resolution", + "rejecting unknown lint crate `{}`", + name.as_str() + ); return Err(InstallerError::LintCrateNotFound { name: name.clone() }); } if is_experimental_crate(name) && !options.experimental { + debug!( + target: "whitaker_installer::resolution", + "rejecting experimental lint crate `{}` because --experimental is disabled", + name.as_str() + ); return Err(InstallerError::ExperimentalLintRequiresFlag { name: name.clone() }); } + debug!( + target: "whitaker_installer::resolution", + "accepted lint crate `{}` for resolution", + name.as_str() + ); } Ok(()) } From dfcd590f16d2aea618356d9e5395d7af85c27e0b Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 23 May 2026 17:25:20 +0200 Subject: [PATCH 18/21] Document installer crate resolution internals Add the installer architecture notes for `CrateResolutionOptions`, the experimental-crate helper, and the opt-in error returned for explicit experimental lint requests without `--experimental`. --- docs/developers-guide.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 1f48dd97..8ad33cab 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -1545,6 +1545,26 @@ When `try_fast_path_installation` returns `Some`, `run_install` constructs a `FinishInstallContext` from the returned values and delegates to `finish_install_and_record_metrics`, skipping the full build pipeline. +#### Crate resolution + +`installer/src/resolution.rs` is the boundary for deciding which lint crates +the installer may build. `run_install` constructs `CrateResolutionOptions` from +the CLI flags before validating or resolving crate names: + +- `individual_lints` switches resolution from the aggregated suite to the + individual lint crate list. +- `experimental` is the explicit opt-in gate for experimental lints. In + individual-lint mode it allows crates from `EXPERIMENTAL_LINT_CRATES`; in + suite mode the build configuration maps it to suite feature flags. + +Call `validate_crate_names` before `resolve_crates` whenever names come from +user input. Validation rejects unknown names and uses `is_experimental_crate` +to detect explicit experimental lint requests. If a user asks for an +experimental crate without `--experimental`, validation returns +`InstallerError::ExperimentalLintRequiresFlag`; callers should surface that +error unchanged so the CLI reports the missing opt-in rather than treating the +crate as unknown or silently building it. + ## Standard vs Experimental Lints Whitaker categorizes lints into two tiers: From 01c36fbba7ee2317df8ea0e5de9534ea4f9986c3 Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 23 May 2026 17:42:51 +0200 Subject: [PATCH 19/21] Strengthen rstest helper bootstrap tests Cover the check-crate configuration path through the same normalisation helper used by the driver, assert the exact experimental opt-in error variant, and exercise `rstest` macro expansion in the bootstrap UI fixture. --- .../src/driver.rs | 55 +++++++++++++++---- .../tests/ui/bootstrap_zero_diagnostic.rs | 8 +++ installer/src/resolution.rs | 17 ++++++ 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/crates/rstest_helper_should_be_fixture/src/driver.rs b/crates/rstest_helper_should_be_fixture/src/driver.rs index d512df31..0efcbcb5 100644 --- a/crates/rstest_helper_should_be_fixture/src/driver.rs +++ b/crates/rstest_helper_should_be_fixture/src/driver.rs @@ -103,6 +103,25 @@ impl Default for RstestHelperShouldBeFixture { } impl RstestHelperShouldBeFixture { + fn apply_loaded_crate_configuration( + &mut self, + config: ConfigLoadResult, + shared_config: SharedConfig, + ) { + let config = match config { + Ok(config) => config, + Err(error) => { + debug!( + target: LINT_NAME, + "failed to parse `{LINT_NAME}` configuration: {error}; using defaults" + ); + Config::default() + } + }; + + self.apply_crate_configuration(config, shared_config); + } + fn apply_crate_configuration(&mut self, config: Config, shared_config: SharedConfig) { debug!( target: LINT_NAME, @@ -124,18 +143,7 @@ impl RstestHelperShouldBeFixture { impl<'tcx> LateLintPass<'tcx> for RstestHelperShouldBeFixture { fn check_crate(&mut self, _cx: &LateContext<'tcx>) { - let config = match load_configuration() { - Ok(config) => config, - Err(error) => { - debug!( - target: LINT_NAME, - "failed to parse `{LINT_NAME}` configuration: {error}; using defaults" - ); - Config::default() - } - }; - - self.apply_crate_configuration(config, load_shared_config()); + self.apply_loaded_crate_configuration(load_configuration(), load_shared_config()); } } @@ -349,6 +357,29 @@ mod tests { assert_eq!(pass.detection_options.provider_param_attributes().len(), 2); } + #[rstest] + fn check_crate_configuration_loads_and_normalizes_config() { + let mut pass = RstestHelperShouldBeFixture::default(); + let config = Config { + min_calls: 0, + min_distinct_tests: 1, + provider_param_attributes: vec!["rstest::case".to_string()], + use_source_callee_fallback: true, + ..Config::default() + }; + + pass.apply_loaded_crate_configuration( + loaded_configuration::(Ok(Some(config))), + SharedConfig::default(), + ); + + assert_eq!(pass.config.min_calls, 2); + assert_eq!(pass.config.min_distinct_tests, 2); + assert_eq!(pass.config.provider_param_attributes, ["case"]); + assert!(pass.detection_options.use_expansion_trace_fallback()); + assert_eq!(pass.detection_options.provider_param_attributes().len(), 2); + } + proptest! { #[test] fn normalized_configuration_is_idempotent( diff --git a/crates/rstest_helper_should_be_fixture/tests/ui/bootstrap_zero_diagnostic.rs b/crates/rstest_helper_should_be_fixture/tests/ui/bootstrap_zero_diagnostic.rs index 0935f41a..384ed452 100644 --- a/crates/rstest_helper_should_be_fixture/tests/ui/bootstrap_zero_diagnostic.rs +++ b/crates/rstest_helper_should_be_fixture/tests/ui/bootstrap_zero_diagnostic.rs @@ -1,3 +1,11 @@ //! Zero-diagnostic fixture for the `rstest_helper_should_be_fixture` bootstrap. +use rstest::rstest; + fn main() {} + +#[rstest] +#[case::single_use("helper input")] +fn rstest_macro_fixture_compiles_without_diagnostics(#[case] value: &str) { + assert_eq!(value, "helper input"); +} diff --git a/installer/src/resolution.rs b/installer/src/resolution.rs index 077ad59a..e7df03a8 100644 --- a/installer/src/resolution.rs +++ b/installer/src/resolution.rs @@ -234,4 +234,21 @@ mod tests { ); } } + + #[test] + fn validate_crate_names_returns_experimental_lint_requires_flag_error() { + let name = CrateName::from("rstest_helper_should_be_fixture"); + let options = CrateResolutionOptions { + experimental: false, + ..CrateResolutionOptions::default() + }; + + let error = validate_crate_names(std::slice::from_ref(&name), &options) + .expect_err("experimental lint should require explicit opt-in"); + + assert!( + matches!(&error, InstallerError::ExperimentalLintRequiresFlag { name: error_name } if *error_name == name), + "expected ExperimentalLintRequiresFlag, got {error:?}" + ); + } } From 054e5f2348e1d7f08435e4c4efe8d7620508d487 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 03:37:41 +0200 Subject: [PATCH 20/21] Split rstest helper driver tests Move the rstest helper driver unit tests into a sibling test module so the production driver stays below the project file-size cap without widening the runtime visibility of its internals. --- .../src/driver.rs | 206 +----------------- .../src/driver_tests.rs | 204 +++++++++++++++++ 2 files changed, 206 insertions(+), 204 deletions(-) create mode 100644 crates/rstest_helper_should_be_fixture/src/driver_tests.rs diff --git a/crates/rstest_helper_should_be_fixture/src/driver.rs b/crates/rstest_helper_should_be_fixture/src/driver.rs index 0efcbcb5..827cc020 100644 --- a/crates/rstest_helper_should_be_fixture/src/driver.rs +++ b/crates/rstest_helper_should_be_fixture/src/driver.rs @@ -199,207 +199,5 @@ where } #[cfg(test)] -mod tests { - //! Unit tests for driver configuration normalization, loading boundaries, - //! and `rstest` detection option construction. - //! - //! NOTE: `SharedConfig::load` is treated as infallible at the driver call - //! site pending https://github.com/leynos/whitaker/issues/233. - - use super::*; - use proptest::prelude::*; - use rstest::rstest; - - #[rstest] - fn default_configuration_matches_design() { - let config = Config::default(); - - assert_eq!(config.min_calls, 2); - assert_eq!(config.min_distinct_tests, 2); - assert!(!config.require_identical_fixture_arg_names); - assert_eq!( - config.provider_param_attributes, - ["case", "values", "files", "future", "context"] - ); - assert!(!config.use_source_callee_fallback); - } - - #[rstest] - fn deserializes_valid_configuration() { - let config: Config = toml::from_str::( - r#" - min_calls = 3 - min_distinct_tests = 4 - require_identical_fixture_arg_names = true - provider_param_attributes = ["case", "custom_provider"] - use_source_callee_fallback = true - "#, - ) - .expect("valid configuration should deserialize") - .normalized(); - - assert_eq!(config.min_calls, 3); - assert_eq!(config.min_distinct_tests, 4); - assert!(config.require_identical_fixture_arg_names); - assert_eq!( - config.provider_param_attributes, - ["case", "custom_provider"] - ); - assert!(config.use_source_callee_fallback); - } - - #[rstest] - fn rejects_unknown_configuration_fields() { - let result = toml::from_str::("unexpected = true"); - - assert!(result.is_err()); - } - - #[rstest] - fn normalizes_numeric_thresholds_to_two() { - let config = Config { - min_calls: 0, - min_distinct_tests: 0, - ..Config::default() - } - .normalized(); - - assert_eq!(config.min_calls, 2); - assert_eq!(config.min_distinct_tests, 2); - } - - #[rstest] - #[case::plain(vec!["case".to_string()], vec!["case"])] - #[case::qualified(vec!["rstest::values".to_string()], vec!["values"])] - #[case::blank(vec![" ".to_string()], vec!["case", "values", "files", "future", "context"])] - fn normalizes_provider_attributes(#[case] input: Vec, #[case] expected: Vec<&str>) { - let normalized = normalize_provider_attributes(input); - let expected: Vec = expected.into_iter().map(ToString::to_string).collect(); - - assert_eq!(normalized, expected); - } - - #[rstest] - fn detection_options_expand_plain_and_rstest_qualified_provider_paths() { - let config = Config { - provider_param_attributes: vec!["case".to_string(), "custom".to_string()], - use_source_callee_fallback: true, - ..Config::default() - }; - let options = config.detection_options(); - let paths: Vec = options - .provider_param_attributes() - .iter() - .map(ToString::to_string) - .collect(); - - assert_eq!(paths, ["case", "rstest::case", "custom", "rstest::custom"]); - assert!(options.use_expansion_trace_fallback()); - } - - #[rstest] - fn lint_pass_default_derives_detection_options_from_config() { - let pass = RstestHelperShouldBeFixture::default(); - - assert_eq!(pass.config, Config::default()); - assert_eq!( - pass.detection_options.provider_param_attributes().len(), - DEFAULT_PROVIDER_PARAM_ATTRIBUTES.len() * 2 - ); - } - - #[rstest] - fn loaded_configuration_uses_default_when_config_is_absent() { - assert_eq!( - loaded_configuration::(Ok(None)).expect("missing config should default"), - Config::default(), - ); - } - - #[rstest] - fn loaded_configuration_returns_error_when_config_errors() { - assert_eq!( - loaded_configuration(Err("invalid config")).expect_err("invalid config should error"), - "invalid config", - ); - } - - #[rstest] - fn loaded_configuration_normalizes_present_config() { - let config = Config { - min_calls: 1, - min_distinct_tests: 1, - provider_param_attributes: vec!["rstest::case".to_string()], - ..Config::default() - }; - - assert_eq!( - loaded_configuration::(Ok(Some(config))) - .expect("present config should load") - .provider_param_attributes, - ["case"] - ); - } - - #[rstest] - fn applying_crate_configuration_initializes_pass_state() { - let mut pass = RstestHelperShouldBeFixture::default(); - let config = Config { - provider_param_attributes: vec!["custom".to_string()], - use_source_callee_fallback: true, - ..Config::default() - }; - - pass.apply_crate_configuration(config.clone(), SharedConfig::default()); - - assert_eq!(pass.config, config); - assert!(pass.detection_options.use_expansion_trace_fallback()); - assert_eq!(pass.detection_options.provider_param_attributes().len(), 2); - } - - #[rstest] - fn check_crate_configuration_loads_and_normalizes_config() { - let mut pass = RstestHelperShouldBeFixture::default(); - let config = Config { - min_calls: 0, - min_distinct_tests: 1, - provider_param_attributes: vec!["rstest::case".to_string()], - use_source_callee_fallback: true, - ..Config::default() - }; - - pass.apply_loaded_crate_configuration( - loaded_configuration::(Ok(Some(config))), - SharedConfig::default(), - ); - - assert_eq!(pass.config.min_calls, 2); - assert_eq!(pass.config.min_distinct_tests, 2); - assert_eq!(pass.config.provider_param_attributes, ["case"]); - assert!(pass.detection_options.use_expansion_trace_fallback()); - assert_eq!(pass.detection_options.provider_param_attributes().len(), 2); - } - - proptest! { - #[test] - fn normalized_configuration_is_idempotent( - min_calls in 0usize..8, - min_distinct_tests in 0usize..8, - require_identical_fixture_arg_names in any::(), - provider_param_attributes in prop::collection::vec("[ a-z:]{0,24}", 0..8), - use_source_callee_fallback in any::(), - ) { - let config = Config { - min_calls, - min_distinct_tests, - require_identical_fixture_arg_names, - provider_param_attributes, - use_source_callee_fallback, - }; - let once = config.normalized(); - let twice = once.clone().normalized(); - - prop_assert_eq!(once, twice); - } - } -} +#[path = "driver_tests.rs"] +mod tests; diff --git a/crates/rstest_helper_should_be_fixture/src/driver_tests.rs b/crates/rstest_helper_should_be_fixture/src/driver_tests.rs new file mode 100644 index 00000000..dbc4d53d --- /dev/null +++ b/crates/rstest_helper_should_be_fixture/src/driver_tests.rs @@ -0,0 +1,204 @@ +//! Unit tests for driver configuration normalization, loading boundaries, and +//! `rstest` detection option construction. +//! +//! NOTE: `SharedConfig::load` is treated as infallible at the driver call site +//! pending https://github.com/leynos/whitaker/issues/233. + +use proptest::prelude::*; +use rstest::rstest; +use whitaker::SharedConfig; + +use super::*; + +#[rstest] +fn default_configuration_matches_design() { + let config = Config::default(); + + assert_eq!(config.min_calls, 2); + assert_eq!(config.min_distinct_tests, 2); + assert!(!config.require_identical_fixture_arg_names); + assert_eq!( + config.provider_param_attributes, + ["case", "values", "files", "future", "context"] + ); + assert!(!config.use_source_callee_fallback); +} + +#[rstest] +fn deserializes_valid_configuration() { + let config: Config = toml::from_str::( + r#" + min_calls = 3 + min_distinct_tests = 4 + require_identical_fixture_arg_names = true + provider_param_attributes = ["case", "custom_provider"] + use_source_callee_fallback = true + "#, + ) + .expect("valid configuration should deserialize") + .normalized(); + + assert_eq!(config.min_calls, 3); + assert_eq!(config.min_distinct_tests, 4); + assert!(config.require_identical_fixture_arg_names); + assert_eq!( + config.provider_param_attributes, + ["case", "custom_provider"] + ); + assert!(config.use_source_callee_fallback); +} + +#[rstest] +fn rejects_unknown_configuration_fields() { + let result = toml::from_str::("unexpected = true"); + + assert!(result.is_err()); +} + +#[rstest] +fn normalizes_numeric_thresholds_to_two() { + let config = Config { + min_calls: 0, + min_distinct_tests: 0, + ..Config::default() + } + .normalized(); + + assert_eq!(config.min_calls, 2); + assert_eq!(config.min_distinct_tests, 2); +} + +#[rstest] +#[case::plain(vec!["case".to_string()], vec!["case"])] +#[case::qualified(vec!["rstest::values".to_string()], vec!["values"])] +#[case::blank(vec![" ".to_string()], vec!["case", "values", "files", "future", "context"])] +fn normalizes_provider_attributes(#[case] input: Vec, #[case] expected: Vec<&str>) { + let normalized = normalize_provider_attributes(input); + let expected: Vec = expected.into_iter().map(ToString::to_string).collect(); + + assert_eq!(normalized, expected); +} + +#[rstest] +fn detection_options_expand_plain_and_rstest_qualified_provider_paths() { + let config = Config { + provider_param_attributes: vec!["case".to_string(), "custom".to_string()], + use_source_callee_fallback: true, + ..Config::default() + }; + let options = config.detection_options(); + let paths: Vec = options + .provider_param_attributes() + .iter() + .map(ToString::to_string) + .collect(); + + assert_eq!(paths, ["case", "rstest::case", "custom", "rstest::custom"]); + assert!(options.use_expansion_trace_fallback()); +} + +#[rstest] +fn lint_pass_default_derives_detection_options_from_config() { + let pass = RstestHelperShouldBeFixture::default(); + + assert_eq!(pass.config, Config::default()); + assert_eq!( + pass.detection_options.provider_param_attributes().len(), + DEFAULT_PROVIDER_PARAM_ATTRIBUTES.len() * 2 + ); +} + +#[rstest] +fn loaded_configuration_uses_default_when_config_is_absent() { + assert_eq!( + loaded_configuration::(Ok(None)).expect("missing config should default"), + Config::default(), + ); +} + +#[rstest] +fn loaded_configuration_returns_error_when_config_errors() { + assert_eq!( + loaded_configuration(Err("invalid config")).expect_err("invalid config should error"), + "invalid config", + ); +} + +#[rstest] +fn loaded_configuration_normalizes_present_config() { + let config = Config { + min_calls: 1, + min_distinct_tests: 1, + provider_param_attributes: vec!["rstest::case".to_string()], + ..Config::default() + }; + + assert_eq!( + loaded_configuration::(Ok(Some(config))) + .expect("present config should load") + .provider_param_attributes, + ["case"] + ); +} + +#[rstest] +fn applying_crate_configuration_initializes_pass_state() { + let mut pass = RstestHelperShouldBeFixture::default(); + let config = Config { + provider_param_attributes: vec!["custom".to_string()], + use_source_callee_fallback: true, + ..Config::default() + }; + + pass.apply_crate_configuration(config.clone(), SharedConfig::default()); + + assert_eq!(pass.config, config); + assert!(pass.detection_options.use_expansion_trace_fallback()); + assert_eq!(pass.detection_options.provider_param_attributes().len(), 2); +} + +#[rstest] +fn check_crate_configuration_loads_and_normalizes_config() { + let mut pass = RstestHelperShouldBeFixture::default(); + let config = Config { + min_calls: 0, + min_distinct_tests: 1, + provider_param_attributes: vec!["rstest::case".to_string()], + use_source_callee_fallback: true, + ..Config::default() + }; + + pass.apply_loaded_crate_configuration( + loaded_configuration::(Ok(Some(config))), + SharedConfig::default(), + ); + + assert_eq!(pass.config.min_calls, 2); + assert_eq!(pass.config.min_distinct_tests, 2); + assert_eq!(pass.config.provider_param_attributes, ["case"]); + assert!(pass.detection_options.use_expansion_trace_fallback()); + assert_eq!(pass.detection_options.provider_param_attributes().len(), 2); +} + +proptest! { + #[test] + fn normalized_configuration_is_idempotent( + min_calls in 0usize..8, + min_distinct_tests in 0usize..8, + require_identical_fixture_arg_names in any::(), + provider_param_attributes in prop::collection::vec("[ a-z:]{0,24}", 0..8), + use_source_callee_fallback in any::(), + ) { + let config = Config { + min_calls, + min_distinct_tests, + require_identical_fixture_arg_names, + provider_param_attributes, + use_source_callee_fallback, + }; + let once = config.normalized(); + let twice = once.clone().normalized(); + + prop_assert_eq!(once, twice); + } +} From f72c4181bb183992b053a83ff6ce6b021588ba7e Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 24 May 2026 04:50:24 +0200 Subject: [PATCH 21/21] Cover provider attribute deduplication Deduplicate equivalent provider attribute spellings during rstest helper configuration normalization, add the mixed spelling regression case, and link the no-driver stub expectation to tracked follow-up work. --- crates/rstest_helper_should_be_fixture/src/driver.rs | 9 +++++++-- .../rstest_helper_should_be_fixture/src/driver_tests.rs | 6 +++++- crates/rstest_helper_should_be_fixture/src/lib.rs | 5 ++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/crates/rstest_helper_should_be_fixture/src/driver.rs b/crates/rstest_helper_should_be_fixture/src/driver.rs index 827cc020..815759c5 100644 --- a/crates/rstest_helper_should_be_fixture/src/driver.rs +++ b/crates/rstest_helper_should_be_fixture/src/driver.rs @@ -148,11 +148,16 @@ impl<'tcx> LateLintPass<'tcx> for RstestHelperShouldBeFixture { } fn normalize_provider_attributes(attributes: Vec) -> Vec { - let normalized: Vec = attributes + let mut normalized = Vec::new(); + for attribute in attributes .into_iter() .map(|attribute| attribute.trim().trim_start_matches("rstest::").to_owned()) .filter(|attribute| !attribute.is_empty()) - .collect(); + { + if !normalized.contains(&attribute) { + normalized.push(attribute); + } + } if normalized.is_empty() { return default_provider_param_attributes(); diff --git a/crates/rstest_helper_should_be_fixture/src/driver_tests.rs b/crates/rstest_helper_should_be_fixture/src/driver_tests.rs index dbc4d53d..ecc59bbe 100644 --- a/crates/rstest_helper_should_be_fixture/src/driver_tests.rs +++ b/crates/rstest_helper_should_be_fixture/src/driver_tests.rs @@ -71,6 +71,10 @@ fn normalizes_numeric_thresholds_to_two() { #[rstest] #[case::plain(vec!["case".to_string()], vec!["case"])] #[case::qualified(vec!["rstest::values".to_string()], vec!["values"])] +#[case::mixed_equivalent_spellings( + vec!["case".to_string(), "rstest::case".to_string()], + vec!["case"] +)] #[case::blank(vec![" ".to_string()], vec!["case", "values", "files", "future", "context"])] fn normalizes_provider_attributes(#[case] input: Vec, #[case] expected: Vec<&str>) { let normalized = normalize_provider_attributes(input); @@ -152,7 +156,7 @@ fn applying_crate_configuration_initializes_pass_state() { pass.apply_crate_configuration(config.clone(), SharedConfig::default()); - assert_eq!(pass.config, config); + assert_eq!(pass.config, config.normalized()); assert!(pass.detection_options.use_expansion_trace_fallback()); assert_eq!(pass.detection_options.provider_param_attributes().len(), 2); } diff --git a/crates/rstest_helper_should_be_fixture/src/lib.rs b/crates/rstest_helper_should_be_fixture/src/lib.rs index 1734ebd4..a1c00964 100644 --- a/crates/rstest_helper_should_be_fixture/src/lib.rs +++ b/crates/rstest_helper_should_be_fixture/src/lib.rs @@ -22,6 +22,9 @@ mod stub { //! `rstest_helper_should_be_fixture_disabled_stub` as the inert public //! symbol for that build mode. - #[expect(dead_code, reason = "stub used when the driver feature is disabled")] + #[expect( + dead_code, + reason = "stub used when the driver feature is disabled; tracked in https://github.com/leynos/whitaker/issues/233" + )] pub fn rstest_helper_should_be_fixture_disabled_stub() {} }