diff --git a/Cargo.lock b/Cargo.lock index bd7f99a8..0ad2229a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -130,6 +130,15 @@ dependencies = [ "windows-targets 0.52.6", ] +[[package]] +name = "backtrace-ext" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "537beee3be4a18fb023b570f80e3ae28003db9167a751266b259926e25539d50" +dependencies = [ + "backtrace", +] + [[package]] name = "bitflags" version = "1.3.2" @@ -230,7 +239,7 @@ dependencies = [ "encode_unicode", "libc", "once_cell", - "unicode-width", + "unicode-width 0.2.1", "windows-sys 0.59.0", ] @@ -690,6 +699,12 @@ dependencies = [ "libc", ] +[[package]] +name = "is_ci" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7655c9839580ee829dfacba1d1278c2b7883e50a277ff7541299489d6bdfdc45" + [[package]] name = "is_terminal_polyfill" version = "1.70.1" @@ -799,6 +814,36 @@ version = "2.7.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32a282da65faaf38286cf3be983213fcf1d2e2a58700e808f83f4ea9a4804bc0" +[[package]] +name = "miette" +version = "7.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5f98efec8807c63c752b5bd61f862c165c115b0a35685bdcfd9238c7aeb592b7" +dependencies = [ + "backtrace", + "backtrace-ext", + "cfg-if", + "miette-derive", + "owo-colors", + "supports-color", + "supports-hyperlinks", + "supports-unicode", + "terminal_size", + "textwrap", + "unicode-width 0.1.14", +] + +[[package]] +name = "miette-derive" +version = "7.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "db5b29714e950dbb20d5e6f74f9dcec4edbcc1067bb7f8ed198c097b8c1a818b" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.104", +] + [[package]] name = "minijinja" version = "2.11.0" @@ -882,6 +927,7 @@ dependencies = [ "insta", "itertools 0.12.1", "itoa", + "miette", "minijinja", "mockable", "mockall", @@ -894,6 +940,7 @@ dependencies = [ "serde_yml", "serial_test", "sha2", + "strip-ansi-escapes", "tempfile", "test_support", "thiserror", @@ -979,6 +1026,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" +[[package]] +name = "owo-colors" +version = "4.2.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "48dd4f4a2c8405440fd0462561f0e5806bd0f77e86f51c761481bdd4018b545e" + [[package]] name = "parking_lot" version = "0.12.4" @@ -1440,12 +1493,42 @@ version = "0.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" +[[package]] +name = "strip-ansi-escapes" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2a8f8038e7e7969abb3f1b7c2a811225e9296da208539e0f79c5251d6cac0025" +dependencies = [ + "vte", +] + [[package]] name = "strsim" version = "0.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" +[[package]] +name = "supports-color" +version = "3.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c64fc7232dd8d2e4ac5ce4ef302b1d81e0b80d055b9d77c7c4f51f6aa4c867d6" +dependencies = [ + "is_ci", +] + +[[package]] +name = "supports-hyperlinks" +version = "3.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "804f44ed3c63152de6a9f90acbea1a110441de43006ea51bcce8f436196a288b" + +[[package]] +name = "supports-unicode" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7401a30af6cb5818bb64852270bb722533397edcfc7344954a38f420819ece2" + [[package]] name = "syn" version = "1.0.109" @@ -1547,7 +1630,7 @@ checksum = "c13547615a44dc9c452a8a534638acdf07120d4b6847c8178705da06306a3057" dependencies = [ "smawk", "unicode-linebreak", - "unicode-width", + "unicode-width 0.2.1", ] [[package]] @@ -1700,6 +1783,12 @@ version = "0.1.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" +[[package]] +name = "unicode-width" +version = "0.1.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7dd6e30e90baa6f72411720665d41d89b9a3d039dc45b8faea1ddd07f617f6af" + [[package]] name = "unicode-width" version = "0.2.1" @@ -1724,6 +1813,15 @@ version = "0.9.5" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "0b928f33d975fc6ad9f86c8f283853ad26bdd5b10b7f1542aa2fa15e2289105a" +[[package]] +name = "vte" +version = "0.14.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "231fdcd7ef3037e8330d8e17e61011a2c244126acc0a982f4040ac3f9f0bc077" +dependencies = [ + "memchr", +] + [[package]] name = "wait-timeout" version = "0.2.1" diff --git a/Cargo.toml b/Cargo.toml index 0a4bd03b..7a929fbe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,6 +11,7 @@ minijinja = "2.11.0" semver = { version = "1", features = ["serde"] } anyhow = "1" thiserror = "1" +miette = { version = "7.6.0", features = ["fancy"] } sha2 = "0.10" itoa = "1" itertools = "0.12" @@ -71,6 +72,7 @@ mockable = { version = "0.3", features = ["mock"] } serial_test = "3" mockall = "0.11" test_support = { path = "test_support" } +strip-ansi-escapes = "0.2" [[test]] name = "cucumber" diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 40421add..bb17d416 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -1269,12 +1269,13 @@ three fundamental questions: Found a tab character, which is not allowed. Hint: Use spaces for indentation instead."). -### 7.2 Crate Selection and Strategy: `anyhow` and `thiserror` +### 7.2 Crate Selection and Strategy: `anyhow`, `thiserror`, and `miette` -To implement this philosophy, Netsuke will adopt a hybrid error handling -strategy using the `anyhow` and `thiserror` crates. This is a common and highly -effective pattern in the Rust ecosystem for creating robust applications and -libraries.[^27] +To implement this philosophy, Netsuke adopts a hybrid error handling strategy +using the `anyhow`, `thiserror`, and `miette` crates. This is a common and +highly effective pattern in the Rust ecosystem for creating robust applications +and libraries.[^27] `miette` renders user-facing diagnostics, computing spans +directly from parser locations. - `thiserror`: This crate will be used *within* Netsuke's internal library modules (e.g., `parser`, `ir`, `ninja_gen`) to define specific, structured @@ -1316,6 +1317,9 @@ pub enum IrGenError { `.with_context()` methods for adding high-level, human-readable context to errors as they bubble up the call stack.[^31] +- `miette`: Presents human-friendly diagnostics, highlighting exact error + locations with computed spans. + ### 7.3 Error Handling Flow The flow of an error from its origin to the user follows a clear path of @@ -1563,15 +1567,15 @@ goal. This table serves as a quick-reference guide to the core third-party crates selected for this project and the rationale for their inclusion. -| Component | Recommended Crate | Rationale | -| -------------- | ------------------ | ----------------------------------------------------------------------------------------------------------------------- | -| CLI Parsing | clap | The Rust standard for powerful, derive-based CLI development. | -| YAML Parsing | serde_yml | Mature, stable, and provides seamless integration with the serde framework. | -| Templating | minijinja | High compatibility with Jinja2, minimal dependencies, and supports runtime template loading. | -| Shell Quoting | shell-quote | A critical security component; provides robust, shell-specific escaping for command arguments. | -| Error Handling | anyhow + thiserror | An idiomatic and powerful combination for creating rich, contextual, and user-friendly error reports. | -| Logging | tracing | Structured, levelled diagnostic output for debugging and insight. | -| Versioning | semver | The standard library for parsing and evaluating Semantic Versioning strings, essential for the `netsuke_version` field. | +| Component | Recommended Crate | Rationale | +| -------------- | --------------------------- | ------------------------------------------------------------------------------------------------------------------------------- | +| CLI Parsing | clap | The Rust standard for powerful, derive-based CLI development. | +| YAML Parsing | serde_yml | Mature, stable, and provides seamless integration with the serde framework. | +| Templating | minijinja | High compatibility with Jinja2, minimal dependencies, and supports runtime template loading. | +| Shell Quoting | shell-quote | A critical security component; provides robust, shell-specific escaping for command arguments. | +| Error Handling | anyhow + thiserror + miette | An idiomatic and powerful combination for creating rich, contextual, and user-friendly error reports with precise source spans. | +| Logging | tracing | Structured, levelled diagnostic output for debugging and insight. | +| Versioning | semver | The standard library for parsing and evaluating Semantic Versioning strings, essential for the `netsuke_version` field. | ### 9.3 Future Enhancements diff --git a/src/manifest.rs b/src/manifest.rs index 7f2bfd00..30438522 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -5,14 +5,103 @@ //! evaluated only within string values or the `foreach` and `when` keys. use crate::ast::{NetsukeManifest, Recipe, StringOrList, Target, Vars}; -use anyhow::{Context, Result, anyhow}; +use miette::{Context, Diagnostic, IntoDiagnostic, NamedSource, Report, Result, SourceSpan}; use minijinja::{Environment, UndefinedBehavior, context, value::Value}; +use serde_yml::{Error as YamlError, Location}; use serde_yml::{Mapping as YamlMapping, Value as YamlValue}; use std::{fs, path::Path}; +use thiserror::Error; -const ERR_INITIAL_YAML_PARSE: &str = "initial YAML parse error"; const ERR_MANIFEST_PARSE: &str = "manifest parse error"; +// Compute a narrow highlight span from a location. +fn to_span(src: &str, loc: Location) -> SourceSpan { + let at = loc.index(); + let bytes = src.as_bytes(); + let (start, end) = match bytes.get(at) { + Some(&b) if b != b'\n' => (at, at + 1), + _ => { + // Fallback: highlight the previous byte on the same line when possible. + let start = if at > 0 && bytes.get(at - 1).is_some_and(|p| *p != b'\n') { + at - 1 + } else { + at + }; + (start, at) + } + }; + let len = end.saturating_sub(start); + #[allow(clippy::useless_conversion, reason = "future-proof span length type")] + SourceSpan::new(start.into(), len.into()) +} + +#[derive(Debug, Error, Diagnostic)] +#[error("{message}")] +#[diagnostic(code(netsuke::yaml::parse))] +pub(crate) struct YamlDiagnostic { + #[source_code] + src: NamedSource, + #[label("parse error here")] + span: Option, + #[help] + help: Option, + #[source] + source: YamlError, + message: String, +} + +fn hint_for(err_str: &str, src: &str, loc: Option) -> Option { + let lower = err_str.to_lowercase(); + if let Some(loc) = loc { + let idx = loc.index(); + let bytes = src.as_bytes(); + let line_start = bytes + .get(..idx) + .and_then(|b| b.iter().rposition(|b| *b == b'\n').map(|p| p + 1)) + .unwrap_or(0); + let line_end = bytes + .get(idx..) + .and_then(|b| b.iter().position(|b| *b == b'\n').map(|p| idx + p)) + .unwrap_or(bytes.len()); + if bytes + .get(line_start..line_end) + .unwrap_or(&[]) + .iter() + .take_while(|b| **b == b' ' || **b == b'\t') + .any(|b| *b == b'\t') + { + return Some("Use spaces for indentation; tabs are invalid in YAML.".into()); + } + } + if lower.contains("did not find expected '-'") { + Some("Start list items with '-' and ensure proper indentation.".into()) + } else if lower.contains("expected ':'") { + Some("Ensure each key is followed by ':' separating key and value.".into()) + } else { + None + } +} + +fn map_yaml_error(err: YamlError, src: &str, name: &str) -> Report { + let loc = err.location(); + let (line, col, span) = loc.map_or((1, 1, None), |l| { + (l.line(), l.column(), Some(to_span(src, l))) + }); + let err_str = err.to_string(); + let hint = hint_for(&err_str, src, loc); + let message = format!("YAML parse error at line {line}, column {col}: {err_str}"); + + let diag = YamlDiagnostic { + src: NamedSource::new(name, src.to_string()), + span, + help: hint, + source: err, + message, + }; + + Report::new(diag) +} + /// Parse a manifest string using Jinja for value templating. /// /// The input YAML must be valid on its own. Jinja expressions are evaluated @@ -21,8 +110,9 @@ const ERR_MANIFEST_PARSE: &str = "manifest parse error"; /// # Errors /// /// Returns an error if YAML parsing or Jinja evaluation fails. -pub fn from_str(yaml: &str) -> Result { - let mut doc: YamlValue = serde_yml::from_str(yaml).context(ERR_INITIAL_YAML_PARSE)?; +fn from_str_named(yaml: &str, name: &str) -> Result { + let mut doc: YamlValue = + serde_yml::from_str(yaml).map_err(|e| map_yaml_error(e, yaml, name))?; let mut env = Environment::new(); env.set_undefined_behavior(UndefinedBehavior::Strict); @@ -31,7 +121,7 @@ pub fn from_str(yaml: &str) -> Result { for (k, v) in vars { let key = k .as_str() - .ok_or_else(|| anyhow!("non-string key in 'vars' mapping: {k:?}"))? + .ok_or_else(|| Report::msg(format!("non-string key in 'vars' mapping: {k:?}")))? .to_string(); env.add_global(key, Value::from_serialize(v)); } @@ -39,11 +129,25 @@ pub fn from_str(yaml: &str) -> Result { expand_foreach(&mut doc, &env)?; - let manifest: NetsukeManifest = serde_yml::from_value(doc).context(ERR_MANIFEST_PARSE)?; + let manifest: NetsukeManifest = serde_yml::from_value(doc) + .into_diagnostic() + .wrap_err(ERR_MANIFEST_PARSE)?; render_manifest(manifest, &env) } +/// Parse a manifest string using Jinja for value templating. +/// +/// The input YAML must be valid on its own. Jinja expressions are evaluated +/// only inside recognised string fields and the `foreach` and `when` keys. +/// +/// # Errors +/// +/// Returns an error if YAML parsing or Jinja evaluation fails. +pub fn from_str(yaml: &str) -> Result { + from_str_named(yaml, "Netsukefile") +} + /// Expand `foreach` entries within the raw YAML document. fn expand_foreach(doc: &mut YamlValue, env: &Environment) -> Result<()> { let Some(targets) = doc.get_mut("targets").and_then(|v| v.as_sequence_mut()) else { @@ -90,7 +194,8 @@ fn parse_foreach_values(expr_val: &YamlValue, env: &Environment) -> Result R None => YamlMapping::new(), Some(YamlValue::Mapping(m)) => m, Some(other) => { - return Err(anyhow!("target.vars must be a mapping, got: {other:?}")); + return Err(Report::msg(format!( + "target.vars must be a mapping, got: {other:?}" + ))); } }; vars.insert( YamlValue::String("item".into()), - serde_yml::to_value(item).context("serialise item")?, + serde_yml::to_value(item) + .into_diagnostic() + .wrap_err("serialise item")?, ); vars.insert( YamlValue::String("index".into()), @@ -134,14 +243,28 @@ fn inject_iteration_vars(map: &mut YamlMapping, item: &Value, index: usize) -> R fn as_str<'a>(value: &'a YamlValue, field: &str) -> Result<&'a str> { value .as_str() - .with_context(|| format!("{field} must be a string expression")) + .ok_or_else(|| Report::msg(format!("{field} must be a string expression"))) } fn eval_expression(env: &Environment, name: &str, expr: &str, ctx: Value) -> Result { env.compile_expression(expr) - .with_context(|| format!("{name} expression parse error"))? + .into_diagnostic() + .wrap_err_with(|| format!("{name} expression parse error"))? .eval(ctx) - .with_context(|| format!("{name} evaluation error")) + .into_diagnostic() + .wrap_err_with(|| format!("{name} evaluation error")) +} + +/// Render a Jinja template and label any error with the given context. +fn render_str_with( + env: &Environment, + tpl: &str, + ctx: &impl serde::Serialize, + what: impl FnOnce() -> String, +) -> Result { + env.render_str(tpl, ctx) + .into_diagnostic() + .wrap_err_with(what) } /// Render all templated strings in the manifest. @@ -160,21 +283,16 @@ fn render_manifest(mut manifest: NetsukeManifest, env: &Environment) -> Result Result<()> { if let Some(desc) = &mut rule.description { - *desc = env - .render_str(desc, context! {}) - .context("render rule description")?; + *desc = render_str_with(env, desc, &context! {}, || "render rule description".into())?; } render_string_or_list(&mut rule.deps, env, &Vars::new())?; match &mut rule.recipe { Recipe::Command { command } => { - *command = env - .render_str(command, context! {}) - .context("render rule command")?; + *command = + render_str_with(env, command, &context! {}, || "render rule command".into())?; } Recipe::Script { script } => { - *script = env - .render_str(script, context! {}) - .context("render rule script")?; + *script = render_str_with(env, script, &context! {}, || "render rule script".into())?; } Recipe::Rule { rule: r } => render_string_or_list(r, env, &Vars::new())?, } @@ -189,14 +307,12 @@ fn render_target(target: &mut Target, env: &Environment) -> Result<()> { render_string_or_list(&mut target.order_only_deps, env, &target.vars)?; match &mut target.recipe { Recipe::Command { command } => { - *command = env - .render_str(command, &target.vars) - .context("render target command")?; + *command = render_str_with(env, command, &target.vars, || { + "render target command".into() + })?; } Recipe::Script { script } => { - *script = env - .render_str(script, &target.vars) - .context("render target script")?; + *script = render_str_with(env, script, &target.vars, || "render target script".into())?; } Recipe::Rule { rule } => render_string_or_list(rule, env, &target.vars)?, } @@ -207,9 +323,7 @@ fn render_vars(vars: &mut Vars, env: &Environment) -> Result<()> { let snapshot = vars.clone(); for (key, value) in vars.iter_mut() { if let YamlValue::String(s) = value { - *s = env - .render_str(s, &snapshot) - .with_context(|| format!("render var '{key}'"))?; + *s = render_str_with(env, s, &snapshot, || format!("render var '{key}'"))?; } } Ok(()) @@ -218,11 +332,11 @@ fn render_vars(vars: &mut Vars, env: &Environment) -> Result<()> { fn render_string_or_list(value: &mut StringOrList, env: &Environment, ctx: &Vars) -> Result<()> { match value { StringOrList::String(s) => { - *s = env.render_str(s, ctx).context("render string value")?; + *s = render_str_with(env, s, ctx, || "render string value".into())?; } StringOrList::List(list) => { for item in list { - *item = env.render_str(item, ctx).context("render list value")?; + *item = render_str_with(env, item, ctx, || "render list value".into())?; } } StringOrList::Empty => {} @@ -238,6 +352,7 @@ fn render_string_or_list(value: &mut StringOrList, env: &Environment, ctx: &Vars pub fn from_path(path: impl AsRef) -> Result { let path_ref = path.as_ref(); let data = fs::read_to_string(path_ref) - .with_context(|| format!("Failed to read {}", path_ref.display()))?; - from_str(&data) + .into_diagnostic() + .wrap_err_with(|| format!("Failed to read {}", path_ref.display()))?; + from_str_named(&data, &path_ref.display().to_string()) } diff --git a/src/runner.rs b/src/runner.rs index 3fb30948..ffc43a5b 100644 --- a/src/runner.rs +++ b/src/runner.rs @@ -6,7 +6,7 @@ use crate::cli::{BuildArgs, Cli, Commands}; use crate::{ir::BuildGraph, manifest, ninja_gen}; -use anyhow::{Context, Result}; +use miette::{Context, IntoDiagnostic, Result}; use serde_json; use std::borrow::Cow; use std::fs; @@ -174,7 +174,9 @@ fn handle_build(cli: &Cli, args: &BuildArgs) -> Result<()> { } let program = resolve_ninja_program(); - run_ninja(program.as_path(), cli, build_path.as_ref(), &targets)?; + run_ninja(program.as_path(), cli, build_path.as_ref(), &targets) + .into_diagnostic() + .wrap_err("run ninja")?; drop(tmp_file); Ok(()) } @@ -196,7 +198,8 @@ fn create_temp_ninja_file(content: &NinjaContent) -> Result { .prefix("netsuke.") .suffix(".ninja") .tempfile() - .context("create temp file")?; + .into_diagnostic() + .wrap_err("create temp file")?; write_ninja_file(tmp.path(), content)?; Ok(tmp) } @@ -217,10 +220,12 @@ fn write_ninja_file(path: &Path, content: &NinjaContent) -> Result<()> { // do not attempt to create the current directory on some platforms. if let Some(parent) = path.parent().filter(|p| !p.as_os_str().is_empty()) { fs::create_dir_all(parent) - .with_context(|| format!("failed to create parent directory {}", parent.display()))?; + .into_diagnostic() + .wrap_err_with(|| format!("failed to create parent directory {}", parent.display()))?; } fs::write(path, content.as_str()) - .with_context(|| format!("failed to write Ninja file to {}", path.display()))?; + .into_diagnostic() + .wrap_err_with(|| format!("failed to write Ninja file to {}", path.display()))?; info!("Generated Ninja file at {}", path.display()); Ok(()) } @@ -249,9 +254,13 @@ fn generate_ninja(cli: &Cli) -> Result { let manifest_path = resolve_manifest_path(cli); let manifest = manifest::from_path(&manifest_path) .with_context(|| format!("loading manifest at {}", manifest_path.display()))?; - let ast_json = serde_json::to_string_pretty(&manifest).context("serialising manifest")?; + let ast_json = serde_json::to_string_pretty(&manifest) + .into_diagnostic() + .wrap_err("serialising manifest")?; debug!("AST:\n{ast_json}"); - let graph = BuildGraph::from_manifest(&manifest).context("building graph")?; + let graph = BuildGraph::from_manifest(&manifest) + .into_diagnostic() + .wrap_err("building graph")?; Ok(NinjaContent::new(ninja_gen::generate(&graph))) } diff --git a/tests/ast_tests.rs b/tests/ast_tests.rs index a5818d0a..132c20ac 100644 --- a/tests/ast_tests.rs +++ b/tests/ast_tests.rs @@ -1,11 +1,12 @@ //! Unit tests for Netsuke manifest AST deserialisation. +use miette::Result; use netsuke::{ast::*, manifest}; use rstest::rstest; use semver::Version; /// Convenience wrapper around the library manifest parser for tests. -fn parse_manifest(yaml: &str) -> anyhow::Result { +fn parse_manifest(yaml: &str) -> Result { manifest::from_str(yaml) } diff --git a/tests/runner_tests.rs b/tests/runner_tests.rs index 0a6e6d7f..347e8ab7 100644 --- a/tests/runner_tests.rs +++ b/tests/runner_tests.rs @@ -67,10 +67,13 @@ fn run_exits_with_manifest_error_on_invalid_version() { })), }; - let result = run(&cli); - assert!(result.is_err()); - let err = result.expect_err("should have error"); - assert!(err.chain().any(|e| e.to_string().contains("version"))); + let err = run(&cli).expect_err("should have error"); + assert!(err.to_string().contains("loading manifest at")); + let chain: Vec = err.chain().map(ToString::to_string).collect(); + assert!( + chain.iter().any(|s| s.contains("manifest parse error")), + "expected error chain to include 'manifest parse error', got: {chain:?}" + ); } #[rstest] diff --git a/tests/steps/ir_steps.rs b/tests/steps/ir_steps.rs index a74d3ce9..ceac21b9 100644 --- a/tests/steps/ir_steps.rs +++ b/tests/steps/ir_steps.rs @@ -2,6 +2,7 @@ use crate::CliWorld; use cucumber::{given, then, when}; +use miette::{Context, IntoDiagnostic}; use netsuke::ir::BuildGraph; fn assert_graph(world: &CliWorld) { @@ -50,7 +51,12 @@ fn graph_defaults(world: &mut CliWorld, count: usize) { #[when(expr = "the manifest file {string} is compiled to IR")] fn compile_manifest(world: &mut CliWorld, path: String) { match netsuke::manifest::from_path(&path) - .and_then(|m| BuildGraph::from_manifest(&m).map_err(anyhow::Error::from)) + .and_then(|m| { + BuildGraph::from_manifest(&m) + .into_diagnostic() + .wrap_err("building IR from manifest") + }) + .with_context(|| format!("IR generation failed for {path}")) { Ok(graph) => { world.build_graph = Some(graph); diff --git a/tests/yaml_error_tests.rs b/tests/yaml_error_tests.rs new file mode 100644 index 00000000..b1ca755a --- /dev/null +++ b/tests/yaml_error_tests.rs @@ -0,0 +1,48 @@ +//! Regression tests for YAML parse errors. +//! +//! These tests ensure diagnostics include line numbers and optional hints, and +//! that rendering is stable across terminals. + +use miette::GraphicalReportHandler; +use netsuke::manifest; +use rstest::rstest; +use strip_ansi_escapes::strip; + +fn normalise_report(report: &str) -> String { + String::from_utf8(strip(report.as_bytes())).expect("utf8") +} + +#[rstest] +#[case( + "targets:\n\t- name: test\n", + &["line 2, column 1", "Use spaces for indentation"], +)] +#[case( + "targets:\n - name: hi\n command echo\n", + &["line 3", "expected ':'", "Ensure each key is followed by ':'"], +)] +#[case( + concat!( + "targets:\n", + " - name: ok\n", + " command: echo\n", + " name: missing\n", + " command: echo\n", + ), + &["line 4", "did not find expected '-'", "Start list items with '-'"], +)] +#[case( + "targets:\n - name: 'unterminated\n", + &["YAML parse error", "line 2"], +)] +fn yaml_diagnostics_are_actionable(#[case] yaml: &str, #[case] needles: &[&str]) { + let err = manifest::from_str(yaml).expect_err("parse should fail"); + let mut msg = String::new(); + GraphicalReportHandler::new() + .render_report(&mut msg, err.as_ref()) + .expect("render yaml error"); + let msg = normalise_report(&msg); + for needle in needles { + assert!(msg.contains(needle), "missing: {needle}\nmessage: {msg}"); + } +}