Auto merge of #115436 - GuillaumeGomez:fix-type-based-search, r=notriddle

[rustdoc] Fix type based search

Fixes #114522.

The problem was a bit more tricky than I originally thought it would be: we only kept type ID and generics in short, but as soon as there was a full path in the user query, the element didn't get an ID anymore because the ID map didn't know about `x::y` (although it knew about `y`). So for this first problem, I instead always pass the element name to get the ID.

Then a new problem occurred: we actually needed to check if paths matched, otherwise whatever the path, as long as the "end types" match, it's all good. meaning, we needed to add path information, but to do so, we needed it to be added into the search index directly as there was no mapping between `"p"` and `"q"`.

I hope this explanation makes sense to someone else than me. ^^'

r? `@notriddle`

Author: bors

src/librustdoc/html/render/search_index.rs

@@ -1,10 +1,10 @@
 use std::collections::hash_map::Entry;
 use std::collections::BTreeMap;
 
-use rustc_data_structures::fx::FxHashMap;
+use rustc_data_structures::fx::{FxHashMap, FxIndexMap};
 use rustc_middle::ty::TyCtxt;
 use rustc_span::symbol::Symbol;
-use serde::ser::{Serialize, SerializeStruct, Serializer};
+use serde::ser::{Serialize, SerializeSeq, SerializeStruct, Serializer};
 
 use crate::clean;
 use crate::clean::types::{Function, Generics, ItemId, Type, WherePredicate};
@@ -78,17 +78,17 @@ pub(crate) fn build_index<'tcx>(
             map: &mut FxHashMap<F, usize>,
             itemid: F,
             lastpathid: &mut usize,
-            crate_paths: &mut Vec<(ItemType, Symbol)>,
+            crate_paths: &mut Vec<(ItemType, Vec<Symbol>)>,
             item_type: ItemType,
-            path: Symbol,
+            path: &[Symbol],
         ) {
             match map.entry(itemid) {
                 Entry::Occupied(entry) => ty.id = Some(RenderTypeId::Index(*entry.get())),
                 Entry::Vacant(entry) => {
                     let pathid = *lastpathid;
                     entry.insert(pathid);
                     *lastpathid += 1;
-                    crate_paths.push((item_type, path));
+                    crate_paths.push((item_type, path.to_vec()));
                     ty.id = Some(RenderTypeId::Index(pathid));
                 }
             }
@@ -100,7 +100,7 @@ pub(crate) fn build_index<'tcx>(
             itemid_to_pathid: &mut FxHashMap<ItemId, usize>,
             primitives: &mut FxHashMap<Symbol, usize>,
             lastpathid: &mut usize,
-            crate_paths: &mut Vec<(ItemType, Symbol)>,
+            crate_paths: &mut Vec<(ItemType, Vec<Symbol>)>,
         ) {
             if let Some(generics) = &mut ty.generics {
                 for item in generics {
@@ -131,7 +131,7 @@ pub(crate) fn build_index<'tcx>(
                             lastpathid,
                             crate_paths,
                             item_type,
-                            *fqp.last().unwrap(),
+                            fqp,
                         );
                     } else {
                         ty.id = None;
@@ -146,7 +146,7 @@ pub(crate) fn build_index<'tcx>(
                         lastpathid,
                         crate_paths,
                         ItemType::Primitive,
-                        sym,
+                        &[sym],
                     );
                 }
                 RenderTypeId::Index(_) => {}
@@ -191,7 +191,7 @@ pub(crate) fn build_index<'tcx>(
                         lastpathid += 1;
 
                         if let Some(&(ref fqp, short)) = paths.get(&defid) {
-                            crate_paths.push((short, *fqp.last().unwrap()));
+                            crate_paths.push((short, fqp.clone()));
                             Some(pathid)
                         } else {
                             None
@@ -213,118 +213,163 @@ pub(crate) fn build_index<'tcx>(
     struct CrateData<'a> {
         doc: String,
         items: Vec<&'a IndexItem>,
-        paths: Vec<(ItemType, Symbol)>,
+        paths: Vec<(ItemType, Vec<Symbol>)>,
         // The String is alias name and the vec is the list of the elements with this alias.
         //
         // To be noted: the `usize` elements are indexes to `items`.
         aliases: &'a BTreeMap<String, Vec<usize>>,
     }
 
