fix(equatable_if_let): don't lint if pattern or initializer come from expansion

[ada4a]

This means some existing test cases won't get linted anymore, but imo that's preferable to false-positives.

Fixes #14548

changelog: [equatable_if_let]: don't lint if pattern or initializer come from expansion

[rustbot]

r? @dswij

rustbot has assigned @dswij.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[github-actions]

Lintcheck changes for 8bcc093

Lint	Added	Removed	Changed
clippy::equatable_if_let	0	21	0

This comment will be updated if you push new changes

[ada4a]

Looking at Lintcheck:

• encoding_rs is TP->FN: both sides came from inside the macro, and so the suggestion was correct.
• serde is TP->FN: the scrutinee comes from a particular macro invocation, for which we know if the lint should fire, and whether we should suggest == or matches!
• wasmi is FP->TN: the scrutinee comes from outside of the macro definition, but we gave a separate suggestion for each individual invocation, even though they might not be valid for the macro definition in general, which is a similar problem to the original issue. This is not to mention that the suggestion replaced the $expr metavariable with the contents of the arguments from the individual invocations.

For the first one, unfortunately I don't think there's a way to allow cases like that while disallowing the case from original issue, since in both cases, the two sides will have equal, non-root SyntaxContexts.

To fix the second and third one, we'd need some way to only allow the sides (probably only one of them?) to come from a macro invocation (e.g. if let m!() = a -> if a == m!() is fine), but not an expansion inside a macro (if let $pat = a -> if a == $pat is not fine), and I don't know how to do that.

[rustbot]

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[dswij]

Are we expecting this test case to not be linted too?

[ada4a]

From #15958 (comment):

2. serde is TP->FN: the scrutinee comes from a particular macro invocation, for which we know if the lint should fire, and whether we should suggest == or matches!
3. wasmi is FP->TN: the scrutinee comes from outside of the macro definition, but we gave a separate suggestion for each individual invocation, even though they might not be valid for the macro definition in general, which is a similar problem to the original issue. This is not to mention that the suggestion replaced the $expr metavariable with the contents of the arguments from the individual invocations.

[..]

To fix the second and third one, we'd need some way to only allow the sides (probably only one of them?) to come from a macro invocation (e.g. if let m!() = a -> if a == m!() is fine), but not an expansion inside a macro (if let $pat = a -> if a == $pat is not fine), and I don't know how to do that.

[ada4a]

Now that I think about it, I think the if let m!() = a/ if let a = m!() cases can be detected by checking that at most 1 of the sides has a non-root SyntaxContext, right?

EDIT: Nope, that fails on the following:

macro_rules! generate_script {
    ($f:expr) => {
        if let "bar" = $f {}
    };
}

generate_script! { "foo" }