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

Lint using |_| () instead of drop #2523

Open
clarfonthey opened this issue Mar 13, 2018 · 8 comments · May be fixed by #13545
Open

Lint using |_| () instead of drop #2523

clarfonthey opened this issue Mar 13, 2018 · 8 comments · May be fixed by #13545
Labels
S-needs-discussion Status: Needs further discussion before merging or work can be started

Comments

@clarfonthey
Copy link
Contributor

Basically, any occurrence of |_| () as a closure could be more clearly worded as drop.

@oli-obk oli-obk added good-first-issue These issues are a good way to get started with Clippy T-middle Type: Probably requires verifiying types A-lint Area: New lints labels Mar 13, 2018
@birkenfeld
Copy link
Contributor

I don't agree. To me, drop signals that the important point is to destroy the value while |_| () says that the important point is to ignore it. Especially with a primitive type like an integer, it'd be a "huh?" moment.

@oli-obk oli-obk added S-needs-discussion Status: Needs further discussion before merging or work can be started and removed A-lint Area: New lints T-middle Type: Probably requires verifiying types good-first-issue These issues are a good way to get started with Clippy labels Mar 16, 2018
@clarfonthey
Copy link
Contributor Author

@birkenfeld Do you have a good example of a case like that?

Something like Some(5).map(|_| ()) makes more sense as Some(5).map(drop) to me.

@CryZe
Copy link

CryZe commented Apr 17, 2019

drop is definitely much more readable imo and other people seem to be confused by "the toilet closure" too: https://www.reddit.com/r/rust/comments/bdqxch/actixweb_what_map_err_is_really_doing/

@Manishearth
Copy link
Member

I don't find drop to be readable in an error context. The toilet closure is confusing too, but at least it's become a norm.

@tesuji
Copy link
Contributor

tesuji commented Jan 6, 2020

People in rust-lang/rust#67672 prefer map(drop).

@kpp
Copy link

kpp commented Feb 14, 2020

To me, drop signals that the important point is to destroy the value while |_| () says that the important point is to ignore it.

The same for me. How about alias for drop:

fn ignore<T>(_x: T) { }

fn main() {
    dbg!(Some(5).map(ignore));
}

@llogiq
Copy link
Contributor

llogiq commented Feb 14, 2020

I may be weird, but I think dropping is empathically not the same as ignoring it. And in Rust, we don't tend to ignore things anyway.

@clarfonthey
Copy link
Contributor Author

I'm very much not in favour of things doing exactly the same thing but having different names. Toilet closures are unnamed, although I still find drop easier to read. If we were to make an actual exported alias for drop I'd be against that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-needs-discussion Status: Needs further discussion before merging or work can be started
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants