Skip to content

Commit cd72d90

Browse files
jyn514jyn514
committed
Find the first segment that failed to resolve for _any_ namespace
Moves this detection into `resolution_failure` to avoid doing unnecessary work and make the control flow a little easier to work with.
1 parent 8318a18 commit cd72d90

File tree

3 files changed

+102
-59
lines changed

3 files changed

+102
-59
lines changed

‎src/librustdoc/passes/collect_intra_doc_links.rs

+64-45
Original file line numberDiff line numberDiff line change
@@ -60,8 +60,8 @@ enum ResolutionFailure<'a> {
6060
/// This has a partial resolution, but is not in the TypeNS and so cannot
6161
/// have associated items or fields.
6262
CannotHaveAssociatedItems(Res, Namespace),
63-
/// `String` is the base name of the path (not necessarily the whole link)
64-
NotInScope(Cow<'a, str>),
63+
/// `name` is the base name of the path (not necessarily the whole link)
64+
NotInScope { module_id: DefId, name: Cow<'a, str> },
6565
/// this is a primitive type without an impls (no associated methods)
6666
/// when will this actually happen?
6767
/// the `Res` is the primitive it resolved to
@@ -92,7 +92,7 @@ impl ResolutionFailure<'a> {
9292
| NotAVariant(res, _)
9393
| WrongNamespace(res, _)
9494
| CannotHaveAssociatedItems(res, _) => Some(*res),
95-
NotInScope(_) | NoParentItem | Dummy => None,
95+
NotInScope { .. } | NoParentItem | Dummy => None,
9696
}
9797
}
9898

@@ -142,7 +142,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
142142
.expect("fold_item should ensure link is non-empty");
143143
let variant_name =
144144
// we're not sure this is a variant at all, so use the full string
145-
split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope(path_str.into())))?;
145+
split.next().map(|f| Symbol::intern(f)).ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope{
146+
module_id,
147+
name: path_str.into(),
148+
}))?;
146149
let path = split
147150
.next()
148151
.map(|f| {
@@ -153,38 +156,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
153156
}
154157
f.to_owned()
155158
})
156-
.ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope(
157-
variant_name.to_string().into(),
158-
)))?;
159+
.ok_or(ErrorKind::Resolve(ResolutionFailure::NotInScope {
160+
module_id,
161+
name: variant_name.to_string().into(),
162+
}))?;
159163
let ty_res = cx
160164
.enter_resolver(|resolver| {
161165
resolver.resolve_str_path_error(DUMMY_SP, &path, TypeNS, module_id)
162166
})
163167
.map(|(_, res)| res)
164168
.unwrap_or(Res::Err);
165-
// This code only gets hit if three path segments in a row don't get resolved.
166-
// It's a good time to check if _any_ parent of the path gets resolved.
167-
// If so, report it and say the first which failed; if not, say the first path segment didn't resolve.
168169
if let Res::Err = ty_res {
169-
let mut current = path.as_str();
170-
while let Some(parent) = current.rsplitn(2, "::").nth(1) {
171-
current = parent;
172-
if let Some(res) = self.check_full_res(
173-
TypeNS,
174-
&current,
175-
Some(module_id),
176-
current_item,
177-
extra_fragment,
178-
) {
179-
return Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem(
180-
res,
181-
Symbol::intern(&path),
182-
)));
183-
}
184-
}
185-
return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope(
186-
current.to_string().into(),
187-
)));
170+
return Err(ErrorKind::Resolve(ResolutionFailure::NotInScope {
171+
module_id,
172+
name: path.into(),
173+
}));
188174
}
189175
let ty_res = ty_res.map_id(|_| panic!("unexpected node_id"));
190176
match ty_res {
@@ -301,7 +287,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
301287
});
302288
}
303289
}
304-
Err(ResolutionFailure::NotInScope(path_str.into()))
290+
Err(ResolutionFailure::NotInScope {
291+
module_id: parent_id.expect("already saw `Some` when resolving as a macro"),
292+
name: path_str.into(),
293+
})
305294
})
306295
}
307296
/// Resolves a string as a path within a particular namespace. Also returns an optional
@@ -384,7 +373,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
384373
// So we can be sure that `rustc_resolve` was accurate when it said it wasn't resolved.
385374
.ok_or_else(|| {
386375
debug!("found no `::`, assumming {} was correctly not in scope", item_name);
387-
ErrorKind::Resolve(ResolutionFailure::NotInScope(item_name.to_string().into()))
376+
ErrorKind::Resolve(ResolutionFailure::NotInScope {
377+
module_id,
378+
name: item_name.to_string().into(),
379+
})
388380
})?;
389381

390382
if let Some((path, prim)) = is_primitive(&path_root, TypeNS) {
@@ -451,7 +443,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
451443
}
452444
}
453445
}
454-
ResolutionFailure::NotInScope(path_root.into())
446+
ResolutionFailure::NotInScope { module_id, name: path_root.into() }
455447
});
456448
Err(ErrorKind::Resolve(kind))
457449
};
@@ -996,7 +988,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
996988
}
997989
}
998990
resolution_failure(
999-
cx,
991+
self,
1000992
&item,
1001993
path_str,
1002994
disambiguator,
@@ -1076,7 +1068,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
10761068
if len == 0 {
10771069
drop(candidates_iter);
10781070
resolution_failure(
1079-
cx,
1071+
self,
10801072
&item,
10811073
path_str,
10821074
disambiguator,
@@ -1096,8 +1088,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
10961088
} else {
10971089
drop(candidates_iter);
10981090
if is_derive_trait_collision(&candidates) {
1099-
candidates.macro_ns =
1100-
Err(ResolutionFailure::NotInScope(path_str.into()));
1091+
candidates.macro_ns = Err(ResolutionFailure::Dummy);
11011092
}
11021093
// If we're reporting an ambiguity, don't mention the namespaces that failed
11031094
let candidates =
@@ -1131,7 +1122,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
11311122
}
11321123
}
11331124
resolution_failure(
1134-
cx,
1125+
self,
11351126
&item,
11361127
path_str,
11371128
disambiguator,
@@ -1507,7 +1498,7 @@ fn report_diagnostic(
15071498
}
15081499

15091500
fn resolution_failure(
1510-
cx: &DocContext<'_>,
1501+
collector: &LinkCollector<'_, '_>,
15111502
item: &Item,
15121503
path_str: &str,
15131504
disambiguator: Option<Disambiguator>,
@@ -1516,19 +1507,23 @@ fn resolution_failure(
15161507
kinds: SmallVec<[ResolutionFailure<'_>; 3]>,
15171508
) {
15181509
report_diagnostic(
1519-
cx,
1510+
collector.cx,
15201511
&format!("unresolved link to `{}`", path_str),
15211512
item,
15221513
dox,
15231514
&link_range,
15241515
|diag, sp| {
15251516
let in_scope = kinds.iter().any(|kind| kind.res().is_some());
15261517
let item = |res: Res| {
1527-
format!("the {} `{}`", res.descr(), cx.tcx.item_name(res.def_id()).to_string())
1518+
format!(
1519+
"the {} `{}`",
1520+
res.descr(),
1521+
collector.cx.tcx.item_name(res.def_id()).to_string()
1522+
)
15281523
};
15291524
let assoc_item_not_allowed = |res: Res, diag: &mut DiagnosticBuilder<'_>| {
15301525
let def_id = res.def_id();
1531-
let name = cx.tcx.item_name(def_id);
1526+
let name = collector.cx.tcx.item_name(def_id);
15321527
let note = format!(
15331528
"`{}` is {} {}, not a module or type, and cannot have associated items",
15341529
name,
@@ -1539,18 +1534,42 @@ fn resolution_failure(
15391534
};
15401535
// ignore duplicates
15411536
let mut variants_seen = SmallVec::<[_; 3]>::new();
1542-
for failure in kinds {
1537+
for mut failure in kinds {
1538+
// Check if _any_ parent of the path gets resolved.
1539+
// If so, report it and say the first which failed; if not, say the first path segment didn't resolve.
1540+
if let ResolutionFailure::NotInScope { module_id, name } = &mut failure {
1541+
let mut current = name.as_ref();
1542+
loop {
1543+
current = match current.rsplitn(2, "::").nth(1) {
1544+
Some(p) => p,
1545+
None => {
1546+
*name = current.to_owned().into();
1547+
break;
1548+
}
1549+
};
1550+
if let Some(res) = collector.check_full_res(
1551+
TypeNS,
1552+
&current,
1553+
Some(*module_id),
1554+
&None,
1555+
&None,
1556+
) {
1557+
failure = ResolutionFailure::NoAssocItem(res, Symbol::intern(current));
1558+
break;
1559+
}
1560+
}
1561+
}
15431562
let variant = std::mem::discriminant(&failure);
15441563
if variants_seen.contains(&variant) {
15451564
continue;
15461565
}
15471566
variants_seen.push(variant);
15481567
match failure {
1549-
ResolutionFailure::NotInScope(base) => {
1568+
ResolutionFailure::NotInScope { name, .. } => {
15501569
if in_scope {
15511570
continue;
15521571
}
1553-
diag.note(&format!("no item named `{}` is in scope", base));
1572+
diag.note(&format!("no item named `{}` is in scope", name));
15541573
// If the link has `::` in the path, assume it's meant to be an intra-doc link
15551574
if !path_str.contains("::") {
15561575
// Otherwise, the `[]` might be unrelated.
@@ -1608,7 +1627,7 @@ fn resolution_failure(
16081627
x,
16091628
),
16101629
};
1611-
let name = cx.tcx.item_name(def_id);
1630+
let name = collector.cx.tcx.item_name(def_id);
16121631
let path_description = if let Some(disambiguator) = disambiguator {
16131632
disambiguator.descr()
16141633
} else {

‎src/test/rustdoc-ui/intra-link-errors.rs

+8
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,14 @@
88
//~^ ERROR unresolved link
99
//~| NOTE no item named `path` is in scope
1010

11+
/// [path::to::nonexistent::macro!]
12+
//~^ ERROR unresolved link
13+
//~| NOTE no item named `path` is in scope
14+
15+
/// [type@path::to::nonexistent::type]
16+
//~^ ERROR unresolved link
17+
//~| NOTE no item named `path` is in scope
18+
1119
/// [std::io::not::here]
1220
//~^ ERROR unresolved link
1321
//~| NOTE the module `io` has no inner item

‎src/test/rustdoc-ui/intra-link-errors.stderr

+30-14
Original file line numberDiff line numberDiff line change
@@ -11,109 +11,125 @@ LL | #![deny(broken_intra_doc_links)]
1111
| ^^^^^^^^^^^^^^^^^^^^^^
1212
= note: no item named `path` is in scope
1313

14-
error: unresolved link to `std::io::not::here`
14+
error: unresolved link to `path::to::nonexistent::macro`
1515
--> $DIR/intra-link-errors.rs:11:6
1616
|
17+
LL | /// [path::to::nonexistent::macro!]
18+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
19+
|
20+
= note: no item named `path` is in scope
21+
22+
error: unresolved link to `path::to::nonexistent::type`
23+
--> $DIR/intra-link-errors.rs:15:6
24+
|
25+
LL | /// [type@path::to::nonexistent::type]
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
27+
|
28+
= note: no item named `path` is in scope
29+
30+
error: unresolved link to `std::io::not::here`
31+
--> $DIR/intra-link-errors.rs:19:6
32+
|
1733
LL | /// [std::io::not::here]
1834
| ^^^^^^^^^^^^^^^^^^
1935
|
2036
= note: the module `io` has no inner item named `not`
2137

2238
error: unresolved link to `std::io::Error::x`
23-
--> $DIR/intra-link-errors.rs:15:6
39+
--> $DIR/intra-link-errors.rs:23:6
2440
|
2541
LL | /// [std::io::Error::x]
2642
| ^^^^^^^^^^^^^^^^^
2743
|
2844
= note: the struct `Error` has no field or associated item named `x`
2945

3046
error: unresolved link to `std::io::ErrorKind::x`
31-
--> $DIR/intra-link-errors.rs:19:6
47+
--> $DIR/intra-link-errors.rs:27:6
3248
|
3349
LL | /// [std::io::ErrorKind::x]
3450
| ^^^^^^^^^^^^^^^^^^^^^
3551
|
3652
= note: the enum `ErrorKind` has no variant or associated item named `x`
3753

3854
error: unresolved link to `f::A`
39-
--> $DIR/intra-link-errors.rs:23:6
55+
--> $DIR/intra-link-errors.rs:31:6
4056
|
4157
LL | /// [f::A]
4258
| ^^^^
4359
|
4460
= note: `f` is a function, not a module or type, and cannot have associated items
4561

4662
error: unresolved link to `S::A`
47-
--> $DIR/intra-link-errors.rs:27:6
63+
--> $DIR/intra-link-errors.rs:35:6
4864
|
4965
LL | /// [S::A]
5066
| ^^^^
5167
|
5268
= note: the struct `S` has no field or associated item named `A`
5369

5470
error: unresolved link to `S::fmt`
55-
--> $DIR/intra-link-errors.rs:31:6
71+
--> $DIR/intra-link-errors.rs:39:6
5672
|
5773
LL | /// [S::fmt]
5874
| ^^^^^^
5975
|
6076
= note: the struct `S` has no field or associated item named `fmt`
6177

6278
error: unresolved link to `E::D`
63-
--> $DIR/intra-link-errors.rs:35:6
79+
--> $DIR/intra-link-errors.rs:43:6
6480
|
6581
LL | /// [E::D]
6682
| ^^^^
6783
|
6884
= note: the enum `E` has no variant or associated item named `D`
6985

7086
error: unresolved link to `u8::not_found`
71-
--> $DIR/intra-link-errors.rs:39:6
87+
--> $DIR/intra-link-errors.rs:47:6
7288
|
7389
LL | /// [u8::not_found]
7490
| ^^^^^^^^^^^^^
7591
|
7692
= note: the builtin type `u8` does not have an associated item named `not_found`
7793

7894
error: unresolved link to `S`
79-
--> $DIR/intra-link-errors.rs:43:6
95+
--> $DIR/intra-link-errors.rs:51:6
8096
|
8197
LL | /// [S!]
8298
| ^^ help: to link to the struct, prefix with `struct@`: `struct@S`
8399
|
84100
= note: this link resolves to the struct `S`, which is not in the macro namespace
85101

86102
error: unresolved link to `T::g`
87-
--> $DIR/intra-link-errors.rs:61:6
103+
--> $DIR/intra-link-errors.rs:69:6
88104
|
89105
LL | /// [type@T::g]
90106
| ^^^^^^^^^ help: to link to the associated function, add parentheses: `T::g()`
91107
|
92108
= note: this link resolves to the associated function `g`, which is not in the type namespace
93109

94110
error: unresolved link to `T::h`
95-
--> $DIR/intra-link-errors.rs:66:6
111+
--> $DIR/intra-link-errors.rs:74:6
96112
|
97113
LL | /// [T::h!]
98114
| ^^^^^
99115
|
100116
= note: the trait `T` has no macro named `h`
101117

102118
error: unresolved link to `S::h`
103-
--> $DIR/intra-link-errors.rs:53:6
119+
--> $DIR/intra-link-errors.rs:61:6
104120
|
105121
LL | /// [type@S::h]
106122
| ^^^^^^^^^ help: to link to the associated function, add parentheses: `S::h()`
107123
|
108124
= note: this link resolves to the associated function `h`, which is not in the type namespace
109125

110126
error: unresolved link to `m`
111-
--> $DIR/intra-link-errors.rs:73:6
127+
--> $DIR/intra-link-errors.rs:81:6
112128
|
113129
LL | /// [m()]
114130
| ^^^ help: to link to the macro, add an exclamation mark: `m!`
115131
|
116132
= note: this link resolves to the macro `m`, which is not in the value namespace
117133

118-
error: aborting due to 14 previous errors
134+
error: aborting due to 16 previous errors
119135

0 commit comments

Comments
 (0)