Skip to content

fix(lib): remove .unwrap() from library crates #134

Description

@lukekania

Goal

Bring every library crate into compliance with the project rule documented in CLAUDE.md:

No .unwrap() in library crates — use ? or explicit match.

A pre-flight scan before tagging v1.0.0 found 281 .unwrap() calls across the 9 library crates. They date back to early prototyping and were never cleaned up. Each one is a panic waiting for the right input — fine in tests, not fine in a build tool that runs against arbitrary user code.

Scope

crates/diagnostics/src       (low — small crate)
crates/project-resolver/src
crates/ts-transform/src
crates/bundler/src
crates/template-compiler/src
crates/npm-resolver/src
crates/linker/src
crates/watch/src
crates/dev-server/src

#[cfg(test)] blocks and the crates/cli binary crate are out of scope (CLAUDE.md's rule is library-only; the binary's process::exit(1) on error is fine).

Approach

  1. Land a clippy lint to make this enforceable post-cleanup. Add to .cargo/config.toml:
    [target.'cfg(all())']
    rustflags = ["-Wclippy::unwrap_used"]
    Then bump to -D once the cleanup is done.
  2. Per crate, in dependency order (diagnostics first, dev-server last):
    • Replace result.unwrap()? if in a NgcResult-returning fn, else match
    • Replace option.unwrap()ok_or_else(...) + ?, else expect(...) with a real invariant message (still a panic but documented)
    • Replace Mutex::lock().unwrap()expect("mutex poisoned — earlier panic in another thread") (panic-on-poison is the std behaviour anyway, but documenting it keeps the lint happy)
  3. Run cargo test --workspace --no-fail-fast after each crate to catch regressions.
  4. Run RUST_BACKTRACE=1 cargo run -p ngc-rs -- build against a known-good fixture to verify no new error paths surface.

Definition of done

  • grep -rn '\.unwrap()' crates/{diagnostics,project-resolver,ts-transform,bundler,template-compiler,npm-resolver,linker,watch,dev-server}/src returns 0 matches outside #[cfg(test)] blocks
  • cargo clippy --workspace --all-targets -- -D warnings -D clippy::unwrap_used passes
  • All existing tests still pass
  • No new .unwrap() callers can land — clippy-deny enforces it

Notes

This is a lint hygiene fix, not a functional change — the binary's behaviour stays identical for valid inputs. The win is in error reporting: panics with no source context become real NgcErrors that surface through --output-json and the architect builder shim.

Estimated effort: ~1-2 days. Many of the 281 are Mutex::lock().unwrap() and Path::parent().unwrap() patterns that compress to a single expect() line each.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions