Skip to content

Commit d84907b

Browse files
davidtwcodavidtwco
committed
Suggest macro import from crate root.
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]`.
1 parent 126ac9e commit d84907b

File tree

6 files changed

+134
-41
lines changed

6 files changed

+134
-41
lines changed

‎src/librustc_resolve/error_reporting.rs

Lines changed: 69 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,16 @@ use log::debug;
55
use rustc::hir::def::{Def, CtorKind, Namespace::*};
66
use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId};
77
use rustc::session::config::nightly_options;
8-
use syntax::ast::{Expr, ExprKind};
8+
use syntax::ast::{Expr, ExprKind, Ident};
9+
use syntax::ext::base::MacroKind;
910
use syntax::symbol::keywords;
1011
use syntax_pos::Span;
1112

1213
use crate::macros::ParentScope;
13-
use crate::resolve_imports::ImportResolver;
14+
use crate::resolve_imports::{ImportDirective, ImportResolver};
1415
use crate::{import_candidate_to_enum_paths, is_self_type, is_self_value, path_names_to_string};
1516
use crate::{AssocSuggestion, CrateLint, ImportSuggestion, ModuleOrUniformRoot, PathResult,
16-
PathSource, Resolver, Segment};
17+
PathSource, Resolver, Segment, Suggestion};
1718

1819
impl<'a> Resolver<'a> {
1920
/// Handles error reporting for `smart_resolve_path_fragment` function.
@@ -428,7 +429,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
428429
span: Span,
429430
mut path: Vec<Segment>,
430431
parent_scope: &ParentScope<'b>,
431-
) -> Option<(Vec<Segment>, Option<String>)> {
432+
) -> Option<(Vec<Segment>, Vec<String>)> {
432433
debug!("make_path_suggestion: span={:?} path={:?}", span, path);
433434

434435
match (path.get(0), path.get(1)) {
@@ -463,13 +464,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
463464
span: Span,
464465
mut path: Vec<Segment>,
465466
parent_scope: &ParentScope<'b>,
466-
) -> Option<(Vec<Segment>, Option<String>)> {
467+
) -> Option<(Vec<Segment>, Vec<String>)> {
467468
// Replace first ident with `self` and check if that is valid.
468469
path[0].ident.name = keywords::SelfLower.name();
469470
let result = self.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
470471
debug!("make_missing_self_suggestion: path={:?} result={:?}", path, result);
471472
if let PathResult::Module(..) = result {
472-
Some((path, None))
473+
Some((path, Vec::new()))
473474
} else {
474475
None
475476
}
@@ -487,19 +488,19 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
487488
span: Span,
488489
mut path: Vec<Segment>,
489490
parent_scope: &ParentScope<'b>,
490-
) -> Option<(Vec<Segment>, Option<String>)> {
491+
) -> Option<(Vec<Segment>, Vec<String>)> {
491492
// Replace first ident with `crate` and check if that is valid.
492493
path[0].ident.name = keywords::Crate.name();
493494
let result = self.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
494495
debug!("make_missing_crate_suggestion: path={:?} result={:?}", path, result);
495496
if let PathResult::Module(..) = result {
496497
Some((
497498
path,
498-
Some(
499+
vec![
499500
"`use` statements changed in Rust 2018; read more at \
500501
<https://doc.rust-lang.org/edition-guide/rust-2018/module-system/path-\
501502
clarity.html>".to_string()
502-
),
503+
],
503504
))
504505
} else {
505506
None
@@ -518,13 +519,13 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
518519
span: Span,
519520
mut path: Vec<Segment>,
520521
parent_scope: &ParentScope<'b>,
521-
) -> Option<(Vec<Segment>, Option<String>)> {
522+
) -> Option<(Vec<Segment>, Vec<String>)> {
522523
// Replace first ident with `crate` and check if that is valid.
523524
path[0].ident.name = keywords::Super.name();
524525
let result = self.resolve_path(&path, None, parent_scope, false, span, CrateLint::No);
525526
debug!("make_missing_super_suggestion: path={:?} result={:?}", path, result);
526527
if let PathResult::Module(..) = result {
527-
Some((path, None))
528+
Some((path, Vec::new()))
528529
} else {
529530
None
530531
}
@@ -545,7 +546,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
545546
span: Span,
546547
mut path: Vec<Segment>,
547548
parent_scope: &ParentScope<'b>,
548-
) -> Option<(Vec<Segment>, Option<String>)> {
549+
) -> Option<(Vec<Segment>, Vec<String>)> {
549550
if path[1].ident.span.rust_2015() {
550551
return None;
551552
}
@@ -564,10 +565,65 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
564565
debug!("make_external_crate_suggestion: name={:?} path={:?} result={:?}",
565566
name, path, result);
566567
if let PathResult::Module(..) = result {
567-
return Some((path, None));
568+
return Some((path, Vec::new()));
568569
}
569570
}
570571

571572
None
572573
}
574+
575+
/// Suggests importing a macro from the root of the crate rather than a module within
576+
/// the crate.
577+
///
578+
/// ```
579+
/// help: a macro with this name exists at the root of the crate
580+
/// |
581+
/// LL | use issue_59764::makro;
582+
/// | ^^^^^^^^^^^^^^^^^^
583+
/// |
584+
/// = note: this could be because a macro annotated with `#[macro_export]` will be exported
585+
/// at the root of the crate instead of the module where it is defined
586+
/// ```
587+
pub(crate) fn check_for_module_export_macro(
588+
&self,
589+
directive: &'b ImportDirective<'b>,
590+
module: ModuleOrUniformRoot<'b>,
591+
ident: Ident,
592+
) -> Option<(Option<Suggestion>, Vec<String>)> {
593+
let mut crate_module = if let ModuleOrUniformRoot::Module(module) = module {
594+
module
595+
} else {
596+
return None;
597+
};
598+
599+
while let Some(parent) = crate_module.parent {
600+
crate_module = parent;
601+
}
602+
603+
if ModuleOrUniformRoot::same_def(ModuleOrUniformRoot::Module(crate_module), module) {
604+
// Don't make a suggestion if the import was already from the root of the
605+
// crate.
606+
return None;
607+
}
608+
609+
let resolutions = crate_module.resolutions.borrow();
610+
let resolution = resolutions.get(&(ident, MacroNS))?;
611+
let binding = resolution.borrow().binding()?;
612+
if let Def::Macro(_, MacroKind::Bang) = binding.def() {
613+
let name = crate_module.kind.name().unwrap();
614+
let suggestion = Some((
615+
directive.span,
616+
String::from("a macro with this name exists at the root of the crate"),
617+
format!("{}::{}", name, ident),
618+
Applicability::MaybeIncorrect,
619+
));
620+
let note = vec![
621+
"this could be because a macro annotated with `#[macro_export]` will be exported \
622+
at the root of the crate instead of the module where it is defined".to_string(),
623+
];
624+
Some((suggestion, note))
625+
} else {
626+
None
627+
}
628+
}
573629
}

