From f05bbfeea2722287e3405932c5d614b54f1f0281 Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Sun, 4 Oct 2026 11:39:33 -0400 Subject: [PATCH] Never crash: engine, MCP and tools Commands return errors instead of expect()/unreachable!() (book, library, undo/redo, paste, find/replace, Pathfinder, tables, QR, select, swatches, anchors, articles, transform again, XML import). The sample document builder propagates errors. Commands that add a key to their parameters no longer panic when a caller (control channel, MCP, script) passes an array or a string, and corrupt crash-recovery metadata is ignored. engine, mcp and tools now deny unwrap/expect/panic/unimplemented/todo/unreachable. Co-Authored-By: Claude Opus 5.5 --- clippy.toml | 4 ++ crates/engine/src/cmd/anchors.rs | 5 ++- crates/engine/src/cmd/articles.rs | 5 ++- crates/engine/src/cmd/book.rs | 18 +++++--- crates/engine/src/cmd/buttons.rs | 4 +- crates/engine/src/cmd/edit.rs | 9 ++-- crates/engine/src/cmd/endnotes.rs | 7 ++- crates/engine/src/cmd/export.rs | 4 +- crates/engine/src/cmd/find.rs | 4 +- crates/engine/src/cmd/library.rs | 18 ++++---- crates/engine/src/cmd/mod.rs | 7 +++ crates/engine/src/cmd/object.rs | 17 +++----- crates/engine/src/cmd/paths.rs | 15 ++++--- crates/engine/src/cmd/place_text.rs | 10 +++-- crates/engine/src/cmd/qr.rs | 4 +- crates/engine/src/cmd/select.rs | 9 ++-- crates/engine/src/cmd/style.rs | 7 ++- crates/engine/src/cmd/table.rs | 26 +++++------ crates/engine/src/cmd/xml.rs | 2 +- crates/engine/src/lib.rs | 3 +- crates/engine/src/recovery.rs | 23 +++++++++- crates/engine/src/sample.rs | 68 ++++++++++++++++------------- crates/engine/src/tests.rs | 12 +++++ crates/mcp/src/lib.rs | 1 + crates/tools/src/lib.rs | 1 + 25 files changed, 175 insertions(+), 108 deletions(-) create mode 100644 clippy.toml diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 00000000..4b6c79f5 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,4 @@ +allow-unwrap-in-tests = true +allow-expect-in-tests = true +allow-panic-in-tests = true +allow-indexing-slicing-in-tests = true diff --git a/crates/engine/src/cmd/anchors.rs b/crates/engine/src/cmd/anchors.rs index 6427450b..c5015c7c 100644 --- a/crates/engine/src/cmd/anchors.rs +++ b/crates/engine/src/cmd/anchors.rs @@ -88,8 +88,9 @@ fn erase_or_smooth(s: &mut Session, p: &Value, smooth: bool) -> Result { }; let on: Vec<(usize, f64)> = hits.iter().filter(|h| h.0 == si).map(|h| (h.1, h.2)).collect(); let k = |a: &(usize, f64)| a.0 as f64 + a.1; - let a = *on.iter().min_by(|x, y| k(x).total_cmp(&k(y))).expect("hits"); - let b = *on.iter().max_by(|x, y| k(x).total_cmp(&k(y))).expect("hits"); + let (Some(&a), Some(&b)) = (on.iter().min_by(|x, y| k(x).total_cmp(&k(y))), on.iter().max_by(|x, y| k(x).total_cmp(&k(y)))) else { + return Err(bad(cmd, "the drag didn't touch the path")); + }; let sp = it.path.subpaths[si].clone(); if smooth { it.path.subpaths[si] = designcraft_geom::edit_path::smooth(&sp, a, b, tol * scale); diff --git a/crates/engine/src/cmd/articles.rs b/crates/engine/src/cmd/articles.rs index bd3710df..1749889c 100644 --- a/crates/engine/src/cmd/articles.rs +++ b/crates/engine/src/cmd/articles.rs @@ -20,7 +20,10 @@ pub fn specs() -> Vec { let name = match want { Some(n) if d.articles.iter().any(|a| a.name == n) => return Err(bad("article.new", format!("an article named `{n}` exists"))), Some(n) => n, - None => (1..).map(|i| format!("Article {i}")).find(|n| !d.articles.iter().any(|a| a.name == *n)).expect("free"), + None => (1..=d.articles.len() + 1) + .map(|i| format!("Article {i}")) + .find(|n| !d.articles.iter().any(|a| a.name == *n)) + .ok_or_else(|| bad("article.new", "no free article name"))?, }; d.articles.push(Article { name: name.clone(), items: ids.into_iter().filter(|i| d.item(*i).is_some()).collect(), export: true }); Ok(json!({"name": name})) diff --git a/crates/engine/src/cmd/book.rs b/crates/engine/src/cmd/book.rs index 542a9c70..df4bfc9a 100644 --- a/crates/engine/src/cmd/book.rs +++ b/crates/engine/src/cmd/book.rs @@ -21,6 +21,10 @@ fn has_book(s: &Session) -> std::result::Result<(), String> { if s.book.is_some() { Ok(()) } else { Err("no book open".into()) } } +fn no_book() -> EngineError { + bad("book", "no book open") +} + #[cfg(not(target_arch = "wasm32"))] fn save(b: &Book) -> Result<()> { let bytes = serde_json::to_vec_pretty(b).map_err(|e| EngineError::Other(e.to_string()))?; @@ -104,7 +108,7 @@ pub fn specs() -> Vec { cmd!(noundo "book.add", "Add Document", [], None, "{path, at?: index}", has_book, |s, p| { let path = str_param(p, "path").ok_or_else(|| bad("book.add", "`path` required"))?.to_string(); load(&path)?; - let b = s.book.as_mut().expect("enabled"); + let b = s.book.as_mut().ok_or_else(no_book)?; let at = p.get("at").and_then(Value::as_u64).map_or(b.documents.len(), |i| (i as usize).min(b.documents.len())); b.documents.insert(at, path); save(b)?; @@ -112,7 +116,7 @@ pub fn specs() -> Vec { }), cmd!(noundo "book.remove", "Remove Document", [], None, "{index}", has_book, |s, p| { let i = p.get("index").and_then(Value::as_u64).ok_or_else(|| bad("book.remove", "`index` required"))? as usize; - let b = s.book.as_mut().expect("enabled"); + let b = s.book.as_mut().ok_or_else(no_book)?; if i >= b.documents.len() { return Err(bad("book.remove", format!("no document {i}"))); } @@ -125,7 +129,7 @@ pub fn specs() -> Vec { }), cmd!(noundo "book.styleSource", "Style Source", [], None, "{index}", has_book, |s, p| { let i = p.get("index").and_then(Value::as_u64).unwrap_or(0) as usize; - let b = s.book.as_mut().expect("enabled"); + let b = s.book.as_mut().ok_or_else(no_book)?; if i >= b.documents.len() { return Err(bad("book.styleSource", format!("no document {i}"))); } @@ -134,7 +138,7 @@ pub fn specs() -> Vec { ok() }), cmd!(query "book.list", "Book", [], None, "{} → {path, styleSource, documents: [{path, pages, firstPage}]}", has_book, |s, _| { - let b = s.book.as_ref().expect("enabled"); + let b = s.book.as_ref().ok_or_else(no_book)?; let mut first = 1; let mut docs = Vec::new(); for p in &b.documents { @@ -145,7 +149,7 @@ pub fn specs() -> Vec { Ok(json!({"path": b.path, "styleSource": b.style_source, "documents": docs})) }), cmd!(noundo "book.paginate", "Update Numbering", [], None, "{} — each document starts numbering where the previous one ended (files are saved)", has_book, |s, _| { - let b = s.book.clone().expect("enabled"); + let b = s.book.clone().ok_or_else(no_book)?; let mut next = 1u32; for p in &b.documents { let mut d = load(p)?; @@ -156,7 +160,7 @@ pub fn specs() -> Vec { Ok(json!({"pages": next - 1})) }), cmd!(noundo "book.syncStyles", "Synchronize Book", [], None, "{} — paragraph and character styles and swatches of the style source go into every document (by name)", has_book, |s, _| { - let b = s.book.clone().expect("enabled"); + let b = s.book.clone().ok_or_else(no_book)?; let src_path = b.documents.get(b.style_source).ok_or_else(|| bad("book.syncStyles", "the book is empty"))?; let src = load(src_path)?; let mut n = 0; @@ -189,7 +193,7 @@ pub fn specs() -> Vec { Ok(json!({"synced": n})) }), cmd!(noundo "book.exportPdf", "Export Book to PDF…", [], None, "{path?} — every document in order as one PDF → {path, bytes, pages} (no path: {base64, …})", has_book, |s, p| { - let b = s.book.clone().expect("enabled"); + let b = s.book.clone().ok_or_else(no_book)?; let mut parts = Vec::new(); for path in &b.documents { let d = load(path)?; diff --git a/crates/engine/src/cmd/buttons.rs b/crates/engine/src/cmd/buttons.rs index ef4301ce..5a644503 100644 --- a/crates/engine/src/cmd/buttons.rs +++ b/crates/engine/src/cmd/buttons.rs @@ -46,9 +46,7 @@ pub fn specs() -> Vec { } ), cmd!("button.clear", "Convert to Object", ["Object", "Interactive"], None, "{ids?}", has_selection, |s, p| { - let mut q = p.clone(); - q["action"] = json!("none"); - s.execute("button.set", &q) + s.execute("button.set", &super::with_param(p, "action", json!("none"))) }), cmd!( "form.set", diff --git a/crates/engine/src/cmd/edit.rs b/crates/engine/src/cmd/edit.rs index 7f3a4548..e7190acb 100644 --- a/crates/engine/src/cmd/edit.rs +++ b/crates/engine/src/cmd/edit.rs @@ -105,7 +105,7 @@ pub fn specs() -> Vec { vec![ cmd!(noundo "edit.undo", "Undo", ["Edit"], Some("Cmd+Z"), "{}", can_undo, |s, _| { let st = s.doc_mut()?; - let e = st.history.undo.pop().expect("checked"); + let e = st.history.undo.pop().ok_or_else(|| super::bad("edit.undo", "nothing to undo"))?; st.history.redo.push(HistoryEntry { label: e.label.clone(), doc: st.doc.clone(), selection: st.selection.clone() }); st.doc = e.doc; st.selection = e.selection; @@ -114,7 +114,7 @@ pub fn specs() -> Vec { }), cmd!(noundo "edit.redo", "Redo", ["Edit"], Some("Cmd+Shift+Z"), "{}", can_redo, |s, _| { let st = s.doc_mut()?; - let e = st.history.redo.pop().expect("checked"); + let e = st.history.redo.pop().ok_or_else(|| super::bad("edit.redo", "nothing to redo"))?; st.history.undo.push(HistoryEntry { label: e.label.clone(), doc: st.doc.clone(), selection: st.selection.clone() }); st.doc = e.doc; st.selection = e.selection; @@ -233,10 +233,9 @@ pub fn specs() -> Vec { if s.doc()?.selection.text.is_some() { return paste_text(s, super::str_param(p, "text").map(str::to_string), true); } - if s.clipboard.is_none() { + let Some(clip) = s.clipboard.clone() else { return Err(super::bad("edit.paste", "clipboard is empty")); - } - let clip = s.clipboard.clone().expect("checked"); + }; let off = if bool_or(p, "inPlace", false) { 0.0 } else { 12.0 }; let ids: Vec = clip.spreads.first().map(|sp| sp.items.iter().map(|i| i.id).collect()).unwrap_or_default(); s.edit(|d, sel| { diff --git a/crates/engine/src/cmd/endnotes.rs b/crates/engine/src/cmd/endnotes.rs index e97fc45d..bdf9ca69 100644 --- a/crates/engine/src/cmd/endnotes.rs +++ b/crates/engine/src/cmd/endnotes.rs @@ -34,7 +34,12 @@ pub fn specs() -> Vec { s.edit(|d, _| { let st = d.story_mut(sid).ok_or_else(|| bad("endnote.delete", "no such story"))?; let k = st.endnotes.iter().position(|n| n.id == id).ok_or_else(|| bad("endnote.delete", format!("no endnote {id}")))?; - let at = st.text.match_indices(designcraft_doc::ENDNOTE_REF).nth(k).map(|(i, _)| i).expect("one reference per endnote"); + let at = st + .text + .match_indices(designcraft_doc::ENDNOTE_REF) + .nth(k) + .map(|(i, _)| i) + .ok_or_else(|| bad("endnote.delete", format!("endnote {id} has no reference in the text")))?; st.delete(at..at + designcraft_doc::ENDNOTE_REF.len_utf8()); ok() }) diff --git a/crates/engine/src/cmd/export.rs b/crates/engine/src/cmd/export.rs index 47921614..3d2d6b3c 100644 --- a/crates/engine/src/cmd/export.rs +++ b/crates/engine/src/cmd/export.rs @@ -17,9 +17,7 @@ pub fn specs() -> Vec { cmd!(noundo "file.exportFixedEpub", "Export EPUB (Fixed Layout)…", ["File"], None, "{path?, title?, author?, language?} — pre-paginated EPUB → {path, bytes} (no path: {base64, bytes})", has_doc, |s, p| { - let mut q = p.clone(); - q["fixedLayout"] = serde_json::json!(true); - export_epub(s, &q) + export_epub(s, &super::with_param(p, "fixedLayout", serde_json::json!(true))) }), cmd!(noundo "file.printBooklet", "Print Booklet…", ["File"], None, "{path?, type?: saddleStitch|twoUpConsecutive, spaceBetween? (pt)} — printer spreads as PDF (pages imposed in booklet order) → {path, bytes, sheets} (no path: {base64, …})", diff --git a/crates/engine/src/cmd/find.rs b/crates/engine/src/cmd/find.rs index 27c30405..12c09dff 100644 --- a/crates/engine/src/cmd/find.rs +++ b/crates/engine/src/cmd/find.rs @@ -188,7 +188,7 @@ fn change(s: &mut Session, p: &Value) -> Result { let text = story.text[r.clone()].to_string(); let mut edits: Vec<(std::ops::Range, String)> = Vec::new(); for caps in re.captures_iter(&text) { - let m = caps.get(0).expect("match"); + let Some(m) = caps.get(0) else { continue }; let mut out = String::new(); if grep { caps.expand(&rep, &mut out); @@ -200,7 +200,7 @@ fn change(s: &mut Session, p: &Value) -> Result { break; } } - let st = d.story_mut(sid).expect("story"); + let Some(st) = d.story_mut(sid) else { continue }; for (range, new) in edits.iter().rev() { st.replace(range.clone(), new); if !over.is_empty() && !new.is_empty() { diff --git a/crates/engine/src/cmd/library.rs b/crates/engine/src/cmd/library.rs index 465d79f7..3a1b40be 100644 --- a/crates/engine/src/cmd/library.rs +++ b/crates/engine/src/cmd/library.rs @@ -39,6 +39,10 @@ fn has_library(s: &Session) -> std::result::Result<(), String> { if s.library.is_some() { Ok(()) } else { Err("no library open".into()) } } +fn no_library() -> crate::EngineError { + bad("library", "no library open") +} + pub fn specs() -> Vec { vec![ cmd!(noundo "library.new", "New Library", ["File", "New"], None, "{path?} — an empty Object Library (saved to `path` when given)", always, |s, p| { @@ -66,11 +70,11 @@ pub fn specs() -> Vec { Ok(json!({"items": n})) }), cmd!(query "library.list", "Library Items", [], None, "{} → [{index, name, description}]", has_library, |s, _| { - let lib = s.library.as_ref().expect("enabled"); + let lib = s.library.as_ref().ok_or_else(no_library)?; Ok(Value::Array(lib.items.iter().enumerate().map(|(i, it)| json!({"index": i, "name": it.name, "description": it.description})).collect())) }), cmd!(query "library.json", "Library Contents", [], None, "{} → the library as JSON text (to save where there's no file system)", has_library, |s, _| { - Ok(json!({"json": serde_json::to_string(s.library.as_ref().expect("enabled")).unwrap_or_default()})) + Ok(json!({"json": serde_json::to_string(s.library.as_ref().ok_or_else(no_library)?).unwrap_or_default()})) }), cmd!(noundo "library.add", "Add Item", [], None, "{name?, description?} — the selection → {index}", has_selection, |s, p| { has_library(s).map_err(|e| bad("library.add", e))?; @@ -82,7 +86,7 @@ pub fn specs() -> Vec { let n = st.selection.items.len(); (str_param(p, "name").map(str::to_string).unwrap_or_else(|| if n > 1 { format!("{n} objects") } else { kind.clone() }), str_param(p, "description").unwrap_or("").to_string()) }; - let lib = s.library.as_mut().expect("checked"); + let lib = s.library.as_mut().ok_or_else(no_library)?; lib.items.push(LibraryItem { name, description, snippet: super::file::base64_encode(&snip) }); lib.save()?; Ok(json!({"index": lib.items.len() - 1})) @@ -98,13 +102,12 @@ pub fn specs() -> Vec { let lib = s.library.as_ref().ok_or_else(|| bad("library.place", "no library open"))?; let i = p.get("index").and_then(Value::as_u64).ok_or_else(|| bad("library.place", "`index` required"))? as usize; let it = lib.items.get(i).ok_or_else(|| bad("library.place", format!("no item {i}")))?; - let mut q = p.clone(); - q["base64"] = json!(it.snippet); + let q = super::with_param(p, "base64", json!(it.snippet)); super::file::snippet_place(s, &q) } ), cmd!(noundo "library.remove", "Delete Item", [], None, "{index}", has_library, |s, p| { - let lib = s.library.as_mut().expect("enabled"); + let lib = s.library.as_mut().ok_or_else(no_library)?; let i = p.get("index").and_then(Value::as_u64).ok_or_else(|| bad("library.remove", "`index` required"))? as usize; if i >= lib.items.len() { return Err(bad("library.remove", format!("no item {i}"))); @@ -139,8 +142,7 @@ pub fn specs() -> Vec { |s, p| { let i = p.get("index").and_then(Value::as_u64).unwrap_or(0) as usize; let (_, snip) = s.conveyor.get(i).cloned().ok_or_else(|| bad("conveyor.place", "the conveyor is empty"))?; - let mut q = p.clone(); - q["base64"] = json!(super::file::base64_encode(&snip)); + let q = super::with_param(p, "base64", json!(super::file::base64_encode(&snip))); let r = super::file::snippet_place(s, &q)?; if !p.get("keep").and_then(Value::as_bool).unwrap_or(false) { s.conveyor.remove(i); diff --git a/crates/engine/src/cmd/mod.rs b/crates/engine/src/cmd/mod.rs index 97a2d1ee..85e47551 100644 --- a/crates/engine/src/cmd/mod.rs +++ b/crates/engine/src/cmd/mod.rs @@ -214,6 +214,13 @@ pub fn find_command(id: &str) -> Option<&'static CommandSpec> { pub(crate) fn bad(cmd: &str, msg: impl Into) -> EngineError { EngineError::BadParams { cmd: cmd.into(), msg: msg.into() } } +/// `p` with `key` set to `v`. Parameters come from callers (control channel, MCP, scripts) and +/// may not be an object: `p[key] = v` panics on an array or a string, so start from `{}` then. +pub(crate) fn with_param(p: &Value, key: &str, v: Value) -> Value { + let mut m = p.as_object().cloned().unwrap_or_default(); + m.insert(key.to_string(), v); + Value::Object(m) +} pub(crate) fn f64_or(p: &Value, key: &str, default: f64) -> f64 { p.get(key).and_then(Value::as_f64).unwrap_or(default) } diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 3f07a895..53915d94 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -626,9 +626,7 @@ pub fn specs() -> Vec { for c in 0..cols { let x0 = rect.x0 + c as f64 * (cw + gutter); let y0 = rect.y0 + r as f64 * (ch + gutter); - let mut q = p.clone(); - q["rect"] = json!([x0, y0, x0 + cw, y0 + ch]); - q["caret"] = json!(false); + let q = super::with_param(&super::with_param(p, "rect", json!([x0, y0, x0 + cw, y0 + ch])), "caret", json!(false)); ids.push(frame_create(s, &q)?["id"].clone()); } } @@ -1071,15 +1069,14 @@ fn transform_again(s: &mut Session, p: &Value) -> Result { if list.is_empty() { return Err(bad("transform.again", "no transform to repeat")); } - let steps: Vec<(String, Value)> = if bool_or(p, "sequence", false) { list.clone() } else { vec![list.last().cloned().expect("non-empty")] }; + let steps: Vec<(String, Value)> = if bool_or(p, "sequence", false) { list.clone() } else { list.last().cloned().into_iter().collect() }; let mut groups: Vec> = if bool_or(p, "individually", false) { ids.iter().map(|i| vec![*i]).collect() } else { vec![ids.clone()] }; // Run the steps directly (no recording, one undo step for the whole repeat). let saved = s.transforms.clone(); for (cmd, params) in &steps { let spec = super::find_command(cmd).ok_or_else(|| bad("transform.again", "unknown transform"))?; for g in &groups { - let mut q = params.clone(); - q["ids"] = json!(g.iter().map(|i| i.0).collect::>()); + let q = super::with_param(params, "ids", json!(g.iter().map(|i| i.0).collect::>())); (spec.run)(s, &q)?; } // Moving a copy selects the copies: later steps (and the next repeat) act on them. @@ -1192,7 +1189,7 @@ fn transform_values(s: &mut Session, p: &Value, ids: &[ItemId], rf: usize) -> Re let delta = designcraft_geom::decompose::compose(&t) * lin(cur).inverse(); // Δ in measuring space about `a`, brought into the item's container space. let m = parent.inverse() * Affine::translate(a.to_vec2()) * delta * Affine::translate(-a.to_vec2()) * parent; - let it = d.item_mut(*id).expect("found"); + let it = d.item_mut(*id).ok_or(designcraft_doc::DocError::NoItem(*id))?; it.xf = m * it.xf; // Scale: relative to what is shown (frames show 100% — their scale lives in the geometry). if sx.is_some() || sy.is_some() { @@ -1281,7 +1278,7 @@ fn arrange(s: &mut Session, p: &Value) -> Result { if loc.path.len() != 1 { continue; } - let sp = d.spread_mut(loc.spread).expect("found"); + let Some(sp) = d.spread_mut(loc.spread) else { continue }; let i = loc.path[0]; let n = sp.items.len(); let it = sp.items.remove(i); @@ -1333,7 +1330,7 @@ fn ungroup(s: &mut Session, p: &Value) -> Result { if loc.path.len() != 1 { continue; } - let sp = d.spread_mut(loc.spread).expect("found"); + let Some(sp) = d.spread_mut(loc.spread) else { continue }; sp.items.remove(loc.path[0]); for (k, c) in items.iter().enumerate() { let mut c = (**c).clone(); @@ -1711,7 +1708,7 @@ impl RemoveKeep for Document { if loc.path.len() != 1 { return Err(bad("object", "nested items can't be detached")); } - let sp = self.spread_mut(loc.spread).expect("found"); + let sp = self.spread_mut(loc.spread).ok_or(designcraft_doc::DocError::NoItem(id))?; Ok(Arc::unwrap_or_clone(sp.items.remove(loc.path[0]))) } } diff --git a/crates/engine/src/cmd/paths.rs b/crates/engine/src/cmd/paths.rs index 207f559e..700bfa5d 100644 --- a/crates/engine/src/cmd/paths.rs +++ b/crates/engine/src/cmd/paths.rs @@ -145,7 +145,7 @@ fn pathfinder(s: &mut Session, p: &Value) -> Result { if order.len() < 2 { return Err(bad("object.pathfinder", "select two or more shapes on one spread (not text frames or groups)")); } - let keep = if op == Op::MinusBack { *order.last().expect("two") } else { order[0] }; + let keep = if op == Op::MinusBack { order[order.len() - 1] } else { order[0] }; let shapes: Vec = order.iter().filter_map(|id| spread_xf(d, *id).and_then(|(_, xf)| Some(d.item(*id)?.path.transformed(xf)))).collect(); let Some(result) = combine(op, &shapes) else { return Err(bad("object.pathfinder", "the shapes don't overlap: nothing is left")) }; @@ -257,13 +257,14 @@ fn create_outlines(s: &mut Session, p: &Value) -> Result { let skew = if st.skew != 0.0 { Affine::new([1.0, 0.0, -st.skew.to_radians().tan(), 1.0, 0.0, 0.0]) } else { Affine::IDENTITY }; let a = Affine::translate((g.x, l.baseline + g.y)) * skew * Affine::scale_non_uniform(g.sx, g.sy); let key = (st.fill.clone(), st.fill_tint, st.stroke.clone(), st.stroke_tint, st.stroke_weight); - let bp = match runs.iter_mut().find(|r| r.0 == key) { - Some(r) => &mut r.1, + let k = match runs.iter().position(|r| r.0 == key) { + Some(k) => k, None => { runs.push((key, BezPath::new())); - &mut runs.last_mut().expect("pushed").1 + runs.len() - 1 } }; + let bp = &mut runs[k].1; bp.extend((a * outline.as_ref().clone()).elements().iter().copied()); } } @@ -285,8 +286,10 @@ fn create_outlines(s: &mut Session, p: &Value) -> Result { }; let mut parts: Vec = runs.into_iter().map(|r| make(d, r)).collect(); d.remove_item(*id)?; - let new = if parts.len() == 1 { - parts.pop().expect("one") + let new = if parts.len() == 1 + && let Some(one) = parts.pop() + { + one } else { let mut g = Item::new(ItemId(d.alloc()), it.layer, Shape::Group, PathData::default()); for p in &mut parts { diff --git a/crates/engine/src/cmd/place_text.rs b/crates/engine/src/cmd/place_text.rs index 1c784974..39805bfb 100644 --- a/crates/engine/src/cmd/place_text.rs +++ b/crates/engine/src/cmd/place_text.rs @@ -28,12 +28,14 @@ fn map_styles(d: &Document, imp: &mut designcraft_textimport::Imported, map: &st if conflict == Conflict::AutoRename { let st = &d.styles; for s in imp.para_styles.iter().filter(|s| !map.contains_key(&s.name) && st.para(&s.name).is_some()) { - let n = (1..).map(|i| format!("{}_wrd_{i}", s.name)).find(|n| st.para(n).is_none()).expect("free name"); - rename.insert(s.name.clone(), n); + if let Some(n) = (1..=st.paragraph.len() + 1).map(|i| format!("{}_wrd_{i}", s.name)).find(|n| st.para(n).is_none()) { + rename.insert(s.name.clone(), n); + } } for s in imp.char_styles.iter().filter(|s| !map.contains_key(&s.name) && st.char_style(&s.name).is_some()) { - let n = (1..).map(|i| format!("{}_wrd_{i}", s.name)).find(|n| st.char_style(n).is_none()).expect("free name"); - rename.insert(s.name.clone(), n); + if let Some(n) = (1..=st.character.len() + 1).map(|i| format!("{}_wrd_{i}", s.name)).find(|n| st.char_style(n).is_none()) { + rename.insert(s.name.clone(), n); + } } } if rename.is_empty() { diff --git a/crates/engine/src/cmd/qr.rs b/crates/engine/src/cmd/qr.rs index 77b0b707..c201de4e 100644 --- a/crates/engine/src/cmd/qr.rs +++ b/crates/engine/src/cmd/qr.rs @@ -83,7 +83,7 @@ fn qr_code(s: &mut Session, p: &Value) -> Result { let quiet = side / (n + 4) as f64 * 2.0; let r = Rect::from_center_size(b.center(), (side + 2.0 * quiet, side + 2.0 * quiet)); let fresh = qr_item(t, it.layer, &text, r, &color).map_err(|e| bad(ID, e))?; - let it = d.item_mut(t).expect("found"); + let it = d.item_mut(t).ok_or(designcraft_doc::DocError::NoItem(t))?; it.path = fresh.path; it.fill = fresh.fill; it.alt_text = fresh.alt_text; @@ -95,7 +95,7 @@ fn qr_code(s: &mut Session, p: &Value) -> Result { } let id = ItemId(d.alloc()); let code = qr_item(id, it.layer, &text, it.inner_bounds(), &color).map_err(|e| bad(ID, e))?; - let f = d.item_mut(t).expect("found"); + let f = d.item_mut(t).ok_or(designcraft_doc::DocError::NoItem(t))?; f.content = Content::Group { items: vec![std::sync::Arc::new(code)] }; t } diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index 38722676..0d09ac86 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -117,10 +117,13 @@ fn select_sibling(s: &mut Session, dir: isize) -> Result { let Some(parent) = st.doc.item_at(&pl) else { return ok() }; let kids = parent.children(); let n = kids.len() as isize; - let i = *l.path.last().expect("nested") as isize; + let Some(&i) = l.path.last() else { return ok() }; + if n == 0 { + return ok(); + } // InDesign's "next" goes down the stack (towards the back), wrapping. - let j = (i - dir).rem_euclid(n) as usize; - let next = kids[j].id; + let j = (i as isize - dir).rem_euclid(n) as usize; + let Some(next) = kids.get(j).map(|k| k.id) else { return ok() }; st.selection = Selection::items(vec![next]); st.revision += 1; Ok(json!({"id": next.0})) diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index c3e1330b..d1fccebf 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -254,7 +254,10 @@ pub fn specs() -> Vec { let name = match want { Some(n) if taken(&n) => return Err(bad("swatch.newColorGroup", format!("a group named `{n}` exists"))), Some(n) => n, - None => (1..).map(|i| format!("Color Group {i}")).find(|n| !taken(n)).expect("free name"), + None => (1..=d.color_groups.len() + 1) + .map(|i| format!("Color Group {i}")) + .find(|n| !taken(n)) + .ok_or_else(|| bad("swatch.newColorGroup", "no free group name"))?, }; for g in &mut d.color_groups { g.swatches.retain(|w| !names.contains(w)); @@ -876,7 +879,7 @@ fn add_to_swatches(s: &mut Session, p: &Value) -> Result { if d.swatches.iter().any(|w| w.name == new) { return Err(bad("swatch.addToSwatches", format!("a swatch named `{new}` exists"))); } - let w = d.swatches.iter_mut().find(|w| w.name == name).expect("found above"); + let w = d.swatches.iter_mut().find(|w| w.name == name).ok_or_else(|| bad("swatch.addToSwatches", format!("no swatch `{name}`")))?; w.name = new.clone(); w.named = true; for g in &mut d.color_groups { diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index 11fa1dc8..fff2102d 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -214,19 +214,13 @@ fn table_specs() -> Vec { ), cmd!(noundo "table.select", "Select Cells", [], None, "{story?, table?, rows?: [a,b], cols?: [a,b], what?: cell|row|column|table}", in_table_or_ids, select), cmd!(noundo "table.selectTable", "Select Table", ["Table", "Select"], None, "{}", in_table, |s, p| { - let mut p = p.clone(); - p["what"] = json!("table"); - select(s, &p) + select(s, &super::with_param(p, "what", json!("table"))) }), cmd!(noundo "table.selectRow", "Select Row", ["Table", "Select"], None, "{}", in_table, |s, p| { - let mut p = p.clone(); - p["what"] = json!("row"); - select(s, &p) + select(s, &super::with_param(p, "what", json!("row"))) }), cmd!(noundo "table.selectColumn", "Select Column", ["Table", "Select"], None, "{}", in_table, |s, p| { - let mut p = p.clone(); - p["what"] = json!("column"); - select(s, &p) + select(s, &super::with_param(p, "what", json!("column"))) }), cmd!(noundo "table.nextCell", "Next Cell", [], None, "{}", in_cell, |s, _| step_cell(s, true)), cmd!(noundo "table.prevCell", "Previous Cell", [], None, "{}", in_cell, |s, _| step_cell(s, false)), @@ -238,7 +232,7 @@ fn table_specs() -> Vec { fn in_table(s: &Session) -> std::result::Result<(), String> { has_doc(s)?; - let st = s.active().expect("doc"); + let st = s.active().ok_or("no document open")?; if st.selection.cells.is_some() || st.selection.text.is_some_and(|t| t.cell.is_some()) { return Ok(()); } @@ -636,7 +630,7 @@ fn set_cell(s: &mut Session, p: &Value) -> Result { if owners[r * nc + c] != (r, c) { continue; } - let cell = t.cell_mut(r, c).expect("in range"); + let Some(cell) = t.cell_mut(r, c) else { continue }; if let Some(f) = &fill { cell.fill = f.clone(); } @@ -870,10 +864,10 @@ fn get(s: &mut Session, p: &Value) -> Result { let cells: Vec = (0..t.nrows()) .flat_map(|r| (0..t.ncols()).map(move |c| (r, c))) .filter(|&(r, c)| owners[r * t.ncols() + c] == (r, c)) - .map(|(r, c)| { - let cell = t.cell(r, c).expect("in range"); - json!({"row": r, "col": c, "rowSpan": cell.row_span, "colSpan": cell.col_span, "text": cell.text.text, "fill": cell.fill, - "insets": cell.insets, "vj": cell.vj, "style": cell.style}) + .filter_map(|(r, c)| { + let cell = t.cell(r, c)?; + Some(json!({"row": r, "col": c, "rowSpan": cell.row_span, "colSpan": cell.col_span, "text": cell.text.text, "fill": cell.fill, + "insets": cell.insets, "vj": cell.vj, "style": cell.style})) }) .collect(); Ok(json!({ @@ -1244,7 +1238,7 @@ fn place_graphic(s: &mut Session, p: &Value) -> Result { let (nw, nh) = (pw as f64, ph as f64); let k = if fill { (bw / nw).max(bh / nh) } else { (bw / nw).min(bh / nh) }; let xf = designcraft_geom::Affine::translate(((bw - nw * k) / 2.0, (bh - nh * k) / 2.0)) * designcraft_geom::Affine::scale(k); - let cell = t.cell_mut(r, c).expect("exists"); + let cell = t.cell_mut(r, c).ok_or_else(|| bad(ID, format!("no cell {r},{c}")))?; cell.graphic = Some(designcraft_doc::Graphic { asset: aid, size: (nw, nh), diff --git a/crates/engine/src/cmd/xml.rs b/crates/engine/src/cmd/xml.rs index 3c73f30f..32252a9d 100644 --- a/crates/engine/src/cmd/xml.rs +++ b/crates/engine/src/cmd/xml.rs @@ -152,7 +152,7 @@ fn parse(xml: &str) -> std::result::Result, String> { _ => String::new(), }, }, - _ => unreachable!(), + _ => String::new(), }; match (depth, child.as_mut(), out.last_mut()) { (3, Some(c), _) => c.1.push_str(&s), diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index 106577ed..8bda2df5 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -4,6 +4,7 @@ //! `layout.pages.insert`…) and JSON parameters. The egui UI, the CLI, the control channel and MCP //! all go through [`Session::execute`]. Tools (pointer gestures) are hosted here and reduce to //! commands, so every gesture is journaled and replayable. +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] #![forbid(unsafe_code)] pub mod cmd; @@ -407,7 +408,7 @@ impl Session { if story.frames.len() < 2 { break; } - let last = *story.frames.last().expect("frames"); + let Some(&last) = story.frames.last() else { break }; let cs = designcraft_compose::compose_story(&d, sid, &Default::default()); if cs.frame(last).is_some_and(|ft| !ft.range.is_empty()) { break; diff --git a/crates/engine/src/recovery.rs b/crates/engine/src/recovery.rs index d71b4c18..8b24afe6 100644 --- a/crates/engine/src/recovery.rs +++ b/crates/engine/src/recovery.rs @@ -67,7 +67,12 @@ pub fn list(dir: &Path) -> Vec<(u64, Value)> { continue; } let Some(uid) = p.file_stem().and_then(|s| s.to_str()).and_then(|s| s.parse::().ok()) else { continue }; - let meta = std::fs::read_to_string(p.with_extension("json")).ok().and_then(|t| serde_json::from_str(&t).ok()).unwrap_or(json!({})); + // Metadata is a file on disk: anything but an object (corrupt, hand-edited) counts as none. + let meta = std::fs::read_to_string(p.with_extension("json")) + .ok() + .and_then(|t| serde_json::from_str::(&t).ok()) + .filter(Value::is_object) + .unwrap_or(json!({})); out.push((uid, meta)); } out.sort_by_key(|e| e.0); @@ -133,6 +138,22 @@ mod tests { mod file_tests { use super::*; + /// `file.recovery.list` adds `uid` to each entry's metadata: metadata that wasn't an object + /// (a corrupt `.json` beside the recovery file) panicked on `m["uid"] = …`. + #[test] + fn corrupt_recovery_metadata_is_ignored() { + let dir = std::env::temp_dir().join(format!("dc-recovery-corrupt-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("7.designcraft"), b"not a document").unwrap(); + std::fs::write(dir.join("7.json"), b"[1, 2]").unwrap(); + let mut s = Session::new(); + s.recovery_dir = Some(dir.clone()); + let r = s.execute("file.recovery.list", &json!({})).unwrap(); + assert_eq!(r[0]["uid"], json!(7)); + let _ = std::fs::remove_dir_all(&dir); + } + #[test] fn revert_and_save_a_copy() { let dir = std::env::temp_dir().join(format!("dc-revert-{}", std::process::id())); diff --git a/crates/engine/src/sample.rs b/crates/engine/src/sample.rs index 4b67f78d..15a05d08 100644 --- a/crates/engine/src/sample.rs +++ b/crates/engine/src/sample.rs @@ -72,7 +72,7 @@ fn image(d: &mut Document, name: &str, w: u32, h: u32, hue: f32) -> AssetId { id } -fn graphic_frame(d: &mut Document, sr: SpreadRef, r: Rect, asset: AssetId, px: (u32, u32)) -> ItemId { +fn graphic_frame(d: &mut Document, sr: SpreadRef, r: Rect, asset: AssetId, px: (u32, u32)) -> designcraft_doc::Result { let id = ItemId(d.alloc()); let lid = d.default_layer(); let mut it = Item::new(id, lid, Shape::Rectangle, shapes::rectangle(r)); @@ -87,16 +87,23 @@ fn graphic_frame(d: &mut Document, sr: SpreadRef, r: Rect, asset: AssetId, px: ( crop: [0.0; 4], }); it.object_style = designcraft_doc::BASIC_GRAPHICS_FRAME.into(); - d.insert_item(sr, it, None).expect("spread"); - id + d.insert_item(sr, it, None)?; + Ok(id) } -fn text(d: &mut Document, sr: SpreadRef, r: Rect, t: &str, style: &str) -> (ItemId, designcraft_doc::StoryId) { +fn text(d: &mut Document, sr: SpreadRef, r: Rect, t: &str, style: &str) -> designcraft_doc::Result<(ItemId, designcraft_doc::StoryId)> { let lid = d.default_layer(); - d.add_text_frame(sr, r, lid, t, ParaFormat { style: style.into(), ..Default::default() }).expect("spread") + d.add_text_frame(sr, r, lid, t, ParaFormat { style: style.into(), ..Default::default() }) } pub fn magazine() -> Document { + build().unwrap_or_else(|e| { + log::error!("sample document: {e}"); + Document::new(&NewDocument::default()) + }) +} + +fn build() -> designcraft_doc::Result { let mut d = Document::new(&NewDocument { title: "Quarterly — Spring Issue".into(), pages: 4, @@ -251,9 +258,10 @@ pub fn magazine() -> Document { // Parent A: folio and running head. let pa = SpreadRef::Parent(0); { - let (_, s) = text(&mut d, pa, Rect::new(42.0, 750.0, 300.0, 762.0), &format!("{} · QUARTERLY", story::PAGE_NUMBER), "Folio"); + let (_, s) = text(&mut d, pa, Rect::new(42.0, 750.0, 300.0, 762.0), &format!("{} · QUARTERLY", story::PAGE_NUMBER), "Folio")?; let _ = s; - let (_, _) = text(&mut d, pa, Rect::new(w + 312.0, 750.0, w + w - 42.0, 762.0), &format!("SPRING ISSUE · {}", story::PAGE_NUMBER), "Folio"); + let (_, _) = + text(&mut d, pa, Rect::new(w + 312.0, 750.0, w + w - 42.0, 762.0), &format!("SPRING ISSUE · {}", story::PAGE_NUMBER), "Folio")?; if let Some(st) = d.story_mut(designcraft_doc::StoryId(d.next_id - 1)) { st.format_paras(0..0, |p| p.para.align = Some(Align::Right)); } @@ -262,20 +270,20 @@ pub fn magazine() -> Document { let id = ItemId(d.alloc()); let mut l = Item::new(id, lid, Shape::GraphicLine, shapes::line((x, 742.0).into(), (x + w - 96.0, 742.0).into())); l.stroke = Stroke { weight: 0.5, swatch: "[Black]".into(), tint: 0.4, ..Stroke::default() }; - d.insert_item(pa, l, None).expect("parent"); + d.insert_item(pa, l, None)?; } } // Page 1: cover (spread 0, single right page, no parent). let _ = d.apply_parent(&[0], None); let cover_art = image(&mut d, "dusk-cover.png", 1400, 1812, 0.0); - graphic_frame(&mut d, SpreadRef::Doc(0), Rect::new(-9.0, -9.0, w + 9.0, h + 9.0), cover_art, (1400, 1812)); - let (_, _) = text(&mut d, SpreadRef::Doc(0), Rect::new(42.0, 60.0, 570.0, 120.0), "THE SPRING ISSUE · NO. 01", "Kicker"); + graphic_frame(&mut d, SpreadRef::Doc(0), Rect::new(-9.0, -9.0, w + 9.0, h + 9.0), cover_art, (1400, 1812))?; + let (_, _) = text(&mut d, SpreadRef::Doc(0), Rect::new(42.0, 60.0, 570.0, 120.0), "THE SPRING ISSUE · NO. 01", "Kicker")?; if let Some(st) = d.stories.values().last().map(|s| s.id) && let Some(st) = d.story_mut(st) { st.format_chars(0..st.len(), |f| f.over.fill = Some("[Paper]".into())); } - let (_, cs) = text(&mut d, SpreadRef::Doc(0), Rect::new(42.0, 420.0, 570.0, 698.0), "The Quiet\nArt of Layout", "Cover Title"); + let (_, cs) = text(&mut d, SpreadRef::Doc(0), Rect::new(42.0, 420.0, 570.0, 698.0), "The Quiet\nArt of Layout", "Cover Title")?; let _ = cs; text( &mut d, @@ -283,13 +291,13 @@ pub fn magazine() -> Document { Rect::new(42.0, 700.0, 420.0, 760.0), "Grids, type and color: how great pages are made — and the tools that make them.", "Cover Deck", - ); + )?; // Spread 1 (pages 2–3). let s1 = SpreadRef::Doc(1); let lx = 42.0; // left page outside margin - text(&mut d, s1, Rect::new(lx, 54.0, w - 54.0, 70.0), "Feature · Design", "Kicker"); - let (_, hs) = text(&mut d, s1, Rect::new(lx, 80.0, w - 54.0, 190.0), "Notes on the Grid", "Headline"); + text(&mut d, s1, Rect::new(lx, 54.0, w - 54.0, 70.0), "Feature · Design", "Kicker")?; + let (_, hs) = text(&mut d, s1, Rect::new(lx, 80.0, w - 54.0, 190.0), "Notes on the Grid", "Headline")?; let _ = hs; text( &mut d, @@ -297,9 +305,9 @@ pub fn magazine() -> Document { Rect::new(lx, 182.0, w - 120.0, 240.0), "A good layout is invisible. Here is what is going on underneath the page — from the baseline grid to the paragraph composer.", "Deck", - ); + )?; let art2 = image(&mut d, "hills.png", 1200, 760, 0.6); - let pic = graphic_frame(&mut d, s1, Rect::new(lx, 252.0, w - 54.0, 252.0 + 300.0), art2, (1200, 760)); + let pic = graphic_frame(&mut d, s1, Rect::new(lx, 252.0, w - 54.0, 252.0 + 300.0), art2, (1200, 760))?; let _ = pic; text( &mut d, @@ -307,19 +315,19 @@ pub fn magazine() -> Document { Rect::new(lx, 558.0, w - 54.0, 572.0), "Evening light over layered hills — generated entirely in code for this sample.", "Caption", - ); + )?; // Body: threaded through 2 columns on the left page and 3 columns on the right page. - let (f1, body) = text(&mut d, s1, Rect::new(lx, 584.0, w - 54.0, 738.0), BODY, "Body"); + let (f1, body) = text(&mut d, s1, Rect::new(lx, 584.0, w - 54.0, 738.0), BODY, "Body")?; if let Some(it) = d.item_mut(f1).and_then(Item::text_frame_mut) { it.options.columns = 2; it.options.gutter = 14.0; } - let (f2, _) = text(&mut d, s1, Rect::new(w + 54.0, 54.0, 2.0 * w - 42.0, 708.0), "", "Body"); + let (f2, _) = text(&mut d, s1, Rect::new(w + 54.0, 54.0, 2.0 * w - 42.0, 708.0), "", "Body")?; if let Some(it) = d.item_mut(f2).and_then(Item::text_frame_mut) { it.options.columns = 3; it.options.gutter = 14.0; } - d.thread(f1, f2).expect("thread"); + d.thread(f1, f2)?; // First paragraph: Body First + accent lead-in. if let Some(st) = d.story_mut(body) { st.paras[0].style = "Body First".into(); @@ -344,7 +352,7 @@ pub fn magazine() -> Document { Rect::new(w + 54.0 + 178.0, 300.0, 2.0 * w - 42.0, 420.0), "“The best layouts disappear. What remains is the story.”", "Pull Quote", - ); + )?; if let Some(it) = d.item_mut(pq) { it.fill = Fill::swatch("Paper Warm"); it.wrap.mode = WrapMode::BoundingBox; @@ -361,17 +369,17 @@ pub fn magazine() -> Document { let id = ItemId(d.alloc()); let mut bar = Item::new(id, lid, Shape::Rectangle, shapes::rectangle(Rect::new(w + 54.0, 600.0 + 120.0, 2.0 * w - 42.0, 600.0 + 126.0))); bar.fill = Fill { swatch: "Dusk Gradient".into(), ..Fill::none() }; - d.insert_item(s1, bar, None).expect("spread"); + d.insert_item(s1, bar, None)?; // Page 4: color page. let s2 = SpreadRef::Doc(2); let id = ItemId(d.alloc()); let mut bg = Item::new(id, lid, Shape::Rectangle, shapes::rectangle(Rect::new(-9.0, -9.0, w + 9.0, 396.0))); bg.fill = Fill::swatch("Ink Plum"); - d.insert_item(s2, bg, None).expect("spread"); - let (_, k) = text(&mut d, s2, Rect::new(42.0, 80.0, 400.0, 100.0), "Coming next", "Kicker"); + d.insert_item(s2, bg, None)?; + let (_, k) = text(&mut d, s2, Rect::new(42.0, 80.0, 400.0, 100.0), "Coming next", "Kicker")?; let _ = k; - let (t4, _) = text(&mut d, s2, Rect::new(42.0, 110.0, 560.0, 330.0), "Color, Ink & Paper", "Cover Title"); + let (t4, _) = text(&mut d, s2, Rect::new(42.0, 110.0, 560.0, 330.0), "Color, Ink & Paper", "Cover Title")?; if let Some(sid) = d.item(t4).and_then(|i| i.text_frame()).map(|t| t.story) && let Some(st) = d.story_mut(sid) { @@ -386,10 +394,10 @@ pub fn magazine() -> Document { if *sw == "Paper Warm" { c.stroke = Stroke { weight: 0.5, ..Stroke::default() }; } - d.insert_item(s2, c, None).expect("spread"); - text(&mut d, s2, Rect::new(x, 566.0, x + 116.0, 580.0), sw, "Caption"); + d.insert_item(s2, c, None)?; + text(&mut d, s2, Rect::new(x, 566.0, x + 116.0, 580.0), sw, "Caption")?; } - let (f4, _) = text(&mut d, s2, Rect::new(42.0, 610.0, 570.0, 730.0), &BODY[..BODY.find('\n').unwrap_or(BODY.len())], "Body First"); + let (f4, _) = text(&mut d, s2, Rect::new(42.0, 610.0, 570.0, 730.0), &BODY[..BODY.find('\n').unwrap_or(BODY.len())], "Body First")?; if let Some(it) = d.item_mut(f4).and_then(Item::text_frame_mut) { it.options.columns = 2; } @@ -411,7 +419,7 @@ pub fn magazine() -> Document { for (r, row) in rows.iter().enumerate() { let style = if r == 0 { "Table Head" } else { "Table Body" }; for (c, txt) in row.iter().enumerate() { - let cell = t.cell_mut(r, c).expect("cell"); + let Some(cell) = t.cell_mut(r, c) else { continue }; *cell = designcraft_doc::Cell::with_text(txt, ParaFormat { style: style.into(), ..Default::default() }); cell.insets = [4.0, 6.0, 4.0, 6.0]; cell.vj = VerticalJustification::Center; @@ -439,5 +447,5 @@ pub fn magazine() -> Document { st.format_paras(anchor..anchor, |p| p.para.start_paragraph = Some(designcraft_doc::StartParagraph::NextColumn)); } debug_assert!(d.check().is_ok(), "{:?}", d.check()); - d + Ok(d) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 0a510983..72f6a6b9 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -443,3 +443,15 @@ fn gap_tool_drag_moves_the_gap() { assert_eq!(b.x1, 220.0, "{b:?}"); assert_eq!(s.doc().unwrap().history.undo.len(), 2, "create + one gap move"); } + +/// Parameters reach commands from the control channel, MCP and scripts as any JSON value. +/// Commands that add a key to their parameters (`q["fixedLayout"] = …`) panicked on an array. +#[test] +fn commands_take_non_object_params_without_panicking() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + for p in [json!([1, 2]), json!("text"), json!(3), Value::Null] { + let r = s.execute("file.exportFixedEpub", &p); + assert!(r.is_ok(), "{p}: {r:?}"); + } +} diff --git a/crates/mcp/src/lib.rs b/crates/mcp/src/lib.rs index 01cfed73..be55a518 100644 --- a/crates/mcp/src/lib.rs +++ b/crates/mcp/src/lib.rs @@ -12,6 +12,7 @@ //! agents can build layouts and look at them without a window. //! //! Entry points: [`Server::serve`] (stdio loop) and [`Server::handle_line`] (one message). +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] #![forbid(unsafe_code)] mod backend; diff --git a/crates/tools/src/lib.rs b/crates/tools/src/lib.rs index 3bf504c3..0b22a51d 100644 --- a/crates/tools/src/lib.rs +++ b/crates/tools/src/lib.rs @@ -6,6 +6,7 @@ //! replayable by the control channel and MCP, and tools are testable without a UI. //! //! Pointer positions are **canvas** coordinates (see [`layout::CanvasLayout`]). +#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] #![forbid(unsafe_code)] mod anchors;