fix(engine): report invalid index on document.inspect and document.activate (#1557)

crates/engine/src/adjust_cmds.rs

@@ -204,7 +204,11 @@ fn has_pixels(s: &Session) -> std::result::Result<(), String> {
     }
     let d = s.active().ok_or("no document open")?;
     let l = d.active_layer.and_then(|id| d.doc.layer(id)).ok_or("no active layer")?;
-    if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name())) }
+    if matches!(l.content, LayerContent::Raster(_)) {
+        Ok(())
+    } else {
+        Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name()))
+    }
 }
 
 fn has_doc(s: &Session) -> std::result::Result<(), String> {

crates/engine/src/commands.rs

@@ -39,7 +39,11 @@ fn has_layer(s: &Session) -> std::result::Result<(), String> {
 }
 fn has_pixel_layer(s: &Session) -> std::result::Result<(), String> {
     let l = crate::active_layer_of(s)?;
-    if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name())) }
+    if matches!(l.content, LayerContent::Raster(_)) {
+        Ok(())
+    } else {
+        Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name()))
+    }
 }
 /// A pixel layer, or a targeted alpha channel / Quick Mask (adjustments and fills apply to it).
 fn has_pixel_or_channel(s: &Session) -> std::result::Result<(), String> {
@@ -868,8 +872,13 @@ fn build() -> Vec<CommandSpec> {
             params: r##"{"document":index?}"##,
             enabled: has_doc,
             run: |s, p| {
-                let i = p.get("document").and_then(Value::as_u64).map(|v| v as usize).or(s.active_index()).ok_or(EngineError::NoDocument)?;
-                let d = s.documents().get(i).ok_or(EngineError::NoDocument)?;
+                let d = match p.get("document") {
+                    Some(v) => {
+                        let i = v.as_u64().and_then(|v| usize::try_from(v).ok()).ok_or_else(|| bad("document.inspect", "`document` must be an index"))?;
+                        s.documents().get(i).ok_or_else(|| EngineError::Other(format!("no document at index {i}")))?
+                    }
+                    None => s.active().ok_or(EngineError::NoDocument)?,
+                };
                 Ok(inspect::document(d))
             },
             journal: false,
@@ -883,7 +892,8 @@ fn build() -> Vec<CommandSpec> {
             enabled: has_doc,
             run: |s, p| {
                 let i = p.get("document").and_then(Value::as_u64).ok_or_else(|| bad("document.activate", "missing `document`"))?;
-                if s.set_active(i as usize) { Ok(Value::Null) } else { Err(EngineError::NoDocument) }
+                let idx = usize::try_from(i).map_err(|_| bad("document.activate", "`document` out of range"))?;
+                if s.set_active(idx) { Ok(Value::Null) } else { Err(EngineError::Other(format!("no document at index {idx}"))) }
             },
             journal: false,
         },

crates/engine/src/eraser_cmds.rs

@@ -47,7 +47,11 @@ fn enum_param<T: serde::de::DeserializeOwned>(p: &Value, cmd: &str, k: &str, def
 fn has_pixel_layer(s: &Session) -> std::result::Result<(), String> {
     let d = s.active().ok_or("no document open")?;
     let l = d.active_layer.and_then(|id| d.doc.layer(id)).ok_or("no active layer")?;
-    if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name())) }
+    if matches!(l.content, LayerContent::Raster(_)) {
+        Ok(())
+    } else {
+        Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name()))
+    }
 }
 
 fn active_pixel_layer(s: &Session, cmd: &str) -> Result<LayerId> {

crates/engine/src/layer_menu_cmds.rs

@@ -38,7 +38,11 @@ fn has_layer(s: &Session) -> Enabled {
 
 fn has_raster(s: &Session) -> Enabled {
     let l = active_layer(s)?;
-    if matches!(l.content, LayerContent::Raster(_)) { Ok(()) } else { Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name())) }
+    if matches!(l.content, LayerContent::Raster(_)) {
+        Ok(())
+    } else {
+        Err(format!("active layer is {} {} layer, not a pixel layer", l.content.article(), l.content.kind_name()))
+    }
 }
 
 fn has_raster_with_mask(s: &Session) -> Enabled {

crates/engine/src/smartselect_cmds.rs

@@ -188,9 +188,9 @@ fn refine_edge(s: &mut Session, p: &Value) -> Result<Value> {
         _ => {
             let id = active.ok_or_else(|| EngineError::Other("no active layer".into()))?;
             let src_layer = d.doc.layer(id).ok_or(EngineError::NoLayer(id))?;
-            let src = src_layer
-                .surface()
-                .ok_or_else(|| EngineError::Other(format!("the active layer is {} {} layer without pixels", src_layer.content.article(), src_layer.content.kind_name())))?;
+            let src = src_layer.surface().ok_or_else(|| {
+                EngineError::Other(format!("the active layer is {} {} layer without pixels", src_layer.content.article(), src_layer.content.kind_name()))
+            })?;
             let name = d.doc.copy_name(&src_layer.name);
             let empty = Region { bbox: Rect::EMPTY, mask: Vec::new() };
             let reg = region.as_ref().unwrap_or(&empty);

crates/engine/src/tests.rs

@@ -1105,3 +1105,18 @@ fn adjustment_layer_disabled_diagnostics_use_grammatical_article() {
     assert!(!err_clear.contains("a Adjustment layer"), "found 'a Adjustment layer': {err_clear}");
 }
 
+#[test]
+fn document_inspect_and_activate_invalid_index() {
+    let mut s = Session::new();
+    s.execute("file.new", json!({"width": 2, "height": 2, "name": "first"})).unwrap();
+    s.execute("file.new", json!({"width": 2, "height": 2, "name": "second"})).unwrap();
+
+    assert!(s.execute("document.inspect", json!({})).is_ok());
+    assert!(s.execute("document.inspect", json!({"document": 0})).is_ok());
+
+    let err = s.execute("document.inspect", json!({"document": 9})).unwrap_err();
+    assert_eq!(err.to_string(), "no document at index 9");
+
+    let err_act = s.execute("document.activate", json!({"document": 9})).unwrap_err();
+    assert_eq!(err_act.to_string(), "no document at index 9");
+}

crates/engine/src/trap_cmds.rs

@@ -27,7 +27,10 @@ fn trap(s: &mut Session, p: &Value) -> Result<Value> {
         let bounds = doc.bounds();
         let l = doc.layer_mut(id).ok_or(EngineError::NoLayer(id))?;
         let LayerContent::Raster(surf) = &mut l.content else {
-            return Err(EngineError::BadParams { cmd: cmd.into(), msg: format!("Trap needs a pixel layer (active layer is {} {})", l.content.article(), l.content.kind_name()) });
+            return Err(EngineError::BadParams {
+                cmd: cmd.into(),
+                msg: format!("Trap needs a pixel layer (active layer is {} {})", l.content.article(), l.content.kind_name()),
+            });
         };
         if bounds.is_empty() {
             return Ok(());