Improve match like matches lint

[alex-700]

fixes #6186

changelog: improve MATCH_LIKE_MATCHES_MACRO lint

[rust-highfive]

r? @ebroto

(rust_highfive has picked a reviewer for you, use r? to override)

[alex-700]

@ebroto, I remember about this, but don't know how to do it without global refactoring near these two lints. Do you have some other ideas? Is there a big issue for clippy: don't lint two different lints for the same code?

[ebroto]

but don't know how to do it without global refactoring near these two lints.

One approach is to move match_same_arms into the matches module where match_like_matches_macro lives, and not trigger the former if the latter lints. I just did something similar in this commit for another lint.

In this case we would have to move some functions from the copies module to utils. Let me know if I explained my idea correctly :)

[ebroto]

Is there a big issue for clippy: don't lint two different lints for the same code?

Giving two different suggestions for the same code can be misleading if they suggest different things, so we try to avoid it. Moreover, if both lints have auto-applicable suggestions, trying to apply the second one would fail.

[alex-700]

Let me know if I explained my idea correctly

Yeap, I think that your idea is perfectly clear. It's the one I called "global refactoring near these two lints" :) The main problem that all examples in clippy code, I see, were straightforward methods, which call cx.span_* after "batch of ifs". For reusing the code for non-linting something I need some another approach. I'm choosing between the following:

• introduce method which will return Option<SpanContext>, instead of calling cx.span_*, where SpanContext is some information that this lint gathered during "batch of ifs". But what should be stored in this SpanContext?..
• like the first one, but returns Option<impl Fn()> or mb Option<dyn Fn()>, where call of this function is exactly "spanning the lint"
• have some set of spans of other lints, which can conflict with my one (like in your example). In this case I need to track the order of checks (introduce common lint-group for this one), also I need to share between several fns information about which of these spans I saved into the set. IMHO, this option isn't good for following code support.

@ebroto did you get, what I'm trying to say? Is the third idea common for clippy codebase? If it is, can you point several others examples? Personally I like the second idea in cases where there is no some cpu-consuming calculations in this Option<...> evaluation.

[ebroto]

The implementation LGTM, I've just suggested to add some tests.

About the interaction with match_same_arms, I apologize for not being more precise in my first comment. I pointed you to that commit to show that we can move a lint to another module, but the solution of storing the spans is very specific to that case because the conflicting lints start a different levels, one in check_block and the other one in check_expr.

In this case, I think it's easier because both lints work at the expression level, and if the expression is a match. So the solution should be applied in this section of the matches module and doing something like

let matches_applied = check_match_like_matches(cx, expr);
// ...
if !matches_applied {
    check_match_same_arms(cx, expr);
}

I want to say that every time we try to avoid conflicting lints, it's a hack. We need a more general solution, probably implemented in rustc itself.

[ebroto]

@bors r+

Thanks for taking care of this!

[bors]

📌 Commit 2b7dd31 has been approved by ebroto

[bors]

⌛ Testing commit 2b7dd31 with merge de83f09...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: ebroto
Pushing de83f09 to master...