+    struct Paths {
+        ty: ItemType,
+        name: Symbol,
+        path: Option<usize>,
+    }
+
+    impl Serialize for Paths {
+        fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+        where
+            S: Serializer,
+        {
+            let mut seq = serializer.serialize_seq(None)?;
+            seq.serialize_element(&self.ty)?;
+            seq.serialize_element(self.name.as_str())?;
+            if let Some(ref path) = self.path {
+                seq.serialize_element(path)?;
+            }
+            seq.end()
+        }
+    }
+
     impl<'a> Serialize for CrateData<'a> {
         fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
         where
             S: Serializer,
         {
+            let mut extra_paths = FxHashMap::default();
+            // We need to keep the order of insertion, hence why we use an `IndexMap`. Then we will
+            // insert these "extra paths" (which are paths of items from external crates) into the
+            // `full_paths` list at the end.
+            let mut revert_extra_paths = FxIndexMap::default();
+            let mut mod_paths = FxHashMap::default();
+            for (index, item) in self.items.iter().enumerate() {
+                if item.path.is_empty() {
+                    continue;
+                }
+                mod_paths.insert(&item.path, index);
+            }
+            let mut paths = Vec::with_capacity(self.paths.len());
+            for (ty, path) in &self.paths {
+                if path.len() < 2 {
+                    paths.push(Paths { ty: *ty, name: path[0], path: None });
+                    continue;
+                }
+                let full_path = join_with_double_colon(&path[..path.len() - 1]);
+                if let Some(index) = mod_paths.get(&full_path) {
+                    paths.push(Paths { ty: *ty, name: *path.last().unwrap(), path: Some(*index) });
+                    continue;
+                }
+                // It means it comes from an external crate so the item and its path will be
+                // stored into another array.
+                //
+                // `index` is put after the last `mod_paths`
+                let index = extra_paths.len() + self.items.len();
+                if !revert_extra_paths.contains_key(&index) {
+                    revert_extra_paths.insert(index, full_path.clone());
+                }
+                match extra_paths.entry(full_path) {
+                    Entry::Occupied(entry) => {
+                        paths.push(Paths {
+                            ty: *ty,
+                            name: *path.last().unwrap(),
+                            path: Some(*entry.get()),
+                        });
+                    }
+                    Entry::Vacant(entry) => {
+                        entry.insert(index);
+                        paths.push(Paths {
+                            ty: *ty,
+                            name: *path.last().unwrap(),
+                            path: Some(index),
+                        });
+                    }
+                }
+            }
+
+            let mut names = Vec::with_capacity(self.items.len());
+            let mut types = String::with_capacity(self.items.len());
+            let mut full_paths = Vec::with_capacity(self.items.len());
+            let mut descriptions = Vec::with_capacity(self.items.len());
+            let mut parents = Vec::with_capacity(self.items.len());
+            let mut functions = Vec::with_capacity(self.items.len());
+            let mut deprecated = Vec::with_capacity(self.items.len());
+
+            for (index, item) in self.items.iter().enumerate() {
+                let n = item.ty as u8;
+                let c = char::try_from(n + b'A').expect("item types must fit in ASCII");
+                assert!(c <= 'z', "item types must fit within ASCII printables");
+                types.push(c);
+
+                assert_eq!(
+                    item.parent.is_some(),
+                    item.parent_idx.is_some(),
+                    "`{}` is missing idx",
+                    item.name
+                );
+                // 0 is a sentinel, everything else is one-indexed
+                parents.push(item.parent_idx.map(|x| x + 1).unwrap_or(0));
+
+                names.push(item.name.as_str());
+                descriptions.push(&item.desc);
+
+                if !item.path.is_empty() {
+                    full_paths.push((index, &item.path));
+                }
+
+                // Fake option to get `0` out as a sentinel instead of `null`.
+                // We want to use `0` because it's three less bytes.
+                enum FunctionOption<'a> {
+                    Function(&'a IndexItemFunctionType),
+                    None,
+                }
+                impl<'a> Serialize for FunctionOption<'a> {
+                    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+                    where
+                        S: Serializer,
+                    {
+                        match self {
+                            FunctionOption::None => 0.serialize(serializer),
+                            FunctionOption::Function(ty) => ty.serialize(serializer),
+                        }
+                    }
+                }
+                functions.push(match &item.search_type {
+                    Some(ty) => FunctionOption::Function(ty),
+                    None => FunctionOption::None,
+                });
+
+                if item.deprecation.is_some() {
+                    deprecated.push(index);
+                }
+            }
+
+            for (index, path) in &revert_extra_paths {
+                full_paths.push((*index, path));
+            }
+
             let has_aliases = !self.aliases.is_empty();
             let mut crate_data =
                 serializer.serialize_struct("CrateData", if has_aliases { 9 } else { 8 })?;
             crate_data.serialize_field("doc", &self.doc)?;
-            crate_data.serialize_field(
-                "t",
-                &self
-                    .items
-                    .iter()
-                    .map(|item| {
-                        let n = item.ty as u8;
-                        let c = char::try_from(n + b'A').expect("item types must fit in ASCII");
-                        assert!(c <= 'z', "item types must fit within ASCII printables");
-                        c
-                    })
-                    .collect::<String>(),
-            )?;
-            crate_data.serialize_field(
-                "n",
-                &self.items.iter().map(|item| item.name.as_str()).collect::<Vec<_>>(),
-            )?;
-            crate_data.serialize_field(
-                "q",
-                &self
-                    .items
-                    .iter()
-                    .enumerate()
-                    // Serialize as an array of item indices and full paths
-                    .filter_map(
-                        |(index, item)| {
-                            if item.path.is_empty() { None } else { Some((index, &item.path)) }
-                        },
-                    )
-                    .collect::<Vec<_>>(),
-            )?;
-            crate_data.serialize_field(
-                "d",
-                &self.items.iter().map(|item| &item.desc).collect::<Vec<_>>(),
-            )?;
-            crate_data.serialize_field(
-                "i",
-                &self
-                    .items
-                    .iter()
-                    .map(|item| {
-                        assert_eq!(
-                            item.parent.is_some(),
-                            item.parent_idx.is_some(),
-                            "`{}` is missing idx",
-                            item.name
-                        );
-                        // 0 is a sentinel, everything else is one-indexed
-                        item.parent_idx.map(|x| x + 1).unwrap_or(0)
-                    })
-                    .collect::<Vec<_>>(),
-            )?;
-            crate_data.serialize_field(
-                "f",
-                &self
-                    .items
-                    .iter()
-                    .map(|item| {
-                        // Fake option to get `0` out as a sentinel instead of `null`.
-                        // We want to use `0` because it's three less bytes.
-                        enum FunctionOption<'a> {
-                            Function(&'a IndexItemFunctionType),
-                            None,
-                        }
-                        impl<'a> Serialize for FunctionOption<'a> {
-                            fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
-                            where
-                                S: Serializer,
-                            {
-                                match self {
-                                    FunctionOption::None => 0.serialize(serializer),
-                                    FunctionOption::Function(ty) => ty.serialize(serializer),
-                                }
-                            }
-                        }
-                        match &item.search_type {
-                            Some(ty) => FunctionOption::Function(ty),
-                            None => FunctionOption::None,
-                        }
-                    })
-                    .collect::<Vec<_>>(),
-            )?;
-            crate_data.serialize_field(
-                "c",
-                &self
-                    .items
-                    .iter()
-                    .enumerate()
-                    // Serialize as an array of deprecated item indices
-                    .filter_map(|(index, item)| item.deprecation.map(|_| index))
-                    .collect::<Vec<_>>(),
-            )?;
-            crate_data.serialize_field(
-                "p",
-                &self.paths.iter().map(|(it, s)| (it, s.as_str())).collect::<Vec<_>>(),
-            )?;
+            crate_data.serialize_field("t", &types)?;
+            crate_data.serialize_field("n", &names)?;
+            // Serialize as an array of item indices and full paths
+            crate_data.serialize_field("q", &full_paths)?;
+            crate_data.serialize_field("d", &descriptions)?;
+            crate_data.serialize_field("i", &parents)?;
+            crate_data.serialize_field("f", &functions)?;
+            crate_data.serialize_field("c", &deprecated)?;
+            crate_data.serialize_field("p", &paths)?;
             if has_aliases {
                 crate_data.serialize_field("a", &self.aliases)?;
             }

src/librustdoc/html/static/js/search.js

@@ -263,7 +263,6 @@ function initSearch(rawSearchIndex) {
      * @returns {integer}
      */
     function buildTypeMapIndex(name) {
-
         if (name === "" || name === null) {
             return -1;
         }
@@ -1380,7 +1379,7 @@ function initSearch(rawSearchIndex) {
              * @type Map<integer, QueryElement[]>
              */
             const queryElemSet = new Map();
-            const addQueryElemToQueryElemSet = function addQueryElemToQueryElemSet(queryElem) {
+            const addQueryElemToQueryElemSet = queryElem => {
                 let currentQueryElemList;
                 if (queryElemSet.has(queryElem.id)) {
                     currentQueryElemList = queryElemSet.get(queryElem.id);
@@ -1397,7 +1396,7 @@ function initSearch(rawSearchIndex) {
              * @type Map<integer, FunctionType[]>
              */
             const fnTypeSet = new Map();
-            const addFnTypeToFnTypeSet = function addFnTypeToFnTypeSet(fnType) {
+            const addFnTypeToFnTypeSet = fnType => {
                 // Pure generic, or an item that's not matched by any query elems.
                 // Try [unboxing] it.
                 //
@@ -1463,6 +1462,32 @@ function initSearch(rawSearchIndex) {
                     if (!typePassesFilter(queryElem.typeFilter, fnType.ty)) {
                         continue;
                     }
+                    const queryElemPathLength = queryElem.pathWithoutLast.length;
+                    // If the query element is a path (it contains `::`), we need to check if this
+                    // path is compatible with the target type.
+                    if (queryElemPathLength > 0) {
+                        const fnTypePath = fnType.path !== undefined && fnType.path !== null ?
+                            fnType.path.split("::") : [];
+                        // If the path provided in the query element is longer than this type,
+                        // no need to check it since it won't match in any case.
+                        if (queryElemPathLength > fnTypePath.length) {
+                            continue;
+                        }
+                        let i = 0;
+                        for (const path of fnTypePath) {
+                            if (path === queryElem.pathWithoutLast[i]) {
+                                i += 1;
+                                if (i >= queryElemPathLength) {
+                                    break;
+                                }
+                            }
+                        }
+                        if (i < queryElemPathLength) {
+                            // If we didn't find all parts of the path of the query element inside
+                            // the fn type, then it's not the right one.
+                            continue;
+                        }
+                    }
                     if (queryElem.generics.length === 0 || checkGenerics(fnType, queryElem)) {
                         currentFnTypeList.splice(i, 1);
                         const result = doHandleQueryElemList(currentFnTypeList, queryElemList);
@@ -1863,14 +1888,14 @@ function initSearch(rawSearchIndex) {
              * @param {QueryElement} elem
              */
             function convertNameToId(elem) {
-                if (typeNameIdMap.has(elem.name)) {
-                    elem.id = typeNameIdMap.get(elem.name);
+                if (typeNameIdMap.has(elem.pathLast)) {
+                    elem.id = typeNameIdMap.get(elem.pathLast);
                 } else if (!parsedQuery.literalSearch) {
                     let match = -1;
                     let matchDist = maxEditDistance + 1;
                     let matchName = "";
                     for (const [name, id] of typeNameIdMap) {
-                        const dist = editDistance(name, elem.name, maxEditDistance);
+                        const dist = editDistance(name, elem.pathLast, maxEditDistance);
                         if (dist <= matchDist && dist <= maxEditDistance) {
                             if (dist === matchDist && matchName > name) {
                                 continue;
@@ -2385,12 +2410,20 @@ ${item.displayPath}<span class="${type}">${name}</span>\
                     lowercasePaths
                 );
             }
+            // `0` is used as a sentinel because it's fewer bytes than `null`
+            if (pathIndex === 0) {
+                return {
+                    id: -1,
+                    ty: null,
+                    path: null,
+                    generics: generics,
+                };
+            }
+            const item = lowercasePaths[pathIndex - 1];
             return {
-                // `0` is used as a sentinel because it's fewer bytes than `null`
-                id: pathIndex === 0
-                    ? -1
-                    : buildTypeMapIndex(lowercasePaths[pathIndex - 1].name),
-                ty: pathIndex === 0 ? null : lowercasePaths[pathIndex - 1].ty,
+                id: buildTypeMapIndex(item.name),
+                ty: item.ty,
+                path: item.path,
                 generics: generics,
             };
         });
@@ -2422,13 +2455,22 @@ ${item.displayPath}<span class="${type}">${name}</span>\
         let inputs, output;
         if (typeof functionSearchType[INPUTS_DATA] === "number") {
             const pathIndex = functionSearchType[INPUTS_DATA];
-            inputs = [{
-                id: pathIndex === 0
-                    ? -1
-                    : buildTypeMapIndex(lowercasePaths[pathIndex - 1].name),
-                ty: pathIndex === 0 ? null : lowercasePaths[pathIndex - 1].ty,
-                generics: [],
-            }];
+            if (pathIndex === 0) {
+                inputs = [{
+                    id: -1,
+                    ty: null,
+                    path: null,
+                    generics: [],
+                }];
+            } else {
+                const item = lowercasePaths[pathIndex - 1];
+                inputs = [{
+                    id: buildTypeMapIndex(item.name),
+                    ty: item.ty,
+                    path: item.path,
+                    generics: [],
+                }];
+            }
         } else {
             inputs = buildItemSearchTypeAll(
                 functionSearchType[INPUTS_DATA],
@@ -2438,13 +2480,22 @@ ${item.displayPath}<span class="${type}">${name}</span>\
         if (functionSearchType.length > 1) {
             if (typeof functionSearchType[OUTPUT_DATA] === "number") {
                 const pathIndex = functionSearchType[OUTPUT_DATA];
-                output = [{
-                    id: pathIndex === 0
-                        ? -1
-                        : buildTypeMapIndex(lowercasePaths[pathIndex - 1].name),
-                    ty: pathIndex === 0 ? null : lowercasePaths[pathIndex - 1].ty,
-                    generics: [],
-                }];
+                if (pathIndex === 0) {
+                    output = [{
+                        id: -1,
+                        ty: null,
+                        path: null,
+                        generics: [],
+                    }];
+                } else {
+                    const item = lowercasePaths[pathIndex - 1];
+                    output = [{
+                        id: buildTypeMapIndex(item.name),
+                        ty: item.ty,
+                        path: item.path,
+                        generics: [],
+                    }];
+                }
             } else {
                 output = buildItemSearchTypeAll(
                     functionSearchType[OUTPUT_DATA],
@@ -2577,9 +2628,19 @@ ${item.displayPath}<span class="${type}">${name}</span>\
             // convert `rawPaths` entries into object form
             // generate normalizedPaths for function search mode
             let len = paths.length;
+            let lastPath = itemPaths.get(0);
             for (let i = 0; i < len; ++i) {
-                lowercasePaths.push({ty: paths[i][0], name: paths[i][1].toLowerCase()});
-                paths[i] = {ty: paths[i][0], name: paths[i][1]};
+                const elem = paths[i];
+                const ty = elem[0];
+                const name = elem[1];
+                let path = null;
+                if (elem.length > 2) {
+                    path = itemPaths.has(elem[2]) ? itemPaths.get(elem[2]) : lastPath;
+                    lastPath = path;
+                }
+
+                lowercasePaths.push({ty: ty, name: name.toLowerCase(), path: path});
+                paths[i] = {ty: ty, name: name, path: path};
             }
 
             // convert `item*` into an object form, and construct word indices.
@@ -2589,8 +2650,8 @@ ${item.displayPath}<span class="${type}">${name}</span>\
             // operation that is cached for the life of the page state so that
             // all other search operations have access to this cached data for
             // faster analysis operations
+            lastPath = "";
             len = itemTypes.length;
-            let lastPath = "";
             for (let i = 0; i < len; ++i) {
                 let word = "";
                 // This object should have exactly the same set of fields as the "crateRow"
@@ -2599,11 +2660,12 @@ ${item.displayPath}<span class="${type}">${name}</span>\
                     word = itemNames[i].toLowerCase();
                 }
                 searchWords.push(word);
+                const path = itemPaths.has(i) ? itemPaths.get(i) : lastPath;
                 const row = {
                     crate: crate,
                     ty: itemTypes.charCodeAt(i) - charA,
                     name: itemNames[i],
-                    path: itemPaths.has(i) ? itemPaths.get(i) : lastPath,
+                    path: path,
                     desc: itemDescs[i],
                     parent: itemParentIdxs[i] > 0 ? paths[itemParentIdxs[i] - 1] : undefined,
                     type: buildFunctionSearchType(

tests/rustdoc-js-std/full-path-function.js

@@ -0,0 +1,7 @@
+const EXPECTED = {
+    'query': 'vec::vec -> usize',
+    'others': [
+        { 'path': 'std::vec::Vec', 'name': 'len' },
+        { 'path': 'std::vec::Vec', 'name': 'capacity' },
+    ],
+};

tests/rustdoc-js/full-path-function.js

@@ -0,0 +1,43 @@
+// exact-check
+
+const EXPECTED = [
+    {
+        'query': 'sac -> usize',
+        'others': [
+            { 'path': 'full_path_function::b::Sac', 'name': 'bar' },
+            { 'path': 'full_path_function::b::Sac', 'name': 'len' },
+            { 'path': 'full_path_function::sac::Sac', 'name': 'len' },
+        ],
+    },
+    {
+        'query': 'b::sac -> usize',
+        'others': [
+            { 'path': 'full_path_function::b::Sac', 'name': 'bar' },
+            { 'path': 'full_path_function::b::Sac', 'name': 'len' },
+        ],
+    },
+    {
+        'query': 'b::sac -> u32',
+        'others': [
+            { 'path': 'full_path_function::b::Sac', 'name': 'bar2' },
+        ],
+    },
+    {
+        'query': 'string::string -> u32',
+        'others': [
+            { 'path': 'full_path_function::b::Sac', 'name': 'string' },
+        ],
+    },
+    {
+        'query': 'alloc::string::string -> u32',
+        'others': [
+            { 'path': 'full_path_function::b::Sac', 'name': 'string' },
+        ],
+    },
+    {
+        'query': 'alloc::string -> u32',
+        'others': [
+            { 'path': 'full_path_function::b::Sac', 'name': 'string' },
+        ],
+    },
+];

tests/rustdoc-js/full-path-function.rs

@@ -0,0 +1,17 @@
+pub mod sac {
+    pub struct Sac;
+
+    impl Sac {
+        pub fn len(&self) -> usize { 0 }
+    }
+}
+
+pub mod b {
+    pub struct Sac;
+    impl Sac {
+        pub fn len(&self) -> usize { 0 }
+        pub fn bar(&self, w: u32) -> usize { 0 }
+        pub fn bar2(&self, w: u32) -> u32 { 0 }
+        pub fn string(w: String) -> u32 { 0 }
+    }
+}