Merge pull request #7294 from dotCooCoo/fix/live-shape-unknown-keys

Live Shapes: object.setLiveShape refuses unknown keys and invalid sides or radius

crates/engine/src/cmd/object/shape.rs

@@ -135,9 +135,7 @@ pub(super) fn set_live_shape(s: &mut Session, p: &Value) -> Result<Value> {
                 .map_err(|_| bad("object.setLiveShape", format!("`kind` must be \"round\", \"invertedRound\" or \"chamfer\", not {v}")))
         })
         .transpose()?;
-    let radius = p.get("radius").and_then(Value::as_f64);
     let relative_radii = p.get("relativeRadii").and_then(Value::as_bool);
-    let sides = p.get("sides").and_then(Value::as_u64);
     // A number that must be `what` (None when absent).
     let number = |k: &str, ok: fn(f64) -> bool, what: &str| -> Result<Option<f64>> {
         match p.get(k) {
@@ -151,6 +149,9 @@ pub(super) fn set_live_shape(s: &mut Session, p: &Value) -> Result<Value> {
     };
     let angle = |k: &str| Ok::<_, EngineError>(number(k, |_| true, "an angle in degrees")?.map(|a| a.rem_euclid(360.0)));
     let length = |k: &str| number(k, |x| x > 0.0 && x <= crate::MAX_COORD, "a length in points above 0");
+    let radius = number("radius", |r| r >= 0.0, "a radius of 0 or more")?;
+    // A polygon's side count: a whole number from 3 to 1000.
+    let sides = number("sides", |n| n.fract() == 0.0 && (3.0..=1000.0).contains(&n), "a whole number from 3 to 1000")?.map(|n| n as u64);
     // An ellipse's pie (Ellipse Properties: Pie Start and End Angle, degrees; Invert Pie swaps them).
     let (pie_start, pie_end) = (angle("pieStart")?, angle("pieEnd")?);
     let invert = p.get("invertPie").and_then(Value::as_bool) == Some(true);

crates/engine/src/cmd/params.rs

@@ -8,7 +8,9 @@
 //! (`…settings as file.new`), or, naming none, any key (the options of the format an export picks). Objects inside a value aren't
 //! described key by key, so only the top level is checked, except in the options of an export, a
 //! save or Export for Screens: those are checked against the options of the formats the command
-//! can write, in `marks`, `bleed` and the other objects too ([`check_export_options`]).
+//! can write, in `marks`, `bleed` and the other objects too ([`check_export_options`]); and in the
+//! objects of an array whose keys `MEMBER_KEYS` lists, such as the `items` of
+//! `object.setLiveShape`.
 //!
 //! [`Session::execute`] answers a key the description doesn't name with a parameter error
 //! ([`check_params`]), for every caller (the app, the control channel, MCP, the CLI) and for the
@@ -269,6 +271,41 @@ pub(crate) fn unknown(id: &str, key: &str, keys: &[&str]) -> crate::EngineError
     )
 }
 
+/// Commands whose params hold an array of objects with keys of their own: the command, the key of
+/// the array and the keys each of its objects takes.
+const MEMBER_KEYS: &[(&str, &str, &[&str])] = &[("object.setLiveShape", "items", &["id", "corners"])];
+
+/// Each object in the arrays [`MEMBER_KEYS`] names for command `id` has only the keys it lists, or
+/// the parameter error naming the first other one with its place (`items[0].corner`), and the
+/// nearest accepted key or the accepted ones. A member that isn't an object is the command's to
+/// report.
+fn check_member_keys(id: &str, params: &Value) -> Result<()> {
+    for (_, list, keys) in MEMBER_KEYS.iter().filter(|(c, ..)| *c == id) {
+        let Some(members) = params.get(*list).and_then(Value::as_array) else { continue };
+        for (i, member) in members.iter().enumerate() {
+            let Value::Object(given) = member else { continue };
+            let Some(key) = given.keys().find(|k| !keys.contains(&k.as_str())) else { continue };
+            let at = |k: &str| format!("{list}[{i}].{k}");
+            let near = nearest(key, keys);
+            let key = at(&shown_key(key));
+            return Err(bad(
+                id,
+                match near {
+                    Some(near) => {
+                        let near = at(near);
+                        format!("unknown parameter `{key}`; did you mean `{near}`?")
+                    }
+                    None => {
+                        let keys = keys.iter().map(|k| at(k)).collect::<Vec<_>>().join(", ");
+                        format!("unknown parameter `{key}`; the command takes: {keys}")
+                    }
+                },
+            ));
+        }
+    }
+    Ok(())
+}
+
 /// `params` of built-in effect `effect`, as command `id` was given them (#1592): only keys the
 /// effect takes (those of its dialog defaults and of its params description), each of the type
 /// the effect reads ([`check_effect_types`]), a `color` the effect can draw ([`check_effect_color`])
@@ -526,7 +563,6 @@ pub const UNCHECKED: &[&str] = &[
     "object.rasterize",
     "object.repeat.options",
     "object.setBounds",
-    "object.setLiveShape",
     "object.setOverprint",
     "object.setProps",
     "object.slice.divide",
@@ -833,6 +869,7 @@ fn check_params_in(id: &str, params: &Value, in_batch: bool) -> Result<()> {
         return Ok(());
     }
     check_keys(id, spec.params, params)?;
+    check_member_keys(id, params)?;
     if in_batch {
         crate::schemas::check(spec, params, false)?;
     }

crates/engine/src/tests/livecorners.rs

@@ -89,6 +89,43 @@ fn bad_corners_and_kinds_are_errors() {
     assert_eq!(corners(&s, id), ([0.0; 4], [CornerKind::Round; 4], 4), "nothing changed");
 }
 
+/// A side count that isn't a whole number from 3 to 1000, a negative radius and an unknown key
+/// (at the top level or in one of `items`) are errors naming the key; the shapes and the undo
+/// history stay as they were. A radius of 0 and the ends of the side range still work.
+#[test]
+fn bad_sides_radii_and_keys_are_errors() {
+    let (mut s, id) = session();
+    let poly = NodeId(s.execute("shape.polygon", &json!({"cx": 200, "cy": 200, "radius": 80, "sides": 6})).unwrap()["id"].as_u64().unwrap());
+    let state = |s: &Session| {
+        let st = s.doc().unwrap();
+        (corners(s, id), st.doc.node(poly).unwrap().clone(), st.history.undo.len())
+    };
+    let before = state(&s);
+    for (p, key) in [
+        (json!({"ids": [poly.0], "sides": 2.5}), "`sides` must be a whole number from 3 to 1000, not 2.5"),
+        (json!({"ids": [poly.0], "sides": -1}), "`sides` must be a whole number from 3 to 1000, not -1"),
+        (json!({"ids": [poly.0], "sides": 2}), "`sides` must be a whole number from 3 to 1000, not 2"),
+        (json!({"ids": [poly.0], "sides": 1001}), "`sides` must be a whole number from 3 to 1000, not 1001"),
+        (json!({"ids": [id.0, poly.0], "radius": -4}), "`radius` must be a radius of 0 or more, not -4"),
+        (json!({"items": [{"id": id.0, "corners": [0]}], "radius": -0.5}), "`radius` must be a radius of 0 or more, not -0.5"),
+        (json!({"id": id.0, "path": [[0, 0]], "radius": 5}), "unknown parameter `path`"),
+        (json!({"items": [{"id": id.0, "corner": [0]}], "radius": 5}), "unknown parameter `items[0].corner`; did you mean `items[0].corners`?"),
+    ] {
+        match s.execute("object.setLiveShape", &p) {
+            Err(EngineError::BadParams { cmd, msg }) => assert!(cmd == "object.setLiveShape" && msg.starts_with(key), "{p}: {msg}"),
+            other => panic!("{p}: {other:?}"),
+        }
+        assert_eq!(state(&s), before, "{p}");
+    }
+    run(&mut s, json!({"ids": [poly.0], "sides": 3}));
+    assert!(matches!(node_path(&s, poly).1, Some(LiveShape::Polygon { sides: 3, .. })));
+    run(&mut s, json!({"ids": [poly.0], "sides": 1000.0}));
+    assert!(matches!(node_path(&s, poly).1, Some(LiveShape::Polygon { sides: 1000, .. })));
+    run(&mut s, json!({"id": id.0, "radius": 6}));
+    run(&mut s, json!({"id": id.0, "radius": 0}));
+    assert_eq!(corners(&s, id).0, [0.0; 4]);
+}
+
 #[test]
 fn direct_selected_corners_round_alone_and_stay_selected() {
     let (mut s, id) = session();

crates/engine/src/tests/params.rs

@@ -400,6 +400,43 @@ fn stroke_set_refuses_an_unknown_key() {
     }
 }
 
+/// `object.setLiveShape` names every key it reads, and each of its `items` takes `id` and
+/// `corners` alone: another key, at the top level, in an item or in a batch step, is refused
+/// instead of being ignored.
+#[test]
+fn set_live_shape_refuses_an_unknown_key() {
+    const ID: &str = "object.setLiveShape";
+    assert!(!UNCHECKED.contains(&ID));
+    let e = refusal(ID, json!({"id": 3, "path": [[0, 0], [1, 1]], "radius": 5}));
+    assert!(e.starts_with("unknown parameter `path`; the command takes: ") && e.contains("relativeRadii"), "{e}");
+    let e = refusal(ID, json!({"radiuss": 5}));
+    assert_eq!(e, "unknown parameter `radiuss`; did you mean `radius`?");
+    let e = refusal(ID, json!({"items": [{"id": 3, "corners": [0]}, {"id": 4, "corner": [0]}], "radius": 5}));
+    assert_eq!(e, "unknown parameter `items[1].corner`; did you mean `items[1].corners`?");
+    let e = refusal(ID, json!({"items": [{"id": 3, "radius": 5}]}));
+    assert_eq!(e, "unknown parameter `items[0].radius`; the command takes: items[0].id, items[0].corners");
+    let step = |params| json!({"commands": [{"command": "select.all", "params": {}}, {"command": ID, "params": params}]});
+    for (bad, key) in [(json!({"path": "x"}), "path"), (json!({"items": [{"id": 3, "kind": "chamfer"}]}), "items[0].kind")] {
+        match check_params("command.batch", &step(bad.clone())) {
+            Err(EngineError::BadParams { cmd, msg }) => {
+                assert!(cmd == ID && msg.starts_with(&format!("unknown parameter `{key}`")), "{bad}: {cmd} {msg}")
+            }
+            other => panic!("{bad}: {other:?}"),
+        }
+    }
+    for ok in [
+        json!({"items": [{"id": 3, "corners": [0, 2]}, {"id": 4}], "radius": 5}),
+        json!({"corners": [0], "kind": "chamfer", "radius": 2, "relativeRadii": false}),
+        json!({"id": 3, "radius": 5}),
+        json!({"ids": [3], "sides": 6, "polygonAngle": 15, "polygonRadius": 20, "sideLength": null, "makeSidesEqual": true}),
+        json!({"pieStart": 30, "pieEnd": 90, "invertPie": true}),
+        json!({"items": null}),
+    ] {
+        assert!(check_params(ID, &ok).is_ok(), "{ok}");
+        assert!(check_params("command.batch", &step(ok.clone())).is_ok(), "{ok}");
+    }
+}
+
 /// An object id given as something other than a whole number (`"5"`, `-1`) is refused as such,
 /// naming its key, not as a missing id; an id left out is still a missing one.
 #[test]

crates/ui-egui/src/dialogs/corners.rs

@@ -105,7 +105,7 @@ fn body(app: &mut VectorcraftApp, ui: &mut egui::Ui, d: &mut Dialog) -> bool {
                 d.fields.insert("radius".into(), json!(n));
             }
         } else {
-            form::length(ui, d, "radius", unit, 120.0);
+            form::length_within(ui, d, "radius", unit, 120.0, Some(0.0..=f64::INFINITY));
         }
         ui.end_row();
     });

