Skip to content

New lint for all/any after mapping to bool. #7339

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

Open
duongdominhchau opened this issue Jun 10, 2021 · 10 comments
Open

New lint for all/any after mapping to bool. #7339

duongdominhchau opened this issue Jun 10, 2021 · 10 comments
Assignees
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience.

Comments

@duongdominhchau
Copy link

duongdominhchau commented Jun 10, 2021

What it does

Eliminate map to bool if it is before a call to all/any

Categories (optional)

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

Shorter code.

Drawbacks

If the map operation has side-effect, applying the suggestion will change the meaning of the program.

Example

iter
    .map(|value| value > 0)
    .all(|x| x)

Could be written as:

iter.all(|value| value > 0)
@duongdominhchau duongdominhchau added the A-lint Area: New lints label Jun 10, 2021
@giraffate giraffate added the good-first-issue These issues are a good way to get started with Clippy label Jun 11, 2021
@camsteffen
Copy link
Contributor

camsteffen commented Jun 14, 2021

In addition to any and all, this may include a large number of transformers: all, any, map, position, flat_map, find, find_map, filter, filter_map, fold.

The lint could be named map_then_identity_tranformer or collapsible_identity_tranformer.

Also this can be generalized further. Not just identity, but any closure where the input variable is referenced only one time in both map and the following transformer. For example:

iter.map(|x| foo(x)).flat_map(|x| bar(x))
// to
iter.flat_map(|x| bar(foo(x)))

This can be called collapsible_map. I think I would prefer to only have this lint and that would cover all cases.

@henrifrancois
Copy link

I think I'd like to give this a go. Safe to claim it?

@camsteffen
Copy link
Contributor

Absolutely!

@henrifrancois
Copy link

@rustbot claim

@henrifrancois
Copy link

henrifrancois commented Jun 29, 2021

Hard time finding time to work on this. Will still be trying to implement this but in the meantime anyone feel free to give this a shot.

@henrifrancois
Copy link

@rustbot unclaim

@camsteffen camsteffen added E-medium Call for participation: Medium difficulty level problem and requires some initial experience. and removed good-first-issue These issues are a good way to get started with Clippy labels Jun 29, 2021
@tamaroning
Copy link
Contributor

@rustbot claim

@tamaroning
Copy link
Contributor

Hi, @camsteffen
Do you know any way to check if a closure param is referenced one time in the closure body?
Just parsing an Expr tree is a little bit tough ( ̄~ ̄;)

@camsteffen
Copy link
Contributor

@tamaroning First get the ID of the closure parameter binding which is the HirId in PatKind::Binding. Then use expr_visitor to create a visitor to scan the closure body. In the visitor, use path_to_local_id to check if the current expression matches the binding.

@tamaroning
Copy link
Contributor

@camsteffen
I didn't know that! Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints E-medium Call for participation: Medium difficulty level problem and requires some initial experience.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants