diff --git a/Cargo.toml b/Cargo.toml index 23d5827..03ce69b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -49,3 +49,8 @@ dbg_macro = "deny" todo = "deny" # Forbid explicit `panic!` calls; unreachable arms belong in `unreachable!` panic = "deny" +# Forbid `.expect()`: it aborts the process on an unexpected value instead of +# surfacing a recoverable error. Call sites that print a stack trace by design +# (invariants the caller already checked, intentionally documented panics) +# carry a localized `#[allow]`; everywhere else propagate via `anyhow::Result`. +expect_used = "deny" diff --git a/src/commands/restore.rs b/src/commands/restore.rs index 089505b..9204238 100644 --- a/src/commands/restore.rs +++ b/src/commands/restore.rs @@ -1,4 +1,4 @@ -use anyhow::Result; +use anyhow::{Context, Result}; use worktree_io::git::{create_worktree, git_worktree_prune}; use worktree_io::ttl::WorkspaceRegistry; @@ -12,7 +12,7 @@ use worktree_io::ttl::WorkspaceRegistry; /// automatically because the original project path is not stored in the /// registry; the user is asked to run `worktree open ` instead. pub fn cmd_restore() -> Result<()> { - let home = dirs::home_dir().expect("could not determine home directory"); + let home = dirs::home_dir().context("could not determine home directory")?; let local_prefix = home.join("worktrees").join("local"); let registry = WorkspaceRegistry::load()?; @@ -47,6 +47,7 @@ pub fn cmd_restore() -> Result<()> { }; // If file_name() returned Some, the path is not a root, so parent() // always returns Some here. + #[allow(clippy::expect_used)] let bare_path = path.parent().expect("non-root path must have a parent"); if !bare_path.exists() { diff --git a/src/issue/parse/centy.rs b/src/issue/parse/centy.rs index 58179ab..8a44d36 100644 --- a/src/issue/parse/centy.rs +++ b/src/issue/parse/centy.rs @@ -29,9 +29,9 @@ pub(super) fn find_centy_root(start: &Path) -> Option { /// cannot be determined, or no `.centy/` directory is found in the current directory or /// any of its ancestors. pub(super) fn parse_centy(s: &str) -> Result { - let id_str = s - .strip_prefix("centy:") - .expect("caller checked starts_with(\"centy:\")"); + let Some(id_str) = s.strip_prefix("centy:") else { + unreachable!("caller checked starts_with(\"centy:\")") + }; let Ok(display_number) = id_str.parse::() else { return Err(anyhow::anyhow!( "Invalid Centy issue number: {id_str:?} — expected a positive integer" diff --git a/src/issue/parse/gh.rs b/src/issue/parse/gh.rs index 5048a56..c75254a 100644 --- a/src/issue/parse/gh.rs +++ b/src/issue/parse/gh.rs @@ -30,9 +30,9 @@ pub(super) fn parse_github_remote_url(url: &str) -> Option<(String, String)> { /// determined, the `origin` remote URL cannot be read, or the URL is not a /// GitHub remote. pub(super) fn parse_gh(s: &str) -> Result { - let num_str = s - .strip_prefix("gh:") - .expect("caller checked starts_with(\"gh:\")"); + let Some(num_str) = s.strip_prefix("gh:") else { + unreachable!("caller checked starts_with(\"gh:\")") + }; let Ok(number) = num_str.parse::() else { return Err(anyhow::anyhow!( "Invalid issue number for gh shorthand: {num_str:?} — expected a positive integer" diff --git a/src/issue/parse/gitlab.rs b/src/issue/parse/gitlab.rs index 78939f0..eb564d7 100644 --- a/src/issue/parse/gitlab.rs +++ b/src/issue/parse/gitlab.rs @@ -68,9 +68,9 @@ pub(super) fn parse_gitlab_url(s: &str) -> Result { /// determined, the `origin` remote URL cannot be read, or the URL is not a /// GitLab remote. pub(super) fn parse_gl(s: &str) -> Result { - let num_str = s - .strip_prefix("gl:") - .expect("caller checked starts_with(\"gl:\")"); + let Some(num_str) = s.strip_prefix("gl:") else { + unreachable!("caller checked starts_with(\"gl:\")") + }; let Ok(number) = num_str.parse::() else { return Err(anyhow::anyhow!( "Invalid issue number for gl shorthand: {num_str:?} — expected a positive integer" diff --git a/src/issue/paths.rs b/src/issue/paths.rs index ec8cda3..8753dba 100644 --- a/src/issue/paths.rs +++ b/src/issue/paths.rs @@ -29,6 +29,11 @@ impl IssueRef { let base = if temp { std::env::temp_dir() } else { + // Documented in `bare_clone_path`'s `# Panics` section: this is a + // narrow, path-building helper with no `Result` to propagate + // into, and a missing home directory is an unrecoverable + // environment problem for every caller. + #[allow(clippy::expect_used)] dirs::home_dir().expect("could not determine home directory") } .join("worktrees"); diff --git a/src/lib.rs b/src/lib.rs index 1412ca7..035d015 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,7 @@ //! `worktree-io` — open GitHub, Linear, and local Centy issues as git worktree workspaces. +// `.expect()` with a descriptive message is the idiomatic way to fail a test +// fast on a broken fixture; only production code paths need to avoid it. +#![cfg_attr(test, allow(clippy::expect_used))] /// Configuration loading and serialization. pub mod config; diff --git a/src/multi_workspace.rs b/src/multi_workspace.rs index b0b6458..55efb26 100644 --- a/src/multi_workspace.rs +++ b/src/multi_workspace.rs @@ -47,13 +47,13 @@ pub fn create_multi_workspace(specs: &[MultiSpec], workspaces_root: &Path) -> Re Ok(root) } -fn github_bare_path(owner: &str, repo: &str) -> PathBuf { - dirs::home_dir() - .expect("could not determine home directory") +fn github_bare_path(owner: &str, repo: &str) -> Result { + Ok(dirs::home_dir() + .context("could not determine home directory")? .join("worktrees") .join("github") .join(owner) - .join(repo) + .join(repo)) } fn open_one(spec: &MultiSpec, root: &Path) -> Result<()> { @@ -82,7 +82,7 @@ fn open_one_issue(issue: &IssueRef, root: &Path) -> Result<()> { } fn open_one_bare(owner: &str, repo: &str, root: &Path) -> Result<()> { - let bare_path = github_bare_path(owner, repo); + let bare_path = github_bare_path(owner, repo)?; let url = format!("https://github.com/{owner}/{repo}.git"); if bare_path.exists() { eprintln!("Fetching origin for {}…", bare_path.display()); diff --git a/tests/snapshot_tests.rs b/tests/snapshot_tests.rs index 14e25e2..543861d 100644 --- a/tests/snapshot_tests.rs +++ b/tests/snapshot_tests.rs @@ -5,6 +5,9 @@ //! clean Docker container (see `docker/Dockerfile.test`) eliminates host-state //! pollution and keeps the snapshots deterministic. #![allow(missing_docs)] +// Integration test fixtures: `.expect()` with a descriptive message is the +// idiomatic way to fail a test fast on a broken fixture. +#![allow(clippy::expect_used)] use worktree_io::repo_hooks_scaffold::SCAFFOLD; use worktree_io::{config::Config, ttl::WorkspaceRegistry};