lint #1674: replace struct name with Self when applicable

[tnielens]

Hello,

This tries to implement issue #1674. Let me know if it needs modifications in order to be merged.

Cheers

[oli-obk]

Just some nits and a suggestion would be great.

[oli-obk]

elemnt -> element

[oli-obk]

You can compare name with http://manishearth.github.io/rust-internals-docs/syntax_pos/symbol/keywords/constant.SelfType.html in order to not have to do as_str and compare with a string

[oli-obk]

Can you add a suggestion that suggests replacing the explicit name with Self?

Grep for span_suggestion if you need any examples on how to do it. If you run into any problems don't hesitate to ask

[oli-obk]

The span of this one isn't quite right. It should only point to the Foo part

[oli-obk]

Nevermind, this is a rustc bug. Everything's good

[oli-obk]

Thanks. This one will clean up a lot of code.

[tnielens]

Thank you for the review!

[oli-obk]

@montrivo this introduced some false positives. Do you want to take a crack at them?

Most notably, this triggers on #[derive(Copy, Clone)] on a struct (so you need an is_macro) check.

But it also triggers on things like fn foo<'b>(x: &'b str) -> Foo<'b>, where Foo<'b> can't be replaced by Self. Maybe just bail out if there are generic arguments for now?

[tnielens]

Yes. Should I add the lint to one of the groups? What tests exactly did you run to find out about the false positives?

[oli-obk]

I already fixed the Copy, Clone issue, since it was bugging me during clippy development. But the other issue is still there.

What tests exactly did you run to find out about the false positives?

I didn't run any, but I found them inside clippy (the utils/sugg.rs file to be exact)

An example is:

struct Foo<'a>(&'a str);
impl<'a> Foo<'a> {
    // Cannot use `Self` as return type, because the function is actually `fn foo<'b>(s: &'b str) -> Foo<'b>`
    fn foo(s: &str) -> Foo {
        Self(s)
    }
    // cannot replace with `Self`, because that's `Foo<'a>`
    fn foo() -> Foo<'static> {
        Self("foo")
    }
}

There are probably more issues once you have generic structs.

I think the only time the suggestion should come in impls, is when it is impl $something and the replacement is for $something exactly, without any changes in the generic or lifetime parameters. This also applies to impl SomeTrait for $something.

I think simply adding a lot of tests with various features like associated types, trait impls setting said associtated types to Self and all kinds of generics would go a long way to ensuring we don't break the lint in the future, too.

[Arnavion]

@oli-obk The lint also fires for #[derive(Debug)] struct Foo { } because the expanded code contains match *self { Foo { } => ... Your fix for Copy and Clone fixes this too, right?

[tnielens]

Is Self even supported for named tuples?
I get

------------------------------------------
stderr:
------------------------------------------
error[E0423]: expected function, found self type `Self`
  --> tests/ui/use_self.rs:61:13
   |
61 |             Self("foo")
   |             ^^^^ did you mean `Self { /* fields */ }`?

with

mod named_tuples {
    struct Foo(&'static str);
    impl Foo {
        fn new() -> Self {
            Self("foo")
        }
    }
}