Address review comments and cleanup code

src/librustc_resolve/lib.rs

@@ -5134,62 +5134,59 @@ impl<'a> Resolver<'a> {
  );
 
  // See https://github.com/rust-lang/rust/issues/32354
- if old_binding.is_import() || new_binding.is_import() {
- let binding = if new_binding.is_import() && !new_binding.span.is_dummy() {
- new_binding
+ let directive = match (&new_binding.kind, &old_binding.kind) {
+ (NameBindingKind::Import { directive, .. }, _) if !new_binding.span.is_dummy() =>
+ Some((directive, new_binding.span)),
+ (_, NameBindingKind::Import { directive, .. }) if !old_binding.span.is_dummy() =>
+ Some((directive, old_binding.span)),
+ _ => None,
+ };
+ if let Some((directive, binding_span)) = directive {
+ let suggested_name = if name.as_str().chars().next().unwrap().is_uppercase() {
+ format!("Other{}", name)
  } else {
- old_binding
+ format!("other_{}", name)
  };
 
- 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),
- NameBindingKind::Import { directive, ..},
- false,
- false,
- ) = (
- cm.span_to_snippet(binding.span),
- binding.kind.clone(),
- binding.span.is_dummy(),
- binding.span.ctxt().outer().expn_info().is_some(),
- ) {
- let suggested_name = if name.as_str().chars().next().unwrap().is_uppercase() {
- format!("Other{}", name)
- } else {
- format!("other_{}", name)
- };
+ let mut suggestion = None;
+ match directive.subclass {
+ ImportDirectiveSubclass::SingleImport { type_ns_only: true, .. } =>
+ suggestion = Some(format!("self as {}", suggested_name)),
+ ImportDirectiveSubclass::SingleImport { source, .. } => {
+ if let Some(pos) = source.span.hi().0.checked_sub(binding_span.lo().0)
+ .map(|pos| pos as usize) {
+ if let Ok(snippet) = self.session.source_map()
+ .span_to_snippet(binding_span) {
+ if pos <= snippet.len() {
+ suggestion = Some(format!(
+ "{} as {}{}",
+ &snippet[..pos],
+ suggested_name,
+ if snippet.ends_with(";") { ";" } else { "" }
+ ))
+ }
+ }
+ }
+ }
+ ImportDirectiveSubclass::ExternCrate { source, target, .. } =>
+ suggestion = Some(format!(
+ "extern crate {} as {};",
+ source.unwrap_or(target.name),
+ suggested_name,
+ )),
+ _ => unreachable!(),
+ }
 
+ let rename_msg = "you can use `as` to change the binding name of the import";
+ if let Some(suggestion) = suggestion {
  err.span_suggestion_with_applicability(
- binding.span,
- &rename_msg,
- match directive.subclass {
- ImportDirectiveSubclass::SingleImport { type_ns_only: true, .. } =>
- 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!(),
- },
+ binding_span,
+ rename_msg,
+ suggestion,
  Applicability::MaybeIncorrect,
  );
  } else {
- err.span_label(binding.span, rename_msg);
+ err.span_label(binding_span, rename_msg);
  }
  }
 

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

@@ -3,14 +3,14 @@ macro_rules! import {
  $(
  mod $name;
  pub use self::$name;
- //~^ ERROR the name `issue_56411` is defined multiple times
- //~| ERROR `issue_56411` is private, and cannot be re-exported
+ //~^ ERROR the name `issue_56411_aux` is defined multiple times
+ //~| ERROR `issue_56411_aux` is private, and cannot be re-exported
 
  )*
  }
 }
 
-import!(issue_56411);
+import!(issue_56411_aux);
 
 fn main() {
  println!("Hello, world!");

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

@@ -1,29 +1,29 @@
-error[E0255]: the name `issue_56411` is defined multiple times
+error[E0255]: the name `issue_56411_aux` is defined multiple times
  --> $DIR/issue-56411.rs:5:21
  |
 LL | mod $name;
- | ---------- previous definition of the module `issue_56411` here
+ | ---------- previous definition of the module `issue_56411_aux` here
 LL | pub use self::$name;
  | ^^^^^^^^^^^
  | |
- | `issue_56411` reimported here
+ | `issue_56411_aux` reimported here
  | you can use `as` to change the binding name of the import
 ...
-LL | import!(issue_56411);
- | --------------------- in this macro invocation
+LL | import!(issue_56411_aux);
+ | ------------------------- in this macro invocation
  |
- = note: `issue_56411` must be defined only once in the type namespace of this module
+ = note: `issue_56411_aux` must be defined only once in the type namespace of this module
 
-error[E0365]: `issue_56411` is private, and cannot be re-exported
+error[E0365]: `issue_56411_aux` is private, and cannot be re-exported
  --> $DIR/issue-56411.rs:5:21
  |
 LL | pub use self::$name;
- | ^^^^^^^^^^^ re-export of private `issue_56411`
+ | ^^^^^^^^^^^ re-export of private `issue_56411_aux`
 ...
-LL | import!(issue_56411);
- | --------------------- in this macro invocation
+LL | import!(issue_56411_aux);
+ | ------------------------- in this macro invocation
  |
- = note: consider declaring type or module `issue_56411` with `pub`
+ = note: consider declaring type or module `issue_56411_aux` with `pub`
 
 error: aborting due to 2 previous errors