Skip to content

Commit 56d3f39

Browse files
jyn514jyn514
committed
Unify error reporting for intra-doc links
- Give a suggestion even if there is no span available - Give a more accurate description of the change than 'use the disambiguator' - Write much less code
1 parent 18e7a1b commit 56d3f39

6 files changed

+137
-102
lines changed

Diff for: ‎src/librustdoc/passes/collect_intra_doc_links.rs

+51-60
Original file line numberDiff line numberDiff line change
@@ -898,20 +898,8 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
898898
specified.article(),
899899
specified.descr()
900900
);
901-
let suggestion = resolved.display_for(path_str);
902-
let help_msg =
903-
format!("to link to the {}, use its disambiguator", resolved.descr());
904901
diag.note(&note);
905-
if let Some(sp) = sp {
906-
diag.span_suggestion(
907-
sp,
908-
&help_msg,
909-
suggestion,
910-
Applicability::MaybeIncorrect,
911-
);
912-
} else {
913-
diag.help(&format!("{}: {}", help_msg, suggestion));
914-
}
902+
suggest_disambiguator(resolved, diag, path_str, &dox, sp, &link_range);
915903
});
916904
};
917905
if let Res::PrimTy(_) = res {
@@ -1047,17 +1035,31 @@ impl Disambiguator {
10471035
}
10481036
}
10491037

