Skip to content

Add map unwrap or lint for Result #4822

New issue

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

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

Already on GitHub? # to your account

Closed
wants to merge 6 commits into from

Conversation

guanqun
Copy link

@guanqun guanqun commented Nov 16, 2019

with the introduction of map_or() in Result by @lzutao , we should add the similar lint.


changelog: none

@guanqun
Copy link
Author

guanqun commented Nov 17, 2019

hmm. why following errors? I can build locally.

error[E0063]: missing field `force_run_in_process` in initializer of `test::TestOpts`

   --> /home/travis/.cargo/registry/src/github.heygears.com-1ecc6299db9ec823/compiletest_rs-0.3.25/src/lib.rs:110:5

    |

110 |     test::TestOpts {

    |     ^^^^^^^^^^^^^^ missing `force_run_in_process`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0063`.

@JohnTitor
Copy link
Member

clippy build with the latest master is currently broken, it's not your fault.

@JohnTitor JohnTitor added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 17, 2019
@flip1995 flip1995 added S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 19, 2019
@flip1995
Copy link
Member

This is blocked on the stabilization of Result::map_or

@jfrimmel
Copy link

cc rust-lang/rust#66570

@bors
Copy link
Contributor

bors commented Nov 23, 2019

☔ The latest upstream changes (presumably #4839) made this pull request unmergeable. Please resolve the merge conflicts.

@flip1995 flip1995 added the A-lint Area: New lints label Nov 25, 2019
@tesuji
Copy link
Contributor

tesuji commented Dec 16, 2019

rust-lang/rust#66570 merged. This could be continued.

@JohnTitor JohnTitor removed the S-blocked Status: marked as blocked ❌ on something else such as an RFC or other implementation work label Dec 16, 2019
@guanqun
Copy link
Author

guanqun commented Dec 17, 2019

Thank you @lzutao for the headsup, will work on the conflicts.

@guanqun
Copy link
Author

guanqun commented Dec 17, 2019

conflicts resolved, could you please help review again?

@JohnTitor JohnTitor added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Dec 17, 2019
@guanqun
Copy link
Author

guanqun commented Dec 18, 2019

@lzutao could you please help review again? Thanks!

@bors
Copy link
Contributor

bors commented Dec 22, 2019

☔ The latest upstream changes (presumably #4930) made this pull request unmergeable. Please resolve the merge conflicts.

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Sorry for the late reply, I was on vacation. The formatting of the lint message, note and help can still be improved.

Comment on lines +64 to +65
"called `map(f).unwrap_or(a)` on a Result value. \
This can be done more directly by calling `map_or(a, f)` instead"
Copy link
Member

Choose a reason for hiding this comment

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

Can you do this by using the function

span_lint_and_then(
    ..,
    |db| {
         db.note("this can be done more directly by calling `map_or(a, f)` instead");
         if multiline {
               db.span_help(.., format!("replace `map({}).unwrap_or({})` with `{}`", ..);
         }
    },
);

@bors
Copy link
Contributor

bors commented Dec 27, 2019

☔ The latest upstream changes (presumably #4962) made this pull request unmergeable. Please resolve the merge conflicts.

@tesuji
Copy link
Contributor

tesuji commented Dec 27, 2019

Can we suggest with Result::ok().map_or case too?

@flip1995 flip1995 added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 28, 2019
@flip1995
Copy link
Member

Thanks for contributing to Clippy! Sadly this PR was not updated in quite some time. If you waited on input from a reviewer, we're sorry that this fell under the radar. If you want to continue to work on this, just reopen the PR and/or ping a reviewer.

@flip1995 flip1995 closed this May 25, 2020
@flip1995 flip1995 added S-inactive-closed Status: Closed due to inactivity and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels May 25, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-lint Area: New lints S-inactive-closed Status: Closed due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants