apply review

Author: mockersf

src/librustc_resolve/lib.rs

@@ -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(),
@@ -4783,10 +4774,9 @@ 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";
+            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()) {
+            if let Ok(snippet) = cm.span_to_snippet(binding.span) {
                 let suggested_name = if name.as_str().chars().next().unwrap().is_uppercase() {
                     format!("Other{}", name)
                 } else {
@@ -4796,20 +4786,22 @@ impl<'a, 'crateloader: 'a> Resolver<'a, 'crateloader> {
                 err.span_suggestion_with_applicability(
                     binding.span,
                     rename_msg,
-                    if snippet.contains(" as ") {
-                        format!(
-                            "{} as {}",
+                    match (snippet.split_whitespace().find(|w| *w == "as"), snippet.ends_with(";")) {
+                        (Some(_), false) => format!("{} as {}",
                             &snippet[..snippet.find(" as ").unwrap()],
                             suggested_name,
-                        )
-                    } else {
-                        if snippet.ends_with(';') {
-                            format!("{} as {};", &snippet[..snippet.len() - 1], suggested_name)
-                        } else {
-                            format!("{} as {}", snippet, suggested_name)
-                        }
+                        ),
+                        (Some(_), true) => format!("{} as {};",
+                            &snippet[..snippet.find(" as ").unwrap()],
+                            suggested_name,
+                        ),
+                        (None, false) => format!("{} as {}", snippet, suggested_name),
+                        (None, true) => format!("{} as {};", 
+                            &snippet[..snippet.len() - 1],
+                            suggested_name
+                        ),
                     },
-                    Applicability::MachineApplicable,
+                    Applicability::MaybeIncorrect,
                 );
             } else {
                 err.span_label(binding.span, rename_msg);

src/test/ui/issues/issue-45829/issue-45829.rs

@@ -0,0 +1,18 @@
+// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+mod foo {
+    pub struct A;
+    pub struct B;
+}
+
+use foo::{A, B as A};
+
+fn main() {}

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

@@ -0,0 +1,17 @@
+error[E0252]: the name `A` is defined multiple times
+  --> $DIR/issue-45829.rs:16:14
+   |
+LL | 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
+   |
+LL | use foo::{A, B as OtherA};
+   |              ^^^^^^^^^^^
+
+error: aborting due to previous error
+
+For more information about this error, try `rustc --explain E0252`.

src/test/ui/issues/issue-45829/rename-extern-vs-use.rs

@@ -10,7 +10,11 @@
 
 // aux-build:issue_45829_b.rs
 
-use std;
-extern crate issue_45829_b as std;
+mod foo {
+    pub mod bar {}
+}
+
+use foo::bar;
+extern crate issue_45829_b as bar;
 
 fn main() {}

src/test/ui/issues/issue-45829/rename-extern-vs-use.stderr

@@ -1,27 +1,17 @@
-error[E0259]: the name `std` is defined multiple times
-  --> $DIR/rename-extern-vs-use.rs:14:1
+error[E0254]: the name `bar` is defined multiple times
+  --> $DIR/rename-extern-vs-use.rs:18:1
    |
-LL | extern crate issue_45829_b as std;
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-   | |
-   | `std` reimported here
-   | You can use `as` to change the binding name of the import
+LL | use foo::bar;
+   |     -------- previous import of the module `bar` here
+LL | extern crate issue_45829_b as bar;
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ `bar` reimported here
    |
-   = note: `std` must be defined only once in the type namespace of this module
-
-error[E0254]: the name `std` is defined multiple times
-  --> $DIR/rename-extern-vs-use.rs:13:5
-   |
-LL | use std;
-   |     ^^^ `std` reimported here
+   = note: `bar` 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
    |
-   = note: `std` 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 issue_45829_b as other_bar;
    |
-LL | use std as other_std;
-   |     ^^^^^^^^^^^^^^^^
 
-error: aborting due to 2 previous errors
+error: aborting due to previous error
 
-Some errors occurred: E0254, E0259.
-For more information about an error, try `rustc --explain E0254`.
+For more information about this error, try `rustc --explain E0254`.

src/test/ui/issues/issue-45829/rename-extern.stderr

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

src/test/ui/issues/issue-45829/rename-use-vs-extern.stderr

@@ -7,7 +7,7 @@ LL | use std as issue_45829_b;
    |     ^^^^^^^^^^^^^^^^^^^^ `issue_45829_b` reimported here
    |
    = note: `issue_45829_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 std as other_issue_45829_b;
    |     ^^^^^^^^^^^^^^^^^^^^^^^^^^

src/test/ui/issues/issue-45829/rename-with-path.stderr

@@ -7,7 +7,7 @@ LL | use std::{collections::HashMap as A, sync::Arc as A};
    |           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
+help: you can use `as` to change the binding name of the import
    |
 LL | use std::{collections::HashMap as A, sync::Arc as OtherA};
    |                                      ^^^^^^^^^^^^^^^^^^^

src/test/ui/issues/issue-45829/rename.stderr

@@ -7,7 +7,7 @@ LL | use std as core;
    |     ^^^^^^^^^^^ `core` reimported here
    |
    = note: `core` 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 as other_core;
    |     ^^^^^^^^^^^^^^^^^