1050-
fn display_for(self, path_str: &str) -> String {
1038+
fn from_res(res: Res) -> Self {
1039+
match res {
1040+
Res::Def(kind, _) => Disambiguator::Kind(kind),
1041+
Res::PrimTy(_) => Disambiguator::Primitive,
1042+
_ => Disambiguator::Namespace(res.ns().expect("can't call `from_res` on Res::err")),
1043+
}
1044+
}
1045+
1046+
/// Return (description of the change, suggestion)
1047+
fn display_for(self, path_str: &str) -> (&'static str, String) {
1048+
const PREFIX: &str = "prefix with the item kind";
1049+
const FUNCTION: &str = "add parentheses";
1050+
const MACRO: &str = "add an exclamation mark";
1051+
10511052
let kind = match self {
1052-
Disambiguator::Primitive => return format!("prim@{}", path_str),
1053+
Disambiguator::Primitive => return (PREFIX, format!("prim@{}", path_str)),
10531054
Disambiguator::Kind(kind) => kind,
10541055
Disambiguator::Namespace(_) => panic!("display_for cannot be used on namespaces"),
10551056
};
10561057
if kind == DefKind::Macro(MacroKind::Bang) {
1057-
return format!("{}!", path_str);
1058+
return (MACRO, format!("{}!", path_str));
10581059
} else if kind == DefKind::Fn || kind == DefKind::AssocFn {
1059-
return format!("{}()", path_str);
1060+
return (FUNCTION, format!("{}()", path_str));
10601061
}
1062+
10611063
let prefix = match kind {
10621064
DefKind::Struct => "struct",
10631065
DefKind::Enum => "enum",
@@ -1079,7 +1081,9 @@ impl Disambiguator {
10791081
Namespace::MacroNS => "macro",
10801082
},
10811083
};
1082-
format!("{}@{}", prefix, path_str)
1084+
1085+
// FIXME: if this is an implied shortcut link, it's bad style to suggest `@`
1086+
(PREFIX, format!("{}@{}", prefix, path_str))
10831087
}
10841088

10851089
fn ns(self) -> Namespace {
@@ -1276,52 +1280,39 @@ fn ambiguity_error(
12761280
report_diagnostic(cx, &msg, item, dox, link_range.clone(), |diag, sp| {
12771281
if let Some(sp) = sp {
12781282
diag.span_label(sp, "ambiguous link");
1283+
} else {
1284+
diag.note("ambiguous link");
1285+
}
12791286

1280-
let link_range = link_range.expect("must have a link range if we have a span");
1281-
1282-
for (res, ns) in candidates {
1283-
let (action, mut suggestion) = match res {
1284-
Res::Def(DefKind::AssocFn | DefKind::Fn, _) => {
1285-
("add parentheses", format!("{}()", path_str))
1286-
}
1287-
Res::Def(DefKind::Macro(MacroKind::Bang), _) => {
1288-
("add an exclamation mark", format!("{}!", path_str))
1289-
}
1290-
_ => {
1291-
let type_ = match (res, ns) {
1292-
(Res::PrimTy(_), _) => "prim",
1293-
(Res::Def(DefKind::Const, _), _) => "const",
1294-
(Res::Def(DefKind::Static, _), _) => "static",
1295-
(Res::Def(DefKind::Struct, _), _) => "struct",
1296-
(Res::Def(DefKind::Enum, _), _) => "enum",
1297-
(Res::Def(DefKind::Union, _), _) => "union",
1298-
(Res::Def(DefKind::Trait, _), _) => "trait",
1299-
(Res::Def(DefKind::Mod, _), _) => "module",
1300-
(_, TypeNS) => "type",
1301-
(_, ValueNS) => "value",
1302-
(Res::Def(DefKind::Macro(MacroKind::Derive), _), MacroNS) => "derive",
1303-
(_, MacroNS) => "macro",
1304-
};
1305-
1306-
// FIXME: if this is an implied shortcut link, it's bad style to suggest `@`
1307-
("prefix with the item type", format!("{}@{}", type_, path_str))
1308-
}
1309-
};
1287+
for (res, _ns) in candidates {
1288+
let disambiguator = Disambiguator::from_res(res);
1289+
suggest_disambiguator(disambiguator, diag, path_str, dox, sp, &link_range);
1290+
}
1291+
});
1292+
}
13101293

1311-
if dox.bytes().nth(link_range.start) == Some(b'`') {
1312-
suggestion = format!("`{}`", suggestion);
1313-
}
1294+
fn suggest_disambiguator(
1295+
disambiguator: Disambiguator,
1296+
diag: &mut DiagnosticBuilder<'_>,
1297+
path_str: &str,
1298+
dox: &str,
1299+
sp: Option<rustc_span::Span>,
1300+
link_range: &Option<Range<usize>>,
1301+
) {
1302+
let (action, mut suggestion) = disambiguator.display_for(path_str);
1303+
let help = format!("to link to the {}, {}", disambiguator.descr(), action);
13141304

1315-
// FIXME: Create a version of this suggestion for when we don't have the span.
1316-
diag.span_suggestion(
1317-
sp,
1318-
&format!("to link to the {}, {}", res.descr(), action),
1319-
suggestion,
1320-
Applicability::MaybeIncorrect,
1321-
);
1322-
}
1305+
if let Some(sp) = sp {
1306+
let link_range = link_range.as_ref().expect("must have a link range if we have a span");
1307+
if dox.bytes().nth(link_range.start) == Some(b'`') {
1308+
suggestion = format!("`{}`", suggestion);
13231309
}
1324-
});
1310+
1311+
// FIXME: Create a version of this suggestion for when we don't have the span.
1312+
diag.span_suggestion(sp, &help, suggestion, Applicability::MaybeIncorrect);
1313+
} else {
1314+
diag.help(&format!("{}: {}", help, suggestion));
1315+
}
13251316
}
13261317

13271318
fn privacy_error(

Diff for: ‎src/test/rustdoc-ui/intra-link-prim-conflict.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,13 @@
1818
1919
/// [struct@char]
2020
//~^ ERROR incompatible link
21-
//~| HELP use its disambiguator
21+
//~| HELP prefix with the item kind
2222
//~| NOTE resolved to a module
2323
pub mod char {}
2424

2525
pub mod inner {
2626
//! [struct@char]
2727
//~^ ERROR incompatible link
28-
//~| HELP use its disambiguator
28+
//~| HELP prefix with the item kind
2929
//~| NOTE resolved to a builtin type
3030
}

Diff for: ‎src/test/rustdoc-ui/intra-link-prim-conflict.stderr

+18-10
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ note: the lint level is defined here
99
|
1010
LL | #![deny(broken_intra_doc_links)]
1111
| ^^^^^^^^^^^^^^^^^^^^^^
12-
help: to link to the module, prefix with the item type
12+
help: to link to the module, prefix with the item kind
1313
|
14-
LL | /// [module@char]
15-
| ^^^^^^^^^^^
16-
help: to link to the builtin type, prefix with the item type
14+
LL | /// [mod@char]
15+
| ^^^^^^^^
16+
help: to link to the builtin type, prefix with the item kind
1717
|
1818
LL | /// [prim@char]
1919
| ^^^^^^^^^
@@ -24,11 +24,11 @@ error: `char` is both a module and a builtin type
2424
LL | /// [type@char]
2525
| ^^^^^^^^^ ambiguous link
2626
|
27-
help: to link to the module, prefix with the item type
27+
help: to link to the module, prefix with the item kind
2828
|
29-
LL | /// [module@char]
30-
| ^^^^^^^^^^^
31-
help: to link to the builtin type, prefix with the item type
29+
LL | /// [mod@char]
30+
| ^^^^^^^^
31+
help: to link to the builtin type, prefix with the item kind
3232
|
3333
LL | /// [prim@char]
3434
| ^^^^^^^^^
@@ -37,17 +37,25 @@ error: incompatible link kind for `char`
3737
--> $DIR/intra-link-prim-conflict.rs:19:6
3838
|
3939
LL | /// [struct@char]
40-
| ^^^^^^^^^^^ help: to link to the module, use its disambiguator: `mod@char`
40+
| ^^^^^^^^^^^
4141
|
4242
= note: this link resolved to a module, which is not a struct
43+
help: to link to the module, prefix with the item kind
44+
|
45+
LL | /// [mod@char]
46+
| ^^^^^^^^
4347

4448
error: incompatible link kind for `char`
4549
--> $DIR/intra-link-prim-conflict.rs:26:10
4650
|
4751
LL | //! [struct@char]
48-
| ^^^^^^^^^^^ help: to link to the builtin type, use its disambiguator: `prim@char`
52+
| ^^^^^^^^^^^
4953
|
5054
= note: this link resolved to a builtin type, which is not a struct
55+
help: to link to the builtin type, prefix with the item kind
56+
|
57+
LL | //! [prim@char]
58+
| ^^^^^^^^^
5159

5260
error: aborting due to 4 previous errors
5361

Diff for: ‎src/test/rustdoc-ui/intra-links-ambiguity.stderr

+8-8
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ note: the lint level is defined here
99
|
1010
LL | #![deny(broken_intra_doc_links)]
1111
| ^^^^^^^^^^^^^^^^^^^^^^
12-
help: to link to the struct, prefix with the item type
12+
help: to link to the struct, prefix with the item kind
1313
|
1414
LL | /// [`struct@ambiguous`] is ambiguous.
1515
| ^^^^^^^^^^^^^^^^^^
@@ -24,7 +24,7 @@ error: `ambiguous` is both a struct and a function
2424
LL | /// [ambiguous] is ambiguous.
2525
| ^^^^^^^^^ ambiguous link
2626
|
27-
help: to link to the struct, prefix with the item type
27+
help: to link to the struct, prefix with the item kind
2828
|
2929
LL | /// [struct@ambiguous] is ambiguous.
3030
| ^^^^^^^^^^^^^^^^
@@ -39,7 +39,7 @@ error: `multi_conflict` is a struct, a function, and a macro
3939
LL | /// [`multi_conflict`] is a three-way conflict.
4040
| ^^^^^^^^^^^^^^^^ ambiguous link
4141
|
42-
help: to link to the struct, prefix with the item type
42+
help: to link to the struct, prefix with the item kind
4343
|
4444
LL | /// [`struct@multi_conflict`] is a three-way conflict.
4545
| ^^^^^^^^^^^^^^^^^^^^^^^
@@ -58,11 +58,11 @@ error: `type_and_value` is both a module and a constant
5858
LL | /// Ambiguous [type_and_value].
5959
| ^^^^^^^^^^^^^^ ambiguous link
6060
|
61-
help: to link to the module, prefix with the item type
61+
help: to link to the module, prefix with the item kind
6262
|
63-
LL | /// Ambiguous [module@type_and_value].
64-
| ^^^^^^^^^^^^^^^^^^^^^
65-
help: to link to the constant, prefix with the item type
63+
LL | /// Ambiguous [mod@type_and_value].
64+
| ^^^^^^^^^^^^^^^^^^
65+
help: to link to the constant, prefix with the item kind
6666
|
6767
LL | /// Ambiguous [const@type_and_value].
6868
| ^^^^^^^^^^^^^^^^^^^^
@@ -73,7 +73,7 @@ error: `foo::bar` is both an enum and a function
7373
LL | /// Ambiguous non-implied shortcut link [`foo::bar`].
7474
| ^^^^^^^^^^ ambiguous link
7575
|
76-
help: to link to the enum, prefix with the item type
76+
help: to link to the enum, prefix with the item kind
7777
|
7878
LL | /// Ambiguous non-implied shortcut link [`enum@foo::bar`].
7979
| ^^^^^^^^^^^^^^^

Diff for: ‎src/test/rustdoc-ui/intra-links-disambiguator-mismatch.rs

+11-11
Original file line numberDiff line numberDiff line change
@@ -14,55 +14,55 @@ trait T {}
1414
/// Link to [struct@S]
1515
//~^ ERROR incompatible link kind for `S`
1616
//~| NOTE this link resolved
17-
//~| HELP use its disambiguator
17+
//~| HELP prefix with the item kind
1818

1919
/// Link to [mod@S]
2020
//~^ ERROR incompatible link kind for `S`
2121
//~| NOTE this link resolved
22-
//~| HELP use its disambiguator
22+
//~| HELP prefix with the item kind
2323

2424
/// Link to [union@S]
2525
//~^ ERROR incompatible link kind for `S`
2626
//~| NOTE this link resolved
27-
//~| HELP use its disambiguator
27+
//~| HELP prefix with the item kind
2828

2929
/// Link to [trait@S]
3030
//~^ ERROR incompatible link kind for `S`
3131
//~| NOTE this link resolved
32-
//~| HELP use its disambiguator
32+
//~| HELP prefix with the item kind
3333

3434
/// Link to [struct@T]
3535
//~^ ERROR incompatible link kind for `T`
3636
//~| NOTE this link resolved
37-
//~| HELP use its disambiguator
37+
//~| HELP prefix with the item kind
3838

3939
/// Link to [derive@m]
4040
//~^ ERROR incompatible link kind for `m`
4141
//~| NOTE this link resolved
42-
//~| HELP use its disambiguator
42+
//~| HELP add an exclamation mark
4343

4444
/// Link to [const@s]
4545
//~^ ERROR incompatible link kind for `s`
4646
//~| NOTE this link resolved
47-
//~| HELP use its disambiguator
47+
//~| HELP prefix with the item kind
4848

4949
/// Link to [static@c]
5050
//~^ ERROR incompatible link kind for `c`
5151
//~| NOTE this link resolved
52-
//~| HELP use its disambiguator
52+
//~| HELP prefix with the item kind
5353

5454
/// Link to [fn@c]
5555
//~^ ERROR incompatible link kind for `c`
5656
//~| NOTE this link resolved
57-
//~| HELP use its disambiguator
57+
//~| HELP prefix with the item kind
5858

5959
/// Link to [c()]
6060
//~^ ERROR incompatible link kind for `c`
6161
//~| NOTE this link resolved
62-
//~| HELP use its disambiguator
62+
//~| HELP prefix with the item kind
6363

6464
/// Link to [const@f]
6565
//~^ ERROR incompatible link kind for `f`
6666
//~| NOTE this link resolved
67-
//~| HELP use its disambiguator
67+
//~| HELP add parentheses
6868
pub fn f() {}

0 commit comments

Comments
 (0)