‎src/librustc_resolve/lib.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,6 +1091,16 @@ enum ModuleKind {
10911091
Def(Def, Name),
10921092
}
10931093

1094+
impl ModuleKind {
1095+
/// Get name of the module.
1096+
pub fn name(&self) -> Option<Name> {
1097+
match self {
1098+
ModuleKind::Block(..) => None,
1099+
ModuleKind::Def(_, name) => Some(*name),
1100+
}
1101+
}
1102+
}
1103+
10941104
/// One node in the tree of modules.
10951105
pub struct ModuleData<'a> {
10961106
parent: Option<Module<'a>>,

‎src/librustc_resolve/resolve_imports.rs

Lines changed: 30 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ pub struct NameResolution<'a> {
145145

146146
impl<'a> NameResolution<'a> {
147147
// Returns the binding for the name if it is known or None if it not known.
148-
fn binding(&self) -> Option<&'a NameBinding<'a>> {
148+
pub(crate) fn binding(&self) -> Option<&'a NameBinding<'a>> {
149149
self.binding.and_then(|binding| {
150150
if !binding.is_glob_import() ||
151151
self.single_imports.is_empty() { Some(binding) } else { None }
@@ -636,7 +636,7 @@ impl<'a> Resolver<'a> {
636636
struct UnresolvedImportError {
637637
span: Span,
638638
label: Option<String>,
639-
note: Option<String>,
639+
note: Vec<String>,
640640
suggestion: Option<Suggestion>,
641641
}
642642

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

759-
let (span, msg, note) = if errors.is_empty() {
760-
(span.unwrap(), "unresolved import".to_string(), None)
759+
let (span, msg) = if errors.is_empty() {
760+
(span.unwrap(), "unresolved import".to_string())
761761
} else {
762762
let span = MultiSpan::from_spans(
763763
errors
@@ -766,11 +766,6 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
766766
.collect(),
767767
);
768768

769-
let note = errors
770-
.iter()
771-
.filter_map(|(_, err)| err.note.as_ref())
772-
.last();
773-
774769
let paths = errors
775770
.iter()
776771
.map(|(path, _)| format!("`{}`", path))
@@ -782,13 +777,15 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
782777
paths.join(", "),
783778
);
784779

785-
(span, msg, note)
780+
(span, msg)
786781
};
787782

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

790-
if let Some(note) = &note {
791-
diag.note(note);
785+
if let Some((_, UnresolvedImportError { note, .. })) = errors.iter().last() {
786+
for message in note {
787+
diag.note(&message);
788+
}
792789
}
793790

794791
for (_, err) in errors.into_iter().take(MAX_LABEL_COUNT) {
@@ -961,7 +958,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
961958
UnresolvedImportError {
962959
span,
963960
label: Some(label),
964-
note: None,
961+
note: Vec::new(),
965962
suggestion,
966963
}
967964
}
@@ -1006,7 +1003,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
10061003
return Some(UnresolvedImportError {
10071004
span: directive.span,
10081005
label: Some(String::from("cannot glob-import a module into itself")),
1009-
note: None,
1006+
note: Vec::new(),
10101007
suggestion: None,
10111008
});
10121009
}
@@ -1114,15 +1111,22 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
11141111
}
11151112
});
11161113

1117-
let lev_suggestion =
1118-
find_best_match_for_name(names, &ident.as_str(), None).map(|suggestion| {
1119-
(
1120-
ident.span,
1121-
String::from("a similar name exists in the module"),
1122-
suggestion.to_string(),
1123-
Applicability::MaybeIncorrect,
1124-
)
1125-
});
1114+
let (suggestion, note) = if let Some((suggestion, note)) =
1115+
self.check_for_module_export_macro(directive, module, ident)
1116+
{
1117+
1118+
(
1119+
suggestion.or_else(||
1120+
find_best_match_for_name(names, &ident.as_str(), None)
1121+
.map(|suggestion|
1122+
(ident.span, String::from("a similar name exists in the module"),
1123+
suggestion.to_string(), Applicability::MaybeIncorrect)
1124+
)),
1125+
note,
1126+
)
1127+
} else {
1128+
(None, Vec::new())
1129+
};
11261130

11271131
let label = match module {
11281132
ModuleOrUniformRoot::Module(module) => {
@@ -1143,11 +1147,12 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> {
11431147
}
11441148
}
11451149
};
1150+
11461151
Some(UnresolvedImportError {
11471152
span: directive.span,
11481153
label: Some(label),
1149-
note: None,
1150-
suggestion: lev_suggestion,
1154+
note,
1155+
suggestion,
11511156
})
11521157
} else {
11531158
// `resolve_ident_in_module` reported a privacy error.

‎src/test/ui/issue-59764.fixed

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// aux-build:issue-59764.rs
2+
// compile-flags:--extern issue_59764
3+
// edition:2018
4+
// run-rustfix
5+
6+
use issue_59764::makro;
7+
//~^ ERROR unresolved import `issue_59764::foo::makro` [E0432]
8+
9+
makro!(bar);
10+
//~^ ERROR cannot determine resolution for the macro `makro`
11+
12+
fn main() {
13+
bar();
14+
//~^ ERROR cannot find function `bar` in this scope [E0425]
15+
}

‎src/test/ui/issue-59764.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// aux-build:issue-59764.rs
22
// compile-flags:--extern issue_59764
33
// edition:2018
4+
// run-rustfix
45

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

‎src/test/ui/issue-59764.stderr

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,25 @@
11
error[E0432]: unresolved import `issue_59764::foo::makro`
2-
--> $DIR/issue-59764.rs:5:5
2+
--> $DIR/issue-59764.rs:6:5
33
|
44
LL | use issue_59764::foo::makro;
55
| ^^^^^^^^^^^^^^^^^^^^^^^ no `makro` in `foo`
6+
|
7+
= 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
8+
help: a macro with this name exists at the root of the crate
9+
|
10+
LL | use issue_59764::makro;
11+
| ^^^^^^^^^^^^^^^^^^
612

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

1521
error[E0425]: cannot find function `bar` in this scope
16-
--> $DIR/issue-59764.rs:12:5
22+
--> $DIR/issue-59764.rs:13:5
1723
|
1824
LL | bar();
1925
| ^^^ not found in this scope

0 commit comments

Comments
 (0)