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

New lint: replace iter.filter(x).count() > 0 with iter.any(x) #5932

Open
ebroto opened this issue Aug 20, 2020 · 6 comments
Open

New lint: replace iter.filter(x).count() > 0 with iter.any(x) #5932

ebroto opened this issue Aug 20, 2020 · 6 comments
Labels
A-lint Area: New lints L-complexity Lint: Belongs in the complexity lint group S-needs-discussion Status: Needs further discussion before merging or work can be started

Comments

@ebroto
Copy link
Member

ebroto commented Aug 20, 2020

What it does

It suggests replacing iter.filter(x).count() > 0 with iter.any(x). The suggestion should be MachineApplicable.

Categories (optional)

  • Kind: complexity or performance

What is the advantage of the recommended code over the original code

It's more concise and it should be faster since any is allowed to return early while filter().count() has to walk through all items.

Drawbacks

None.

Example

segments.iter().filter(|ps| ps.ident.as_str() == "prelude").count() > 0

Could be written as:

segments.iter().any(|ps| ps.ident.as_str() == "prelude")
@ebroto ebroto added good-first-issue These issues are a good way to get started with Clippy A-lint Area: New lints L-complexity Lint: Belongs in the complexity lint group labels Aug 20, 2020
@matthiaskrgr
Copy link
Member

I think this might as well fit into the perf category since .filter().count() has to walk through all iterator items but .any() can return early?

@matthiaskrgr
Copy link
Member

Hmm, but that might also cause false positives for the case that the .filter() call has side effects and the .any() call reduces the number of side-effects that happen :/

@ebroto
Copy link
Member Author

ebroto commented Aug 20, 2020

Indeed! That's similar to #3351

@ebroto ebroto added S-needs-discussion Status: Needs further discussion before merging or work can be started and removed good-first-issue These issues are a good way to get started with Clippy labels Aug 20, 2020
@flip1995
Copy link
Member

The lint idea is good, but we have to think about side-effects. I think this is hard to do/to get right. Cases where we have side effects would be:

  • mutating an UpVar
  • calling a function, which has side effects (hiding this inside an iterator is a bad idea IMO, but suggesting replacing this with any would still change the semantics)
  • Any kind of I/O
  • ???

@camsteffen
Copy link
Contributor

This could be generalized to suggest iter.count() > 0 -> iter.next().is_some() in some cases.

@pickfire
Copy link
Contributor

all() could also be applied here as well as != 0 rather than > 0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints L-complexity Lint: Belongs in the complexity lint group S-needs-discussion Status: Needs further discussion before merging or work can be started
Projects
None yet
Development

No branches or pull requests

5 participants