Skip to content

Recommend contains() for find().is_some() #6010

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
pickfire opened this issue Sep 5, 2020 · 8 comments · Fixed by #6119
Closed

Recommend contains() for find().is_some() #6010

pickfire opened this issue Sep 5, 2020 · 8 comments · Fixed by #6119
Labels
A-lint Area: New lints C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good first issue These issues are a good way to get started with Clippy L-complexity Lint: Belongs in the complexity lint group L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@pickfire
Copy link
Contributor

pickfire commented Sep 5, 2020

What it does

Recommend .contains() if find().is_some() is used on a &str (including String) and slice.

Categories (optional)

  • Kind: complexity

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

For example:

  • Improve readibility

Drawbacks

None.

Example

"hello world".find("world").is_some();

Could be written as:

"hello world".contains("world");
@pickfire pickfire added the A-lint Area: New lints label Sep 5, 2020
@flip1995 flip1995 added L-complexity Lint: Belongs in the complexity lint group L-suggestion Lint: Improving, adding or fixing lint suggestions good first issue These issues are a good way to get started with Clippy C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages labels Sep 5, 2020
@longlb
Copy link

longlb commented Sep 6, 2020

I'd like to tackle this one.

@longlb
Copy link

longlb commented Sep 7, 2020

@pickfire How would you call find().is_some() against a slice? Would it be something like slice.iter().find().is_some() or another way, because there is a currently a lint for the iter case that recommends any() be used in its place.

@pickfire
Copy link
Contributor Author

pickfire commented Sep 8, 2020

What you said is correct but later when array comes out to have find we need to add it too.

@rsulli55
Copy link
Contributor

rsulli55 commented Oct 5, 2020

I am interested on working on this, if @longlb has moved on. I have a good start and should be able to submit a pull request tomorrow.

@longlb
Copy link

longlb commented Oct 5, 2020

Go for it, I'd actually love to see how you did it because I've been unable to figure out a lot of the compiler stuff.

@rsulli55
Copy link
Contributor

rsulli55 commented Oct 5, 2020

Ok, great!

@pickfire So I made a new lint to do this (basically following the tutorial), but I am wondering if it would be better to instead add this as an extra case of lint_search_is_some() in methods/mod.rs?

@pickfire
Copy link
Contributor Author

pickfire commented Oct 6, 2020

@rsulli55 Not quite sure but probably good. Maybe you can take inspiration from find_map() https://github.com/rust-lang/rust-clippy/blob/master/clippy_lints/src/methods/mod.rs#L2905?

@rsulli55
Copy link
Contributor

rsulli55 commented Oct 6, 2020

@pickfire Ok, thanks! I opened a WIP PR for now with what I have but I will be happy to move the code around if it should be placed somewhere else.

@bors bors closed this as completed in ad4f829 Nov 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-lint Area: New lints C-enhancement Category: Enhancement of lints, like adding more cases or adding help messages good first issue These issues are a good way to get started with Clippy L-complexity Lint: Belongs in the complexity lint group L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants