fix redundant_pattern_matching lint

[alex-700]

• now it handles while let case (related to Suggest Option::is_some for while let Some(_) = iterator.next() #5462)
• better suggestions in if let case

changelog: Fix suggestion in redundant_pattern_matching and also apply this lint to the while let case

[alex-700]

And I have a couple questions. (so PR is WIP for now)

• this lint for while let "conflicts" with the while-let-on-iterator lint. What should we do with this? It seems not okay to give out two lints about one piece of code.

• before this PR the most of suggestions from this lint was completely wrong. It seems that we need replace only while/if let PATS = EXPR part, but not whole expression/statement. But this way can generate a code like if o.is_some() { true } else { false } that should be fix by another lint. What should we do with this?

Thanks.

[flip1995]

• This is kind of a problem, but can be fixed quite easily. You can just copy this check

  rust-clippy/clippy_lints/src/loops.rs

  Lines 570 to 571 in 52dacbc

  	if method_path.ident.name == sym!(next)
  	&& match_trait_method(cx, match_expr, &paths::ITERATOR)

  to this lint and return if the match_expr is a Iterator::next method call. match_expr is in this case op and the method_path you can get by matching if let ExprKind::MethodCall(method_path) = op.kind.

• Thanks for fixing this suggestion! When a suggestion leads to another lint triggering, it is not a problem as long as it doesn't become a cycling suggestion, which is not the case here.

[flip1995]

I think this is

Suggested change

	Applicability::MaybeIncorrect, // snippet
	Applicability::MachineApplicable, // snippet

now

[alex-700]

Agree. Fixed.

[alex-700]

@flip1995

• I'll do this, but it looks strange a bit. Is it a usual situation to check that we don't trigger lint in the code of the other lint? Maybe is there a better machinery for it?
• Is it ok to allow some "triggered after fixit" lints in tests (as I did here)?

[flip1995]

• Lints overlapping is pretty unusual, so we don't have anything to prevent this. Normally, if lints overlap, they are pretty similar and therefore are implemented in the same module, where it is easier to separate them ("if a -> trigger lint 1; if b -> trigger lint 2"). I think just introducing this check here (with a comment why it is there) is the easiest way to do this in this case.

• Yes, that is the standard approach for this.

[flip1995]

Thanks!

[flip1995]

@bors r+

[bors]

📌 Commit 092c459 has been approved by flip1995

[bors]

⌛ Testing commit 092c459 with merge 5d549c8...

[bors]

💔 Test failed - checks-action_test

[alex-700]

@flip1995 sorry, should I fix something here? Changelog? What should be written in changelog?

And I think that I fix #4344 too.

[flip1995]

Don't worry about the changelog, this only means, that the PR body must contain a line changelog: ... which describes, what this PR does in one sentence. I added that for you.

@bors retry

[bors]

⌛ Testing commit 092c459 with merge 1c0e4e5...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 1c0e4e5 to master...