Error message for duplicate names suggests invalid Rust

[Twey]

Consider the following library (with hello and aleph externs provided):

extern crate hello;
extern crate aleph as hello;

rustc will reasonably identify a problem:

error[E0259]: the name `hello` is defined multiple times
 --> lib.rs:2:1
  |
1 | extern crate hello;
  | ------------------- previous import of the extern crate `hello` here
2 | extern crate aleph as hello;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `hello` reimported here
  |
  = note: `hello` must be defined only once in the type namespace of this module

but will then go on to suggest a rather unhelpful solution that isn't even valid Rust code:

help: You can use `as` to change the binding name of the import
  |
2 | extern crate aleph as hello; as Otherhello
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[estebank]

Related to #45799.

#45660 should verify wether the crate has already been renamed before providing the help. I believe this can be done by checking wether the NameBinding.kind::<Import> fields target and source are the same.

[Cldfire]

@Twey, thanks for noticing that and filing an issue 😄

And @estebank, thanks for the instructions. I'll add a commit that fixes this as a part of #45820 when I get the chance.

EDIT: Looks like you've got it handled 👍

[pietroalbini]

This also happens with uses:

#![allow(unused_imports)]

mod foo {
    pub struct A;
    pub struct B;
}

use foo::{A, B as A};

fn main() {}

error[E0252]: the name `A` is defined multiple times
 --> src/main.rs:8:14
  |
8 | use foo::{A, B as A};
  |           -  ^^^^^^ `A` reimported here
  |           |
  |           previous import of the type `A` here
  |
  = note: `A` must be defined only once in the type namespace of this module
help: You can use `as` to change the binding name of the import
  |
8 | use foo::{A, B as A as OtherA};
  |              ^^^^^^^^^^^^^^^^

[estebank]

@pietroalbini thanks for fishing this out! The fix for use will likely be very similar to #45856.

[mockersf]

Hello ! I took a shot at this.

I noticed that rust no longer suggests a new name for an extern crate import that has already been renamed.
This is because the suggestion is behind this check (https://github.com/rust-lang/rust/blob/master/src/librustc_resolve/lib.rs#L4788):

           if let (Ok(snippet), false) = (cm.span_to_snippet(binding.span),
                                           binding.is_renamed_extern_crate()) {

I didn't change that in #55113 in case it was done willingly, but that should be easy to change.

[estebank]

@mockersf commented on the PR. If you can apply the fix to extern statements as well, go ahead :)