diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index 461d02e515d3..99aa6e39f7b7 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -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. @@ -428,7 +429,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { debug!("make_path_suggestion: span={:?} path={:?}", span, path); match (path.get(0), path.get(1)) { @@ -463,13 +464,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { // 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 } @@ -487,7 +488,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { // 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); @@ -495,11 +496,11 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { if let PathResult::Module(..) = result { Some(( path, - Some( + vec![ "`use` statements changed in Rust 2018; read more at \ ".to_string() - ), + ], )) } else { None @@ -518,13 +519,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { // 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 } @@ -545,7 +546,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { span: Span, mut path: Vec, parent_scope: &ParentScope<'b>, - ) -> Option<(Vec, Option)> { + ) -> Option<(Vec, Vec)> { if path[1].ident.span.rust_2015() { return None; } @@ -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, Vec)> { + 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 + } + } } diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 5216156c0cab..0fb1ca429835 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1091,6 +1091,16 @@ enum ModuleKind { Def(Def, Name), } +impl ModuleKind { + /// Get name of the module. + pub fn name(&self) -> Option { + match self { + ModuleKind::Block(..) => None, + ModuleKind::Def(_, name) => Some(*name), + } + } +} + /// One node in the tree of modules. pub struct ModuleData<'a> { parent: Option>, diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index 97b630ba5f29..6a01af5d1153 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/src/librustc_resolve/resolve_imports.rs @@ -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 } @@ -636,7 +636,7 @@ impl<'a> Resolver<'a> { struct UnresolvedImportError { span: Span, label: Option, - note: Option, + note: Vec, suggestion: Option, } @@ -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 @@ -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)) @@ -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) = ¬e { - 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) { @@ -961,7 +958,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { UnresolvedImportError { span, label: Some(label), - note: None, + note: Vec::new(), suggestion, } } @@ -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, }); } @@ -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) => { @@ -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. diff --git a/src/test/ui/issue-59764.fixed b/src/test/ui/issue-59764.fixed new file mode 100644 index 000000000000..d7a01ce5f4d7 --- /dev/null +++ b/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] +} diff --git a/src/test/ui/issue-59764.rs b/src/test/ui/issue-59764.rs index 8ac9b9eaba18..e6f7205bfca3 100644 --- a/src/test/ui/issue-59764.rs +++ b/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] diff --git a/src/test/ui/issue-59764.stderr b/src/test/ui/issue-59764.stderr index a428488b009e..6b3237e9272c 100644 --- a/src/test/ui/issue-59764.stderr +++ b/src/test/ui/issue-59764.stderr @@ -1,11 +1,17 @@ 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); | ^^^^^ @@ -13,7 +19,7 @@ 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