MCP: a required number given as a string is refused as the wrong type, not reported as missing

A command that needs a number, such as `x` of `shape.rectangle`, `intensity`
of `edit.colors.saturate` or `index` of `object.envelope.setMeshPoint`,
answered "missing number `x`" both when the key was left out and when it was
given as a value that is not a number, such as "40". A value that is given but
is not a number is now refused with "`x` must be a number, not "40"", the
message the optional number checks already use, showing the value as given.
A key left out or given as null is still a missing number. Neither error
changes the document or records an undo step.

Docs: docs/mcp.md, docs/target-app-parity.md.

crates/engine/src/cmd.rs

@@ -426,8 +426,13 @@ pub(crate) fn bad(cmd: &str, msg: impl Into<String>) -> EngineError {
 pub(crate) fn f64_or(p: &Value, key: &str, default: f64) -> f64 {
     p.get(key).and_then(Value::as_f64).unwrap_or(default)
 }
+/// The number `p` gives for `key`. A missing key or null is "missing number `{key}`"; any other
+/// value, such as `"40"`, is "`{key}` must be a number, not {v}", as [`number_or`] words it.
 pub(crate) fn f64_req(p: &Value, key: &str, cmd: &str) -> Result<f64> {
-    p.get(key).and_then(Value::as_f64).ok_or_else(|| bad(cmd, format!("missing number `{key}`")))
+    match p.get(key) {
+        None | Some(Value::Null) => Err(bad(cmd, format!("missing number `{key}`"))),
+        Some(v) => v.as_f64().ok_or_else(|| bad(cmd, format!("`{key}` must be a number, not {v}"))),
+    }
 }
 /// 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}".

crates/engine/src/tests/editcolors.rs

@@ -77,10 +77,12 @@ fn saturate_needs_a_number_intensity() {
     let mut s = session();
     let a = rect(&mut s, json!("#cc6666"));
     let undo = s.doc().unwrap().history.undo.len();
-    for p in [json!({}), json!({"intensity": "40"}), json!({"intensity": null})] {
+    for p in [json!({}), json!({"intensity": null})] {
         let e = s.execute("edit.colors.saturate", &p).unwrap_err().to_string();
         assert!(e.contains("missing number `intensity`"), "{p}: {e}");
     }
+    let e = s.execute("edit.colors.saturate", &json!({"intensity": "40"})).unwrap_err().to_string();
+    assert!(e.contains("`intensity` must be a number, not \"40\""), "{e}");
     assert_eq!(fill_of(&s, a).to_hex(), "#cc6666");
     assert_eq!(s.doc().unwrap().history.undo.len(), undo, "no undo step");
     s.execute("edit.colors.saturate", &json!({"intensity": 100})).unwrap();

crates/engine/src/tests/params.rs

@@ -484,6 +484,57 @@ fn a_malformed_object_id_is_not_reported_as_missing() {
     assert_eq!(undo(&s), before);
 }
 
+/// A required number given as something other than a number (`"40"`, `true`, `[]`, `{}`) is refused
+/// as the wrong type, naming its key and showing the value as given, not as a missing number; one left
+/// out or given as null is still a missing number. Neither changes the document or records an undo step.
+#[test]
+fn a_malformed_required_number_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 cases = [
+        ("shape.rectangle", json!({"x": 0, "y": 0, "width": 10, "height": 10}), "x"),
+        ("edit.colors.saturate", json!({"intensity": 40}), "intensity"),
+        ("object.envelope.setMeshPoint", json!({"id": 1, "index": 0, "x": 0, "y": 0}), "index"),
+        ("object.envelope.setMeshPoint", json!({"id": 1, "index": 0, "x": 0, "y": 0}), "x"),
+        ("object.mesh.movePoint", json!({"id": 1, "index": 0, "x": 0, "y": 0}), "y"),
+        ("guide.add", json!({"pos": 40}), "pos"),
+    ];
+    for (cmd, base, key) in &cases {
+        for (v, shown) in [(json!("40"), "\"40\""), (json!(true), "true"), (json!([]), "[]"), (json!({}), "{}")] {
+            let p = with(base, key, Some(v));
+            match s.execute(cmd, &p) {
+                Err(EngineError::BadParams { cmd: c, msg }) => {
+                    assert_eq!((c.as_str(), msg), (*cmd, format!("`{key}` must be a number, not {shown}")), "{p}");
+                }
+                other => panic!("{cmd} {p}: {other:?}"),
+            }
+        }
+        for p in [with(base, key, None), with(base, key, Some(json!(null)))] {
+            match s.execute(cmd, &p) {
+                Err(EngineError::BadParams { cmd: c, msg }) => assert_eq!((c.as_str(), msg), (*cmd, format!("missing number `{key}`")), "{p}"),
+                other => panic!("{cmd} {p}: {other:?}"),
+            }
+        }
+    }
+    assert_eq!(s.doc().unwrap().doc, doc);
+    assert_eq!(s.doc().unwrap().history.undo.len(), undo);
+    // A whole JSON number is a number for a fractional parameter.
+    s.execute("guide.add", &json!({"pos": 40})).unwrap();
+    assert_eq!(s.doc().unwrap().history.undo.len(), undo + 1);
+}
+
 /// 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() {