Never crash: engine without expect() (job polling, commands, menus)

Removes the engine's production expect() calls: background-job polling (camera, mask, roto,
point and warp tracking, render queue) uses let-else / if-let instead of take().expect("job"),
command helpers bind what they checked instead of re-reading it with expect, the menu bar falls
back to an empty tree (logged) instead of expect()ing that a constant parses, and
autotrace/text animator group lookups no longer expect a just-inserted group. perf.rs, the
benchmark harness behind `effectcraft-cli bench --ops`, opts out with a module-level allow.
Test-only unreachable!() calls become panic!() (clippy.toml allows panics in tests). Denies the
panic lints in the crate.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Brandon ThomasandClaude Opus 5.5 committed 2026-10-04 13:40:46 -04:00
1 parent 10644a83aa
commit 3d5432c318
18 files changed
+57 -48

No files matched your search

+4
View File
@@ -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
+1 -1
View File
@@ -382,7 +382,7 @@ impl Session {
if !job.is_finished() {
return false;
}
let mut job = self.camera_job.take().expect("job");
let Some(mut job) = self.camera_job.take() else { return false };
if let Some(t) = job.thread.take() {
let _ = t.join();
}
+9 -7
View File
@@ -162,7 +162,7 @@ fn run(s: &mut Session, p: &Value) -> Result<Value> {
let base = masks.children.len();
for k in 0..n_masks {
// Depth decides the mode: outlines add, holes subtract.
let first = frames.iter().find_map(|f| f.get(k)).expect("mask");
let Some(first) = frames.iter().find_map(|f| f.get(k)) else { continue };
let mode = if first.depth % 2 == 0 { MaskMode::Add } else { MaskMode::Subtract };
let mode = if k == 0 { MaskMode::Add } else { mode };
let mut g = build::mask(
@@ -173,14 +173,16 @@ fn run(s: &mut Session, p: &Value) -> Result<Value> {
build::MASK_COLORS[(base + k) % build::MASK_COLORS.len()],
);
if frames.len() > 1 {
let path = g.get_mut("path").expect("path");
for (f, lt) in frames.iter().zip(&layer_times) {
if let Some(tr) = f.get(k) {
effectcraft_keyframe::set_key(&mut path.keys, Keyframe::new(*lt, KV::Path(tr.path.clone())));
if let Some(path) = g.get_mut("path") {
for (f, lt) in frames.iter().zip(&layer_times) {
if let Some(tr) = f.get(k) {
effectcraft_keyframe::set_key(&mut path.keys, Keyframe::new(*lt, KV::Path(tr.path.clone())));
}
}
}
let op = g.get_mut("opacity").expect("opacity");
if frames.iter().any(|f| f.get(k).is_none()) {
if let Some(op) = g.get_mut("opacity")
&& frames.iter().any(|f| f.get(k).is_none())
{
for (f, lt) in frames.iter().zip(&layer_times) {
let mut key = Keyframe::new(*lt, KV::Scalar(if f.get(k).is_some() { 100.0 } else { 0.0 }));
key.in_interp = Interp::Hold;
+5 -6
View File
@@ -2,7 +2,7 @@
//! Language menu (the ⓕ button next to an expression), written from the public Expression
//! Language Reference.
use effectcraft_project::{ItemId, Layer, Property};
use effectcraft_project::{Expression, ItemId, Layer, Property};
use effectcraft_render::EvalCtx;
use effectcraft_time::Tick;
use serde::Serialize;
@@ -45,14 +45,13 @@ pub fn errors(s: &Session, cid: ItemId, t: Tick) -> Vec<ExprError> {
let ctx = EvalCtx { footage: Some(s.footage.as_ref()), expr: s.expr.as_deref(), ..EvalCtx::new(&s.project, cid, comp, t) };
let mut out = vec![];
for (i, l) in comp.layers.iter().enumerate() {
let mut props: Vec<&Property> = vec![];
let mut props: Vec<(&Property, &Expression)> = vec![];
l.props.walk("", &mut |_, p| {
if p.expr.as_ref().is_some_and(|e| !e.text.trim().is_empty()) {
props.push(p);
if let Some(e) = p.expr.as_ref().filter(|e| !e.text.trim().is_empty()) {
props.push((p, e));
}
});
for p in props {
let e = p.expr.as_ref().expect("filtered");
for (p, e) in props {
let msg = if e.enabled {
match &ctx.expr {
Some(h) => h.eval(&ctx, l, p, &p.value_at(l.layer_time(t))).err(),
+1 -1
View File
@@ -955,8 +955,8 @@ fn set_text(s: &mut Session, p: &Value) -> Result<Value> {
&& let Some(doc) = super::text_edit::layer_doc(s, lid)
&& doc.char_len() > 0
&& let Some(obj) = p.as_object()
&& let Some(e) = s.state.text_edit.as_mut()
{
let e = s.state.text_edit.as_mut().expect("checked");
let mut st = e.pending.clone().unwrap_or_else(|| doc.insertion_style(r.start));
let mut any = false;
for (k, v) in obj {
+1 -1
View File
@@ -136,7 +136,7 @@ fn apply(s: &mut Session, cid: ItemId, lid: LayerId, mode: Mode, cuts: &[Tick])
let i = cm.layers.iter().position(|l| l.id == lid).ok_or(EngineError::NoComp)?;
let mut ids = vec![lid];
for t in cuts {
let cur = cm.layers.iter().position(|l| l.id == *ids.last().expect("piece")).unwrap_or(i);
let cur = ids.last().and_then(|last| cm.layers.iter().position(|l| l.id == *last)).unwrap_or(i);
let l = &mut cm.layers[cur];
if !(*t > l.in_point && *t < l.out_point) {
continue;
+10 -16
View File
@@ -72,14 +72,11 @@ fn animator_uid(s: &Session, l: &Layer, p: &Value, cmd: &str) -> Result<u64> {
/// Add the properties of `kind` to an animator's Properties group (skipping ones it has).
fn add_props(ids: &mut Ids, anim: &mut PropGroup, kind: &str, three_d: bool) -> usize {
let list = build::text_anim_props(ids, kind, three_d);
let props = match anim.sub_mut("properties") {
Some(p) => p,
None => {
let g = ids.group("properties", "Properties");
anim.children.push(g.into());
anim.sub_mut("properties").expect("just added")
}
};
if anim.sub("properties").is_none() {
let g = ids.group("properties", "Properties");
anim.children.push(g.into());
}
let Some(props) = anim.sub_mut("properties") else { return 0 };
let mut n = 0;
for pr in list {
if props.get(&pr.match_id).is_none() {
@@ -161,14 +158,11 @@ pub(crate) fn add_selector(s: &mut Session, p: &Value) -> Result<Value> {
let mut ids = Ids(&mut next);
let l = layer_mut(proj, cid, lid)?;
let anim = l.props.find_group_mut(auid).ok_or_else(|| bad(C, "no such animator"))?;
let sels = match anim.sub_mut("selectors") {
Some(g) => g,
None => {
let g = ids.group("selectors", "Selectors");
anim.children.insert(0, g.into());
anim.sub_mut("selectors").expect("just added")
}
};
if anim.sub("selectors").is_none() {
let g = ids.group("selectors", "Selectors");
anim.children.insert(0, g.into());
}
let sels = anim.sub_mut("selectors").ok_or_else(|| bad(C, "the animator has no Selectors group"))?;
let (m, label) = match kind.as_str() {
"range" => ("rangeSelector", "Range Selector"),
"wiggly" => ("wigglySelector", "Wiggly Selector"),
+3 -2
View File
@@ -455,8 +455,9 @@ pub(crate) fn sanitize(s: &mut Session) {
match layer_doc(s, e.layer) {
Some(d) => {
let n = d.char_len();
if e.anchor > n || e.caret > n {
let ed = s.state.text_edit.as_mut().expect("checked");
if (e.anchor > n || e.caret > n)
&& let Some(ed) = s.state.text_edit.as_mut()
{
ed.anchor = ed.anchor.min(n);
ed.caret = ed.caret.min(n);
}
+2
View File
@@ -8,6 +8,8 @@
//! The project is an `Arc<Project>` edited copy-on-write; undo keeps whole-project snapshots
//! (compositions are `Arc`s, so untouched comps are shared).
#![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)]
pub mod autosave;
pub mod camera_track;
pub mod commands;
+1 -2
View File
@@ -439,8 +439,7 @@ impl Session {
}
self.bump();
}
if finished {
let mut job = self.mask_job.take().expect("job");
if finished && let Some(mut job) = self.mask_job.take() {
if let Some(t) = job.thread.take() {
let _ = t.join();
}
+7 -1
View File
@@ -196,7 +196,13 @@ pub fn top_level() -> Vec<&'static str> {
/// The menu bar for this platform.
pub fn menu_bar() -> &'static [MenuNode] {
static BAR: OnceLock<Vec<MenuNode>> = OnceLock::new();
BAR.get_or_init(|| parse(TREE, cfg!(target_os = "macos")).expect("menu tree parses"))
BAR.get_or_init(|| {
// The tree is a constant (a test parses it on every platform); never panic over it.
parse(TREE, cfg!(target_os = "macos")).unwrap_or_else(|e| {
log::error!("menu tree: {e}");
Vec::new()
})
})
}
/// Every entry with its submenu path (`["Layer", "Blending Mode"]`), depth first.
+4
View File
@@ -3,6 +3,10 @@
//! frame). The UI-side numbers (first UI frame, timeline and Project panel draw) live in
//! `effectcraft-ui-egui`'s `bench` module. See `docs/architecture.md` ▸ Performance.
// A benchmark harness (`effectcraft-cli bench --ops`, tests): a failed step should fail loudly,
// as in tests and benches (AGENTS.md, "Never crash"). Nothing here runs in the app.
#![allow(clippy::expect_used)]
use std::sync::Arc;
use effectcraft_keyframe::Value as KValue;
+3 -4
View File
@@ -513,8 +513,8 @@ impl Session {
off.cancel(r.id);
}
self.drain_remote_render();
let j = self.render_job.as_ref().expect("job");
let r = j.remote.as_ref().expect("remote");
let Some(j) = self.render_job.as_ref() else { return true };
let Some(r) = j.remote.as_ref() else { return true };
let finished: Vec<u64> =
lock(&j.shared.updates).iter().filter_map(|u| if let ItemUpdate::Finished { id, .. } = u { Some(*id) } else { None }).collect();
for id in &r.ids {
@@ -570,8 +570,7 @@ impl Session {
}
self.bump();
}
if finished {
let mut job = self.render_job.take().expect("job");
if finished && let Some(mut job) = self.render_job.take() {
if let Some(t) = job.thread.take() {
let _ = t.join();
}
+1 -1
View File
@@ -587,7 +587,7 @@ impl Session {
if !job.is_finished() {
return false;
}
let mut job = self.roto_job.take().expect("job");
let Some(mut job) = self.roto_job.take() else { return false };
if let Some(t) = job.thread.take() {
let _ = t.join();
}
+1 -1
View File
@@ -185,7 +185,7 @@ fn opening_an_old_project_upgrades_effect_instances() {
{
let comp = std::sync::Arc::make_mut(match &mut old.items.get_mut(&cid).unwrap().kind {
effectcraft_project::ItemKind::Comp(c) => c,
_ => unreachable!(),
_ => panic!("not a composition"),
});
let l = comp.layers.iter_mut().find(|l| l.id == LayerId(a)).unwrap();
for n in &mut l.props.sub_mut("effects").unwrap().children {
+2 -2
View File
@@ -182,7 +182,7 @@ fn interpret_proxy_settings_apply_to_the_proxy() {
assert!(px.footage.invert_alpha);
let f = match &s.project.item(full).unwrap().kind {
ItemKind::Footage(f) => f.clone(),
_ => unreachable!(),
_ => panic!("not footage"),
};
assert_eq!(f.alpha, AlphaMode::Straight, "the main footage keeps its interpretation");
}
@@ -243,7 +243,7 @@ fn guess_alpha_invert_and_linear_light() {
let h = import(&mut s, "/m/half.png");
let foot = |s: &Session, i: ItemId| match &s.project.item(i).unwrap().kind {
ItemKind::Footage(f) => f.clone(),
_ => unreachable!(),
_ => panic!("not footage"),
};
s.execute("file.interpretFootage", json!({"items": [b.0, w.0, h.0], "alpha": "guess"})).unwrap();
assert_eq!(foot(&s, b).alpha, AlphaMode::Premultiplied);
+1 -2
View File
@@ -407,8 +407,7 @@ impl Session {
}
self.bump();
}
if finished {
let mut job = self.track_job.take().expect("job");
if finished && let Some(mut job) = self.track_job.take() {
if let Some(t) = job.thread.take() {
let _ = t.join();
}
+1 -1
View File
@@ -375,7 +375,7 @@ impl Session {
if !job.is_finished() {
return false;
}
let mut job = self.warp_job.take().expect("job");
let Some(mut job) = self.warp_job.take() else { return false };
if let Some(t) = job.thread.take() {
let _ = t.join();
}