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 Iterator::folds that can be one of the special methods #2237

Closed
oli-obk opened this issue Nov 20, 2017 · 7 comments
Closed

lint Iterator::folds that can be one of the special methods #2237

oli-obk opened this issue Nov 20, 2017 · 7 comments
Labels
good-first-issue These issues are a good way to get started with Clippy L-unnecessary Lint: Warn about unnecessary code

Comments

@oli-obk
Copy link
Contributor

oli-obk commented Nov 20, 2017

I just found .fold(false, |acc, x| acc || x.starts_with("--target")); in rustc. That should obviously be .any(|x| x.starts_with("--target"))

@oli-obk oli-obk added L-unnecessary Lint: Warn about unnecessary code good-first-issue These issues are a good way to get started with Clippy labels Nov 20, 2017
@clarfonthey
Copy link
Contributor

Also .fold(1, |acc, x| acc * x) and .fold(0, |acc, x| acc + x) should be replaced with .sum() and .product() respectively.

@clarfonthey
Copy link
Contributor

Bonus points: .rfold(1, |acc, x| acc * x) and .rfold(0, |acc, x| acc * x) could be .rev().product() and .rev().sum().

@theotherphil
Copy link
Contributor

I'd like to give this a go. Any pointers to get started?

@oli-obk
Copy link
Contributor Author

oli-obk commented Dec 23, 2017

@theotherphil

There are already many iterator method lints in

https://github.com/rust-lang-nursery/rust-clippy/blob/master/clippy_lints/src/methods.rs#L718

You should be able to add a new branch for fold. You'll need to extract some info from the closure (if there is one). Have a look at

https://github.com/rust-lang-nursery/rust-clippy/blob/f9682ca563ad9ec6bc2da520ad92970d30d5c1e0/clippy_lints/src/map_clone.rs#L34

which does this for the |x| *x closure

@theotherphil
Copy link
Contributor

Great, thanks!

@ghost
Copy link

ghost commented Oct 21, 2018

@oli-obk
@theotherphil
.any and .all are not always safe replacements of .fold: those two are short-circuiting, while .fold evaluates every element in the iterator. If the iterator incurs side effects the two methods will have different behaviour.
Imho clippy should at least warn the user for the possible difference instead of suggesting .all and .any indifferently.

@camsteffen
Copy link
Contributor

Fixed by #2350. We also have #2401 for more fold cases.

@oli-obk oli-obk closed this as completed Dec 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good-first-issue These issues are a good way to get started with Clippy L-unnecessary Lint: Warn about unnecessary code
Projects
None yet
Development

No branches or pull requests

4 participants