From c241e04640815a95993c2b641dcad46a127bb850 Mon Sep 17 00:00:00 2001 From: tupe12334 Date: Tue, 30 Jun 2026 21:41:06 +0300 Subject: [PATCH] chore(lint): enable clippy::expect_used `.expect()` aborts the process with no chance to recover or attach context, unlike the `anyhow::Result` propagation this crate already uses throughout its command layer. Fix the seven production call sites: three `strip_prefix(...).expect("caller checked ...")` invariants in `issue/parse/*` (already excluded from the coverage gate) become `unreachable!()` instead of a panic-macro detour, since `clippy::panic` is already denied here; two `dirs::home_dir()` lookups now propagate via `anyhow::Context` in functions that already return `Result`; and the remaining two (a path-building helper with no `Result` to propagate into, and a true invariant in `cmd_restore` that would need a dead, coverage-breaking branch to express any other way) keep their documented panic behind a localized, reasoned `#[allow]`. Test-only `.expect()` calls (fixtures, where a descriptive panic message is the idiomatic way to fail fast) are exempted via a single `cfg(test)` crate attribute and an integration test file allow. Closes #97 --- Cargo.toml | 5 +++++ src/commands/restore.rs | 5 +++-- src/issue/parse/centy.rs | 6 +++--- src/issue/parse/gh.rs | 6 +++--- src/issue/parse/gitlab.rs | 6 +++--- src/issue/paths.rs | 5 +++++ src/lib.rs | 3 +++ src/multi_workspace.rs | 10 +++++----- tests/snapshot_tests.rs | 3 +++ 9 files changed, 33 insertions(+), 16 deletions(-) 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};