diff --git a/crates/engine/src/cmd.rs b/crates/engine/src/cmd.rs index 94d8add68b..d4fb2c39cd 100644 --- a/crates/engine/src/cmd.rs +++ b/crates/engine/src/cmd.rs @@ -440,6 +440,15 @@ pub(crate) fn f64_req(p: &Value, key: &str, cmd: &str) -> Result { Some(v) => v.as_f64().ok_or_else(|| bad(cmd, format!("`{key}` must be a number, not {v}"))), } } +/// The whole number of 0 or more that `p` gives for `key`. A missing key or null is "missing integer +/// `{key}`"; any other value, such as `"3"`, `2.5`, `-1` or `true`, is "`{key}` must be a whole +/// number", as [`object_param`] words it. +pub(crate) fn u64_req(p: &Value, key: &str, cmd: &str) -> Result { + match p.get(key) { + None | Some(Value::Null) => Err(bad(cmd, format!("missing integer `{key}`"))), + Some(v) => v.as_u64().ok_or_else(|| bad(cmd, format!("`{key}` must be a whole number"))), + } +} /// The number `p` gives for `key`, or `default` when `p` leaves it out or gives null. Any other /// value, such as `"40"`, is an error: "`{key}` must be a number, not {v}". pub(crate) fn number_or(p: &Value, key: &str, default: f64, cmd: &str) -> Result { diff --git a/crates/engine/src/cmd/appearance.rs b/crates/engine/src/cmd/appearance.rs index 02bc111746..bed4b33b76 100644 --- a/crates/engine/src/cmd/appearance.rs +++ b/crates/engine/src/cmd/appearance.rs @@ -36,7 +36,7 @@ use super::paint::paint_from; use super::{AppearanceAttrs, EyedropperAttrs, FillAttrs, StrokeAttrs}; use super::{ CommandSpec, NodeId, Result, Session, bad, check_numbers, check_switches, cmd, has_doc, has_selection, ids_param, index, leaf_targets, - object_param, ok, paint_targets, str_param, switch_or, targets, + object_param, ok, paint_targets, str_param, switch_or, targets, u64_req, }; use crate::EngineError; @@ -604,9 +604,9 @@ pub(crate) fn edits_stroke(s: &Session, p: &Value, item: ItemTarget, default: bo Ok(ids.get(item.anchor(s, &ids)).and_then(|id| d.node(*id)?.appearance.items.get(index)).map_or(default, |it| !it.is_fill())) } -/// The whole number of 0 or more that `key` holds, read with [`index`]. +/// The whole number of 0 or more that `key` holds, read with [`u64_req`] and [`index`]. pub(crate) fn index_param(p: &Value, key: &str, cmd: &str) -> Result { - p.get(key).and_then(Value::as_u64).map(index).ok_or_else(|| bad(cmd, format!("missing integer `{key}`"))) + u64_req(p, key, cmd).map(index) } fn set_active_item(s: &mut Session, p: &Value) -> Result { diff --git a/crates/engine/src/cmd/draw2/common.rs b/crates/engine/src/cmd/draw2/common.rs index 0ca376b243..fd8967f3fa 100644 --- a/crates/engine/src/cmd/draw2/common.rs +++ b/crates/engine/src/cmd/draw2/common.rs @@ -231,9 +231,9 @@ pub(super) fn within_input_bound(q: Point) -> bool { q.x.is_finite() && q.y.is_finite() && q.x.abs() < MAX_INPUT && q.y.abs() < MAX_INPUT } -/// The whole number of 0 or more that `key` holds, read with [`index`]. +/// The whole number of 0 or more that `key` holds, read with [`u64_req`](crate::cmd::u64_req) and [`index`]. pub(super) fn usize_req(p: &Value, key: &str, cmd: &str) -> Result { - p.get(key).and_then(Value::as_u64).map(index).ok_or_else(|| bad(cmd, format!("missing integer `{key}`"))) + crate::cmd::u64_req(p, key, cmd).map(index) } /// The whole number of 0 or more that `key` holds, or `None` when `key` is absent or null. Any other diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 990ac1a1c6..576b05ffd6 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -11,7 +11,7 @@ use super::clipboard::SwatchChoices; use super::{ CommandSpec, Result, Session, always, bad, can_clear_history, can_redo, can_undo, check_numbers, checked_ids_param, clock_date, cmd, date_param, f64_req, finite_numbers, has_clipboard, has_doc, has_selection, has_selection_guides_or_artboard_tool, has_selection_or_guides, index, number_or, - object_param, ok, point_param, str_param, switch_or, + object_param, ok, point_param, str_param, switch_or, u64_req, }; use crate::{Clipboard, EngineError, SourceLayer, inspect}; @@ -249,7 +249,10 @@ fn file_close(s: &mut Session, p: &Value) -> Result { } fn doc_activate(s: &mut Session, p: &Value) -> Result { - let i = p.get("index").and_then(Value::as_u64).map(index).ok_or_else(|| bad("document.activate", "missing index"))?; + let i = match p.get("index") { + None | Some(Value::Null) => return Err(bad("document.activate", "missing index")), + Some(v) => v.as_u64().map(index).ok_or_else(|| bad("document.activate", "`index` must be a whole number"))?, + }; if s.set_active(i) { ok() } else { Err(bad("document.activate", "no such document")) } } @@ -262,7 +265,10 @@ fn doc_move(s: &mut Session, p: &Value) -> Result { if from >= n { return Err(bad("document.move", "no such document")); } - let to = p.get("to").and_then(Value::as_u64).map(index).ok_or_else(|| bad("document.move", "missing `to`: a non-negative integer"))?; + let to = match p.get("to") { + None | Some(Value::Null) => return Err(bad("document.move", "missing `to`: a non-negative integer")), + Some(v) => v.as_u64().map(index).ok_or_else(|| bad("document.move", "`to` must be a whole number"))?, + }; let to = to.min(n.saturating_sub(1)); if !s.move_document(from, to) { return Err(bad("document.move", "no such document")); @@ -1190,8 +1196,7 @@ fn clear(s: &mut Session, p: &Value) -> Result { /// Document ids scope node ids: two tabs may contain objects with the same node id. fn drop_selection(s: &mut Session, p: &Value) -> Result { const CMD: &str = "edit.dropSelection"; - let key = "sourceDocument"; - let uid = p[key].as_u64().ok_or_else(|| bad(CMD, format!("missing integer `{key}`")))?; + let uid = u64_req(p, "sourceDocument", CMD)?; let point = |key: &str| finite_numbers::<2>(&p[key]).ok_or_else(|| bad(CMD, format!("{key} must be [x, y] with finite coordinates"))); let grab = point("grab")?; let at = point("at")?; diff --git a/crates/engine/src/tests/params.rs b/crates/engine/src/tests/params.rs index 6fb8d30467..8a33376075 100644 --- a/crates/engine/src/tests/params.rs +++ b/crates/engine/src/tests/params.rs @@ -572,6 +572,64 @@ fn a_malformed_required_number_is_not_reported_as_missing() { assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1); } +/// A required whole number given as something other than a whole number of 0 or more (`"3"`, `2.5`, +/// `-1`, `true`, `[]`, `{}`) is refused as the wrong type, naming its key, not as a missing integer; one +/// left out or given as null is still missing. An optional one left out or null keeps its default. +/// Neither changes the document or records an undo step. +#[test] +fn a_malformed_required_integer_is_not_reported_as_missing() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("shape.rectangle", &json!({"x": 10, "y": 10, "width": 50, "height": 40})).unwrap(); + let doc = s.doc().unwrap().doc.clone(); + let undo = s.doc().unwrap().history.undo.len(); + let with = |base: &serde_json::Value, key: &str, v: Option| { + let mut p = base.clone(); + let o = p.as_object_mut().unwrap(); + match v { + Some(v) => o.insert(key.into(), v), + None => o.remove(key), + }; + p + }; + let refused = |s: &mut Session, cmd: &str, p: &serde_json::Value, want: &str| match s.execute(cmd, p) { + Err(EngineError::BadParams { cmd: c, msg }) => assert_eq!((c.as_str(), msg.as_str()), (cmd, want), "{p}"), + other => panic!("{cmd} {p}: {other:?}"), + }; + let ends = json!({"sourceDocument": 1, "grab": [0, 0], "at": [0, 0], "ids": [1]}); + let cases = [ + ("appearance.setItem", json!({"index": 0}), "index", "missing integer `index`"), + ("path.removeAnchor", json!({"id": 1, "anchor": 0}), "anchor", "missing integer `anchor`"), + ("path.split", json!({"id": 1, "anchor": 0}), "anchor", "need `anchor` or `segment` + `t`"), + ("addon.pen.removePoint", json!({"anchor": 0}), "anchor", "missing integer `anchor`"), + ("edit.dropSelection", ends, "sourceDocument", "missing integer `sourceDocument`"), + ("document.activate", json!({"index": 0}), "index", "missing index"), + ("document.move", json!({"to": 0}), "to", "missing `to`: a non-negative integer"), + ]; + let malformed = [json!("3"), json!(2.5), json!(-1), json!(true), json!([]), json!({})]; + for (cmd, base, key, missing) in &cases { + for v in &malformed { + refused(&mut s, cmd, &with(base, key, Some(v.clone())), &format!("`{key}` must be a whole number")); + } + for p in [with(base, key, None), with(base, key, Some(json!(null)))] { + refused(&mut s, cmd, &p, missing); + } + } + // An optional `subpath` given as anything else is refused the same way; left out or null, it is + // subpath 0 and the command goes on to read `anchor`. + let base = json!({"id": 1, "anchor": "3"}); + for v in &malformed { + refused(&mut s, "path.removeAnchor", &with(&base, "subpath", Some(v.clone())), "`subpath` must be a whole number"); + } + for p in [with(&base, "subpath", None), with(&base, "subpath", Some(json!(null)))] { + refused(&mut s, "path.removeAnchor", &p, "`anchor` must be a whole number"); + } + assert_eq!(s.doc().unwrap().doc, doc); + assert_eq!(s.doc().unwrap().history.undo.len(), undo); + // A whole JSON number is a whole number. + s.execute("document.activate", &json!({"index": 0})).unwrap(); +} + /// A command that takes the keys of one that isn't checked yet isn't checked either. #[test] fn a_reference_to_an_unchecked_command_is_unchecked() { diff --git a/docs/mcp.md b/docs/mcp.md index ef3ff788a5..26f1300e0f 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -564,7 +564,13 @@ key, and report a key left out or null as missing. The optional `group` of `live (`x` of `shape.rectangle`, `pos` of `guide.add`, `index`, `x` and `y` of the `object.mesh.*` and `object.envelope.setMeshPoint` point commands, and the others like them) refuses a value that is not a number with `` `x` must be a number, not "40" ``, naming the key and showing the value as given, and reports a key left out or null -as `` missing number `x` ``; it changes nothing either way. A command that leaves out an object selected inside a +as `` missing number `x` ``; it changes nothing either way. A command that needs a whole number of 0 or more (`index` +of `appearance.setItem`, `anchor` of `path.removeAnchor` and `addon.pen.removePoint`, `sourceDocument` of +`edit.dropSelection`, `index` of `document.activate`, `to` of `document.move`, and the others like them) refuses a +value such as `"3"`, `2.5`, `-1` or `true` with `` `index` must be a whole number ``, naming the key, and reports a key +left out or null as missing (such as `` missing integer `index` ``). An optional one, such as `subpath`, keeps its default when +left out or null; `appearance.setActiveItem` reads a null `index` as clearing the active item. +A command that leaves out an object selected inside a selected group also leaves it out when `ids` list it together with a group that holds it. Every command that takes `ids` (or `slices`, for the slice commands) refuses a list with an entry that is not a non-negative integer, or a value that is not a list, and changes nothing; it does not act on the other entries. The error for an entry names the key, @@ -5111,7 +5117,7 @@ path by its control points when one of them has an x or y that is infinite or la the canvas (coordinates out of range)". `path.curvatureEdit`, `path.removeAnchor`, `path.convertAnchor`, `path.reshapeSegment`, `path.split`, `path.setHandle`, `path.moveAnchor` and `path.insertAnchor` edit subpath 0 when `subpath` is absent or null, and refuse a `subpath` that -is not a whole number of 0 or more, such as 1.5, -1 or "1" ("missing integer `subpath`"); `path.split` refuses such +is not a whole number of 0 or more, such as 1.5, -1 or "1" ("`subpath` must be a whole number"); `path.split` refuses such an `anchor` or `segment` in the same way. The document does not change and no undo step is recorded. `path.moveAnchor` to the point the anchor is at, and `path.setHandle` to the point the handle is at on an anchor that keeps its kind, leave the document as it is and record no undo step. diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index f3707717e6..1dfaf846f5 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1086,7 +1086,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur A path segment with a control point whose x or y is infinite or larger in size than `f64::MAX / 16` is measured by its control points, so its bounds hold the whole curve and the canvas check refuses an edit that leaves such a path selected (`path.create` included). With Direct Selection, the Pen and the anchor tools, a group, compound path or clip group selected whole shows the anchors of its paths filled, as a path selected whole does; the direction handles still show only on a path that is selected itself. Public references: [Adjust anchor point, handle, and bounding box display size](https://web.archive.org/web/20251114182206/https://helpx.adobe.com/illustrator/desktop/draw-shapes-and-paths/learn-drawing-basics/adjust-anchor-point-handle-and-bounding-box-display-size.html) and [Drawing basics](https://web.archive.org/web/20231130064834/https://helpx.adobe.com/illustrator/using/drawing-basics.html) (archived copies). When the Add Anchor Point tool, the Pen's Auto Add, the Smart Pen's Alt-click on a segment or Alt-click with the Scissors adds an anchor to a path with direct-selected anchors or segments (`path.insertAnchor`), the same points stay selected: the anchors and segments after the new anchor are numbered one higher, and a selected segment that the new anchor splits stays selected as its two halves. - `path.curvatureEdit`, `path.removeAnchor`, `path.convertAnchor`, `path.reshapeSegment`, `path.split`, `path.setHandle`, `path.moveAnchor` and `path.insertAnchor` refuse a `subpath` that is not a whole number of 0 or more, and `path.split` refuses such an `anchor` or `segment`; without a `subpath` they edit subpath 0. + `path.curvatureEdit`, `path.removeAnchor`, `path.convertAnchor`, `path.reshapeSegment`, `path.split`, `path.setHandle`, `path.moveAnchor` and `path.insertAnchor` refuse a `subpath` that is not a whole number of 0 or more, and `path.split` refuses such an `anchor` or `segment`; without a `subpath` they edit subpath 0. A value given that is not a whole number, such as `"1"` or `1.5`, is refused with `` `subpath` must be a whole number ``, naming its key, as are a required index of the Appearance panel and Smart Pen commands, `sourceDocument` of `edit.dropSelection` and `index` and `to` of `document.activate` and `document.move`; a required one left out or null is reported as missing (`tests::params::a_malformed_required_integer_is_not_reported_as_missing`). `path.reshapeSegment` refuses a delta of 1e9 pt or longer, as `object.blend.spine.reshapeSegment` does, and `path.convertAnchor` and `path.freehand` refuse a point whose x or y is 1e9 pt or more in size. `path.reshapeSegment`, `object.blend.spine.reshapeSegment`, `path.convertAnchor`, `path.curvatureEdit`, `path.insertAnchor`, `path.moveAnchors`, `path.reshape`, `path.close` and the Pencil, Curvature, Knife, Eraser, Path Eraser, Smooth, Join and Blob Brush commands refuse a number, point or handle given as text, such as "10", and leave the document unchanged; `path.convertAnchor` refuses an `x` without a `y`, or a `y` without an `x`. `path.close` and `path.setHandle` (`independent`), `path.moveAnchors` (`patterns`), the Pencil (`closed`, `keepSelected`, `fill`), Curvature (`closed` and a point's `corner`), `path.curvatureEdit` (`corner`), `path.reshapeSegment` (`perpendicular`) and the Blob Brush (`keepSelected`, `mergeOnlySelection`, `merge`) refuse a value other than true, false or null, such as "true", and leave the document unchanged.