From 7454beb0478a9fb53de56b3aea1d6bea4c476bf4 Mon Sep 17 00:00:00 2001 From: Leynos Date: Thu, 23 Apr 2026 22:48:55 +0000 Subject: [PATCH 1/6] docs(execplans): add detailed execplan for argument and paragraph fingerprint data models This new document outlines the design and implementation plan for roadmap item 8.1.3. It specifies the creation of shared argument and paragraph fingerprint data models in the common::rstest module, intended to serve future lint implementations. The execplan covers purpose, constraints, risks, progress checkpoints, and detailed decisions to ensure a stable and deterministic API. It includes comprehensive guidance on test coverage, API design, and documentation updates, forming the foundation for subsequent development steps. Co-authored-by: devboxerhub[bot] --- ...t-and-paragraph-fingerprint-data-models.md | 456 ++++++++++++++++++ 1 file changed, 456 insertions(+) create mode 100644 docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md diff --git a/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md new file mode 100644 index 00000000..3270bc92 --- /dev/null +++ b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md @@ -0,0 +1,456 @@ +# Add shared argument and paragraph fingerprint data models + +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 + +This document must be maintained in accordance with `AGENTS.md`. The canonical +plan file is +`docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md`. + +This draft must be approved before implementation begins. Do not start code +changes for roadmap item 8.1.3 until the user explicitly approves this plan. + +## Purpose / big picture + +Roadmap item 8.1.3 fills the remaining shared-data gap between strict `rstest` +detection and the later lint implementations. After this change, Whitaker will +have one pure, reusable place that can represent and compare the two kinds of +repeatable setup evidence the later lints need: + +1. Argument fingerprints for repeated helper calls in `#[rstest]` tests + (roadmap 8.2.x / lint A). +2. Paragraph fingerprints for repeated assertion-free setup blocks in + `#[rstest]` tests (roadmap 8.4.x / lint C). + +Success is observable when: + +1. `whitaker-common` exposes public, documented argument and paragraph + fingerprint types under `common::rstest`. +2. The paragraph API includes a deterministic local-slot normalization seam, so + equivalent paragraphs with different local variable names compare equal. +3. Unit tests cover happy paths, unhappy paths, and determinism edges for both + fingerprint families. +4. Behavioural tests using `rstest-bdd` v0.5.0 describe the same contracts in + user terms through `common/tests/`. +5. `docs/lints-for-rstest-fixtures-and-test-hygiene.md` records the final + implementation decisions for 8.1.3. +6. `docs/roadmap.md` marks 8.1.3 done only after implementation, + documentation, and all required gates succeed. +7. The implementation turn ends with `make fmt`, `make markdownlint`, + `make nixie`, `make check-fmt`, `make lint`, and `make test` all passing. + +## Constraints + +- Scope only roadmap item 8.1.3. Do not implement lint crates, HIR traversal, + call-site collection, paragraph slicing, diagnostics, crate-post emission, or + UI fixtures in this change. +- Keep the new fingerprint layer pure and `rustc_private`-free inside + `whitaker-common`. Compiler-aware lowering from HIR into these models belongs + to later roadmap items. +- Keep the public API in `common::rstest`, because 8.2.x and 8.4.x both depend + on the same shared models and should not duplicate them in crate-local code. +- Preserve the design-doc shape from + `docs/lints-for-rstest-fixtures-and-test-hygiene.md` unless implementation + proves a narrowly scoped refinement is necessary. Any refinement must be + recorded in the design doc and in this plan's `Decision Log`. +- Determinism is part of the feature, not an implementation detail. The shared + models must use stable ordering and stable equality semantics across runs and + test ordering. +- Keep unsupported and unknown cases explicit in the models rather than + silently dropping them. Later lint passes need to distinguish + "groupable/consistent" from "present but unsupported". +- Use public constructors or builders that can be exercised from unit tests, + doctests, and `rstest-bdd` scenarios without requiring HIR values. +- Keep every Rust source file under 400 lines. The current + `common/src/rstest/tests.rs` is already 248 lines, so new fingerprint tests + must be split into dedicated test modules rather than appended until the file + violates the repository limit. +- Public APIs added to `common` require Rustdoc comments with examples that + compile under the doctest model described in `docs/rust-doctest-dry-guide.md`. +- Use workspace-pinned `rstest`, `rstest-bdd`, and `rstest-bdd-macros` at + `0.5.0`. +- Behaviour tests must respect the workspace Clippy threshold of 4 arguments + per step function. Each step may parse at most 3 values in addition to the + world fixture. +- Do not mark roadmap item 8.1.3 done until implementation, design-doc + updates, and all quality gates succeed. + +## Tolerances + +- Scope: if implementation needs more than 10 touched files or roughly 900 net + new lines, stop and escalate before continuing. +- API shape: if keeping the paragraph fingerprint layer deterministic requires + exposing raw `rustc_hir` or unstable compiler identifiers in `common`, stop + and escalate with the competing shapes. +- Model drift: if the design-doc fingerprint sketches prove insufficient and + the implementation needs materially different public types, stop and review + that drift before proceeding. +- Test support: if behaviour tests cannot exercise the public fingerprint API + without adding non-trivial test-only adapters, stop and justify that seam + before adding it. +- Validation: if `make check-fmt`, `make lint`, or `make test` still fail + after 3 targeted fix iterations, stop and escalate with the saved logs. + +## Risks + +- Semantics risk: argument fingerprints and paragraph fingerprints are similar + only at a distance. If they share too much internal machinery, the public API + could become vague or over-generalized. Mitigation: keep the two model + families sibling modules under `common::rstest`, with only the deterministic + conventions shared. +- Determinism risk: paragraph grouping depends on local-name normalization by + first appearance order. A leaky API could let later callers assign slots + inconsistently and produce unstable grouping. Mitigation: put slot + normalization inside the shared layer rather than leaving it to each future + lint crate. +- Over-validation risk: the design sketches show mostly data models, not rich + validation rules. If constructors reject too much, later HIR lowering may + become awkward; if they reject too little, later lints may duplicate checks. + Mitigation: validate only the invariants needed for determinism and expose + explicit `Unsupported` or `Unknown` variants for everything else. +- File-size risk: the current `common::rstest` module tree is compact, but the + test file is close enough to the 400-line cap that naive additions will + breach it. Mitigation: split tests by topic as part of the implementation. +- BDD ergonomics risk: the repository has already hit `rstest-bdd` gotchas + around `And` keyword binding and workspace-wide Clippy denials in tests. + Mitigation: keep the BDD world small, prefer explicit `Given`/`When`/`Then` + transitions, and avoid `.expect()` in test code. + +## Progress + +- [x] (2026-04-23) Reviewed roadmap item 8.1.3, the linked design document, + the current `common::rstest` module, and the prior 8.1.1 and 8.1.2 ExecPlans. +- [x] (2026-04-23) Reviewed the current `rstest` unit and behaviour test + patterns in `common/`. +- [x] (2026-04-23) Drafted this ExecPlan at + `docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md`. +- [ ] Establish the red baseline with focused unit and behavioural tests that + describe the missing fingerprint contracts. +- [ ] Implement the shared argument fingerprint models and public constructors. +- [ ] Implement the shared paragraph fingerprint models and deterministic + local-slot normalization helpers. +- [ ] Re-export the new API from `common/src/rstest/mod.rs` and + `common/src/lib.rs`, with Rustdoc examples. +- [ ] Record implementation decisions in + `docs/lints-for-rstest-fixtures-and-test-hygiene.md`. +- [ ] Mark roadmap item 8.1.3 done in `docs/roadmap.md`. +- [ ] Run `make fmt`, `make markdownlint`, `make nixie`, `make check-fmt`, + `make lint`, and `make test`. +- [ ] Finalize the living sections in this document after implementation. + +## Surprises & Discoveries + +- `common::rstest` already contains the right architectural precedent for this + work: 8.1.1 added pure detection and parameter classification, and 8.1.2 + added pure span recovery with a thin compiler-aware adapter elsewhere. That + means 8.1.3 should remain pure as well. +- The design document already sketches the exact public vocabulary for both + fingerprint families (`ArgAtom`, `ArgFingerprint`, `ParagraphFingerprint`, + `StmtShape`, `ExprShape`, and `CalleeShape`), so the implementation should + not invent a materially different naming scheme without a strong reason. +- `common/src/rstest/tests.rs` is already 248 lines long, so even moderate + new coverage should trigger an early test split rather than a late cleanup. +- The stale `rstest-bdd` comment in `common/Cargo.toml` has already been fixed + to `0.5.x`; 8.1.3 does not need to revisit that documentation hygiene. +- The current behaviour harnesses in + `common/tests/rstest_detection_behaviour.rs` + and `common/tests/rstest_span_recovery_behaviour.rs` are good templates for a + small, public-API-first fingerprint harness. +- Previous 8.1.x work already documented one `rstest-bdd` caveat: `And` + continues the previous keyword family. The fingerprint harness should prefer + explicit step types instead of relying on subtle keyword transitions. + +## Decision Log + +- Decision: keep 8.1.3 in `common::rstest` rather than creating a separate + top-level `common::fingerprint` module. Rationale: these fingerprints are not + generic clone-detection hashes or cross-domain utilities; they exist to serve + later `rstest` fixture-hygiene lints. Date/Author: 2026-04-23 / plan author. +- Decision: implement deterministic paragraph local-slot normalization inside + the shared layer rather than in future lint crates. Rationale: "same + paragraph, different local names" is the core cross-test grouping contract, + so the shared foundation should own it. Date/Author: 2026-04-23 / plan author. +- Decision: keep unsupported and unknown states explicit in the public models. + Rationale: later lints need to know whether a candidate was unsupported, not + merely absent, and silent dropping would make false-positive control harder. + Date/Author: 2026-04-23 / plan author. +- Decision: split new unit tests into dedicated modules instead of growing + `common/src/rstest/tests.rs` in place. Rationale: the repository's 400-line + limit is a hard constraint, not a cleanup suggestion. Date/Author: 2026-04-23 + / plan author. + +## Context and orientation + +### Repository state + +The relevant code and tests already live in one shared cluster: + +- `common/src/rstest/detection.rs` contains the pure 8.1.1 detection helpers. +- `common/src/rstest/parameter.rs` contains parameter classification for + fixture-local versus provider-driven inputs. +- `common/src/rstest/span.rs` contains the pure 8.1.2 span-recovery models. +- `common/src/rstest/mod.rs` re-exports the shared `rstest` API surface. +- `common/src/lib.rs` re-exports that API at the crate root. +- `common/tests/rstest_detection_behaviour.rs` and + `common/tests/rstest_span_recovery_behaviour.rs` show the current BDD shape + for pure shared helpers. + +The relevant documentation already exists: + +- `docs/roadmap.md` defines 8.1.3 as the shared fingerprint foundation needed + before 8.2.x and 8.4.x. +- `docs/lints-for-rstest-fixtures-and-test-hygiene.md` defines the argument and + paragraph fingerprint sketches under lint A and lint C. +- `docs/rust-testing-with-rstest-fixtures.md` explains the fixture-oriented + testing style already used in this repository. +- `docs/rstest-bdd-users-guide.md` documents the `rstest-bdd` conventions and + limitations that the behaviour harness must follow. +- `docs/rust-doctest-dry-guide.md` defines how public Rustdoc examples should + stay compileable without unnecessary execution complexity. +- `docs/complexity-antipatterns-and-refactoring-strategies.md` reinforces the + "small, focused helpers" bias that matters here because fingerprint lowering + logic can otherwise become a future bumpy-road hotspot. + +### Relevant skills + +The later implementation should explicitly lean on these skills: + +- `execplans` to keep this document current during implementation. +- `rust-router` to choose the smallest Rust-specialist skill when code work + begins. +- `rust-types-and-apis` for the shared public enums, structs, constructors, + and newtypes used to model deterministic fingerprints. +- `nextest` for understanding the repository's `make test` behaviour and the + distinction between targeted Rust test runs and full workspace validation. +- `en-gb-oxendict-style` for the design-doc and roadmap updates. + +### What 8.1.3 must provide + +This roadmap item is narrower than "implement lint A" or "implement lint C". It +provides the shared data models those later tasks will lower into and group by. + +For lint A, the shared layer must represent: + +- fixture-local arguments, +- stable literal arguments, +- stable constant-path arguments, and +- unsupported argument shapes. + +For lint C, the shared layer must represent: + +- normalized statement shapes, +- normalized callee identity where known, +- normalized local slots by first appearance order, and +- explicit unknown shapes where canonicalization stops. + +The shared layer does not need to walk HIR, recover spans, decide thresholds, +or emit diagnostics. Later roadmap items will do that lowering and policy work. + +## Proposed implementation shape + +Add new shared modules under `common/src/rstest/` and keep them sibling to the +existing detection and span helpers: + +- `common/src/rstest/argument_fingerprint.rs` +- `common/src/rstest/paragraph_fingerprint.rs` +- `common/src/rstest/mod.rs` +- `common/src/lib.rs` + +Split unit tests by topic so the file-size limit remains intact: + +- `common/src/rstest/tests/mod.rs` +- `common/src/rstest/tests/detection.rs` +- `common/src/rstest/tests/span.rs` +- `common/src/rstest/tests/fingerprint.rs` + +Add behaviour coverage: + +- `common/tests/rstest_fingerprint_behaviour.rs` +- `common/tests/features/rstest_fingerprint.feature` + +Update the docs: + +- `docs/lints-for-rstest-fixtures-and-test-hygiene.md` +- `docs/roadmap.md` + +The public surface should stay close to the design document, with one likely +addition: a deterministic slot newtype or builder helper so paragraph +normalization is owned by the shared layer. + +```rust +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum ArgAtom { + FixtureLocal { name: String }, + ConstLit { text: String }, + ConstPath { def_path: String }, + Unsupported, +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct ArgFingerprint { + atoms: Vec, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct LocalSlot(u16); + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum CalleeShape { + DefPath(String), + Unknown, +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum ExprShape { + Call { callee: CalleeShape, argc: usize }, + MethodCall { method: String, argc: usize }, + Path, + Lit, + Other, +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum StmtShape { + Let { init: ExprShape }, + MutCall { receiver: Option, callee: CalleeShape }, +} + +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub struct ParagraphFingerprint { + shapes: Vec, +} +``` + +Whether `LocalSlot` is exposed directly or only through a builder is an +implementation detail to finalize during the implementation turn. The important +contract is that equivalent paragraphs must normalize to the same stable slot +sequence regardless of local variable names. + +## Red-green contract + +Start by adding failing tests that describe the public behaviour before any +production code is written. + +Unit-test coverage in `common/src/rstest/tests/fingerprint.rs` must cover at +least: + +1. Argument fingerprint equality for identical fixture-local and literal input + sequences. +2. Argument fingerprint inequality when positionally different atoms are used. +3. Explicit unsupported argument atoms surviving in the fingerprint instead of + being dropped. +4. Paragraph fingerprints normalizing different local names to the same slot + sequence when first appearance order matches. +5. Paragraph fingerprints diverging when statement shape, callee identity, or + argument counts differ. +6. Deterministic slot assignment for repeated normalization runs over the same + logical paragraph. + +Behaviour-test scenarios in `common/tests/rstest_fingerprint_behaviour.rs` and +`common/tests/features/rstest_fingerprint.feature` should cover: + +1. Happy path: equivalent helper-call arguments yield the same fingerprint. +2. Happy path: equivalent setup paragraphs with renamed locals still group + together. +3. Unhappy path: unsupported arguments remain explicitly unsupported. +4. Unhappy path: structurally different paragraphs do not group together. +5. Edge case: first-appearance order controls slot numbering, not lexical sort + of local names. + +## Concrete implementation steps + +### Milestone 1: Lock the contract in tests first + +Create the new unit-test module and the new BDD harness before implementing the +fingerprint code. Keep the tests public-API-first: they should construct +fingerprints through constructors or builders that a later lint crate could +also use. + +### Milestone 2: Add argument fingerprint models + +Implement the lint-A-facing pure models in +`common/src/rstest/argument_fingerprint.rs`. The code should stay close to the +design doc: + +- store atoms in call-site order, +- preserve `Unsupported` atoms explicitly, +- derive equality and hashing in the stable order provided by `Vec`, and +- expose small constructors/helpers with Rustdoc examples. + +This milestone should not try to infer literals or definition paths from HIR. +It only models already-lowered atoms. + +### Milestone 3: Add paragraph fingerprint models + +Implement the lint-C-facing pure models in +`common/src/rstest/paragraph_fingerprint.rs`. This is the place to encode +deterministic local-slot normalization. Prefer a small helper such as a builder +or normalizer that: + +1. receives already-lowered local names or local references, +2. assigns slots by first appearance order, +3. emits stable `LocalSlot` values, and +4. preserves `Unknown` or `Other` variants where canonicalization stops. + +Keep the API explicit rather than clever. Future lint crates should be able to +read the model and predict the grouping outcome without reverse-engineering a +compact abstraction. + +### Milestone 4: Re-export and document the shared API + +Update `common/src/rstest/mod.rs` and `common/src/lib.rs` to re-export the new +types. Add Rustdoc examples that compile cleanly under the doctest guidance in +`docs/rust-doctest-dry-guide.md`. If a builder or normalizer is public, +document one example that shows renamed locals normalizing to equal paragraph +fingerprints. + +### Milestone 5: Update design and roadmap docs + +Add an `Implementation decisions (8.1.3)` section to +`docs/lints-for-rstest-fixtures-and-test-hygiene.md`. Record at least: + +1. where the shared fingerprint models live, +2. whether local-slot normalization is builder-driven or constructor-driven, +3. how unsupported and unknown shapes are represented, and +4. any small naming refinement taken from the draft design. + +Only after the code, tests, and gates pass should `docs/roadmap.md` mark 8.1.3 +done. + +## Validation commands + +The implementation turn should capture focused and full validation logs with +`tee` and `set -o pipefail` from the repository root. + +```sh +set -o pipefail && cargo test -p whitaker-common rstest:: 2>&1 | tee \ + /tmp/8-1-3-common-rstest-unit.log +set -o pipefail && cargo test -p whitaker-common --test rstest_fingerprint_behaviour \ + 2>&1 | tee /tmp/8-1-3-common-rstest-bdd.log +set -o pipefail && cargo clippy -p whitaker-common --all-targets --all-features -- \ + -D warnings 2>&1 | tee /tmp/8-1-3-common-rstest-clippy.log +set -o pipefail && make fmt 2>&1 | tee /tmp/8-1-3-fmt.log +set -o pipefail && make markdownlint 2>&1 | tee /tmp/8-1-3-markdownlint.log +set -o pipefail && make nixie 2>&1 | tee /tmp/8-1-3-nixie.log +set -o pipefail && make check-fmt 2>&1 | tee /tmp/8-1-3-check-fmt.log +set -o pipefail && make lint 2>&1 | tee /tmp/8-1-3-lint.log +set -o pipefail && make test 2>&1 | tee /tmp/8-1-3-test.log +``` + +Expected success signals: + +- the focused unit test run includes the new fingerprint assertions, +- the new BDD binary passes all fingerprint scenarios, +- the targeted Clippy run stays warning-free, +- the documentation gates pass because this task edits Markdown, and +- the full workspace gates pass unchanged. + +## Outcomes & Retrospective + +Pending implementation. This draft is complete enough for review and approval, +but no code, roadmap, or design-document completion state should change until +the implementation turn is approved and finished. From 7697bcd6b72cb2adff9e8bf5911770d54e4b4420 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 1 May 2026 00:59:43 +0200 Subject: [PATCH 2/6] Add rstest fingerprint data models Introduce shared argument and paragraph fingerprint models in `whitaker-common` so later rstest hygiene lints can group repeated helper arguments and setup paragraphs without depending on compiler-private data. Add deterministic paragraph local-slot normalization, preserve unsupported argument atoms explicitly, and cover the public API with focused unit tests and rstest-bdd behaviour scenarios. Record the implementation decisions in the design notes and roadmap, and add legacy Markdown lint configuration so `make fmt` and `make markdownlint` share the same policy. --- .markdownlint.json | 11 + common/src/lib.rs | 9 +- common/src/rstest/argument_fingerprint.rs | 126 ++++++++ common/src/rstest/mod.rs | 15 +- common/src/rstest/paragraph_fingerprint.rs | 273 ++++++++++++++++++ common/src/rstest/tests/fingerprint.rs | 110 +++++++ common/src/rstest/{tests.rs => tests/mod.rs} | 4 +- .../tests/features/rstest_fingerprint.feature | 29 ++ common/tests/rstest_fingerprint_behaviour.rs | 197 +++++++++++++ ...t-and-paragraph-fingerprint-data-models.md | 81 ++++-- ...ts-for-rstest-fixtures-and-test-hygiene.md | 25 ++ docs/roadmap.md | 2 +- 12 files changed, 852 insertions(+), 30 deletions(-) create mode 100644 .markdownlint.json create mode 100644 common/src/rstest/argument_fingerprint.rs create mode 100644 common/src/rstest/paragraph_fingerprint.rs create mode 100644 common/src/rstest/tests/fingerprint.rs rename common/src/rstest/{tests.rs => tests/mod.rs} (99%) create mode 100644 common/tests/features/rstest_fingerprint.feature create mode 100644 common/tests/rstest_fingerprint_behaviour.rs diff --git a/.markdownlint.json b/.markdownlint.json new file mode 100644 index 00000000..91e9c054 --- /dev/null +++ b/.markdownlint.json @@ -0,0 +1,11 @@ +{ + "MD004": { "style": "dash" }, + "MD010": { "code_blocks": false }, + "MD013": { + "line_length": 80, + "code_block_line_length": 120, + "tables": false, + "headings": false + }, + "MD029": { "style": "ordered" } +} diff --git a/common/src/lib.rs b/common/src/lib.rs index 3ed7d15a..bb7214a0 100644 --- a/common/src/lib.rs +++ b/common/src/lib.rs @@ -55,9 +55,10 @@ pub use i18n::{ pub use lcom4::{MethodInfo, MethodInfoBuilder, cohesion_components, collect_method_infos}; pub use path::SimplePath; pub use rstest::{ - ExpansionTrace, ParameterBinding, RstestDetectionOptions, RstestParameter, RstestParameterKind, - SpanRecoveryFrame, UserEditableSpan, classify_rstest_parameter, fixture_local_names, - is_rstest_fixture, is_rstest_fixture_with, is_rstest_test, is_rstest_test_with, - recover_user_editable_span, + ArgAtom, ArgFingerprint, CalleeShape, ExpansionTrace, ExprShape, LocalSlot, + ParagraphFingerprint, ParagraphNormalizer, ParameterBinding, RstestDetectionOptions, + RstestParameter, RstestParameterKind, SpanRecoveryFrame, StmtShape, UserEditableSpan, + classify_rstest_parameter, fixture_local_names, is_rstest_fixture, is_rstest_fixture_with, + is_rstest_test, is_rstest_test_with, recover_user_editable_span, }; pub use span::{SourceLocation, SourceSpan, SpanError, span_line_count, span_to_lines}; diff --git a/common/src/rstest/argument_fingerprint.rs b/common/src/rstest/argument_fingerprint.rs new file mode 100644 index 00000000..b18cdc3d --- /dev/null +++ b/common/src/rstest/argument_fingerprint.rs @@ -0,0 +1,126 @@ +//! Pure argument fingerprints for repeated `rstest` helper-call evidence. + +/// A lowered argument value that participates in helper-call grouping. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum ArgAtom { + /// An argument supplied by an `rstest` fixture-local parameter. + FixtureLocal { name: String }, + /// A stable literal argument, stored as canonical source text. + ConstLit { text: String }, + /// A stable constant path, stored as a canonical definition path. + ConstPath { def_path: String }, + /// A present argument shape that later lowering does not support. + Unsupported, +} + +impl ArgAtom { + /// Builds a fixture-local argument atom. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::ArgAtom; + /// + /// let atom = ArgAtom::fixture_local("db"); + /// assert_eq!(atom, ArgAtom::FixtureLocal { name: "db".to_string() }); + /// ``` + #[must_use] + pub fn fixture_local(name: impl Into) -> Self { + Self::FixtureLocal { name: name.into() } + } + + /// Builds a stable literal argument atom. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::ArgAtom; + /// + /// let atom = ArgAtom::const_lit("42"); + /// assert_eq!(atom, ArgAtom::ConstLit { text: "42".to_string() }); + /// ``` + #[must_use] + pub fn const_lit(text: impl Into) -> Self { + Self::ConstLit { text: text.into() } + } + + /// Builds a stable constant-path argument atom. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::ArgAtom; + /// + /// let atom = ArgAtom::const_path("crate::defaults::TIMEOUT"); + /// assert_eq!( + /// atom, + /// ArgAtom::ConstPath { + /// def_path: "crate::defaults::TIMEOUT".to_string(), + /// }, + /// ); + /// ``` + #[must_use] + pub fn const_path(def_path: impl Into) -> Self { + Self::ConstPath { + def_path: def_path.into(), + } + } + + /// Builds an explicit unsupported argument atom. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::ArgAtom; + /// + /// assert_eq!(ArgAtom::unsupported(), ArgAtom::Unsupported); + /// ``` + #[must_use] + pub const fn unsupported() -> Self { + Self::Unsupported + } +} + +/// A positional fingerprint for one helper-call argument list. +#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] +pub struct ArgFingerprint { + atoms: Vec, +} + +impl ArgFingerprint { + /// Builds a fingerprint from argument atoms in call-site order. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::{ArgAtom, ArgFingerprint}; + /// + /// let fingerprint = ArgFingerprint::new([ + /// ArgAtom::fixture_local("db"), + /// ArgAtom::const_lit("42"), + /// ]); + /// + /// assert_eq!(fingerprint.atoms().len(), 2); + /// ``` + #[must_use] + pub fn new(atoms: I) -> Self + where + I: IntoIterator, + { + Self { + atoms: atoms.into_iter().collect(), + } + } + + /// Returns the stored atoms in positional order. + #[must_use] + pub fn atoms(&self) -> &[ArgAtom] { + &self.atoms + } + + /// Consumes the fingerprint and returns the stored atoms. + #[must_use] + pub fn into_atoms(self) -> Vec { + self.atoms + } +} diff --git a/common/src/rstest/mod.rs b/common/src/rstest/mod.rs index 19fbfaa3..279ba547 100644 --- a/common/src/rstest/mod.rs +++ b/common/src/rstest/mod.rs @@ -1,22 +1,29 @@ -//! Shared helpers for strict `rstest` detection, parameter analysis, and span -//! recovery. +//! Shared helpers for strict `rstest` detection, parameter analysis, span +//! recovery, and fingerprint data models. //! //! The module exports detection helpers such as [`ExpansionTrace`], //! [`RstestDetectionOptions`], [`is_rstest_fixture`], and [`is_rstest_test`]; //! parameter helpers such as [`ParameterBinding`], [`RstestParameter`], //! [`RstestParameterKind`], [`classify_rstest_parameter`], and -//! [`fixture_local_names`]; and span-recovery helpers such as +//! [`fixture_local_names`]; span-recovery helpers such as //! [`SpanRecoveryFrame`], [`UserEditableSpan`], and -//! [`recover_user_editable_span`]. +//! [`recover_user_editable_span`]; and fingerprint models such as +//! [`ArgFingerprint`] and [`ParagraphFingerprint`]. +mod argument_fingerprint; mod detection; +mod paragraph_fingerprint; mod parameter; mod span; +pub use argument_fingerprint::{ArgAtom, ArgFingerprint}; pub use detection::{ ExpansionTrace, RstestDetectionOptions, is_rstest_fixture, is_rstest_fixture_with, is_rstest_test, is_rstest_test_with, }; +pub use paragraph_fingerprint::{ + CalleeShape, ExprShape, LocalSlot, ParagraphFingerprint, ParagraphNormalizer, StmtShape, +}; pub use parameter::{ ParameterBinding, RstestParameter, RstestParameterKind, classify_rstest_parameter, fixture_local_names, diff --git a/common/src/rstest/paragraph_fingerprint.rs b/common/src/rstest/paragraph_fingerprint.rs new file mode 100644 index 00000000..787295f2 --- /dev/null +++ b/common/src/rstest/paragraph_fingerprint.rs @@ -0,0 +1,273 @@ +//! Pure paragraph fingerprints for repeated `rstest` setup evidence. + +use std::collections::BTreeMap; + +/// A deterministic local-variable slot assigned by first appearance order. +#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct LocalSlot(u32); + +impl LocalSlot { + /// Builds a local slot from its stable ordinal. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::LocalSlot; + /// + /// let slot = LocalSlot::new(0); + /// assert_eq!(slot.index(), 0); + /// ``` + #[must_use] + pub const fn new(index: u32) -> Self { + Self(index) + } + + /// Returns the stable slot ordinal. + #[must_use] + pub const fn index(self) -> u32 { + self.0 + } +} + +/// Assigns deterministic local slots by first appearance order. +#[derive(Clone, Debug, Default)] +pub struct ParagraphNormalizer { + slots: BTreeMap, + next_slot: u32, +} + +impl ParagraphNormalizer { + /// Builds an empty paragraph normalizer. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::ParagraphNormalizer; + /// + /// let mut normalizer = ParagraphNormalizer::new(); + /// + /// assert_eq!(normalizer.local_slot("user").index(), 0); + /// assert_eq!(normalizer.local_slot("cache").index(), 1); + /// assert_eq!(normalizer.local_slot("user").index(), 0); + /// ``` + #[must_use] + pub const fn new() -> Self { + Self { + slots: BTreeMap::new(), + next_slot: 0, + } + } + + /// Returns the deterministic slot for a local name. + /// + /// First appearance controls numbering. Later uses of the same local name + /// reuse the original slot. + #[must_use] + pub fn local_slot(&mut self, local_name: impl Into) -> LocalSlot { + let local_name = local_name.into(); + if let Some(slot) = self.slots.get(&local_name) { + return *slot; + } + + let slot = LocalSlot::new(self.next_slot); + self.next_slot += 1; + self.slots.insert(local_name, slot); + slot + } +} + +/// A normalized callee identity used inside paragraph statements. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum CalleeShape { + /// A known canonical definition path. + DefPath(String), + /// A present callee whose identity is not known to the shared model. + Unknown, +} + +impl CalleeShape { + /// Builds a known definition-path callee shape. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::CalleeShape; + /// + /// let callee = CalleeShape::def_path("crate::make_user"); + /// assert_eq!(callee, CalleeShape::DefPath("crate::make_user".to_string())); + /// ``` + #[must_use] + pub fn def_path(def_path: impl Into) -> Self { + Self::DefPath(def_path.into()) + } + + /// Builds an unknown callee shape. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::CalleeShape; + /// + /// assert_eq!(CalleeShape::unknown(), CalleeShape::Unknown); + /// ``` + #[must_use] + pub const fn unknown() -> Self { + Self::Unknown + } +} + +/// A normalized expression shape used by paragraph statements. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum ExprShape { + /// A function call with known or unknown callee identity and arity. + Call { callee: CalleeShape, argc: usize }, + /// A method call with method name and arity. + MethodCall { method: String, argc: usize }, + /// A stable path expression. + Path, + /// A stable literal expression. + Lit, + /// A present expression shape outside the supported model. + Other, +} + +impl ExprShape { + /// Builds a function-call expression shape. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::{CalleeShape, ExprShape}; + /// + /// let shape = ExprShape::call(CalleeShape::def_path("crate::make_user"), 2); + /// assert_eq!( + /// shape, + /// ExprShape::Call { + /// callee: CalleeShape::def_path("crate::make_user"), + /// argc: 2, + /// }, + /// ); + /// ``` + #[must_use] + pub const fn call(callee: CalleeShape, argc: usize) -> Self { + Self::Call { callee, argc } + } + + /// Builds a method-call expression shape. + #[must_use] + pub fn method_call(method: impl Into, argc: usize) -> Self { + Self::MethodCall { + method: method.into(), + argc, + } + } + + /// Builds a path expression shape. + #[must_use] + pub const fn path() -> Self { + Self::Path + } + + /// Builds a literal expression shape. + #[must_use] + pub const fn lit() -> Self { + Self::Lit + } + + /// Builds an explicit unsupported expression shape. + #[must_use] + pub const fn other() -> Self { + Self::Other + } +} + +/// A normalized statement shape used for paragraph grouping. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum StmtShape { + /// A `let` statement represented by its initializer shape. + Let { init: ExprShape }, + /// A mutating call, optionally tied to a normalized local receiver slot. + MutCall { + receiver: Option, + callee: CalleeShape, + }, +} + +impl StmtShape { + /// Builds a `let` statement shape from its initializer. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::{ExprShape, StmtShape}; + /// + /// assert_eq!( + /// StmtShape::let_binding(ExprShape::lit()), + /// StmtShape::Let { init: ExprShape::Lit }, + /// ); + /// ``` + #[must_use] + pub const fn let_binding(init: ExprShape) -> Self { + Self::Let { init } + } + + /// Builds a mutating-call statement shape. + #[must_use] + pub const fn mutable_call(receiver: Option, callee: CalleeShape) -> Self { + Self::MutCall { receiver, callee } + } +} + +/// A normalized fingerprint for one assertion-free setup paragraph. +#[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] +pub struct ParagraphFingerprint { + shapes: Vec, +} + +impl ParagraphFingerprint { + /// Builds a paragraph fingerprint from normalized statement shapes. + /// + /// # Examples + /// + /// ``` + /// use whitaker_common::rstest::{ + /// CalleeShape, ExprShape, ParagraphFingerprint, ParagraphNormalizer, + /// StmtShape, + /// }; + /// + /// let mut normalizer = ParagraphNormalizer::new(); + /// let first = ParagraphFingerprint::new([ + /// StmtShape::let_binding(ExprShape::call(CalleeShape::def_path("crate::build"), 0)), + /// StmtShape::mutable_call(Some(normalizer.local_slot("user")), CalleeShape::unknown()), + /// ]); + /// + /// let mut renamed = ParagraphNormalizer::new(); + /// let second = ParagraphFingerprint::new([ + /// StmtShape::let_binding(ExprShape::call(CalleeShape::def_path("crate::build"), 0)), + /// StmtShape::mutable_call(Some(renamed.local_slot("account")), CalleeShape::unknown()), + /// ]); + /// + /// assert_eq!(first, second); + /// ``` + #[must_use] + pub fn new(shapes: I) -> Self + where + I: IntoIterator, + { + Self { + shapes: shapes.into_iter().collect(), + } + } + + /// Returns the stored statement shapes in paragraph order. + #[must_use] + pub fn shapes(&self) -> &[StmtShape] { + &self.shapes + } + + /// Consumes the fingerprint and returns the stored statement shapes. + #[must_use] + pub fn into_shapes(self) -> Vec { + self.shapes + } +} diff --git a/common/src/rstest/tests/fingerprint.rs b/common/src/rstest/tests/fingerprint.rs new file mode 100644 index 00000000..dcc05178 --- /dev/null +++ b/common/src/rstest/tests/fingerprint.rs @@ -0,0 +1,110 @@ +//! Unit tests for shared `rstest` fingerprint data models. + +use crate::rstest::{ + ArgAtom, ArgFingerprint, CalleeShape, ExprShape, LocalSlot, ParagraphFingerprint, + ParagraphNormalizer, StmtShape, +}; +use rstest::rstest; + +#[rstest] +fn argument_fingerprints_compare_identical_atom_sequences() { + let first = ArgFingerprint::new([ + ArgAtom::fixture_local("db"), + ArgAtom::const_lit("42"), + ArgAtom::const_path("crate::defaults::TIMEOUT"), + ]); + let second = ArgFingerprint::new([ + ArgAtom::fixture_local("db"), + ArgAtom::const_lit("42"), + ArgAtom::const_path("crate::defaults::TIMEOUT"), + ]); + + assert_eq!(first, second); +} + +#[rstest] +fn argument_fingerprints_preserve_positional_differences() { + let fixture_then_literal = + ArgFingerprint::new([ArgAtom::fixture_local("db"), ArgAtom::const_lit("42")]); + let literal_then_fixture = + ArgFingerprint::new([ArgAtom::const_lit("42"), ArgAtom::fixture_local("db")]); + + assert_ne!(fixture_then_literal, literal_then_fixture); +} + +#[rstest] +fn unsupported_argument_atoms_remain_present() { + let fingerprint = ArgFingerprint::new([ + ArgAtom::fixture_local("db"), + ArgAtom::unsupported(), + ArgAtom::const_lit("42"), + ]); + + assert_eq!( + fingerprint.atoms(), + &[ + ArgAtom::fixture_local("db"), + ArgAtom::unsupported(), + ArgAtom::const_lit("42"), + ] + ); +} + +#[rstest] +fn paragraph_fingerprints_normalize_renamed_locals_by_first_appearance() { + let first = paragraph_for_renamed_locals("user", "cache"); + let second = paragraph_for_renamed_locals("account", "store"); + + assert_eq!(first, second); +} + +#[rstest] +fn paragraph_fingerprints_diverge_for_structural_differences() { + let two_argument_call = ParagraphFingerprint::new([StmtShape::let_binding(ExprShape::call( + CalleeShape::def_path("crate::make_user"), + 2, + ))]); + let one_argument_call = ParagraphFingerprint::new([StmtShape::let_binding(ExprShape::call( + CalleeShape::def_path("crate::make_user"), + 1, + ))]); + + assert_ne!(two_argument_call, one_argument_call); +} + +#[rstest] +fn paragraph_normalization_is_deterministic_across_runs() { + let first = paragraph_for_renamed_locals("zeta", "alpha"); + let second = paragraph_for_renamed_locals("zeta", "alpha"); + + assert_eq!(first, second); + assert_eq!( + first.shapes(), + &[ + StmtShape::let_binding(ExprShape::call(CalleeShape::def_path("crate::load"), 0)), + StmtShape::mutable_call( + Some(LocalSlot::new(0)), + CalleeShape::def_path("crate::prepare"), + ), + StmtShape::mutable_call( + Some(LocalSlot::new(1)), + CalleeShape::def_path("crate::prepare"), + ), + ] + ); +} + +fn paragraph_for_renamed_locals(first_name: &str, second_name: &str) -> ParagraphFingerprint { + let mut normalizer = ParagraphNormalizer::new(); + ParagraphFingerprint::new([ + StmtShape::let_binding(ExprShape::call(CalleeShape::def_path("crate::load"), 0)), + StmtShape::mutable_call( + Some(normalizer.local_slot(first_name)), + CalleeShape::def_path("crate::prepare"), + ), + StmtShape::mutable_call( + Some(normalizer.local_slot(second_name)), + CalleeShape::def_path("crate::prepare"), + ), + ]) +} diff --git a/common/src/rstest/tests.rs b/common/src/rstest/tests/mod.rs similarity index 99% rename from common/src/rstest/tests.rs rename to common/src/rstest/tests/mod.rs index 618348a6..fd47cb51 100644 --- a/common/src/rstest/tests.rs +++ b/common/src/rstest/tests/mod.rs @@ -1,4 +1,6 @@ -//! Unit tests for strict `rstest` detection helpers. +//! Unit tests for shared `rstest` helpers. + +mod fingerprint; use super::{ ExpansionTrace, ParameterBinding, RstestDetectionOptions, RstestParameter, RstestParameterKind, diff --git a/common/tests/features/rstest_fingerprint.feature b/common/tests/features/rstest_fingerprint.feature new file mode 100644 index 00000000..f394f197 --- /dev/null +++ b/common/tests/features/rstest_fingerprint.feature @@ -0,0 +1,29 @@ +Feature: Shared rstest fingerprint models + + Scenario: Equivalent helper-call arguments share a fingerprint + Given helper-call arguments for fixture db and literal 42 + And matching helper-call arguments for fixture db and literal 42 + When I compare the argument fingerprints + Then the argument fingerprints match + + Scenario: Renamed setup paragraphs share a fingerprint + Given a setup paragraph using locals user and cache + And a matching setup paragraph using locals account and store + When I compare the paragraph fingerprints + Then the paragraph fingerprints match + + Scenario: Unsupported arguments remain explicit + Given helper-call arguments containing an unsupported argument + When I inspect the argument fingerprint + Then the unsupported argument is still present + + Scenario: Structurally different setup paragraphs diverge + Given a setup paragraph with a one-argument constructor + And a matching setup paragraph with a two-argument constructor + When I compare the paragraph fingerprints + Then the paragraph fingerprints differ + + Scenario: Local slots follow first appearance order + Given a setup paragraph using locals zeta and alpha + When I inspect the paragraph fingerprint + Then zeta has slot 0 and alpha has slot 1 diff --git a/common/tests/rstest_fingerprint_behaviour.rs b/common/tests/rstest_fingerprint_behaviour.rs new file mode 100644 index 00000000..058fa87b --- /dev/null +++ b/common/tests/rstest_fingerprint_behaviour.rs @@ -0,0 +1,197 @@ +//! Behaviour-driven tests for shared `rstest` fingerprint models. + +use rstest::fixture; +use rstest_bdd_macros::{given, scenario, then, when}; +use std::cell::RefCell; +use whitaker_common::rstest::{ + ArgAtom, ArgFingerprint, CalleeShape, ExprShape, LocalSlot, ParagraphFingerprint, + ParagraphNormalizer, StmtShape, +}; + +#[derive(Default)] +struct FingerprintWorld { + first_args: RefCell>, + second_args: RefCell>, + first_paragraph: RefCell>, + second_paragraph: RefCell>, + args_match: RefCell>, + paragraphs_match: RefCell>, +} + +impl FingerprintWorld { + fn set_first_args(&self, fingerprint: ArgFingerprint) { + self.first_args.replace(Some(fingerprint)); + } + + fn set_second_args(&self, fingerprint: ArgFingerprint) { + self.second_args.replace(Some(fingerprint)); + } + + fn set_first_paragraph(&self, fingerprint: ParagraphFingerprint) { + self.first_paragraph.replace(Some(fingerprint)); + } + + fn set_second_paragraph(&self, fingerprint: ParagraphFingerprint) { + self.second_paragraph.replace(Some(fingerprint)); + } + + fn compare_args(&self) { + self.args_match.replace(Some( + *self.first_args.borrow() == *self.second_args.borrow(), + )); + } + + fn compare_paragraphs(&self) { + self.paragraphs_match.replace(Some( + *self.first_paragraph.borrow() == *self.second_paragraph.borrow(), + )); + } +} + +#[fixture] +fn world() -> FingerprintWorld { + FingerprintWorld::default() +} + +#[given("helper-call arguments for fixture db and literal 42")] +fn given_helper_args(world: &FingerprintWorld) { + world.set_first_args(helper_args()); +} + +#[given("matching helper-call arguments for fixture db and literal 42")] +fn given_matching_helper_args(world: &FingerprintWorld) { + world.set_second_args(helper_args()); +} + +#[given("helper-call arguments containing an unsupported argument")] +fn given_unsupported_args(world: &FingerprintWorld) { + world.set_first_args(ArgFingerprint::new([ + ArgAtom::fixture_local("db"), + ArgAtom::unsupported(), + ])); +} + +#[given("a setup paragraph using locals {first} and {second}")] +fn given_setup_paragraph(world: &FingerprintWorld, first: String, second: String) { + world.set_first_paragraph(setup_paragraph(&first, &second, 1)); +} + +#[given("a matching setup paragraph using locals {first} and {second}")] +fn given_matching_setup_paragraph(world: &FingerprintWorld, first: String, second: String) { + world.set_second_paragraph(setup_paragraph(&first, &second, 1)); +} + +#[given("a setup paragraph with a one-argument constructor")] +fn given_one_arg_paragraph(world: &FingerprintWorld) { + world.set_first_paragraph(setup_paragraph("user", "cache", 1)); +} + +#[given("a matching setup paragraph with a two-argument constructor")] +fn given_two_arg_paragraph(world: &FingerprintWorld) { + world.set_second_paragraph(setup_paragraph("account", "store", 2)); +} + +#[when("I compare the argument fingerprints")] +fn when_compare_args(world: &FingerprintWorld) { + world.compare_args(); +} + +#[when("I compare the paragraph fingerprints")] +fn when_compare_paragraphs(world: &FingerprintWorld) { + world.compare_paragraphs(); +} + +#[when("I inspect the argument fingerprint")] +fn when_inspect_args(world: &FingerprintWorld) { + let _ = world; +} + +#[when("I inspect the paragraph fingerprint")] +fn when_inspect_paragraph(world: &FingerprintWorld) { + let _ = world; +} + +#[then("the argument fingerprints match")] +fn then_args_match(world: &FingerprintWorld) { + assert_eq!(*world.args_match.borrow(), Some(true)); +} + +#[then("the paragraph fingerprints match")] +fn then_paragraphs_match(world: &FingerprintWorld) { + assert_eq!(*world.paragraphs_match.borrow(), Some(true)); +} + +#[then("the unsupported argument is still present")] +fn then_unsupported_argument_is_present(world: &FingerprintWorld) { + let fingerprint = world.first_args.borrow(); + let atoms = fingerprint + .as_ref() + .map(ArgFingerprint::atoms) + .unwrap_or_default(); + + assert!(atoms.contains(&ArgAtom::unsupported())); +} + +#[then("the paragraph fingerprints differ")] +fn then_paragraphs_differ(world: &FingerprintWorld) { + assert_eq!(*world.paragraphs_match.borrow(), Some(false)); +} + +#[then("zeta has slot 0 and alpha has slot 1")] +fn then_slots_follow_first_appearance(world: &FingerprintWorld) { + let fingerprint = world.first_paragraph.borrow(); + let shapes = fingerprint + .as_ref() + .map(ParagraphFingerprint::shapes) + .unwrap_or_default(); + + assert_eq!( + shapes, + &[ + StmtShape::let_binding(ExprShape::call(CalleeShape::def_path("crate::build"), 1)), + StmtShape::mutable_call(Some(LocalSlot::new(0)), CalleeShape::unknown()), + StmtShape::mutable_call(Some(LocalSlot::new(1)), CalleeShape::unknown()), + ] + ); +} + +fn helper_args() -> ArgFingerprint { + ArgFingerprint::new([ArgAtom::fixture_local("db"), ArgAtom::const_lit("42")]) +} + +fn setup_paragraph(first: &str, second: &str, constructor_argc: usize) -> ParagraphFingerprint { + let mut normalizer = ParagraphNormalizer::new(); + ParagraphFingerprint::new([ + StmtShape::let_binding(ExprShape::call( + CalleeShape::def_path("crate::build"), + constructor_argc, + )), + StmtShape::mutable_call(Some(normalizer.local_slot(first)), CalleeShape::unknown()), + StmtShape::mutable_call(Some(normalizer.local_slot(second)), CalleeShape::unknown()), + ]) +} + +#[scenario(path = "tests/features/rstest_fingerprint.feature", index = 0)] +fn scenario_equivalent_arguments_match(world: FingerprintWorld) { + let _ = world; +} + +#[scenario(path = "tests/features/rstest_fingerprint.feature", index = 1)] +fn scenario_renamed_paragraphs_match(world: FingerprintWorld) { + let _ = world; +} + +#[scenario(path = "tests/features/rstest_fingerprint.feature", index = 2)] +fn scenario_unsupported_arguments_remain_explicit(world: FingerprintWorld) { + let _ = world; +} + +#[scenario(path = "tests/features/rstest_fingerprint.feature", index = 3)] +fn scenario_structural_paragraphs_diverge(world: FingerprintWorld) { + let _ = world; +} + +#[scenario(path = "tests/features/rstest_fingerprint.feature", index = 4)] +fn scenario_first_appearance_order_controls_slots(world: FingerprintWorld) { + let _ = world; +} diff --git a/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md index 3270bc92..5ae33124 100644 --- a/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md +++ b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md @@ -5,14 +5,14 @@ This ExecPlan (execution plan) is a living document. The sections `Decision Log`, and `Outcomes & Retrospective` must be kept up to date as work proceeds. -Status: DRAFT +Status: COMPLETE This document must be maintained in accordance with `AGENTS.md`. The canonical plan file is `docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md`. -This draft must be approved before implementation begins. Do not start code -changes for roadmap item 8.1.3 until the user explicitly approves this plan. +This plan was approved for implementation on 2026-05-01. Code changes for +roadmap item 8.1.3 may proceed within the tolerances below. ## Purpose / big picture @@ -128,19 +128,28 @@ Success is observable when: patterns in `common/`. - [x] (2026-04-23) Drafted this ExecPlan at `docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md`. -- [ ] Establish the red baseline with focused unit and behavioural tests that - describe the missing fingerprint contracts. -- [ ] Implement the shared argument fingerprint models and public constructors. -- [ ] Implement the shared paragraph fingerprint models and deterministic - local-slot normalization helpers. -- [ ] Re-export the new API from `common/src/rstest/mod.rs` and - `common/src/lib.rs`, with Rustdoc examples. -- [ ] Record implementation decisions in +- [x] (2026-05-01) Implementation approved by the user and plan status moved + to `IN PROGRESS`. +- [x] (2026-05-01) Established the red baseline with focused unit and + behavioural tests. `cargo test -p whitaker-common rstest::` fails only + because the new fingerprint types are not yet exported. +- [x] (2026-05-01) Implemented the shared argument fingerprint models and + public constructors in `common/src/rstest/argument_fingerprint.rs`. +- [x] (2026-05-01) Implemented the shared paragraph fingerprint models and + deterministic `ParagraphNormalizer` helper in + `common/src/rstest/paragraph_fingerprint.rs`. +- [x] (2026-05-01) Re-exported the new API from `common/src/rstest/mod.rs` and + `common/src/lib.rs`, with Rustdoc examples on the public constructors. +- [x] (2026-05-01) Recorded implementation decisions in `docs/lints-for-rstest-fixtures-and-test-hygiene.md`. -- [ ] Mark roadmap item 8.1.3 done in `docs/roadmap.md`. -- [ ] Run `make fmt`, `make markdownlint`, `make nixie`, `make check-fmt`, - `make lint`, and `make test`. -- [ ] Finalize the living sections in this document after implementation. +- [x] (2026-05-01) Marked roadmap item 8.1.3 done in + `docs/roadmap.md` after the implementation and formatter-config fix were + validated. +- [x] (2026-05-01) Ran `make fmt`, `make markdownlint`, `make nixie`, + `make check-fmt`, `make lint`, and `make test`; all passed after adding the + legacy Markdown lint configuration. +- [x] (2026-05-01) Finalized the living sections in this document after + implementation. ## Surprises & Discoveries @@ -157,12 +166,22 @@ Success is observable when: - The stale `rstest-bdd` comment in `common/Cargo.toml` has already been fixed to `0.5.x`; 8.1.3 does not need to revisit that documentation hygiene. - The current behaviour harnesses in - `common/tests/rstest_detection_behaviour.rs` - and `common/tests/rstest_span_recovery_behaviour.rs` are good templates for a + `common/tests/rstest_detection_behaviour.rs` and + `common/tests/rstest_span_recovery_behaviour.rs` are good templates for a small, public-API-first fingerprint harness. - Previous 8.1.x work already documented one `rstest-bdd` caveat: `And` continues the previous keyword family. The fingerprint harness should prefer explicit step types instead of relying on subtle keyword transitions. +- `rstest-bdd` binds step fixtures by parameter name. No-op inspection steps + still need the parameter named `world`; `_world` is treated as a missing + fixture. +- The repository-wide `make fmt` target currently reaches unrelated Markdown + line-length failures after `cargo fmt --all` succeeds. This was observed + before any task-specific documentation completion check. +- `make fmt` uses `mdformat-all`, which invokes the legacy `markdownlint` + binary rather than `markdownlint-cli2`. The legacy binary did not read + `.markdownlint-cli2.jsonc`, so it reported table and heading line-length + failures that `make markdownlint` correctly ignored. ## Decision Log @@ -182,6 +201,16 @@ Success is observable when: `common/src/rstest/tests.rs` in place. Rationale: the repository's 400-line limit is a hard constraint, not a cleanup suggestion. Date/Author: 2026-04-23 / plan author. +- Decision: expose `LocalSlot` as a public `u32` newtype instead of the draft + `u16` shape. Rationale: the normalizer can keep a simple infallible API + without an overflow panic or Clippy-forbidden `expect`, while preserving the + same deterministic equality and ordering contract. Date/Author: 2026-05-01 / + implementation. +- Decision: add `.markdownlint.json` mirroring `.markdownlint-cli2.jsonc`. + Rationale: `make fmt` shells out to `markdownlint --fix` through + `mdformat-all`, while `make markdownlint` uses `markdownlint-cli2`; both + entry points now share the same table, heading, and line-length policy. + Date/Author: 2026-05-01 / implementation. ## Context and orientation @@ -451,6 +480,18 @@ Expected success signals: ## Outcomes & Retrospective -Pending implementation. This draft is complete enough for review and approval, -but no code, roadmap, or design-document completion state should change until -the implementation turn is approved and finished. +Roadmap item 8.1.3 is complete. `whitaker-common` now exposes public argument +and paragraph fingerprint models through `common::rstest`, with deterministic +paragraph local-slot normalization owned by `ParagraphNormalizer`. Unsupported +arguments and unknown paragraph shapes remain explicit in the model layer for +later lint policy decisions. + +The implementation added focused unit coverage and a public-API-first +`rstest-bdd` behaviour harness. The design document records the final API +placement and the `u32` slot ordinal refinement. The roadmap marks 8.1.3 done. + +The main lesson was that Markdown validation had two entry points with +different configuration discovery: `make markdownlint` used +`markdownlint-cli2`, while `make fmt` used legacy `markdownlint --fix` through +`mdformat-all`. Adding `.markdownlint.json` aligned the formatter with the +existing repository policy and made the full gate set reproducible. diff --git a/docs/lints-for-rstest-fixtures-and-test-hygiene.md b/docs/lints-for-rstest-fixtures-and-test-hygiene.md index 57c8982c..d148e218 100644 --- a/docs/lints-for-rstest-fixtures-and-test-hygiene.md +++ b/docs/lints-for-rstest-fixtures-and-test-hygiene.md @@ -257,6 +257,31 @@ Local identifiers should be normalized to deterministic slots by first-appearance order. Deep Abstract Syntax Tree (AST) canonicalization is intentionally out of scope. +### Implementation decisions for 8.1.3 + +Roadmap item 8.1.3 implements the shared fingerprint model layer in +`common::rstest`. The argument models live in +`common/src/rstest/argument_fingerprint.rs`, and the paragraph models live in +`common/src/rstest/paragraph_fingerprint.rs`. The public API is re-exported +from both `whitaker_common::rstest` and the crate root so later lint crates can +construct fingerprints without depending on compiler-private types. + +Argument fingerprints are represented by `ArgFingerprint` and `ArgAtom`. +`ArgAtom::Unsupported` remains an explicit atom in the positional sequence +rather than being dropped, so later lint passes can distinguish an unsupported +argument from a shorter, groupable argument list. + +Paragraph fingerprints are represented by `ParagraphFingerprint`, `StmtShape`, +`ExprShape`, `CalleeShape`, and `LocalSlot`. Local-name normalization is +builder-driven through `ParagraphNormalizer`, which assigns `LocalSlot` values +by first appearance order within a paragraph. This means two paragraphs with +the same statement structure and renamed locals compare equal when those locals +appear in the same order. + +The draft design used a `u16` slot ordinal. The implementation uses a `u32` +ordinal to avoid a panic or fallible constructor in the normalizer while +preserving deterministic equality and ordering semantics. + ### Emission strategy Collect candidates during block/body checks, then emit during diff --git a/docs/roadmap.md b/docs/roadmap.md index 127137d1..7ef8683b 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -461,7 +461,7 @@ [rstest fixture and test hygiene lints](lints-for-rstest-fixtures-and-test-hygiene.md) §Integration constraints and §Lint A: call-site fixture extraction. Requires 1.1.1. -- [ ] 8.1.3. Add shared argument and paragraph fingerprint data models for +- [x] 8.1.3. Add shared argument and paragraph fingerprint data models for deterministic grouping across tests. See [rstest fixture and test hygiene lints](lints-for-rstest-fixtures-and-test-hygiene.md) §Lint A: call-site fixture extraction and §Lint C: repeated fixture From d023e542b56b7b1a346651eaaef9e800f1dfe1e2 Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 1 May 2026 01:00:55 +0200 Subject: [PATCH 3/6] Make validation tools discoverable in hooks Prepend the standard user-local binary directories to the Makefile path so hook-driven validation can find `cargo`, `markdownlint-cli2`, and related local developer tools even when the hook environment does not inherit an interactive shell path. Record the post-turn hook finding in the 8.1.3 ExecPlan so future runs explain why the Makefile carries the explicit path setup. --- Makefile | 2 ++ ...1-3-argument-and-paragraph-fingerprint-data-models.md | 9 +++++++++ 2 files changed, 11 insertions(+) diff --git a/Makefile b/Makefile index cfee51dc..b34d258b 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,7 @@ .PHONY: help all clean test build release lint fmt check-fmt markdownlint nixie publish-check typecheck install-smoke package-lints workflow-test workflow-test-deps verus kani verus-clone-detector kani-clone-detector +export PATH := $(HOME)/.cargo/bin:$(HOME)/.bun/bin:$(HOME)/.local/bin:$(PATH) + APP ?= whitaker-installer CARGO ?= cargo BUILD_JOBS ?= diff --git a/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md index 5ae33124..8a4c4119 100644 --- a/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md +++ b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md @@ -182,6 +182,10 @@ Success is observable when: binary rather than `markdownlint-cli2`. The legacy binary did not read `.markdownlint-cli2.jsonc`, so it reported table and heading line-length failures that `make markdownlint` correctly ignored. +- Post-turn hooks run `make` in an environment where user-local binary + directories are not guaranteed to be on `PATH`. The Makefile now prepends the + repository's expected user-local tool directories so `cargo`, + `markdownlint-cli2`, and related developer tools resolve consistently. ## Decision Log @@ -211,6 +215,11 @@ Success is observable when: `mdformat-all`, while `make markdownlint` uses `markdownlint-cli2`; both entry points now share the same table, heading, and line-length policy. Date/Author: 2026-05-01 / implementation. +- Decision: export a Makefile-local `PATH` containing `$(HOME)/.cargo/bin`, + `$(HOME)/.bun/bin`, and `$(HOME)/.local/bin`. Rationale: hook runs may not + inherit an interactive shell path, but the repository already documents and + uses tools installed in those user-local locations. Date/Author: 2026-05-01 / + implementation. ## Context and orientation From 248213a5782b52ac5fd308ff2ae3f73f58daf87d Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 4 May 2026 02:32:58 +0200 Subject: [PATCH 4/6] Scope tooling PATH to Makefile targets that need it Drop the global PATH prepend so default recipes inherit the caller's environment. Append `TOOL_PATH_SUFFIX` only for targets that invoke user-local binaries (tests, publish-check, fmt helpers, markdownlint, nixie, workflow tooling). Align the 8.1.3 ExecPlan sketch and BDD note with the implemented `LocalSlot(u32)` contract and test `expect()` policy. Co-authored-by: Cursor --- Makefile | 27 +++++++++++-------- ...t-and-paragraph-fingerprint-data-models.md | 4 +-- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index b34d258b..c6401474 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,9 @@ .PHONY: help all clean test build release lint fmt check-fmt markdownlint nixie publish-check typecheck install-smoke package-lints workflow-test workflow-test-deps verus kani verus-clone-detector kani-clone-detector -export PATH := $(HOME)/.cargo/bin:$(HOME)/.bun/bin:$(HOME)/.local/bin:$(PATH) +# Appended only on targets that invoke binaries commonly installed under these +# prefixes (cargo/bun/user-local), so the default recipe environment stays +# aligned with the caller's PATH. +TOOL_PATH_SUFFIX := $(HOME)/.cargo/bin:$(HOME)/.bun/bin:$(HOME)/.local/bin APP ?= whitaker-installer CARGO ?= cargo @@ -32,13 +35,14 @@ clean: ## Remove build artifacts $(CARGO) clean test: ## Run tests with warnings treated as errors - @command -v cargo-nextest >/dev/null || { echo "Install cargo-nextest (cargo install cargo-nextest)"; exit 1; } + @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v cargo-nextest >/dev/null || { echo "Install cargo-nextest (cargo install cargo-nextest)"; exit 1; } @# Prefer dynamic linking during local `cargo test` runs to avoid rustc_private @# linkage pitfalls when building cdylib-based lints; `publish-check` omits @# this flag to exercise production-like linking behaviour. @# Run tests with backup/restore safeguard in a single shell with trap @# to ensure cleanup runs even when tests fail. @set -eu; \ + export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; \ WHITAKER_BACKUP=""; \ HAD_WHITAKER=false; \ cleanup() { \ @@ -79,8 +83,8 @@ test: ## Run tests with warnings treated as errors fi workflow-test: workflow-test-deps ## Run opt-in GitHub workflow smoke tests with act + pytest - @command -v act >/dev/null || { echo "Install act to run workflow tests"; exit 1; } - @command -v $(UV) >/dev/null || { echo "uv is required for workflow tests"; exit 1; } + @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v act >/dev/null || { echo "Install act to run workflow tests"; exit 1; } + @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v $(UV) >/dev/null || { echo "uv is required for workflow tests"; exit 1; } @test -x "$(WORKFLOW_TEST_VENV)/bin/python" || { \ echo "workflow-test virtualenv is missing or invalid:"; \ echo " expected: $(WORKFLOW_TEST_VENV)/bin/python"; \ @@ -90,9 +94,9 @@ workflow-test: workflow-test-deps ## Run opt-in GitHub workflow smoke tests with @ACT_WORKFLOW_TESTS=1 $(WORKFLOW_TEST_VENV)/bin/python -m pytest tests/workflows workflow-test-deps: ## Install Python dependencies for workflow tests - @command -v $(UV) >/dev/null || { echo "uv is required for workflow tests"; exit 1; } - @$(UV) venv --allow-existing $(WORKFLOW_TEST_VENV) - @$(UV) pip install --python $(WORKFLOW_TEST_VENV)/bin/python -r tests/workflows/requirements.txt + @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v $(UV) >/dev/null || { echo "uv is required for workflow tests"; exit 1; } + @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; $(UV) venv --allow-existing $(WORKFLOW_TEST_VENV) + @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; $(UV) pip install --python $(WORKFLOW_TEST_VENV)/bin/python -r tests/workflows/requirements.txt target/%/$(APP): ## Build binary in debug or release mode manifest=$$(grep -l whitaker-installer */Cargo.toml crates/*/Cargo.toml); \ @@ -104,18 +108,18 @@ lint: ## Run Clippy with warnings denied fmt: ## Format Rust and Markdown sources $(CARGO) fmt --all - mdformat-all + export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; mdformat-all check-fmt: ## Verify formatting $(CARGO) fmt --all -- --check markdownlint: ## Lint Markdown files - $(MDLINT) '**/*.md' + export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; $(MDLINT) '**/*.md' nixie: # CI currently requires --no-sandbox; remove once nixie supports # environment variable control for this option - nixie --no-sandbox + export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; $(NIXIE) --no-sandbox typecheck: RUSTFLAGS="-C prefer-dynamic -Z force-unstable-if-unmarked $(RUST_FLAGS)" $(CARGO) check $(CARGO_FLAGS) @@ -147,7 +151,8 @@ install-smoke: ## Install whitaker-installer and verify basic functionality whitaker-installer --version >/dev/null publish-check: ## Build, test, and validate packages before publishing - @command -v cargo-nextest >/dev/null || { echo "Install cargo-nextest (cargo install cargo-nextest)"; exit 1; } + @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v cargo-nextest >/dev/null || { echo "Install cargo-nextest (cargo install cargo-nextest)"; exit 1; } + export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; \ PINNED_TOOLCHAIN=$$(awk -F '\"' '/^channel/ {print $$2}' rust-toolchain.toml); \ TOOLCHAIN="$$PINNED_TOOLCHAIN"; \ ORIG_DIR="$(CURDIR)"; \ diff --git a/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md index 8a4c4119..c8f56950 100644 --- a/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md +++ b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md @@ -118,7 +118,7 @@ Success is observable when: - BDD ergonomics risk: the repository has already hit `rstest-bdd` gotchas around `And` keyword binding and workspace-wide Clippy denials in tests. Mitigation: keep the BDD world small, prefer explicit `Given`/`When`/`Then` - transitions, and avoid `.expect()` in test code. + transitions, and use `.expect()` messages deliberately in tests. ## Progress @@ -334,7 +334,7 @@ pub struct ArgFingerprint { } #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct LocalSlot(u16); +pub struct LocalSlot(u32); #[derive(Clone, Debug, PartialEq, Eq, Hash)] pub enum CalleeShape { From fd4cfe07e584a4981aba9ab0275294c190766233 Mon Sep 17 00:00:00 2001 From: leynos Date: Thu, 7 May 2026 19:44:32 +0200 Subject: [PATCH 5/6] Use recipe-scoped validation tool paths Resolve review feedback by using shell-expanded user-local tool paths in Makefile recipes instead of freezing `HOME` or `PATH` at parse time. Update the 8.1.3 ExecPlan to describe the completed state and the recipe-level PATH handling accurately. --- Makefile | 26 +++++++++---------- ...t-and-paragraph-fingerprint-data-models.md | 23 +++++++++------- 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/Makefile b/Makefile index c6401474..f8596de5 100644 --- a/Makefile +++ b/Makefile @@ -3,7 +3,7 @@ # Appended only on targets that invoke binaries commonly installed under these # prefixes (cargo/bun/user-local), so the default recipe environment stays # aligned with the caller's PATH. -TOOL_PATH_SUFFIX := $(HOME)/.cargo/bin:$(HOME)/.bun/bin:$(HOME)/.local/bin +TOOL_PATH_SUFFIX = $$HOME/.cargo/bin:$$HOME/.bun/bin:$$HOME/.local/bin APP ?= whitaker-installer CARGO ?= cargo @@ -35,14 +35,14 @@ clean: ## Remove build artifacts $(CARGO) clean test: ## Run tests with warnings treated as errors - @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v cargo-nextest >/dev/null || { echo "Install cargo-nextest (cargo install cargo-nextest)"; exit 1; } + @export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; command -v cargo-nextest >/dev/null || { echo "Install cargo-nextest (cargo install cargo-nextest)"; exit 1; } @# Prefer dynamic linking during local `cargo test` runs to avoid rustc_private @# linkage pitfalls when building cdylib-based lints; `publish-check` omits @# this flag to exercise production-like linking behaviour. @# Run tests with backup/restore safeguard in a single shell with trap @# to ensure cleanup runs even when tests fail. @set -eu; \ - export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; \ + export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; \ WHITAKER_BACKUP=""; \ HAD_WHITAKER=false; \ cleanup() { \ @@ -83,8 +83,8 @@ test: ## Run tests with warnings treated as errors fi workflow-test: workflow-test-deps ## Run opt-in GitHub workflow smoke tests with act + pytest - @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v act >/dev/null || { echo "Install act to run workflow tests"; exit 1; } - @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v $(UV) >/dev/null || { echo "uv is required for workflow tests"; exit 1; } + @export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; command -v act >/dev/null || { echo "Install act to run workflow tests"; exit 1; } + @export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; command -v $(UV) >/dev/null || { echo "uv is required for workflow tests"; exit 1; } @test -x "$(WORKFLOW_TEST_VENV)/bin/python" || { \ echo "workflow-test virtualenv is missing or invalid:"; \ echo " expected: $(WORKFLOW_TEST_VENV)/bin/python"; \ @@ -94,9 +94,9 @@ workflow-test: workflow-test-deps ## Run opt-in GitHub workflow smoke tests with @ACT_WORKFLOW_TESTS=1 $(WORKFLOW_TEST_VENV)/bin/python -m pytest tests/workflows workflow-test-deps: ## Install Python dependencies for workflow tests - @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v $(UV) >/dev/null || { echo "uv is required for workflow tests"; exit 1; } - @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; $(UV) venv --allow-existing $(WORKFLOW_TEST_VENV) - @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; $(UV) pip install --python $(WORKFLOW_TEST_VENV)/bin/python -r tests/workflows/requirements.txt + @export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; command -v $(UV) >/dev/null || { echo "uv is required for workflow tests"; exit 1; } + @export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; $(UV) venv --allow-existing $(WORKFLOW_TEST_VENV) + @export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; $(UV) pip install --python $(WORKFLOW_TEST_VENV)/bin/python -r tests/workflows/requirements.txt target/%/$(APP): ## Build binary in debug or release mode manifest=$$(grep -l whitaker-installer */Cargo.toml crates/*/Cargo.toml); \ @@ -108,18 +108,18 @@ lint: ## Run Clippy with warnings denied fmt: ## Format Rust and Markdown sources $(CARGO) fmt --all - export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; mdformat-all + export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; mdformat-all check-fmt: ## Verify formatting $(CARGO) fmt --all -- --check markdownlint: ## Lint Markdown files - export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; $(MDLINT) '**/*.md' + export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; $(MDLINT) '**/*.md' nixie: # CI currently requires --no-sandbox; remove once nixie supports # environment variable control for this option - export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; $(NIXIE) --no-sandbox + export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; $(NIXIE) --no-sandbox typecheck: RUSTFLAGS="-C prefer-dynamic -Z force-unstable-if-unmarked $(RUST_FLAGS)" $(CARGO) check $(CARGO_FLAGS) @@ -151,8 +151,8 @@ install-smoke: ## Install whitaker-installer and verify basic functionality whitaker-installer --version >/dev/null publish-check: ## Build, test, and validate packages before publishing - @export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; command -v cargo-nextest >/dev/null || { echo "Install cargo-nextest (cargo install cargo-nextest)"; exit 1; } - export PATH="$(PATH):$(TOOL_PATH_SUFFIX)"; \ + @export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; command -v cargo-nextest >/dev/null || { echo "Install cargo-nextest (cargo install cargo-nextest)"; exit 1; } + export PATH="$$PATH:$(TOOL_PATH_SUFFIX)"; \ PINNED_TOOLCHAIN=$$(awk -F '\"' '/^channel/ {print $$2}' rust-toolchain.toml); \ TOOLCHAIN="$$PINNED_TOOLCHAIN"; \ ORIG_DIR="$(CURDIR)"; \ diff --git a/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md index c8f56950..4568d6c5 100644 --- a/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md +++ b/docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md @@ -11,8 +11,8 @@ This document must be maintained in accordance with `AGENTS.md`. The canonical plan file is `docs/execplans/8-1-3-argument-and-paragraph-fingerprint-data-models.md`. -This plan was approved for implementation on 2026-05-01. Code changes for -roadmap item 8.1.3 may proceed within the tolerances below. +This plan was approved and implemented on 2026-05-01; roadmap item 8.1.3 is now +complete. ## Purpose / big picture @@ -183,9 +183,11 @@ Success is observable when: `.markdownlint-cli2.jsonc`, so it reported table and heading line-length failures that `make markdownlint` correctly ignored. - Post-turn hooks run `make` in an environment where user-local binary - directories are not guaranteed to be on `PATH`. The Makefile now prepends the - repository's expected user-local tool directories so `cargo`, - `markdownlint-cli2`, and related developer tools resolve consistently. + directories are not guaranteed to be on `PATH`. Makefile recipes now + temporarily extend `PATH` for target commands with the repository's expected + user-local tool directories, so `cargo`, `markdownlint-cli2`, and related + developer tools resolve consistently during post-turn hooks without changing + the caller's global `PATH`. ## Decision Log @@ -215,11 +217,12 @@ Success is observable when: `mdformat-all`, while `make markdownlint` uses `markdownlint-cli2`; both entry points now share the same table, heading, and line-length policy. Date/Author: 2026-05-01 / implementation. -- Decision: export a Makefile-local `PATH` containing `$(HOME)/.cargo/bin`, - `$(HOME)/.bun/bin`, and `$(HOME)/.local/bin`. Rationale: hook runs may not - inherit an interactive shell path, but the repository already documents and - uses tools installed in those user-local locations. Date/Author: 2026-05-01 / - implementation. +- Decision: use target-scoped Makefile recipe `PATH` extensions containing + `$$HOME/.cargo/bin`, `$$HOME/.bun/bin`, and `$$HOME/.local/bin`. Rationale: + hook runs may not inherit an interactive shell path, but the repository + already documents and uses tools installed in those user-local locations; + shell-time expansion avoids freezing `HOME` or `PATH` when Make parses the + file. Date/Author: 2026-05-01 / implementation. ## Context and orientation From a1566406bad5c917babc0d34f7bc78c87ed3bdaa Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 8 May 2026 00:57:38 +0200 Subject: [PATCH 6/6] Document and exercise fingerprint helpers Add developer-guide coverage for the shared argument and paragraph fingerprint APIs introduced under `common::rstest`. Extend unit and property coverage for empty fingerprints, boundary atom values, `LocalSlot` roundtrips, and `ParagraphNormalizer` ordering and idempotence. --- Cargo.lock | 147 +++++++++++++++++++ common/Cargo.toml | 1 + common/src/rstest/tests/fingerprint.rs | 56 +++++++ common/src/rstest/tests/fingerprint_props.rs | 66 +++++++++ common/src/rstest/tests/mod.rs | 1 + docs/developers-guide.md | 62 ++++++++ 6 files changed, 333 insertions(+) create mode 100644 common/src/rstest/tests/fingerprint_props.rs diff --git a/Cargo.lock b/Cargo.lock index 7f49391b..5fd3fe84 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -114,6 +114,12 @@ dependencies = [ "rustversion", ] +[[package]] +name = "autocfg" +version = "1.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c08606f8c3cbf4ce6ec8e28fb0014a2c086708fe954eaa885384a6165172e7e8" + [[package]] name = "base64" version = "0.22.1" @@ -129,6 +135,21 @@ dependencies = [ "serde", ] +[[package]] +name = "bit-set" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "08807e080ed7f9d5433fa9b275196cfc35414f66a0c79d864dc51a0d825231a3" +dependencies = [ + "bit-vec", +] + +[[package]] +name = "bit-vec" +version = "0.8.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5e764a1d40d510daf35e07be9eb06e75770908c27d411ee6c92109c9840eaaf7" + [[package]] name = "bitflags" version = "2.11.0" @@ -850,6 +871,12 @@ dependencies = [ "spin", ] +[[package]] +name = "fnv" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" + [[package]] name = "foldhash" version = "0.1.5" @@ -1780,6 +1807,15 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c6673768db2d862beb9b39a78fdcb1a69439615d5794a1be50caa9bc92c81967" +[[package]] +name = "num-traits" +version = "0.2.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841" +dependencies = [ + "autocfg", +] + [[package]] name = "num_cpus" version = "1.17.0" @@ -1940,6 +1976,15 @@ version = "0.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "439ee305def115ba05938db6eb1644ff94165c5ab5e9420d1c1bcedbba909391" +[[package]] +name = "ppv-lite86" +version = "0.2.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "85eae3c4ed2f50dcfe72643da4befc30deadb458a9b590d720cde2f2b1e97da9" +dependencies = [ + "zerocopy", +] + [[package]] name = "predicates" version = "3.1.4" @@ -2024,6 +2069,31 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "proptest" +version = "1.11.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b45fcc2344c680f5025fe57779faef368840d0bd1f42f216291f0dc4ace4744" +dependencies = [ + "bit-set", + "bit-vec", + "bitflags", + "num-traits", + "rand", + "rand_chacha", + "rand_xorshift", + "regex-syntax", + "rusty-fork", + "tempfile", + "unarray", +] + +[[package]] +name = "quick-error" +version = "1.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a1d01941d82fa2ab50be1e79e6714289dd7cde78eba4c074bc5a4374f650dfe0" + [[package]] name = "quote" version = "1.0.45" @@ -2045,6 +2115,44 @@ version = "6.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf" +[[package]] +name = "rand" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44c5af06bb1b7d3216d91932aed5265164bf384dc89cd6ba05cf59a35f5f76ea" +dependencies = [ + "rand_chacha", + "rand_core", +] + +[[package]] +name = "rand_chacha" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d3022b5f1df60f26e1ffddd6c66e8aa15de382ae63b3a0c1bfc0e4d3e3f325cb" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.9.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "76afc826de14238e6e8c374ddcc1fa19e374fd8dd986b0d2af0d02377261d83c" +dependencies = [ + "getrandom 0.3.4", +] + +[[package]] +name = "rand_xorshift" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "513962919efc330f829edb2535844d1b912b0fbe2ca165d613e4e8788bb05a5a" +dependencies = [ + "rand_core", +] + [[package]] name = "redox_syscall" version = "0.5.18" @@ -2379,6 +2487,18 @@ version = "1.0.22" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d" +[[package]] +name = "rusty-fork" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc6bf79ff24e648f6da1f8d1f011e9cac26491b619e6b9280f2b47f1774e6ee2" +dependencies = [ + "fnv", + "quick-error", + "tempfile", + "wait-timeout", +] + [[package]] name = "same-file" version = "1.0.6" @@ -2930,6 +3050,12 @@ version = "1.19.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "562d481066bde0658276a35467c4af00bdc6ee726305698a55b86e61d7ad82bb" +[[package]] +name = "unarray" +version = "0.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eaea85b334db583fe3274d12b4cd1880032beab409c0d774be044d4480ab9a94" + [[package]] name = "unic-langid" version = "0.9.6" @@ -3251,6 +3377,7 @@ dependencies = [ "log", "logtest", "once_cell", + "proptest", "regex", "rstest", "rstest-bdd", @@ -3630,6 +3757,26 @@ dependencies = [ "synstructure", ] +[[package]] +name = "zerocopy" +version = "0.8.48" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "eed437bf9d6692032087e337407a86f04cd8d6a16a37199ed57949d415bd68e9" +dependencies = [ + "zerocopy-derive", +] + +[[package]] +name = "zerocopy-derive" +version = "0.8.48" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70e3cd084b1788766f53af483dd21f93881ff30d7320490ec3ef7526d203bad4" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "zerofrom" version = "0.1.7" diff --git a/common/Cargo.toml b/common/Cargo.toml index 92cb08d9..4f147272 100644 --- a/common/Cargo.toml +++ b/common/Cargo.toml @@ -30,6 +30,7 @@ unic-langid = { workspace = true } rstest = { workspace = true } rstest-bdd = { workspace = true } rstest-bdd-macros = { workspace = true } +proptest = "1" regex = "1.10.4" logtest = "2.0.0" diff --git a/common/src/rstest/tests/fingerprint.rs b/common/src/rstest/tests/fingerprint.rs index dcc05178..563c3900 100644 --- a/common/src/rstest/tests/fingerprint.rs +++ b/common/src/rstest/tests/fingerprint.rs @@ -94,6 +94,62 @@ fn paragraph_normalization_is_deterministic_across_runs() { ); } +#[rstest] +fn empty_argument_fingerprint_equals_another_empty() { + let first = ArgFingerprint::new([]); + let second = ArgFingerprint::new([]); + assert_eq!(first, second); + assert!(first.atoms().is_empty()); +} + +#[rstest] +fn empty_paragraph_fingerprint_equals_another_empty() { + let first = ParagraphFingerprint::new([]); + let second = ParagraphFingerprint::new([]); + assert_eq!(first, second); + assert!(first.shapes().is_empty()); +} + +#[rstest] +fn arg_atom_constructors_accept_empty_string() { + let fl = ArgAtom::fixture_local(""); + let cl = ArgAtom::const_lit(""); + let cp = ArgAtom::const_path(""); + assert_eq!(fl, ArgAtom::fixture_local("")); + assert_eq!(cl, ArgAtom::const_lit("")); + assert_eq!(cp, ArgAtom::const_path("")); +} + +#[rstest] +fn arg_atom_constructors_accept_long_string() { + let long: String = "x".repeat(4096); + let atom = ArgAtom::fixture_local(&long); + assert_eq!(atom, ArgAtom::fixture_local(long)); +} + +#[rstest] +fn local_slot_new_roundtrips_index() { + assert_eq!(LocalSlot::new(0).index(), 0); + assert_eq!(LocalSlot::new(u32::MAX).index(), u32::MAX); +} + +#[rstest] +fn paragraph_normalizer_returns_same_slot_for_repeated_name() { + let mut norm = ParagraphNormalizer::new(); + let first = norm.local_slot("foo"); + let second = norm.local_slot("foo"); + assert_eq!(first, second); +} + +#[rstest] +fn paragraph_normalizer_assigns_slots_in_first_appearance_order() { + let mut norm = ParagraphNormalizer::new(); + let zeta = norm.local_slot("zeta"); + let alpha = norm.local_slot("alpha"); + assert_eq!(zeta.index(), 0); + assert_eq!(alpha.index(), 1); +} + fn paragraph_for_renamed_locals(first_name: &str, second_name: &str) -> ParagraphFingerprint { let mut normalizer = ParagraphNormalizer::new(); ParagraphFingerprint::new([ diff --git a/common/src/rstest/tests/fingerprint_props.rs b/common/src/rstest/tests/fingerprint_props.rs new file mode 100644 index 00000000..6c1c2e70 --- /dev/null +++ b/common/src/rstest/tests/fingerprint_props.rs @@ -0,0 +1,66 @@ +//! Property tests for shared `rstest` fingerprint data models. + +use crate::rstest::{ArgAtom, ArgFingerprint, LocalSlot, ParagraphNormalizer}; +use proptest::prelude::*; + +proptest! { + /// Slot indices are assigned in strict first-appearance order: the first + /// distinct name always receives slot 0, the second slot 1, etc. + #[test] + fn paragraph_normalizer_assigns_slots_in_first_appearance_order( + names in prop::collection::vec("[a-z]{1,16}", 1..=32_usize) + ) { + let mut norm = ParagraphNormalizer::new(); + let mut seen: Vec = Vec::new(); + for name in &names { + let slot = norm.local_slot(name.as_str()); + if !seen.contains(name) { + let expected = seen.len() as u32; + prop_assert_eq!( + slot.index(), + expected, + "first appearance of {:?} expected slot {}, got {}", + name, + expected, + slot.index() + ); + seen.push(name.clone()); + } + } + } + + /// Calling `local_slot` twice with the same name returns the same slot. + #[test] + fn paragraph_normalizer_is_idempotent_for_same_name( + name in "[a-z]{1,16}", + prefix in prop::collection::vec("[a-z]{1,16}", 0..=8_usize) + ) { + let mut norm = ParagraphNormalizer::new(); + for p in &prefix { + let _ = norm.local_slot(p.as_str()); + } + let first = norm.local_slot(name.as_str()); + let second = norm.local_slot(name.as_str()); + prop_assert_eq!(first, second); + } + + /// Two `ArgFingerprint` values built from equal atom sequences must compare equal. + #[test] + fn argument_fingerprint_equality_holds_for_equal_sequences( + texts in prop::collection::vec("[a-z]{1,16}", 0..=16_usize) + ) { + let atoms: Vec = texts + .iter() + .map(|t| ArgAtom::fixture_local(t.as_str())) + .collect(); + let fp1 = ArgFingerprint::new(atoms.clone()); + let fp2 = ArgFingerprint::new(atoms); + prop_assert_eq!(fp1, fp2); + } + + /// A `LocalSlot` always roundtrips its index value. + #[test] + fn local_slot_roundtrips_index(index in 0_u32..=u32::MAX) { + prop_assert_eq!(LocalSlot::new(index).index(), index); + } +} diff --git a/common/src/rstest/tests/mod.rs b/common/src/rstest/tests/mod.rs index fd47cb51..9df51af8 100644 --- a/common/src/rstest/tests/mod.rs +++ b/common/src/rstest/tests/mod.rs @@ -1,6 +1,7 @@ //! Unit tests for shared `rstest` helpers. mod fingerprint; +mod fingerprint_props; use super::{ ExpansionTrace, ParameterBinding, RstestDetectionOptions, RstestParameter, RstestParameterKind, diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 376ccfac..4029645c 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -835,6 +835,68 @@ To handle a new `T` (e.g., a custom span type in a test harness): No adapter code is needed unless `T` is `rustc_span::Span`. +## Shared fingerprint helpers + +`common::rstest` exposes two families of pure data model for deterministic +grouping of `rstest` call sites. + +### Argument fingerprints + +`ArgFingerprint` stores a positional sequence of `ArgAtom` values in call-site +order. Use it to group helper calls that have the same argument shape +regardless of which fixtures are bound at the call site. + +```rust +use whitaker_common::{ArgAtom, ArgFingerprint}; + +let fp = ArgFingerprint::new([ + ArgAtom::fixture_local("db"), + ArgAtom::const_lit("42"), + ArgAtom::const_path("crate::defaults::TIMEOUT"), + ArgAtom::unsupported(), // retained explicitly; never silently dropped +]); + +assert_eq!(fp.atoms().len(), 4); +``` + +`ArgAtom` variants: + +| Variant | Constructor | Meaning | +| -------------- | ------------------------------- | ------------------------------- | +| `FixtureLocal` | `ArgAtom::fixture_local(name)` | Fixture-local parameter | +| `ConstLit` | `ArgAtom::const_lit(text)` | Stable literal value | +| `ConstPath` | `ArgAtom::const_path(def_path)` | Stable constant path | +| `Unsupported` | `ArgAtom::unsupported()` | Explicit positional placeholder | + +### Paragraph fingerprints + +`ParagraphFingerprint` stores an ordered sequence of `StmtShape` values. Use +`ParagraphNormalizer` to assign deterministic `LocalSlot` indices (by +first-appearance order) before constructing the fingerprint, so paragraphs with +renamed locals compare equal when they are structurally equivalent. + +```rust +use whitaker_common::{ + CalleeShape, ExprShape, LocalSlot, ParagraphFingerprint, + ParagraphNormalizer, StmtShape, +}; + +let mut norm = ParagraphNormalizer::new(); +let fp = ParagraphFingerprint::new([ + StmtShape::let_binding(ExprShape::call( + CalleeShape::def_path("crate::load"), + 0, + )), + StmtShape::mutable_call( + Some(norm.local_slot("result")), + CalleeShape::def_path("crate::prepare"), + ), +]); +``` + +`LocalSlot` indices are assigned in first-appearance order and are +deterministic across repeated normalization runs over the same name sequence. + ### Test coverage - **Unit tests** for the pure policy live in `common/src/rstest/tests.rs`.