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

Extend result_map_or_into_option lint to handle Result::map_or_else(|_| None, Some) #11845

Merged

Conversation

GuillaumeGomez
Copy link
Member

Fixes #10365.

As indicated in the title, it extends the result_map_or_into_option lint to handle Result::map_or_else(|_| None, Some).

changelog: extension of the result_map_or_into_option lint to handle Result::map_or_else(|_| None, Some)

r? @blyxyas

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 20, 2023
@GuillaumeGomez GuillaumeGomez force-pushed the result_map_or_into_option-extension branch from 9e56112 to 3bc412a Compare November 20, 2023 14:01
@GuillaumeGomez
Copy link
Member Author

Re-assigning to someone else as @blyxyas is busy IRL.

r? @flip1995

@rustbot rustbot assigned flip1995 and unassigned blyxyas Nov 22, 2023
@GuillaumeGomez GuillaumeGomez force-pushed the result_map_or_into_option-extension branch from 3bc412a to c5cd899 Compare November 22, 2023 14:35
@GuillaumeGomez GuillaumeGomez force-pushed the result_map_or_into_option-extension branch 2 times, most recently from f4815ed to 773db59 Compare November 22, 2023 15:56
@GuillaumeGomez
Copy link
Member Author

I added the None check and extended UI test a bit to ensure it is checked.

clippy_lints/src/methods/result_map_or_else_none.rs Outdated Show resolved Hide resolved
clippy_lints/src/methods/result_map_or_else_none.rs Outdated Show resolved Hide resolved
tests/ui/result_map_or_into_option.rs Outdated Show resolved Hide resolved
tests/ui/result_map_or_into_option.rs Outdated Show resolved Hide resolved
@GuillaumeGomez GuillaumeGomez force-pushed the result_map_or_into_option-extension branch 2 times, most recently from a60fd9a to 1b17a5b Compare November 22, 2023 20:45
@GuillaumeGomez
Copy link
Member Author

Applied your suggestions, the code is much shorter now. :)

@flip1995
Copy link
Member

LGTM. r=me with above comment addressed

@bors delegate+

@bors
Copy link
Collaborator

bors commented Nov 23, 2023

✌️ @GuillaumeGomez, you can now approve this pull request!

If @flip1995 told you to "r=me" after making some further change, please make that change, then do @bors r=@flip1995

@GuillaumeGomez GuillaumeGomez force-pushed the result_map_or_into_option-extension branch from 1b17a5b to 1384ebe Compare November 23, 2023 09:41
@GuillaumeGomez
Copy link
Member Author

Updated!

@bors r=@flip1995

@bors
Copy link
Collaborator

bors commented Nov 23, 2023

📌 Commit 1384ebe has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 23, 2023

⌛ Testing commit 1384ebe with merge 6411456...

bors added a commit that referenced this pull request Nov 23, 2023
…sion, r=flip1995

Extend `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)`

Fixes #10365.

As indicated in the title, it extends the `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)`.

changelog: extension of the `result_map_or_into_option` lint to handle `Result::map_or_else(|_| None, Some)`

r? `@blyxyas`
@bors
Copy link
Collaborator

bors commented Nov 23, 2023

💔 Test failed - checks-action_test

@GuillaumeGomez
Copy link
Member Author

Failed because of:

 error: actual output differed from expected
Execute `cargo uibless` to update `tests/ui/manual_main_separator_str.stderr` to the actual output
--- tests/ui/manual_main_separator_str.stderr
+++ <stderr output>
-error: taking a reference on `std::path::MAIN_SEPARATOR` conversion to `String`
-  --> $DIR/manual_main_separator_str.rs:21:19
-   |
-LL |     let _: &str = &MAIN_SEPARATOR.to_string();
-   |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `std::path::MAIN_SEPARATOR_STR`
-   |
-   = note: `-D clippy::manual-main-separator-str` implied by `-D warnings`
-   = help: to override `-D warnings` add `#[allow(clippy::manual_main_separator_str)]`
-
-error: taking a reference on `std::path::MAIN_SEPARATOR` conversion to `String`
-  --> $DIR/manual_main_separator_str.rs:22:17
-   |
-LL |     let _ = len(&MAIN_SEPARATOR.to_string());
-   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `std::path::MAIN_SEPARATOR_STR`
-
-error: taking a reference on `std::path::MAIN_SEPARATOR` conversion to `String`
-  --> $DIR/manual_main_separator_str.rs:23:23
-   |
-LL |     let _: Vec<u16> = MAIN_SEPARATOR.to_string().encode_utf16().collect();
-   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `std::path::MAIN_SEPARATOR_STR`
-
-error: taking a reference on `std::path::MAIN_SEPARATOR` conversion to `String`
-  --> $DIR/manual_main_separator_str.rs:27:12
-   |
-LL |         f: &MAIN_SEPARATOR.to_string(),
-   |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `std::path::MAIN_SEPARATOR_STR`
-
-error: aborting due to 4 previous errors
-

Not related to this PR but just in case I'll rebase.

@GuillaumeGomez GuillaumeGomez force-pushed the result_map_or_into_option-extension branch from 1384ebe to 5d330d0 Compare November 23, 2023 09:57
@GuillaumeGomez
Copy link
Member Author

Ok, let's try again.

@bors r=@flip1995

@bors
Copy link
Collaborator

bors commented Nov 23, 2023

📌 Commit 5d330d0 has been approved by flip1995

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Nov 23, 2023

⌛ Testing commit 5d330d0 with merge 840e227...

@bors
Copy link
Collaborator

bors commented Nov 23, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 840e227 to master...

@bors bors merged commit 840e227 into rust-lang:master Nov 23, 2023
5 checks passed
@GuillaumeGomez GuillaumeGomez deleted the result_map_or_into_option-extension branch November 23, 2023 10:28
@GuillaumeGomez
Copy link
Member Author

The rebase fixed it. Oh well. Dark magic.

bors added a commit that referenced this pull request Nov 23, 2023
Improve error messages format

Following review in #11845, since there is already suggestions, no need to add the second part of the sentence every time.

r? `@flip1995`

changelog: Improve some error messages
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest Result::ok() for result.map_or_else(|_| None, Some)
5 participants