Skip to content

fix: replace .unwrap() with .expect() on infallible calls#168

Open
arhtur007 wants to merge 1 commit intoopenabdev:mainfrom
arhtur007:aw/remove-unwrap
Open

fix: replace .unwrap() with .expect() on infallible calls#168
arhtur007 wants to merge 1 commit intoopenabdev:mainfrom
arhtur007:aw/remove-unwrap

Conversation

@arhtur007
Copy link
Copy Markdown

@arhtur007 arhtur007 commented Apr 9, 2026

Description

Replace four .unwrap() calls with .expect("...") so the panic message documents the invariant.

Changes

  • src/config.rsexpand_env_vars regex literal
  • src/discord.rssender_ctx JSON serialization, strip_mention regex, shorten_thread_name GitHub URL regex

Rationale

All four call sites are infallible in practice (regex literals known at compile time, and a serde_json::Value built from a json! macro). .expect("reason") is the conventional form
for "should never panic" — if it ever does fire, the message points at the violated invariant.
.unwrap_or_else() is not suitable here because there is no meaningful fallback value for an invalid regex or a failed serialization — the program cannot continue correctly, so a panic
with a descriptive message is the right behavior.

@arhtur007 arhtur007 requested a review from thepagent as a code owner April 9, 2026 16:46
Copy link
Copy Markdown
Contributor

@masami-agent masami-agent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — straightforward improvement. All four sites are infallible (compile-time regex literals and json! macro output), so .expect() with a reason string is the idiomatic Rust convention here. Good housekeeping 👍

The four .unwrap() calls all sit on values that cannot fail in
practice (regex literals known at compile time, and a serde_json
value built from a json! literal). Switch to .expect() so the panic
message documents the invariant if it ever does fire.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants