Create real parser for search queries

[GuillaumeGomez]

You can test it here.

This PR adds a real parser for the query engine in rustdoc. The parser is quite simple but it allows to makes query handling much easier. I added a new testsuite to ensure it works as expected and ran fuzzing checks on it for a few hours without problems.

So about the parser: as you can see in the screenshot, it handles recursive generics parsing. It also allows to set which item should use exact matching by adding double-quotes around it (look for exact_search in the screenshot).

Now about the query engine itself: I simplified it a lot thanks to the parsed query. It behaves mostly the same when there is only one argument, but is much more powerful when there are more than one.

When making this change, we also removed the support for multi-query.

PS: A big part of the PR is tests and test-related code. :)

r? @camelid

[rust-highfive]

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez

[GuillaumeGomez]

Ok, seems like I fixed all tidy issues. :)

[jyn514]

I really wish you'd talked this over with the team before adding lots more custom syntax. I was under the impression you were fixing bugs, not adding entirely new features.

[jyn514]

Please do not force-skip this. Make the default condition whether node is installed instead.

[GuillaumeGomez]

👍

[jyn514]

Why does this need special syntax? There's already a "function parameters" tab.

[GuillaumeGomez]

It's not a special syntax or anything. Maybe I badly wrote it but it's simply that you can look at function arguments specifically if you want. Extra explanations:

item

This will look for item pretty much everywhere: in the items' name, in their arguments if they have any and in the returned type (if there is any).

item(arg)

It'll now look for item only in item's name and will look for arg only in arguments.

And so this:

(arg)

Will look for arg only in arguments. This is what I meant here.

[GuillaumeGomez]

I really wish you'd talked this over with the team before adding lots more custom syntax. I was under the impression you were fixing bugs, not adding entirely new features.

Except I didn't add any new syntax in here... All this was already supported before in very hackish ways. Test it on the current docs and you'll see. I just wrote a proper parser over the already existing syntax.

[jyn514]

Except I didn't add any new syntax in here... All this was already supported before in very hackish ways.

... this seems pretty clearly not true? Rustdoc doesn't currently support double-quotes to mean "exact match", nor the tuple (u8, Vec) syntax, nor whatever the macro<f>:foo syntax is doing.

[GuillaumeGomez]

Oh you're right: it doesn't support specifying arguments directly (with ()). Sorry, I was really sure it already did...

However, it supports double quotes, it supports name<generic> and it supports ->.

[camelid]

I have neither the time nor the knowledge of rustdoc's JS to review this, but I can give some feedback on the UI :)

@jyn514 I'm assigning to you for now since you've started reviewing, but feel free to re-assign. r? @jyn514

[GuillaumeGomez]

@camelid Then please tell what you think of the UI. ;)

[camelid]

To allow users to see how the queries are "understood" by the query engine, I added the following:

I really don't think we should be adding this. It exposes way too much of rustdoc's internals, and it'll likely confuse users. Instead, we should try to make rustdoc's search "Just Work" the way users expect.

[camelid]

Also, the search doesn't seem to be working? String -> PathBuf doesn't work.

[GuillaumeGomez]

Also, the search doesn't seem to be working? String -> PathBuf doesn't work.

Indeed... If we keep this approach, I'll check what's wrong.

I really don't think we should be adding this. It exposes way too much of rustdoc's internals, and it'll likely confuse users. Instead, we should try to make rustdoc's search "Just Work" the way users expect.

This the big question: what do users expect? Maybe the approach I took here to allow to have more advanced check isn't the right way. Maybe we should simplify this as follows: generating the items "signature" and then simply run levenshtein on all of them and sort the result. But that approach would have so many downsides as well.

Or did you simply suggest to not display this?

[camelid]

Or did you simply suggest to not display this?

Yes, that's what I suggest :)

[GuillaumeGomez]

Fine by me!

[jsha]

Trying to understand quotes for exact match better. On current nightly, if you search for str, you'll get a lot of results, including substring matches like test::test::TestExecTime::to_string, test::test::TestResult, std::ffi::CStr, and std::ffi::OsStr:

https://doc.rust-lang.org/nightly/settings.html?search=str

If you add quotation marks, you'll only get matches where the last component of the path matches "str" case-insensitively: str (primitive type), alloc::str, core::str, std::str:

https://doc.rust-lang.org/nightly/settings.html?search=%22str%22

That seems like pretty reasonable behavior. Am I right in guessing that we use quotes for this exact match meaning because that's how Google indicates exact match? https://support.google.com/websearch/answer/2466433?hl=en. Google's use of quotes for exact match originates in text search: normally a search for [foo bar] means "return all documents that have foo somewhere and bar somewhere." But a search for ["foo bar"] means "return all documents that have foo immediately before bar". As stemming and other approximate-match searches became more prominent, it became useful for quotes to also mean "turn off stemming (etc)". So a search for ["foo"] means "return all documents that have foo, and foos, fooing, etc don't count."

It's useful to know that history, because our use case for quotes is different: we never have multi-word searches (so far), and we don't do stemming. We do do sub-token matching, which is not something traditional search engines usually do because it's fairly expensive. For instance str can match TestResult.

It looks like this PR intends to allow quotes for exact match in all positions. For instance, I could search for Vec<"&str">. That would find all types that include somewhere in their signature a Vec<&str>, or superstrings of Vec like Moravec<&str>, but not superstrings of str like Vec<&TestResult>.

That seems like overkill, feature-wise. Instead, we should say:

• For simple searches containing only letters, we do substring matching (and levenshtein matching, which we do today).
• However, as soon as the query get any more complicated, like searching for function signatures or including generics, we do exact match on each token.
• For simple searches, substring matching can be overridden by putting a quote in the first position and in the last position.
• Quotes are disallowed in any other position.

By the way, other reviewers: you may notice that searches on the demo link are slower to load than on doc.rust-lang.org. I believe that's because I haven't set up automatic compression on rustdoc.crud.net, so the search JS is larger than it would be in real life. I'll try to fix that later this week.

[GuillaumeGomez]

Updated (with the correct handling of whitespaces for type filter this time...).

[notriddle]

--- rustdoc.json	
+++ rustdoc-search-ref.json	
@@ -3,7 +3,7 @@
   "foundElems": 0,
   "original": "mod: :",
   "returned": [],
-  "typeFilter": 0,
+  "typeFilter": -1,
   "userQuery": "mod: :",
-  "error": null
+  "error": "ERROR"
 }
\ No newline at end of file
Error: "FAIL"
Query: mod: :

[GuillaumeGomez]

And fixed this one as well.

[notriddle]

Well, all four-token checks pass when run against the fuzzer. They seem to be parsing the same language now!

[GuillaumeGomez]

Awesome! Thanks a lot for writing the eBNF fuzzer!

[notriddle]

@bors r+

[bors]

📌 Commit 4d26bde has been approved by notriddle