diff --git a/Cargo.lock b/Cargo.lock index a8c37e89..49965b16 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1141,9 +1141,9 @@ dependencies = [ [[package]] name = "minijinja" -version = "2.11.0" +version = "2.12.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4e60ac08614cc09062820e51d5d94c2fce16b94ea4e5003bb81b99a95f84e876" +checksum = "a9f264d75233323f4b7d2f03aefe8a990690cdebfbfe26ea86bcbaec5e9ac990" dependencies = [ "memo-map", "self_cell", diff --git a/Cargo.toml b/Cargo.toml index 06a37b1c..174740e1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ legacy-digests = ["sha1", "md5"] clap = { version = "4.5.0", features = ["derive"] } serde = { version = "1", features = ["derive"] } serde_yml = "0.0.12" -minijinja = { version = "2.11.0", features = ["loader"] } +minijinja = { version = "2.12.0", features = ["loader"] } cap-std = { version = "3.4.4", features = ["fs_utf8"] } camino = "1.2.0" semver = { version = "1", features = ["serde"] } diff --git a/docs/netsuke-design.md b/docs/netsuke-design.md index 96b657f2..0b81be6f 100644 --- a/docs/netsuke-design.md +++ b/docs/netsuke-design.md @@ -417,6 +417,18 @@ unnecessary project risk. `serde_yml` is mature, widely adopted, and battle-tested, making it the prudent choice for production-quality software. +**Maintenance risk.** `serde_yml` is archived upstream and carries unsoundness +advisories. Netsuke relies on it today, but we will investigate a maintained +successor such as `serde_yaml_ng`. A follow-up ADR will outline the migration +plan and compatibility testing. + +Follow-up actions: + +- Draft an ADR comparing `serde_yml` with candidate replacements and recording + the migration decision. +- Schedule a migration spike to prototype deserialisation with the preferred + crate and capture compatibility notes. + ### 3.2 Core Data Structures (`ast.rs`) The Rust structs that `serde_yml` will deserialise into form the Abstract @@ -539,6 +551,7 @@ use netsuke::ast::*; let ast = NetsukeManifest { netsuke_version: Version::parse("1.0.0").unwrap(), vars: HashMap::new(), + macros: vec![], rules: vec![], actions: vec![], targets: vec![Target { @@ -720,6 +733,13 @@ If a macro name matches a built-in function or filter, the macro overrides the built-in definition. This mirrors Jinja's behaviour and follows `minijinja` semantics where later definitions shadow earlier ones. +The manifest loader compiles each macro definition into an internal template +and registers a wrapper function that evaluates the macro on demand. The +wrapper constructs a fresh MiniJinja state for every invocation so macro calls +do not depend on the lifetime of the manifest parsing state. This preserves +MiniJinja's argument handling, including keyword parameters and `caller` +support, while allowing later macros to override earlier ones. + ### 4.4 Essential Custom Functions To transform `minijinja` from a general-purpose templating engine into a diff --git a/docs/roadmap.md b/docs/roadmap.md index ba6fb067..9efa60c4 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -107,7 +107,7 @@ configurations with variables, control flow, and custom functions. - [x] Implement the critical glob(pattern) custom function to perform file path globbing, with results sorted lexicographically. - - [ ] Support user-defined Jinja macros declared in a top-level macros list, + - [x] Support user-defined Jinja macros declared in a top-level macros list, registering them with the environment before rendering. - **Success Criterion:** @@ -117,6 +117,13 @@ configurations with variables, control flow, and custom functions. custom macros, and the `glob()` function to discover and operate on source files. +- [ ] **YAML Parser Migration:** + + - [ ] Draft an ADR evaluating maintained replacements for `serde_yml` + (for example `serde_yaml_ng`) and record the migration decision. + - [ ] Run a migration spike with the preferred crate, exercising the manifest + fixtures to capture compatibility notes and required mitigations. + ## Phase 3: The "Friendly" Polish 🛡️ Objective: To implement the advanced features that deliver a superior, secure, diff --git a/src/ast.rs b/src/ast.rs index 816dd5d9..431fda2d 100644 --- a/src/ast.rs +++ b/src/ast.rs @@ -56,6 +56,15 @@ where /// assert_eq!(manifest.targets.len(), 1); /// # Ok(()) } /// ``` +#[derive(Debug, Clone, Deserialize, Serialize)] +#[serde(deny_unknown_fields)] +pub struct MacroDefinition { + /// Full macro signature as accepted by `MiniJinja`. + pub signature: String, + /// Body of the macro written using YAML block style. + pub body: String, +} + #[derive(Debug, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct NetsukeManifest { @@ -66,6 +75,10 @@ pub struct NetsukeManifest { #[serde(default)] pub vars: Vars, + /// Optional list of user-defined Jinja macros registered before rendering. + #[serde(default)] + pub macros: Vec, + /// Named rule templates that can be referenced by targets. #[serde(default)] pub rules: Vec, diff --git a/src/manifest.rs b/src/manifest.rs index cc39b7fe..192509c6 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -7,11 +7,59 @@ //! filesystem patterns during template evaluation. Both helpers fail fast when //! inputs are missing or patterns are invalid. -use crate::ast::NetsukeManifest; +use crate::ast::{MacroDefinition, NetsukeManifest}; use anyhow::{Context, Result}; -use minijinja::{Environment, Error, ErrorKind, UndefinedBehavior, value::Value}; +use minijinja::{ + AutoEscape, Environment, Error, ErrorKind, State, UndefinedBehavior, + value::{Kwargs, Object, Rest, Value}, +}; use serde_yml::Value as YamlValue; -use std::{fs, path::Path}; +use std::{fs, path::Path, ptr, sync::Arc}; + +/// A display name for a manifest source, used in error reporting. +#[derive(Debug, Clone)] +pub struct ManifestName(String); + +impl ManifestName { + /// Construct a manifest name for diagnostics. + /// + /// # Examples + /// + /// ```rust,ignore + /// use netsuke::manifest::ManifestName; + /// let name = ManifestName::new("Netsukefile"); + /// assert_eq!(name.to_string(), "Netsukefile"); + /// ``` + pub fn new(name: impl Into) -> Self { + Self(name.into()) + } + + #[must_use] + /// Borrow the manifest name as a string slice. + /// + /// # Examples + /// + /// ```rust,ignore + /// use netsuke::manifest::ManifestName; + /// let name = ManifestName::new("Config"); + /// assert_eq!(name.as_str(), "Config"); + /// ``` + pub fn as_str(&self) -> &str { + &self.0 + } +} + +impl std::fmt::Display for ManifestName { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} + +impl AsRef for ManifestName { + fn as_ref(&self) -> &str { + self.0.as_str() + } +} mod diagnostics; mod expand; @@ -57,6 +105,256 @@ fn env_var(name: &str) -> std::result::Result { } } +/// Extract the macro identifier from a signature string. +/// +/// The signature must follow the form `name(params)` where `name` is a valid +/// Jinja identifier and `params` is a parameter list (possibly empty). +/// +/// # Errors +/// +/// Returns an error if the signature is empty, lacks a parameter list, or the +/// identifier before `(` is empty. +/// +/// # Examples +/// +/// ```rust,ignore +/// let name = parse_macro_name("greet(name)").expect("valid signature"); +/// assert_eq!(name, "greet"); +/// ``` +fn parse_macro_name(signature: &str) -> Result { + let trimmed = signature.trim(); + if trimmed.is_empty() { + return Err(anyhow::anyhow!( + "macro signature '{signature}' is missing an identifier" + )); + } + let Some((name, _rest)) = trimmed.split_once('(') else { + return Err(anyhow::anyhow!( + "macro signature '{signature}' must include parameter list" + )); + }; + let name = name.trim(); + if name.is_empty() { + return Err(anyhow::anyhow!( + "macro signature '{signature}' is missing an identifier" + )); + } + Ok(name.to_string()) +} + +/// Register a single manifest macro in the Jinja environment. +/// +/// Compiles the macro body into a template and registers a callable function +/// with the extracted macro name. The template name is synthesised using the +/// provided index to ensure uniqueness. +/// +/// # Errors +/// +/// Returns an error if the macro signature is invalid or template compilation +/// fails. +fn register_macro(env: &mut Environment, macro_def: &MacroDefinition, index: usize) -> Result<()> { + let name = parse_macro_name(¯o_def.signature)?; + let template_name = format!("__manifest_macro_{index}_{name}"); + let template_source = format!( + "{{% macro {} %}}{}{{% endmacro %}}", + macro_def.signature, macro_def.body + ); + + env.add_template_owned(template_name.clone(), template_source) + .with_context(|| format!("compile macro '{name}'"))?; + + env.add_function(name.clone(), make_macro_fn(template_name, name)); + Ok(()) +} + +/// Register all manifest macros from a YAML document. +/// +/// Expects the YAML to have a `macros` key containing a sequence of mappings, +/// each with `signature` and `body` string fields. Registers each macro in the +/// environment using [`register_macro`]. +/// +/// # Errors +/// +/// Returns an error if the YAML shape is invalid, any macro signature is +/// malformed, or template compilation fails. +fn register_manifest_macros(doc: &YamlValue, env: &mut Environment) -> Result<()> { + let Some(macros) = doc.get("macros").cloned() else { + return Ok(()); + }; + + let defs: Vec = serde_yml::from_value(macros) + .context("macros must be a sequence of mappings with string signature/body")?; + + for (idx, def) in defs.iter().enumerate() { + register_macro(env, def, idx) + .with_context(|| format!("register macro '{}'", def.signature))?; + } + Ok(()) +} + +/// Invoke a `MiniJinja` value with optional keyword arguments. +/// +/// `MiniJinja` encodes keyword arguments by appending a [`Kwargs`] value to the +/// positional slice. This helper hides that convention so callers can pass the +/// keyword collection explicitly. +/// +/// # Examples +/// +/// ```rust,ignore +/// use minijinja::{Environment, value::{Kwargs, Value}}; +/// use netsuke::manifest::call_macro_value; +/// +/// let mut env = Environment::new(); +/// env.add_template( +/// "macro", +/// "{% macro greet(name='friend') %}hi {{ name }}{% endmacro %}", +/// ) +/// .unwrap(); +/// let template = env.get_template("macro").unwrap(); +/// let state = template.eval_to_state(()).unwrap(); +/// let value = state.lookup("greet").unwrap(); +/// let kwargs = Kwargs::from_iter([(String::from("name"), Value::from("Ada"))]); +/// let rendered = call_macro_value(&state, &value, &[], Some(kwargs)).unwrap(); +/// assert_eq!(rendered.to_string(), "hi Ada"); +/// ``` +fn call_macro_value( + state: &State, + macro_value: &Value, + positional: &[Value], + kwargs: Option, +) -> Result { + kwargs.map_or_else( + || macro_value.call(state, positional), + |kwargs| { + let mut call_args = Vec::with_capacity(positional.len() + 1); + call_args.extend_from_slice(positional); + call_args.push(Value::from(kwargs)); + macro_value.call(state, call_args.as_slice()) + }, + ) +} + +/// Create a wrapper that invokes a compiled manifest macro on demand. +/// +/// The returned closure fetches the internal template, resolves the macro, and +/// forwards the provided positional and keyword arguments to the call. +/// +/// # Examples +/// +/// ```rust,ignore +/// # use minijinja::Environment; +/// # use minijinja::value::{Kwargs, Rest, Value}; +/// # use netsuke::manifest::make_macro_fn; +/// let mut env = Environment::new(); +/// env.add_template( +/// "macro", +/// "{% macro greet(name='friend') %}hi {{ name }}{% endmacro %}", +/// ) +/// .unwrap(); +/// let wrapper = make_macro_fn("macro".into(), "greet".into()); +/// let state = env +/// .get_template("macro") +/// .unwrap() +/// .eval_to_state(()) +/// .unwrap(); +/// let kwargs = Kwargs::from_iter([ +/// (String::from("name"), Value::from("Ada")), +/// ]); +/// let output = wrapper(&state, Rest(vec![]), kwargs) +/// .unwrap() +/// .to_string(); +/// assert_eq!(output, "hi Ada"); +/// ``` +/// +/// # Errors +/// +/// The wrapper returns an error if the macro cannot be located or execution +/// fails. +fn make_macro_fn( + template_name: String, + macro_name: String, +) -> impl Fn(&State, Rest, Kwargs) -> Result { + move |state, Rest(args), kwargs| { + let template = state.env().get_template(&template_name)?; + let macro_state = template.eval_to_state(())?; + let macro_value = macro_state.lookup(¯o_name).ok_or_else(|| { + Error::new( + ErrorKind::InvalidOperation, + format!("macro '{macro_name}' not defined in '{template_name}'"), + ) + })?; + + // MiniJinja requires keyword arguments to be appended as a trailing + // `Kwargs` value within the positional slice. Build that value lazily so + // we avoid allocating when no keywords were supplied. + let mut entries: Vec<(String, Value)> = Vec::new(); + for key in kwargs.args() { + let mut value = kwargs.peek::(key)?; + if key == "caller" { + value = Value::from_object(CallerAdapter::new(state, value)); + } + entries.push((key.to_string(), value)); + } + let maybe_kwargs = if entries.is_empty() { + None + } else { + Some(entries.into_iter().collect::()) + }; + + let rendered_value = call_macro_value(¯o_state, ¯o_value, &args, maybe_kwargs)?; + let rendered: String = rendered_value.into(); + let value = if matches!(state.auto_escape(), AutoEscape::None) { + Value::from(rendered) + } else { + Value::from_safe_string(rendered) + }; + Ok(value) + } +} + +/// Adapter to preserve the outer template state for caller block invocation. +/// +/// `MiniJinja` executes the wrapper closure with a synthetic [`State`] that +/// differs from the one active when the manifest macro was called. Caller +/// blocks, however, must run within the original context to access the +/// manifest's globals and captured variables. This adapter stores a raw +/// pointer to that state so the macro can invoke the block later. +/// +/// # Safety +/// +/// The raw pointer is only valid while the outer [`State`] is alive. Safety +/// hinges on: +/// +/// - the macro invocation remaining synchronous (no `async` suspension) +/// - the original state outliving every [`CallerAdapter`] invocation +/// - the adapter not moving across threads despite the `Send`/`Sync` impls +/// +/// The unsynchronised `Send` and `Sync` impls mirror `MiniJinja`'s built-in +/// macro objects. They rely on the engine executing caller blocks on the same +/// thread that created the adapter, which matches the runtime's behaviour. +#[derive(Debug)] +struct CallerAdapter { + caller: Value, + state: *const State<'static, 'static>, +} + +impl CallerAdapter { + fn new(state: &State, caller: Value) -> Self { + let ptr = ptr::from_ref(state).cast::>(); + Self { caller, state: ptr } + } +} + +unsafe impl Send for CallerAdapter {} +unsafe impl Sync for CallerAdapter {} + +impl Object for CallerAdapter { + fn call(self: &Arc, _state: &State, args: &[Value]) -> Result { + let state = unsafe { &*self.state }; + self.caller.call(state, args) + } +} + /// Parse a manifest string using Jinja for value templating. /// /// The input YAML must be valid on its own. Jinja expressions are evaluated @@ -65,9 +363,9 @@ fn env_var(name: &str) -> std::result::Result { /// # Errors /// /// Returns an error if YAML parsing or Jinja evaluation fails. -fn from_str_named(yaml: &str, name: &str) -> Result { +fn from_str_named(yaml: &str, name: &ManifestName) -> Result { let mut doc: YamlValue = serde_yml::from_str(yaml).map_err(|e| ManifestError::Parse { - source: map_yaml_error(e, yaml, name), + source: map_yaml_error(e, yaml, name.as_ref()), })?; let mut jinja = Environment::new(); @@ -87,11 +385,13 @@ fn from_str_named(yaml: &str, name: &str) -> Result { } } + register_manifest_macros(&doc, &mut jinja)?; + expand_foreach(&mut doc, &jinja)?; let manifest: NetsukeManifest = serde_yml::from_value(doc).map_err(|e| ManifestError::Parse { - source: map_yaml_error(e, yaml, name), + source: map_yaml_error(e, yaml, name.as_ref()), })?; render_manifest(manifest, &jinja) @@ -106,7 +406,7 @@ fn from_str_named(yaml: &str, name: &str) -> Result { /// /// Returns an error if YAML parsing or Jinja evaluation fails. pub fn from_str(yaml: &str) -> Result { - from_str_named(yaml, "Netsukefile") + from_str_named(yaml, &ManifestName::new("Netsukefile")) } /// Load a [`NetsukeManifest`] from the given file path. @@ -118,7 +418,8 @@ 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_named(&data, &path_ref.display().to_string()) + let name = ManifestName::new(path_ref.display().to_string()); + from_str_named(&data, &name) } #[cfg(test)] diff --git a/src/manifest/render.rs b/src/manifest/render.rs index a2dc76bd..616c4eb8 100644 --- a/src/manifest/render.rs +++ b/src/manifest/render.rs @@ -142,6 +142,7 @@ mod tests { Ok(NetsukeManifest { netsuke_version: Version::parse("1.0.0")?, vars: manifest_vars, + macros: Vec::new(), rules: vec![rule], actions: Vec::new(), targets: vec![target], diff --git a/src/manifest/tests.rs b/src/manifest/tests.rs index df2793d3..546447a9 100644 --- a/src/manifest/tests.rs +++ b/src/manifest/tests.rs @@ -1,175 +1,119 @@ -use super::diagnostics::map_yaml_error; -use super::glob::{glob_paths, normalize_separators}; -use super::hints::YAML_HINTS; -use rstest::{fixture, rstest}; -use serde::de::Error as _; -use serde_yml::Error as YamlError; +//! Tests for manifest parsing and macro registration helpers. -#[test] -fn yaml_error_without_location_defaults_to_first_line() { - let err = YamlError::custom("boom"); - let msg = map_yaml_error(err, "", "test").to_string(); - assert!(msg.contains("line 1, column 1"), "message: {msg}"); -} +use super::*; +use anyhow::Result as AnyResult; +use minijinja::Environment; +use rstest::{fixture, rstest}; +use serde_yml::value::Mapping; -#[test] -fn yaml_hint_needles_are_lowercase() { - for (needle, _) in YAML_HINTS { - assert_eq!( - needle, - needle.to_lowercase(), - "needle not lower-case: {needle}" - ); - } +fn render_with(env: &Environment, template: &str) -> AnyResult { + Ok(env.render_str(template, ())?) } -#[test] -fn glob_paths_invalid_pattern_sets_syntax_error() { - let err = glob_paths("[").expect_err("expected pattern error"); - assert_eq!(err.kind(), minijinja::ErrorKind::SyntaxError); +#[fixture] +fn strict_env() -> Environment<'static> { + let mut env = Environment::new(); + env.set_undefined_behavior(UndefinedBehavior::Strict); + env } -#[cfg(unix)] #[rstest] -#[case("\\[")] -#[case("\\]")] -#[case("\\{")] -#[case("\\}")] -fn normalize_separators_preserves_bracket_escape_variants(#[case] pat: &str) { - assert_eq!(normalize_separators(pat), pat); +#[case("greet(name)", "greet")] +#[case("shout(text='hi')", "shout")] +#[case("joiner(*items)", "joiner")] +#[case("format(name, caller=None)", "format")] +#[case("complex(value, /, *, flag=false, **kw)", "complex")] +fn parse_macro_name_extracts_identifier(#[case] signature: &str, #[case] expected: &str) { + let name = parse_macro_name(signature).expect("parse name"); + assert_eq!(name, expected); } -#[cfg(unix)] #[rstest] -#[case("a\\b\\*", "a/b\\*")] -#[case("a\\b\\?", "a/b\\?")] -#[case("config\\*.yml", "config/*.yml")] -#[case("data\\?x.csv", "data\\?x.csv")] -fn normalize_separators_preserves_wildcard_escape_variants( - #[case] pat: &str, - #[case] expected: &str, -) { - // Inputs like "config\\*.yml" are treated as Windows-style and turn the - // backslash into a separator on Unix; other cases keep '\\' to force a - // literal via bracket-class rewrite downstream. - assert_eq!(normalize_separators(pat), expected); +#[case("greet", "include parameter list")] +#[case("(name)", "missing an identifier")] +#[case(" ", "missing an identifier")] +fn parse_macro_name_errors(#[case] signature: &str, #[case] message: &str) { + let err = parse_macro_name(signature).expect_err("should fail"); + assert!(err.to_string().contains(message), "{err}"); } -#[cfg(unix)] #[rstest] -#[case("assets/\\*.\\?", "assets//*.\\?")] -#[case("src/\\[a\\].c", "src/\\[a\\].c")] -#[case("build/\\{debug,release\\}/lib", "build/\\{debug,release\\}/lib")] -fn normalize_separators_preserves_specific_escape_patterns( - #[case] pat: &str, +#[case("greet()", "Hello", "{{ greet() }}", "Hello")] +#[case("echo(text='hi')", "{{ text }}", "{{ echo() }}", "hi")] +#[case( + "joiner(items)", + "{{ items | join(',') }}", + "{{ joiner(['a', 'b', 'c']) }}", + "a,b,c" +)] +#[case( + "show(name, excited=false)", + "{{ name ~ ('!' if excited else '') }}", + "{{ show('Netsuke', excited=true) }}", + "Netsuke!" +)] +fn register_macro_handles_arguments( + #[case] signature: &str, + #[case] body: &str, + #[case] template: &str, #[case] expected: &str, + mut strict_env: Environment, ) { - // Escaped metacharacters are preserved verbatim on Unix. - assert_eq!(normalize_separators(pat), expected); -} - -#[cfg(unix)] -#[test] -fn normalize_separators_handles_mixed_slashes() { - let input = r"dir\sub/leaf"; - assert_eq!(normalize_separators(input), "dir/sub/leaf"); -} - -#[fixture] -fn tmp() -> tempfile::TempDir { - tempfile::tempdir().expect("temp dir") + let macro_def = MacroDefinition { + signature: signature.to_string(), + body: body.to_string(), + }; + register_macro(&mut strict_env, ¯o_def, 0).expect("register"); + let rendered = render_with(&strict_env, template).expect("render"); + assert_eq!(rendered, expected); } -#[cfg(unix)] #[rstest] -#[case("\\*", "*")] -#[case("\\?", "?")] -#[case("prefix\\*suffix", "prefix*suffix")] -#[case("pre\\?post", "pre?post")] -#[case("mid\\*-", "mid*-")] -#[case("mid\\?_", "mid?_")] -fn glob_paths_treats_escaped_wildcards_as_literals( - tmp: tempfile::TempDir, - #[case] pattern_suffix: &str, - #[case] filename: &str, -) { - std::fs::write(tmp.path().join(filename), "a").expect("write file"); - std::fs::write(tmp.path().join("other"), "b").expect("write file"); - let pattern = format!("{}/{}", tmp.path().display(), pattern_suffix); - let out = glob_paths(&pattern).expect("glob ok"); - assert_eq!( - out, - vec![format!("{}/{}", tmp.path().display(), filename).replace('\\', "/"),], - ); +fn register_macro_supports_keyword_invocation(mut strict_env: Environment) { + let macro_def = MacroDefinition { + signature: "salute(name='friend')".to_string(), + body: "Hello {{ name }}".to_string(), + }; + register_macro(&mut strict_env, ¯o_def, 0).expect("register"); + let rendered = render_with(&strict_env, "{{ salute(name='Ada') }}").expect("render"); + assert_eq!(rendered.trim(), "Hello Ada"); } -#[cfg(unix)] #[rstest] -#[case( - vec![("sub", true), ("a.txt", false)], // (name, is_dir) - "*", - vec!["a.txt"], - "ignores directories and matches only files" -)] -#[case( - vec![ - ("dir", true), - ("dir/prefix*suffix", true), - ("dir/prefix*suffix/next", false), - ], - "dir/prefix\\*suffix/*", - vec!["dir/prefix*suffix/next"], - "treats escaped '*' as literal inside a segment across subsequent segments", -)] -#[case( - vec![("[ab]", false), ("b", false)], - "\\[ab\\]", - vec!["[ab]"], - "respects bracket escapes - treats as literals" -)] -#[case( - vec![("{debug,release}", false)], - "\\{debug,release\\}", - vec!["{debug,release}"], - "treats braces as literals when escaped" -)] -fn glob_paths_behavior_scenarios( - tmp: tempfile::TempDir, - #[case] files_to_create: Vec<(&str, bool)>, // (name, is_directory) - #[case] pattern_suffix: &str, - #[case] expected_matches: Vec<&str>, - #[case] description: &str, -) { - for (name, is_dir) in &files_to_create { - let path = tmp.path().join(name); - if *is_dir { - std::fs::create_dir(&path).expect("create dir"); - } else { - std::fs::write(&path, "content").expect("write file"); - } - } - - let pattern = format!("{}/{}", tmp.path().display(), pattern_suffix); - let mut result = glob_paths(&pattern).expect("glob ok"); - result.sort(); - let mut expected: Vec = expected_matches - .iter() - .map(|name| format!("{}/{}", tmp.path().display(), name).replace('\\', "/")) - .collect(); - expected.sort(); - assert_eq!(result, expected, "Test case: {description}"); +fn register_macro_forwards_caller(mut strict_env: Environment) { + let macro_def = MacroDefinition { + signature: "wrap(prefix, caller)".to_string(), + body: "{{ prefix }}{{ caller() }}".to_string(), + }; + register_macro(&mut strict_env, ¯o_def, 0).expect("register"); + let rendered = + render_with(&strict_env, "{% call wrap('Hi ') %}World{% endcall %}").expect("render"); + assert_eq!(rendered.trim(), "Hi World"); } #[rstest] -fn glob_paths_returns_empty_for_no_matches(tmp: tempfile::TempDir) { - let pattern = format!("{}/missing*", tmp.path().display()); - let result = glob_paths(&pattern).expect("glob ok"); - assert!(result.is_empty()); +fn register_manifest_macros_validates_shape(mut strict_env: Environment) { + let mut mapping = Mapping::new(); + mapping.insert( + YamlValue::from("macros"), + YamlValue::from(vec![YamlValue::from(42)]), + ); + let doc = YamlValue::Mapping(mapping); + let err = register_manifest_macros(&doc, &mut strict_env).expect_err("shape error"); + assert!( + err.to_string() + .contains("macros must be a sequence of mappings"), + "{err}" + ); } -#[test] -fn glob_paths_reports_unmatched_brace() { - let err = glob_paths("foo/{bar").expect_err("expected brace error"); - assert_eq!(err.kind(), minijinja::ErrorKind::SyntaxError); - assert!(err.to_string().contains("unmatched '{'")); +#[rstest] +fn register_manifest_macros_supports_multiple(mut strict_env: Environment) { + let yaml = serde_yml::from_str::( + "macros:\n - signature: \"greet(name)\"\n body: |\n Hello {{ name }}\n - signature: \"shout(text)\"\n body: |\n {{ text | upper }}\n", + ) + .expect("yaml value"); + register_manifest_macros(&yaml, &mut strict_env).expect("register"); + let rendered = render_with(&strict_env, "{{ shout(greet('netsuke')) }}").expect("render"); + assert_eq!(rendered.trim(), "HELLO NETSUKE"); } diff --git a/tests/ast_tests.rs b/tests/ast_tests.rs index 19ea6844..edc35953 100644 --- a/tests/ast_tests.rs +++ b/tests/ast_tests.rs @@ -198,6 +198,45 @@ fn optional_fields() { assert!(matches!(rule.deps, StringOrList::Empty)); } +#[rstest] +fn parses_macro_definitions() { + let yaml = r#" + netsuke_version: "1.0.0" + macros: + - signature: "greet(name)" + body: |- + Hello {{ name }} + targets: + - name: hello + command: "{{ greet('world') }}" + "#; + + let manifest = parse_manifest(yaml).expect("parse"); + assert_eq!(manifest.macros.len(), 1); + let macro_def = manifest.macros.first().expect("macro"); + assert_eq!(macro_def.signature, "greet(name)"); + assert!(macro_def.body.contains("Hello {{ name }}")); + + let serialised = serde_yml::to_string(&manifest.macros).expect("serialise macros"); + assert!(serialised.contains("greet(name)")); + assert!(serialised.contains("Hello {{ name }}")); +} + +#[test] +fn macro_definition_rejects_invalid_types() { + let yaml = r#" + netsuke_version: "1.0.0" + macros: + - signature: 42 + body: [] + targets: + - name: hello + command: noop + "#; + + assert!(parse_manifest(yaml).is_err()); +} + #[rstest] #[case::invalid_enum_variant( r#" diff --git a/tests/data/jinja_macro_arguments.yml b/tests/data/jinja_macro_arguments.yml new file mode 100644 index 00000000..5ab145bd --- /dev/null +++ b/tests/data/jinja_macro_arguments.yml @@ -0,0 +1,23 @@ +netsuke_version: "1.0.0" +macros: + - signature: "no_args()" + body: |- + ready + - signature: "defaulted(name='world')" + body: |- + Hi {{ name }} + - signature: "joiner(items)" + body: |- + {{ items | join(',') }} + - signature: "show(name, excited=false)" + body: |- + {{ name ~ ('!' if excited else '') }} +targets: + - name: ready + command: "{{ no_args() }}" + - name: default + command: "{{ defaulted() }}" + - name: join + command: "{{ joiner(['a', 'b', 'c']) }}" + - name: excite + command: "{{ show('Netsuke', excited=true) }}" diff --git a/tests/data/jinja_macro_invalid.yml b/tests/data/jinja_macro_invalid.yml new file mode 100644 index 00000000..09135eba --- /dev/null +++ b/tests/data/jinja_macro_invalid.yml @@ -0,0 +1,7 @@ +netsuke_version: "1.0.0" +macros: + - body: | + Missing signature +targets: + - name: oops + command: "{{ greet('world') }}" diff --git a/tests/data/jinja_macro_missing_parens.yml b/tests/data/jinja_macro_missing_parens.yml new file mode 100644 index 00000000..0b049de3 --- /dev/null +++ b/tests/data/jinja_macro_missing_parens.yml @@ -0,0 +1,8 @@ +netsuke_version: "1.0.0" +macros: + - signature: "greet" + body: |- + hi +targets: + - name: noop + command: "noop" diff --git a/tests/data/jinja_macros.yml b/tests/data/jinja_macros.yml new file mode 100644 index 00000000..ff4489b2 --- /dev/null +++ b/tests/data/jinja_macros.yml @@ -0,0 +1,11 @@ +netsuke_version: "1.0.0" +macros: + - signature: "greet(name)" + body: |- + Hello {{ name }}! + - signature: "shout(text)" + body: |- + {{ text | upper }} +targets: + - name: greeting + command: "{{ shout(greet('netsuke')) }}" diff --git a/tests/features/manifest.feature b/tests/features/manifest.feature index f666440a..60f874f8 100644 --- a/tests/features/manifest.feature +++ b/tests/features/manifest.feature @@ -62,12 +62,39 @@ Feature: Manifest Parsing When the manifest is checked Then the first target command is "echo world" + Scenario: Rendering manifest macros + Given the manifest file "tests/data/jinja_macros.yml" is parsed + When the manifest is checked + Then the first target command is "HELLO NETSUKE!" + + Scenario: Rendering manifest macros with varied signatures + Given the manifest file "tests/data/jinja_macro_arguments.yml" is parsed + When the manifest is checked + Then the manifest has 4 macros + And the macro 1 signature is "no_args()" + And the target 1 command is "ready" + And the target 2 command is "Hi world" + And the target 3 command is "a,b,c" + And the target 4 command is "Netsuke!" + Scenario: Parsing fails when an environment variable is undefined Given the environment variable "NETSUKE_UNDEFINED_ENV" is unset And the manifest file "tests/data/jinja_env_missing.yml" is parsed When the parsing result is checked Then parsing the manifest fails + Scenario: Parsing fails when a macro is missing its signature + Given the manifest file "tests/data/jinja_macro_invalid.yml" is parsed + When the parsing result is checked + Then parsing the manifest fails + And the error message contains "signature" + + Scenario: Parsing fails when a macro omits parentheses + Given the manifest file "tests/data/jinja_macro_missing_parens.yml" is parsed + When the parsing result is checked + Then parsing the manifest fails + And the error message contains "parameter list" + Scenario: Rendering Jinja conditionals in a manifest Given the manifest file "tests/data/jinja_if.yml" is parsed When the manifest is checked diff --git a/tests/manifest_jinja_tests.rs b/tests/manifest_jinja_tests.rs index 58bcf784..506f2910 100644 --- a/tests/manifest_jinja_tests.rs +++ b/tests/manifest_jinja_tests.rs @@ -162,6 +162,104 @@ fn undefined_variable_errors() { assert!(manifest::from_str(&yaml).is_err()); } +#[rstest] +fn registers_manifest_macros() { + let yaml = manifest_yaml(concat!( + "macros:\n", + " - signature: \"greet(name)\"\n", + " body: |-\n", + " Hello {{ name }}!\n", + " - signature: \"shout(text)\"\n", + " body: |-\n", + " {{ text | upper }}\n", + "targets:\n", + " - name: greet\n", + " command: \"{{ shout(greet('world')) }}\"\n", + )); + + let manifest = manifest::from_str(&yaml).expect("parse macros"); + let target = manifest.targets.first().expect("target"); + match &target.recipe { + Recipe::Command { command } => assert_eq!(command, "HELLO WORLD!"), + other => panic!("expected command recipe, got {other:?}"), + } +} + +#[rstest] +#[case( + concat!( + "macros:\n", + " - signature: \"no_args()\"\n", + " body: |-\n", + " ready\n", + ), + "{{ no_args() }}", + "ready", +)] +#[case( + concat!( + "macros:\n", + " - signature: \"defaulted(name='world')\"\n", + " body: |-\n", + " Hi {{ name }}\n", + ), + "{{ defaulted() }}", + "Hi world", +)] +#[case( + concat!( + "macros:\n", + " - signature: \"joiner(items)\"\n", + " body: |-\n", + " {{ items | join(',') }}\n", + ), + "{{ joiner(['a', 'b', 'c']) }}", + "a,b,c", +)] +#[case( + concat!( + "macros:\n", + " - signature: \"show(name, excited=false)\"\n", + " body: |-\n", + " {{ name ~ ('!' if excited else '') }}\n", + ), + "{{ show('Netsuke', excited=true) }}", + "Netsuke!", +)] +fn registers_manifest_macro_argument_variants( + #[case] macros_block: &str, + #[case] expression: &str, + #[case] expected: &str, +) { + let command = expression.replace('"', "\\\""); + let yaml = manifest_yaml(&format!( + "{macros_block}targets:\n - name: test\n command: \"{command}\"\n", + )); + + let manifest = manifest::from_str(&yaml).expect("parse macros"); + let target = manifest.targets.first().expect("target"); + match &target.recipe { + Recipe::Command { command } => assert_eq!(command, expected), + other => panic!("expected command recipe, got {other:?}"), + } +} + +#[rstest] +fn manifest_macro_with_missing_signature_errors() { + let yaml = manifest_yaml(concat!( + "macros:\n", + " - body: |\n", + " hi\n", + "targets:\n", + " - name: noop\n", + " command: noop\n", + )); + + let err = manifest::from_str(&yaml).expect_err("macro signature required"); + let msg = format!("{err:?}"); + assert!(msg.contains("signature"), "error message: {msg}"); +} + #[rstest] fn syntax_error_errors() { let yaml = manifest_yaml("targets:\n - name: hello\n command: echo {{ who\n"); diff --git a/tests/steps/manifest_steps.rs b/tests/steps/manifest_steps.rs index ca38e6ff..bfce808d 100644 --- a/tests/steps/manifest_steps.rs +++ b/tests/steps/manifest_steps.rs @@ -229,6 +229,27 @@ fn manifest_has_targets(world: &mut CliWorld, count: usize) { assert_eq!(manifest.targets.len(), count); } +#[then(expr = "the manifest has {int} macros")] +fn manifest_has_macros(world: &mut CliWorld, count: usize) { + let manifest = world.manifest.as_ref().expect("manifest"); + assert_eq!(manifest.macros.len(), count); +} + +#[then(expr = "the macro {int} signature is {string}")] +#[expect( + clippy::needless_pass_by_value, + reason = "Cucumber step requires owned String" +)] +fn macro_signature_is(world: &mut CliWorld, index: usize, signature: String) { + let manifest = world.manifest.as_ref().expect("manifest"); + let idx = index.checked_sub(1).expect("macros use 1-based index"); + let macro_def = manifest + .macros + .get(idx) + .unwrap_or_else(|| panic!("missing macro {index}")); + assert_eq!(macro_def.signature, signature); +} + #[then(expr = "the manifest has targets named {string}")] #[expect( clippy::needless_pass_by_value,