Add new unconditional_recursion lint

[GuillaumeGomez]

Currently, rustc unconditional_recursion doesn't detect cases like:

enum Foo {
    A,
    B,
}

impl PartialEq for Foo {
    fn eq(&self, other: &Self) -> bool {
        self == other
    }
}

This is because the lint is currently implemented only for one level, and in the above code, self == other will then call impl PartialEq for &T, escaping from the detection. The fix for it seems to be a bit tricky (I started investigating potential solution to add one extra level of recursion here but completely broken at the moment).

I expect that this situation will remain for a while. In the meantime, I think it's acceptable to check it directly into clippy for the time being as a lot of easy cases like this one can be easily checked (next I plan to extend it to cover other traits like ToString).

changelog: Add new unconditional_recursion lint

[rustbot]

r? @llogiq

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

[GuillaumeGomez]

Finally fixed CI. ^^'

[llogiq]

I think more tests with e.g. other == self and/or various wrapper type combinations would be good to rule out any false positives.

[GuillaumeGomez]

I added a lot more tests. Please tell me if you have other ideas for tests to be added.

[bors]

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

[GuillaumeGomez]

Fixed merge conflict.

[bors]

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

[GuillaumeGomez]

Fixed merge conflict.

[llogiq]

How about the following test case:

Impl PartialEq for Foo {
    fn eq(&self, other: &Self) -> bool {
        Bar(self) == Bar(other)
    }
}

There might also be some macro test cases that prove hairy, e.g. when the impl is supplied by proc macro.

[GuillaumeGomez]

I disabled the lint if the code comes from macro expansion. Maybe I should just disable it from proc-macro expansion?

As for your code example: not detected. But in this case, I can put recursion. I'm just not sure how "deep" we want to make it. Let's start with 3 nested levels and we'll see.

[llogiq]

No, leaving the example undetected is fine, loop detection would be cool but slow.

And yes, detecting code inside macros would be great if we can pull off the reporting without showing weird macro-expanded code that will only confuse people.

[GuillaumeGomez]

I have put back the (proc) macros checks, only excluding the ones generated from rustc. I also disabled a lint in the test to have less noise and added some comments in the code to make it easier to understand what's going on.

[llogiq]

Thank you!

@bors r+

[bors]

📌 Commit 6b444f3 has been approved by llogiq

It is now in the queue for this repository.

[bors]

⌛ Testing commit 6b444f3 with merge 9907b90...

[bors]

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: llogiq
Pushing 9907b90 to master...

[llogiq]

Thanks, this looks good.

@bors r+