Form controls: a fixed input range is refused for a macro that takes no inputs

A button could be given a fixed input range for any macro name; pressing it then passed the range
to macro.run as `inputs`, which check_params refuses for a macro without inputs ("it takes no
inputs"), so such a button failed on every press. Now insert.button and control.assign refuse a
fixed input when the named macro exists and declares no inputs, naming the macro; a macro the
workbook does not have yet is not checked, and the assignment is kept as given. On open, a stored
fixed input for a macro without inputs becomes the selection: the file itself assigns the macro
on the selection, and the stored range would fail every press.

Tests: a_fixed_input_for_a_macro_without_inputs_is_refused_when_assigned (insert and assign are
refused with the reason in any letter case; a macro with an input and a macro not yet defined
take the range; a stored fixed input comes back as the selection). format_and_assign and
a_kept_button_action_comes_back_only_on_the_same_button give their macro an input.

crates/engine/src/cmd/controls.rs

@@ -404,6 +404,14 @@ pub(crate) fn take_requests(s: &mut Session, queued: usize, name: &str) -> Optio
     blocked
 }
 
+/// The stored name of the workbook's macro `name` when it exists and declares no inputs: a fixed
+/// input range would make every press fail (`macro.run` refuses inputs it doesn't take), so such
+/// an assignment is refused when it is made and dropped to the selection when a file brings one
+/// in. A macro the workbook doesn't have (yet) is not checked: the assignment is kept as given.
+fn takes_no_inputs(wb: &Workbook, name: &str) -> Option<String> {
+    crate::macros::find(wb, name).and_then(|i| wb.macros.get(i)).filter(|m| m.inputs.is_empty()).map(|m| m.name.clone())
+}
+
 /// A button action from `macro` (with `input` and `output`) or `command` (with `params`).
 fn action_param(s: &Session, cmd: &str, p: &Json) -> Result<Option<ControlAction>> {
     if let Some(name) = str_param(p, "macro").map(str::trim).filter(|n| !n.is_empty()) {
@@ -418,6 +426,9 @@ fn action_param(s: &Session, cmd: &str, p: &Json) -> Result<Option<ControlAction
                 if gridcraft_core::RangeRef::parse(body).is_none() {
                     return Err(bad(cmd, format!("`input` must be \"selection\" or a range such as B2:D9, not `{t}`")));
                 }
+                if let Some(stored) = takes_no_inputs(&s.doc()?.wb, name) {
+                    return Err(bad(cmd, format!("macro '{stored}' takes no inputs, so `input` must be \"selection\"")));
+                }
                 MacroInput::Fixed(t.to_string())
             }
         };
