Skip to content

Commit 552ff07

Browse files
estebankestebank
committed
Point at path segment on module not found
Point at the correct path segment on a import statement where a module doesn't exist. New output: ```rust error[E0432]: unresolved import `std::bar` --> <anon>:1:10 | 1 | use std::bar::{foo1, foo2}; | ^^^ Could not find `bar` in `std` ``` instead of: ```rust error[E0432]: unresolved import `std::bar::foo1` --> <anon>:1:16 | 1 | use std::bar::{foo1, foo2}; | ^^^^ Could not find `bar` in `std` error[E0432]: unresolved import `std::bar::foo2` --> <anon>:1:22 | 1 | use std::bar::{foo1, foo2}; | ^^^^ Could not find `bar` in `std` ```
1 parent 6c94965 commit 552ff07

18 files changed

+194
-129
lines changed

‎src/librustc_resolve/build_reduced_graph.rs

+14-8
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ use syntax::attr;
3535
use syntax::ast::{self, Block, ForeignItem, ForeignItemKind, Item, ItemKind};
3636
use syntax::ast::{Mutability, StmtKind, TraitItem, TraitItemKind};
3737
use syntax::ast::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple};
38+
use syntax::codemap::respan;
3839
use syntax::ext::base::SyntaxExtension;
3940
use syntax::ext::base::Determinacy::Undetermined;
4041
use syntax::ext::hygiene::Mark;
@@ -119,22 +120,24 @@ impl<'a> Resolver<'a> {
119120
.unwrap()
120121
.1
121122
.iter()
122-
.map(|seg| seg.identifier)
123+
.map(|seg| respan(seg.span, seg.identifier))
123124
.collect()
124125
}
125126

126127
ViewPathGlob(ref module_ident_path) |
127128
ViewPathList(ref module_ident_path, _) => {
128129
module_ident_path.segments
129130
.iter()
130-
.map(|seg| seg.identifier)
131+
.map(|seg| respan(seg.span, seg.identifier))
131132
.collect()
132133
}
133134
};
134135

135136
// This can be removed once warning cycle #36888 is complete.
136-
if module_path.len() >= 2 && module_path[0].name == keywords::CrateRoot.name() &&
137-
token::Ident(module_path[1]).is_path_segment_keyword() {
137+
if module_path.len() >= 2 &&
138+
module_path[0].node.name == keywords::CrateRoot.name() &&
139+
token::Ident(module_path[1].node).is_path_segment_keyword()
140+
{
138141
module_path.remove(0);
139142
}
140143

@@ -202,10 +205,13 @@ impl<'a> Resolver<'a> {
202205
let (module_path, ident, rename, type_ns_only) = {
203206
if node.name.name != keywords::SelfValue.name() {
204207
let rename = node.rename.unwrap_or(node.name);
205-
(module_path.clone(), node.name, rename, false)
208+
(module_path.clone(),
209+
respan(source_item.span, node.name),
210+
rename,
211+
false)
206212
} else {
207213
let ident = *module_path.last().unwrap();
208-
if ident.name == keywords::CrateRoot.name() {
214+
if ident.node.name == keywords::CrateRoot.name() {
209215
resolve_error(
210216
self,
211217
source_item.span,
@@ -215,13 +221,13 @@ impl<'a> Resolver<'a> {
215221
continue;
216222
}
217223
let module_path = module_path.split_last().unwrap().1;
218-
let rename = node.rename.unwrap_or(ident);
224+
let rename = node.rename.unwrap_or(ident.node);
219225
(module_path.to_vec(), ident, rename, true)
220226
}
221227
};
222228
let subclass = SingleImport {
223229
target: rename,
224-
source: ident,
230+
source: ident.node,
225231
result: self.per_ns(|_, _| Cell::new(Err(Undetermined))),
226232
type_ns_only: type_ns_only,
227233
};

‎src/librustc_resolve/lib.rs

+80-61
Large diffs are not rendered by default.

‎src/librustc_resolve/macros.rs

+12-7
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use rustc::hir::map::{self, DefCollector};
1919
use rustc::{ty, lint};
2020
use syntax::ast::{self, Name, Ident};
2121
use syntax::attr::{self, HasAttrs};
22+
use syntax::codemap::respan;
2223
use syntax::errors::DiagnosticBuilder;
2324
use syntax::ext::base::{self, Annotatable, Determinacy, MultiModifier, MultiDecorator};
2425
use syntax::ext::base::{MacroKind, SyntaxExtension, Resolver as SyntaxResolver};
@@ -393,7 +394,7 @@ impl<'a> Resolver<'a> {
393394
return Err(Determinacy::Determined);
394395
}
395396

396-
let path: Vec<_> = segments.iter().map(|seg| seg.identifier).collect();
397+
let path: Vec<_> = segments.iter().map(|seg| respan(seg.span, seg.identifier)).collect();
397398
let invocation = self.invocations[&scope];
398399
self.current_module = invocation.module.get();
399400

@@ -418,16 +419,19 @@ impl<'a> Resolver<'a> {
418419
Err(Determinacy::Determined)
419420
},
420421
};
422+
let path = path.iter().map(|p| p.node).collect::<Vec<_>>();
421423
self.current_module.nearest_item_scope().macro_resolutions.borrow_mut()
422424
.push((path.into_boxed_slice(), span));
423425
return def;
424426
}
425427

426-
let legacy_resolution = self.resolve_legacy_scope(&invocation.legacy_scope, path[0], false);
428+
let legacy_resolution = self.resolve_legacy_scope(&invocation.legacy_scope,
429+
path[0].node,
430+
false);
427431
let result = if let Some(MacroBinding::Legacy(binding)) = legacy_resolution {
428432
Ok(Def::Macro(binding.def_id, MacroKind::Bang))
429433
} else {
430-
match self.resolve_lexical_macro_path_segment(path[0], MacroNS, false, span) {
434+
match self.resolve_lexical_macro_path_segment(path[0].node, MacroNS, false, span) {
431435
Ok(binding) => Ok(binding.binding().def_ignoring_ambiguity()),
432436
Err(Determinacy::Undetermined) if !force => return Err(Determinacy::Undetermined),
433437
Err(_) => {
@@ -438,7 +442,7 @@ impl<'a> Resolver<'a> {
438442
};
439443

440444
self.current_module.nearest_item_scope().legacy_macro_resolutions.borrow_mut()
441-
.push((scope, path[0], span, kind));
445+
.push((scope, path[0].node, span, kind));
442446

443447
result
444448
}
@@ -576,9 +580,10 @@ impl<'a> Resolver<'a> {
576580
pub fn finalize_current_module_macro_resolutions(&mut self) {
577581
let module = self.current_module;
578582
for &(ref path, span) in module.macro_resolutions.borrow().iter() {
579-
match self.resolve_path(path, Some(MacroNS), true, span) {
583+
let path = path.iter().map(|p| respan(span, *p)).collect::<Vec<_>>();
584+
match self.resolve_path(&path, Some(MacroNS), true, span) {
580585
PathResult::NonModule(_) => {},
581-
PathResult::Failed(msg, _) => {
586+
PathResult::Failed(span, msg, _) => {
582587
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));
583588
}
584589
_ => unreachable!(),
@@ -652,7 +657,7 @@ impl<'a> Resolver<'a> {
652657
}
653658
};
654659
let ident = Ident::from_str(name);
655-
self.lookup_typo_candidate(&vec![ident], MacroNS, is_macro, span)
660+
self.lookup_typo_candidate(&vec![respan(span, ident)], MacroNS, is_macro, span)
656661
});
657662

658663
if let Some(suggestion) = suggestion {

‎src/librustc_resolve/resolve_imports.rs

+44-27
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ use rustc::ty;
2121
use rustc::lint::builtin::PUB_USE_OF_PRIVATE_EXTERN_CRATE;
2222
use rustc::hir::def_id::DefId;
2323
use rustc::hir::def::*;
24-
use rustc::util::nodemap::FxHashMap;
24+
use rustc::util::nodemap::{FxHashMap, FxHashSet};
2525

26-
use syntax::ast::{Ident, NodeId};
26+
use syntax::ast::{Ident, SpannedIdent, NodeId};
2727
use syntax::ext::base::Determinacy::{self, Determined, Undetermined};
2828
use syntax::ext::hygiene::Mark;
2929
use syntax::parse::token;
@@ -57,7 +57,7 @@ pub enum ImportDirectiveSubclass<'a> {
5757
pub struct ImportDirective<'a> {
5858
pub id: NodeId,
5959
pub parent: Module<'a>,
60-
pub module_path: Vec<Ident>,
60+
pub module_path: Vec<SpannedIdent>,
6161
pub imported_module: Cell<Option<Module<'a>>>, // the resolution of `module_path`
6262
pub subclass: ImportDirectiveSubclass<'a>,
6363
pub span: Span,
@@ -256,7 +256,7 @@ impl<'a> Resolver<'a> {
256256

257257
// Add an import directive to the current module.
258258
pub fn add_import_directive(&mut self,
259-
module_path: Vec<Ident>,
259+
module_path: Vec<SpannedIdent>,
260260
subclass: ImportDirectiveSubclass<'a>,
261261
span: Span,
262262
id: NodeId,
@@ -478,9 +478,10 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
478478
}
479479

480480
let mut errors = false;
481+
let mut seen_spans = FxHashSet();
481482
for i in 0 .. self.determined_imports.len() {
482483
let import = self.determined_imports[i];
483-
if let Some(err) = self.finalize_import(import) {
484+
if let Some((span, err)) = self.finalize_import(import) {
484485
errors = true;
485486

486487
if let SingleImport { source, ref result, .. } = import.subclass {
@@ -496,9 +497,14 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
496497
// If the error is a single failed import then create a "fake" import
497498
// resolution for it so that later resolve stages won't complain.
498499
self.import_dummy_binding(import);
499-
let path = import_path_to_string(&import.module_path, &import.subclass);
500-
let error = ResolutionError::UnresolvedImport(Some((&path, &err)));
501-
resolve_error(self.resolver, import.span, error);
500+
if !seen_spans.contains(&span) {
501+
let path = import_path_to_string(&import.module_path[..],
502+
&import.subclass,
503+
span);
504+
let error = ResolutionError::UnresolvedImport(Some((span, &path, &err)));
505+
resolve_error(self.resolver, span, error);
506+
seen_spans.insert(span);
507+
}
502508
}
503509
}
504510

@@ -516,7 +522,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
516522
/// If successful, the resolved bindings are written into the module.
517523
fn resolve_import(&mut self, directive: &'b ImportDirective<'b>) -> bool {
518524
debug!("(resolving import for module) resolving import `{}::...` in `{}`",
519-
names_to_string(&directive.module_path),
525+
names_to_string(&directive.module_path[..]),
520526
module_to_string(self.current_module));
521527

522528
self.current_module = directive.parent;
@@ -528,7 +534,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
528534
// For better failure detection, pretend that the import will not define any names
529535
// while resolving its module path.
530536
directive.vis.set(ty::Visibility::Invisible);
531-
let result = self.resolve_path(&directive.module_path, None, false, directive.span);
537+
let result = self.resolve_path(&directive.module_path[..], None, false, directive.span);
532538
directive.vis.set(vis);
533539

534540
match result {
@@ -593,23 +599,25 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
593599
}
594600

595601
// If appropriate, returns an error to report.
596-
fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> Option<String> {
602+
fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> Option<(Span, String)> {
597603
self.current_module = directive.parent;
598604

599605
let ImportDirective { ref module_path, span, .. } = *directive;
600606
let module_result = self.resolve_path(&module_path, None, true, span);
601607
let module = match module_result {
602608
PathResult::Module(module) => module,
603-
PathResult::Failed(msg, _) => {
609+
PathResult::Failed(span, msg, _) => {
604610
let (mut self_path, mut self_result) = (module_path.clone(), None);
605-
if !self_path.is_empty() && !token::Ident(self_path[0]).is_path_segment_keyword() {
606-
self_path[0].name = keywords::SelfValue.name();
611+
if !self_path.is_empty() &&
612+
!token::Ident(self_path[0].node).is_path_segment_keyword()
613+
{
614+
self_path[0].node.name = keywords::SelfValue.name();
607615
self_result = Some(self.resolve_path(&self_path, None, false, span));
608616
}
609617
return if let Some(PathResult::Module(..)) = self_result {
610-
Some(format!("Did you mean `{}`?", names_to_string(&self_path)))
618+
Some((span, format!("Did you mean `{}`?", names_to_string(&self_path[..]))))
611619
} else {
612-
Some(msg)
620+
Some((span, msg))
613621
};
614622
},
615623
_ => return None,
@@ -619,7 +627,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
619627
SingleImport { source, ref result, type_ns_only, .. } => (source, result, type_ns_only),
620628
GlobImport { .. } if module.def_id() == directive.parent.def_id() => {
621629
// Importing a module into itself is not allowed.
622-
return Some("Cannot glob-import a module into itself.".to_string());
630+
return Some((directive.span,
631+
"Cannot glob-import a module into itself.".to_string()));
623632
}
624633
GlobImport { is_prelude, ref max_vis } => {
625634
if !is_prelude &&
@@ -708,7 +717,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
708717
} else {
709718
format!("no `{}` in `{}`{}", ident, module_str, lev_suggestion)
710719
};
711-
Some(msg)
720+
Some((span, msg))
712721
} else {
713722
// `resolve_ident_in_module` reported a privacy error.
714723
self.import_dummy_binding(directive);
@@ -888,16 +897,24 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
888897
}
889898
}
890899

891-
fn import_path_to_string(names: &[Ident], subclass: &ImportDirectiveSubclass) -> String {
892-
let global = !names.is_empty() && names[0].name == keywords::CrateRoot.name();
893-
let names = if global { &names[1..] } else { names };
894-
if names.is_empty() {
895-
import_directive_subclass_to_string(subclass)
900+
fn import_path_to_string(names: &[SpannedIdent],
901+
subclass: &ImportDirectiveSubclass,
902+
span: Span) -> String {
903+
let pos = names.iter()
904+
.position(|p| span == p.span && p.node.name != keywords::CrateRoot.name());
905+
let global = !names.is_empty() && names[0].node.name == keywords::CrateRoot.name();
906+
if let Some(pos) = pos {
907+
let names = if global { &names[1..pos + 1] } else { &names[..pos + 1] };
908+
names_to_string(names)
896909
} else {
897-
(format!("{}::{}",
898-
names_to_string(names),
899-
import_directive_subclass_to_string(subclass)))
900-
.to_string()
910+
let names = if global { &names[1..] } else { names };
911+
if names.is_empty() {
912+
import_directive_subclass_to_string(subclass)
913+
} else {
914+
(format!("{}::{}",
915+
names_to_string(names),
916+
import_directive_subclass_to_string(subclass)))
917+
}
901918
}
902919
}
903920

‎src/test/compile-fail/dollar-crate-is-keyword-2.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ mod a {}
1313
macro_rules! m {
1414
() => {
1515
use a::$crate; //~ ERROR unresolved import `a::$crate`
16-
use a::$crate::b; //~ ERROR unresolved import `a::$crate::b`
16+
use a::$crate::b; //~ ERROR unresolved import `a::$crate`
1717
type A = a::$crate; //~ ERROR cannot find type `$crate` in module `a`
1818
}
1919
}

‎src/test/compile-fail/import2.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
use baz::zed::bar; //~ ERROR unresolved import `baz::zed::bar` [E0432]
11+
use baz::zed::bar; //~ ERROR unresolved import `baz::zed` [E0432]
1212
//~^ Could not find `zed` in `baz`
1313

1414
mod baz {}

‎src/test/compile-fail/issue-1697.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010

1111
// Testing that we don't fail abnormally after hitting the errors
1212

13-
use unresolved::*; //~ ERROR unresolved import `unresolved::*` [E0432]
13+
use unresolved::*; //~ ERROR unresolved import `unresolved` [E0432]
1414
//~^ Maybe a missing `extern crate unresolved;`?
1515

1616
fn main() {}

‎src/test/compile-fail/issue-30560.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@
1010

1111
type Alias = ();
1212
use Alias::*;
13-
//~^ ERROR unresolved import `Alias::*` [E0432]
13+
//~^ ERROR unresolved import `Alias` [E0432]
1414
//~| Not a module `Alias`
1515
use std::io::Result::*;
16-
//~^ ERROR unresolved import `std::io::Result::*` [E0432]
16+
//~^ ERROR unresolved import `std::io::Result` [E0432]
1717
//~| Not a module `Result`
1818

1919
trait T {}

‎src/test/compile-fail/issue-33464.rs

+3-6
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,10 @@
1111
// Make sure that the spans of import errors are correct.
1212

1313
use abc::one_el;
14-
//~^ ERROR 13:5: 13:16
14+
//~^ ERROR 13:5: 13:8
1515
use abc::{a, bbb, cccccc};
16-
//~^ ERROR 15:11: 15:12
17-
//~^^ ERROR 15:14: 15:17
18-
//~^^^ ERROR 15:19: 15:25
16+
//~^ ERROR 15:5: 15:8
1917
use a_very_long_name::{el, el2};
20-
//~^ ERROR 19:24: 19:26
21-
//~^^ ERROR 19:28: 19:31
18+
//~^ ERROR 17:5: 17:21
2219

2320
fn main() {}

‎src/test/compile-fail/resolve_self_super_hint.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -13,19 +13,19 @@
1313
mod a {
1414
extern crate alloc;
1515
use alloc::HashMap;
16-
//~^ ERROR unresolved import `alloc::HashMap` [E0432]
16+
//~^ ERROR unresolved import `alloc` [E0432]
1717
//~| Did you mean `self::alloc`?
1818
mod b {
1919
use alloc::HashMap;
20-
//~^ ERROR unresolved import `alloc::HashMap` [E0432]
20+
//~^ ERROR unresolved import `alloc` [E0432]
2121
//~| Did you mean `a::alloc`?
2222
mod c {
2323
use alloc::HashMap;
24-
//~^ ERROR unresolved import `alloc::HashMap` [E0432]
24+
//~^ ERROR unresolved import `alloc` [E0432]
2525
//~| Did you mean `a::alloc`?
2626
mod d {
2727
use alloc::HashMap;
28-
//~^ ERROR unresolved import `alloc::HashMap` [E0432]
28+
//~^ ERROR unresolved import `alloc` [E0432]
2929
//~| Did you mean `a::alloc`?
3030
}
3131
}

‎src/test/compile-fail/super-at-top-level.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
use super::f; //~ ERROR unresolved import `super::f` [E0432]
11+
use super::f; //~ ERROR unresolved import `super` [E0432]
1212
//~^ There are too many initial `super`s.
1313

1414
fn main() {

0 commit comments

Comments
 (0)