From d341c1565a82a8ab9e652018b106835a7ec6bc9d Mon Sep 17 00:00:00 2001 From: konojunya Date: Thu, 3 Sep 2026 17:05:16 +0900 Subject: [PATCH] Add atomic stack fmt command --- .github/workflows/ci.yaml | 24 +- Cargo.toml | 8 + README.md | 17 +- src/lib.rs | 545 +++++++++++++++++++++++++++++++-- src/main.rs | 2 + tests/cli.rs | 195 +++++++++++- tests/formatter_conformance.rs | 76 +++++ tests/specification-revision | 1 + 8 files changed, 841 insertions(+), 27 deletions(-) create mode 100644 tests/formatter_conformance.rs create mode 100644 tests/specification-revision diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index e5fadd3..34239b9 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -15,6 +15,19 @@ jobs: steps: - name: Check out repository uses: actions/checkout@v7 + - name: Read supported specification revision + id: specification + shell: bash + run: | + grep -Eq '^[0-9a-f]{40}$' tests/specification-revision + revision=$(tr -d '\n' < tests/specification-revision) + printf 'revision=%s\n' "$revision" >> "$GITHUB_OUTPUT" + - name: Check out canonical specification + uses: actions/checkout@v7 + with: + repository: stack-sh/specification + ref: ${{ steps.specification.outputs.revision }} + path: .stack-specification - name: Check whitespace run: git diff --check "$(git hash-object -t tree /dev/null)" HEAD - name: Install latest stable Rust toolchain @@ -23,8 +36,12 @@ jobs: run: cargo +stable fmt --check - name: Run tests run: cargo +stable test --locked + - name: Run canonical formatter suite + env: + STACK_SPECIFICATION_DIR: ${{ github.workspace }}/.stack-specification + run: cargo +stable test --features conformance --test formatter-conformance --locked - name: Run Clippy - run: cargo +stable clippy --all-targets --locked -- -D warnings + run: cargo +stable clippy --all-targets --all-features --locked -- -D warnings - name: Build documentation env: RUSTDOCFLAGS: -D warnings @@ -35,6 +52,8 @@ jobs: tool: cargo-llvm-cov@0.9.0 fallback: none - name: Enforce test coverage + env: + STACK_SPECIFICATION_DIR: ${{ github.workspace }}/.stack-specification run: cargo +stable llvm-cov --all-features --locked --fail-under-lines 90 --fail-under-functions 95 --fail-under-regions 90 - name: Build release binary run: cargo +stable build --release --locked @@ -50,6 +69,7 @@ jobs: test -s Cargo.toml test -s Cargo.lock test -s src/main.rs + test -s tests/specification-revision msrv: name: Minimum supported Rust @@ -62,4 +82,4 @@ jobs: - name: Run tests run: cargo +1.85.0 test --locked - name: Run Clippy - run: cargo +1.85.0 clippy --all-targets --locked -- -D warnings + run: cargo +1.85.0 clippy --all-targets --all-features --locked -- -D warnings diff --git a/Cargo.toml b/Cargo.toml index e5186f5..0456099 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,14 @@ path = "src/main.rs" [dependencies] stack-engine = { git = "https://github.com/stack-sh/engine.git", rev = "07b71c783a3c1f9037d865b19672a9522eb0b24d" } +[features] +conformance = [] + +[[test]] +name = "formatter-conformance" +path = "tests/formatter_conformance.rs" +required-features = ["conformance"] + [lints.clippy] expect_used = "deny" panic = "deny" diff --git a/README.md b/README.md index bf963db..cf167ef 100644 --- a/README.md +++ b/README.md @@ -2,28 +2,31 @@ `stack-sh/cli` is the private source repository for the native Rust `stack` command. -The repository now contains the first native command: `stack check`. The CLI is not yet distributed as a supported external binary and its interface remains pre-release. +The repository contains native validation and formatting commands. The CLI is not yet distributed as a supported external binary and its interface remains pre-release. ## Commands ```text stack check arch.stack +stack fmt arch.stack +stack fmt --check arch.stack +stack fmt - ``` `stack check` reads the file as bytes and runs the full compiler, theme, layout, and routing validation pipeline without changing the source. Diagnostics are written to standard error in source order. Standard output remains empty. +`stack fmt` uses the engine formatter and preserves comments. File mode replaces changed source atomically through a temporary file in the same directory; unchanged files are not replaced. Syntax, encoding, and host I/O failures leave the original file untouched. `stack fmt -` reads bytes from standard input and writes only canonical source to standard output. `--check` never writes source and exits with status `1` when formatting is required. + | Result | Exit status | | --- | ---: | | No error diagnostics, including warning-only input | `0` | -| One or more Stack error diagnostics | `1` | +| One or more Stack error diagnostics, or `fmt --check` finds a difference | `1` | | Invalid arguments, host I/O failure, or engine operational failure | `2` | -The remaining planned commands are: +The remaining planned command is: ```text stack render arch.stack -o arch.svg -stack fmt arch.stack -stack fmt --check arch.stack ``` The CLI will link `stack-engine` as a native Rust dependency. It owns filesystem and standard-stream behavior, process exit codes, configuration discovery, and command presentation. It must not duplicate compiler, formatter, layout, or SVG-rendering logic. @@ -37,11 +40,13 @@ The CLI requires Rust 1.85 or newer. ```sh cargo run -- check arch.stack cargo test --locked -cargo clippy --all-targets --locked -- -D warnings +cargo clippy --all-targets --all-features --locked -- -D warnings ``` CI validates formatting, unit and process-level integration tests, at least 90% line/region coverage and 95% function coverage, Clippy, documentation, a release build, `--help`, and `--version` on stable Rust. Tests and Clippy also run on Rust 1.85. +Canonical formatter behavior is checked against the pinned `stack-sh/specification` fixture revision recorded in `tests/specification-revision`. + ## Licensing The private source code in this repository is not currently offered under an open-source license. See [LICENSING.md](./LICENSING.md) for the decisions required before external distribution. diff --git a/src/lib.rs b/src/lib.rs index 157e3d9..7dc9a01 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,11 +4,11 @@ use std::ffi::{OsStr, OsString}; use std::fmt::Write as _; -use std::fs; -use std::io::{self, Write}; -use std::path::Path; +use std::fs::{self, File, OpenOptions}; +use std::io::{self, Read, Write}; +use std::path::{Path, PathBuf}; -use stack_engine::{CheckOutput, Diagnostic, Engine, OperationalError, Severity}; +use stack_engine::{CheckOutput, Diagnostic, Engine, FormatOutput, OperationalError, Severity}; /// Exit status used when a command completes without Stack error diagnostics. pub const EXIT_SUCCESS: u8 = 0; @@ -17,13 +17,21 @@ pub const EXIT_STACK_ERROR: u8 = 1; /// Exit status used for argument, host I/O, or engine operational failures. pub const EXIT_USAGE_OR_IO: u8 = 2; -const GENERAL_HELP: &str = "Stack diagram toolchain\n\nUsage:\n stack check \n stack --help\n stack --version\n\nCommands:\n check Validate a Stack source file without modifying it\n"; +const GENERAL_HELP: &str = "Stack diagram toolchain\n\nUsage:\n stack check \n stack fmt [--check] \n stack --help\n stack --version\n\nCommands:\n check Validate a Stack source file without modifying it\n fmt Format a file in place or read from standard input\n"; const CHECK_HELP: &str = "Validate a Stack source file without modifying it\n\nUsage:\n stack check \n"; +const FORMAT_HELP: &str = "Format Stack source canonically\n\nUsage:\n stack fmt \n stack fmt --check \n stack fmt -\n\nArguments:\n Format the file atomically in place\n - Read from standard input and write to standard output\n\nOptions:\n --check Report whether formatting is required without writing output\n"; + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum FormatMode { + Write, + Check, +} /// Runs the CLI with explicit streams and returns its process exit status. pub fn run( arguments: impl IntoIterator, + stdin: &mut dyn Read, stdout: &mut dyn Write, stderr: &mut dyn Write, ) -> u8 { @@ -57,6 +65,9 @@ pub fn run( if command == OsStr::new("check") { return run_check(arguments, stdout, stderr); } + if command == OsStr::new("fmt") { + return run_format(arguments, stdin, stdout, stderr); + } argument_error( &format!("unknown command '{}'", command.to_string_lossy()), @@ -64,6 +75,53 @@ pub fn run( ) } +fn run_format( + mut arguments: impl Iterator, + stdin: &mut dyn Read, + stdout: &mut dyn Write, + stderr: &mut dyn Write, +) -> u8 { + let Some(first) = arguments.next() else { + return argument_error("missing file for 'stack fmt'", stderr); + }; + if first == OsStr::new("--help") || first == OsStr::new("-h") { + if let Some(extra) = arguments.next() { + return argument_error( + &format!("unexpected argument '{}'", extra.to_string_lossy()), + stderr, + ); + } + return write_stdout(FORMAT_HELP, stdout, stderr); + } + + let (mode, input) = if first == OsStr::new("--check") { + let Some(input) = arguments.next() else { + return argument_error("missing file for 'stack fmt --check'", stderr); + }; + (FormatMode::Check, input) + } else { + (FormatMode::Write, first) + }; + if input != OsStr::new("-") && input.to_string_lossy().starts_with('-') { + return argument_error( + &format!("unknown option '{}'", input.to_string_lossy()), + stderr, + ); + } + if let Some(extra) = arguments.next() { + return argument_error( + &format!("unexpected argument '{}'", extra.to_string_lossy()), + stderr, + ); + } + + if input == OsStr::new("-") { + format_stdin(mode, stdin, stdout, stderr) + } else { + format_file(mode, Path::new(&input), stderr) + } +} + fn run_check( mut arguments: impl Iterator, stdout: &mut dyn Write, @@ -125,13 +183,88 @@ fn check_file_with( ); } }; - let has_errors = output - .diagnostics - .iter() - .any(|diagnostic| diagnostic.severity == Severity::Error); - let rendered = render_diagnostics(path, &output.diagnostics); - if !rendered.is_empty() && stderr.write_all(rendered.as_bytes()).is_err() { - return EXIT_USAGE_OR_IO; + let has_errors = match write_diagnostics(path, &output.diagnostics, stderr) { + Ok(has_errors) => has_errors, + Err(()) => return EXIT_USAGE_OR_IO, + }; + + if has_errors { + EXIT_STACK_ERROR + } else { + EXIT_SUCCESS + } +} + +fn format_file(mode: FormatMode, path: &Path, stderr: &mut dyn Write) -> u8 { + format_file_with( + mode, + path, + stderr, + |source| Engine::bundled().format(source), + atomic_replace, + ) +} + +fn format_file_with( + mode: FormatMode, + path: &Path, + stderr: &mut dyn Write, + format: impl FnOnce(&[u8]) -> Result, + replace: impl FnOnce(&Path, &[u8]) -> io::Result<()>, +) -> u8 { + let source = match fs::read(path) { + Ok(source) => source, + Err(error) => { + return write_stderr_error( + &format!( + "cannot read '{}': {}", + path.display(), + stable_io_error(error.kind()) + ), + stderr, + ); + } + }; + let output = match format(&source) { + Ok(output) => output, + Err(error) => { + return write_stderr_error( + &format!("cannot format '{}': {error}", path.display()), + stderr, + ); + } + }; + let has_errors = match write_diagnostics(path, &output.diagnostics, stderr) { + Ok(has_errors) => has_errors, + Err(()) => return EXIT_USAGE_OR_IO, + }; + let Some(formatted) = output.formatted_source else { + return if has_errors { + EXIT_STACK_ERROR + } else { + write_stderr_error("formatter produced no source or error diagnostic", stderr) + }; + }; + let changed = formatted.as_bytes() != source; + + if mode == FormatMode::Check { + return if changed || has_errors { + EXIT_STACK_ERROR + } else { + EXIT_SUCCESS + }; + } + if changed { + if let Err(error) = replace(path, formatted.as_bytes()) { + return write_stderr_error( + &format!( + "cannot replace '{}': {}", + path.display(), + stable_io_error(error.kind()) + ), + stderr, + ); + } } if has_errors { @@ -141,6 +274,116 @@ fn check_file_with( } } +fn format_stdin( + mode: FormatMode, + stdin: &mut dyn Read, + stdout: &mut dyn Write, + stderr: &mut dyn Write, +) -> u8 { + let mut source = Vec::new(); + if stdin.read_to_end(&mut source).is_err() { + return write_stderr_error("cannot read standard input", stderr); + } + format_stdin_with(mode, &source, stdout, stderr, |source| { + Engine::bundled().format(source) + }) +} + +fn format_stdin_with( + mode: FormatMode, + source: &[u8], + stdout: &mut dyn Write, + stderr: &mut dyn Write, + format: impl FnOnce(&[u8]) -> Result, +) -> u8 { + let output = match format(source) { + Ok(output) => output, + Err(error) => return write_stderr_error(&format!("cannot format stdin: {error}"), stderr), + }; + let has_errors = match write_diagnostics(Path::new(""), &output.diagnostics, stderr) { + Ok(has_errors) => has_errors, + Err(()) => return EXIT_USAGE_OR_IO, + }; + let Some(formatted) = output.formatted_source else { + return if has_errors { + EXIT_STACK_ERROR + } else { + write_stderr_error("formatter produced no source or error diagnostic", stderr) + }; + }; + let changed = formatted.as_bytes() != source; + + if mode == FormatMode::Check { + return if changed || has_errors { + EXIT_STACK_ERROR + } else { + EXIT_SUCCESS + }; + } + if stdout.write_all(formatted.as_bytes()).is_err() { + return write_stderr_error("cannot write formatted source", stderr); + } + if has_errors { + EXIT_STACK_ERROR + } else { + EXIT_SUCCESS + } +} + +fn atomic_replace(path: &Path, contents: &[u8]) -> io::Result<()> { + let permissions = fs::metadata(path)?.permissions(); + let parent = path + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or_else(|| Path::new(".")); + let (temporary_path, mut temporary_file) = create_temporary_file(parent)?; + + let prepared = temporary_file + .write_all(contents) + .and_then(|()| temporary_file.set_permissions(permissions)) + .and_then(|()| temporary_file.sync_all()); + drop(temporary_file); + if let Err(error) = prepared { + let _ = fs::remove_file(&temporary_path); + return Err(error); + } + if let Err(error) = fs::rename(&temporary_path, path) { + let _ = fs::remove_file(&temporary_path); + return Err(error); + } + Ok(()) +} + +fn create_temporary_file(parent: &Path) -> io::Result<(PathBuf, File)> { + for attempt in 0..128_u8 { + let path = parent.join(format!(".stack-tmp-{}-{attempt}", std::process::id())); + match OpenOptions::new().write(true).create_new(true).open(&path) { + Ok(file) => return Ok((path, file)), + Err(error) if error.kind() == io::ErrorKind::AlreadyExists => {} + Err(error) => return Err(error), + } + } + Err(io::Error::new( + io::ErrorKind::AlreadyExists, + "could not reserve an atomic replacement file", + )) +} + +fn write_diagnostics( + path: &Path, + diagnostics: &[Diagnostic], + stderr: &mut dyn Write, +) -> Result { + let has_errors = diagnostics + .iter() + .any(|diagnostic| diagnostic.severity == Severity::Error); + let rendered = render_diagnostics(path, diagnostics); + if !rendered.is_empty() && stderr.write_all(rendered.as_bytes()).is_err() { + return Err(()); + } + Ok(has_errors) +} + fn render_diagnostics(path: &Path, diagnostics: &[Diagnostic]) -> String { let mut rendered = String::new(); for diagnostic in diagnostics { @@ -207,6 +450,14 @@ mod tests { struct FailingWriter; + struct FailingReader; + + impl Read for FailingReader { + fn read(&mut self, _buffer: &mut [u8]) -> io::Result { + Err(io::Error::other("test reader failure")) + } + } + impl Write for FailingWriter { fn write(&mut self, _buffer: &[u8]) -> io::Result { Err(io::Error::other("test writer failure")) @@ -217,12 +468,20 @@ mod tests { } } + fn run_without_input( + arguments: impl IntoIterator, + stdout: &mut dyn Write, + stderr: &mut dyn Write, + ) -> u8 { + run(arguments, &mut io::empty(), stdout, stderr) + } + #[test] fn help_version_and_argument_errors_have_stable_streams() { let mut stdout = Vec::new(); let mut stderr = Vec::new(); assert_eq!( - run([OsString::from("--version")], &mut stdout, &mut stderr), + run_without_input([OsString::from("--version")], &mut stdout, &mut stderr), EXIT_SUCCESS ); assert_eq!(stdout, b"stack 0.1.0\n"); @@ -230,13 +489,16 @@ mod tests { stdout.clear(); assert_eq!( - run([OsString::from("--help")], &mut stdout, &mut stderr), + run_without_input([OsString::from("--help")], &mut stdout, &mut stderr), EXIT_SUCCESS ); assert_eq!(stdout, GENERAL_HELP.as_bytes()); stdout.clear(); - assert_eq!(run([], &mut stdout, &mut stderr), EXIT_USAGE_OR_IO); + assert_eq!( + run_without_input([], &mut stdout, &mut stderr), + EXIT_USAGE_OR_IO + ); assert!(stdout.is_empty()); assert!(String::from_utf8_lossy(&stderr).contains("error: missing command")); } @@ -247,7 +509,7 @@ mod tests { let mut stdout = Vec::new(); let mut stderr = Vec::new(); assert_eq!( - run([OsString::from(alias)], &mut stdout, &mut stderr), + run_without_input([OsString::from(alias)], &mut stdout, &mut stderr), EXIT_SUCCESS ); assert!(!stdout.is_empty()); @@ -264,6 +526,19 @@ mod tests { OsString::from("--help"), OsString::from("extra"), ], + vec![OsString::from("fmt")], + vec![ + OsString::from("fmt"), + OsString::from("--help"), + OsString::from("extra"), + ], + vec![OsString::from("fmt"), OsString::from("--check")], + vec![OsString::from("fmt"), OsString::from("--unknown")], + vec![ + OsString::from("fmt"), + OsString::from("file.stack"), + OsString::from("extra"), + ], vec![ OsString::from("check"), OsString::from("file.stack"), @@ -272,7 +547,10 @@ mod tests { ] { let mut stdout = Vec::new(); let mut stderr = Vec::new(); - assert_eq!(run(arguments, &mut stdout, &mut stderr), EXIT_USAGE_OR_IO); + assert_eq!( + run_without_input(arguments, &mut stdout, &mut stderr), + EXIT_USAGE_OR_IO + ); assert!(stdout.is_empty()); assert!(String::from_utf8_lossy(&stderr).starts_with("error:")); } @@ -280,7 +558,7 @@ mod tests { let mut stdout = Vec::new(); let mut stderr = Vec::new(); assert_eq!( - run( + run_without_input( [OsString::from("check"), OsString::from("-h")], &mut stdout, &mut stderr, @@ -289,6 +567,17 @@ mod tests { ); assert_eq!(stdout, CHECK_HELP.as_bytes()); assert!(stderr.is_empty()); + + stdout.clear(); + assert_eq!( + run_without_input( + [OsString::from("fmt"), OsString::from("-h")], + &mut stdout, + &mut stderr, + ), + EXIT_SUCCESS + ); + assert_eq!(stdout, FORMAT_HELP.as_bytes()); } #[test] @@ -375,4 +664,224 @@ mod tests { assert_eq!(operational_status, EXIT_USAGE_OR_IO); assert!(String::from_utf8_lossy(&operational_stderr).contains("error: cannot check")); } + + #[test] + fn format_stream_failures_use_host_exit_status() { + let source = b"stack 1.0 diagram \"Valid\" { node api \"API\" }"; + let mut stdout = Vec::new(); + let mut stderr = Vec::new(); + assert_eq!( + run( + [OsString::from("fmt"), OsString::from("-")], + &mut FailingReader, + &mut stdout, + &mut stderr, + ), + EXIT_USAGE_OR_IO + ); + assert!(String::from_utf8_lossy(&stderr).contains("cannot read standard input")); + + stderr.clear(); + assert_eq!( + format_stdin_with(FormatMode::Write, source, &mut stdout, &mut stderr, |_| { + Err(OperationalError::InvalidIntermediateRepresentation { + reason: "test failure", + }) + },), + EXIT_USAGE_OR_IO + ); + assert!(String::from_utf8_lossy(&stderr).contains("cannot format stdin")); + + let output = Engine::bundled().format(source); + assert!(output.is_ok()); + let Ok(mut empty_output) = output else { + return; + }; + empty_output.formatted_source = None; + empty_output.diagnostics.clear(); + stderr.clear(); + assert_eq!( + format_stdin_with(FormatMode::Write, source, &mut stdout, &mut stderr, |_| Ok( + empty_output + ),), + EXIT_USAGE_OR_IO + ); + + let syntax = b"stack 1.0 diagram \"Incomplete\" {"; + let mut failed_stderr = FailingWriter; + assert_eq!( + format_stdin_with( + FormatMode::Write, + syntax, + &mut stdout, + &mut failed_stderr, + |source| Engine::bundled().format(source), + ), + EXIT_USAGE_OR_IO + ); + + stderr.clear(); + assert_eq!( + format_stdin_with( + FormatMode::Write, + syntax, + &mut stdout, + &mut stderr, + |source| Engine::bundled().format(source), + ), + EXIT_STACK_ERROR + ); + + let semantic = b"stack 1.0 diagram \"Invalid\" { node api \"A\" node api \"B\" }"; + stdout.clear(); + stderr.clear(); + assert_eq!( + format_stdin_with( + FormatMode::Write, + semantic, + &mut stdout, + &mut stderr, + |source| Engine::bundled().format(source), + ), + EXIT_STACK_ERROR + ); + assert!(!stdout.is_empty()); + + let mut failed_stdout = FailingWriter; + stderr.clear(); + assert_eq!( + format_stdin_with( + FormatMode::Write, + source, + &mut failed_stdout, + &mut stderr, + |source| Engine::bundled().format(source), + ), + EXIT_USAGE_OR_IO + ); + } + + #[test] + fn format_file_failures_do_not_replace_the_source() { + let path = std::env::temp_dir().join(format!( + "stack-cli-format-failures-{}.stack", + std::process::id() + )); + let source = b"stack 1.0 diagram \"Valid\"{node api \"API\"}"; + assert!(fs::write(&path, source).is_ok()); + + let mut stderr = Vec::new(); + assert_eq!( + format_file_with( + FormatMode::Write, + &path, + &mut stderr, + |_| { + Err(OperationalError::InvalidIntermediateRepresentation { + reason: "test failure", + }) + }, + atomic_replace, + ), + EXIT_USAGE_OR_IO + ); + + let syntax = b"stack 1.0 diagram \"Incomplete\" {"; + assert!(fs::write(&path, syntax).is_ok()); + let mut failed_stderr = FailingWriter; + assert_eq!( + format_file_with( + FormatMode::Write, + &path, + &mut failed_stderr, + |source| Engine::bundled().format(source), + atomic_replace, + ), + EXIT_USAGE_OR_IO + ); + assert_eq!(fs::read(&path).ok().as_deref(), Some(syntax.as_slice())); + assert!(fs::write(&path, source).is_ok()); + + let output = Engine::bundled().format(source); + assert!(output.is_ok()); + let Ok(mut empty_output) = output else { + return; + }; + empty_output.formatted_source = None; + empty_output.diagnostics.clear(); + stderr.clear(); + assert_eq!( + format_file_with( + FormatMode::Write, + &path, + &mut stderr, + |_| Ok(empty_output), + atomic_replace, + ), + EXIT_USAGE_OR_IO + ); + + stderr.clear(); + assert_eq!( + format_file_with( + FormatMode::Write, + &path, + &mut stderr, + |source| Engine::bundled().format(source), + |_, _| Err(io::Error::from(io::ErrorKind::PermissionDenied)), + ), + EXIT_USAGE_OR_IO + ); + assert_eq!(fs::read(&path).ok().as_deref(), Some(source.as_slice())); + assert!(fs::remove_file(path).is_ok()); + } + + #[test] + fn atomic_replace_removes_its_temporary_file_after_rename_failure() { + let root = + std::env::temp_dir().join(format!("stack-cli-rename-failure-{}", std::process::id())); + let target = root.join("target"); + assert!(fs::create_dir(&root).is_ok()); + assert!(fs::create_dir(&target).is_ok()); + assert!(fs::write(target.join("keep"), b"keep").is_ok()); + + assert!(atomic_replace(&target, b"formatted").is_err()); + assert_eq!( + fs::read_dir(&root).ok().map(|entries| entries.count()), + Some(1) + ); + assert!(target.join("keep").is_file()); + assert!(fs::remove_dir_all(root).is_ok()); + } + + #[test] + fn temporary_file_creation_skips_collisions_and_is_bounded() { + let parent = + std::env::temp_dir().join(format!("stack-cli-temporary-files-{}", std::process::id())); + assert!(fs::create_dir(&parent).is_ok()); + for attempt in 0..128_u8 { + assert!( + fs::write( + parent.join(format!(".stack-tmp-{}-{attempt}", std::process::id())), + b"collision", + ) + .is_ok() + ); + } + let exhausted = create_temporary_file(&parent); + assert_eq!( + exhausted.err().map(|error| error.kind()), + Some(io::ErrorKind::AlreadyExists) + ); + assert!(fs::remove_dir_all(parent).is_ok()); + + let missing = + std::env::temp_dir().join(format!("stack-cli-missing-parent-{}", std::process::id())); + assert_eq!( + create_temporary_file(&missing) + .err() + .map(|error| error.kind()), + Some(io::ErrorKind::NotFound) + ); + } } diff --git a/src/main.rs b/src/main.rs index 071cd0e..73ee76b 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,10 +5,12 @@ use std::io; use std::process::ExitCode; fn main() -> ExitCode { + let mut stdin = io::stdin().lock(); let mut stdout = io::stdout().lock(); let mut stderr = io::stderr().lock(); ExitCode::from(stack_cli::run( env::args_os().skip(1), + &mut stdin, &mut stdout, &mut stderr, )) diff --git a/tests/cli.rs b/tests/cli.rs index 4f11e25..1823e0a 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -2,8 +2,9 @@ use std::env; use std::error::Error; use std::ffi::OsStr; use std::fs; +use std::io::Write; use std::path::{Path, PathBuf}; -use std::process::{Command, Output}; +use std::process::{Command, Output, Stdio}; use std::sync::atomic::{AtomicU64, Ordering}; static CASE_ID: AtomicU64 = AtomicU64::new(0); @@ -42,6 +43,24 @@ fn stack(arguments: impl IntoIterator>) -> Result>, + input: &[u8], +) -> Result> { + let mut child = Command::new(env!("CARGO_BIN_EXE_stack")) + .args(arguments) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + let Some(mut stdin) = child.stdin.take() else { + return Err("missing child standard input".into()); + }; + stdin.write_all(input)?; + drop(stdin); + Ok(child.wait_with_output()?) +} + fn assert_unchanged(path: &Path, expected: &[u8]) -> Result<(), Box> { assert_eq!(fs::read(path)?, expected); Ok(()) @@ -130,3 +149,177 @@ fn help_and_version_are_stdout_only() -> Result<(), Box> { assert!(version.stderr.is_empty()); Ok(()) } + +#[test] +fn format_replaces_only_changed_files_and_is_idempotent() -> Result<(), Box> { + let directory = TestDirectory::new("format-in-place")?; + let source = b"// kept\nstack 1 . 0 diagram \"Valid\"{node api \"API\"}"; + let expected = b"// kept\nstack 1.0\n\ndiagram \"Valid\" {\n node api \"API\"\n}\n"; + let path = directory.file("format.stack", source)?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + fs::set_permissions(&path, fs::Permissions::from_mode(0o640))?; + } + + let first = stack([OsStr::new("fmt"), path.as_os_str()])?; + assert_eq!(first.status.code(), Some(0)); + assert!(first.stdout.is_empty()); + assert!(first.stderr.is_empty()); + assert_eq!(fs::read(&path)?, expected); + #[cfg(unix)] + { + use std::os::unix::fs::{MetadataExt, PermissionsExt}; + let metadata = fs::metadata(&path)?; + assert_eq!(metadata.permissions().mode() & 0o777, 0o640); + assert_ne!(metadata.ino(), 0); + } + + #[cfg(unix)] + let formatted_inode = { + use std::os::unix::fs::MetadataExt; + fs::metadata(&path)?.ino() + }; + + let second = stack([OsStr::new("fmt"), path.as_os_str()])?; + assert_eq!(second.status.code(), Some(0)); + assert!(second.stdout.is_empty()); + assert!(second.stderr.is_empty()); + assert_eq!(fs::read(&path)?, expected); + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + assert_eq!(fs::metadata(&path)?.ino(), formatted_inode); + } + assert_eq!(fs::read_dir(&directory.path)?.count(), 1); + Ok(()) +} + +#[test] +fn format_check_reports_differences_without_writing() -> Result<(), Box> { + let directory = TestDirectory::new("format-check")?; + let unformatted = b"stack 1.0 diagram \"Check\"{node api \"API\"}"; + let formatted = b"stack 1.0\n\ndiagram \"Check\" {\n node api \"API\"\n}\n"; + let path = directory.file("check.stack", unformatted)?; + + let different = stack([OsStr::new("fmt"), OsStr::new("--check"), path.as_os_str()])?; + assert_eq!(different.status.code(), Some(1)); + assert!(different.stdout.is_empty()); + assert!(different.stderr.is_empty()); + assert_unchanged(&path, unformatted)?; + + fs::write(&path, formatted)?; + let clean = stack([OsStr::new("fmt"), OsStr::new("--check"), path.as_os_str()])?; + assert_eq!(clean.status.code(), Some(0)); + assert!(clean.stdout.is_empty()); + assert!(clean.stderr.is_empty()); + assert_unchanged(&path, formatted) +} + +#[test] +fn semantic_errors_are_formatted_but_exit_one() -> Result<(), Box> { + let directory = TestDirectory::new("format-semantic-error")?; + let source = b"// preserved\nstack 1.0 diagram \"Invalid\"{node api \"First\" node api \"Second\" edge api->missing}"; + let path = directory.file("semantic.stack", source)?; + + let checked = stack([OsStr::new("fmt"), OsStr::new("--check"), path.as_os_str()])?; + assert_eq!(checked.status.code(), Some(1)); + assert!(checked.stdout.is_empty()); + assert!(String::from_utf8(checked.stderr)?.contains("error[STK3002]")); + assert_unchanged(&path, source)?; + + let output = stack([OsStr::new("fmt"), path.as_os_str()])?; + let formatted = fs::read_to_string(&path)?; + + assert_eq!(output.status.code(), Some(1)); + assert!(output.stdout.is_empty()); + assert!(String::from_utf8(output.stderr)?.contains("error[STK3002]")); + assert!(formatted.starts_with("// preserved\nstack 1.0\n")); + assert!(formatted.contains("node api \"First\"\n")); + assert!(formatted.contains("edge api -> missing\n")); + assert_ne!(formatted.as_bytes(), source); + Ok(()) +} + +#[test] +fn syntax_errors_never_modify_files() -> Result<(), Box> { + let directory = TestDirectory::new("format-syntax-error")?; + let source = b"stack 1.0 diagram \"Incomplete\" {"; + let path = directory.file("syntax.stack", source)?; + + for arguments in [ + vec![OsStr::new("fmt"), path.as_os_str()], + vec![OsStr::new("fmt"), OsStr::new("--check"), path.as_os_str()], + ] { + let output = stack(arguments)?; + assert_eq!(output.status.code(), Some(1)); + assert!(output.stdout.is_empty()); + assert!(String::from_utf8(output.stderr)?.contains("error[STK2003]")); + assert_unchanged(&path, source)?; + } + assert_eq!(fs::read_dir(&directory.path)?.count(), 1); + Ok(()) +} + +#[test] +fn stdin_formatting_has_explicit_stdout_and_check_semantics() -> Result<(), Box> { + let source = b"stack 1.0 diagram \"Stdin\"{node api \"API\"}"; + let expected = b"stack 1.0\n\ndiagram \"Stdin\" {\n node api \"API\"\n}\n"; + + let formatted = stack_with_input(["fmt", "-"], source)?; + assert_eq!(formatted.status.code(), Some(0)); + assert_eq!(formatted.stdout, expected); + assert!(formatted.stderr.is_empty()); + + let check_clean = stack_with_input(["fmt", "--check", "-"], expected)?; + assert_eq!(check_clean.status.code(), Some(0)); + assert!(check_clean.stdout.is_empty()); + assert!(check_clean.stderr.is_empty()); + + let check_different = stack_with_input(["fmt", "--check", "-"], source)?; + assert_eq!(check_different.status.code(), Some(1)); + assert!(check_different.stdout.is_empty()); + assert!(check_different.stderr.is_empty()); + Ok(()) +} + +#[test] +fn format_missing_file_is_a_host_failure() -> Result<(), Box> { + let directory = TestDirectory::new("format-missing")?; + let path = directory.path.join("missing.stack"); + + let output = stack([OsStr::new("fmt"), path.as_os_str()])?; + assert_eq!(output.status.code(), Some(2)); + assert!(output.stdout.is_empty()); + assert_eq!( + String::from_utf8(output.stderr)?, + format!("error: cannot read '{}': file not found\n", path.display()) + ); + assert!(!path.exists()); + Ok(()) +} + +#[cfg(unix)] +#[test] +fn atomic_io_failure_keeps_the_original_file() -> Result<(), Box> { + use std::os::unix::fs::PermissionsExt; + + let directory = TestDirectory::new("format-atomic-failure")?; + let source = b"stack 1.0 diagram \"Valid\"{node api \"API\"}"; + let path = directory.file("readonly.stack", source)?; + let original_permissions = fs::metadata(&directory.path)?.permissions(); + fs::set_permissions(&directory.path, fs::Permissions::from_mode(0o555))?; + + let output = stack([OsStr::new("fmt"), path.as_os_str()]); + let restored = fs::set_permissions(&directory.path, original_permissions); + restored?; + let output = output?; + + assert_eq!(output.status.code(), Some(2)); + assert!(output.stdout.is_empty()); + assert!(String::from_utf8(output.stderr)?.contains("cannot replace")); + assert_unchanged(&path, source)?; + assert_eq!(fs::read_dir(&directory.path)?.count(), 1); + Ok(()) +} diff --git a/tests/formatter_conformance.rs b/tests/formatter_conformance.rs new file mode 100644 index 0000000..c04bd24 --- /dev/null +++ b/tests/formatter_conformance.rs @@ -0,0 +1,76 @@ +use std::env; +use std::error::Error; +use std::fs; +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::process::{Command, Output, Stdio}; + +#[test] +fn canonical_formatter_fixtures_match_cli_output() -> Result<(), Box> { + let root = specification_root()?.join("conformance/formatter"); + let mut cases = fs::read_dir(&root)?.collect::, _>>()?; + cases.sort_by_key(|entry| entry.file_name()); + if cases.is_empty() { + return Err(format!("no formatter cases found in {}", root.display()).into()); + } + + for entry in cases { + let case = entry.path(); + if !case.is_dir() { + continue; + } + let input = fs::read(case.join("input.stack"))?; + let expected = fs::read(case.join("expected.stack"))?; + + let formatted = stack_with_input(["fmt", "-"], &input)?; + if formatted.status.code() != Some(0) + || formatted.stdout != expected + || !formatted.stderr.is_empty() + { + return Err(format!("{} did not match canonical output", case.display()).into()); + } + + let clean = stack_with_input(["fmt", "--check", "-"], &expected)?; + if clean.status.code() != Some(0) || !clean.stdout.is_empty() || !clean.stderr.is_empty() { + return Err(format!("{} expected output was not clean", case.display()).into()); + } + + let input_check = stack_with_input(["fmt", "--check", "-"], &input)?; + let expected_status = if input == expected { Some(0) } else { Some(1) }; + if input_check.status.code() != expected_status + || !input_check.stdout.is_empty() + || !input_check.stderr.is_empty() + { + return Err(format!("{} input check result was incorrect", case.display()).into()); + } + } + Ok(()) +} + +fn stack_with_input( + arguments: impl IntoIterator>, + input: &[u8], +) -> Result> { + let mut child = Command::new(env!("CARGO_BIN_EXE_stack")) + .args(arguments) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + let Some(mut stdin) = child.stdin.take() else { + return Err("missing child standard input".into()); + }; + stdin.write_all(input)?; + drop(stdin); + Ok(child.wait_with_output()?) +} + +fn specification_root() -> Result> { + let configured = env::var_os("STACK_SPECIFICATION_DIR") + .ok_or("STACK_SPECIFICATION_DIR must point to stack-sh/specification")?; + let root = Path::new(&configured); + if !root.is_dir() { + return Err(format!("{} is not a directory", root.display()).into()); + } + Ok(root.to_path_buf()) +} diff --git a/tests/specification-revision b/tests/specification-revision new file mode 100644 index 0000000..6da9620 --- /dev/null +++ b/tests/specification-revision @@ -0,0 +1 @@ +f382069928c805fe69b7a192bfd6a877036bc036