Skip to content

Commit

Permalink
Point at path segment on module not found
Browse files Browse the repository at this point in the history
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`
```
  • Loading branch information
estebank committed Jul 26, 2017
1 parent 6c94965 commit 552ff07
Show file tree
Hide file tree
Showing 18 changed files with 194 additions and 129 deletions.
22 changes: 14 additions & 8 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use syntax::attr;
use syntax::ast::{self, Block, ForeignItem, ForeignItemKind, Item, ItemKind};
use syntax::ast::{Mutability, StmtKind, TraitItem, TraitItemKind};
use syntax::ast::{Variant, ViewPathGlob, ViewPathList, ViewPathSimple};
use syntax::codemap::respan;
use syntax::ext::base::SyntaxExtension;
use syntax::ext::base::Determinacy::Undetermined;
use syntax::ext::hygiene::Mark;
Expand Down Expand Up @@ -119,22 +120,24 @@ impl<'a> Resolver<'a> {
.unwrap()
.1
.iter()
.map(|seg| seg.identifier)
.map(|seg| respan(seg.span, seg.identifier))
.collect()
}

ViewPathGlob(ref module_ident_path) |
ViewPathList(ref module_ident_path, _) => {
module_ident_path.segments
.iter()
.map(|seg| seg.identifier)
.map(|seg| respan(seg.span, seg.identifier))
.collect()
}
};

// This can be removed once warning cycle #36888 is complete.
if module_path.len() >= 2 && module_path[0].name == keywords::CrateRoot.name() &&
token::Ident(module_path[1]).is_path_segment_keyword() {
if module_path.len() >= 2 &&
module_path[0].node.name == keywords::CrateRoot.name() &&
token::Ident(module_path[1].node).is_path_segment_keyword()
{
module_path.remove(0);
}

Expand Down Expand Up @@ -202,10 +205,13 @@ impl<'a> Resolver<'a> {
let (module_path, ident, rename, type_ns_only) = {
if node.name.name != keywords::SelfValue.name() {
let rename = node.rename.unwrap_or(node.name);
(module_path.clone(), node.name, rename, false)
(module_path.clone(),
respan(source_item.span, node.name),
rename,
false)
} else {
let ident = *module_path.last().unwrap();
if ident.name == keywords::CrateRoot.name() {
if ident.node.name == keywords::CrateRoot.name() {
resolve_error(
self,
source_item.span,
Expand All @@ -215,13 +221,13 @@ impl<'a> Resolver<'a> {
continue;
}
let module_path = module_path.split_last().unwrap().1;
let rename = node.rename.unwrap_or(ident);
let rename = node.rename.unwrap_or(ident.node);
(module_path.to_vec(), ident, rename, true)
}
};
let subclass = SingleImport {
target: rename,
source: ident,
source: ident.node,
result: self.per_ns(|_, _| Cell::new(Err(Undetermined))),
type_ns_only: type_ns_only,
};
Expand Down
141 changes: 80 additions & 61 deletions src/librustc_resolve/lib.rs

Large diffs are not rendered by default.

19 changes: 12 additions & 7 deletions src/librustc_resolve/macros.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use rustc::hir::map::{self, DefCollector};
use rustc::{ty, lint};
use syntax::ast::{self, Name, Ident};
use syntax::attr::{self, HasAttrs};
use syntax::codemap::respan;
use syntax::errors::DiagnosticBuilder;
use syntax::ext::base::{self, Annotatable, Determinacy, MultiModifier, MultiDecorator};
use syntax::ext::base::{MacroKind, SyntaxExtension, Resolver as SyntaxResolver};
Expand Down Expand Up @@ -393,7 +394,7 @@ impl<'a> Resolver<'a> {
return Err(Determinacy::Determined);
}

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

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

let legacy_resolution = self.resolve_legacy_scope(&invocation.legacy_scope, path[0], false);
let legacy_resolution = self.resolve_legacy_scope(&invocation.legacy_scope,
path[0].node,
false);
let result = if let Some(MacroBinding::Legacy(binding)) = legacy_resolution {
Ok(Def::Macro(binding.def_id, MacroKind::Bang))
} else {
match self.resolve_lexical_macro_path_segment(path[0], MacroNS, false, span) {
match self.resolve_lexical_macro_path_segment(path[0].node, MacroNS, false, span) {
Ok(binding) => Ok(binding.binding().def_ignoring_ambiguity()),
Err(Determinacy::Undetermined) if !force => return Err(Determinacy::Undetermined),
Err(_) => {
Expand All @@ -438,7 +442,7 @@ impl<'a> Resolver<'a> {
};

self.current_module.nearest_item_scope().legacy_macro_resolutions.borrow_mut()
.push((scope, path[0], span, kind));
.push((scope, path[0].node, span, kind));

result
}
Expand Down Expand Up @@ -576,9 +580,10 @@ impl<'a> Resolver<'a> {
pub fn finalize_current_module_macro_resolutions(&mut self) {
let module = self.current_module;
for &(ref path, span) in module.macro_resolutions.borrow().iter() {
match self.resolve_path(path, Some(MacroNS), true, span) {
let path = path.iter().map(|p| respan(span, *p)).collect::<Vec<_>>();
match self.resolve_path(&path, Some(MacroNS), true, span) {
PathResult::NonModule(_) => {},
PathResult::Failed(msg, _) => {
PathResult::Failed(span, msg, _) => {
resolve_error(self, span, ResolutionError::FailedToResolve(&msg));
}
_ => unreachable!(),
Expand Down Expand Up @@ -652,7 +657,7 @@ impl<'a> Resolver<'a> {
}
};
let ident = Ident::from_str(name);
self.lookup_typo_candidate(&vec![ident], MacroNS, is_macro, span)
self.lookup_typo_candidate(&vec![respan(span, ident)], MacroNS, is_macro, span)
});

if let Some(suggestion) = suggestion {
Expand Down
71 changes: 44 additions & 27 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ use rustc::ty;
use rustc::lint::builtin::PUB_USE_OF_PRIVATE_EXTERN_CRATE;
use rustc::hir::def_id::DefId;
use rustc::hir::def::*;
use rustc::util::nodemap::FxHashMap;
use rustc::util::nodemap::{FxHashMap, FxHashSet};

use syntax::ast::{Ident, NodeId};
use syntax::ast::{Ident, SpannedIdent, NodeId};
use syntax::ext::base::Determinacy::{self, Determined, Undetermined};
use syntax::ext::hygiene::Mark;
use syntax::parse::token;
Expand Down Expand Up @@ -57,7 +57,7 @@ pub enum ImportDirectiveSubclass<'a> {
pub struct ImportDirective<'a> {
pub id: NodeId,
pub parent: Module<'a>,
pub module_path: Vec<Ident>,
pub module_path: Vec<SpannedIdent>,
pub imported_module: Cell<Option<Module<'a>>>, // the resolution of `module_path`
pub subclass: ImportDirectiveSubclass<'a>,
pub span: Span,
Expand Down Expand Up @@ -256,7 +256,7 @@ impl<'a> Resolver<'a> {

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

let mut errors = false;
let mut seen_spans = FxHashSet();
for i in 0 .. self.determined_imports.len() {
let import = self.determined_imports[i];
if let Some(err) = self.finalize_import(import) {
if let Some((span, err)) = self.finalize_import(import) {
errors = true;

if let SingleImport { source, ref result, .. } = import.subclass {
Expand All @@ -496,9 +497,14 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
// 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);
let path = import_path_to_string(&import.module_path, &import.subclass);
let error = ResolutionError::UnresolvedImport(Some((&path, &err)));
resolve_error(self.resolver, import.span, error);
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);
seen_spans.insert(span);
}
}
}

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

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

match result {
Expand Down Expand Up @@ -593,23 +599,25 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}

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

let ImportDirective { ref module_path, span, .. } = *directive;
let module_result = self.resolve_path(&module_path, None, true, span);
let module = match module_result {
PathResult::Module(module) => module,
PathResult::Failed(msg, _) => {
PathResult::Failed(span, msg, _) => {
let (mut self_path, mut self_result) = (module_path.clone(), None);
if !self_path.is_empty() && !token::Ident(self_path[0]).is_path_segment_keyword() {
self_path[0].name = keywords::SelfValue.name();
if !self_path.is_empty() &&
!token::Ident(self_path[0].node).is_path_segment_keyword()
{
self_path[0].node.name = keywords::SelfValue.name();
self_result = Some(self.resolve_path(&self_path, None, false, span));
}
return if let Some(PathResult::Module(..)) = self_result {
Some(format!("Did you mean `{}`?", names_to_string(&self_path)))
Some((span, format!("Did you mean `{}`?", names_to_string(&self_path[..]))))
} else {
Some(msg)
Some((span, msg))
};
},
_ => return None,
Expand All @@ -619,7 +627,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
SingleImport { source, ref result, type_ns_only, .. } => (source, result, type_ns_only),
GlobImport { .. } if module.def_id() == directive.parent.def_id() => {
// Importing a module into itself is not allowed.
return Some("Cannot glob-import a module into itself.".to_string());
return Some((directive.span,
"Cannot glob-import a module into itself.".to_string()));
}
GlobImport { is_prelude, ref max_vis } => {
if !is_prelude &&
Expand Down Expand Up @@ -708,7 +717,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
} else {
format!("no `{}` in `{}`{}", ident, module_str, lev_suggestion)
};
Some(msg)
Some((span, msg))
} else {
// `resolve_ident_in_module` reported a privacy error.
self.import_dummy_binding(directive);
Expand Down Expand Up @@ -888,16 +897,24 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}
}

fn import_path_to_string(names: &[Ident], subclass: &ImportDirectiveSubclass) -> String {
let global = !names.is_empty() && names[0].name == keywords::CrateRoot.name();
let names = if global { &names[1..] } else { names };
if names.is_empty() {
import_directive_subclass_to_string(subclass)
fn import_path_to_string(names: &[SpannedIdent],
subclass: &ImportDirectiveSubclass,
span: Span) -> String {
let pos = names.iter()
.position(|p| span == p.span && p.node.name != keywords::CrateRoot.name());
let global = !names.is_empty() && names[0].node.name == keywords::CrateRoot.name();
if let Some(pos) = pos {
let names = if global { &names[1..pos + 1] } else { &names[..pos + 1] };
names_to_string(names)
} else {
(format!("{}::{}",
names_to_string(names),
import_directive_subclass_to_string(subclass)))
.to_string()
let names = if global { &names[1..] } else { names };
if names.is_empty() {
import_directive_subclass_to_string(subclass)
} else {
(format!("{}::{}",
names_to_string(names),
import_directive_subclass_to_string(subclass)))
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/dollar-crate-is-keyword-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ mod a {}
macro_rules! m {
() => {
use a::$crate; //~ ERROR unresolved import `a::$crate`
use a::$crate::b; //~ ERROR unresolved import `a::$crate::b`
use a::$crate::b; //~ ERROR unresolved import `a::$crate`
type A = a::$crate; //~ ERROR cannot find type `$crate` in module `a`
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/import2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

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

mod baz {}
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/issue-1697.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

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

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

fn main() {}
4 changes: 2 additions & 2 deletions src/test/compile-fail/issue-30560.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@

type Alias = ();
use Alias::*;
//~^ ERROR unresolved import `Alias::*` [E0432]
//~^ ERROR unresolved import `Alias` [E0432]
//~| Not a module `Alias`
use std::io::Result::*;
//~^ ERROR unresolved import `std::io::Result::*` [E0432]
//~^ ERROR unresolved import `std::io::Result` [E0432]
//~| Not a module `Result`

trait T {}
Expand Down
9 changes: 3 additions & 6 deletions src/test/compile-fail/issue-33464.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,10 @@
// Make sure that the spans of import errors are correct.

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

fn main() {}
8 changes: 4 additions & 4 deletions src/test/compile-fail/resolve_self_super_hint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@
mod a {
extern crate alloc;
use alloc::HashMap;
//~^ ERROR unresolved import `alloc::HashMap` [E0432]
//~^ ERROR unresolved import `alloc` [E0432]
//~| Did you mean `self::alloc`?
mod b {
use alloc::HashMap;
//~^ ERROR unresolved import `alloc::HashMap` [E0432]
//~^ ERROR unresolved import `alloc` [E0432]
//~| Did you mean `a::alloc`?
mod c {
use alloc::HashMap;
//~^ ERROR unresolved import `alloc::HashMap` [E0432]
//~^ ERROR unresolved import `alloc` [E0432]
//~| Did you mean `a::alloc`?
mod d {
use alloc::HashMap;
//~^ ERROR unresolved import `alloc::HashMap` [E0432]
//~^ ERROR unresolved import `alloc` [E0432]
//~| Did you mean `a::alloc`?
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/compile-fail/super-at-top-level.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

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

fn main() {
Expand Down
Loading

0 comments on commit 552ff07

Please sign in to comment.