diff --git a/Cargo.toml b/Cargo.toml index cb3a3a3a..59fe5673 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,6 +18,7 @@ pkg-fmt = "tgz" [dependencies] anyhow = "1" +# Production file output and integration tests share UTF-8 capability paths. camino = "1.2.4" cap-std = { version = "4.0.2", features = ["fs_utf8"] } clap = { version = "4", features = ["derive"] } diff --git a/Makefile b/Makefile index fe353d71..2737cce9 100644 --- a/Makefile +++ b/Makefile @@ -42,13 +42,7 @@ check-ripgrep: ## Verify ripgrep is available } check-static-regexes: check-ripgrep ## Reject hand-rolled static regular expressions - @status=0; \ - $(RG) -U --glob '*.rs' '\bstatic\b[^;=]*=\s*(?:[[:alnum:]_]+::)*LazyLock::new\s*\(\s*\|\|\s*(\{\s*)?(?:[[:alnum:]_]+::)*Regex::new' . || status=$$?; \ - case $$status in \ - 0) echo "static regular expressions must use lazy_regex!"; exit 1 ;; \ - 1) ;; \ - *) echo "failed to scan Rust sources (rg exit $$status)" >&2; exit $$status ;; \ - esac + @RG='$(RG)' scripts/check-static-regexes.sh . markdownlint: ## Lint Markdown files $(MDLINT) "**/*.md" diff --git a/docs/contents.md b/docs/contents.md index 6ed4430b..902578df 100644 --- a/docs/contents.md +++ b/docs/contents.md @@ -40,6 +40,8 @@ Rust documentation tests that avoid brittle or misleading examples. - [Rust testing with rstest fixtures](rust-testing-with-rstest-fixtures.md): Reference for the fixture and parameterization patterns used in tests. +- [Whitaker user's guide](whitaker-users-guide.md): Imported reference for + Whitaker lints, including path-level `no_std_fs_operations` exclusions. - [Trailing spaces](trailing-spaces.md): Notes on preserving Markdown hard line breaks and other trailing-space-sensitive content. diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 10431ae2..017b1122 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -67,6 +67,11 @@ restores the separator row with widths derived from the final table body. - `check-static-regexes`: Runs before Clippy as part of `make lint` and uses ripgrep (`rg`) to reject hand-rolled static regular expression declarations. + The scan lives in `scripts/check-static-regexes.sh` and rejects any `static` + that wraps `Regex::new` directly in a supported lazy-wrapper constructor — + `std::sync::LazyLock::new` or `once_cell::sync::Lazy::new`, whether spelled + directly or fully qualified — so `lazy_regex!` remains the sole sanctioned + idiom. `tests/static_regex_lint.rs` exercises every supported form. Contributors must install ripgrep locally; Continuous Integration (CI) installs the pinned version before running the lint gate. @@ -985,7 +990,43 @@ integration-test binary crates. The `#[expect(unused_macros)]` suppressions that previously guarded them were replaced by the export attribute when it became clear that multiple test binaries depend on them. -### 2.3. `test-macros` crate +### 2.3. Capability-scoped test filesystem access + +All integration tests under `tests/` use `camino::Utf8Path` and +`camino::Utf8PathBuf` for runtime paths and perform filesystem operations +through `cap_std::fs_utf8::Dir`. Absolute paths are retained only for process +boundaries such as `std::process::Command`; reads, writes, metadata queries, +copies, and permission changes use paths relative to an opened directory +capability. + +The `camino` and `cap-std` crates are normal dependencies because the +production CLI already uses them for file output. The integration suite shares +those dependencies instead of declaring redundant dev-dependencies. This keeps +production and test filesystem semantics aligned while avoiding two independent +version declarations. + +`tests/common/fs.rs` owns `TestDir`, the reusable temporary-directory boundary. +It converts `tempfile::TempDir`'s platform path to a UTF-8 path once, opens the +directory capability, and retains the `TempDir` guard for the test's lifetime. +Only integration tests may include this helper. Callers compose it by using +relative paths with `TestDir::directory()` and by deriving an absolute +`Utf8PathBuf` from `TestDir::path()` only when invoking a child process. It +must not absorb fixture-specific logic or production filesystem behaviour. + +Repository fixture access remains an explicit ambient boundary. Focused modules +such as `tests/cli_matrix/fixture_io.rs` open `CARGO_MANIFEST_DIR` once and +expose only capability-scoped fixture queries. New tests must follow the same +pattern instead of calling `std::fs`, storing `std::path::PathBuf`, or relying +on `Utf8Path::exists`, which performs an ambient metadata query. + +Whitaker's `no_std_fs_operations` lint supports `excluded_paths` for genuinely +unavoidable ambient boundaries. Prefer the capability pattern above; use a +path-level exclusion only when the boundary cannot be expressed through +`cap_std`, and record the reason beside the configuration. See the imported +[Whitaker user's guide](whitaker-users-guide.md#no_std_fs_operations) for the +configuration syntax and segment-matching rules. + +### 2.4. `test-macros` crate The `test-macros` workspace crate provides the `allow_fixture_expansion_lints` proc-macro attribute. It suppresses the `unused_braces` lint that `rstest` diff --git a/docs/repository-layout.md b/docs/repository-layout.md index 260379af..b0ef2d50 100644 --- a/docs/repository-layout.md +++ b/docs/repository-layout.md @@ -26,6 +26,7 @@ output and most test fixtures. │ ├── developers-guide.md │ ├── documentation-style-guide.md │ ├── repository-layout.md +│ ├── whitaker-users-guide.md │ └── users-guide.md ├── src/ │ ├── fences/ @@ -69,6 +70,8 @@ _Figure 1: Simplified repository tree._ the Markdown processing pipeline. - `docs/documentation-style-guide.md`: Documentation style rules imported from the shared Rust agent template. +- `docs/whitaker-users-guide.md`: Imported Whitaker lint reference, including + crate-level and path-level filesystem exclusion configuration. - `docs/adrs/`: Accepted architecture decision records. Add narrow decision records here when design choices need a durable audit trail. - `docs/execplans/`: Living execution plans and roadmap documents for larger @@ -98,8 +101,9 @@ _Figure 1: Simplified repository tree._ - `tests/*.rs`: Integration and behavioural tests for command-line workflows and Markdown transformations. -- `tests/common/`: Shared integration test support. Keep helpers explicit and - avoid direct environment mutation. +- `tests/common/`: Shared integration test support. `fs.rs` owns temporary + directory capability setup; keep other helpers explicit and avoid direct + environment mutation. - `tests/cli_matrix/`: Matrix-test support for command-line option combinations and invariants. - `tests/data/`: Input and expected-output fixtures. Treat these as reviewable diff --git a/docs/whitaker-users-guide.md b/docs/whitaker-users-guide.md new file mode 100644 index 00000000..a84c8333 --- /dev/null +++ b/docs/whitaker-users-guide.md @@ -0,0 +1,750 @@ +# Whitaker User's Guide + +Whitaker is a collection of opinionated Dylint lints for Rust. This guide +explains how to integrate the lints into a project and configure them. + +For contributors who want to develop new lints or work on Whitaker itself, see +the [Developer's Guide](developers-guide.md). + +## Quick Setup + +### Prerequisites + +Install `cargo-dylint` and `dylint-link`: + +```sh +cargo install cargo-dylint dylint-link +``` + +### Standalone installation (recommended) + +The simplest way to use Whitaker is via the standalone installer, which handles +setup automatically: + +```sh +cargo install whitaker-installer +whitaker-installer +whitaker --all +``` + +This: + +1. Installs `cargo-dylint` and `dylint-link` if not present + The installer first attempts to download pre-built dependency binaries from + Whitaker's GitHub Releases page for the current platform. If the release + asset is absent (HTTP 404 or 410), the installer skips `cargo binstall` and + falls back directly to building from source with `cargo install`, pinned to + the version recorded in the dependency manifest. If the release asset is + present but the download fails for another reason, the installer falls back + to `cargo binstall` when available and then to `cargo install`. When the + source-build path is taken, the installer reports that it is falling back to + Cargo, and on success it prints + `Installed from source with cargo install.`. After installation, + `cargo-dylint` is verified by running `cargo dylint --version`. + `dylint-link` is never executed: it is a linker wrapper that forwards its + arguments to the underlying linker, so it has no reliable self-reporting + subcommand. A release artefact is trusted once its checksum, extraction, and + executable permissions have been established, and a Cargo-managed copy is + checked by resolving it on `PATH` and comparing the version Cargo recorded. +2. Clones the Whitaker repository to a platform-specific data directory +3. Builds the lint libraries +4. Creates `whitaker` and `whitaker-ls` wrapper scripts. `whitaker` invokes + `cargo dylint` with the correct `DYLINT_LIBRARY_PATH`, and `whitaker-ls` + lists installed Whitaker suite libraries +5. Ensures the pinned Rust toolchain and components are installed via rustup + +After installation, run `whitaker --all` in any Rust project to lint it. Use +`whitaker-ls` to list the installed Whitaker suite libraries. + +On Windows, the installer's `PATH` check honours `PATHEXT` and falls back to +the usual executable suffixes when `PATHEXT` is unset, so a normal +Cargo-installed executable such as `dylint-link.exe` in +`%USERPROFILE%\.cargo\bin` is located correctly and then matched against the +version Cargo recorded for it, without needing a separate wrapper or manual +environment-variable workaround. + +**Options:** + +- `--cranelift` — Tell the installer to add the + `rustc-codegen-cranelift` component via `rustup component add`. The + `rustc-codegen-cranelift` component is not included in the standard nightly + toolchain, so enable `--cranelift` when a project or CI pipeline requires the + Cranelift back-end and would otherwise need an explicit + `rustc-codegen-cranelift` component-add step before running the installer. +- `--skip-deps` — Skip `cargo-dylint`/`dylint-link` installation check +- `--skip-wrapper` — Skip wrapper script generation (prints + `DYLINT_LIBRARY_PATH` instructions instead) +- `--no-update` — Don't update existing repository clone + +### Adding Whitaker to a project + +Add the following to the workspace `Cargo.toml`: + +```toml +[workspace.metadata.dylint] +libraries = [ + { git = "https://github.com/leynos/whitaker", pattern = "whitaker_suite" } +] +``` + +Then run the lints: + +```sh +cargo dylint --all +``` + +### Version pinning + +For reproducible builds, pin to a specific release tag or commit: + +```toml +[workspace.metadata.dylint] +libraries = [ + { git = "https://github.com/leynos/whitaker", pattern = "whitaker_suite", tag = "v0.1.0" } +] +``` + +Or pin to a specific commit: + +```toml +[workspace.metadata.dylint] +libraries = [ + { git = "https://github.com/leynos/whitaker", pattern = "whitaker_suite", rev = "abc123def456" } +] +``` + +### Rolling release downloads + +Whitaker publishes a `rolling` pre-release tag that is continuously updated and +overwritten on every push to `main`. It is intended for early adopters who want +the latest available build outputs before the next stable release is cut. + +Rolling releases are best-effort builds. If some matrix legs fail, Whitaker +still publishes the artefacts that were built successfully. For example, a +target-specific `cargo-dylint` archive may be missing from one rolling release +even though other target archives were updated successfully. Do not assume that +every supported target is present in every rolling release. + +Stable releases differ from `rolling`: a stable tag is expected to contain the +complete artefact set for the release. For production installs, pin to a stable +release tag rather than consuming `rolling`. + +Scripts or CI pipelines that consume rolling-release archives should verify +that the required target archive exists before proceeding. Treat missing +archives as an expected condition for rolling releases rather than assuming the +artefact set is complete. + +### Selecting individual lints + +To load specific lints instead of the full suite, specify each lint explicitly: + +```toml +[workspace.metadata.dylint] +libraries = [ + { git = "https://github.com/leynos/whitaker", pattern = "crates/module_max_lines" }, + { git = "https://github.com/leynos/whitaker", pattern = "crates/no_expect_outside_tests" } +] +``` + +### Standard vs Experimental Lints + +Whitaker lints are divided into two categories: + +- **Standard lints** are stable, well-tested, and included in the default suite. + They are recommended for general use and have predictable behaviour. +- **Experimental lints** are newer or more aggressive checks that may produce + false positives or undergo breaking changes between releases. They must be + explicitly enabled. + +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 + +#### Via standalone installer + +```sh +whitaker-installer --experimental +``` + +This enables experimental suite features when building `whitaker_suite`. To +build experimental lints as individual libraries, combine it with +`--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 + +Configure lint behaviour in `dylint.toml` at the workspace root: + +```toml +# Diagnostic language (default: en-GB) +locale = "cy" + +# Module size threshold (default: 400) +[module_max_lines] +max_lines = 500 + +# Conditional branch limit (default: 2) +[conditional_max_n_branches] +max_branches = 3 + +# Custom test attributes +[no_expect_outside_tests] +additional_test_attributes = ["my_framework::test", "wasm_bindgen_test"] + +# Additional test markers for `test_must_not_have_example` +[test_must_not_have_example] +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 + +Whitaker supports multiple languages for diagnostic messages. Set the locale +via the `DYLINT_LOCALE` environment variable or in `dylint.toml`: + +```toml +locale = "cy" +``` + +Available locales: + +- `en-GB` (default) - English +- `cy` - Welsh (Cymraeg) +- `gd` - Scottish Gaelic (Gàidhlig) + +______________________________________________________________________ + +## Available Lints + +### `bumpy_road_function` + +#### Purpose + +Detects functions with multiple distinct clusters of nested conditional +complexity. + +#### Scope and behaviour + +Flags a function when peak detection finds two or more separated complexity +regions above the configured threshold. Detection smooths the local complexity +signal with the configured `window` and only considers peaks spanning at least +`min_bump_lines`. + +The default threshold was lowered from 3.0 to 2.5 to detect bumpy road patterns +in match expressions with nested conditionals. The moving-average smoothing +(window=3) reduces raw peaks by approximately 15–20%, so a threshold of 3.0 can +mask genuine two-bump patterns in match arms with nested `if` guards. + +#### Configuration + +```toml +[bumpy_road_function] +threshold = 2.5 # Raise to 3.0 or higher to reduce false positives +window = 3 +min_bump_lines = 2 +``` + +#### What is allowed + +- A single complexity peak in a function. +- Simple predicates that remain below the configured threshold. + +#### What is denied + +- Two or more separated complexity peaks above the configured threshold. + +#### How to fix + +Split complex regions into helper functions and simplify branch-heavy +predicates. + +______________________________________________________________________ + +### `conditional_max_n_branches` + +Limits the complexity of conditional predicates by enforcing a maximum number +of boolean branches. + +**Configuration:** + +```toml +[conditional_max_n_branches] +max_branches = 2 +``` + +The default threshold is 2 branches. A predicate like `a && b && c` has three +branches and would trigger the lint. + +**How to fix:** Extract complex conditions into helper functions: + +```rust +// Before: Too many branches +if condition_a && condition_b && condition_c { + // action +} + +// After: Extract to helper function +fn should_proceed() -> bool { + condition_a && condition_b && condition_c +} + +if should_proceed() { + // action +} +``` + +______________________________________________________________________ + +### `function_attrs_follow_docs` + + +#### Purpose + +Ensures doc comments appear before other outer attributes on functions, +methods, and trait methods. + + +#### Scope and behaviour + +When attributes are generated or reordered by a procedural macro (for example, +`rstest` or `derive`), the lint recovers the original source span from the +macro expansion chain. Attributes whose spans cannot be traced back to any +user-written source location (macro-only glue) are silently excluded from the +ordering check, so the lint never fires on compiler- or macro-generated code +that the developer cannot edit. + + +#### Configuration + +`function_attrs_follow_docs` has no configuration knobs. + + +#### What is allowed + +- Doc comments that appear before every other outer attribute on the same + function, method, or trait method. +- Macro-generated attributes whose spans are excluded because they are + macro-only. +- Inner attributes, which are outside the lint's scope. + + +#### What is denied + +- Outer attributes that appear before a doc comment on the same function, + method, or trait method. +- Macro-expanded attributes that recover to a user-editable source span and + sort before the doc comment. + + +#### How to fix + +Move doc comments so they appear before other outer attributes: + +```rust +// Wrong +#[inline] +/// This function does something. +fn example() {} + +// Correct +/// This function does something. +#[inline] +fn example() {} +``` + +With `rstest`, place the doc comment before all attributes, including the test +annotation: + +```rust +// Wrong +#[rstest] +#[case(1, 2, 3)] +/// Verifies addition. +fn adds(#[case] a: i32, #[case] b: i32, #[case] expected: i32) { + assert_eq!(a + b, expected); +} + +// Correct +/// Verifies addition. +#[rstest] +#[case(1, 2, 3)] +fn adds(#[case] a: i32, #[case] b: i32, #[case] expected: i32) { + assert_eq!(a + b, expected); +} +``` + +______________________________________________________________________ + +### `module_max_lines` + +Warns when modules exceed a configurable line count threshold. + +**Configuration:** + +```toml +[module_max_lines] +max_lines = 400 +``` + +**How to fix:** Split large modules into smaller, focused submodules. + +______________________________________________________________________ + +### `module_must_have_inner_docs` + +Enforces that every module begins with an inner documentation comment (`//!`). + +**How to fix:** + +```rust +mod my_module { + //! Explain the module's purpose here. + pub fn value() {} +} +``` + +______________________________________________________________________ + +### `no_expect_outside_tests` + + +#### Purpose + +Detect test attributes correctly so `no_expect_outside_tests` can allow +`.expect()` in recognized test-only code while still flagging production use. + + +#### Scope and behaviour + +Whitaker recognizes `#[test]`, prelude-qualified `#[test]` forms, +`#[tokio::test]`, `#[async_std::test]`, `#[gpui::test]`, `#[rstest]`, +`#[rstest::rstest]`, `#[rstest_parametrize]`, `#[rstest::rstest_parametrize]`, +`#[case]`, and `#[rstest::case]` by default. The `additional_test_attributes` +setting extends that matching list with project-specific markers, so the lint +treats those annotated functions as tests too. + + +#### Configuration + +```toml +[no_expect_outside_tests] +additional_test_attributes = ["my_framework::test", "wasm_bindgen_test"] +``` + +Set `additional_test_attributes` to an array of attribute paths written as +strings. Each entry should match the path Whitaker sees on the test function, +for example `my_framework::test` or `wasm_bindgen_test`. + + +#### Ancestor context propagation + +`additional_test_attributes` now apply during ancestor context detection as +well as direct annotation matching. If a parent function is annotated with a +configured custom test attribute, Whitaker treats nested code within that +function as test context too, so `.expect()` remains allowed throughout that +ancestry chain. + +```rust +// dylint.toml +// [no_expect_outside_tests] +// additional_test_attributes = ["my_framework::test"] + +#[my_framework::test] +async fn my_test() { + helper(); // allowed — ancestor is a recognized test function +} + +fn helper() { + let v: Option = Some(1); + let _ = v.expect("value present"); // allowed — called from within test ancestry +} +``` + + +#### What is allowed + +- Default markers such as `#[test]`, `#[::test]`, + `#[::std::prelude::v1::test]`, `#[tokio::test]`, `#[async_std::test]`, + `#[gpui::test]`, `#[rstest]`, `#[rstest::rstest]`, `#[rstest_parametrize]`, + `#[rstest::rstest_parametrize]`, `#[case]`, and `#[rstest::case]` +- Project-specific markers listed in `additional_test_attributes`, such as + `#[wasm_bindgen_test]` + + +#### What is denied + +Functions using `.expect()` will still be flagged when their test attribute is +not in Whitaker's default list and is not listed in +`additional_test_attributes`. + + +#### How to fix + +- Add the missing test marker to `additional_test_attributes` if the function is + genuinely part of a supported test framework +- Change the attribute usage to a recognized form such as `#[test]`, + `#[::test]`, `#[::std::prelude::v1::test]`, `#[tokio::test]`, + `#[async_std::test]`, `#[gpui::test]`, `#[rstest]`, `#[rstest::rstest]`, + `#[rstest_parametrize]`, `#[rstest::rstest_parametrize]`, `#[case]`, or + `#[rstest::case]` where appropriate +- If the function is not test-only code, replace `.expect()` with explicit error + handling such as `?` or `map_err` + +______________________________________________________________________ + +### `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, loads +configuration defaults, and passively collects local helper calls inside strict +`#[rstest]` tests, fingerprinting fixture-local, literal, `const`, and `static` +arguments for later aggregation. The lint remains diagnostic-silent: threshold +evaluation and actionable diagnostics are tracked by 8.2.3, while UI pass/fail +coverage is tracked by 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 + +- 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. + +______________________________________________________________________ + +### `test_must_not_have_example` + +Warns when test function documentation includes example headings (for example +`# Examples`) or fenced code blocks. + +**Configuration:** + +```toml +[test_must_not_have_example] +additional_test_attributes = ["actix_rt::test", "my_framework::test"] +``` + +Use `additional_test_attributes` for frameworks not covered by default test +markers such as `#[test]`, `#[tokio::test]`, `#[async_std::test]`, +`#[gpui::test]`, and `#[rstest]`. + +**How to fix:** Keep test docs focused on intent and assertions, and move +example/tutorial snippets into user-facing documentation. + +```rust +// Before +#[test] +/// # Examples +/// ```rust +/// assert_eq!(sum(2, 2), 4); +/// ``` +fn sums_values() { /* ... */ } + +// After +#[test] +/// Verifies summation handles two positive integers. +fn sums_values() { /* ... */ } +``` + +______________________________________________________________________ + +### `no_std_fs_operations` + +Enforces capability-based filesystem access by forbidding direct use of +`std::fs` operations. + +**Configuration:** + +```toml +[no_std_fs_operations] +excluded_crates = ["my_cli_entrypoint", "my_test_utilities"] +excluded_paths = ["my_app::legacy_io", "my_app::bin::migrate"] +``` + +The `excluded_crates` option allows specified crates to use `std::fs` +operations without triggering diagnostics. This is useful for: + +- CLI entry points where ambient filesystem access is the intended boundary +- Test support utilities that manage fixtures with ambient access +- Build scripts or code generators that require direct filesystem operations + +The `excluded_paths` option narrows an exclusion to individual modules rather +than a whole crate. Each entry is a fully qualified path anchored at the crate +identifier, and it matches on segment boundaries: `my_app::legacy_io` exempts +that module and everything nested beneath it (for example +`my_app::legacy_io::reader`), but never a sibling such as +`my_app::legacy_io_utils`. Reach for this when only a bounded corner of a crate +needs ambient filesystem access, while the rest stays under the capability +policy. + +> **Note:** For both options, use Rust identifiers (underscores), not Cargo +> package names (hyphens). For example, use `my_cli_app` rather than +> `my-cli-app`, and `my_app::legacy_io` rather than `my-app::legacy_io`. +> +> **Tip:** For an ad hoc, single-site exemption that travels with the code, a +> standard `#[allow(no_std_fs_operations)]` attribute on the item or module +> also works, since the lint honours Rust's lint-level attributes. + +**How to fix:** Replace `std::fs` with `cap_std`: + +```rust +// Before +use std::fs; +fn read_config() -> std::io::Result { + fs::read_to_string("config.toml") +} + +// After +use cap_std::fs::Dir; +use camino::Utf8Path; +fn read_config(config_dir: &Dir, path: &Utf8Path) -> std::io::Result { + config_dir.read_to_string(path) +} +``` + +______________________________________________________________________ + +### `no_unwrap_or_else_panic` + +Denies panicking `unwrap_or_else` fallbacks on `Option`/`Result`, including +tests. Doctest runs remain exempt. + +**Configuration:** + +```toml +[no_unwrap_or_else_panic] +allow_in_main = true +``` + +**What is allowed:** + +- Panicking `unwrap_or_else` fallbacks inside doctests +- Panicking `unwrap_or_else` fallbacks inside `main` when + `allow_in_main = true` +- `unwrap_or_else(|| panic!("value was {:?}", value))` inside test code when + the closure interpolates a runtime value into the panic message +- Non-panicking `unwrap_or_else` fallbacks + +**What is denied:** + +- `unwrap_or_else(|| panic!(..))` outside tests, subject to the standard + denial +- `unwrap_or_else(|| panic!(..))` in tests unless the `panic!` message + interpolates runtime values +- `unwrap_or_else(|| panic!("static message"))` in tests; use + `.expect("static message")` instead +- `unwrap_or_else(|| value.unwrap())` + +**How to fix:** Propagate errors with `?` or use `.expect()` with a clear +message if a panic is truly intended. In tests, replace +`unwrap_or_else(|| panic!("msg"))` with `.expect("msg")` for clarity and +brevity, unless the closure needs to interpolate runtime state for a more +useful diagnostic. The rule denies only closures whose `panic!` message does +not meet the interpolated-only test exception. When the closure contains a +static string literal in tests, prefer `.expect("static message")`; only +interpolated-only `panic!` fallbacks are permitted there. + +## Clone Detection: AST Feature Extraction + +Whitaker's experimental clone detector runs in two passes. Pass A is a token +scan over the workspace; Pass B lifts each candidate span into an abstract +syntax tree (AST) substrate that later scoring will consume. This release ships +the Pass B substrate only — it does not yet report clones (see below). + +**What the substrate does.** Given a source file and a candidate byte range, +`lower_span` validates a non-empty, UTF-8-aligned `ByteSpan`, parses the +supplied source, and lowers the smallest syntax subtree that covers the span +into a parser-independent `NormalizedTree`. Working from the smallest covering +subtree keeps the representation tied to the candidate rather than the whole +file. + +**Feature vectors.** From a `NormalizedTree`, extraction derives a +deterministic set of features: + +- exact counts of each syntax kind; +- dyadic fixed-point depth weighting (`2^63 >> depth`), so weights halve with + depth and collapse to zero beyond depth 63; +- production bigrams and trigrams (parent-to-child and + grandparent-to-parent-to-child kind sequences); and +- an opaque hexadecimal canonical hash of the normalized subtree. + +**Schema-versioned hashes.** Every canonical hash mixes in +`PARSER_SCHEMA_VERSION`, which is tied to the pinned parser snapshot. A +parser-schema change therefore changes every hash by design, intentionally +invalidating persisted snapshots and caches so stale AST fingerprints are never +reused across incompatible parser versions. + +**Parser feature.** `whitaker_clones_core` enables its exact-pinned +`ra_ap_syntax` parser adapter through the default `parser` feature. The feature +is on by default; building the crate with it disabled makes `lower_span` return +`AstError::ParserUnavailable` instead of lowering anything. + +**Not yet emitted.** Type-3 clone scoring and SARIF Run 1 emission are deferred +to roadmap item 7.3.2. This release builds and exposes the AST substrate only; +it does not score clones or emit AST-based SARIF results. + +Contributors maintaining the pinned parser should follow the +[`ra_ap_syntax` re-pinning runbook](developers-guide.md#ra_ap_syntax-re-pinning-runbook) +in the Developer's Guide. diff --git a/scripts/check-static-regexes.sh b/scripts/check-static-regexes.sh new file mode 100755 index 00000000..987311eb --- /dev/null +++ b/scripts/check-static-regexes.sh @@ -0,0 +1,40 @@ +#!/usr/bin/env bash +# Reject hand-rolled static regular expressions that bypass the `lazy_regex!` +# convention. +# +# The guard scans Rust sources for `static` declarations that wrap `Regex::new` +# directly in a supported lazy-wrapper constructor. Two wrapper families are +# supported, each matched whether spelled directly or fully qualified: +# +# * `std::sync::LazyLock::new` +# * `once_cell::sync::Lazy::new` +# +# Usage: check-static-regexes.sh [SCAN_DIR] +# +# SCAN_DIR defaults to the current directory. The RG environment variable +# overrides the ripgrep command (default: `rg`). It is split on whitespace, so +# it may carry arguments — for example `RG='rg --pcre2'` — matching the way the +# Makefile's `$(RG)` expansion behaved before the scan was extracted here. +# +# Exit status: +# 0 no prohibited declaration found +# 1 a prohibited declaration was found (diagnostic on stdout) +# * ripgrep failed to scan (diagnostic on stderr; rg's status propagated) +set -euo pipefail + +read -r -a rg_cmd <<<"${RG:-rg}" +scan_dir="${1:-.}" + +# `(?:[[:alnum:]_]+::)*` absorbs any module qualification (for example the +# `once_cell::sync::` in `once_cell::sync::Lazy::new`), so both the direct and +# fully qualified spellings of each supported constructor are rejected. +# `(?:move\s+)?` covers `move` closures such as `LazyLock::new(move || ...)`. +pattern='\bstatic\b[^;=]*=\s*(?:[[:alnum:]_]+::)*(?:LazyLock|Lazy)::new\s*\(\s*(?:move\s+)?\|\|\s*(\{\s*)?(?:[[:alnum:]_]+::)*Regex::new' + +status=0 +"${rg_cmd[@]}" -U --glob '*.rs' "$pattern" "$scan_dir" || status=$? +case $status in + 0) echo "static regular expressions must use lazy_regex!"; exit 1 ;; + 1) exit 0 ;; + *) echo "failed to scan Rust sources (rg exit $status)" >&2; exit "$status" ;; +esac diff --git a/tests/cli.rs b/tests/cli.rs index b110114e..fea9a289 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -7,14 +7,15 @@ //! - Processing of Markdown files through the CLI interface use assert_cmd::Command; -use camino::{Utf8Path, Utf8PathBuf}; -use cap_std::{ambient_authority, fs_utf8::Dir}; +use camino::Utf8Path; use rstest::rstest; -use tempfile::{TempDir, tempdir}; #[macro_use] #[path = "common/mod.rs"] mod common; +#[path = "common/fs.rs"] +mod test_fs; +use test_fs::TestDir; #[path = "cli/ellipsis.rs"] mod ellipsis; #[path = "support/fixtures.rs"] @@ -52,8 +53,9 @@ fn test_cli_version_flag() { /// and asserts that the output is the expected fixed table. #[rstest] fn test_cli_process_file(broken_table: Vec) { - let dir = tempdir().expect("failed to create temporary directory"); - let (directory, parent_path) = capability_directory(&dir); + let dir = TestDir::new().expect("failed to create temporary directory"); + let directory = dir.directory(); + let parent_path = dir.path(); let file_name = Utf8Path::new("sample.md"); directory .write(file_name, format!("{}\n", broken_table.join("\n"))) @@ -71,8 +73,9 @@ fn test_cli_process_file(broken_table: Vec) { fn cli_output_modes_snapshot_table_prose() { let input = include_str!("data/cli-output-parity.dat"); let expected = include_str!("data/cli-output-parity.expected.md"); - let dir = tempdir().expect("failed to create temporary directory"); - let (directory, parent_path) = capability_directory(&dir); + let dir = TestDir::new().expect("failed to create temporary directory"); + let directory = dir.directory(); + let parent_path = dir.path(); let stdout_path = parent_path.join("stdout.md"); let in_place_path = parent_path.join("in-place.md"); directory @@ -305,8 +308,9 @@ fn test_cli_footnotes_option() { /// Executes an in-place rewrite with the provided flags and asserts idempotence. fn run_in_place(flags: &[&str], input: &str, expected: &str) { - let dir = tempdir().expect("failed to create temporary directory"); - let (directory, parent_path) = capability_directory(&dir); + let dir = TestDir::new().expect("failed to create temporary directory"); + let directory = dir.directory(); + let parent_path = dir.path(); let file_name = Utf8Path::new("sample.md"); let file_path = parent_path.join(file_name); directory @@ -353,15 +357,6 @@ fn run_in_place(flags: &[&str], input: &str, expected: &str) { assert_eq!(out2, out); } -/// Opens a temporary directory as the capability-scoped I/O boundary for a test. -fn capability_directory(tempdir: &TempDir) -> (Dir, Utf8PathBuf) { - let path = Utf8PathBuf::from_path_buf(tempdir.path().to_path_buf()) - .expect("temporary directory path is UTF-8"); - let directory = Dir::open_ambient_dir(&path, ambient_authority()) - .expect("failed to open temporary directory"); - (directory, path) -} - /// Ensures `--in-place` rewrites files correctly for multiple flag combinations. #[rstest] #[case(&["--fences"], "Rust\n```\nfn main() {}\n```\n", "```rust\nfn main() {}\n```\n")] diff --git a/tests/cli_frontmatter.rs b/tests/cli_frontmatter.rs index 4b9eb694..39648a8b 100644 --- a/tests/cli_frontmatter.rs +++ b/tests/cli_frontmatter.rs @@ -3,18 +3,30 @@ use assert_cmd::Command; use rstest::{fixture, rstest}; +#[path = "common/fs.rs"] +mod test_fs; +use test_fs::TestDir; + /// Fixture providing an in-place test runner closure. #[fixture] fn in_place_runner() -> impl Fn(&[&str], &str, &str) { |args: &[&str], input: &str, expected: &str| { - let temp = tempfile::NamedTempFile::new().expect("create temp file"); - std::fs::write(temp.path(), input).expect("write temp file"); + let dir = TestDir::new().expect("create temp directory"); + dir.directory() + .write("input.md", input) + .expect("write temp file"); + let file_path = dir.path().join("input.md"); let mut cmd = Command::cargo_bin("mdtablefix").expect("find binary"); - cmd.arg("--in-place").args(args).arg(temp.path()); + cmd.arg("--in-place") + .args(args) + .arg(file_path.as_std_path()); cmd.assert().success(); - let actual = std::fs::read_to_string(temp.path()).expect("read temp file"); + let actual = dir + .directory() + .read_to_string("input.md") + .expect("read temp file"); assert_eq!(actual, expected, "in-place content mismatch"); } } diff --git a/tests/cli_matrix.rs b/tests/cli_matrix.rs index 14dc6e68..5f62b5d6 100644 --- a/tests/cli_matrix.rs +++ b/tests/cli_matrix.rs @@ -12,6 +12,7 @@ use support::{ PhysicalCase, WrapVariant, assert_transform_invariants, + fixture_exists, fixture_path, has_flag, is_case_id, @@ -39,11 +40,11 @@ fn matrix_case_ids_accept_documented_characters() { fn matrix_case_fixtures_are_dat_files() { for case in BASE_MATRIX_CASES { let fixture = fixture_path(case.fixture); - assert!(fixture.exists(), "missing fixture {}", fixture.display()); - assert_eq!( - fixture.extension().and_then(|ext| ext.to_str()), - Some("dat") + assert!( + fixture_exists(case.fixture).expect("inspect matrix fixture"), + "missing fixture {fixture}" ); + assert_eq!(fixture.extension(), Some("dat")); } } diff --git a/tests/cli_matrix/fixture_io.rs b/tests/cli_matrix/fixture_io.rs new file mode 100644 index 00000000..e8c98398 --- /dev/null +++ b/tests/cli_matrix/fixture_io.rs @@ -0,0 +1,36 @@ +//! Capability-scoped fixture access for CLI matrix tests. + +use anyhow::{Context as _, Result}; +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ambient_authority, fs_utf8::Dir}; + +/// Returns the repository-relative path to a matrix fixture. +pub(crate) fn fixture_path(file_name: &str) -> Utf8PathBuf { + Utf8Path::new("tests") + .join("data") + .join("cli-matrix") + .join(file_name) +} + +pub(super) fn manifest_directory() -> Result { + Dir::open_ambient_dir( + Utf8Path::new(env!("CARGO_MANIFEST_DIR")), + ambient_authority(), + ) + .context("open repository root capability") +} + +/// Returns whether a repository fixture is accessible through its directory capability. +pub(crate) fn fixture_exists(file_name: &str) -> Result { + Ok(manifest_directory()? + .metadata(fixture_path(file_name)) + .is_ok()) +} + +/// Reads a matrix fixture through the repository directory capability. +pub(super) fn read_fixture(file_name: &str) -> Result { + let path = fixture_path(file_name); + manifest_directory()? + .read_to_string(&path) + .with_context(|| format!("read matrix fixture '{path}'")) +} diff --git a/tests/cli_matrix/invariants.rs b/tests/cli_matrix/invariants.rs index 3b74f42c..45aad08a 100644 --- a/tests/cli_matrix/invariants.rs +++ b/tests/cli_matrix/invariants.rs @@ -1,10 +1,8 @@ //! Independent output invariants for CLI matrix snapshots. -use std::fs; +use anyhow::Result; -use anyhow::{Context as _, Result}; - -use super::{LogicalCase, TransformFlag, fixture_path}; +use super::{LogicalCase, TransformFlag, read_fixture}; const ELLIPSIS_UTF8: &[u8] = b"\xE2\x80\xA6"; @@ -15,9 +13,7 @@ struct OrderedMarker { /// Asserts output properties that prove enabled transforms changed matching input. pub(crate) fn assert_transform_invariants(logical: &LogicalCase, stdout: &[u8]) -> Result<()> { - let fixture_path = fixture_path(logical.fixture); - let fixture = fs::read_to_string(&fixture_path) - .with_context(|| format!("read matrix fixture '{}'", fixture_path.display()))?; + let fixture = read_fixture(logical.fixture)?; let output = String::from_utf8_lossy(stdout); if logical.flags.contains(&TransformFlag::Ellipsis) && fixture.contains("...") { diff --git a/tests/cli_matrix/support.rs b/tests/cli_matrix/support.rs index 419a9d6e..522711d5 100644 --- a/tests/cli_matrix/support.rs +++ b/tests/cli_matrix/support.rs @@ -1,17 +1,20 @@ //! Support types and runners for the CLI matrix integration test. -use std::{ - fs, - path::{Path, PathBuf}, - process::Output, -}; +use std::process::Output; use anyhow::{Context as _, Result}; use assert_cmd::Command; -use tempfile::tempdir; +use camino::{Utf8Path, Utf8PathBuf}; +#[path = "fixture_io.rs"] +mod fixture_io; #[path = "invariants.rs"] mod invariants; +#[path = "../common/fs.rs"] +mod test_fs; +pub(crate) use fixture_io::{fixture_exists, fixture_path}; +use fixture_io::{manifest_directory, read_fixture}; +use test_fs::TestDir; /// Represents a non-wrap CLI transform flag. #[derive(Clone, Copy, Debug, Eq, Ord, PartialEq, PartialOrd)] @@ -293,30 +296,30 @@ pub(crate) fn assert_transform_invariants(logical: &LogicalCase, stdout: &[u8]) /// Copies a matrix fixture into the temporary command directory. /// /// The staged input preserves the fixture extension for debugging clarity. -pub(crate) fn stage_fixture(case: &PhysicalCase, dir: &Path) -> Result { +pub(crate) fn stage_fixture(case: &PhysicalCase, dir: &TestDir) -> Result { let fixture = fixture_path(case.logical.fixture); - let file_path = dir - .join("input") - .with_extension(fixture.extension().unwrap_or_default()); - fs::copy(&fixture, &file_path).with_context(|| { - format!( - "copy fixture '{}' to '{}'", - case.logical.fixture, - file_path.display(), - ) - })?; + let file_path = Utf8Path::new("input").with_extension(fixture.extension().unwrap_or_default()); + let contents = manifest_directory()? + .read(&fixture) + .with_context(|| format!("read fixture '{fixture}'"))?; + dir.directory() + .write(&file_path, contents) + .with_context(|| format!("copy fixture '{}' to '{file_path}'", case.logical.fixture))?; Ok(file_path) } /// Builds a run result from process output and the temporary input file. pub(crate) fn collect_result( output: Output, - file_path: &Path, + dir: &TestDir, + file_path: &Utf8Path, mode: ExecutionMode, ) -> Result { let file_content = match mode { - ExecutionMode::Stdout | ExecutionMode::InPlace => fs::read(file_path) - .with_context(|| format!("read file '{}' after {:?} run", file_path.display(), mode))?, + ExecutionMode::Stdout | ExecutionMode::InPlace => dir + .directory() + .read(file_path) + .with_context(|| format!("read file '{file_path}' after {mode:?} run"))?, }; Ok(RunResult { output, @@ -326,27 +329,19 @@ pub(crate) fn collect_result( /// Runs a physical matrix case through the real `mdtablefix` binary. pub(crate) fn run_physical_case(case: &PhysicalCase) -> Result { - let dir = tempdir().context("create temporary directory for matrix case")?; - let file_path = stage_fixture(case, dir.path())?; + let dir = TestDir::new().context("create temporary directory for matrix case")?; + let file_path = stage_fixture(case, &dir)?; + let command_path = dir.path().join(&file_path); let mut command = Command::cargo_bin("mdtablefix").context("create mdtablefix test command")?; - command.args(case.args()).arg(&file_path); + command.args(case.args()).arg(command_path.as_std_path()); let output = command.output().with_context(|| { format!( "execute mdtablefix for matrix case '{}'", case.snapshot_name() ) })?; - collect_result(output, &file_path, case.mode) -} - -/// Returns the repository-relative path to a matrix fixture. -pub(crate) fn fixture_path(file_name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests") - .join("data") - .join("cli-matrix") - .join(file_name) + collect_result(output, &dir, &file_path, case.mode) } /// Returns whether a matrix case identifier uses the documented character set. @@ -371,30 +366,5 @@ pub(crate) fn non_wrap_signature(fixture: &str, flags: &[TransformFlag]) -> Stri pub(crate) fn has_flag(case: &BaseCase, flag: TransformFlag) -> bool { case.flags.contains(&flag) } #[cfg(test)] -#[rustfmt::skip] -mod tests { - //! Unit tests for CLI-matrix support helpers. - - use super::{BaseCase, TransformFlag, has_flag, is_case_id, non_wrap_signature}; - use rstest::rstest; - - #[rstest] - #[case("row_001", true)] #[case("row-001", true)] #[case("abc123", true)] - #[case("", false)] #[case("Row_001", false)] #[case("row 001", false)] - fn is_case_id_returns_expected_value(#[case] id: &str, #[case] expected: bool) { - assert_eq!(is_case_id(id), expected); - } - - #[test] fn non_wrap_signature_ignores_wrap_variant() { - let flags = [TransformFlag::Renumber, TransformFlag::Fences]; let (unwrapped, wrapped) = (false, true); - assert_ne!(unwrapped, wrapped); assert_eq!(non_wrap_signature("fixture.dat", &flags), non_wrap_signature("fixture.dat", &flags)); } - #[test] fn non_wrap_signature_distinguishes_flag_lists() { - assert_ne!(non_wrap_signature("fixture.dat", &[TransformFlag::Renumber]), non_wrap_signature("fixture.dat", &[TransformFlag::Fences])); } - - #[rstest] - #[case(TransformFlag::Renumber, true)] #[case(TransformFlag::Fences, false)] - fn has_flag_returns_expected_value(#[case] flag: TransformFlag, #[case] expected: bool) { - let case = BaseCase { id: "row_001", fixture: "fixture.dat", flags: &[TransformFlag::Renumber] }; - assert_eq!(has_flag(&case, flag), expected); - } -} +#[path = "support_tests.rs"] +mod tests; diff --git a/tests/cli_matrix/support_tests.rs b/tests/cli_matrix/support_tests.rs new file mode 100644 index 00000000..755127a8 --- /dev/null +++ b/tests/cli_matrix/support_tests.rs @@ -0,0 +1,47 @@ +//! Unit tests for CLI-matrix support helpers. + +use rstest::rstest; + +use super::{BaseCase, TransformFlag, has_flag, is_case_id, non_wrap_signature}; + +#[rstest] +#[case("row_001", true)] +#[case("row-001", true)] +#[case("abc123", true)] +#[case("", false)] +#[case("Row_001", false)] +#[case("row 001", false)] +fn is_case_id_returns_expected_value(#[case] id: &str, #[case] expected: bool) { + assert_eq!(is_case_id(id), expected); +} + +#[test] +fn non_wrap_signature_ignores_wrap_variant() { + let flags = [TransformFlag::Renumber, TransformFlag::Fences]; + let (unwrapped, wrapped) = (false, true); + assert_ne!(unwrapped, wrapped); + assert_eq!( + non_wrap_signature("fixture.dat", &flags), + non_wrap_signature("fixture.dat", &flags) + ); +} + +#[test] +fn non_wrap_signature_distinguishes_flag_lists() { + assert_ne!( + non_wrap_signature("fixture.dat", &[TransformFlag::Renumber]), + non_wrap_signature("fixture.dat", &[TransformFlag::Fences]) + ); +} + +#[rstest] +#[case(TransformFlag::Renumber, true)] +#[case(TransformFlag::Fences, false)] +fn has_flag_returns_expected_value(#[case] flag: TransformFlag, #[case] expected: bool) { + let case = BaseCase { + id: "row_001", + fixture: "fixture.dat", + flags: &[TransformFlag::Renumber], + }; + assert_eq!(has_flag(&case, flag), expected); +} diff --git a/tests/code_emphasis.rs b/tests/code_emphasis.rs index 511a11bd..fe6d4b56 100644 --- a/tests/code_emphasis.rs +++ b/tests/code_emphasis.rs @@ -2,17 +2,36 @@ //! //! Verifies that emphasis markers adjacent to inline code are normalized. -use std::fs; - use rstest::rstest; #[path = "support/cli_args.rs"] mod cli_args; #[path = "support/cli_stdin.rs"] mod cli_stdin; +#[path = "common/fs.rs"] +mod test_fs; use cli_args::run_cli_with_args; use cli_stdin::run_cli_with_stdin; -use tempfile::tempdir; +use test_fs::TestDir; + +fn assert_in_place_result(file_name: &str, input: &str, expected: &str) { + let dir = TestDir::new().expect("failed to create temporary directory"); + dir.directory() + .write(file_name, input) + .expect("failed to write test file"); + let file_path = dir.path().join(file_name); + + run_cli_with_args(&["--code-emphasis", "--in-place", file_path.as_str()]) + .expect("failed to run mdtablefix") + .success() + .stdout(""); + + let output = dir + .directory() + .read_to_string(file_name) + .expect("failed to read output file"); + assert_eq!(output, expected); +} #[test] fn cli_stdin_code_emphasis() -> Result<(), Box> { @@ -44,75 +63,25 @@ fn cli_preserves_emphasised_code( #[test] fn cli_in_place_code_emphasis() { - let dir = tempdir().expect("failed to create temporary directory"); - let file_path = dir.path().join("sample.md"); let input = "`StepContext`** Enhancement (in **`crates/rstest-bdd/src/context.rs`**)**\n"; let expected = "**`StepContext` Enhancement (in `crates/rstest-bdd/src/context.rs`)**\n"; - fs::write(&file_path, input).expect("failed to write test file"); - run_cli_with_args(&[ - "--code-emphasis", - "--in-place", - file_path.to_str().expect("path is not valid UTF-8"), - ]) - .unwrap() - .success() - .stdout(""); - let out = fs::read_to_string(&file_path).expect("failed to read output file"); - assert_eq!(out, expected); + assert_in_place_result("sample.md", input, expected); } #[test] -fn cli_in_place_code_emphasis_empty_file() { - let dir = tempdir().expect("failed to create temporary directory"); - let file_path = dir.path().join("empty.md"); - fs::write(&file_path, "").expect("failed to write test file"); - run_cli_with_args(&[ - "--code-emphasis", - "--in-place", - file_path.to_str().expect("path is not valid UTF-8"), - ]) - .unwrap() - .success() - .stdout(""); - let out = fs::read_to_string(&file_path).expect("failed to read output file"); - assert_eq!(out, ""); -} +fn cli_in_place_code_emphasis_empty_file() { assert_in_place_result("empty.md", "", ""); } #[test] fn cli_in_place_code_emphasis_whitespace_file() { - let dir = tempdir().expect("failed to create temporary directory"); - let file_path = dir.path().join("whitespace.md"); let input = " \n\t "; let expected = " \n\t \n"; - fs::write(&file_path, input).expect("failed to write test file"); - run_cli_with_args(&[ - "--code-emphasis", - "--in-place", - file_path.to_str().expect("path is not valid UTF-8"), - ]) - .unwrap() - .success() - .stdout(""); - let out = fs::read_to_string(&file_path).expect("failed to read output file"); - assert_eq!(out, expected); + assert_in_place_result("whitespace.md", input, expected); } #[test] fn cli_in_place_preserves_inner_backticks() { - let dir = tempdir().expect("failed to create temporary directory"); - let file_path = dir.path().join("inner.md"); let input = "```` ``a`b`` ````\n"; - fs::write(&file_path, input).expect("failed to write test file"); - run_cli_with_args(&[ - "--code-emphasis", - "--in-place", - file_path.to_str().expect("path is not valid UTF-8"), - ]) - .unwrap() - .success() - .stdout(""); - let out = fs::read_to_string(&file_path).expect("failed to read output file"); - assert_eq!(out, input); + assert_in_place_result("inner.md", input, input); } #[test] diff --git a/tests/common/fs.rs b/tests/common/fs.rs new file mode 100644 index 00000000..adf72e04 --- /dev/null +++ b/tests/common/fs.rs @@ -0,0 +1,35 @@ +//! Capability-scoped filesystem support for integration tests. + +use anyhow::{Context as _, Result, anyhow}; +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ambient_authority, fs_utf8::Dir}; +use tempfile::TempDir; + +/// Owns a temporary directory and its capability-scoped filesystem handle. +pub(crate) struct TestDir { + directory: Dir, + path: Utf8PathBuf, + _guard: TempDir, +} + +impl TestDir { + /// Creates a temporary directory with a UTF-8 absolute path. + pub(crate) fn new() -> Result { + let guard = TempDir::new().context("create temporary test directory")?; + let path = Utf8PathBuf::from_path_buf(guard.path().to_path_buf()) + .map_err(|path| anyhow!("temporary directory path is not UTF-8: {}", path.display()))?; + let directory = Dir::open_ambient_dir(&path, ambient_authority()) + .context("open temporary directory capability")?; + Ok(Self { + directory, + path, + _guard: guard, + }) + } + + /// Returns the capability used for relative filesystem operations. + pub(crate) fn directory(&self) -> &Dir { &self.directory } + + /// Returns the UTF-8 absolute path used at process boundaries. + pub(crate) fn path(&self) -> &Utf8Path { &self.path } +} diff --git a/tests/data/static_regex/clean.rs.txt b/tests/data/static_regex/clean.rs.txt new file mode 100644 index 00000000..783f83b6 --- /dev/null +++ b/tests/data/static_regex/clean.rs.txt @@ -0,0 +1,2 @@ +static RE: LazyLock = lazy_regex!("clean"); +fn build() { let _ = Regex::new("local").unwrap(); } diff --git a/tests/data/static_regex/lazylock_direct.rs.txt b/tests/data/static_regex/lazylock_direct.rs.txt new file mode 100644 index 00000000..3760b95e --- /dev/null +++ b/tests/data/static_regex/lazylock_direct.rs.txt @@ -0,0 +1 @@ +static RE: LazyLock = LazyLock::new(|| Regex::new("a").unwrap()); diff --git a/tests/data/static_regex/lazylock_move.rs.txt b/tests/data/static_regex/lazylock_move.rs.txt new file mode 100644 index 00000000..75a1b363 --- /dev/null +++ b/tests/data/static_regex/lazylock_move.rs.txt @@ -0,0 +1 @@ +static RE: LazyLock = LazyLock::new(move || Regex::new("e").unwrap()); diff --git a/tests/data/static_regex/lazylock_qualified.rs.txt b/tests/data/static_regex/lazylock_qualified.rs.txt new file mode 100644 index 00000000..758fb742 --- /dev/null +++ b/tests/data/static_regex/lazylock_qualified.rs.txt @@ -0,0 +1 @@ +static RE: LazyLock = std::sync::LazyLock::new(|| Regex::new("b").unwrap()); diff --git a/tests/data/static_regex/once_cell_lazy_direct.rs.txt b/tests/data/static_regex/once_cell_lazy_direct.rs.txt new file mode 100644 index 00000000..a601be38 --- /dev/null +++ b/tests/data/static_regex/once_cell_lazy_direct.rs.txt @@ -0,0 +1 @@ +static RE: Lazy = Lazy::new(|| Regex::new("c").unwrap()); diff --git a/tests/data/static_regex/once_cell_lazy_move.rs.txt b/tests/data/static_regex/once_cell_lazy_move.rs.txt new file mode 100644 index 00000000..59ab1dd7 --- /dev/null +++ b/tests/data/static_regex/once_cell_lazy_move.rs.txt @@ -0,0 +1 @@ +static RE: Lazy = once_cell::sync::Lazy::new(move || Regex::new("f").unwrap()); diff --git a/tests/data/static_regex/once_cell_lazy_qualified.rs.txt b/tests/data/static_regex/once_cell_lazy_qualified.rs.txt new file mode 100644 index 00000000..9b2879df --- /dev/null +++ b/tests/data/static_regex/once_cell_lazy_qualified.rs.txt @@ -0,0 +1 @@ +static RE: Lazy = once_cell::sync::Lazy::new(|| Regex::new("d").unwrap()); diff --git a/tests/parallel.rs b/tests/parallel.rs index 7f49029c..745542dd 100644 --- a/tests/parallel.rs +++ b/tests/parallel.rs @@ -1,14 +1,14 @@ //! Tests for parallel CLI processing of multiple files. -use std::{fs::File, io::Write}; - use assert_cmd::Command; use rstest::rstest; -use tempfile::tempdir; #[macro_use] #[path = "common/mod.rs"] mod common; +#[path = "common/fs.rs"] +mod test_fs; +use test_fs::TestDir; #[path = "support/cli_args.rs"] mod cli_args; @@ -25,48 +25,40 @@ fn test_cli_parallel_empty_file_list() -> Result<(), Box> #[rstest] fn test_cli_parallel_multiple_files() -> Result<(), Box> { - let dir = tempdir().expect("failed to create temporary directory"); + let dir = TestDir::new().expect("failed to create temporary directory"); let mut files = Vec::new(); let mut expected = String::new(); for i in 0..4 { - let path = dir.path().join(format!("file{i}.md")); + let file_name = format!("file{i}.md"); + let path = dir.path().join(&file_name); let table = vec![ format!("| A{i} | B{i} | |"), format!("| {i} | {i} | | {i} | {i} |"), ]; - let mut f = File::create(&path).expect("failed to create temporary file"); - for line in &table { - writeln!(f, "{line}").expect("failed to write line"); - } - f.flush().expect("failed to flush file"); - drop(f); + dir.directory() + .write(&file_name, format!("{}\n", table.join("\n"))) + .expect("failed to write temporary file"); expected.push_str(&mdtablefix::reflow_table(&table).join("\n")); expected.push('\n'); files.push(path); } - let args: Vec<&str> = files - .iter() - .map(|p| p.to_str().expect("path is not valid UTF-8")) - .collect(); + let args: Vec<&str> = files.iter().map(|path| path.as_str()).collect(); run_cli_with_args(&args)?.success().stdout(expected); Ok(()) } #[rstest] fn test_cli_parallel_missing_file_error() { - let dir = tempdir().expect("failed to create temporary directory"); + let dir = TestDir::new().expect("failed to create temporary directory"); let good = dir.path().join("good.md"); let table = vec![ "| Q | R | |".to_string(), "| 1 | 2 | | 3 | 4 |".to_string(), ]; - let mut f = File::create(&good).expect("failed to create file"); - for line in &table { - writeln!(f, "{line}").expect("failed to write line"); - } - f.flush().expect("failed to flush file"); - drop(f); + dir.directory() + .write("good.md", format!("{}\n", table.join("\n"))) + .expect("failed to write file"); let expected = mdtablefix::reflow_table(&table).join("\n") + "\n"; let missing = dir.path().join("missing.md"); @@ -84,18 +76,15 @@ fn test_cli_parallel_missing_file_error() { fn test_cli_parallel_missing_file_in_place( broken_table: Vec, ) -> Result<(), Box> { - let dir = tempdir().expect("failed to create temporary directory"); + let dir = TestDir::new().expect("failed to create temporary directory"); + dir.directory() + .write("good.md", format!("{}\n", broken_table.join("\n"))) + .expect("failed to write file"); let good = dir.path().join("good.md"); - let mut f = File::create(&good).expect("failed to create file"); - for line in &broken_table { - writeln!(f, "{line}").expect("failed to write line"); - } - f.flush().expect("failed to flush file"); - drop(f); let missing = dir.path().join("missing.md"); - let good_str = good.to_str().expect("path is not valid UTF-8"); - let missing_str = missing.to_str().expect("path is not valid UTF-8"); + let good_str = good.as_str(); + let missing_str = missing.as_str(); run_cli_with_args(&["--in-place", good_str, missing_str])? .failure() .stderr(predicates::str::contains("missing.md")); diff --git a/tests/static_regex_lint.rs b/tests/static_regex_lint.rs new file mode 100644 index 00000000..aac68de0 --- /dev/null +++ b/tests/static_regex_lint.rs @@ -0,0 +1,215 @@ +//! Regression coverage for the `check-static-regexes` lint guard. +//! +//! The guard lives in `scripts/check-static-regexes.sh` (invoked by the +//! Makefile's `check-static-regexes` target). It rejects hand-rolled static +//! regular expressions that bypass the `lazy_regex!` convention by wrapping +//! `Regex::new` directly in a supported lazy-wrapper constructor. +//! +//! These tests exercise the script directly so the Makefile and the tests +//! share a single source of truth for the scan. Every supported wrapper form +//! is asserted to be rejected, a clean fixture is asserted to pass, and a +//! ripgrep scan failure is asserted to propagate. +//! +//! Fixtures live under `tests/data/static_regex/` with a `.rs.txt` extension +//! so the guard (which scans `*.rs`) does not match the fixtures in place; the +//! tests copy each fixture into a temporary directory as a `.rs` file before +//! scanning. + +use std::process::Command; + +use camino::{Utf8Path, Utf8PathBuf}; +use cap_std::{ + ambient_authority, + fs_utf8::{Dir, PermissionsExt as _}, +}; +use rstest::rstest; + +#[path = "common/fs.rs"] +mod test_fs; +use test_fs::TestDir; + +/// The diagnostic emitted when a prohibited declaration is found. +const PROHIBITED_DIAGNOSTIC: &str = "static regular expressions must use lazy_regex!"; + +/// Every lazy-wrapper constructor form the guard must reject when it directly +/// wraps `Regex::new`. Each label maps to a fixture under +/// `tests/data/static_regex/` exercising one supported spelling: +/// +/// * `lazylock_direct` — `LazyLock::new(|| Regex::new(...))` +/// * `lazylock_qualified` — `std::sync::LazyLock::new(|| Regex::new(...))` +/// * `lazylock_move` — `LazyLock::new(move || Regex::new(...))` +/// * `once_cell_lazy_direct` — `Lazy::new(|| Regex::new(...))` +/// * `once_cell_lazy_qualified` — `once_cell::sync::Lazy::new(|| Regex::new(...))` +/// * `once_cell_lazy_move` — `once_cell::sync::Lazy::new(move || Regex::new(...))` +const PROHIBITED_FORMS: &[&str] = &[ + "lazylock_direct", + "lazylock_qualified", + "lazylock_move", + "once_cell_lazy_direct", + "once_cell_lazy_qualified", + "once_cell_lazy_move", +]; + +fn manifest_path() -> &'static Utf8Path { Utf8Path::new(env!("CARGO_MANIFEST_DIR")) } + +fn manifest_directory() -> Dir { + Dir::open_ambient_dir(manifest_path(), ambient_authority()) + .expect("failed to open repository root") +} + +fn script_path() -> Utf8PathBuf { manifest_path().join("scripts/check-static-regexes.sh") } + +fn fixture(label: &str) -> String { + let path = Utf8PathBuf::from(format!("tests/data/static_regex/{label}.rs.txt")); + manifest_directory() + .read_to_string(&path) + .unwrap_or_else(|error| panic!("failed to read fixture {path}: {error}")) +} + +/// Materialize `label`'s fixture as a `.rs` file inside a fresh temp directory. +fn scan_dir_with(label: &str) -> TestDir { + let dir = TestDir::new().expect("failed to create temp dir"); + dir.directory() + .write(format!("{label}.rs"), fixture(label)) + .expect("failed to write fixture into temp dir"); + dir +} + +/// Run the guard against `scan_dir`, optionally overriding the `RG` ripgrep +/// command. +/// +/// `rg` is the raw `RG` value, so it may carry arguments (for example +/// `rg --pcre2`); the guard splits it on whitespace. Passing `None` clears any +/// ambient `RG` so default-path runs exercise the guard's own `rg` default +/// deterministically. +fn run_guard(scan_dir: &Utf8Path, rg: Option<&str>) -> std::process::Output { + let mut cmd = Command::new(script_path().as_std_path()); + cmd.arg(scan_dir); + match rg { + Some(rg) => cmd.env("RG", rg), + None => cmd.env_remove("RG"), + }; + cmd.output() + .expect("failed to execute check-static-regexes.sh") +} + +/// Write `script` to `/` and mark it executable. +fn write_stub(dir: &TestDir, name: &str, script: &str) -> Utf8PathBuf { + dir.directory() + .write(name, script) + .expect("failed to write stub"); + #[cfg(unix)] + { + let permissions = cap_std::fs_utf8::Permissions::from_mode(0o755); + dir.directory() + .set_permissions(name, permissions) + .expect("failed to chmod stub"); + } + dir.path().join(name) +} + +#[rstest] +fn rejects_prohibited_lazy_wrapper_form(#[values(0, 1, 2, 3, 4, 5)] index: usize) { + let label = PROHIBITED_FORMS[index]; + let dir = scan_dir_with(label); + + let output = run_guard(dir.path(), None); + + assert_eq!( + output.status.code(), + Some(1), + "form `{label}` should be rejected with status 1" + ); + let stdout = String::from_utf8_lossy(&output.stdout); + assert!( + stdout.contains(PROHIBITED_DIAGNOSTIC), + "form `{label}` should emit the prohibited diagnostic, got: {stdout}" + ); +} + +#[test] +fn accepts_clean_sources() { + // The sanctioned `lazy_regex!` idiom plus an unrelated non-static + // `Regex::new` call that must not trip the guard. + let dir = scan_dir_with("clean"); + + let output = run_guard(dir.path(), None); + + assert_eq!( + output.status.code(), + Some(0), + "clean sources should pass; stdout: {}", + String::from_utf8_lossy(&output.stdout) + ); +} + +#[test] +fn propagates_ripgrep_scan_failure() { + let dir = TestDir::new().expect("failed to create temp dir"); + // A stub standing in for ripgrep that fails with a distinctive status. + let stub = write_stub(&dir, "rg-stub.sh", "#!/bin/sh\nexit 3\n"); + + let output = run_guard(dir.path(), Some(stub.as_str())); + + assert_eq!( + output.status.code(), + Some(3), + "a ripgrep scan failure should propagate its exit status" + ); + let stderr = String::from_utf8_lossy(&output.stderr); + assert!( + stderr.contains("failed to scan Rust sources (rg exit 3)"), + "scan failure should emit the scan-failure diagnostic, got: {stderr}" + ); +} + +/// An `RG` override may carry arguments — the Makefile's `$(RG)` expansion +/// supported this before the scan moved into the script, and `check-ripgrep` +/// still validates only `$(firstword $(RG))`. The guard must therefore split +/// `RG` on whitespace and forward the extra arguments to ripgrep ahead of its +/// own, rather than treating the whole value as one executable name. +#[test] +fn preserves_arguments_supplied_through_rg() { + let dir = TestDir::new().expect("failed to create temp dir"); + let argv_log = dir.path().join("argv.txt"); + // A stub that records its argv, then reports "no matches" so the guard + // takes its clean-scan path. + let stub = write_stub( + &dir, + "rg-stub.sh", + &format!( + "#!/bin/sh\nfor a in \"$@\"; do printf '%s\\n' \"$a\"; done > '{argv_log}'\nexit 1\n" + ), + ); + + let output = run_guard(dir.path(), Some(&format!("{stub} --pcre2"))); + + assert_eq!( + output.status.code(), + Some(0), + "an argument-bearing RG override must still run; stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let argv: Vec = dir + .directory() + .read_to_string("argv.txt") + .expect("stub should have recorded its argv") + .lines() + .map(str::to_owned) + .collect(); + assert_eq!( + argv.first().map(String::as_str), + Some("--pcre2"), + "the RG override's own arguments must be forwarded first, got: {argv:?}" + ); + assert!( + argv.contains(&"-U".to_owned()) && argv.contains(&"--glob".to_owned()), + "the guard's own ripgrep arguments must follow, got: {argv:?}" + ); + assert_eq!( + argv.last().map(String::as_str), + Some(dir.path().as_str()), + "the scan directory must remain the final argument, got: {argv:?}" + ); +} diff --git a/tests/wrap/cli_files.rs b/tests/wrap/cli_files.rs index e3735f81..8aa0a1dd 100644 --- a/tests/wrap/cli_files.rs +++ b/tests/wrap/cli_files.rs @@ -1,12 +1,13 @@ //! File-backed CLI regression tests for the parameterless wrapping flag. -use std::fs; - use assert_cmd::Command; use mdtablefix::process::WRAP_COLS; -use tempfile::NamedTempFile; use unicode_width::UnicodeWidthStr; +#[path = "../common/fs.rs"] +mod test_fs; +use test_fs::TestDir; + /// Ensures a path after `--wrap` remains a positional input file. #[test] fn cli_wrap_processes_positional_file() -> Result<(), Box> { @@ -17,13 +18,14 @@ fn cli_wrap_processes_positional_file() -> Result<(), Box "漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 ", "漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂 漢字🙂.\n", ); - let file = NamedTempFile::new()?; - fs::write(file.path(), input)?; + let dir = TestDir::new()?; + dir.directory().write("input.md", input)?; + let file_path = dir.path().join("input.md"); let mut command = Command::cargo_bin("mdtablefix")?; let output = command .arg("--wrap") - .arg(file.path()) + .arg(file_path.as_std_path()) .assert() .success() .get_output() diff --git a/tests/wrap_cli.rs b/tests/wrap_cli.rs index f086d370..d1b8c266 100644 --- a/tests/wrap_cli.rs +++ b/tests/wrap_cli.rs @@ -1,25 +1,27 @@ //! CLI regression tests for wrap behaviour around verbatim code blocks. -use std::fs; - use assert_cmd::Command; use rstest::rstest; -use tempfile::NamedTempFile; + +#[path = "common/fs.rs"] +mod test_fs; +use test_fs::TestDir; fn run_wrap_in_place_and_read_back(input: &str) -> Result> { - let temp = NamedTempFile::new()?; - fs::write(temp.path(), input)?; + let dir = TestDir::new()?; + dir.directory().write("input.md", input)?; + let file_path = dir.path().join("input.md"); let mut command = Command::cargo_bin("mdtablefix")?; command .args(["--wrap", "--in-place"]) - .arg(temp.path()) + .arg(file_path.as_std_path()) .assert() .success() .stdout("") .stderr(""); - Ok(fs::read_to_string(temp.path())?) + Ok(dir.directory().read_to_string("input.md")?) } /// Guards issue #261 by asserting `--wrap --in-place` leaves shell code blocks