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

Suggestion does not compile: use self with associated types #4140

Closed
lopopolo opened this issue May 25, 2019 · 5 comments · Fixed by #7288
Closed

Suggestion does not compile: use self with associated types #4140

lopopolo opened this issue May 25, 2019 · 5 comments · Fixed by #7288
Assignees
Labels
C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions

Comments

@lopopolo
Copy link

lopopolo commented May 25, 2019

$ cargo clippy -V
clippy 0.0.212 (fc96aa03 2019-05-04)

I have two traits that have the same associated types that are shaped like From and TryFrom. I provide a default TryFrom implementation for types that implement From. Clippy suggests I modify the code below to replace the To::From and To::To references with Self::From and Self::To, which causes a recursive overflow.

pub trait FromMrb<T> {
    type From;
    type To;

    fn from_mrb(interp: &Mrb, value: T) -> Self;
}

pub trait TryFromMrb<T>
where
    Self: Sized,
{
    type From;
    type To;

    unsafe fn try_from_mrb(interp: &Mrb, value: T) -> Result<Self, Error<Self::From, Self::To>>;
}

impl<From, To> TryFromMrb<From> for To
where
    To: FromMrb<From>,
{
    type From = To::From;
    type To = To::To;

    unsafe fn try_from_mrb(interp: &Mrb, value: From) -> Result<Self, Error<Self::From, Self::To>> {
        Ok(FromMrb::from_mrb(interp, value))
    }
}
error: unnecessary structure name repetition
  --> mruby/src/convert.rs:50:17
   |
50 |     type From = To::From;
   |                 ^^ help: use the applicable keyword: `Self`
   |
note: lint level defined here
  --> mruby/src/lib.rs:1:22
   |
1  | #![deny(clippy::all, clippy::pedantic)]
   |                      ^^^^^^^^^^^^^^^^
   = note: #[deny(clippy::use_self)] implied by #[deny(clippy::pedantic)]
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#use_self

error: unnecessary structure name repetition
  --> mruby/src/convert.rs:51:15
   |
51 |     type To = To::To;
   |               ^^ help: use the applicable keyword: `Self`
   |
   = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#use_self
@lopopolo lopopolo changed the title Suggestion does not compile: use self with type parameters Suggestion does not compile: use self with associated types May 25, 2019
@lopopolo
Copy link
Author

This is a more minimal and self-contained example:

#![deny(clippy::all, clippy::pedantic)]

pub struct Error<From, To> {
    _from: From,
    _too: To,
}

pub trait From<T> {
    type From;
    type To;

    fn from(value: T) -> Self;
}

pub trait TryFrom<T>
where
    Self: Sized,
{
    type From;
    type To;

    fn try_from(value: T) -> Result<Self, Error<Self::From, Self::To>>;
}

impl<F, T> TryFrom<F> for T
where
    T: From<F>,
{
    type From = T::From;
    type To = T::To;

    fn try_from(value: F) -> Result<Self, Error<Self::From, Self::To>> {
        Ok(From::from(value))
    }
}

impl From<bool> for i64 {
    type From = bool;
    type To = Self;

    fn from(value: bool) -> Self {
        if value {
            100
        } else {
            0
        }
    }
}

fn main() {}

Following clippy's suggestions results in this compilation error:

$ cargo clippy
    Checking clippy-use-self v0.1.0 (/Users/lopopolo/Desktop/clippy-use-self)
error[E0275]: overflow evaluating the requirement `<T as TryFrom<F>>::To`
  --> src/main.rs:25:12
   |
25 | impl<F, T> TryFrom<F> for T
   |            ^^^^^^^^^^

error[E0275]: overflow evaluating the requirement `<T as TryFrom<F>>::From`
  --> src/main.rs:29:5
   |
29 |     type From = Self::From;
   |     ^^^^^^^^^^^^^^^^^^^^^^^

error[E0275]: overflow evaluating the requirement `<T as TryFrom<F>>::To`
  --> src/main.rs:30:5
   |
30 |     type To = Self::To;
   |     ^^^^^^^^^^^^^^^^^^^

error[E0275]: overflow evaluating the requirement `<T as TryFrom<F>>::From`
  --> src/main.rs:32:5
   |
32 | /     fn try_from(value: F) -> Result<Self, Error<Self::From, Self::To>> {
33 | |         Ok(From::from(value))
34 | |     }
   | |_____^

error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0275`.
error: Could not compile `clippy-use-self`.

To learn more, run the command again with --verbose.

@lopopolo
Copy link
Author

It seems the way to make the compiler and clippy happy is to do this:

impl<F, T> TryFrom<F> for T
where
    T: From<F>,
{
    type From = <Self as From<F>>::From;
    type To = <Self as From<F>>::To;

    fn try_from(value: F) -> Result<Self, Error<Self::From, Self::To>> {
        Ok(From::from(value))
    }
}

@flip1995 flip1995 added C-bug Category: Clippy is not doing the correct thing I-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied labels May 28, 2019
lopopolo added a commit to artichoke/ferrocarril that referenced this issue Jul 4, 2019
@phansch phansch added the L-suggestion Lint: Improving, adding or fixing lint suggestions label Aug 24, 2019
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
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 added a commit that referenced this issue Feb 12, 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.
@repi
Copy link

repi commented Mar 6, 2021

Really like the use_self lint, and this seems to be the last remaining issue that until it can be generally used which would be lovely once resolved.

@Y-Nak
Copy link
Contributor

Y-Nak commented Apr 14, 2021

@rustbot claim

@0969397110
Copy link

4140

kraktus added a commit to kraktus/rust-clippy that referenced this issue Oct 26, 2022
Previously the following wrong suggestion was given

```rust
impl Error for std::fmt::Error {
    fn custom<T: std::fmt::Display>(_msg: T) -> Self {
-        std::fmt::Error // Should lint
+        Self::Error // Should lint
    }
}
```

Also remove known problem line related to rust-lang#4140 since it's been closed, and refactor the lint
kraktus added a commit to kraktus/rust-clippy that referenced this issue Oct 26, 2022
Previously the following wrong suggestion was given

```rust
impl Error for std::fmt::Error {
    fn custom<T: std::fmt::Display>(_msg: T) -> Self {
-        std::fmt::Error // Should lint
+        Self::Error // Should lint
    }
}
```

Also remove known problem line related to rust-lang#4140 since it's been closed, and refactor the lint
bors added a commit that referenced this issue Oct 27, 2022
[`use_self`] fix suggestion when full path to struct was given

Previously the following wrong suggestion was given

```rust
impl Error for std::fmt::Error {
    fn custom<T: std::fmt::Display>(_msg: T) -> Self {
-        std::fmt::Error // Should lint
+        Self::Error // Should lint
    }
}
```

Also remove known problem line related to #4140 since it's been closed, and refactor the lint

changelog: [`use_self`] fix suggestion when full path to struct was given
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-suggestion-causes-error Issue: The suggestions provided by this Lint cause an ICE/error when applied L-suggestion Lint: Improving, adding or fixing lint suggestions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants