Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add manual unwrap or lint #127

Closed
wants to merge 2 commits into from

Conversation

coxmars
Copy link
Contributor

@coxmars coxmars commented Sep 30, 2024

Summary 📌

This PR introduces a new lint manual_unwrap_or that detects and suggests replacing manual match patterns for Option<T> or Result<T, E> with the more concise and readable unwrap_or function, improving code clarity and conciseness.

Changes Made ⚡️

Implementation of manual_unwrap_or Lint

  • Added a new lint in manual_unwrap_or.rs that detects match expressions reimplementing the behavior of unwrap_or for Option<T> and Result<T, E>.
  • Implemented the fix logic in fix.rs, converting manual match patterns to the more compact unwrap_or call, preserving the original logic.
  • Integrated the lint within the manual module to centralize logic for lints detecting manual patterns.

Test Coverage 🥷🏼

  • Added test cases in test_files/manual_unwrap_or to validate the manual_unwrap_or lint:
    • Conversion of a simple Option::Some and Option::None match to unwrap_or.
    • Conversion of Result::Ok and Result::Err matches.

Test Results ✅

Tests have been added and validated for the manual_unwrap_or lint. Some tests are failing due to potential conflicts between lints; however, individual validation confirms that the manual_unwrap_or lint works as intended. I would appreciate feedback on this point. @0xLucqs 🫡

image

@0xLucqs
Copy link
Contributor

0xLucqs commented Oct 2, 2024

do you think you'll have the time to rebase and fix everything by tomorrow ?

@coxmars
Copy link
Contributor Author

coxmars commented Oct 2, 2024

do you think you'll have the time to rebase and fix everything by tomorrow ?

I'm not 100% sure but I will try to do it 🫡

@0xLucqs 0xLucqs closed this Oct 7, 2024
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