Skip to content

Commit af945cb

Browse files
committed
Find derive macro attributes even if the macro isn't imported directly
1 parent 43a6cd6 commit af945cb

File tree

3 files changed

+152
-5
lines changed

3 files changed

+152
-5
lines changed

compiler/rustc_resolve/src/diagnostics.rs

+82-5
Original file line numberDiff line numberDiff line change
@@ -1116,6 +1116,83 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
11161116
}
11171117
}
11181118

1119+
/// Visit every module reachable from `start_module` and bring any proc and derive macro
1120+
/// encountered into `self.macro_map` to be used for suggestions.
1121+
fn lookup_macros_from_module(&mut self) {
1122+
let parent_scope = ParentScope::module(self.graph_root, self);
1123+
let mut seen_modules = FxHashSet::default();
1124+
let mut worklist = vec![(self.graph_root, ThinVec::<ast::PathSegment>::new(), true)];
1125+
let mut worklist_via_import = vec![];
1126+
1127+
while let Some((in_module, path_segments, accessible)) = match worklist.pop() {
1128+
None => worklist_via_import.pop(),
1129+
Some(x) => Some(x),
1130+
} {
1131+
let in_module_is_extern = !in_module.opt_def_id().map_or(false, |id| id.is_local());
1132+
// We have to visit module children in deterministic order to avoid
1133+
// instabilities in reported imports (#43552).
1134+
in_module.for_each_child(self, |this, ident, _ns, name_binding| {
1135+
if let Res::Def(DefKind::Macro(MacroKind::Derive | MacroKind::Attr), def_id) =
1136+
name_binding.res()
1137+
{
1138+
// By doing this all *imported* macros get added to the `macro_map` even if they
1139+
// are *unused*, which makes the later suggestions find them and work.
1140+
let _ = this.get_macro_by_def_id(def_id);
1141+
}
1142+
// avoid non-importable candidates
1143+
if !name_binding.is_importable() {
1144+
return;
1145+
}
1146+
1147+
let child_accessible =
1148+
accessible && this.is_accessible_from(name_binding.vis, parent_scope.module);
1149+
1150+
// do not venture inside inaccessible items of other crates
1151+
if in_module_is_extern && !child_accessible {
1152+
return;
1153+
}
1154+
1155+
let via_import = name_binding.is_import() && !name_binding.is_extern_crate();
1156+
1157+
// There is an assumption elsewhere that paths of variants are in the enum's
1158+
// declaration and not imported. With this assumption, the variant component is
1159+
// chopped and the rest of the path is assumed to be the enum's own path. For
1160+
// errors where a variant is used as the type instead of the enum, this causes
1161+
// funny looking invalid suggestions, i.e `foo` instead of `foo::MyEnum`.
1162+
if via_import && name_binding.is_possibly_imported_variant() {
1163+
return;
1164+
}
1165+
1166+
// #90113: Do not count an inaccessible reexported item as a candidate.
1167+
if let NameBindingKind::Import { binding, .. } = name_binding.kind {
1168+
if this.is_accessible_from(binding.vis, parent_scope.module)
1169+
&& !this.is_accessible_from(name_binding.vis, parent_scope.module)
1170+
{
1171+
return;
1172+
}
1173+
}
1174+
1175+
// collect submodules to explore
1176+
if let Some(module) = name_binding.module() {
1177+
// form the path
1178+
let mut path_segments = path_segments.clone();
1179+
path_segments.push(ast::PathSegment::from_ident(ident));
1180+
1181+
let is_extern_crate_that_also_appears_in_prelude =
1182+
name_binding.is_extern_crate();
1183+
1184+
if !is_extern_crate_that_also_appears_in_prelude {
1185+
// add the module to the lookup
1186+
if seen_modules.insert(module.def_id()) {
1187+
if via_import { &mut worklist_via_import } else { &mut worklist }
1188+
.push((module, path_segments, child_accessible));
1189+
}
1190+
}
1191+
}
1192+
})
1193+
}
1194+
}
1195+
11191196
fn lookup_import_candidates_from_module<FilterFn>(
11201197
&mut self,
11211198
lookup_ident: Ident,
@@ -1336,6 +1413,7 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13361413
) {
13371414
// Bring imported but unused `derive` macros into `macro_map` so we ensure they can be used
13381415
// for suggestions.
1416+
self.lookup_macros_from_module();
13391417
self.visit_scopes(
13401418
ScopeSet::Macro(MacroKind::Derive),
13411419
&parent_scope,
@@ -1364,16 +1442,15 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
13641442
is_expected,
13651443
);
13661444
if !self.add_typo_suggestion(err, suggestion, ident.span) {
1367-
// FIXME: this only works if the macro that has the helper_attr has already
1368-
// been imported.
1445+
// FIXME: this only works if the crate that owns the macro that has the helper_attr
1446+
// has already been imported.
13691447
let mut derives = vec![];
13701448
let mut all_attrs: FxHashMap<Symbol, Vec<_>> = FxHashMap::default();
13711449
for (def_id, data) in &self.macro_map {
13721450
for helper_attr in &data.ext.helper_attrs {
13731451
let item_name = self.tcx.item_name(*def_id);
13741452
all_attrs.entry(*helper_attr).or_default().push(item_name);
13751453
if helper_attr == &ident.name {
1376-
// FIXME: we should also do Levenshtein distance checks here.
13771454
derives.push(item_name);
13781455
}
13791456
}
@@ -1401,11 +1478,11 @@ impl<'a, 'tcx> Resolver<'a, 'tcx> {
14011478
if span.from_expansion() {
14021479
None
14031480
} else {
1404-
// For enum variants, `sugg_span` is empty, but we can get the `enum`'s `Span`.
1481+
// For enum variants sugg_span is empty but we can get the enum's Span.
14051482
Some(span.shrink_to_lo())
14061483
}
14071484
} else {
1408-
// For items, this `Span` will be populated, everything else it'll be `None`.
1485+
// For items this `Span` will be populated, everything else it'll be None.
14091486
sugg_span
14101487
};
14111488
match sugg_span {

tests/ui/macros/missing-derive-4.rs

+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
// aux-build:serde.rs
2+
// compile-flags:--extern serde --crate-type bin --edition 2021
3+
4+
// derive macros not imported, but namespace imported
5+
use serde;
6+
7+
#[serde(untagged)] //~ ERROR cannot find attribute `serde`
8+
enum A {
9+
A,
10+
B,
11+
}
12+
13+
enum B {
14+
A,
15+
#[serde(untagged)] //~ ERROR cannot find attribute `serde`
16+
B,
17+
}
18+
19+
enum C {
20+
A,
21+
#[sede(untagged)] //~ ERROR cannot find attribute `sede`
22+
B,
23+
}
24+
25+
fn main() {}
+45
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
error: cannot find attribute `sede` in this scope
2+
--> $DIR/missing-derive-4.rs:21:7
3+
|
4+
LL | #[sede(untagged)]
5+
| ^^^^
6+
|
7+
help: the derive macros `Serialize` and `Deserialize` accept the similarly named `serde` attribute
8+
|
9+
LL | #[serde(untagged)]
10+
| ~~~~~
11+
12+
error: cannot find attribute `serde` in this scope
13+
--> $DIR/missing-derive-4.rs:15:7
14+
|
15+
LL | #[serde(untagged)]
16+
| ^^^^^
17+
|
18+
note: `serde` is imported here, but it is a crate, not an attribute
19+
--> $DIR/missing-derive-4.rs:5:5
20+
|
21+
LL | use serde;
22+
| ^^^^^
23+
help: `serde` is an attribute that can be used by the derive macros `Serialize` and `Deserialize`, you might be missing a `derive` attribute
24+
|
25+
LL | #[derive(Serialize, Deserialize)]
26+
|
27+
28+
error: cannot find attribute `serde` in this scope
29+
--> $DIR/missing-derive-4.rs:7:3
30+
|
31+
LL | #[serde(untagged)]
32+
| ^^^^^
33+
|
34+
note: `serde` is imported here, but it is a crate, not an attribute
35+
--> $DIR/missing-derive-4.rs:5:5
36+
|
37+
LL | use serde;
38+
| ^^^^^
39+
help: `serde` is an attribute that can be used by the derive macros `Serialize` and `Deserialize`, you might be missing a `derive` attribute
40+
|
41+
LL | #[derive(Serialize, Deserialize)]
42+
|
43+
44+
error: aborting due to 3 previous errors
45+

0 commit comments

Comments
 (0)