Fix false positive unconditional_recursion

[GuillaumeGomez]

Fixes #12052.

Only checking if both variables are local was not enough, we also need to confirm they have the same type as Self.

changelog: Fix false positive for unconditional_recursion lint

[rustbot]

r? @xFrednet

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

[Jarcho]

Is there any reason for expression type constraints? The only thing that matters for recursion checking are the types of the lhs and rhs of the operator.

[GuillaumeGomez]

Unless I missed something, we don't know which method is called just from here. It is much simpler to instead check the types of lhs and rhs and compare it with self.

[Jarcho]

Sorry, I worded that badly. I meant the constraint that the lhs and rhs are both locals.

[y21]

Maybe this should should also check that there aren't any other explicit returns in the function body, since that can act as the recursion base case?
E.g. https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=7abd6a54f7908a965cdd496144343a6d is fine, but we still emit a warning for that with this fix, right?

[GuillaumeGomez]

Sorry, I worded that badly. I meant the constraint that the lhs and rhs are both locals.

You're right, it doesn't make much sense. I'll remove this check.

@y21: That's a good point. I changed how I check it to ensure that there is no other return statements. If you have other test cases, please share them. :)

[bors]

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

[GuillaumeGomez]

Fixed merge conflict.

[xFrednet]

LGTM, thank you for the update :D

@bors r+

[bors]

📌 Commit 7107aa2 has been approved by xFrednet

It is now in the queue for this repository.

[bors]

⌛ Testing commit 7107aa2 with merge 0153ca9...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 0153ca9 to master...

[faern]

How often are Clippy releases cut for nightly? I have a project where I use nightly rustc/clippy for various reasons, and I get this false positive even for clippy 0.1.77 (190f4c9 2024-01-09)

[GuillaumeGomez]

I think it has been released so if you have a case, please open a new issue with a minimal code to reproduce it and tag me on it so I can fix it.

[faern]

Sure! Done: #12118. Thank you for your work on this!

[flip1995]

This is not synced yet. It will be synced tomorrow, so it will be in the nightly version of Friday 2024-01-12

[faern]

So how does one figure out what version of the code one's clippy is running? Since I was running nightly and the date printed by cargo clippy --version was about a week after when the fix was merged? Since it's nightly I assumed it was the main branch at that date? Is there a good way way in general for a user to figure out?

[flip1995]

You can't. Syncs are every other week on Thursday. This is documented in our book. So you have to know the Clippy sync process and how Rust releases work.

We can't really give better version info, as the Clippy that is shipped with rustup is built in the Rust repo. And as far as that Clippy version is concerned, it is the one from that nightly. However, that version lacks behind by up to 2 weeks (by design).

If there is an important fix, that blocks our users, we're open to requests to do an out-of-cylce sync, so that it gets into nightly earlier. But as the Clippy update is planned for tomorrow, I too lazy to do an additional sync today 😅

[faern]

Thank you for the explanation! That makes sense and I understand doing it differently is hard/has other drawbacks! Optimally cargo clippy --version would print the date of when the source code was branched/synced from this repo, not when it was compiled.

[GuillaumeGomez]

That's worth opening an issue to update --version or provide additional information.

[flip1995]

FWIW, this is the command I'm using to find out the Clippy commit of the last sync in the Rust repo:

git log --oneline -- src/tools/clippy/ | grep -o "Merge commit '[a-f0-9]*' into .*" | head -1 | sed -e "s/Merge commit '\([a-f0-9]*\)' into .*/\1/g"

However, this depends on the exact commit message. If someone would change that during sync, this would no longer work. So using that to get more version information for --version is really flaky.