@@ -654,7 +665,8 @@ pub(crate) fn with_action_names(wb: &Workbook) -> Result<Option<Workbook>> {
 /// button's name and anchor cell, the button at the stored place must have that name and anchor
 /// cell (another application may have added, removed or reordered controls), and it must not
 /// have been given another macro since (a stored macro action comes back only on a button that
-/// still runs that macro, a stored command only on one the file gives no macro).
+/// still runs that macro, a stored command only on one the file gives no macro). A stored fixed
+/// input for a macro that takes no inputs becomes the selection (see [`takes_no_inputs`]).
 pub(crate) fn load_action_names(wb: &mut Workbook) {
     let Some(items) = stash::get_json(wb, ACTIONS_KEY) else { return };
     stash::remove(wb, ACTIONS_KEY);
@@ -681,6 +693,14 @@ pub(crate) fn load_action_names(wb: &mut Workbook) {
         if !valid || !wb.sheet(si).is_some_and(|sh| sh.controls.get(index).is_some_and(same)) {
             continue;
         }
+        let action = match action {
+            // The macro takes no inputs now: the stored range would fail every press, so the
+            // button runs the macro on the selection, as the file itself assigns it.
+            ControlAction::Macro { name, input: MacroInput::Fixed(_), output } if takes_no_inputs(wb, &name).is_some() => {
+                ControlAction::Macro { name, input: MacroInput::Selection, output }
+            }
+            other => other,
+        };
         if let Some(c) = wb.sheet_mut(si).and_then(|sh| sh.controls.get_mut(index)) {
             c.action = Some(action);
         }

crates/engine/src/tests/controls.rs

@@ -148,7 +148,8 @@ fn format_and_assign() {
     s.execute("control.format", json!({"id": c, "value": 25})).unwrap();
     assert_eq!(v(&s, "B1"), Value::Number(50.0), "without a link nothing is written");
     // Buttons: text and action.
-    s.execute("macro.save", json!({"name": "Fill", "steps": [{"command": "cell.set", "params": {"cell": "A1", "input": "x"}}]})).unwrap();
+    // A fixed input needs a macro that takes one.
+    s.execute("macro.save", json!({"name": "Fill", "steps": [{"command": "cell.set", "params": {"cell": "Input.cell(1,1)", "input": "x"}}], "inputs": [{"name": "Input", "kind": "range"}]})).unwrap();
     let b = id(&s.execute("insert.button", json!({"text": "Go", "macro": "Fill", "input": "C2:D4", "output": "F2"})).unwrap());
     let action = control(&s, b).action;
     assert_eq!(action, Some(ControlAction::Macro { name: "Fill".into(), input: MacroInput::Fixed("C2:D4".into()), output: Some("F2".into()) }));
@@ -380,7 +381,7 @@ fn reopen(s: &mut Session) {
 fn a_kept_button_action_comes_back_only_on_the_same_button() {
     use crate::cmd::controls::{load_action_names, with_action_names};
     let mut s = s();
-    s.execute("macro.save", json!({"name": "Mark", "steps": [{"command": "cell.set", "params": {"cell": "A1", "input": "m"}}]})).unwrap();
+    s.execute("macro.save", json!({"name": "Mark", "steps": [{"command": "cell.set", "params": {"cell": "Input.cell(1,1)", "input": "m"}}], "inputs": [{"name": "Input", "kind": "range"}]})).unwrap();
     s.execute("insert.button", json!({"at": "D2", "text": "Run", "command": "cell.set", "params": {"cell": "A9", "input": "cmd"}})).unwrap();
     s.execute("insert.button", json!({"at": "D6", "text": "Other"})).unwrap();
     s.execute("insert.button", json!({"at": "D9", "text": "Fixed", "macro": "Mark", "input": "F1:G2"})).unwrap();
@@ -467,6 +468,39 @@ fn a_kept_action_without_the_buttons_name_or_anchor_is_dropped() {
     assert!(matches!(kept(&items), Some(ControlAction::Command { .. })), "the writer's own entry still comes back");
 }
 
+/// A fixed input range for a macro that declares no inputs would fail every press (`macro.run`
+/// refuses inputs the macro doesn't take), so the assignment is refused when it is made, naming
+/// the macro; a macro the workbook doesn't have yet is not checked, and a macro with an input
+/// takes the range. A file that brings such a stored input in gets the macro on the selection.
+#[test]
+fn a_fixed_input_for_a_macro_without_inputs_is_refused_when_assigned() {
+    use crate::cmd::controls::{load_action_names, with_action_names};
+    let mut s = s();
+    s.execute("macro.save", json!({"name": "Fill", "steps": [{"command": "cell.set", "params": {"cell": "A1", "input": "x"}}]})).unwrap();
+    s.execute("macro.save", json!({"name": "Mark", "steps": [{"command": "cell.set", "params": {"cell": "Input.cell(1,1)", "input": "m"}}], "inputs": [{"name": "Input", "kind": "range"}]})).unwrap();
+    let want = "macro 'Fill' takes no inputs";
+    assert!(err(s.execute("insert.button", json!({"at": "B2", "macro": "Fill", "input": "C2:D4"}))).contains(want));
+    assert_eq!(s.doc().unwrap().wb.sheet(0).unwrap().controls.len(), 0, "no button was placed");
+    let b = id(&s.execute("insert.button", json!({"at": "B2", "macro": "Fill", "output": "F2"})).unwrap());
+    let e = err(s.execute("control.assign", json!({"id": b, "macro": "fill", "input": "C2:D4"})));
+    assert!(e.contains(want), "any letter case: {e}");
+    assert_eq!(control(&s, b).action, Some(ControlAction::Macro { name: "Fill".into(), input: MacroInput::Selection, output: Some("F2".into()) }));
+    s.execute("control.assign", json!({"id": b, "macro": "Mark", "input": "C2:D4"})).unwrap();
+    s.execute("control.assign", json!({"id": b, "macro": "Later", "input": "C2:D4"})).unwrap();
+    assert_eq!(control(&s, b).action, Some(ControlAction::Macro { name: "Later".into(), input: MacroInput::Fixed("C2:D4".into()), output: None }));
+    // A file's stored fixed input for Fill (another version assigned it) comes back as the selection.
+    s.execute("control.assign", json!({"id": b, "macro": "Fill", "output": "F2"})).unwrap();
+    let mut wb = (*s.doc().unwrap().wb).clone();
+    wb.sheet_mut(0).unwrap().controls[0].action =
+        Some(ControlAction::Macro { name: "Fill".into(), input: MacroInput::Fixed("C2:D4".into()), output: Some("F2".into()) });
+    let mut stored = with_action_names(&wb).unwrap().unwrap();
+    load_action_names(&mut stored);
+    assert_eq!(
+        stored.sheets[0].controls[0].action,
+        Some(ControlAction::Macro { name: "Fill".into(), input: MacroInput::Selection, output: Some("F2".into()) })
+    );
+}
+
 /// The saved xlsx bytes of `s`'s workbook.
 fn saved_xlsx(s: &mut Session) -> Vec<u8> {
     let r = s.execute("file.saveBytes", json!({"format": "xlsx"})).unwrap();