Skip to content

Commit

Permalink
Rollup merge of rust-lang#43447 - estebank:import-span, r=nikomatsakis
Browse files Browse the repository at this point in the history 
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`
```

Fix rust-lang#43040.
  • Loading branch information
@Mark-Simulacrum
Mark-Simulacrum authored Jul 26, 2017
2 parents b5b7266 + 552ff07 commit b583392
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
View file
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
View file

Large diffs are not rendered by default.

19 changes: 12 additions & 7 deletions src/librustc_resolve/macros.rs
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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 b583392

Please sign in to comment.