Skip to content

Commit

Permalink
Suggest macro import from crate root.
Browse files Browse the repository at this point in the history
This commit suggests importing a macro from the root of a crate as the
intent may have been to import a macro from the definition location that
was annotated with `#[macro_export]`.
  • Loading branch information
davidtwco committed Apr 11, 2019
1 parent 126ac9e commit d84907b
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 41 deletions.
82 changes: 69 additions & 13 deletions src/librustc_resolve/error_reporting.rs
Expand Up @@ -5,15 +5,16 @@ use log::debug;
use rustc::hir::def::{Def, CtorKind, Namespace::*};
use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
use rustc::session::config::nightly_options;
use syntax::ast::{Expr, ExprKind};
use syntax::ast::{Expr, ExprKind, Ident};
use syntax::ext::base::MacroKind;
use syntax::symbol::keywords;
use syntax_pos::Span;

use crate::macros::ParentScope;
use crate::resolve_imports::ImportResolver;
use crate::resolve_imports::{ImportDirective, ImportResolver};
use crate::{import_candidate_to_enum_paths, is_self_type, is_self_value, path_names_to_string};
use crate::{AssocSuggestion, CrateLint, ImportSuggestion, ModuleOrUniformRoot, PathResult,
PathSource, Resolver, Segment};
PathSource, Resolver, Segment, Suggestion};

impl<'a> Resolver<'a> {
/// Handles error reporting for `smart_resolve_path_fragment` function.
Expand Down Expand Up @@ -428,7 +429,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
span: Span,
mut path: Vec<Segment>,
parent_scope: &ParentScope<'b>,
) -> Option<(Vec<Segment>, Option<String>)> {
) -> Option<(Vec<Segment>, Vec<String>)> {
debug!("make_path_suggestion: span={:?} path={:?}", span, path);

match (path.get(0), path.get(1)) {
Expand Down Expand Up @@ -463,13 +464,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
span: Span,
mut path: Vec<Segment>,
parent_scope: &ParentScope<'b>,
) -> Option<(Vec<Segment>, Option<String>)> {
) -> Option<(Vec<Segment>, Vec<String>)> {
// Replace first ident with `self` and check if that is valid.
path[0].ident.name = keywords::SelfLower.name();
let result = self.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
debug!("make_missing_self_suggestion: path={:?} result={:?}", path, result);
if let PathResult::Module(..) = result {
Some((path, None))
Some((path, Vec::new()))
} else {
None
}
Expand All @@ -487,19 +488,19 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
span: Span,
mut path: Vec<Segment>,
parent_scope: &ParentScope<'b>,
) -> Option<(Vec<Segment>, Option<String>)> {
) -> Option<(Vec<Segment>, Vec<String>)> {
// Replace first ident with `crate` and check if that is valid.
path[0].ident.name = keywords::Crate.name();
let result = self.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
debug!("make_missing_crate_suggestion: path={:?} result={:?}", path, result);
if let PathResult::Module(..) = result {
Some((
path,
Some(
vec![
"`use` statements changed in Rust 2018; read more at \
<https://doc.rust-lang.org/edition-guide/rust-2018/module-system/path-\
clarity.html>".to_string()
),
],
))
} else {
None
Expand All @@ -518,13 +519,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
span: Span,
mut path: Vec<Segment>,
parent_scope: &ParentScope<'b>,
) -> Option<(Vec<Segment>, Option<String>)> {
) -> Option<(Vec<Segment>, Vec<String>)> {
// Replace first ident with `crate` and check if that is valid.
path[0].ident.name = keywords::Super.name();
let result = self.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
debug!("make_missing_super_suggestion: path={:?} result={:?}", path, result);
if let PathResult::Module(..) = result {
Some((path, None))
Some((path, Vec::new()))
} else {
None
}
Expand All @@ -545,7 +546,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
span: Span,
mut path: Vec<Segment>,
parent_scope: &ParentScope<'b>,
) -> Option<(Vec<Segment>, Option<String>)> {
) -> Option<(Vec<Segment>, Vec<String>)> {
if path[1].ident.span.rust_2015() {
return None;
}
Expand All @@ -564,10 +565,65 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
debug!("make_external_crate_suggestion: name={:?} path={:?} result={:?}",
name, path, result);
if let PathResult::Module(..) = result {
return Some((path, None));
return Some((path, Vec::new()));
}
}

None
}

/// Suggests importing a macro from the root of the crate rather than a module within
/// the crate.
///
/// ```
/// help: a macro with this name exists at the root of the crate
/// |
/// LL | use issue_59764::makro;
/// | ^^^^^^^^^^^^^^^^^^
/// |
/// = note: this could be because a macro annotated with `#[macro_export]` will be exported
/// at the root of the crate instead of the module where it is defined
/// ```
pub(crate) fn check_for_module_export_macro(
&self,
directive: &'b ImportDirective<'b>,
module: ModuleOrUniformRoot<'b>,
ident: Ident,
) -> Option<(Option<Suggestion>, Vec<String>)> {
let mut crate_module = if let ModuleOrUniformRoot::Module(module) = module {
module
} else {
return None;
};

while let Some(parent) = crate_module.parent {
crate_module = parent;
}

if ModuleOrUniformRoot::same_def(ModuleOrUniformRoot::Module(crate_module), module) {
// Don't make a suggestion if the import was already from the root of the
// crate.
return None;
}

let resolutions = crate_module.resolutions.borrow();
let resolution = resolutions.get(&(ident, MacroNS))?;
let binding = resolution.borrow().binding()?;
if let Def::Macro(_, MacroKind::Bang) = binding.def() {
let name = crate_module.kind.name().unwrap();
let suggestion = Some((
directive.span,
String::from("a macro with this name exists at the root of the crate"),
format!("{}::{}", name, ident),
Applicability::MaybeIncorrect,
));
let note = vec![
"this could be because a macro annotated with `#[macro_export]` will be exported \
at the root of the crate instead of the module where it is defined".to_string(),
];
Some((suggestion, note))
} else {
None
}
}
}
10 changes: 10 additions & 0 deletions src/librustc_resolve/lib.rs
Expand Up @@ -1091,6 +1091,16 @@ enum ModuleKind {
Def(Def, Name),
}

impl ModuleKind {
/// Get name of the module.
pub fn name(&self) -> Option<Name> {
match self {
ModuleKind::Block(..) => None,
ModuleKind::Def(_, name) => Some(*name),
}
}
}

/// One node in the tree of modules.
pub struct ModuleData<'a> {
parent: Option<Module<'a>>,
Expand Down
55 changes: 30 additions & 25 deletions src/librustc_resolve/resolve_imports.rs
Expand Up @@ -145,7 +145,7 @@ pub struct NameResolution<'a> {

impl<'a> NameResolution<'a> {
// Returns the binding for the name if it is known or None if it not known.
fn binding(&self) -> Option<&'a NameBinding<'a>> {
pub(crate) fn binding(&self) -> Option<&'a NameBinding<'a>> {
self.binding.and_then(|binding| {
if !binding.is_glob_import() ||
self.single_imports.is_empty() { Some(binding) } else { None }
Expand Down Expand Up @@ -636,7 +636,7 @@ impl<'a> Resolver<'a> {
struct UnresolvedImportError {
span: Span,
label: Option<String>,
note: Option<String>,
note: Vec<String>,
suggestion: Option<Suggestion>,
}

Expand Down Expand Up @@ -756,8 +756,8 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
/// Upper limit on the number of `span_label` messages.
const MAX_LABEL_COUNT: usize = 10;

let (span, msg, note) = if errors.is_empty() {
(span.unwrap(), "unresolved import".to_string(), None)
let (span, msg) = if errors.is_empty() {
(span.unwrap(), "unresolved import".to_string())
} else {
let span = MultiSpan::from_spans(
errors
Expand All @@ -766,11 +766,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
.collect(),
);

let note = errors
.iter()
.filter_map(|(_, err)| err.note.as_ref())
.last();

let paths = errors
.iter()
.map(|(path, _)| format!("`{}`", path))
Expand All @@ -782,13 +777,15 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
paths.join(", "),
);

(span, msg, note)
(span, msg)
};

let mut diag = struct_span_err!(self.resolver.session, span, E0432, "{}", &msg);

if let Some(note) = &note {
diag.note(note);
if let Some((_, UnresolvedImportError { note, .. })) = errors.iter().last() {
for message in note {
diag.note(&message);
}
}

for (_, err) in errors.into_iter().take(MAX_LABEL_COUNT) {
Expand Down Expand Up @@ -961,7 +958,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
UnresolvedImportError {
span,
label: Some(label),
note: None,
note: Vec::new(),
suggestion,
}
}
Expand Down Expand Up @@ -1006,7 +1003,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
return Some(UnresolvedImportError {
span: directive.span,
label: Some(String::from("cannot glob-import a module into itself")),
note: None,
note: Vec::new(),
suggestion: None,
});
}
Expand Down Expand Up @@ -1114,15 +1111,22 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}
});

let lev_suggestion =
find_best_match_for_name(names, &ident.as_str(), None).map(|suggestion| {
(
ident.span,
String::from("a similar name exists in the module"),
suggestion.to_string(),
Applicability::MaybeIncorrect,
)
});
let (suggestion, note) = if let Some((suggestion, note)) =
self.check_for_module_export_macro(directive, module, ident)
{

(
suggestion.or_else(||
find_best_match_for_name(names, &ident.as_str(), None)
.map(|suggestion|
(ident.span, String::from("a similar name exists in the module"),
suggestion.to_string(), Applicability::MaybeIncorrect)
)),
note,
)
} else {
(None, Vec::new())
};

let label = match module {
ModuleOrUniformRoot::Module(module) => {
Expand All @@ -1143,11 +1147,12 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
}
}
};

Some(UnresolvedImportError {
span: directive.span,
label: Some(label),
note: None,
suggestion: lev_suggestion,
note,
suggestion,
})
} else {
// `resolve_ident_in_module` reported a privacy error.
Expand Down
15 changes: 15 additions & 0 deletions src/test/ui/issue-59764.fixed
@@ -0,0 +1,15 @@
// aux-build:issue-59764.rs
// compile-flags:--extern issue_59764
// edition:2018
// run-rustfix

use issue_59764::makro;
//~^ ERROR unresolved import `issue_59764::foo::makro` [E0432]

makro!(bar);
//~^ ERROR cannot determine resolution for the macro `makro`

fn main() {
bar();
//~^ ERROR cannot find function `bar` in this scope [E0425]
}
1 change: 1 addition & 0 deletions src/test/ui/issue-59764.rs
@@ -1,6 +1,7 @@
// aux-build:issue-59764.rs
// compile-flags:--extern issue_59764
// edition:2018
// run-rustfix

use issue_59764::foo::makro;
//~^ ERROR unresolved import `issue_59764::foo::makro` [E0432]
Expand Down
12 changes: 9 additions & 3 deletions src/test/ui/issue-59764.stderr
@@ -1,19 +1,25 @@
error[E0432]: unresolved import `issue_59764::foo::makro`
--> $DIR/issue-59764.rs:5:5
--> $DIR/issue-59764.rs:6:5
|
LL | use issue_59764::foo::makro;
| ^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo`
|
= note: this could be because a macro annotated with `#[macro_export]` will be exported at the root of the crate instead of the module where it is defined
help: a macro with this name exists at the root of the crate
|
LL | use issue_59764::makro;
| ^^^^^^^^^^^^^^^^^^

error: cannot determine resolution for the macro `makro`
--> $DIR/issue-59764.rs:8:1
--> $DIR/issue-59764.rs:9:1
|
LL | makro!(bar);
| ^^^^^
|
= note: import resolution is stuck, try simplifying macro imports

error[E0425]: cannot find function `bar` in this scope
--> $DIR/issue-59764.rs:12:5
--> $DIR/issue-59764.rs:13:5
|
LL | bar();
| ^^^ not found in this scope
Expand Down

0 comments on commit d84907b

Please sign in to comment.