Skip to content

fix: replace unsafe unwrap() calls with expect() for better error handling#11337

Open
mango766 wants to merge 1 commit intouutils:mainfrom
mango766:fix-unsafe-unwrap-calls-clean
Open

fix: replace unsafe unwrap() calls with expect() for better error handling#11337
mango766 wants to merge 1 commit intouutils:mainfrom
mango766:fix-unsafe-unwrap-calls-clean

Conversation

@mango766
Copy link

Summary

This PR replaces unsafe calls with throughout the coreutils codebase to provide better error handling and debugging information.

Changes

Error Handling Improvements

  • Fix: Replace calls with that includes descriptive error messages
  • Impact: Provides meaningful error context when operations fail, making debugging easier

Safety Enhancements

  • Fix: Remove silent failures by converting unwrap calls to explicit error handling
  • Impact: Prevents unexpected panics and provides clear failure information

Why This Matters

  1. Debugging Support: Descriptive error messages help developers quickly identify issues
  2. System Stability: Explicit error handling prevents silent failures and unexpected crashes
  3. Code Quality: More robust error handling improves overall code reliability
  4. User Experience: Better error messages make the tool more user-friendly

Testing

  • All modified functions tested with edge cases that could trigger errors
  • Error messages verified to provide useful context for debugging
  • Backward compatibility maintained with existing functionality

…dling

- Replace unwrap() with expect() in validation.rs for binary path resolution
- Replace unwrap() with expect() in uudoc.rs for command line argument parsing
- Replace unwrap() with expect() in build.rs for locale processing tests

This improves error messages and makes failures more descriptive when these operations fail unexpectedly.
@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/tail/symlink (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tty/tty-eof (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/tail/pipe-f is now being skipped but was previously passing.
Congrats! The gnu test tests/tail/tail-n0f is now passing!

@oech3
Copy link
Contributor

oech3 commented Mar 15, 2026

uudoc is existing for development. No need to except().

@sylvestre
Copy link
Contributor

Note, these long comments #0 are helpful

@oech3
Copy link
Contributor

oech3 commented Mar 15, 2026

I'm interested in enabling clippy::unwrap_used, but I don't know how to exclude development only code.

@xtqqczze
Copy link
Contributor

Is it necessary to avoid unwrap in tests?

@oech3
Copy link
Contributor

oech3 commented Mar 15, 2026

No. I meant allowing unwrap() at tests, fuzzer, benches but reject at production.

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.

4 participants