Skip to content

Commit

Permalink
Unify primitive errors with other intra-link errors
Browse files Browse the repository at this point in the history
Now that `PrimTy::name()` exists, there's no need to carry around the
name of the primitive that failed to resolve. This removes the variants
special-casing primitives in favor of `NotResolved`.

- Remove `NoPrimitiveImpl` and `NoPrimitiveAssocItem`
- Remove hacky `has_primitive` check in `resolution_failure()`
- Fixup a couple tests that I forgot to `--bless` before
  • Loading branch information
jyn514 committed Sep 24, 2020
1 parent 472e52e commit 049d29b
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 43 deletions.
48 changes: 14 additions & 34 deletions src/librustdoc/passes/collect_intra_doc_links.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,13 +60,6 @@ impl<'a> From<ResolutionFailure<'a>> for ErrorKind<'a> {

#[derive(Debug)]
enum ResolutionFailure<'a> {
/// this is a primitive type without an impls (no associated methods)
/// when will this actually happen?
/// the `Res` is the primitive it resolved to
NoPrimitiveImpl(Res, String),
/// `[u8::not_found]`
/// the `Res` is the primitive it resolved to
NoPrimitiveAssocItem { res: Res, prim_name: &'a str, assoc_item: Symbol },
/// This resolved, but with the wrong namespace.
/// `Namespace` is the expected namespace (as opposed to the actual).
WrongNamespace(Res, Namespace),
Expand Down Expand Up @@ -326,8 +319,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
})?;

if let Some((path, prim)) = is_primitive(&path_root, TypeNS) {
let impls = primitive_impl(cx, &path)
.ok_or_else(|| ResolutionFailure::NoPrimitiveImpl(prim, path_root.into()))?;
let impls =
primitive_impl(cx, &path).ok_or_else(|| ResolutionFailure::NotResolved {
module_id,
partial_res: Some(prim),
unresolved: item_str.into(),
})?;
for &impl_ in impls {
let link = cx
.tcx
Expand All @@ -354,10 +351,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
item_name,
ns.descr()
);
return Err(ResolutionFailure::NoPrimitiveAssocItem {
res: prim,
prim_name: path,
assoc_item: item_name,
return Err(ResolutionFailure::NotResolved {
module_id,
partial_res: Some(prim),
unresolved: item_str.into(),
}
.into());
}
Expand Down Expand Up @@ -1009,7 +1006,7 @@ impl LinkCollector<'_, '_> {
suggest_disambiguator(resolved, diag, path_str, dox, sp, &link_range);
});
};
if let Res::PrimTy(ty) = res {
if let Res::PrimTy(..) = res {
match disambiguator {
Some(Disambiguator::Primitive | Disambiguator::Namespace(_)) | None => {
item.attrs.links.push(ItemLink {
Expand Down Expand Up @@ -1489,10 +1486,6 @@ fn resolution_failure(
link_range: Option<Range<usize>>,
kinds: SmallVec<[ResolutionFailure<'_>; 3]>,
) {
let has_primitive = kinds.iter().any(|err|
matches!(err, ResolutionFailure::NoPrimitiveAssocItem{..} | ResolutionFailure::NoPrimitiveImpl(_, _))
);

report_diagnostic(
collector.cx,
&format!("unresolved link to `{}`", path_str),
Expand Down Expand Up @@ -1533,7 +1526,7 @@ fn resolution_failure(

let module_id = *module_id;
// FIXME(jynelson): this might conflict with my `Self` fix in #76467
// FIXME: use itertools `collect_tuple` instead
// FIXME: maybe use itertools `collect_tuple` instead?
fn split(path: &str) -> Option<(&str, &str)> {
let mut splitter = path.rsplitn(2, "::");
splitter.next().and_then(|right| splitter.next().map(|left| (left, right)))
Expand Down Expand Up @@ -1600,10 +1593,7 @@ fn resolution_failure(
diagnostic_name = collector.cx.tcx.item_name(def_id).as_str();
(Some(kind), &*diagnostic_name)
}
Res::PrimTy(_) => {
assert!(has_primitive);
continue;
}
Res::PrimTy(ty) => (None, ty.name_str()),
_ => unreachable!("only ADTs and primitives are in scope at module level"),
};
let path_description = if let Some(kind) = kind {
Expand Down Expand Up @@ -1640,7 +1630,7 @@ fn resolution_failure(
Impl | GlobalAsm => unreachable!("not a path"),
}
} else {
res.descr()
"associated item"
};
let note = format!(
"the {} `{}` has no {} named `{}`",
Expand Down Expand Up @@ -1683,16 +1673,6 @@ fn resolution_failure(
diag.level = rustc_errors::Level::Bug;
"all intra doc links should have a parent item".to_owned()
}
ResolutionFailure::NoPrimitiveImpl(res, _) => format!(
"this link partially resolves to {}, which does not have any associated items",
item(res),
),
ResolutionFailure::NoPrimitiveAssocItem { prim_name, assoc_item, .. } => {
format!(
"the builtin type `{}` does not have an associated item named `{}`",
prim_name, assoc_item
)
}
};
if let Some(span) = sp {
diag.span_label(span, &note);
Expand Down
11 changes: 10 additions & 1 deletion src/test/rustdoc-ui/intra-link-errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,16 @@

/// [u8::not_found]
//~^ ERROR unresolved link
//~| NOTE the builtin type `u8` does not have an associated item named `not_found`
//~| NOTE the builtin type `u8` has no associated item named `not_found`

/// [std::primitive::u8::not_found]
//~^ ERROR unresolved link
//~| NOTE the builtin type `u8` has no associated item named `not_found`

/// [type@Vec::into_iter]
//~^ ERROR unresolved link
//~| HELP to link to the associated function, add parentheses
//~| NOTE this link resolves to the associated function `into_iter`

/// [S!]
//~^ ERROR unresolved link
Expand Down
29 changes: 22 additions & 7 deletions src/test/rustdoc-ui/intra-link-errors.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -80,19 +80,34 @@ error: unresolved link to `u8::not_found`
--> $DIR/intra-link-errors.rs:55:6
|
LL | /// [u8::not_found]
| ^^^^^^^^^^^^^ the builtin type `u8` does not have an associated item named `not_found`
| ^^^^^^^^^^^^^ the builtin type `u8` has no associated item named `not_found`

error: unresolved link to `S`
error: unresolved link to `std::primitive::u8::not_found`
--> $DIR/intra-link-errors.rs:59:6
|
LL | /// [std::primitive::u8::not_found]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the builtin type `u8` has no associated item named `not_found`

error: unresolved link to `Vec::into_iter`
--> $DIR/intra-link-errors.rs:63:6
|
LL | /// [type@Vec::into_iter]
| ^^^^^^^^^^^^^^^^^^^
| |
| this link resolves to the associated function `into_iter`, which is not in the type namespace
| help: to link to the associated function, add parentheses: `Vec::into_iter()`

error: unresolved link to `S`
--> $DIR/intra-link-errors.rs:68:6
|
LL | /// [S!]
| ^^
| |
| this link resolves to the struct `S`, which is not in the macro namespace
| help: to link to the struct, prefix with `struct@`: `struct@S`

error: unresolved link to `T::g`
--> $DIR/intra-link-errors.rs:77:6
--> $DIR/intra-link-errors.rs:86:6
|
LL | /// [type@T::g]
| ^^^^^^^^^
Expand All @@ -101,13 +116,13 @@ LL | /// [type@T::g]
| help: to link to the associated function, add parentheses: `T::g()`

error: unresolved link to `T::h`
--> $DIR/intra-link-errors.rs:82:6
--> $DIR/intra-link-errors.rs:91:6
|
LL | /// [T::h!]
| ^^^^^ the trait `T` has no macro named `h`

error: unresolved link to `S::h`
--> $DIR/intra-link-errors.rs:69:6
--> $DIR/intra-link-errors.rs:78:6
|
LL | /// [type@S::h]
| ^^^^^^^^^
Expand All @@ -116,13 +131,13 @@ LL | /// [type@S::h]
| help: to link to the associated function, add parentheses: `S::h()`

error: unresolved link to `m`
--> $DIR/intra-link-errors.rs:89:6
--> $DIR/intra-link-errors.rs:98:6
|
LL | /// [m()]
| ^^^
| |
| this link resolves to the macro `m`, which is not in the value namespace
| help: to link to the macro, add an exclamation mark: `m!`

error: aborting due to 18 previous errors
error: aborting due to 20 previous errors

2 changes: 1 addition & 1 deletion src/test/rustdoc/intra-link-associated-items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

/// [`std::collections::BTreeMap::into_iter`]
/// [`String::from`] is ambiguous as to which `From` impl
/// [type@Vec::into_iter] uses a disambiguator
/// [Vec::into_iter()] uses a disambiguator
// @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/collections/btree/map/struct.BTreeMap.html#method.into_iter"]' 'std::collections::BTreeMap::into_iter'
// @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/string/struct.String.html#method.from"]' 'String::from'
// @has 'intra_link_associated_items/fn.foo.html' '//a[@href="https://doc.rust-lang.org/nightly/alloc/vec/struct.Vec.html#method.into_iter"]' 'Vec::into_iter'
Expand Down

0 comments on commit 049d29b

Please sign in to comment.