Fixed 53359: E0432 unresolved import on the same line is now emiting one diagnostic

PramodBisht · PramodBisht · commit 21ba03e2b910 · 2018-09-08T15:11:04.000Z
Addressed estebank's comments for 53359
diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs
@@ -160,8 +160,6 @@ enum ResolutionError<'a> {
     SelfImportCanOnlyAppearOnceInTheList,
     /// error E0431: `self` import can only appear in an import list with a non-empty prefix
     SelfImportOnlyInImportListWithNonEmptyPrefix,
-    /// error E0432: unresolved import
-    UnresolvedImport(Option<(Span, &'a str, &'a str)>),
     /// error E0433: failed to resolve
     FailedToResolve(&'a str),
     /// error E0434: can't capture dynamic environment in a fn item
@@ -357,17 +355,6 @@ fn resolve_struct_error<'sess, 'a>(resolver: &'sess Resolver,
             err.span_label(span, "can only appear in an import list with a non-empty prefix");
             err
         }
-        ResolutionError::UnresolvedImport(name) => {
-            let (span, msg) = match name {
-                Some((sp, n, _)) => (sp, format!("unresolved import `{}`", n)),
-                None => (span, "unresolved import".to_owned()),
-            };
-            let mut err = struct_span_err!(resolver.session, span, E0432, "{}", msg);
-            if let Some((_, _, p)) = name {
-                err.span_label(span, p);
-            }
-            err
-        }
         ResolutionError::FailedToResolve(msg) => {
             let mut err = struct_span_err!(resolver.session, span, E0433,
                                            "failed to resolve. {}", msg);
diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
@@ -31,7 +31,7 @@ use syntax::ext::base::Determinacy::{self, Determined, Undetermined};
 use syntax::ext::hygiene::Mark;
 use syntax::symbol::keywords;
 use syntax::util::lev_distance::find_best_match_for_name;
-use syntax_pos::Span;
+use syntax_pos::{MultiSpan, Span};
 
 use std::cell::{Cell, RefCell};
 use std::collections::BTreeMap;
@@ -635,6 +635,8 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
 
         let mut errors = false;
         let mut seen_spans = FxHashSet();
+        let mut error_vec = Vec::new();
+        let mut prev_root_id: NodeId = NodeId::new(0);
         for i in 0 .. self.determined_imports.len() {
             let import = self.determined_imports[i];
             let error = self.finalize_import(import);
@@ -689,13 +691,22 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
                 // If the error is a single failed import then create a "fake" import
                 // resolution for it so that later resolve stages won't complain.
                 self.import_dummy_binding(import);
+                if prev_root_id.as_u32() != 0 &&
+                    prev_root_id.as_u32() != import.root_id.as_u32() &&
+                    !error_vec.is_empty(){
+                    // in case of new import line, throw diagnostic message
+                    // for previous line.
+                    let mut empty_vec = vec![];
+                    mem::swap(&mut empty_vec, &mut error_vec);
+                    self.throw_unresolved_import_error(empty_vec, None);
+                }
                 if !seen_spans.contains(&span) {
                     let path = import_path_to_string(&import.module_path[..],
                                                      &import.subclass,
                                                      span);
-                    let error = ResolutionError::UnresolvedImport(Some((span, &path, &err)));
-                    resolve_error(self.resolver, span, error);
+                    error_vec.push((span, path, err));
                     seen_spans.insert(span);
+                    prev_root_id = import.root_id;
                 }
             }
         }
@@ -760,21 +771,48 @@ impl<'a, 'b:'a, 'c: 'b> ImportResolver<'a, 'b, 'c> {
             });
         }
 
+        if !error_vec.is_empty() {
+            self.throw_unresolved_import_error(error_vec.clone(), None);
+        }
+
         // Report unresolved imports only if no hard error was already reported
         // to avoid generating multiple errors on the same import.
         if !errors {
             for import in &self.indeterminate_imports {
                 if import.is_uniform_paths_canary {
                     continue;
                 }
-
-                let error = ResolutionError::UnresolvedImport(None);
-                resolve_error(self.resolver, import.span, error);
+                self.throw_unresolved_import_error(error_vec, Some(MultiSpan::from(import.span)));
                 break;
             }
         }
     }
 
+    fn throw_unresolved_import_error(&self, error_vec: Vec<(Span, String, String)>,
+                                     span: Option<MultiSpan>) {
+        let max_span_label_msg_count = 10;  // upper limit on number of span_label message.
+        let (span,msg) = match error_vec.is_empty() {
+            true => (span.unwrap(), "unresolved import".to_string()),
+            false => {
+                let span = MultiSpan::from_spans(error_vec.clone().into_iter()
+                                    .map(|elem: (Span, String, String)| { elem.0 }
+                                    ).collect());
+                let path_vec: Vec<String> = error_vec.clone().into_iter()
+                                .map(|elem: (Span, String, String)| { format!("`{}`", elem.1) }
+                                ).collect();
+                let path = path_vec.join(", ");
+                let msg = format!("unresolved import{} {}",
+                                if path_vec.len() > 1 { "s" } else { "" },  path);
+                (span, msg)
+            }
+        };
+        let mut err = struct_span_err!(self.resolver.session, span, E0432, "{}", &msg);
+        for span_error in error_vec.into_iter().take(max_span_label_msg_count) {
+            err.span_label(span_error.0, span_error.2);
+        }
+        err.emit();
+    }
+
     /// Attempts to resolve the given import, returning true if its resolution is determined.
     /// If successful, the resolved bindings are written into the module.
     fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> bool {
diff --git a/src/test/ui/dollar-crate/dollar-crate-is-keyword-2.stderr b/src/test/ui/dollar-crate/dollar-crate-is-keyword-2.stderr
@@ -1,17 +1,17 @@
-error[E0432]: unresolved import `a::$crate`
-  --> $DIR/dollar-crate-is-keyword-2.rs:15:13
+error[E0433]: failed to resolve. `$crate` in paths can only be used in start position
+  --> $DIR/dollar-crate-is-keyword-2.rs:16:16
    |
-LL |         use a::$crate; //~ ERROR unresolved import `a::$crate`
-   |             ^^^^^^^^^ no `$crate` in `a`
+LL |         use a::$crate::b; //~ ERROR `$crate` in paths can only be used in start position
+   |                ^^^^^^ `$crate` in paths can only be used in start position
 ...
 LL | m!();
    | ----- in this macro invocation
 
-error[E0433]: failed to resolve. `$crate` in paths can only be used in start position
-  --> $DIR/dollar-crate-is-keyword-2.rs:16:16
+error[E0432]: unresolved import `a::$crate`
+  --> $DIR/dollar-crate-is-keyword-2.rs:15:13
    |
-LL |         use a::$crate::b; //~ ERROR `$crate` in paths can only be used in start position
-   |                ^^^^^^ `$crate` in paths can only be used in start position
+LL |         use a::$crate; //~ ERROR unresolved import `a::$crate`
+   |             ^^^^^^^^^ no `$crate` in `a`
 ...
 LL | m!();
    | ----- in this macro invocation
diff --git a/src/test/ui/issue-53565.rs b/src/test/ui/issue-53565.rs
@@ -0,0 +1,14 @@
+// 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.
+use std::time::{foo, bar, buzz};
+use std::time::{abc, def};
+fn main(){
+   println!("Hello World!");
+}
diff --git a/src/test/ui/issue-53565.stderr b/src/test/ui/issue-53565.stderr
@@ -0,0 +1,20 @@
+error[E0432]: unresolved imports `std::time::foo`, `std::time::bar`, `std::time::buzz`
+  --> $DIR/issue-53565.rs:10:17
+   |
+LL | use std::time::{foo, bar, buzz};
+   |                 ^^^  ^^^  ^^^^ no `buzz` in `time`
+   |                 |    |
+   |                 |    no `bar` in `time`
+   |                 no `foo` in `time`
+
+error[E0432]: unresolved imports `std::time::abc`, `std::time::def`
+  --> $DIR/issue-53565.rs:11:17
+   |
+LL | use std::time::{abc, def};
+   |                 ^^^  ^^^ no `def` in `time`
+   |                 |
+   |                 no `abc` in `time`
+
+error: aborting due to 2 previous errors
+
+For more information about this error, try `rustc --explain E0432`.
