From 696c5f64ef3adeb4bbee6cc8ac480b8f6239ca39 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Thu, 1 Oct 2026 10:20:41 +0200 Subject: [PATCH 1/4] fix(schema): expose div's five aliases, and see multi-line serde attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `every_serde_alias_in_the_sources_is_reachable_from_the_exported_schema` was green while the schema rejected `"type": "card"`, which every example in the repository writes. Its scanner read the sources line by line and kept a line only if it both started with `#[serde(` and carried an `alias = "`. rustfmt splits an attribute with six arguments across six lines, so the one declaring `container`, `card`, `flex`, `grid` and `positioned` as spellings of `div` was invisible to it. `progress_bar`, which fits on one line, was caught — hence a test that looked like it worked. The scanner now takes each `#[serde(...)]` as a whole, paren-matched across lines. That alone turns it red on the five. The reachability check also had to learn that a variant alias widens an `enum` array rather than adding a sibling property, which is why it demanded `card` next to `div` under `properties` and would have stayed red after the real fix. Refs #424 --- crates/rustmotion/src/cli/commands/schema.rs | 73 ++++++++++++++++++-- 1 file changed, 67 insertions(+), 6 deletions(-) diff --git a/crates/rustmotion/src/cli/commands/schema.rs b/crates/rustmotion/src/cli/commands/schema.rs index cd1511f..b38c9e9 100644 --- a/crates/rustmotion/src/cli/commands/schema.rs +++ b/crates/rustmotion/src/cli/commands/schema.rs @@ -28,6 +28,16 @@ const SERDE_ALIASES: &[(&str, &str, &[&str])] = &[ ("AnimationEffect", "float3d", &["float_3d"]), ("ComponentBase", "progress", &["progress_bar"]), ("ChildComponentBase", "progress", &["progress_bar"]), + ( + "ComponentBase", + "div", + &["container", "card", "flex", "grid", "positioned"], + ), + ( + "ChildComponentBase", + "div", + &["container", "card", "flex", "grid", "positioned"], + ), ]; fn widen_enums_with(value: &mut serde_json::Value, canonical: &str, aliases: &[&str]) { @@ -286,6 +296,33 @@ mod serde_alias_exposure_tests { Some(rest[..close].to_string()) } + fn serde_attributes(text: &str) -> Vec { + let mut out = Vec::new(); + let mut rest = text; + while let Some(at) = rest.find("#[serde(") { + rest = &rest[at..]; + let mut depth = 0usize; + let mut end = None; + for (i, c) in rest.char_indices() { + match c { + '(' => depth += 1, + ')' => { + depth -= 1; + if depth == 0 { + end = Some(i + 1); + break; + } + } + _ => {} + } + } + let Some(end) = end else { break }; + out.push(rest[..end].split_whitespace().collect::>().join(" ")); + rest = &rest[end..]; + } + out + } + fn aliases_declared_in_the_sources() -> Vec { let mut files = Vec::new(); rust_sources(&workspace_root().join("crates"), &mut files); @@ -294,13 +331,12 @@ mod serde_alias_exposure_tests { let Ok(text) = std::fs::read_to_string(&file) else { continue; }; - for line in text.lines() { - let trimmed = line.trim_start(); - if !trimmed.starts_with("#[serde(") || !trimmed.contains("alias = \"") { + for attribute in serde_attributes(&text) { + if !attribute.contains("alias = \"") { continue; } - let renamed_sibling = quoted_value_after(trimmed, "rename"); - let mut rest = trimmed; + let renamed_sibling = quoted_value_after(&attribute, "rename"); + let mut rest = attribute.as_str(); while let Some(at) = rest.find("alias = \"") { rest = &rest[at + "alias = \"".len()..]; let Some(close) = rest.find('"') else { break }; @@ -316,6 +352,29 @@ mod serde_alias_exposure_tests { found } + fn some_enum_array_carries_both( + value: &serde_json::Value, + canonical: &str, + alias: &str, + ) -> bool { + match value { + serde_json::Value::Object(map) => { + if let Some(serde_json::Value::Array(variants)) = map.get("enum") { + let has = |want: &str| variants.iter().any(|v| v.as_str() == Some(want)); + if has(canonical) && has(alias) { + return true; + } + } + map.values() + .any(|child| some_enum_array_carries_both(child, canonical, alias)) + } + serde_json::Value::Array(items) => items + .iter() + .any(|item| some_enum_array_carries_both(item, canonical, alias)), + _ => false, + } + } + fn some_properties_object_carries_both( value: &serde_json::Value, canonical: &str, @@ -354,12 +413,14 @@ mod serde_alias_exposure_tests { .filter(|d| match &d.renamed_sibling { Some(canonical) => { !some_properties_object_carries_both(&schema, canonical, &d.alias) + && !some_enum_array_carries_both(&schema, canonical, &d.alias) } None => !flat.contains(&format!("\"{}\"", d.alias)), }) .map(|d| match &d.renamed_sibling { Some(canonical) => format!( - "{} (declared in {}, expected beside {canonical})", + "{} (declared in {}, expected beside {canonical}, as a sibling property or \ + as another value of the same enum)", d.alias, d.file ), None => format!("{} (declared in {})", d.alias, d.file), From 83682dca65731c614cad89cfac5653d5814e5e0a Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Thu, 1 Oct 2026 10:27:46 +0200 Subject: [PATCH 2/4] fix(schema): describe the wire format of the two hand-written deserializers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `AnimationTiming` and `schema::style::FontWeight` both parse through a hand-written Deserialize and both derive JsonSchema, so the exported schema described the Rust shape rather than the JSON the engine takes. Measured against the repository's own examples, these two accounted for 52 of the 62 rejected paths. AnimationTiming deserializes through AnimationTimingWire. Three divergences followed, and the third is the one that costs a user a render: `loop` was declared required although the wire defaults it; `loop` was declared a boolean although it also takes a play count (#330); and `repeat_count`, which is an output of RepeatSpec::into_parts rather than a wire field, was advertised as a property — the wire type is deny_unknown_fields, so a generator that believed the schema got its whole component dropped. AnimationTiming derives neither Serialize nor Deserialize, so its serde attributes are read by schemars alone and changing them cannot move runtime behaviour. FontWeight's visitor takes "normal", "bold" or a number; the derive emitted "Normal", "Bold" and {"Weight": n}. It now carries a hand-written JsonSchema saying what the visitor does, which is also where its description had to move: a doc comment on a type that no longer derives JsonSchema stops being interface text. Both are pinned by a test that asserts the schema and serde agree on the same literals rather than asserting the schema alone. Refs #424 --- crates/rustmotion-core/src/schema/style.rs | 20 ++- crates/rustmotion-core/src/schema/video.rs | 11 +- crates/rustmotion/src/cli/commands/schema.rs | 121 +++++++++++++++++++ 3 files changed, 147 insertions(+), 5 deletions(-) diff --git a/crates/rustmotion-core/src/schema/style.rs b/crates/rustmotion-core/src/schema/style.rs index 9cf1332..0c61b24 100644 --- a/crates/rustmotion-core/src/schema/style.rs +++ b/crates/rustmotion-core/src/schema/style.rs @@ -61,8 +61,7 @@ pub struct TimelineStep { pub style: Option>, } -/// Font weight — named ("normal"/"bold") or numeric (100-900) -#[derive(Debug, Clone, JsonSchema, Default)] +#[derive(Debug, Clone, Default)] pub enum FontWeight { #[default] Normal, @@ -91,6 +90,23 @@ impl Serialize for FontWeight { } } +impl schemars::JsonSchema for FontWeight { + fn schema_name() -> String { + "FontWeight".to_string() + } + + fn json_schema(_: &mut schemars::gen::SchemaGenerator) -> schemars::schema::Schema { + serde_json::from_value(serde_json::json!({ + "description": "Font weight — \"normal\", \"bold\", or a number.", + "anyOf": [ + { "type": "string", "enum": ["normal", "bold"] }, + { "type": "number" } + ] + })) + .expect("FontWeight's schema is a literal written here") + } +} + impl<'de> Deserialize<'de> for FontWeight { fn deserialize>(deserializer: D) -> Result { struct FontWeightVisitor; diff --git a/crates/rustmotion-core/src/schema/video.rs b/crates/rustmotion-core/src/schema/video.rs index 956fc91..f216fa5 100644 --- a/crates/rustmotion-core/src/schema/video.rs +++ b/crates/rustmotion-core/src/schema/video.rs @@ -207,7 +207,8 @@ pub struct AnimationTiming { /// this field. The JSON `"loop"` key this deserializes from accepts /// either shape (see [`AnimationTimingWire`]); `repeat` alone can't /// tell you which one was written — check `repeat_count` for that. - #[serde(rename = "loop")] + #[serde(default, rename = "loop")] + #[schemars(with = "RepeatSpec")] pub repeat: bool, /// How many times the animation plays, when the JSON `"loop"` value /// was a positive integer rather than a bare bool (issue #330) — e.g. @@ -219,7 +220,8 @@ pub struct AnimationTiming { /// [`RepeatSpec::into_parts`]): there is no visible difference /// between "play once" and "loop zero times", so there's no reason to /// carry a count that never changes anything downstream. - #[serde(default)] + #[serde(default, skip)] + #[schemars(skip)] pub repeat_count: Option, /// Reverse direction on every other play (ping-pong) instead of /// snapping back to the start each cycle — GSAP calls this `yoyo`. @@ -256,7 +258,10 @@ fn default_animation_duration() -> f64 { 0.8 } -#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize)] +/// `true` loops forever, `false` plays once, and a positive integer is a +/// total number of plays (`12` for GSAP's `repeat: 11`). `0` and `1` both +/// mean "play once". +#[derive(Debug, Clone, Copy, PartialEq, Serialize, Deserialize, JsonSchema)] #[serde(untagged)] enum RepeatSpec { Loop(bool), diff --git a/crates/rustmotion/src/cli/commands/schema.rs b/crates/rustmotion/src/cli/commands/schema.rs index b38c9e9..f506c5c 100644 --- a/crates/rustmotion/src/cli/commands/schema.rs +++ b/crates/rustmotion/src/cli/commands/schema.rs @@ -483,6 +483,127 @@ mod serde_alias_exposure_tests { ); } + fn inline_refs(schema: &serde_json::Value, node: &serde_json::Value) -> serde_json::Value { + fn step( + defs: &serde_json::Value, + node: &serde_json::Value, + depth: u8, + ) -> serde_json::Value { + if depth == 0 { + return node.clone(); + } + match node { + serde_json::Value::Object(map) => { + if let Some(name) = map + .get("$ref") + .and_then(|r| r.as_str()) + .and_then(|r| r.strip_prefix("#/definitions/")) + { + if let Some(target) = defs.get(name) { + return step(defs, target, depth - 1); + } + } + serde_json::Value::Object( + map.iter() + .map(|(k, v)| (k.clone(), step(defs, v, depth - 1))) + .collect(), + ) + } + serde_json::Value::Array(items) => serde_json::Value::Array( + items.iter().map(|v| step(defs, v, depth - 1)).collect(), + ), + other => other.clone(), + } + } + let defs = schema + .get("definitions") + .cloned() + .unwrap_or(serde_json::Value::Null); + step(&defs, node, 6) + } + + fn preset_animation_branch<'a>( + schema: &'a serde_json::Value, + name: &str, + ) -> &'a serde_json::Value { + schema + .pointer("/definitions/AnimationEffect") + .and_then(|e| e.get("oneOf").or_else(|| e.get("anyOf"))) + .and_then(|b| b.as_array()) + .expect("AnimationEffect is a union") + .iter() + .find(|branch| { + branch + .pointer("/properties/name/enum") + .and_then(|e| e.as_array()) + .is_some_and(|values| values.iter().any(|v| v.as_str() == Some(name))) + }) + .unwrap_or_else(|| panic!("no AnimationEffect branch tagged {name}")) + } + + #[test] + fn animation_timing_is_declared_the_way_its_wire_type_parses() { + let schema = build_schema(); + let fade = preset_animation_branch(&schema, "fade_in"); + + let required: Vec<&str> = fade + .get("required") + .and_then(|r| r.as_array()) + .map(|r| r.iter().filter_map(|v| v.as_str()).collect()) + .unwrap_or_default(); + assert_eq!( + required, + ["name"], + "AnimationTiming carries `repeat: bool` with no serde default, so schemars made `loop` required — while AnimationTimingWire defaults it" + ); + assert!( + fade.pointer("/properties/repeat_count").is_none(), + "repeat_count is an output of RepeatSpec::into_parts, not a wire field, and the wire type is deny_unknown_fields: advertising it hands a generator a key that drops the whole component" + ); + + let text = + inline_refs(&schema, fade.pointer("/properties/loop").expect("loop")).to_string(); + assert!( + text.contains("boolean") && text.contains("integer"), + "`loop` takes a bool or a play count (#330), not a bool alone: {text}" + ); + + let parses = |v: serde_json::Value| { + serde_json::from_value::(v).is_ok() + }; + assert!(parses(serde_json::json!({ "name": "fade_in" }))); + assert!(parses(serde_json::json!({ "name": "fade_in", "loop": 12 }))); + assert!( + !parses(serde_json::json!({ "name": "fade_in", "repeat_count": 3 })), + "if this ever starts parsing, repeat_count belongs back in the schema" + ); + } + + #[test] + fn font_weight_is_declared_the_way_its_visitor_parses() { + let schema = build_schema(); + let node = schema + .pointer("/definitions/RichTextSpan/properties/font-weight") + .expect("a rich_text span carries font-weight"); + let described = inline_refs(&schema, node).to_string(); + assert!( + described.contains("\"bold\""), + "the visitor takes \"bold\"; schemars derived Rust's `Bold` from the variant \ + name: {described}" + ); + assert!( + !described.contains("\"Bold\""), + "\"Bold\" is what the derive emitted and what the parser refuses: {described}" + ); + + let parses = |v: serde_json::Value| { + serde_json::from_value::(v).is_ok() + }; + assert!(parses(serde_json::json!("bold"))); + assert!(parses(serde_json::json!(700))); + assert!(!parses(serde_json::json!("Bold"))); + } + #[test] fn a_gradient_text_stop_accepts_both_spellings_of_its_position() { let schema = build_schema(); From 13f8f65a6a780ef4a607628480dd2c859d50fff4 Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Thu, 1 Oct 2026 10:34:11 +0200 Subject: [PATCH 3/4] feat(schema): accept a $placeholder wherever a scalar is declared MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A templated scenario writes `"delay": "$delay"` where the schema declares a number, so a Draft-7 validator rejected it although the engine substitutes before deserializing and accepts the file. Four of the fifteen examples this repository ships are templated. Every position declaring a number, an integer or a boolean now also accepts a string matching `^\$[A-Za-z_][A-Za-z0-9_]*$`, hoisted into a single `TemplatePlaceholder` definition — inlining it at each of the several thousand sites cost 690KB. Two things are deliberately not widened. A node carrying an `enum` is left alone, which keeps the `type` tag a closed set: it is the discriminator a validator needs to report inside the right branch, and a free string there would cost that for every component. And the pattern means a bare string is still refused where a number is expected, so `"delay": "soon"` stays an error rather than becoming one more thing the schema waves through. The exported schema grows 1.11MB to 1.42MB. A test compiles what `rustmotion schema` actually prints and validates every example against it, so the claim is measured on the artifact rather than on the function that builds it. Closes #424 --- Cargo.lock | 52 +++++++- crates/rustmotion/Cargo.toml | 3 + crates/rustmotion/src/cli/commands/schema.rs | 113 ++++++++++++++++ .../exported_schema_accepts_the_examples.rs | 124 ++++++++++++++++++ 4 files changed, 288 insertions(+), 4 deletions(-) create mode 100644 crates/rustmotion/tests/exported_schema_accepts_the_examples.rs diff --git a/Cargo.lock b/Cargo.lock index c1ec414..e646992 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -117,6 +117,7 @@ checksum = "5a15f179cd60c4584b8a8c596927aadc462e27f2ca70c04e0071964a73ba7a75" dependencies = [ "cfg-if", "const-random", + "getrandom 0.3.4", "once_cell", "version_check", "zerocopy", @@ -252,6 +253,12 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "appendlist" +version = "1.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e149dc73cd30538307e7ffa2acd3d2221148eaeed4871f246657b1c3eaa1cbd2" + [[package]] name = "approx" version = "0.5.1" @@ -806,6 +813,32 @@ dependencies = [ "piper", ] +[[package]] +name = "boon" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "baa187da765010b70370368c49f08244b1ae5cae1d5d33072f76c8cb7112fe3e" +dependencies = [ + "ahash", + "appendlist", + "base64", + "fluent-uri 0.3.2", + "idna", + "once_cell", + "percent-encoding", + "regex", + "regex-syntax", + "serde", + "serde_json", + "url", +] + +[[package]] +name = "borrow-or-share" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dc0b364ead1874514c8c2855ab558056ebfeb775653e7ae45ff72f28f8f3166c" + [[package]] name = "borsh" version = "1.8.1" @@ -1480,7 +1513,7 @@ dependencies = [ "wasm-bindgen", "wasm-bindgen-futures", "web-sys", - "windows 0.61.3", + "windows 0.62.2", ] [[package]] @@ -2150,6 +2183,16 @@ dependencies = [ "bitflags 1.3.2", ] +[[package]] +name = "fluent-uri" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1918b65d96df47d3591bed19c5cca17e3fa5d0707318e4b5ef2eae01764df7e5" +dependencies = [ + "borrow-or-share", + "ref-cast", +] + [[package]] name = "flume" version = "0.12.0" @@ -2568,7 +2611,7 @@ dependencies = [ "log", "presser", "thiserror 2.0.18", - "windows 0.61.3", + "windows 0.62.2", ] [[package]] @@ -3525,7 +3568,7 @@ dependencies = [ "js-sys", "log", "wasm-bindgen", - "windows-core 0.61.2", + "windows-core 0.62.2", ] [[package]] @@ -4215,7 +4258,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "53353550a17c04ac46c585feb189c2db82154fc84b79c7a66c96c2c644f66071" dependencies = [ "bitflags 1.3.2", - "fluent-uri", + "fluent-uri 0.1.4", "serde", "serde_json", "serde_repr", @@ -6577,6 +6620,7 @@ dependencies = [ name = "rustmotion" version = "0.8.0" dependencies = [ + "boon", "clap", "clap_complete", "crossterm", diff --git a/crates/rustmotion/Cargo.toml b/crates/rustmotion/Cargo.toml index b4d35f0..d651f6a 100644 --- a/crates/rustmotion/Cargo.toml +++ b/crates/rustmotion/Cargo.toml @@ -105,3 +105,6 @@ ffmpeg_integration = [] ## Re-expose native Lottie decoding (default-on). Activating this feature here ## propagates to rustmotion-components so consumers only need to touch rustmotion. lottie-native = ["rustmotion-components/lottie-native"] + +[dev-dependencies] +boon = "0.6.1" diff --git a/crates/rustmotion/src/cli/commands/schema.rs b/crates/rustmotion/src/cli/commands/schema.rs index f506c5c..45979db 100644 --- a/crates/rustmotion/src/cli/commands/schema.rs +++ b/crates/rustmotion/src/cli/commands/schema.rs @@ -116,6 +116,104 @@ fn expose_serde_aliases(defs: &mut serde_json::Map) { } } +const PLACEHOLDER_PATTERN: &str = "^\\$[A-Za-z_][A-Za-z0-9_]*$"; + +const PLACEHOLDER_DESCRIPTION: &str = "A `$name` placeholder, substituted before the scenario is \ + deserialized — from `config`, from a `for-each` element's fields, or from a `use`'s `props`. \ + See CLAUDE.md's \"Factorisation\" section."; + +fn is_scalar_schema(map: &serde_json::Map) -> bool { + if map.contains_key("enum") || map.contains_key("const") { + return false; + } + let scalar = |name: &str| matches!(name, "number" | "integer" | "boolean"); + match map.get("type") { + Some(serde_json::Value::String(name)) => scalar(name), + Some(serde_json::Value::Array(names)) => names + .iter() + .filter_map(|v| v.as_str()) + .any(|name| scalar(name)), + _ => false, + } +} + +fn accept_a_placeholder_too(value: &mut serde_json::Value) { + let Some(map) = value.as_object_mut() else { + return; + }; + let description = map.remove("description"); + let default = map.get("default").cloned(); + let mut widened = serde_json::json!({ + "anyOf": [ + value.clone(), + { "$ref": "#/definitions/TemplatePlaceholder" } + ] + }); + if let Some(description) = description { + widened["description"] = description; + } + if let Some(default) = default { + widened["default"] = default; + } + *value = widened; +} + +fn accept_placeholders_where_a_scalar_is_declared(value: &mut serde_json::Value) { + const SCHEMA_VALUED: &[&str] = &[ + "additionalProperties", + "additionalItems", + "not", + "if", + "then", + "else", + "propertyNames", + "contains", + ]; + const SCHEMA_MAPS: &[&str] = &[ + "properties", + "patternProperties", + "definitions", + "dependencies", + ]; + const SCHEMA_LISTS: &[&str] = &["allOf", "anyOf", "oneOf"]; + + let Some(map) = value.as_object_mut() else { + return; + }; + for key in SCHEMA_VALUED { + if let Some(child) = map.get_mut(*key) { + accept_placeholders_where_a_scalar_is_declared(child); + } + } + for key in SCHEMA_MAPS { + if let Some(serde_json::Value::Object(children)) = map.get_mut(*key) { + for child in children.values_mut() { + accept_placeholders_where_a_scalar_is_declared(child); + } + } + } + for key in SCHEMA_LISTS { + if let Some(serde_json::Value::Array(children)) = map.get_mut(*key) { + for child in children.iter_mut() { + accept_placeholders_where_a_scalar_is_declared(child); + } + } + } + match map.get_mut("items") { + Some(serde_json::Value::Array(children)) => { + for child in children.iter_mut() { + accept_placeholders_where_a_scalar_is_declared(child); + } + } + Some(child) => accept_placeholders_where_a_scalar_is_declared(child), + None => {} + } + + if is_scalar_schema(map) { + accept_a_placeholder_too(value); + } +} + fn build_schema() -> serde_json::Value { let mut scenario_schema = schema::generate_json_schema(); let component_schema = serde_json::to_value(schemars::schema_for!(Component)) @@ -168,6 +266,21 @@ fn build_schema() -> serde_json::Value { *template = serde_json::json!({ "$ref": "#/definitions/TemplateValue" }); } + accept_placeholders_where_a_scalar_is_declared(&mut scenario_schema); + if let Some(defs) = scenario_schema + .pointer_mut("/definitions") + .and_then(|d| d.as_object_mut()) + { + defs.insert( + "TemplatePlaceholder".to_string(), + serde_json::json!({ + "type": "string", + "pattern": PLACEHOLDER_PATTERN, + "description": PLACEHOLDER_DESCRIPTION + }), + ); + } + scenario_schema } diff --git a/crates/rustmotion/tests/exported_schema_accepts_the_examples.rs b/crates/rustmotion/tests/exported_schema_accepts_the_examples.rs new file mode 100644 index 0000000..d2cf7a0 --- /dev/null +++ b/crates/rustmotion/tests/exported_schema_accepts_the_examples.rs @@ -0,0 +1,124 @@ +use std::path::{Path, PathBuf}; +use std::process::Command; + +fn workspace_root() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .and_then(Path::parent) + .expect("rustmotion is expected at /crates/rustmotion") + .to_path_buf() +} + +fn exported_schema() -> serde_json::Value { + let out = Command::new(env!("CARGO_BIN_EXE_rustmotion")) + .arg("schema") + .output() + .expect("run `rustmotion schema`"); + assert!(out.status.success(), "`rustmotion schema` failed"); + serde_json::from_slice(&out.stdout).expect("the exported schema is JSON") +} + +fn compiled(schema: serde_json::Value) -> (boon::Schemas, boon::SchemaIndex) { + let mut schemas = boon::Schemas::new(); + let mut compiler = boon::Compiler::new(); + compiler + .add_resource("rustmotion.json", schema) + .expect("the exported schema is a usable resource"); + let index = compiler + .compile("rustmotion.json", &mut schemas) + .expect("the exported schema compiles"); + (schemas, index) +} + +fn example_scenarios() -> Vec { + let mut files: Vec = std::fs::read_dir(workspace_root().join("examples")) + .expect("examples/ exists") + .flatten() + .map(|e| e.path()) + .filter(|p| p.extension().is_some_and(|e| e == "json")) + .collect(); + files.sort(); + files +} + +#[test] +fn every_example_in_the_repository_validates_against_the_exported_schema() { + let (schemas, index) = compiled(exported_schema()); + let files = example_scenarios(); + assert!( + files.len() >= 10, + "test setup: only {} examples found", + files.len() + ); + + let mut rejected = Vec::new(); + for file in &files { + let doc: serde_json::Value = + serde_json::from_slice(&std::fs::read(file).expect("read example")) + .expect("an example is JSON"); + if let Err(e) = schemas.validate(&doc, index) { + let name = file.file_name().unwrap_or_default().to_string_lossy(); + rejected.push(format!("{name}: {e}")); + } + } + + assert!( + rejected.is_empty(), + "`rustmotion schema` is what a generator reads to know what to write, so it must not \ + declare invalid what this repository ships and `rustmotion validate` accepts:\n\n{}", + rejected.join("\n\n") + ); +} + +#[test] +fn a_placeholder_is_only_accepted_where_it_looks_like_one() { + let (schemas, index) = compiled(exported_schema()); + let scenario = |delay: serde_json::Value| { + serde_json::json!({ + "video": { "width": 64, "height": 64, "fps": 30 }, + "scenes": [{ + "duration": 1.0, + "children": [{ + "type": "text", + "content": "x", + "style": { "animation": [{ "name": "fade_in", "delay": delay }] } + }] + }] + }) + }; + + for accepted in [serde_json::json!(0.4), serde_json::json!("$delay")] { + assert!( + schemas.validate(&scenario(accepted.clone()), index).is_ok(), + "{accepted} is what the engine takes for a delay" + ); + } + for refused in [serde_json::json!("soon"), serde_json::json!("$")] { + assert!( + schemas.validate(&scenario(refused.clone()), index).is_err(), + "{refused} is not a number and not a placeholder: widening a scalar must not turn \ + it into a free-form string" + ); + } +} + +#[test] +fn a_component_tag_is_not_widened_into_a_placeholder() { + let (schemas, index) = compiled(exported_schema()); + let with_type = |tag: serde_json::Value| { + serde_json::json!({ + "video": { "width": 64, "height": 64, "fps": 30 }, + "scenes": [{ "duration": 1.0, "children": [{ "type": tag, "content": "x" }] }] + }) + }; + assert!(schemas + .validate(&with_type(serde_json::json!("text")), index) + .is_ok()); + assert!( + schemas + .validate(&with_type(serde_json::json!("nonesuch")), index) + .is_err(), + "the `type` tag is an enum, and widening it would cost the discriminator every \ + validator uses to report inside the right branch" + ); +} From 47e0f79449e0541a99d643d8f1bce50325b31bfb Mon Sep 17 00:00:00 2001 From: Baptiste Parmantier Date: Thu, 1 Oct 2026 10:44:24 +0200 Subject: [PATCH 4/4] feat(schema): steer every union on its tag so a validator names the real fault MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A one-letter typo in an animation name produced a 3611-node error tree whose best match was "is not valid under any of the given schemas", anchored on the whole component. Nothing in that names `style/animation/0/name`, and a generator reading it cannot act on it. Each branch already pinned its tag to a one-element enum, so the tag was never what was missing — the steering was. A union whose branches all pin one key to disjoint values becomes an `allOf` of `if`/`then` on that key, which leaves validation identical and makes a validator descend into the one branch that was meant. It applies to any union shaped that way, which is ComponentBase's 54 variants and AnimationEffect's 59, among others. The component/`for-each`/`use` union has no common tag, so it is steered on key presence instead, in the code that builds it. The same typo now reports at `scenes/0/children/0/style/animation/0/name` with the list of names it could have been, in a tree of 10 nodes. A `for-each` missing its `template` says so, and `propz` instead of `props` is named. Refs #424 --- README.md | 7 + crates/rustmotion/CLAUDE.md | 2 + crates/rustmotion/src/cli/commands/schema.rs | 141 ++++++++++++++++-- .../exported_schema_accepts_the_examples.rs | 47 ++++++ 4 files changed, 183 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index dcbf636..2f45f2b 100644 --- a/README.md +++ b/README.md @@ -207,6 +207,13 @@ Prints the JSON Schema for scenario files (editor autocompletion, LLM prompts). rustmotion schema -o schema.json ``` +It describes a scenario as written, templating included: `"delay": "$delay"` +validates wherever a number is declared, because the engine substitutes before +it deserializes. A bare string there is still an error — the placeholder has to +look like one. Each union is steered by its tag (`type` on a component, `name` +on an animation), so a validator reports the property that is wrong instead of +failing the whole component against all 54 branches. + ### `rustmotion info` Shows information about a scenario (duration, scene count, dimensions, ...). diff --git a/crates/rustmotion/CLAUDE.md b/crates/rustmotion/CLAUDE.md index 67d7aba..46c404f 100644 --- a/crates/rustmotion/CLAUDE.md +++ b/crates/rustmotion/CLAUDE.md @@ -23,6 +23,8 @@ CLI : - `--report r.json` — rapport JSON - `--strict-anim` — vérification frame par frame ; ajoute la détection `animated_text_overflow` (transform animé qui sort du viewport à un instant échantillonné). L'échantillonnage s'arrête à `scene.freeze_at`, puisque rien n'est rendu au-delà. - `--strict-attrs` — promeut en erreurs les attributs inconnus (détection schéma + did-you-mean, activée par défaut en warnings) + +`rustmotion schema` exporte un Draft-7 qui accepte un scénario **templaté** (`"$delay"` passe là où un nombre est déclaré) et qui aiguille chaque union sur son tag, pour qu'un validateur nomme la propriété fautive. Ce que `validate` accepte, le schéma l'accepte : c'est verrouillé par `tests/exported_schema_accepts_the_examples.rs`, qui valide les 15 exemples du dépôt contre ce que la commande imprime vraiment. - `--lenient` — warnings au lieu d'errors ## Google Fonts: the network is denied by default diff --git a/crates/rustmotion/src/cli/commands/schema.rs b/crates/rustmotion/src/cli/commands/schema.rs index 45979db..10b3615 100644 --- a/crates/rustmotion/src/cli/commands/schema.rs +++ b/crates/rustmotion/src/cli/commands/schema.rs @@ -129,10 +129,9 @@ fn is_scalar_schema(map: &serde_json::Map) -> bool { let scalar = |name: &str| matches!(name, "number" | "integer" | "boolean"); match map.get("type") { Some(serde_json::Value::String(name)) => scalar(name), - Some(serde_json::Value::Array(names)) => names - .iter() - .filter_map(|v| v.as_str()) - .any(|name| scalar(name)), + Some(serde_json::Value::Array(names)) => { + names.iter().filter_map(|v| v.as_str()).any(scalar) + } _ => false, } } @@ -214,6 +213,104 @@ fn accept_placeholders_where_a_scalar_is_declared(value: &mut serde_json::Value) } } +fn tag_values(branch: &serde_json::Value, key: &str) -> Option> { + let declared = branch.get("properties")?.get(key)?; + if let Some(one) = declared.get("const").and_then(|v| v.as_str()) { + return Some(vec![one.to_string()]); + } + let listed = declared.get("enum")?.as_array()?; + let values: Vec = listed + .iter() + .filter_map(|v| v.as_str().map(str::to_string)) + .collect(); + (!values.is_empty() && values.len() == listed.len()).then_some(values) +} + +fn discriminating_key(branches: &[serde_json::Value]) -> Option { + let first = branches.first()?.get("properties")?.as_object()?; + for key in first.keys() { + let Some(per_branch) = branches + .iter() + .map(|b| tag_values(b, key)) + .collect::>>() + else { + continue; + }; + let mut seen = std::collections::HashSet::new(); + if per_branch + .iter() + .flatten() + .all(|value| seen.insert(value.clone())) + { + return Some(key.clone()); + } + } + None +} + +fn steer_tagged_unions_on_their_tag(value: &mut serde_json::Value) { + let Some(map) = value.as_object_mut() else { + return; + }; + for child in map.values_mut() { + match child { + serde_json::Value::Array(items) => { + for item in items.iter_mut() { + steer_tagged_unions_on_their_tag(item); + } + } + other => steer_tagged_unions_on_their_tag(other), + } + } + + let union_key = if map.contains_key("oneOf") { + "oneOf" + } else if map.contains_key("anyOf") { + "anyOf" + } else { + return; + }; + let Some(branches) = map.get(union_key).and_then(|b| b.as_array()).cloned() else { + return; + }; + let Some(key) = discriminating_key(&branches) else { + return; + }; + + let every_branch_requires_the_tag = branches.iter().all(|b| { + b.get("required") + .and_then(|r| r.as_array()) + .is_some_and(|r| r.iter().any(|v| v.as_str() == Some(key.as_str()))) + }); + + let mut steered: Vec = Vec::with_capacity(branches.len() + 1); + let all: Vec = branches + .iter() + .filter_map(|b| tag_values(b, &key)) + .flatten() + .collect(); + let mut guard = serde_json::json!({ "properties": { key.clone(): { "enum": all } } }); + if every_branch_requires_the_tag { + guard["required"] = serde_json::json!([key.clone()]); + } + steered.push(guard); + for branch in branches { + let Some(values) = tag_values(&branch, &key) else { + return; + }; + steered.push(serde_json::json!({ + "if": { + "required": [key.clone()], + "properties": { key.clone(): { "enum": values } } + }, + "then": branch + })); + } + + map.remove(union_key); + map.insert("allOf".to_string(), serde_json::Value::Array(steered)); +} + fn build_schema() -> serde_json::Value { let mut scenario_schema = schema::generate_json_schema(); let component_schema = serde_json::to_value(schemars::schema_for!(Component)) @@ -267,6 +364,7 @@ fn build_schema() -> serde_json::Value { } accept_placeholders_where_a_scalar_is_declared(&mut scenario_schema); + steer_tagged_unions_on_their_tag(&mut scenario_schema); if let Some(defs) = scenario_schema .pointer_mut("/definitions") .and_then(|d| d.as_object_mut()) @@ -296,11 +394,13 @@ fn wrap_with_directives(defs_obj: &mut serde_json::Map Vec<&serde_json::Value> { + if let Some(branches) = node + .get("oneOf") + .or_else(|| node.get("anyOf")) + .and_then(|b| b.as_array()) + { + return branches.iter().collect(); + } + node.get("allOf") + .and_then(|b| b.as_array()) + .map(|entries| entries.iter().filter_map(|e| e.get("then")).collect()) + .unwrap_or_default() + } + fn preset_animation_branch<'a>( schema: &'a serde_json::Value, name: &str, ) -> &'a serde_json::Value { - schema + let node = schema .pointer("/definitions/AnimationEffect") - .and_then(|e| e.get("oneOf").or_else(|| e.get("anyOf"))) - .and_then(|b| b.as_array()) - .expect("AnimationEffect is a union") - .iter() + .expect("AnimationEffect is defined"); + union_branches(node) + .into_iter() .find(|branch| { branch .pointer("/properties/name/enum") diff --git a/crates/rustmotion/tests/exported_schema_accepts_the_examples.rs b/crates/rustmotion/tests/exported_schema_accepts_the_examples.rs index d2cf7a0..cbcd180 100644 --- a/crates/rustmotion/tests/exported_schema_accepts_the_examples.rs +++ b/crates/rustmotion/tests/exported_schema_accepts_the_examples.rs @@ -122,3 +122,50 @@ fn a_component_tag_is_not_widened_into_a_placeholder() { validator uses to report inside the right branch" ); } + +fn rejection_of(child: serde_json::Value) -> String { + let (schemas, index) = compiled(exported_schema()); + let doc = serde_json::json!({ + "video": { "width": 64, "height": 64, "fps": 30 }, + "scenes": [{ "duration": 1.0, "children": [child] }] + }); + let err = schemas + .validate(&doc, index) + .expect_err("this scenario must be rejected"); + format!("{err:#}") +} + +#[test] +fn a_rejected_component_is_reported_at_the_property_that_is_wrong() { + let typo = rejection_of(serde_json::json!({ + "type": "text", + "content": "hi", + "style": { "animation": [{ "name": "fade_in_upp" }] } + })); + assert!( + typo.contains("fade_in_up"), + "the report must name the spelling that was meant, not just fail the whole \ + component:\n{typo}" + ); + assert!( + typo.contains("animation"), + "the report must reach the property that is wrong:\n{typo}" + ); +} + +#[test] +fn a_rejected_directive_is_reported_as_that_directive() { + let incomplete = rejection_of(serde_json::json!({ "for-each": [{ "a": 1 }] })); + assert!( + incomplete.contains("template"), + "a `for-each` without its `template` must be reported as the missing key, not as a \ + component that matched no branch:\n{incomplete}" + ); + + let misspelt = rejection_of(serde_json::json!({ "use": "card", "propz": {} })); + assert!( + misspelt.contains("propz"), + "the overrides key is `props`; naming the one that was written is the whole value of \ + discriminating on `use`:\n{misspelt}" + ); +}