crates/ui-egui/src/panels.rs

@@ -123,7 +123,8 @@ fn corner_fields(app: &mut VectorcraftApp, ui: &mut Ui, n: &Node, id: &str, widt
         for (k, label) in rectangle_corners() {
             dim_label(ui, label);
             if let Some(v) = crate::widgets::num_field_named(ui, (id, "corner", k), Some(label), Some(lc.radius(k)), unit, width) {
-                app.run("object.setLiveShape", json!({"radius": v, "corners": [k]})).ok();
+                // A negative radius typed in rounds nothing: the command takes 0 or more.
+                app.run("object.setLiveShape", json!({"radius": v.max(0.0), "corners": [k]})).ok();
             }
             ui.end_row();
         }
@@ -133,7 +134,7 @@ fn corner_fields(app: &mut VectorcraftApp, ui: &mut Ui, n: &Node, id: &str, widt
 /// [`corner_radius_row`]'s field, `width` wide.
 pub(crate) fn corner_radius_field(app: &mut VectorcraftApp, ui: &mut Ui, n: &Node, id: impl std::hash::Hash + std::fmt::Debug, width: f32) {
     if let Some(r) = crate::widgets::num_field(ui, id, corner_radius(app, n), app.session.general_unit(), width) {
-        app.run("object.setLiveShape", json!({"radius": r})).ok();
+        app.run("object.setLiveShape", json!({"radius": r.max(0.0)})).ok();
     }
 }
 

crates/ui-egui/src/panels/transform.rs

@@ -448,7 +448,7 @@ pub(crate) fn polygon_rows(app: &mut VectorcraftApp, ui: &mut Ui, n: &Node, live
     let mut set = None;
     egui::Grid::new((id, "grid")).num_columns(2).spacing([6.0, 6.0]).show(ui, |ui| {
         widgets::dim_label(ui, tl!("Sides:"));
-        if let Some(s) = widgets::range_field_named(ui, (id, "sides"), Some(tl!("Sides:")), f64::from(*sides), 3.0..=f64::INFINITY, "", 0, fw) {
+        if let Some(s) = widgets::range_field_named(ui, (id, "sides"), Some(tl!("Sides:")), f64::from(*sides), 3.0..=1000.0, "", 0, fw) {
             set = Some(("sides", json!(s as u64)));
         }
         ui.end_row();

docs/mcp.md

@@ -5447,6 +5447,11 @@ length that isn't a number above 0 is an error. An ellipse (Ellipse Properties):
 counterclockwise from 3 o'clock; 0 to 360 is the whole ellipse) and `invertPie: true`, which swaps them. Each call is
 one undo step.
 
+`object.setLiveShape` returns a parameter error, and leaves the document and its undo history as they were, for a key
+it does not take, at the top level (`` unknown parameter `path` ``) or in one of `items`
+(`` unknown parameter `items[0].corner`; did you mean `items[0].corners`? ``), for a `sides` that is not a whole number
+from 3 to 1000, such as 2.5, 2 or 1001, and for a negative `radius`. A `radius` of 0 removes the rounding.
+
 ```json
 {"name":"run_command","arguments":{"command":"object.setLiveShape","params":{"sides":8,"polygonAngle":22.5,"sideLength":40}}}
 {"name":"run_command","arguments":{"command":"object.setLiveShape","params":{"makeSidesEqual":true}}}

docs/target-app-parity.md

@@ -1552,6 +1552,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
   Create Object Mosaic (its columns and rows), Transform Each (its reference point and Random seed) and Rearrange All Artboards (its columns) refuse a fraction, a negative number or a string, such as 2.5, -1 or "4"; 4.0 counts as 4.
   Move, Rotate, Scale, Reflect, Shear, Transform Again, Nudge and the Transform panel's values (their copy, Scale Strokes & Effects, Scale Corners, Transform Patterns, Transform Objects and area type options), Align, Distribute and Distribute Spacing (Transform Patterns), Object Properties (visible, locked, Isolate Blending and Opacity & Mask Define Knockout Shape) and Live Shape Properties (Invert Pie, Make Sides Equal and relative rounding) refuse a value other than true, false or null, such as "false", and leave the document as it was.
   Object Properties (its opacity), Live Shape Properties (corner radius and polygon sides), the Transform panel's reference point and Align, Distribute and Distribute Spacing to an artboard (the artboard's number) refuse a value other than a number or null, such as "50", and leave the document as it was.
+  Live Shape Properties refuses a key it does not take, at the top level or in one of its items, a polygon side count that is not a whole number from 3 to 1000 and a negative corner radius, and leaves the document as it was; the Transform panel's Sides field takes 3 to 1000.
   Transform Each (its scales, moves and rotation), Rasterize (its resolution), Create Object Mosaic (its new width and height and its spacing), Create Trim Marks (its offset, length, bleed and weight) and Rearrange All Artboards (its spacing) refuse a value other than a number or null, such as "50", and change nothing. Crop Image refuses a rectangle that is not four numbers, such as "0, 0, 20, 20".
   Transform Each (Reflect X, Reflect Y, Random, Copy, Scale Strokes & Effects, Scale Corners, Transform Objects and Transform Patterns), Create Object Mosaic (gray tiles and deleting the image), Crop Image (its trim), Create Trim Marks (all artboards) and Rearrange All Artboards (by column, Move Artwork with Artboard and Transform Patterns) refuse a value other than true, false or null, such as "false", and change nothing. Create Trim Marks refuses a style that is not text, such as 5.
   New Artboard refuses a position or size other than a number or null, such as "10", and New Artboard and Artboard Options refuse a name other than text or null, such as 5; each leaves the artboards and the undo history as they were.