Skip to content

Suggest replacing .map(f).flatten() with .and_then(f) on Options #6870

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

Closed
scottmcm opened this issue Mar 8, 2021 · 10 comments · Fixed by #6876
Closed

Suggest replacing .map(f).flatten() with .and_then(f) on Options #6870

scottmcm opened this issue Mar 8, 2021 · 10 comments · Fixed by #6876
Labels
A-documentation Area: Adding or improving documentation

Comments

@scottmcm
Copy link
Member

scottmcm commented Mar 8, 2021

What it does

What does this lint do?
Checks for usage of _.map(_).flatten(),

This is like https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten but for Options instead of Iterators

Categories (optional)

  • Kind: clippy::style/complexity

Easier to follow what's happening, shorter

Drawbacks

None.

Example

Inspired by @steffahn's suggestion in Nadrieril/dhall-rust#213 (comment)

@scottmcm scottmcm added the A-lint Area: New lints label Mar 8, 2021
@giraffate
Copy link
Contributor

Do you mean this case?

error: called `map(..).flatten()` on an `Option`
--> $DIR/map_flatten.rs:25:39
|
LL | let _: Option<_> = (Some(Some(1))).map(|x| x).flatten();
| ^^^^^^^^^^^^^^^^^^^^^ help: try using `and_then` instead: `.and_then(|x| x)`
error: aborting due to 6 previous errors

@scottmcm
Copy link
Member Author

scottmcm commented Mar 9, 2021

Ah, so I did! The description in the docs made me think it's only for Iterator::map.

@scottmcm scottmcm closed this as completed Mar 9, 2021
@steffahn
Copy link
Member

steffahn commented Mar 9, 2021

Which means the description should be fixed, right?

@giraffate
Copy link
Contributor

Yes, I think so. I will prepare a fix.

@giraffate giraffate reopened this Mar 9, 2021
@giraffate giraffate added A-documentation Area: Adding or improving documentation and removed A-lint Area: New lints labels Mar 9, 2021
@bors bors closed this as completed in 2b781c9 Mar 9, 2021
@scottmcm
Copy link
Member Author

scottmcm commented Mar 9, 2021

Thanks, @giraffate!

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

Hmm, this doesn't seem to be working: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=d9855ae1539cc222b88fa4baa7b46b92

    let _: Option<&str> = x.as_ref().map(|pb| pb.to_str()).flatten();

doesn't get a warning.

@steffahn
Copy link
Member

steffahn commented Mar 15, 2021

@jyn514 It’s a pedantic lint; not enabled by default.

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=1e75f19dfbb4f5b35b2f2ef4a9d8b200

@jyn514
Copy link
Member

jyn514 commented Mar 15, 2021

Ah ok, that makes sense. What do you think about making it a style lint instead?

@steffahn
Copy link
Member

Seems reasonable to me to have it be a style lint. I’m not really sure I understand the distinction anyways; I see a lot of similar types of lints on both sides of the distinction, e.g. option_map_or_none is a style lint that also suggests and_then for a different type of pattern, and there are lots of iterator-related style lints for cases where a combination of iterator methods can be simplified, e.g. iter_skip_next.

More pedantic lints other than map_flatten that seem similar in spirit to other style lints include filter_map_next and manual_ok_or.

@orhun
Copy link

orhun commented Nov 17, 2021

I think this should be a style lint too. I didn't know about this lint until I figure out I can use and_then instead of map & flatten so it would be good to be warned about this by clippy, as default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-documentation Area: Adding or improving documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants