Skip to content

Commit 418f608

Browse files
jyn514jyn514
committed
Give a better error message when linking to a macro with the wrong disambiguator
Before: ``` warning: unresolved link to `m` --> m.rs:1:6 | 1 | /// [value@m] | ^^^^^^^ | = note: `#[warn(broken_intra_doc_links)]` on by default = note: no item named `m` is in scope = help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]` ``` After: ``` warning: unresolved link to `m` --> m.rs:1:6 | 1 | /// [value@m] | ^^^^^^^ help: to link to the macro, use its disambiguator: `m!` | = note: `#[warn(broken_intra_doc_links)]` on by default = note: this link resolves to the macro `m`, which is not in the value namespace ```
1 parent 6875220 commit 418f608

File tree

3 files changed

+142
-50
lines changed

3 files changed

+142
-50
lines changed

‎src/librustdoc/passes/collect_intra_doc_links.rs‎

Lines changed: 79 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ enum ResolutionFailure<'a> {
7474
NoAssocItem(Res, Symbol),
7575
/// should not ever happen
7676
NoParentItem,
77-
/// the root of this path resolved, but it was not an enum.
78-
NotAnEnum(Res),
7977
/// this could be an enum variant, but the last path fragment wasn't resolved.
8078
/// the `String` is the variant that didn't exist
8179
NotAVariant(Res, Symbol),
@@ -91,7 +89,6 @@ impl ResolutionFailure<'a> {
9189
NoPrimitiveAssocItem { res, .. }
9290
| NoAssocItem(res, _)
9391
| NoPrimitiveImpl(res, _)
94-
| NotAnEnum(res)
9592
| NotAVariant(res, _)
9693
| WrongNamespace(res, _)
9794
| CannotHaveAssociatedItems(res, _) => Some(*res),
@@ -133,6 +130,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
133130
path_str: &'path str,
134131
current_item: &Option<String>,
135132
module_id: DefId,
133+
extra_fragment: &Option<String>,
136134
) -> Result<(Res, Option<String>), ErrorKind<'path>> {
137135
let cx = self.cx;
138136

@@ -202,7 +200,28 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
202200
_ => unreachable!(),
203201
}
204202
}
205-
_ => Err(ErrorKind::Resolve(ResolutionFailure::NotAnEnum(ty_res))),
203+
// `variant_field` looks at 3 different path segments in a row.
204+
// But `NoAssocItem` assumes there are only 2. Check to see if there's
205+
// an intermediate segment that resolves.
206+
_ => {
207+
let intermediate_path = format!("{}::{}", path, variant_name);
208+
// NOTE: we have to be careful here, because we're already in `resolve`.
209+
// We know this doesn't recurse forever because we use a shorter path each time.
210+
// NOTE: this uses `TypeNS` because nothing else has a valid path segment after
211+
let kind = if let Some(intermediate) = self.check_full_res(
212+
TypeNS,
213+
&intermediate_path,
214+
Some(module_id),
215+
current_item,
216+
extra_fragment,
217+
) {
218+
ResolutionFailure::NoAssocItem(intermediate, variant_field_name)
219+
} else {
220+
// Even with the shorter path, it didn't resolve, so say that.
221+
ResolutionFailure::NoAssocItem(ty_res, variant_name)
222+
};
223+
Err(ErrorKind::Resolve(kind))
224+
}
206225
}
207226
}
208227

@@ -376,7 +395,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
376395
let ty_res = match ty_res {
377396
Err(()) | Ok(Res::Err) => {
378397
return if ns == Namespace::ValueNS {
379-
self.variant_field(path_str, current_item, module_id)
398+
self.variant_field(path_str, current_item, module_id, extra_fragment)
380399
} else {
381400
// See if it only broke because of the namespace.
382401
let kind = cx.enter_resolver(|resolver| {
@@ -533,7 +552,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
533552
};
534553
res.unwrap_or_else(|| {
535554
if ns == Namespace::ValueNS {
536-
self.variant_field(path_str, current_item, module_id)
555+
self.variant_field(path_str, current_item, module_id, extra_fragment)
537556
} else {
538557
Err(ErrorKind::Resolve(ResolutionFailure::NoAssocItem(ty_res, item_name)))
539558
}
@@ -543,6 +562,41 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
543562
Err(ErrorKind::Resolve(ResolutionFailure::NoParentItem))
544563
}
545564
}
565+
566+
// used for reporting better errors
567+
fn check_full_res(
568+
&self,
569+
ns: Namespace,
570+
path_str: &str,
571+
base_node: Option<DefId>,
572+
current_item: &Option<String>,
573+
extra_fragment: &Option<String>,
574+
) -> Option<Res> {
575+
let check_full_res_inner = |this: &Self, result: Result<Res, ErrorKind<'_>>| {
576+
let res = match result {
577+
Ok(res) => Some(res),
578+
Err(ErrorKind::Resolve(kind)) => kind.full_res(),
579+
Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) => {
580+
Some(res)
581+
}
582+
Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None,
583+
};
584+
this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res)
585+
};
586+
// cannot be used for macro namespace
587+
let check_full_res = |this: &Self, ns| {
588+
let result = this.resolve(path_str, ns, current_item, base_node, extra_fragment);
589+
check_full_res_inner(this, result.map(|(res, _)| res))
590+
};
591+
let check_full_res_macro = |this: &Self| {
592+
let result = this.macro_resolve(path_str, base_node);
593+
check_full_res_inner(this, result.map_err(ErrorKind::Resolve))
594+
};
595+
match ns {
596+
Namespace::MacroNS => check_full_res_macro(self),
597+
Namespace::TypeNS | Namespace::ValueNS => check_full_res(self, ns),
598+
}
599+
}
546600
}
547601

548602
fn resolve_associated_trait_item(
@@ -652,7 +706,7 @@ fn traits_implemented_by(cx: &DocContext<'_>, type_: DefId, module: DefId) -> Fx
652706
let trait_ref = cx.tcx.impl_trait_ref(impl_).expect("this is not an inherent impl");
653707
// Check if these are the same type.
654708
let impl_type = trait_ref.self_ty();
655-
debug!(
709+
trace!(
656710
"comparing type {} with kind {:?} against type {:?}",
657711
impl_type,
658712
impl_type.kind(),
@@ -875,40 +929,26 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
875929
}
876930
}
877931

878-
// used for reporting better errors
879-
let check_full_res = |this: &mut Self, ns| {
880-
let res =
881-
match this.resolve(path_str, ns, &current_item, base_node, &extra_fragment)
882-
{
883-
Ok(res) => {
884-
debug!(
885-
"check_full_res: saw res for {} in {:?} ns: {:?}",
886-
path_str, ns, res.0
887-
);
888-
Some(res.0)
889-
}
890-
Err(ErrorKind::Resolve(kind)) => kind.full_res(),
891-
Err(ErrorKind::AnchorFailure(
892-
AnchorFailure::RustdocAnchorConflict(res),
893-
)) => Some(res),
894-
Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None,
895-
};
896-
this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res)
897-
};
898-
899932
match disambiguator.map(Disambiguator::ns) {
900933
Some(ns @ (ValueNS | TypeNS)) => {
901934
match self.resolve(path_str, ns, &current_item, base_node, &extra_fragment)
902935
{
903936
Ok(res) => res,
904937
Err(ErrorKind::Resolve(mut kind)) => {
905938
// We only looked in one namespace. Try to give a better error if possible.
906-
// TODO: handle MacroNS too
907939
if kind.full_res().is_none() {
908940
let other_ns = if ns == ValueNS { TypeNS } else { ValueNS };
909-
if let Some(res) = check_full_res(self, other_ns) {
910-
// recall that this stores the _expected_ namespace
911-
kind = ResolutionFailure::WrongNamespace(res, ns);
941+
for &ns in &[other_ns, MacroNS] {
942+
if let Some(res) = self.check_full_res(
943+
ns,
944+
path_str,
945+
base_node,
946+
&current_item,
947+
&extra_fragment,
948+
) {
949+
kind = ResolutionFailure::WrongNamespace(res, ns);
950+
break;
951+
}
912952
}
913953
}
914954
resolution_failure(
@@ -1033,7 +1073,13 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
10331073
Err(mut kind) => {
10341074
// `macro_resolve` only looks in the macro namespace. Try to give a better error if possible.
10351075
for &ns in &[TypeNS, ValueNS] {
1036-
if let Some(res) = check_full_res(self, ns) {
1076+
if let Some(res) = self.check_full_res(
1077+
ns,
1078+
path_str,
1079+
base_node,
1080+
&current_item,
1081+
&extra_fragment,
1082+
) {
10371083
kind = ResolutionFailure::WrongNamespace(res, MacroNS);
10381084
break;
10391085
}
@@ -1525,13 +1571,6 @@ fn resolution_failure(
15251571
ResolutionFailure::CannotHaveAssociatedItems(res, _) => {
15261572
assoc_item_not_allowed(res, diag)
15271573
}
1528-
// TODO: is there ever a case where this happens?
1529-
ResolutionFailure::NotAnEnum(res) => {
1530-
let note =
1531-
format!("this link resolves to {}, which is not an enum", item(res));
1532-
diag.note(&note);
1533-
diag.note("if this were an enum, it might have a variant which resolved");
1534-
}
15351574
ResolutionFailure::NotAVariant(res, variant) => {
15361575
let note = format!(
15371576
"this link partially resolves to {}, but there is no variant named {}",

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,18 @@
1010
//~| NOTE no item named `path::to` is in scope
1111
//~| HELP to escape
1212

13+
/// [std::io::not::here]
14+
//~^ ERROR unresolved link
15+
//~| NOTE the module `io` has no inner item
16+
17+
/// [std::io::Error::x]
18+
//~^ ERROR unresolved link
19+
//~| NOTE the struct `Error` has no field
20+
21+
/// [std::io::ErrorKind::x]
22+
//~^ ERROR unresolved link
23+
//~| NOTE the enum `ErrorKind` has no variant
24+
1325
/// [f::A]
1426
//~^ ERROR unresolved link
1527
//~| NOTE `f` is a function, not a module
@@ -60,3 +72,12 @@ impl S {
6072
pub trait T {
6173
fn g() {}
6274
}
75+
76+
/// [m()]
77+
//~^ ERROR unresolved link
78+
//~| HELP to link to the macro
79+
//~| NOTE not in the value namespace
80+
#[macro_export]
81+
macro_rules! m {
82+
() => {};
83+
}

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

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,64 +12,88 @@ LL | #![deny(broken_intra_doc_links)]
1212
= note: no item named `path::to` is in scope
1313
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
1414

15-
error: unresolved link to `f::A`
15+
error: unresolved link to `std::io::not::here`
1616
--> $DIR/intra-link-errors.rs:13:6
1717
|
18+
LL | /// [std::io::not::here]
19+
| ^^^^^^^^^^^^^^^^^^
20+
|
21+
= note: the module `io` has no inner item named `not`
22+
23+
error: unresolved link to `std::io::Error::x`
24+
--> $DIR/intra-link-errors.rs:17:6
25+
|
26+
LL | /// [std::io::Error::x]
27+
| ^^^^^^^^^^^^^^^^^
28+
|
29+
= note: the struct `Error` has no field or associated item named `x`
30+
31+
error: unresolved link to `std::io::ErrorKind::x`
32+
--> $DIR/intra-link-errors.rs:21:6
33+
|
34+
LL | /// [std::io::ErrorKind::x]
35+
| ^^^^^^^^^^^^^^^^^^^^^
36+
|
37+
= note: the enum `ErrorKind` has no variant or associated item named `x`
38+
39+
error: unresolved link to `f::A`
40+
--> $DIR/intra-link-errors.rs:25:6
41+
|
1842
LL | /// [f::A]
1943
| ^^^^
2044
|
2145
= note: `f` is a function, not a module or type, and cannot have associated items
2246

2347
error: unresolved link to `S::A`
24-
--> $DIR/intra-link-errors.rs:17:6
48+
--> $DIR/intra-link-errors.rs:29:6
2549
|
2650
LL | /// [S::A]
2751
| ^^^^
2852
|
2953
= note: the struct `S` has no field or associated item named `A`
3054

3155
error: unresolved link to `S::fmt`
32-
--> $DIR/intra-link-errors.rs:21:6
56+
--> $DIR/intra-link-errors.rs:33:6
3357
|
3458
LL | /// [S::fmt]
3559
| ^^^^^^
3660
|
3761
= note: the struct `S` has no field or associated item named `fmt`
3862

3963
error: unresolved link to `E::D`
40-
--> $DIR/intra-link-errors.rs:25:6
64+
--> $DIR/intra-link-errors.rs:37:6
4165
|
4266
LL | /// [E::D]
4367
| ^^^^
4468
|
4569
= note: the enum `E` has no variant or associated item named `D`
4670

4771
error: unresolved link to `u8::not_found`
48-
--> $DIR/intra-link-errors.rs:29:6
72+
--> $DIR/intra-link-errors.rs:41:6
4973
|
5074
LL | /// [u8::not_found]
5175
| ^^^^^^^^^^^^^
5276
|
5377
= note: the builtin type `u8` does not have an associated item named `not_found`
5478

5579
error: unresolved link to `S`
56-
--> $DIR/intra-link-errors.rs:33:6
80+
--> $DIR/intra-link-errors.rs:45:6
5781
|
5882
LL | /// [S!]
5983
| ^^ help: to link to the struct, use its disambiguator: `struct@S`
6084
|
6185
= note: this link resolves to the struct `S`, which is not in the macro namespace
6286

6387
error: unresolved link to `T::g`
64-
--> $DIR/intra-link-errors.rs:51:6
88+
--> $DIR/intra-link-errors.rs:63:6
6589
|
6690
LL | /// [type@T::g]
6791
| ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `T::g()`
6892
|
6993
= note: this link resolves to the associated function `g`, which is not in the type namespace
7094

7195
error: unresolved link to `T::h`
72-
--> $DIR/intra-link-errors.rs:56:6
96+
--> $DIR/intra-link-errors.rs:68:6
7397
|
7498
LL | /// [T::h!]
7599
| ^^^^^
@@ -78,12 +102,20 @@ LL | /// [T::h!]
78102
= help: to escape `[` and `]` characters, add '\' before them like `\[` or `\]`
79103

80104
error: unresolved link to `S::h`
81-
--> $DIR/intra-link-errors.rs:43:6
105+
--> $DIR/intra-link-errors.rs:55:6
82106
|
83107
LL | /// [type@S::h]
84108
| ^^^^^^^^^ help: to link to the associated function, use its disambiguator: `S::h()`
85109
|
86110
= note: this link resolves to the associated function `h`, which is not in the type namespace
87111

88-
error: aborting due to 10 previous errors
112+
error: unresolved link to `m`
113+
--> $DIR/intra-link-errors.rs:76:6
114+
|
115+
LL | /// [m()]
116+
| ^^^ help: to link to the macro, use its disambiguator: `m!`
117+
|
118+
= note: this link resolves to the macro `m`, which is not in the value namespace
119+
120+
error: aborting due to 14 previous errors
89121

0 commit comments

Comments
 (0)