Skip to content

Commit

Permalink
rustdoc-search: simplify rules for generics and type params
Browse files Browse the repository at this point in the history
This commit is a response to feedback on the displayed type
signatures results, by making type params looser and generics
act stricter.

Type params are loosened by allowing a single function type param
to match multiple query params. In other words, the relationship
is loosened to N:1 instead of the older 1:1 rule. This change also
allows a type param to be unboxed in one spot and matched in another.

Generics are tightened by making order significant. This means
`Vec<Allocator>` now matches only with a true vector of allocators,
instead of matching the second type param. It also makes unboxing
within generics stricter, so `Result<A, B>` only matches if `B`
is in the error type and `A` is in the success type. The top level
of the function search is unaffected.

Find the discussion on:

* <https://rust-lang.zulipchat.com/#narrow/stream/393423-t-rustdoc.2Fmeetings/topic/meeting.202024-07-08/near/449965149>
* <rust-lang#124544 (comment)>
  • Loading branch information
notriddle committed Jul 25, 2024
1 parent f8b3e1a commit 759df53
Show file tree
Hide file tree
Showing 12 changed files with 287 additions and 131 deletions.
12 changes: 7 additions & 5 deletions src/doc/rustdoc/src/read-documentation/search.md
Original file line number Diff line number Diff line change
Expand Up @@ -130,29 +130,31 @@ pub trait MyTrait {
/// This function can be found using the following search queries:
///
/// MyTrait<First=u8, Second=u32> -> bool
/// MyTrait<u32, First=u8> -> bool
/// MyTrait<Second=u32> -> bool
/// MyTrait<u32, u8> -> bool
///
/// The following queries, however, will *not* match it:
///
/// MyTrait<First=u32> -> bool
/// MyTrait<u32, u32> -> bool
/// MyTrait<u32, First=u8> -> bool
/// MyTrait<u32, u8> -> bool
pub fn my_fn(x: impl MyTrait<First=u8, Second=u32>) -> bool { true }
```

Generics and function parameters are order-agnostic, but sensitive to nesting
Function parameters are order-agnostic, but sensitive to nesting
and number of matches. For example, a function with the signature
`fn read_all(&mut self: impl Read) -> Result<Vec<u8>, Error>`
will match these queries:

* `&mut Read -> Result<Vec<u8>, Error>`
* `Read -> Result<Vec<u8>, Error>`
* `Read -> Result<Error, Vec>`
* `Read -> Result<Vec<u8>>`
* `Read -> u8`

But it *does not* match `Result<Vec, u8>` or `Result<u8<Vec>>`.
But it *does not* match `Result<Vec, u8>` or `Result<u8<Vec>>`,
because those are nested incorrectly, and it does not match
`Result<Error, Vec<u8>>` or `Result<Error>`, because those are
in the wrong order.

To search for a function that accepts a function as a parameter,
like `Iterator::all`, wrap the nested signature in parenthesis,
Expand Down
290 changes: 226 additions & 64 deletions src/librustdoc/html/static/js/search.js

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions tests/rustdoc-gui/search-about-this-result.goml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ assert-count: ("#search-tabs button", 1)
assert-count: (".search-results > a", 1)

assert: "//div[@class='type-signature']/strong[text()='Iterator']"
assert: "//div[@class='type-signature']/strong[text()='(']"
assert: "//div[@class='type-signature']/strong[text()=')']"
assert: "//div[@class='type-signature']/strong[text()='(B']"
assert: "//div[@class='type-signature']/strong[text()='B)']"

assert: "//div[@class='type-signature']/div[@class='where']/strong[text()='FnMut']"
assert: "//div[@class='type-signature']/div[@class='where']/strong[text()='Iterator::Item']"
Expand Down
21 changes: 8 additions & 13 deletions tests/rustdoc-js-std/option-type-signatures.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,6 @@ const EXPECTED = [
'name': 'and',
'displayType': '`Option`<`T`>, `Option`<`U`> -> `Option`<`U`>',
},
{
'path': 'std::option::Option',
'name': 'zip',
'displayType': '`Option`<`T`>, `Option`<`U`> -> `Option`<(T, `U`)>',
},
],
},
{
Expand All @@ -103,12 +98,12 @@ const EXPECTED = [
],
},
{
'query': 'option<t>, option<u> -> option<t, u>',
'query': 'option<t>, option<u> -> option<(t, u)>',
'others': [
{
'path': 'std::option::Option',
'name': 'zip',
'displayType': '`Option`<`T`>, `Option`<`U`> -> `Option`<(`T`, `U`)>',
'displayType': '`Option`<`T`>, `Option`<`U`> -> `Option`<`(T`, `U)`>',
},
],
},
Expand Down Expand Up @@ -174,35 +169,35 @@ const EXPECTED = [
'path': 'std::option::Option',
'name': 'map',
'displayType': '`Option`<`T`>, F -> `Option`<U>',
'displayMappedNames': `T = t, U = u`,
'displayMappedNames': `t = T, u = U`,
'displayWhereClause': "F: `FnOnce` (T) -> `U`",
},
{
'path': 'std::option::Option',
'name': 'and_then',
'displayType': '`Option`<`T`>, F -> `Option`<U>',
'displayMappedNames': `T = t, U = u`,
'displayMappedNames': `t = T, u = U`,
'displayWhereClause': "F: `FnOnce` (T) -> Option<`U`>",
},
{
'path': 'std::option::Option',
'name': 'zip_with',
'displayType': 'Option<T>, `Option`<`U`>, F -> `Option`<R>',
'displayMappedNames': `U = t, R = u`,
'displayMappedNames': `t = U, u = R`,
'displayWhereClause': "F: `FnOnce` (T, U) -> `R`",
},
{
'path': 'std::task::Poll',
'name': 'map_ok',
'displayType': 'Poll<`Option`<Result<`T`, E>>>, F -> Poll<`Option`<Result<U, E>>>',
'displayMappedNames': `T = t, U = u`,
'displayMappedNames': `t = T, u = U`,
'displayWhereClause': "F: `FnOnce` (T) -> `U`",
},
{
'path': 'std::task::Poll',
'name': 'map_err',
'displayType': 'Poll<`Option`<Result<`T`, E>>>, F -> Poll<`Option`<Result<T, U>>>',
'displayMappedNames': `T = t, U = u`,
'displayMappedNames': `t = T, u = U`,
'displayWhereClause': "F: `FnOnce` (E) -> `U`",
},
],
Expand All @@ -214,7 +209,7 @@ const EXPECTED = [
'path': 'std::option::Option',
'name': 'and_then',
'displayType': '`Option`<`T`>, F -> `Option`<U>',
'displayMappedNames': `T = t, U = u`,
'displayMappedNames': `t = T, u = U`,
'displayWhereClause': "F: `FnOnce` (T) -> `Option`<`U`>",
},
],
Expand Down
4 changes: 2 additions & 2 deletions tests/rustdoc-js-std/vec-type-signatures.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ const EXPECTED = [
],
},
{
'query': 'vec<Allocator> -> Box<[T]>',
'query': 'vec<T, Allocator> -> Box<[T]>',
'others': [
{
'path': 'std::boxed::Box',
'name': 'from',
'displayType': '`Vec`<T, `A`> -> `Box`<`[T]`, A>',
'displayType': '`Vec`<`T`, `A`> -> `Box`<`[T]`, A>',
'displayMappedNames': `T = T`,
'displayWhereClause': 'A: `Allocator`',
},
Expand Down
38 changes: 25 additions & 13 deletions tests/rustdoc-js/assoc-type-backtrack.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,19 +89,21 @@ const EXPECTED = [
],
},
{
'query': 'myintofuture<myfuture<t>> -> myfuture<t>',
'query': 'myintofuture<t, myfuture<t>> -> myfuture<t>',
'correction': null,
'others': [
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future' },
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future_2' },
],
},
// Invalid unboxing of the one-argument case.
// If you unbox one of the myfutures, you need to unbox both of them.
// Unboxings of the one-argument case.
{
'query': 'myintofuture<fut=t> -> myfuture<t>',
'correction': null,
'others': [],
'others': [
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future' },
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future_2' },
],
},
// Unboxings of the two-argument case.
{
Expand All @@ -119,7 +121,7 @@ const EXPECTED = [
],
},
{
'query': 'myintofuture<myfuture>, myintofuture<myfuture> -> myfuture',
'query': 'myintofuture<t, myfuture>, myintofuture<t, myfuture> -> myfuture',
'correction': null,
'others': [
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future_2' },
Expand All @@ -132,32 +134,42 @@ const EXPECTED = [
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future_2' },
],
},
// Invalid unboxings of the two-argument case.
// If you unbox one of the myfutures, you need to unbox all of them.
// If you unbox one of the myfutures, you don't need to unbox all of them.
{
'query': 'myintofuture<fut=t>, myintofuture<fut=myfuture<t>> -> myfuture<t>',
'correction': null,
'others': [],
'others': [
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future_2' },
],
},
{
'query': 'myintofuture<fut=myfuture<t>>, myintofuture<fut=t> -> myfuture<t>',
'correction': null,
'others': [],
'others': [
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future_2' },
],
},
{
'query': 'myintofuture<fut=myfuture<t>>, myintofuture<fut=myfuture<t>> -> t',
'correction': null,
'others': [],
'others': [
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future_2' },
],
},
// different generics don't match up either
// different generics will match up (didn't used to, but does now)
{
'query': 'myintofuture<fut=myfuture<u>>, myintofuture<fut=myfuture<t>> -> myfuture<t>',
'correction': null,
'others': [],
'others': [
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future_2' },
],
},
{
'query': 'myintofuture<output=t> -> myfuture<tt>',
'correction': null,
'others': [],
'others': [
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future' },
{ 'path': 'assoc_type_backtrack::MyIntoFuture', 'name': 'into_future_2' },
],
},
];
4 changes: 2 additions & 2 deletions tests/rustdoc-js/assoc-type-unbound.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const EXPECTED = [
'path': 'assoc_type_unbound::MyIter',
'name': 'next',
'displayType': '&mut `MyIter` -> `Option`<`MyIter::Item`>',
'displayMappedNames': 'MyIter::Item = T',
'displayMappedNames': 'T = MyIter::Item',
'displayWhereClause': '',
},
],
Expand All @@ -26,7 +26,7 @@ const EXPECTED = [
'path': 'assoc_type_unbound::MyIter',
'name': 'next',
'displayType': '&mut `MyIter` -> `Option`<`MyIter::Item`>',
'displayMappedNames': 'MyIter::Item = T',
'displayMappedNames': 'T = MyIter::Item',
'displayWhereClause': '',
},
],
Expand Down
12 changes: 4 additions & 8 deletions tests/rustdoc-js/generics-match-ambiguity.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,14 @@ const EXPECTED = [
],
},
{
// strict generics matching; W2<i32, u32> doesn't match W2<W3<i32, u32>>,
// even though W2<i32> works just fine (ignoring the W3)
'query': 'W2<i32>, W2<i32, u32>',
'others': [
{ 'path': 'generics_match_ambiguity', 'name': 'baag' },
{ 'path': 'generics_match_ambiguity', 'name': 'baah' },
],
'others': [],
},
{
'query': 'W2<i32, u32>, W2<i32>',
'others': [
{ 'path': 'generics_match_ambiguity', 'name': 'baag' },
{ 'path': 'generics_match_ambiguity', 'name': 'baah' },
],
'others': [],
},
{
'query': 'W2<i32>, W3<i32, u32>',
Expand Down
5 changes: 2 additions & 3 deletions tests/rustdoc-js/generics-nested.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,8 @@ const EXPECTED = [
],
},
{
// can't put generics out of order
'query': '-> Out<Second, First>',
'others': [
{ 'path': 'generics_nested', 'name': 'bet' },
],
'others': [],
},
];
4 changes: 0 additions & 4 deletions tests/rustdoc-js/generics-unbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,28 +11,24 @@ const EXPECTED = [
'query': 'Inside<T> -> Out3<T>',
'others': [
{ 'path': 'generics_unbox', 'name': 'beta' },
{ 'path': 'generics_unbox', 'name': 'gamma' },
],
},
{
'query': 'Inside<T> -> Out4<T>',
'others': [
{ 'path': 'generics_unbox', 'name': 'beta' },
{ 'path': 'generics_unbox', 'name': 'gamma' },
],
},
{
'query': 'Inside<T> -> Out3<U, T>',
'others': [
{ 'path': 'generics_unbox', 'name': 'beta' },
{ 'path': 'generics_unbox', 'name': 'gamma' },
],
},
{
'query': 'Inside<T> -> Out4<U, T>',
'others': [
{ 'path': 'generics_unbox', 'name': 'beta' },
{ 'path': 'generics_unbox', 'name': 'gamma' },
],
},
];
9 changes: 3 additions & 6 deletions tests/rustdoc-js/nested-unboxed.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,8 @@ const EXPECTED = [
},
{
'query': '-> Result<i32, u32, bool>',
'others': [
{ 'path': 'nested_unboxed', 'name': 'something' },
],
// can't put nested generics out of order
'others': [],
},
{
'query': '-> Result<Object<i32>, bool>',
Expand All @@ -45,9 +44,7 @@ const EXPECTED = [
},
{
'query': '-> Result<Object<u32>, bool>',
'others': [
{ 'path': 'nested_unboxed', 'name': 'something' },
],
'others': [],
},
{
'query': '-> Result<Object<i32>, u32, bool>',
Expand Down
15 changes: 6 additions & 9 deletions tests/rustdoc-js/reference.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ const EXPECTED = [
},
{
'query': 'reference<ring>, reference<ring> -> ()',
'others': [
{ 'path': 'reference::Ring', 'name': 'wear' },
],
// can't leave out the `mut`, because can't reorder like that
'others': [],
},
{
'query': 'reference<mut, ring>, reference<ring> -> ()',
Expand All @@ -102,9 +101,8 @@ const EXPECTED = [
},
{
'query': 'reference<middle>, reference<middle> -> ()',
'others': [
{ 'path': 'reference', 'name': 'show' },
],
// can't leave out the mut
'others': [],
},
{
'query': 'reference<mut, middle>, reference<mut, middle> -> ()',
Expand Down Expand Up @@ -203,9 +201,8 @@ const EXPECTED = [
// middle with shorthand
{
'query': '&middle, &middle -> ()',
'others': [
{ 'path': 'reference', 'name': 'show' },
],
// can't leave out the mut
'others': [],
},
{
'query': '&mut middle, &mut middle -> ()',
Expand Down

0 comments on commit 759df53

Please sign in to comment.