Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

False positive for use_self when using an associated type instead of the type itself. #2843

Closed
DDOtten opened this issue Jun 13, 2018 · 2 comments · Fixed by #6179
Closed
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@DDOtten
Copy link

DDOtten commented Jun 13, 2018

A simple example of this is:

trait Foo {
    type Bar;
}

impl Foo for usize {
    type Bar = u8;
}

impl<T: Foo> Foo for Option<T> {
    type Bar = Option<T::Bar>;
}

here is the playground link.

@flip1995
Copy link
Member

Related to #1993. There is a WIP PR #1997 with a fix, but hasn't been completed for a few months.

@phansch phansch added C-bug Category: Clippy is not doing the correct thing L-suggestion Lint: Improving, adding or fixing lint suggestions labels Dec 6, 2018
@gnzlbg
Copy link
Contributor

gnzlbg commented Nov 28, 2019

Another example:

#![warn(clippy::use_self)]

pub trait NonZeroT {
    type T;
}
impl NonZeroT for i32 {
    type T = std::num::NonZeroU32;
}

https://play.rust-lang.org/?version=stable&mode=debug&edition=2015&gist=9926807d25af6d50d9b34139bd1c27ff

tnielens added a commit to tnielens/rust-clippy that referenced this issue Apr 26, 2020
tnielens added a commit to tnielens/rust-clippy that referenced this issue Apr 26, 2020
tnielens added a commit to tnielens/rust-clippy that referenced this issue Apr 26, 2020
tnielens added a commit to tnielens/rust-clippy that referenced this issue Apr 27, 2020
flip1995 pushed a commit to flip1995/rust-clippy that referenced this issue Apr 29, 2020
bors added a commit that referenced this issue Oct 13, 2020
…itive, r=<try>

rework use_self impl based on ty::Ty comparison #3410

`use_self` lint refactoring. As suggested by `@eddyb` , implement the lint based on `ty::Ty` comparison instead of `hir::Path` checks.

This PR introduces negative regressions. The cases were covered in the previous implementation. See the test file.

It fixes #3410, #2843, #3859, #4734 and #4140. Should fix #4143 (example doesn't compile). #4305 false positive seems also fixed but the uitest fails to compile the unmodified test case in `use_self.rs.fixed`. Implements #5078.
Generally the implementation checks are done at mir level and offers higher guarantees against false positives. It is probably fixing other unreported false positives.

changelog: fix use_self generics false positives
@phansch phansch added the I-false-positive Issue: The lint was triggered on code it shouldn't have label Dec 18, 2020
bors added a commit that referenced this issue Jan 19, 2021
Rework use_self impl based on ty::Ty comparison #3410 | Take 2

This builds on top of #5531

I already reviewed and approved the commits by `@montrivo.` So only the review of my commits should be necessary.

I would also appreciate your review `@montrivo,` since you are familiar with the challenges here.

Fixes #3410 and Fixes #4143 (same problem)
Fixes #2843
Fixes #3859
Fixes #4734 and fixes #6221
Fixes #4305
Fixes #5078 (even at expression level now 🎉)
Fixes #3881 and Fixes #4887 (same problem)
Fixes #3909

Not yet: #4140 (test added)

All the credit for the fixes goes to `@montrivo.` I only refactored and copy and pasted his code.

changelog: rewrite [`use_self`] lint and fix multiple (8) FPs. One to go.
bors added a commit that referenced this issue Feb 10, 2021
Rework use_self impl based on ty::Ty comparison #3410 | Take 2

This builds on top of #5531

I already reviewed and approved the commits by `@montrivo.` So only the review of my commits should be necessary.

I would also appreciate your review `@montrivo,` since you are familiar with the challenges here.

Fixes #3410 and Fixes #4143 (same problem)
Fixes #2843
Fixes #3859
Fixes #4734 and fixes #6221
Fixes #4305
Fixes #5078 (even at expression level now 🎉)
Fixes #3881 and Fixes #4887 (same problem)
Fixes #3909

Not yet: #4140 (test added)

All the credit for the fixes goes to `@montrivo.` I only refactored and copy and pasted his code.

changelog: rewrite [`use_self`] lint and fix multiple (8) FPs. One to go.
@bors bors closed this as completed in 605e9ba Feb 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: Clippy is not doing the correct thing I-false-positive Issue: The lint was triggered on code it shouldn't have L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants