diff --git a/.config/nextest.toml b/.config/nextest.toml index 686ee154..b8b78a0c 100644 --- a/.config/nextest.toml +++ b/.config/nextest.toml @@ -35,6 +35,13 @@ filter = "test(driver::ui::) | test(tests::ui::) | test(ui::ui) | (binary(ui) & test-group = "serial-dylint-ui" retries = { backoff = "exponential", count = 2, delay = "5s" } +[[profile.default.overrides]] +# Serialise ignored exclusion integration tests when they are explicitly run. +# They build and stage the lint library before invoking `cargo dylint`, so they +# share the same target-directory race risk as the UI harnesses above. +filter = "binary(integration_exclusion)" +test-group = "serial-dylint-ui" + # Extend the timeout for toolchain auto-install behavioural tests, since the # isolated rustup setup may need to download and install a full nightly toolchain. [[profile.default.overrides]] diff --git a/Cargo.lock b/Cargo.lock index 4058e87a..f5c57423 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -407,6 +407,17 @@ dependencies = [ "whitaker-common", ] +[[package]] +name = "console" +version = "0.16.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d64e8af5551369d19cf50138de61f1c42074ab970f74e99be916646777f8fc87" +dependencies = [ + "encode_unicode", + "libc", + "windows-sys 0.61.2", +] + [[package]] name = "constant_time_eq" version = "0.3.1" @@ -683,6 +694,12 @@ dependencies = [ "tempfile", ] +[[package]] +name = "encode_unicode" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34aa73646ffb006b8f5147f3dc182bd4bcb190227ce861fc4a4844bf8e3cb2c0" + [[package]] name = "env_filter" version = "1.0.1" @@ -1355,6 +1372,19 @@ dependencies = [ "generic-array", ] +[[package]] +name = "insta" +version = "1.47.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b4a6248eb93a4401ed2f37dfe8ea592d3cf05b7cf4f8efa867b6895af7e094e" +dependencies = [ + "console", + "once_cell", + "serde", + "similar", + "tempfile", +] + [[package]] name = "intl-memoizer" version = "0.5.3" @@ -1732,9 +1762,11 @@ dependencies = [ name = "no_std_fs_operations" version = "0.2.5" dependencies = [ + "anyhow", "cargo_metadata", "dylint_linting", "dylint_testing", + "insta", "log", "mockall", "rstest", @@ -1746,7 +1778,9 @@ dependencies = [ "rustc_session", "rustc_span", "serde", + "serde_json", "serial_test", + "tempfile", "toml 0.9.12+spec-1.1.0", "whitaker", "whitaker-common", @@ -2537,6 +2571,12 @@ version = "0.3.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "703d5c7ef118737c72f1af64ad2f6f8c5e1921f818cdcb97b8fe6fc69bf66214" +[[package]] +name = "similar" +version = "2.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bbbb5d9659141646ae647b42fe094daf6c6192d1620870b449d9557f748b2daa" + [[package]] name = "slab" version = "0.4.12" diff --git a/Cargo.toml b/Cargo.toml index d4788b03..d7f22553 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ dylint_linting = "5" dylint_testing = "5" libc = "0.2" fluent-templates = "^0.13.1" +insta = { version = "1", features = ["json"] } once_cell = "^1.21.3" unic-langid = "^0.9.4" serde = { version = "1.0.228", features = ["derive"] } diff --git a/crates/no_std_fs_operations/Cargo.toml b/crates/no_std_fs_operations/Cargo.toml index dee202ac..b96fd312 100644 --- a/crates/no_std_fs_operations/Cargo.toml +++ b/crates/no_std_fs_operations/Cargo.toml @@ -35,11 +35,15 @@ serde = { workspace = true, optional = true } whitaker = { workspace = true, features = ["dylint-driver"], optional = true } [dev-dependencies] +anyhow = "1.0" cargo_metadata = { workspace = true } +insta = { workspace = true } mockall = { workspace = true } rstest = { workspace = true } rstest-bdd = { workspace = true } rstest-bdd-macros = { workspace = true } dylint_testing = { workspace = true } serial_test = "3.1.0" +serde_json = { workspace = true } +tempfile = { workspace = true } toml = { workspace = true } diff --git a/crates/no_std_fs_operations/tests/fixtures/excluded_project/Cargo.toml b/crates/no_std_fs_operations/tests/fixtures/excluded_project/Cargo.toml deleted file mode 100644 index 5a525eb4..00000000 --- a/crates/no_std_fs_operations/tests/fixtures/excluded_project/Cargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -name = "excluded_test_crate" -version = "0.1.0" -edition = "2024" - -[dependencies] - -[workspace] diff --git a/crates/no_std_fs_operations/tests/fixtures/excluded_project/dylint.toml b/crates/no_std_fs_operations/tests/fixtures/excluded_project/dylint.toml deleted file mode 100644 index 183f2a1e..00000000 --- a/crates/no_std_fs_operations/tests/fixtures/excluded_project/dylint.toml +++ /dev/null @@ -1,2 +0,0 @@ -[no_std_fs_operations] -excluded_crates = ["excluded_test_crate"] diff --git a/crates/no_std_fs_operations/tests/fixtures/excluded_project/src/lib.rs b/crates/no_std_fs_operations/tests/fixtures/excluded_project/src/lib.rs deleted file mode 100644 index 3c06cc20..00000000 --- a/crates/no_std_fs_operations/tests/fixtures/excluded_project/src/lib.rs +++ /dev/null @@ -1,26 +0,0 @@ -//! Test fixture for excluded crate behaviour. -//! -//! This crate is intentionally a standalone fixture; the duplication with -//! `non_excluded_project` is necessary since each must be an independent crate -//! for exclusion testing. - -use std::fs::File; -use std::path::Path; - -/// Opens a file for reading. -/// -/// # Examples -/// -/// ```no_run -/// use excluded_project::open_file; -/// -/// // Open existing file - returns Ok with file handle -/// let file = open_file("Cargo.toml").expect("file should exist"); -/// -/// // Attempt to open non-existent file - returns Err -/// let result = open_file("nonexistent.txt"); -/// assert!(result.is_err()); -/// ``` -pub fn open_file>(path: P) -> std::io::Result { - File::open(path) -} diff --git a/crates/no_std_fs_operations/tests/fixtures/non_excluded_project/Cargo.toml b/crates/no_std_fs_operations/tests/fixtures/non_excluded_project/Cargo.toml deleted file mode 100644 index a26c00c8..00000000 --- a/crates/no_std_fs_operations/tests/fixtures/non_excluded_project/Cargo.toml +++ /dev/null @@ -1,8 +0,0 @@ -[package] -name = "non_excluded_crate" -version = "0.1.0" -edition = "2024" - -[dependencies] - -[workspace] diff --git a/crates/no_std_fs_operations/tests/fixtures/non_excluded_project/dylint.toml b/crates/no_std_fs_operations/tests/fixtures/non_excluded_project/dylint.toml deleted file mode 100644 index 12b538cb..00000000 --- a/crates/no_std_fs_operations/tests/fixtures/non_excluded_project/dylint.toml +++ /dev/null @@ -1,2 +0,0 @@ -[no_std_fs_operations] -excluded_crates = [] diff --git a/crates/no_std_fs_operations/tests/fixtures/non_excluded_project/src/lib.rs b/crates/no_std_fs_operations/tests/fixtures/non_excluded_project/src/lib.rs deleted file mode 100644 index 8732bad5..00000000 --- a/crates/no_std_fs_operations/tests/fixtures/non_excluded_project/src/lib.rs +++ /dev/null @@ -1,26 +0,0 @@ -//! Test fixture for non-excluded crate behaviour. -//! -//! This crate is intentionally a standalone fixture; the duplication with -//! `excluded_project` is necessary since each must be an independent crate -//! for exclusion testing. - -use std::fs::File; -use std::path::Path; - -/// Opens a file for reading. -/// -/// # Examples -/// -/// ```no_run -/// use non_excluded_project::open_file; -/// -/// // Open existing file - returns Ok with file handle -/// let file = open_file("Cargo.toml").expect("file should exist"); -/// -/// // Attempt to open non-existent file - returns Err -/// let result = open_file("nonexistent.txt"); -/// assert!(result.is_err()); -/// ``` -pub fn open_file>(path: P) -> std::io::Result { - File::open(path) -} diff --git a/crates/no_std_fs_operations/tests/integration_exclusion.rs b/crates/no_std_fs_operations/tests/integration_exclusion.rs index 6d300212..116a9dba 100644 --- a/crates/no_std_fs_operations/tests/integration_exclusion.rs +++ b/crates/no_std_fs_operations/tests/integration_exclusion.rs @@ -18,35 +18,41 @@ use std::path::{Path, PathBuf}; use std::process::Command; use std::sync::OnceLock; +use anyhow::Context as _; use cargo_metadata::{Message, Metadata, MetadataCommand}; -use rstest::{fixture, rstest}; +use insta::assert_json_snapshot; +use rstest::rstest; use serial_test::serial; +mod test_support; + +use test_support::create_fixture_project; + const LINT_CRATE_NAME: &str = "no_std_fs_operations"; /// Builds the lint library and returns the path to the release directory. -fn build_lint_library() -> PathBuf { +fn build_lint_library() -> anyhow::Result { let metadata = MetadataCommand::new() .no_deps() .exec() - .expect("failed to fetch cargo metadata"); + .context("failed to fetch cargo metadata")?; - let output = run_lint_crate_build(metadata.workspace_root.as_std_path()); - let package_id = find_package_id(&metadata, LINT_CRATE_NAME); - let cdylib_path = find_cdylib_in_artifacts(&output, &package_id); + let output = run_lint_crate_build(metadata.workspace_root.as_std_path())?; + let package_id = find_package_id(&metadata, LINT_CRATE_NAME)?; + let cdylib_path = find_cdylib_in_artifacts(&output, &package_id)?; let release_dir = cdylib_path .parent() - .expect("cdylib should have a parent directory") + .context("cdylib should have a parent directory")? .to_path_buf(); - stage_toolchain_qualified_library(&cdylib_path, &release_dir); + stage_toolchain_qualified_library(&cdylib_path, &release_dir)?; - release_dir + Ok(release_dir) } /// Executes `cargo build` for the lint crate and returns the build output. -fn run_lint_crate_build(workspace_root: &Path) -> Vec { +fn run_lint_crate_build(workspace_root: &Path) -> anyhow::Result> { let output = Command::new("cargo") .arg("build") .arg("--lib") @@ -58,19 +64,20 @@ fn run_lint_crate_build(workspace_root: &Path) -> Vec { .arg("dylint-driver") .current_dir(workspace_root) .output() - .expect("failed to execute cargo build"); + .context("failed to execute cargo build")?; - assert!( - output.status.success(), - "lint library build failed: {}", - String::from_utf8_lossy(&output.stderr) - ); + if !output.status.success() { + anyhow::bail!( + "lint library build failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + } - output.stdout + Ok(output.stdout) } /// Copies the built library to a toolchain-qualified filename for Dylint discovery. -fn stage_toolchain_qualified_library(cdylib_path: &Path, release_dir: &Path) { +fn stage_toolchain_qualified_library(cdylib_path: &Path, release_dir: &Path) -> anyhow::Result<()> { let toolchain = env::var("RUSTUP_TOOLCHAIN") .ok() .or_else(|| option_env!("RUSTUP_TOOLCHAIN").map(String::from)) @@ -78,7 +85,7 @@ fn stage_toolchain_qualified_library(cdylib_path: &Path, release_dir: &Path) { let file_name = cdylib_path .file_name() - .expect("cdylib should have a filename") + .context("cdylib should have a filename")? .to_string_lossy(); let suffix = env::consts::DLL_SUFFIX; @@ -88,11 +95,16 @@ fn stage_toolchain_qualified_library(cdylib_path: &Path, release_dir: &Path) { ); let target_path = release_dir.join(&target_name); - std::fs::copy(cdylib_path, &target_path).expect("failed to copy lint library"); + std::fs::copy(cdylib_path, &target_path) + .with_context(|| format!("failed to copy lint library to {}", target_path.display()))?; + Ok(()) } /// Locates the package ID for a workspace member by crate name. -fn find_package_id(metadata: &Metadata, crate_name: &str) -> cargo_metadata::PackageId { +fn find_package_id( + metadata: &Metadata, + crate_name: &str, +) -> anyhow::Result { metadata .packages .iter() @@ -104,13 +116,17 @@ fn find_package_id(metadata: &Metadata, crate_name: &str) -> cargo_metadata::Pac .any(|member| member == &package.id) }) .map(|package| package.id.clone()) - .expect("lint crate not found in workspace") + .with_context(|| format!("lint crate `{crate_name}` not found in workspace")) } /// Extracts the cdylib path from cargo build JSON output for a given package. -fn find_cdylib_in_artifacts(stdout: &[u8], package_id: &cargo_metadata::PackageId) -> PathBuf { +fn find_cdylib_in_artifacts( + stdout: &[u8], + package_id: &cargo_metadata::PackageId, +) -> anyhow::Result { for message in Message::parse_stream(Cursor::new(stdout)) { - let Ok(Message::CompilerArtifact(artifact)) = message else { + let message = message.context("failed to parse cargo build JSON output")?; + let Message::CompilerArtifact(artifact) = message else { continue; }; @@ -127,11 +143,11 @@ fn find_cdylib_in_artifacts(stdout: &[u8], package_id: &cargo_metadata::PackageI .iter() .find(|candidate| candidate.as_str().ends_with(env::consts::DLL_SUFFIX)) { - return path.clone().into_std_path_buf(); + return Ok(path.clone().into_std_path_buf()); } } - panic!("cdylib artifact not found in build output"); + anyhow::bail!("cdylib artifact not found in build output for package `{package_id}`") } /// Result of invoking `cargo dylint` against a fixture crate. @@ -142,7 +158,7 @@ struct CargoDylintResult { } /// Runs `cargo dylint` on the given fixture project directory. -fn run_cargo_dylint(fixture_dir: &Path, library_path: &Path) -> CargoDylintResult { +fn run_cargo_dylint(fixture_dir: &Path, library_path: &Path) -> anyhow::Result { let output = Command::new("cargo") .arg("dylint") .arg("--all") @@ -153,21 +169,34 @@ fn run_cargo_dylint(fixture_dir: &Path, library_path: &Path) -> CargoDylintResul .env("DYLINT_LIBRARY_PATH", library_path) .env("DYLINT_RUSTFLAGS", "-D warnings") .output() - .expect("failed to execute cargo dylint"); + .context("failed to execute cargo dylint")?; let stderr = String::from_utf8_lossy(&output.stderr).into_owned(); - CargoDylintResult { + Ok(CargoDylintResult { is_success: output.status.success(), stdout: output.stdout, stderr, - } + }) } /// Counts diagnostics emitted by the `no_std_fs_operations` lint from cargo JSON output. -fn diagnostic_count(output: &[u8]) -> usize { - Message::parse_stream(Cursor::new(output)) - .map(|message| message.expect("cargo dylint should emit valid JSON messages")) +/// +/// Propagates parse errors rather than panicking, including context from the raw stdout +/// so callers can inspect malformed cargo output. +fn diagnostic_count(output: &[u8]) -> Result { + let messages: Vec = Message::parse_stream(Cursor::new(output)) + .collect::, _>>() + .map_err(|e| { + anyhow::anyhow!( + "failed to parse cargo JSON messages: {}\nstdout:\n{}", + e, + String::from_utf8_lossy(output) + ) + })?; + + Ok(messages + .into_iter() .filter_map(|message| match message { Message::CompilerMessage(message) => Some(message.message), _ => None, @@ -178,22 +207,43 @@ fn diagnostic_count(output: &[u8]) -> usize { .as_ref() .is_some_and(|code| code.code == LINT_CRATE_NAME) }) - .count() + .count()) } -/// Returns the path to a named fixture project under `tests/fixtures/`. -fn fixture_path(name: &str) -> PathBuf { - Path::new(env!("CARGO_MANIFEST_DIR")) - .join("tests") - .join("fixtures") - .join(name) +/// Replaces all occurrences of `prefix` in `value` with the fixed +/// placeholder `[FIXTURE_ROOT]` so snapshot output is stable across runs. +fn redact_path_prefix(value: serde_json::Value, prefix: &str) -> serde_json::Value { + match value { + serde_json::Value::String(s) => { + serde_json::Value::String(s.replace(prefix, "[FIXTURE_ROOT]")) + } + serde_json::Value::Array(arr) => serde_json::Value::Array( + arr.into_iter() + .map(|value| redact_path_prefix(value, prefix)) + .collect(), + ), + serde_json::Value::Object(map) => serde_json::Value::Object( + map.into_iter() + .map(|(key, value)| (key, redact_path_prefix(value, prefix))) + .collect(), + ), + other => other, + } } -#[fixture] -fn lint_library_path() -> PathBuf { - static LINT_LIBRARY_PATH: OnceLock = OnceLock::new(); - - LINT_LIBRARY_PATH.get_or_init(build_lint_library).clone() +#[expect( + clippy::useless_asref, + clippy::redundant_closure, + reason = "anyhow::Error is not Clone, so .as_ref().map(Clone::clone) is necessary to convert &Result into Result" +)] +fn lint_library_path() -> anyhow::Result { + static LINT_LIBRARY_PATH: OnceLock> = OnceLock::new(); + + LINT_LIBRARY_PATH + .get_or_init(|| build_lint_library()) + .as_ref() + .map(Clone::clone) + .map_err(|e| anyhow::anyhow!("{e:#}")) } struct Expectation { @@ -201,16 +251,30 @@ struct Expectation { should_succeed: bool, } +/// Shared driver for exclusion integration tests. +fn run_exclusion_test( + crate_name: &str, + is_excluded: bool, + expectation: Expectation, +) -> anyhow::Result<()> { + let lint_library_path = lint_library_path().context("failed to build lint library")?; + let fixture = create_fixture_project(crate_name, is_excluded) + .context("failed to create fixture project")?; + assert_fixture_behaviour(fixture.root(), &lint_library_path, crate_name, expectation) +} + #[rstest] #[case( - "excluded_project", + "excluded_test_crate", + true, Expectation { should_emit_diagnostics: false, should_succeed: true, } )] #[case( - "non_excluded_project", + "non_excluded_crate", + false, Expectation { should_emit_diagnostics: true, should_succeed: false, @@ -218,35 +282,113 @@ struct Expectation { )] #[ignore = "requires cargo-dylint and built lint library"] #[serial] -fn exclusion_behaviour_matches_fixture_configuration( - lint_library_path: PathBuf, - #[case] fixture: &str, - #[case] expectation: Expectation, -) { - let should_emit_diagnostics = expectation.should_emit_diagnostics; - let should_succeed = expectation.should_succeed; - let fixture_dir = fixture_path(fixture); - - let result = run_cargo_dylint(&fixture_dir, &lint_library_path); - let count = diagnostic_count(&result.stdout); +fn exclusion_crates_behaviour_test( + #[case] crate_name: &str, + #[case] is_excluded: bool, + #[case] expected: Expectation, +) -> anyhow::Result<()> { + run_exclusion_test(crate_name, is_excluded, expected) +} + +/// Runs `cargo dylint` against the fixture and counts diagnostics. +fn evaluate_fixture( + fixture_dir: &Path, + lint_library_path: &Path, + crate_name: &str, +) -> anyhow::Result<(bool, usize)> { + let result = run_cargo_dylint(fixture_dir, lint_library_path)?; + let count = diagnostic_count(&result.stdout).with_context(|| { + format!( + "crate `{crate_name}` produced malformed cargo output\nstderr:\n{}", + result.stderr + ) + })?; + Ok((result.is_success, count)) +} + +fn assert_fixture_behaviour( + fixture_dir: &Path, + lint_library_path: &Path, + crate_name: &str, + expectation: Expectation, +) -> anyhow::Result<()> { + let (is_success, count) = evaluate_fixture(fixture_dir, lint_library_path, crate_name)?; assert!( - result.is_success == should_succeed, - "fixture `{fixture}` should return success={should_succeed}, but stderr was:\n{}", - result.stderr + is_success == expectation.should_succeed, + "crate `{crate_name}` should return success={}", + expectation.should_succeed ); - if should_emit_diagnostics { + if expectation.should_emit_diagnostics { assert!( count > 0, - "fixture `{fixture}` should emit `no_std_fs_operations` diagnostics, but stderr was:\n{}", - result.stderr + "crate `{crate_name}` should emit `no_std_fs_operations` diagnostics" ); } else { assert!( count == 0, - "fixture `{fixture}` should emit zero `no_std_fs_operations` diagnostics, but stderr was:\n{}", - result.stderr + "crate `{crate_name}` should emit zero `no_std_fs_operations` diagnostics" ); } + + Ok(()) +} + +/// Snapshot test: verifies the structured JSON diagnostic output emitted by +/// `cargo dylint` for a non-excluded crate. +/// +/// Non-deterministic fields (absolute fixture paths) are redacted to +/// `[FIXTURE_ROOT]` before the snapshot is taken. +#[test] +#[ignore = "requires cargo-dylint and built lint library"] +#[serial] +fn non_excluded_crate_diagnostics_match_snapshot() -> anyhow::Result<()> { + let lint_library_path = lint_library_path().context("failed to build lint library")?; + let fixture = create_fixture_project("non_excluded_crate_snap", false) + .context("failed to create fixture project")?; + + let result = run_cargo_dylint(fixture.root(), &lint_library_path) + .context("failed to run cargo dylint")?; + + let messages = Message::parse_stream(Cursor::new(&result.stdout)) + .collect::, _>>() + .with_context(|| { + format!( + "non_excluded_crate_diagnostics_match_snapshot produced malformed cargo output\nstderr:\n{}", + result.stderr + ) + })?; + + let diagnostics: Vec = messages + .into_iter() + .filter_map(|message| match message { + Message::CompilerMessage(message) + if message + .message + .code + .as_ref() + .is_some_and(|code| code.code == LINT_CRATE_NAME) => + { + Some( + serde_json::to_value(message.message) + .context("failed to serialise diagnostic for snapshot"), + ) + } + _ => None, + }) + .collect::>>()?; + + let prefix = fixture + .root() + .to_str() + .context("fixture root should be valid UTF-8")?; + + let redacted: Vec = diagnostics + .into_iter() + .map(|value| redact_path_prefix(value, prefix)) + .collect(); + + assert_json_snapshot!("non_excluded_crate_diagnostics", redacted); + Ok(()) } diff --git a/crates/no_std_fs_operations/tests/snapshots/integration_exclusion__non_excluded_crate_diagnostics.snap b/crates/no_std_fs_operations/tests/snapshots/integration_exclusion__non_excluded_crate_diagnostics.snap new file mode 100644 index 00000000..d0c371b4 --- /dev/null +++ b/crates/no_std_fs_operations/tests/snapshots/integration_exclusion__non_excluded_crate_diagnostics.snap @@ -0,0 +1,214 @@ +--- +source: crates/no_std_fs_operations/tests/integration_exclusion.rs +expression: redacted +--- +[ + { + "children": [ + { + "children": [], + "code": null, + "level": "note", + "message": "std::fs touches the ambient working directory; accept `cap_std::fs::Dir` handles and camino paths instead so callers choose the capability surface.", + "rendered": null, + "spans": [] + }, + { + "children": [], + "code": null, + "level": "help", + "message": "Pass `cap_std::fs::Dir` plus `camino::Utf8Path`/`Utf8PathBuf` parameters through your APIs instead of calling std::fs directly.", + "rendered": null, + "spans": [] + }, + { + "children": [], + "code": null, + "level": "note", + "message": "`#[deny(no_std_fs_operations)]` on by default", + "rendered": null, + "spans": [] + } + ], + "code": { + "code": "no_std_fs_operations", + "explanation": null + }, + "level": "error", + "message": "std::fs operation `std::fs::File` bypasses the capability-based filesystem policy.", + "rendered": "error: std::fs operation `std::fs::File` bypasses the capability-based filesystem policy.\n --> src/lib.rs:3:5\n |\n3 | use std::fs::File;\n | ^^^^^^^^^^^^^\n |\n = note: std::fs touches the ambient working directory; accept `cap_std::fs::Dir` handles and camino paths instead so callers choose the capability surface.\n = help: Pass `cap_std::fs::Dir` plus `camino::Utf8Path`/`Utf8PathBuf` parameters through your APIs instead of calling std::fs directly.\n = note: `#[deny(no_std_fs_operations)]` on by default\n\n", + "spans": [ + { + "byte_end": 92, + "byte_start": 79, + "column_end": 18, + "column_start": 5, + "expansion": null, + "file_name": "src/lib.rs", + "is_primary": true, + "label": null, + "line_end": 3, + "line_start": 3, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 18, + "highlight_start": 5, + "text": "use std::fs::File;" + } + ] + } + ] + }, + { + "children": [ + { + "children": [], + "code": null, + "level": "note", + "message": "std::fs touches the ambient working directory; accept `cap_std::fs::Dir` handles and camino paths instead so callers choose the capability surface.", + "rendered": null, + "spans": [] + }, + { + "children": [], + "code": null, + "level": "help", + "message": "Pass `cap_std::fs::Dir` plus `camino::Utf8Path`/`Utf8PathBuf` parameters through your APIs instead of calling std::fs directly.", + "rendered": null, + "spans": [] + } + ], + "code": { + "code": "no_std_fs_operations", + "explanation": null + }, + "level": "error", + "message": "std::fs operation `std::fs::File` bypasses the capability-based filesystem policy.", + "rendered": "error: std::fs operation `std::fs::File` bypasses the capability-based filesystem policy.\n --> src/lib.rs:18:62\n |\n18 | pub fn open_file>(path: P) -> std::io::Result {\n | ^^^^\n |\n = note: std::fs touches the ambient working directory; accept `cap_std::fs::Dir` handles and camino paths instead so callers choose the capability surface.\n = help: Pass `cap_std::fs::Dir` plus `camino::Utf8Path`/`Utf8PathBuf` parameters through your APIs instead of calling std::fs directly.\n\n", + "spans": [ + { + "byte_end": 467, + "byte_start": 463, + "column_end": 66, + "column_start": 62, + "expansion": null, + "file_name": "src/lib.rs", + "is_primary": true, + "label": null, + "line_end": 18, + "line_start": 18, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 66, + "highlight_start": 62, + "text": "pub fn open_file>(path: P) -> std::io::Result {" + } + ] + } + ] + }, + { + "children": [ + { + "children": [], + "code": null, + "level": "note", + "message": "std::fs touches the ambient working directory; accept `cap_std::fs::Dir` handles and camino paths instead so callers choose the capability surface.", + "rendered": null, + "spans": [] + }, + { + "children": [], + "code": null, + "level": "help", + "message": "Pass `cap_std::fs::Dir` plus `camino::Utf8Path`/`Utf8PathBuf` parameters through your APIs instead of calling std::fs directly.", + "rendered": null, + "spans": [] + } + ], + "code": { + "code": "no_std_fs_operations", + "explanation": null + }, + "level": "error", + "message": "std::fs operation `std::fs::File::open` bypasses the capability-based filesystem policy.", + "rendered": "error: std::fs operation `std::fs::File::open` bypasses the capability-based filesystem policy.\n --> src/lib.rs:19:5\n |\n19 | File::open(path)\n | ^^^^^^^^^^\n |\n = note: std::fs touches the ambient working directory; accept `cap_std::fs::Dir` handles and camino paths instead so callers choose the capability surface.\n = help: Pass `cap_std::fs::Dir` plus `camino::Utf8Path`/`Utf8PathBuf` parameters through your APIs instead of calling std::fs directly.\n\n", + "spans": [ + { + "byte_end": 485, + "byte_start": 475, + "column_end": 15, + "column_start": 5, + "expansion": null, + "file_name": "src/lib.rs", + "is_primary": true, + "label": null, + "line_end": 19, + "line_start": 19, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 15, + "highlight_start": 5, + "text": " File::open(path)" + } + ] + } + ] + }, + { + "children": [ + { + "children": [], + "code": null, + "level": "note", + "message": "std::fs touches the ambient working directory; accept `cap_std::fs::Dir` handles and camino paths instead so callers choose the capability surface.", + "rendered": null, + "spans": [] + }, + { + "children": [], + "code": null, + "level": "help", + "message": "Pass `cap_std::fs::Dir` plus `camino::Utf8Path`/`Utf8PathBuf` parameters through your APIs instead of calling std::fs directly.", + "rendered": null, + "spans": [] + } + ], + "code": { + "code": "no_std_fs_operations", + "explanation": null + }, + "level": "error", + "message": "std::fs operation `std::fs::File` bypasses the capability-based filesystem policy.", + "rendered": "error: std::fs operation `std::fs::File` bypasses the capability-based filesystem policy.\n --> src/lib.rs:19:5\n |\n19 | File::open(path)\n | ^^^^\n |\n = note: std::fs touches the ambient working directory; accept `cap_std::fs::Dir` handles and camino paths instead so callers choose the capability surface.\n = help: Pass `cap_std::fs::Dir` plus `camino::Utf8Path`/`Utf8PathBuf` parameters through your APIs instead of calling std::fs directly.\n\n", + "spans": [ + { + "byte_end": 479, + "byte_start": 475, + "column_end": 9, + "column_start": 5, + "expansion": null, + "file_name": "src/lib.rs", + "is_primary": true, + "label": null, + "line_end": 19, + "line_start": 19, + "suggested_replacement": null, + "suggestion_applicability": null, + "text": [ + { + "highlight_end": 9, + "highlight_start": 5, + "text": " File::open(path)" + } + ] + } + ] + } +] diff --git a/crates/no_std_fs_operations/tests/test_support/mod.rs b/crates/no_std_fs_operations/tests/test_support/mod.rs new file mode 100644 index 00000000..6e22d765 --- /dev/null +++ b/crates/no_std_fs_operations/tests/test_support/mod.rs @@ -0,0 +1,159 @@ +//! Shared, test-only fixture helpers for `no_std_fs_operations` integration +//! tests. + +use std::fs; +use std::path::{Path, PathBuf}; + +use anyhow::Context as _; +use tempfile::TempDir; + +/// Standalone project fixture created in a temporary directory. +pub(super) struct FixtureProject { + _temp_dir: TempDir, + root: PathBuf, +} + +impl FixtureProject { + /// Returns the fixture project root directory. + pub(super) fn root(&self) -> &Path { + &self.root + } +} + +/// Creates a temporary fixture project for verifying exclusion behaviour. +/// +/// # Examples +/// +/// ```ignore +/// let fixture = create_fixture_project("excluded_test_crate", true)?; +/// assert!(fixture.root().join("dylint.toml").exists()); +/// # Ok::<(), anyhow::Error>(()) +/// ``` +pub(super) fn create_fixture_project( + crate_name: &str, + is_excluded: bool, +) -> anyhow::Result { + let temp_dir = TempDir::new().context("failed to create temporary fixture directory")?; + let root = temp_dir.path().to_path_buf(); + + fs::write( + root.join("Cargo.toml"), + format!( + concat!( + "[package]\n", + "name = {crate_name}\n", + "version = \"0.1.0\"\n", + "edition = \"2024\"\n", + "\n", + "[workspace]\n", + "\n", + "[dependencies]\n", + ), + crate_name = toml::Value::String(crate_name.to_owned()) + ), + ) + .context("failed to write fixture Cargo.toml")?; + + fs::write( + root.join("dylint.toml"), + fixture_dylint_config(crate_name, is_excluded), + ) + .context("failed to write fixture dylint.toml")?; + + let source_dir = root.join("src"); + fs::create_dir(&source_dir).context("failed to create fixture src directory")?; + fs::write(source_dir.join("lib.rs"), fixture_source(crate_name)) + .context("failed to write fixture source")?; + + Ok(FixtureProject { + _temp_dir: temp_dir, + root, + }) +} + +fn fixture_dylint_config(crate_name: &str, is_excluded: bool) -> String { + let excluded_crates = toml::Value::Array(if is_excluded { + vec![toml::Value::String(crate_name.to_owned())] + } else { + Vec::new() + }); + + format!( + concat!( + "[no_std_fs_operations]\n", + "excluded_crates = {excluded_crates}\n", + ), + excluded_crates = excluded_crates + ) +} + +fn fixture_source(crate_name: &str) -> String { + format!( + concat!( + "//! Temporary fixture crate for `no_std_fs_operations` integration tests.\n", + "\n", + "use std::fs::File;\n", + "use std::path::Path;\n", + "\n", + "/// Opens a file for reading.\n", + "///\n", + "/// # Examples\n", + "///\n", + "/// ```no_run\n", + "/// use {crate_name}::open_file;\n", + "///\n", + "/// let file = open_file(\"Cargo.toml\").expect(\"file should exist\");\n", + "/// let result = open_file(\"nonexistent.txt\");\n", + "/// assert!(result.is_err());\n", + "/// # drop(file);\n", + "/// ```\n", + "pub fn open_file>(path: P) -> std::io::Result {{\n", + " File::open(path)\n", + "}}\n", + ), + crate_name = crate_name + ) +} + +#[cfg(test)] +mod tests { + use super::{create_fixture_project, fixture_dylint_config}; + + #[test] + fn dylint_config_escapes_crate_names_as_toml_values() { + let crate_name = "crate\"]\ninjected = true\n[other"; + let config = fixture_dylint_config(crate_name, true); + let parsed: toml::Value = toml::from_str(&config).expect("config should parse as TOML"); + + assert_eq!( + parsed["no_std_fs_operations"]["excluded_crates"][0] + .as_str() + .expect("excluded crate should be a string"), + crate_name + ); + assert!(parsed.get("other").is_none(), "config was:\n{config}"); + assert!(parsed.get("injected").is_none(), "config was:\n{config}"); + } + + #[test] + fn fixture_manifest_escapes_crate_names_as_toml_values() -> anyhow::Result<()> { + let crate_name = "crate\"]\ninjected = true\n[other"; + let fixture = create_fixture_project(crate_name, true)?; + let manifest = std::fs::read_to_string(fixture.root().join("Cargo.toml"))?; + let parsed: toml::Value = toml::from_str(&manifest)?; + + assert_eq!( + parsed["package"]["name"] + .as_str() + .expect("package name should be a string"), + crate_name + ); + assert!(parsed.get("other").is_none(), "manifest was:\n{manifest}"); + assert!( + parsed.get("injected").is_none(), + "manifest was:\n{manifest}" + ); + + Ok(()) + } +} diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 6950f7c9..3469a9c6 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -15,9 +15,9 @@ Whitaker itself. For using Whitaker lints in a project, see the cargo install cargo-dylint dylint-link ``` -CI also installs or provides job-specific tools such as `cargo-nextest`, -`bun`, `uv`, Mermaid CLI, and Nixie before running the targets that need them. -Local runs of those targets require the same tools on `PATH`. +CI also installs or provides job-specific tools such as `cargo-nextest`, `bun`, +`uv`, Mermaid CLI, and Nixie before running the targets that need them. Local +runs of those targets require the same tools on `PATH`. ## Running Tests @@ -37,12 +37,15 @@ the `excluded_crates` configuration. These integration tests invoke `cargo dylint` in a subprocess, so they exercise the full lint-loading and configuration path, rather than only unit-level helpers. -The fixtures live under `crates/no_std_fs_operations/tests/fixtures/` as two -small crates: `excluded_project`, which configures the exclusion, and -`non_excluded_project`, which leaves the lint unexcluded. Each fixture -`Cargo.toml` includes an empty `[workspace]` table so Cargo treats the fixture -as its own workspace root. This prevents Cargo from resolving upwards to the -enclosing Whitaker workspace and inheriting unrelated configuration. +Fixture projects are generated at runtime using `create_fixture_project`, which +writes a `Cargo.toml`, `dylint.toml`, and `src/lib.rs` into a `TempDir` and +returns a `FixtureProject` handle. The `FixtureProject` owns the `TempDir` so +the directory is cleaned up automatically when the handle is dropped. Passing +`is_excluded: true` writes `excluded_crates = [""]` into +`dylint.toml`; `false` writes an empty list. Each fixture `Cargo.toml` contains +an empty `[workspace]` table (omitted here for brevity) so Cargo treats the +fixture as its own workspace root and does not resolve upwards to the enclosing +Whitaker workspace. The harness centres on `run_cargo_dylint`, which executes `cargo dylint --all -- --message-format json` with `DYLINT_LIBRARY_PATH` set to @@ -52,11 +55,17 @@ during the run. `diagnostic_count` then parses the JSON message stream with `code.code` is `no_std_fs_operations`, which keeps the assertions tied to the lint's structured diagnostics instead of brittle text matching. +The shared helper `run_exclusion_test(crate_name, is_excluded, expectation)` +resolves the lint library path via a `OnceLock`-cached `build_lint_library` +call, creates the fixture project, and delegates to `assert_fixture_behaviour`. +Both parametrised cases in `exclusion_crates_behaviour_test` delegate to this +helper. + The tests are annotated with `#[serial]` from `serial_test`, and the repository-level nextest contract also requires them to match the `serial-dylint-ui` test group in `.config/nextest.toml` when they are exercised through `make test`. Both the attribute and the repo-level group are required -for correct serialized execution because nextest runs each test in a separate +for correct serialised execution because nextest runs each test in a separate process, so the in-process `#[serial]` mutex alone is not sufficient. They are also marked `#[ignore]` by default because they depend on external tooling and a buildable workspace. Before running them, install `cargo-dylint` and @@ -69,11 +78,11 @@ cargo test -p no_std_fs_operations --test integration_exclusion -- --ignored cargo nextest run -p no_std_fs_operations --test integration_exclusion --run-ignored ignored-only ``` -The parameterized `#[rstest]` case -`exclusion_behaviour_matches_fixture_configuration` covers both fixtures. For -each case, it asserts the subprocess exit status and the `no_std_fs_operations` -diagnostic count, so the test verifies both the success path for excluded -crates and the failure path for non-excluded crates. +The parametrised `#[rstest]` case `exclusion_crates_behaviour_test` covers both +fixture configurations. For each case it asserts the subprocess exit status and +the `no_std_fs_operations` diagnostic count, so the test verifies both the +success path for excluded crates (zero diagnostics, exit 0) and the failure +path for non-excluded crates (one or more diagnostics, non-zero exit). ### Fixture-based harness regressions @@ -134,16 +143,15 @@ packaging path stays valid. The release dry-run target is a POSIX-shell target; Windows CI runs it under Bash and requires the same command-line tools as local POSIX environments. -Both lanes share the workflow-level environment contract: -`BUILD_PROFILE=debug` narrows `sccache` keys to debug builds only, preventing -cache pollution from release builds; `CARGO_INCREMENTAL=0` disables incremental -compilation, which is incompatible with `sccache`; `RUSTC_WRAPPER=sccache` -routes all `rustc` invocations through `sccache`; `SCCACHE_GHA_ENABLED=true` -activates the GitHub Actions cache backend for `sccache`; and -`RUSTFLAGS=-D warnings` and `RUSTDOCFLAGS=-D warnings` deny compiler and doc -warnings as errors across both lanes. Together, these variables keep the cache -scope narrow, ensure `sccache` is active for all compilation, and enforce a -warnings-as-errors build contract. +Both lanes share the workflow-level environment contract: `BUILD_PROFILE=debug` +narrows `sccache` keys to debug builds only, preventing cache pollution from +release builds; `CARGO_INCREMENTAL=0` disables incremental compilation, which +is incompatible with `sccache`; `RUSTC_WRAPPER=sccache` routes all `rustc` +invocations through `sccache`; `SCCACHE_GHA_ENABLED=true` activates the GitHub +Actions cache backend for `sccache`; and `RUSTFLAGS=-D warnings` and +`RUSTDOCFLAGS=-D warnings` deny compiler and doc warnings as errors across both +lanes. Together, these variables keep the cache scope narrow, ensure `sccache` +is active for all compilation, and enforce a warnings-as-errors build contract. ### CI build caching diff --git a/docs/roadmap.md b/docs/roadmap.md index e67d05f5..cbdee208 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -231,6 +231,10 @@ pipeline. - [ ] 4.1.3. Enforce lint-level deny rules and fail builds on warnings across the workspace. +- [x] 4.1.4. Add end-to-end behavioural integration tests for `excluded_crates` + in `no_std_fs_operations` using runtime-generated `TempDir` fixture projects + and `cargo dylint` subprocess invocation. Closes + [`#111`](https://github.com/leynos/whitaker/issues/111). ### 4.2. Release metadata @@ -321,8 +325,8 @@ advice. Requires 6.4.1. - [x] 6.4.6. Use Kani to verify `propagate_labels` preserves valid label indices, returns one label per input vector, and terminates within the - supplied iteration bound. Verification harnesses are complete modulo the - CBMC state-explosion blocker documented in the design notes. See + supplied iteration bound. Verification harnesses are complete modulo the CBMC + state-explosion blocker documented in the design notes. See [brain trust lints design](brain-trust-lints-design.md) §Decomposition advice. Requires 6.4.1.