rustdoc: handle generics better when matching notable traits

This commit makes the `clean::Type::is_same` non-commutative, so
that a generic `impl` matches a concrete return, but a generic return
does not match a concrete `impl`. It makes slice and vector Write
for `u8` not match on every generic return value.
This commit is contained in:
Michael Howell 2023-03-09 13:41:13 -07:00
parent c6015851f7
commit ee6b228b6a
5 changed files with 96 additions and 6 deletions

View File

@ -1471,11 +1471,37 @@ pub(crate) fn without_borrowed_ref(&self) -> &Type {
result
}
/// Check if two types are "potentially the same".
pub(crate) fn is_borrowed_ref(&self) -> bool {
matches!(self, Type::BorrowedRef { .. })
}
/// Check if two types are "the same" for documentation purposes.
///
/// This is different from `Eq`, because it knows that things like
/// `Placeholder` are possible matches for everything.
///
/// This relation is not commutative when generics are involved:
///
/// ```ignore(private)
/// # // see types/tests.rs:is_same_generic for the real test
/// use rustdoc::format::cache::Cache;
/// use rustdoc::clean::types::{Type, PrimitiveType};
/// let cache = Cache::new(false);
/// let generic = Type::Generic(rustc_span::symbol::sym::Any);
/// let unit = Type::Primitive(PrimitiveType::Unit);
/// assert!(!generic.is_same(&unit, &cache));
/// assert!(unit.is_same(&generic, &cache));
/// ```
///
/// An owned type is also the same as its borrowed variants (this is commutative),
/// but `&T` is not the same as `&mut T`.
pub(crate) fn is_same(&self, other: &Self, cache: &Cache) -> bool {
match (self, other) {
let (self_cleared, other_cleared) = if !self.is_borrowed_ref() || !other.is_borrowed_ref() {
(self.without_borrowed_ref(), other.without_borrowed_ref())
} else {
(self, other)
};
match (self_cleared, other_cleared) {
// Recursive cases.
(Type::Tuple(a), Type::Tuple(b)) => {
a.len() == b.len() && a.iter().zip(b).all(|(a, b)| a.is_same(b, cache))
@ -1489,9 +1515,21 @@ pub(crate) fn is_same(&self, other: &Self, cache: &Cache) -> bool {
Type::BorrowedRef { mutability, type_, .. },
Type::BorrowedRef { mutability: b_mutability, type_: b_type_, .. },
) => mutability == b_mutability && type_.is_same(b_type_, cache),
// Placeholders and generics are equal to all other types.
// Placeholders are equal to all other types.
(Type::Infer, _) | (_, Type::Infer) => true,
(Type::Generic(_), _) | (_, Type::Generic(_)) => true,
// Generics match everything on the right, but not on the left.
(_, Type::Generic(_)) => true,
(Type::Generic(_), _) => false,
// Paths account for both the path itself and its generics.
(Type::Path { path: a }, Type::Path { path: b }) => {
a.def_id() == b.def_id()
&& a.generics()
.zip(b.generics())
.map(|(ag, bg)| {
ag.iter().zip(bg.iter()).all(|(at, bt)| at.is_same(bt, cache))
})
.unwrap_or(true)
}
// Other cases, such as primitives, just use recursion.
(a, b) => a
.def_id(cache)

View File

@ -69,3 +69,14 @@ fn should_not_trim() {
run_test("\t line1 \n\t line2", "line1 \nline2");
run_test(" \tline1 \n \tline2", "line1 \nline2");
}
#[test]
fn is_same_generic() {
use crate::clean::types::{PrimitiveType, Type};
use crate::formats::cache::Cache;
let cache = Cache::new(false);
let generic = Type::Generic(rustc_span::symbol::sym::Any);
let unit = Type::Primitive(PrimitiveType::Unit);
assert!(!generic.is_same(&unit, &cache));
assert!(unit.is_same(&generic, &cache));
}

View File

@ -1291,7 +1291,7 @@ pub(crate) fn notable_traits_button(ty: &clean::Type, cx: &mut Context<'_>) -> O
if let Some(impls) = cx.cache().impls.get(&did) {
for i in impls {
let impl_ = i.inner_impl();
if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) {
if !ty.is_same(&impl_.for_, cx.cache()) {
// Two different types might have the same did,
// without actually being the same.
continue;
@ -1327,7 +1327,7 @@ fn notable_traits_decl(ty: &clean::Type, cx: &Context<'_>) -> (String, String) {
for i in impls {
let impl_ = i.inner_impl();
if !impl_.for_.without_borrowed_ref().is_same(ty.without_borrowed_ref(), cx.cache()) {
if !ty.is_same(&impl_.for_, cx.cache()) {
// Two different types might have the same did,
// without actually being the same.
continue;

View File

@ -18,3 +18,9 @@ pub fn bare_fn_matches() -> &'static [SomeStruct] {
pub fn bare_fn_no_matches() -> &'static [OtherStruct] {
&[]
}
// @has doc_notable_trait_slice/fn.bare_fn_mut_no_matches.html
// @count - '//script[@id="notable-traits-data"]' 0
pub fn bare_fn_mut_no_matches() -> &'static mut [SomeStruct] {
&mut []
}

View File

@ -0,0 +1,35 @@
#![feature(doc_notable_trait)]
// Notable traits SHOULD be shown when the `impl` has a generic type and the
// return type has a concrete type.
pub mod generic_return {
pub struct Wrapper<T>(T);
#[doc(notable_trait)]
pub trait NotableTrait {}
impl NotableTrait for Wrapper<u8> {}
// @has notable_trait_generics/generic_return/fn.returning.html
// @!has - '//a[@class="tooltip"]/@data-notable-ty' 'Wrapper<T>'
pub fn returning<T>() -> Wrapper<T> {
loop {}
}
}
// Notable traits SHOULD NOT be shown when the `impl` has a concrete type and
// the return type has a generic type.
pub mod generic_impl {
pub struct Wrapper<T>(T);
#[doc(notable_trait)]
pub trait NotableTrait {}
impl<T> NotableTrait for Wrapper<T> {}
// @has notable_trait_generics/generic_impl/fn.returning.html
// @has - '//a[@class="tooltip"]/@data-notable-ty' 'Wrapper<u8>'
pub fn returning() -> Wrapper<u8> {
loop {}
}
}