Auto merge of #55113 - mockersf:master, r=estebank

#45829 when a renamed import conflict with a previous import

Fix the suggestion when a renamed import conflict.

It check if the snipped contains `" as "`, and if so uses everything before for the suggestion.

src/librustc_resolve/build_reduced_graph.rs

@@ -449,7 +449,10 @@ impl<'a, 'cl> Resolver<'a, 'cl> {
                     id: item.id,
                     parent,
                     imported_module: Cell::new(Some(ModuleOrUniformRoot::Module(module))),
-                    subclass: ImportDirectiveSubclass::ExternCrate(orig_name),
+                    subclass: ImportDirectiveSubclass::ExternCrate {
+                        source: orig_name,
+                        target: ident,
+                    },
                     root_span: item.span,
                     span: item.span,
                     module_path: Vec::new(),

src/librustc_resolve/check_unused.rs

@@ -144,7 +144,7 @@ pub fn check_crate(resolver: &mut Resolver, krate: &ast::Crate) {
                     }
                 }
             }
-            ImportDirectiveSubclass::ExternCrate(_) => {
+            ImportDirectiveSubclass::ExternCrate { .. } => {
                 resolver.maybe_unused_extern_crates.push((directive.id, directive.span));
             }
             ImportDirectiveSubclass::MacroUse => {

src/librustc_resolve/lib.rs

@@ -1234,7 +1234,7 @@ impl<'a> NameBinding<'a> {
         match self.kind {
             NameBindingKind::Import {
                 directive: &ImportDirective {
-                    subclass: ImportDirectiveSubclass::ExternCrate(_), ..
+                    subclass: ImportDirectiveSubclass::ExternCrate { .. }, ..
                 }, ..
             } => true,
             _ => false,
@@ -1248,15 +1248,6 @@ impl<'a> NameBinding<'a> {
         }
     }
 
-    fn is_renamed_extern_crate(&self) -> bool {
-        if let NameBindingKind::Import { directive, ..} = self.kind {
-            if let ImportDirectiveSubclass::ExternCrate(Some(_)) = directive.subclass {
-                return true;
-            }
-        }
-        false
-    }
-
     fn is_glob_import(&self) -> bool {
         match self.kind {
             NameBindingKind::Import { directive, .. } => directive.is_glob(),
@@ -3812,7 +3803,7 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             if let NameBindingKind::Import { directive: d, .. } = binding.kind {
                 // Careful: we still want to rewrite paths from
                 // renamed extern crates.
-                if let ImportDirectiveSubclass::ExternCrate(None) = d.subclass {
+                if let ImportDirectiveSubclass::ExternCrate { source: None, .. } = d.subclass {
                     return
                 }
             }
@@ -4782,10 +4773,17 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
             };
 
             let cm = self.session.source_map();
-            let rename_msg = "You can use `as` to change the binding name of the import";
-
-            if let (Ok(snippet), false) = (cm.span_to_snippet(binding.span),
-                                           binding.is_renamed_extern_crate()) {
+            let rename_msg = "you can use `as` to change the binding name of the import";
+
+            if let (
+                Ok(snippet),
+                NameBindingKind::Import { directive, ..},
+                _dummy @ false,
+            ) = (
+                cm.span_to_snippet(binding.span),
+                binding.kind.clone(),
+                binding.span.is_dummy(),
+            ) {
                 let suggested_name = if name.as_str().chars().next().unwrap().is_uppercase() {
                     format!("Other{}", name)
                 } else {
@@ -4794,13 +4792,30 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
 
                 err.span_suggestion_with_applicability(
                     binding.span,
-                    rename_msg,
-                    if snippet.ends_with(';') {
-                        format!("{} as {};", &snippet[..snippet.len() - 1], suggested_name)
-                    } else {
-                        format!("{} as {}", snippet, suggested_name)
+                    &rename_msg,
+                    match (&directive.subclass, snippet.as_ref()) {
+                        (ImportDirectiveSubclass::SingleImport { .. }, "self") =>
+                            format!("self as {}", suggested_name),
+                        (ImportDirectiveSubclass::SingleImport { source, .. }, _) =>
+                            format!(
+                                "{} as {}{}",
+                                &snippet[..((source.span.hi().0 - binding.span.lo().0) as usize)],
+                                suggested_name,
+                                if snippet.ends_with(";") {
+                                    ";"
+                                } else {
+                                    ""
+                                }
+                            ),
+                        (ImportDirectiveSubclass::ExternCrate { source, target, .. }, _) =>
+                            format!(
+                                "extern crate {} as {};",
+                                source.unwrap_or(target.name),
+                                suggested_name,
+                            ),
+                        (_, _) => unreachable!(),
                     },
-                    Applicability::MachineApplicable,
+                    Applicability::MaybeIncorrect,
                 );
             } else {
                 err.span_label(binding.span, rename_msg);

src/librustc_resolve/resolve_imports.rs

@@ -52,7 +52,10 @@ pub enum ImportDirectiveSubclass<'a> {
         max_vis: Cell<ty::Visibility>, // The visibility of the greatest re-export.
         // n.b. `max_vis` is only used in `finalize_import` to check for re-export errors.
     },
-    ExternCrate(Option<Name>),
+    ExternCrate {
+        source: Option<Name>,
+        target: Ident,
+    },
     MacroUse,
 }
 
@@ -1336,7 +1339,7 @@ fn import_directive_subclass_to_string(subclass: &ImportDirectiveSubclass) -> St
     match *subclass {
         SingleImport { source, .. } => source.to_string(),
         GlobImport { .. } => "*".to_string(),
-        ExternCrate(_) => "<extern crate>".to_string(),
+        ExternCrate { .. } => "<extern crate>".to_string(),
         MacroUse => "#[macro_use]".to_string(),
     }
 }

src/test/ui/blind/blind-item-block-item-shadow.stderr

@@ -7,7 +7,7 @@ LL |         use foo::Bar;
    |             ^^^^^^^^ `Bar` reimported here
    |
    = note: `Bar` must be defined only once in the type namespace of this block
-help: You can use `as` to change the binding name of the import
+help: you can use `as` to change the binding name of the import
    |
 LL |         use foo::Bar as OtherBar;
    |             ^^^^^^^^^^^^^^^^^^^^

src/test/ui/blind/blind-item-item-shadow.stderr

@@ -8,7 +8,7 @@ LL | use foo::foo;
    |     ^^^^^^^^ `foo` reimported here
    |
    = note: `foo` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use foo::foo as other_foo;
    |     ^^^^^^^^^^^^^^^^^^^^^

src/test/ui/double-import.stderr

@@ -7,7 +7,7 @@ LL | use sub2::foo; //~ ERROR the name `foo` is defined multiple times
    |     ^^^^^^^^^ `foo` reimported here
    |
    = note: `foo` must be defined only once in the value namespace of this module
-help: You can use `as` to change the binding name of the import
+help: you can use `as` to change the binding name of the import
    |
 LL | use sub2::foo as other_foo; //~ ERROR the name `foo` is defined multiple times
    |     ^^^^^^^^^^^^^^^^^^^^^^

src/test/ui/double-type-import.stderr

@@ -7,7 +7,7 @@ LL |     use self::bar::X;
    |         ^^^^^^^^^^^^ `X` reimported here
    |
    = note: `X` 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
+help: you can use `as` to change the binding name of the import
    |
 LL |     use self::bar::X as OtherX;
    |         ^^^^^^^^^^^^^^^^^^^^^^

src/test/ui/duplicate/duplicate-check-macro-exports.stderr

@@ -8,7 +8,7 @@ LL | macro_rules! panic { () => {} } //~ ERROR the name `panic` is defined multi
    | ^^^^^^^^^^^^^^^^^^ `panic` redefined here
    |
    = note: `panic` must be defined only once in the macro namespace of this module
-help: You can use `as` to change the binding name of the import
+help: you can use `as` to change the binding name of the import
    |
 LL | pub use std::panic as other_panic;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^

src/test/ui/error-codes/E0252.stderr

@@ -7,7 +7,7 @@ LL | use bar::baz; //~ ERROR E0252
    |     ^^^^^^^^ `baz` reimported here
    |
    = note: `baz` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use bar::baz as other_baz; //~ ERROR E0252
    |     ^^^^^^^^^^^^^^^^^^^^^

src/test/ui/error-codes/E0254.stderr

@@ -8,7 +8,7 @@ LL | use foo::alloc;
    |     ^^^^^^^^^^ `alloc` reimported here
    |
    = note: `alloc` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use foo::alloc as other_alloc;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^

src/test/ui/error-codes/E0255.stderr

@@ -8,7 +8,7 @@ LL | fn foo() {} //~ ERROR E0255
    | ^^^^^^^^ `foo` redefined here
    |
    = note: `foo` must be defined only once in the value namespace of this module
-help: You can use `as` to change the binding name of the import
+help: you can use `as` to change the binding name of the import
    |
 LL | use bar::foo as other_foo;
    |     ^^^^^^^^^^^^^^^^^^^^^

src/test/ui/error-codes/E0259.stderr

@@ -5,12 +5,13 @@ LL | extern crate alloc;
    | ------------------- previous import of the extern crate `alloc` here
 LL | 
 LL | extern crate libc as alloc;
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   | |
-   | `alloc` reimported here
-   | You can use `as` to change the binding name of the import
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `alloc` reimported here
    |
    = note: `alloc` 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
+   |
+LL | extern crate libc as other_alloc;
+   |
 
 error: aborting due to previous error
 

src/test/ui/error-codes/E0260.stderr

@@ -8,7 +8,7 @@ LL | mod alloc {
    | ^^^^^^^^^ `alloc` redefined here
    |
    = note: `alloc` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | extern crate alloc as other_alloc;
    |

src/test/ui/error-codes/E0430.stderr

@@ -15,7 +15,7 @@ LL | use std::fmt::{self, self}; //~ ERROR E0430
    |                previous import of the module `fmt` here
    |
    = note: `fmt` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use std::fmt::{self, self as other_fmt}; //~ ERROR E0430
    |                      ^^^^^^^^^^^^^^^^^

src/test/ui/extern/extern-crate-rename.stderr

@@ -4,12 +4,13 @@ error[E0259]: the name `m1` is defined multiple times
 LL | extern crate m1;
    | ---------------- previous import of the extern crate `m1` here
 LL | extern crate m2 as m1; //~ ERROR is defined multiple times
-   | ^^^^^^^^^^^^^^^^^^^^^^
-   | |
-   | `m1` reimported here
-   | You can use `as` to change the binding name of the import
+   | ^^^^^^^^^^^^^^^^^^^^^^ `m1` reimported here
    |
    = note: `m1` 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
+   |
+LL | extern crate m2 as other_m1; //~ ERROR is defined multiple times
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 error: aborting due to previous error
 

src/test/ui/imports/duplicate.stderr

@@ -7,7 +7,7 @@ LL |     use a::foo; //~ ERROR the name `foo` is defined multiple times
    |         ^^^^^^ `foo` reimported here
    |
    = note: `foo` must be defined only once in the value namespace of this module
-help: You can use `as` to change the binding name of the import
+help: you can use `as` to change the binding name of the import
    |
 LL |     use a::foo as other_foo; //~ ERROR the name `foo` is defined multiple times
    |         ^^^^^^^^^^^^^^^^^^^

src/test/ui/issues/issue-19498.stderr

@@ -8,7 +8,7 @@ LL | mod A {} //~ ERROR the name `A` is defined multiple times
    | ^^^^^ `A` redefined 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use self::A as OtherA;
    |     ^^^^^^^^^^^^^^^^^
@@ -23,7 +23,7 @@ LL | pub mod B {} //~ ERROR the name `B` is defined multiple times
    | ^^^^^^^^^ `B` redefined here
    |
    = note: `B` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use self::B as OtherB;
    |     ^^^^^^^^^^^^^^^^^
@@ -37,7 +37,7 @@ LL |     mod D {} //~ ERROR the name `D` is defined multiple times
    |     ^^^^^ `D` redefined here
    |
    = note: `D` 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
+help: you can use `as` to change the binding name of the import
    |
 LL |     use C::D as OtherD;
    |         ^^^^^^^^^^^^^^

src/test/ui/issues/issue-24081.stderr

@@ -8,7 +8,7 @@ LL | type Add = bool; //~ ERROR the name `Add` is defined multiple times
    | ^^^^^^^^^^^^^^^^ `Add` redefined here
    |
    = note: `Add` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use std::ops::Add as OtherAdd;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -23,7 +23,7 @@ LL | struct Sub { x: f32 } //~ ERROR the name `Sub` is defined multiple times
    | ^^^^^^^^^^ `Sub` redefined here
    |
    = note: `Sub` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use std::ops::Sub as OtherSub;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -38,7 +38,7 @@ LL | enum Mul { A, B } //~ ERROR the name `Mul` is defined multiple times
    | ^^^^^^^^ `Mul` redefined here
    |
    = note: `Mul` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use std::ops::Mul as OtherMul;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -53,7 +53,7 @@ LL | mod Div { } //~ ERROR the name `Div` is defined multiple times
    | ^^^^^^^ `Div` redefined here
    |
    = note: `Div` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use std::ops::Div as OtherDiv;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -68,7 +68,7 @@ LL | trait Rem {  } //~ ERROR the name `Rem` is defined multiple times
    | ^^^^^^^^^ `Rem` redefined here
    |
    = note: `Rem` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use std::ops::Rem as OtherRem;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^

src/test/ui/issues/issue-25396.stderr

@@ -7,7 +7,7 @@ LL | use bar::baz; //~ ERROR the name `baz` is defined multiple times
    |     ^^^^^^^^ `baz` reimported here
    |
    = note: `baz` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use bar::baz as other_baz; //~ ERROR the name `baz` is defined multiple times
    |     ^^^^^^^^^^^^^^^^^^^^^
@@ -21,7 +21,7 @@ LL | use bar::Quux; //~ ERROR the name `Quux` is defined multiple times
    |     ^^^^^^^^^ `Quux` reimported here
    |
    = note: `Quux` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use bar::Quux as OtherQuux; //~ ERROR the name `Quux` is defined multiple times
    |     ^^^^^^^^^^^^^^^^^^^^^^
@@ -35,7 +35,7 @@ LL | use bar::blah; //~ ERROR the name `blah` is defined multiple times
    |     ^^^^^^^^^ `blah` reimported here
    |
    = note: `blah` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use bar::blah as other_blah; //~ ERROR the name `blah` is defined multiple times
    |     ^^^^^^^^^^^^^^^^^^^^^^^
@@ -49,7 +49,7 @@ LL | use bar::WOMP; //~ ERROR the name `WOMP` is defined multiple times
    |     ^^^^^^^^^ `WOMP` reimported here
    |
    = note: `WOMP` must be defined only once in the value namespace of this module
-help: You can use `as` to change the binding name of the import
+help: you can use `as` to change the binding name of the import
    |
 LL | use bar::WOMP as OtherWOMP; //~ ERROR the name `WOMP` is defined multiple times
    |     ^^^^^^^^^^^^^^^^^^^^^^

src/test/ui/issues/issue-26886.stderr

@@ -7,7 +7,7 @@ LL | use std::sync::Arc; //~ ERROR the name `Arc` is defined multiple times
    |     ^^^^^^^^^^^^^^ `Arc` reimported here
    |
    = note: `Arc` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use std::sync::Arc as OtherArc; //~ ERROR the name `Arc` is defined multiple times
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^
@@ -22,7 +22,7 @@ LL | use std::sync; //~ ERROR the name `sync` is defined multiple times
    |     ^^^^^^^^^ `sync` reimported here
    |
    = note: `sync` 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
+help: you can use `as` to change the binding name of the import
    |
 LL | use std::sync as other_sync; //~ ERROR the name `sync` is defined multiple times
    |     ^^^^^^^^^^^^^^^^^^^^^^^