From f98b086c5f017b2198f14285e2248049bca7f5a7 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sun, 1 Nov 2026 08:23:40 -0800 Subject: [PATCH] Variables: Rename reports a missing variable or data set before a taken name variable.rename and dataset.rename checked whether newName was taken before checking that name exists, so renaming a missing name onto a taken one said the new name already exists. Both now report the missing name first. --- crates/engine/src/cmd/variablecmds.rs | 6 ++++++ crates/engine/src/tests/variables.rs | 14 ++++++++++++++ 2 files changed, 20 insertions(+) diff --git a/crates/engine/src/cmd/variablecmds.rs b/crates/engine/src/cmd/variablecmds.rs index 116149ec7..da1eb33a7 100644 --- a/crates/engine/src/cmd/variablecmds.rs +++ b/crates/engine/src/cmd/variablecmds.rs @@ -160,6 +160,9 @@ fn rename(s: &mut Session, p: &Value) -> Result { let from = var_name(p, C)?; let to = named(p.get("newName").and_then(Value::as_str), "newName", C)?; let st = s.doc()?; + if st.doc.variables.variable(&from).is_none() { + return Err(bad(C, format!("no variable named `{from}`"))); + } if st.doc.variables.variable(&to).is_some() { return Err(bad(C, format!("variable `{to}` already exists"))); } @@ -357,6 +360,9 @@ fn dataset_rename(s: &mut Session, p: &Value) -> Result { let from = var_name(p, C)?; let to = named(p.get("newName").and_then(Value::as_str), "newName", C)?; let st = s.doc()?; + if st.doc.variables.dataset(&from).is_none() { + return Err(bad(C, format!("no dataset named `{from}`"))); + } if st.doc.variables.dataset(&to).is_some() { return Err(bad(C, format!("dataset `{to}` already exists"))); } diff --git a/crates/engine/src/tests/variables.rs b/crates/engine/src/tests/variables.rs index bdacd0e1b..fd502d1b8 100644 --- a/crates/engine/src/tests/variables.rs +++ b/crates/engine/src/tests/variables.rs @@ -652,6 +652,20 @@ fn renaming_carries_the_bindings_and_values_with_it() { assert!(s.execute("dataset.rename", &json!({"name": "Title", "newName": "Taken"})).is_err()); } +/// Renaming a name that does not exist onto one that does reports the missing name, the same +/// error a rename to a free name gives. +#[test] +fn renaming_a_missing_name_onto_a_taken_one_reports_the_missing_name() { + let mut s = session(); + s.execute("variable.define", &json!({"name": "A", "kind": "text"})).unwrap(); + s.execute("dataset.new", &json!({"name": "A"})).unwrap(); + + let e = s.execute("variable.rename", &json!({"name": "Nope", "newName": "A"})).unwrap_err().to_string(); + assert!(e.contains("no variable named `Nope`"), "{e}"); + let e = s.execute("dataset.rename", &json!({"name": "Nope", "newName": "A"})).unwrap_err().to_string(); + assert!(e.contains("no dataset named `Nope`"), "{e}"); +} + /// `dataset.set` is how a value changes after the row exists: replacing it must not need the /// dataset to be deleted and rebuilt (which loses its place in the list and the active one). #[test]