MCP: an integer given as a string or a fraction is refused as the wrong type, not reported as missing

A command that needs a whole number of 0 or more, such as `index` of
`appearance.setItem`, `anchor` of `path.removeAnchor` and
`addon.pen.removePoint`, `sourceDocument` of `edit.dropSelection`, `index`
of `document.activate` and `to` of `document.move`, answered "missing
integer `index`" (or "missing index") both when the key was left out and
when it was given as a value that is not a whole number of 0 or more, such
as "3", 2.5, -1 or true. A value that is given but is not one is now
refused with "`index` must be a whole number", the message object ids
already use. A key left out or given as null is still missing, and an
optional index such as `subpath` keeps its default. Neither error changes
the document or records an undo step.

Docs: docs/mcp.md, docs/target-app-parity.md.
This commit is contained in:
dotCooCoo
2026-11-13 18:02:42 -08:00
parent 71cef8ac8d
commit 78ce56d021
7 changed files with 91 additions and 13 deletions
+9
View File
@@ -440,6 +440,15 @@ pub(crate) fn f64_req(p: &Value, key: &str, cmd: &str) -> Result<f64> {
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<u64> {
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<f64> {
+3 -3
View File
@@ -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<usize> {
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<Value> {
+2 -2
View File
@@ -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<usize> {
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
+10 -5
View File
@@ -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<Value> {
}
fn doc_activate(s: &mut Session, p: &Value) -> Result<Value> {
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<Value> {
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<Value> {
/// Document ids scope node ids: two tabs may contain objects with the same node id.
fn drop_selection(s: &mut Session, p: &Value) -> Result<Value> {
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")?;
+58
View File
@@ -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<serde_json::Value>| {
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() {
+8 -2
View File
@@ -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.
+1 -1
View File
@@ -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.