mikros/rust
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

rustdoc-search: add impl disambiguator to duplicate assoc items

Browse Source 
Helps with #90929

This changes the search results, specifically, when there's more than
one impl with an associated item with the same name. For example,
the search queries `simd<i8> -> simd<i8>` and `simd<i64> -> simd<i64>`
don't link to the same function, but most of the functions have the
same names.

This change should probably be FCP-ed, especially since it adds a new
anchor link format for `main.js` to handle, so that URLs like
`struct.Vec.html#impl-AsMut<[T]>-for-Vec<T,+A>/method.as_mut` redirect
to `struct.Vec.html#method.as_mut-2`. It's a strange design, but there
are a few reasons for it:

* I'd like to avoid making the HTML bigger. Obviously, fixing this bug
  is going to add at least a little more data to the search index, but
  adding more HTML penalises viewers for the benefit of searchers.

* Breaking `struct.Vec.html#method.len` would also be a disappointment.

On the other hand:

* The path-style anchors might be less prone to link rot than the numbered
  anchors. It's definitely less likely to have URLs that appear to "work",
  but silently point at the wrong thing.

* This commit arranges the path-style anchor to redirect to the numbered
  anchor. Nothing stops rustdoc from doing the opposite, making path-style
  anchors the default and redirecting the "legacy" numbered ones.
This commit is contained in:
Michael Howell 2023-03-20 16:02:51 -07:00
parent 9683f8a965
commit 3fbfe2bca5
12 changed files with 293 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
src/librustdoc/formats/cache.rs
View File

@ -350,6 +350,11 @@ fn fold_item(&mut self, item: clean::Item) -> Option<clean::Item> {
desc,
parent,
parent_idx: None,
impl_id: if let Some(ParentStackItem::Impl { item_id, .. }) = self.cache.parent_stack.last() {
item_id.as_def_id()
} else {
None
},
search_type: get_function_type_for_search(
&item,
self.tcx,
@ -369,6 +374,13 @@ fn fold_item(&mut self, item: clean::Item) -> Option<clean::Item> {
parent,
item: item.clone(),
impl_generics,
impl_id: if let Some(ParentStackItem::Impl { item_id, .. }) =
self.cache.parent_stack.last()
{
item_id.as_def_id()
} else {
None
},
});
}
_ => {}
@ -539,6 +551,7 @@ fn fold_item(&mut self, item: clean::Item) -> Option<clean::Item> {
pub(crate) struct OrphanImplItem {
pub(crate) parent: DefId,
pub(crate) impl_id: Option<DefId>,
pub(crate) item: clean::Item,
pub(crate) impl_generics: Option<(clean::Type, clean::Generics)>,
}

41
src/librustdoc/html/render/mod.rs
View File

@ -54,7 +54,7 @@
use rustc_hir::def_id::{DefId, DefIdSet};
use rustc_hir::Mutability;
use rustc_middle::middle::stability;
use rustc_middle::ty::TyCtxt;
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::{
symbol::{sym, Symbol},
BytePos, FileName, RealFileName,
@ -102,6 +102,7 @@ pub(crate) struct IndexItem {
pub(crate) desc: String,
pub(crate) parent: Option<DefId>,
pub(crate) parent_idx: Option<isize>,
pub(crate) impl_id: Option<DefId>,
pub(crate) search_type: Option<IndexItemFunctionType>,
pub(crate) aliases: Box<[Symbol]>,
pub(crate) deprecation: Option<Deprecation>,
@ -1877,7 +1878,7 @@ pub(crate) fn render_impl_summary(
aliases: &[String],
) {
let inner_impl = i.inner_impl();
let id = cx.derive_id(get_id_for_impl(&inner_impl.for_, inner_impl.trait_.as_ref(), cx));
let id = cx.derive_id(get_id_for_impl(cx.tcx(), i.impl_item.item_id));
let aliases = if aliases.is_empty() {
String::new()
} else {
@ -1994,21 +1995,35 @@ fn dont_escape(c: u8) -> bool {
}
}
fn get_id_for_impl(for_: &clean::Type, trait_: Option<&clean::Path>, cx: &Context<'_>) -> String {
match trait_ {
Some(t) => small_url_encode(format!("impl-{:#}-for-{:#}", t.print(cx), for_.print(cx))),
None => small_url_encode(format!("impl-{:#}", for_.print(cx))),
}
fn get_id_for_impl<'tcx>(tcx: TyCtxt<'tcx>, impl_id: ItemId) -> String {
use rustc_middle::ty::print::with_forced_trimmed_paths;
let (type_, trait_) = match impl_id {
ItemId::Auto { trait_, for_ } => {
let ty = tcx.type_of(for_).skip_binder();
(ty, Some(ty::TraitRef::new(tcx, trait_, [ty])))
}
ItemId::Blanket { impl_id, .. } | ItemId::DefId(impl_id) => {
match tcx.impl_subject(impl_id).skip_binder() {
ty::ImplSubject::Trait(trait_ref) => {
(trait_ref.args[0].expect_ty(), Some(trait_ref))
}
ty::ImplSubject::Inherent(ty) => (ty, None),
}
}
};
with_forced_trimmed_paths!(small_url_encode(if let Some(trait_) = trait_ {
format!("impl-{trait_}-for-{type_}", trait_ = trait_.print_only_trait_path())
} else {
format!("impl-{type_}")
}))
}
fn extract_for_impl_name(item: &clean::Item, cx: &Context<'_>) -> Option<(String, String)> {
match *item.kind {
clean::ItemKind::ImplItem(ref i) => {
i.trait_.as_ref().map(|trait_| {
// Alternative format produces no URLs,
// so this parameter does nothing.
(format!("{:#}", i.for_.print(cx)), get_id_for_impl(&i.for_, Some(trait_), cx))
})
clean::ItemKind::ImplItem(ref i) if i.trait_.is_some() => {
// Alternative format produces no URLs,
// so this parameter does nothing.
Some((format!("{:#}", i.for_.print(cx)), get_id_for_impl(cx.tcx(), item.item_id)))
}
_ => None,
}

33
src/librustdoc/html/render/search_index.rs
View File

@ -12,7 +12,7 @@
use crate::formats::item_type::ItemType;
use crate::html::format::join_with_double_colon;
use crate::html::markdown::short_markdown_summary;
use crate::html::render::{IndexItem, IndexItemFunctionType, RenderType, RenderTypeId};
use crate::html::render::{self, IndexItem, IndexItemFunctionType, RenderType, RenderTypeId};
/// Builds the search index from the collected metadata
pub(crate) fn build_index<'tcx>(
@ -26,7 +26,8 @@ pub(crate) fn build_index<'tcx>(
// Attach all orphan items to the type's definition if the type
// has since been learned.
for &OrphanImplItem { parent, ref item, ref impl_generics } in &cache.orphan_impl_items {
for &OrphanImplItem { impl_id, parent, ref item, ref impl_generics } in &cache.orphan_impl_items
{
if let Some((fqp, _)) = cache.paths.get(&parent) {
let desc = short_markdown_summary(&item.doc_value(), &item.link_names(cache));
cache.search_index.push(IndexItem {
@ -36,6 +37,7 @@ pub(crate) fn build_index<'tcx>(
desc,
parent: Some(parent),
parent_idx: None,
impl_id,
search_type: get_function_type_for_search(item, tcx, impl_generics.as_ref(), cache),
aliases: item.attrs.get_doc_aliases(),
deprecation: item.deprecation(tcx),
@ -222,6 +224,29 @@ fn convert_render_type(
})
.collect();
// Find associated items that need disambiguators
let mut associated_item_duplicates = FxHashMap::<(isize, ItemType, Symbol), usize>::default();
for &item in &crate_items {
if item.impl_id.is_some() && let Some(parent_idx) = item.parent_idx {
let count = associated_item_duplicates
.entry((parent_idx, item.ty, item.name))
.or_insert(0);
*count += 1;
}
}
let associated_item_disambiguators = crate_items
.iter()
.enumerate()
.filter_map(|(index, item)| {
let impl_id = ItemId::DefId(item.impl_id?);
let parent_idx = item.parent_idx?;
let count = *associated_item_duplicates.get(&(parent_idx, item.ty, item.name))?;
if count > 1 { Some((index, render::get_id_for_impl(tcx, impl_id))) } else { None }
})
.collect::<Vec<_>>();
struct CrateData<'a> {
doc: String,
items: Vec<&'a IndexItem>,
@ -230,6 +255,8 @@ struct CrateData<'a> {
//
// To be noted: the `usize` elements are indexes to `items`.
aliases: &'a BTreeMap<String, Vec<usize>>,
// Used when a type has more than one impl with an associated item with the same name.
associated_item_disambiguators: &'a Vec<(usize, String)>,
}
struct Paths {
@ -382,6 +409,7 @@ fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
crate_data.serialize_field("f", &functions)?;
crate_data.serialize_field("c", &deprecated)?;
crate_data.serialize_field("p", &paths)?;
crate_data.serialize_field("b", &self.associated_item_disambiguators)?;
if has_aliases {
crate_data.serialize_field("a", &self.aliases)?;
}
@ -398,6 +426,7 @@ fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
items: crate_items,
paths: crate_paths,
aliases: &aliases,
associated_item_disambiguators: &associated_item_disambiguators,
})
.expect("failed serde conversion")
// All these `replace` calls are because we have to go through JS string for JSON content.

3
src/librustdoc/html/render/sidebar.rs
View File

@ -503,8 +503,7 @@ fn sidebar_render_assoc_items(
.iter()
.filter_map(|it| {
let trait_ = it.inner_impl().trait_.as_ref()?;
let encoded =
id_map.derive(super::get_id_for_impl(&it.inner_impl().for_, Some(trait_), cx));
let encoded = id_map.derive(super::get_id_for_impl(cx.tcx(), it.impl_item.item_id));
let prefix = match it.inner_impl().polarity {
ty::ImplPolarity::Positive | ty::ImplPolarity::Reservation => "",

24
src/librustdoc/html/static/js/main.js
View File

@ -354,6 +354,30 @@ function preLoadCss(cssUrl) {
expandSection(pageId);
}
}
if (savedHash.startsWith("#impl-")) {
// impl-disambiguated links, used by the search engine
// format: impl-X[-for-Y]/method.WHATEVER
// turn this into method.WHATEVER[-NUMBER]
const splitAt = savedHash.indexOf("/");
if (splitAt !== -1) {
const implId = savedHash.slice(1, splitAt);
const assocId = savedHash.slice(splitAt + 1);
const implElem = document.getElementById(implId);
if (implElem && implElem.parentElement.tagName === "SUMMARY" &&
implElem.parentElement.parentElement.tagName === "DETAILS") {
onEachLazy(implElem.parentElement.parentElement.querySelectorAll(
`[id^="${assocId}"]`),
item => {
const numbered = /([^-]+)-([0-9]+)/.exec(item.id);
if (item.id === assocId || (numbered && numbered[1] === assocId)) {
expandSection(item.id);
window.location = "#" + item.id;
}
}
);
}
}
}
}
function onHashChange(ev) {

19
src/librustdoc/html/static/js/search.js
View File

@ -1752,6 +1752,7 @@ function initSearch(rawSearchIndex) {
type: item.type,
is_alias: true,
deprecated: item.deprecated,
implDisambiguator: item.implDisambiguator,
};
}
@ -2218,7 +2219,7 @@ function initSearch(rawSearchIndex) {
href = ROOT_PATH + name + "/index.html";
} else if (item.parent !== undefined) {
const myparent = item.parent;
let anchor = "#" + type + "." + name;
let anchor = type + "." + name;
const parentType = itemTypes[myparent.ty];
let pageType = parentType;
let pageName = myparent.name;
@ -2232,16 +2233,19 @@ function initSearch(rawSearchIndex) {
const enumName = item.path.substr(enumNameIdx + 2);
path = item.path.substr(0, enumNameIdx);
displayPath = path + "::" + enumName + "::" + myparent.name + "::";
anchor = "#variant." + myparent.name + ".field." + name;
anchor = "variant." + myparent.name + ".field." + name;
pageType = "enum";
pageName = enumName;
} else {
displayPath = path + "::" + myparent.name + "::";
}
if (item.implDisambiguator !== null) {
anchor = item.implDisambiguator + "/" + anchor;
}
href = ROOT_PATH + path.replace(/::/g, "/") +
"/" + pageType +
"." + pageName +
".html" + anchor;
".html#" + anchor;
} else {
displayPath = item.path + "::";
href = ROOT_PATH + item.path.replace(/::/g, "/") +
@ -2727,6 +2731,10 @@ ${item.displayPath}<span class="${type}">${name}</span>\
* Types are also represented as arrays; the first item is an index into the `p`
* array, while the second is a list of types representing any generic parameters.
*
* b[i] contains an item's impl disambiguator. This is only present if an item
* is defined in an impl block and, the impl block's type has more than one associated
* item with the same name.
*
* `a` defines aliases with an Array of pairs: [name, offset], where `offset`
* points into the n/t/d/q/i/f arrays.
*
@ -2746,6 +2754,7 @@ ${item.displayPath}<span class="${type}">${name}</span>\
* i: Array<Number>,
* f: Array<RawFunctionSearchType>,
* p: Array<Object>,
* b: Array<[Number, String]>,
* c: Array<Number>
* }}
*/
@ -2766,6 +2775,7 @@ ${item.displayPath}<span class="${type}">${name}</span>\
id: id,
normalizedName: crate.indexOf("_") === -1 ? crate : crate.replace(/_/g, ""),
deprecated: null,
implDisambiguator: null,
};
id += 1;
searchIndex.push(crateRow);
@ -2789,6 +2799,8 @@ ${item.displayPath}<span class="${type}">${name}</span>\
const itemFunctionSearchTypes = crateCorpus.f;
// an array of (Number) indices for the deprecated items
const deprecatedItems = new Set(crateCorpus.c);
// an array of (Number) indices for the deprecated items
const implDisambiguator = new Map(crateCorpus.b);
// an array of [(Number) item type,
// (String) name]
const paths = crateCorpus.p;
@ -2849,6 +2861,7 @@ ${item.displayPath}<span class="${type}">${name}</span>\
id: id,
normalizedName: word.indexOf("_") === -1 ? word : word.replace(/_/g, ""),
deprecated: deprecatedItems.has(i),
implDisambiguator: implDisambiguator.has(i) ? implDisambiguator.get(i) : null,
};
id += 1;
searchIndex.push(row);

41
tests/rustdoc-gui/search-result-impl-disambiguation.goml Normal file
View File

@ -0,0 +1,41 @@
// ignore-tidy-linelength
// Checks that, if a type has two methods with the same name, they both get
// linked correctly.
goto: "file://" + |DOC_PATH| + "/test_docs/index.html"
// This should link to the inherent impl
write: (".search-input", "ZyxwvutMethodDisambiguation -> bool")
// To be SURE that the search will be run.
press-key: 'Enter'
// Waiting for the search results to appear...
wait-for: "#search-tabs"
// Check the disambiguated link.
assert-count: ("a.result-method", 1)
assert-attribute: ("a.result-method", {
"href": "../test_docs/struct.ZyxwvutMethodDisambiguation.html#impl-ZyxwvutMethodDisambiguation/method.method_impl_disambiguation"
})
click: "a.result-method"
wait-for: "#impl-ZyxwvutMethodDisambiguation"
assert-document-property: ({
"URL": "struct.ZyxwvutMethodDisambiguation.html#method.method_impl_disambiguation"
}, ENDS_WITH)
goto: "file://" + |DOC_PATH| + "/test_docs/index.html"
// This should link to the trait impl
write: (".search-input", "ZyxwvutMethodDisambiguation, usize -> usize")
// To be SURE that the search will be run.
press-key: 'Enter'
// Waiting for the search results to appear...
wait-for: "#search-tabs"
// Check the disambiguated link.
assert-count: ("a.result-method", 1)
assert-attribute: ("a.result-method", {
"href": "../test_docs/struct.ZyxwvutMethodDisambiguation.html#impl-ZyxwvutTrait-for-ZyxwvutMethodDisambiguation/method.method_impl_disambiguation"
})
click: "a.result-method"
wait-for: "#impl-ZyxwvutMethodDisambiguation"
assert-document-property: ({
"URL": "struct.ZyxwvutMethodDisambiguation.html#method.method_impl_disambiguation-1"
}, ENDS_WITH)

18
tests/rustdoc-gui/src/test_docs/lib.rs
View File

@ -529,3 +529,21 @@ pub mod cfgs {
/// Some docs.
pub mod cfgs {}
}
pub struct ZyxwvutMethodDisambiguation;
impl ZyxwvutMethodDisambiguation {
pub fn method_impl_disambiguation(&self) -> bool {
true
}
}
pub trait ZyxwvutTrait {
fn method_impl_disambiguation(&self, x: usize) -> usize;
}
impl ZyxwvutTrait for ZyxwvutMethodDisambiguation {
fn method_impl_disambiguation(&self, x: usize) -> usize {
x
}
}

70
tests/rustdoc-js-std/simd-type-signatures.js Normal file
View File

@ -0,0 +1,70 @@
// exact-check
// ignore-order
// ignore-tidy-linelength
// This test case verifies that the href points at the correct impl
const FILTER_CRATE = "std";
const EXPECTED = [
{
'query': 'simd<i16>, simd<i16> -> simd<i16>',
'others': [
{
'path': 'std::simd::Simd',
'name': 'simd_max',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci16,+LANES%3E/method.simd_max'
},
{
'path': 'std::simd::Simd',
'name': 'simd_min',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci16,+LANES%3E/method.simd_min'
},
{
'path': 'std::simd::Simd',
'name': 'simd_clamp',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci16,+LANES%3E/method.simd_clamp'
},
{
'path': 'std::simd::Simd',
'name': 'saturating_add',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci16,+LANES%3E/method.saturating_add'
},
{
'path': 'std::simd::Simd',
'name': 'saturating_sub',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci16,+LANES%3E/method.saturating_sub'
},
],
},
{
'query': 'simd<i8>, simd<i8> -> simd<i8>',
'others': [
{
'path': 'std::simd::Simd',
'name': 'simd_max',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci8,+LANES%3E/method.simd_max'
},
{
'path': 'std::simd::Simd',
'name': 'simd_min',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci8,+LANES%3E/method.simd_min'
},
{
'path': 'std::simd::Simd',
'name': 'simd_clamp',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdOrd-for-core::simd::Simd%3Ci8,+LANES%3E/method.simd_clamp'
},
{
'path': 'std::simd::Simd',
'name': 'saturating_add',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci8,+LANES%3E/method.saturating_add'
},
{
'path': 'std::simd::Simd',
'name': 'saturating_sub',
'href': '../std/simd/struct.Simd.html#impl-core::simd::SimdInt-for-core::simd::Simd%3Ci8,+LANES%3E/method.saturating_sub'
},
],
},
];

28
tests/rustdoc-js/search-method-disambiguate.js Normal file
View File

@ -0,0 +1,28 @@
// exact-check
// ignore-order
// ignore-tidy-linelength
const FILTER_CRATE = "search_method_disambiguate";
const EXPECTED = [
{
'query': 'MyTy -> bool',
'others': [
{
'path': 'search_method_disambiguate::MyTy',
'name': 'my_method',
'href': '../search_method_disambiguate/struct.MyTy.html#impl-X-for-MyTy%3Cbool%3E/method.my_method'
},
],
},
{
'query': 'MyTy -> u8',
'others': [
{
'path': 'search_method_disambiguate::MyTy',
'name': 'my_method',
'href': '../search_method_disambiguate/struct.MyTy.html#impl-X-for-MyTy%3Cu8%3E/method.my_method'
},
],
}
];

22
tests/rustdoc-js/search-method-disambiguate.rs Normal file
View File

@ -0,0 +1,22 @@
pub trait X {
type InnerType;
fn my_method(&self) -> Self::InnerType;
}
pub struct MyTy<T> {
pub t: T,
}
impl X for MyTy<bool> {
type InnerType = bool;
fn my_method(&self) -> bool {
self.t
}
}
impl X for MyTy<u8> {
type InnerType = u8;
fn my_method(&self) -> u8 {
self.t
}
}

2
tests/rustdoc/issue-32077-type-alias-impls.rs
View File

@ -22,7 +22,7 @@ impl Bar for GenericStruct<u32> {}
// We check that "Aliased type" is also present as a title in the sidebar.
// @has - '//*[@class="sidebar-elems"]//h3/a[@href="#aliased-type"]' 'Aliased type'
// We check that we have the implementation of the type alias itself.
// @has - '//*[@id="impl-TypedefStruct"]/h3' 'impl TypedefStruct'
// @has - '//*[@id="impl-GenericStruct%3Cu8%3E"]/h3' 'impl TypedefStruct'
// @has - '//*[@id="method.on_alias"]/h4' 'pub fn on_alias()'
// @has - '//*[@id="impl-GenericStruct%3CT%3E"]/h3' 'impl<T> GenericStruct<T>'
// @has - '//*[@id="method.on_gen"]/h4' 'pub fn on_gen(arg: T)'