Dereference one less on search_is_some and make it auto-fixable

[ghost]

Fixes #4453

changelog: none

[ghost]

@flip1995 do you think this is enough, or is some logic required?
If so, could you mentor me?

I can make it auto fixable once everything else works 😉

How can I run doc tests? cargo test --doc SEARCH_IS_SOME doesn't work for me :/

[phansch]

How can I run doc tests? cargo test --doc SEARCH_IS_SOME doesn't work for me :/

I think you'll first have to change to the clippy_lints directory for them to work.

[ghost]

I though this would be easier and the code better to understand. But it seems that I am not capable of fixing the issue 😢

[flip1995]

Your not really far from the goal, and I'm happy to support you to achieve it!

• You already have the closure argument: closure_arg
• You only need to care about closure arguments, that are PatKind::Lit(expr)
• Get the closure_arg_snip = snippet(cx, expr, "..") (This is the name of the closure argument)
• Run search_snippet.replace(&format!("*{}", closure_arg_snip), &closure_arg_snip)

For test cases, see Playground

[ghost]

I think you'll first have to change to the clippy_lints directory for them to work.

Thanks. That worked 😃

[ghost]

@flip1995 thank you for the help 👍

Are you sure I need closure_arg and not the closure_body.value?
PatKind::Lit never finds a match

[flip1995]

Huh, I'm pretty sure that you need closure_arg. body.value is the actual body of the closure.

It should find a match for _.find(|x| ..), except something else is meant by PatKind::Lit. What you can do to find out the correct PatKind for closures like this, is to add a dbg!(closure_arg.node) to the code and then look at the stderr output, when running the testcase: TESTNAME=ui/<name_of_search_is_some_test_without_.rs> cargo +master uitest

[ghost]

LL |     let _ = (0..1).find(|x| *x == 0).is_some();
   |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try this: `any(|x| x == 0)`

it seems like the lint isn't very exact. This causes rust-fix to replace everything with just any(...)

let _ = any(|x| x == 0);

Or did I do something wrong?

[flip1995]

Rustfix always replaces the span you specify with the suggested code. You have to build the span by hand, since it does not exist in the way you need it. This shouldn't be a problem though. Just search in the methods/mod.rs file for span.with_hi to see how to do this.

[ghost]

Rustfix always replaces the span you specify with the suggested code

so I should be able to replace expr.span, with a formatted {0}({1}).is_some(), right?

[flip1995]

No, because the expr.span is everything from the iterator expression ((0..1). in the example above, or e.g. vec.iter().) up to the call of is_some(). Or in other words, everything that is indicated by the ^ in the error message.

You only want to replace the find(..).is_some() with any(..) though. That means you have to construct a span, that starts at find... and ends at ...some(). You already have a span that ends at ...some(): expr.span. Now you need a span that starts with find....

---

I just looked up how to get this span, and saw, that it is not that easy (and not really easy to explain the changes that need to be made). So I decided to write a patch for you, that simplifies this. (I'll notify you once it's ready)

While looking into this, I found, that there already exists a function to get the name of the closure arg:

rust-clippy/clippy_lints/src/utils/mod.rs

Lines 916 to 922 in 949b347

	pub fn get_arg_name(pat: &Pat) -> Option<ast::Name> {
	match pat.node {
	PatKind::Binding(.., ident, None) => Some(ident.name),
	PatKind::Ref(ref subpat, _) => get_arg_name(subpat),
	_ => None,
	}
	}

You can just use this.

[flip1995]

Here's the patch: https://github.com/flip1995/rust-clippy/commits/method_calls_spans

You can look at the second to last commit on the branch on how to use the method spans. Just git cherry-pick the commits on this PR.

[bors]

☔ The latest upstream changes (presumably #4462) made this pull request unmergeable. Please resolve the merge conflicts.

[ghost]

almost worked

[bors]

☔ The latest upstream changes (presumably #4408) made this pull request unmergeable. Please resolve the merge conflicts.

[phansch]

Sorry about the merge conflict, let me know if I can help resolving it =)

[flip1995]

That was my fault in 6035b65, so I'd also resolve it for you, if need be.

[ghost]

I don't get what is going on.
Rebasing just broke my lint again

Please fix it for me :)

[flip1995]

Done 👍

[flip1995]

Could you add a test case where x is derefed multiple times:

let _ = vec.iter().find(|x| **x == 0).is_some();

[flip1995]

Ok one last change and a run of ./util/dev fmt and this is ready to merge. Sorry that this was harder, than expected. I'm really bad when it comes to judging if an issue is a good first issue 😄

[flip1995]

Thanks!

@bors r+

[bors]

📌 Commit 64cd9e4 has been approved by flip1995

[bors]

⌛ Testing commit 64cd9e4 with merge 8239b76...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing 8239b76 to master...