Skip to content

Commit

Permalink
Auto merge of rust-lang#69402 - GuillaumeGomez:extend-search, r=kinnison
Browse files Browse the repository at this point in the history
Extend search

I realized that when looking for "struct:String" in the rustdoc search for example, the "in arguments" and "returned" tabs were always empty. After some investigation, I realized it was because we only provided the name, and not the type, making it impossible to pass the "type filtering" check.

To resolve this, I added the type alongside the name. Note for the future: we could improve this by instead only registering the path id and use the path dictionary directly. The only problem with that solution (which I already tested) is that it becomes complicated for types in other crates. It'd force us to handle both case with an id and a case with `(name, type)`. I found the current PR big enough to not want to provide it directly. However, I think this is definitely worth it to make it work this way in the future.

About the two tests I added: they don't have much interest except checking that we actually have something returned in the search in the cases of a type filtering with and without literal search.

I also had to update a bit the test script to add the new locally global (haha) variable I created (`NO_TYPE_FILTER`). I added this variable to make the code easier to read than just "-1".

r? @kinnison

cc @ollie27
  • Loading branch information
bors committed Mar 19, 2020
2 parents 2602289 + 9b85213 commit f4c675c
Show file tree
Hide file tree
Showing 10 changed files with 237 additions and 108 deletions.
20 changes: 20 additions & 0 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1078,6 +1078,26 @@ impl Clean<PolyTrait> for hir::PolyTraitRef<'_> {
}
}

impl Clean<TypeKind> for hir::def::DefKind {
fn clean(&self, _: &DocContext<'_>) -> TypeKind {
match *self {
hir::def::DefKind::Mod => TypeKind::Module,
hir::def::DefKind::Struct => TypeKind::Struct,
hir::def::DefKind::Union => TypeKind::Union,
hir::def::DefKind::Enum => TypeKind::Enum,
hir::def::DefKind::Trait => TypeKind::Trait,
hir::def::DefKind::TyAlias => TypeKind::Typedef,
hir::def::DefKind::ForeignTy => TypeKind::Foreign,
hir::def::DefKind::TraitAlias => TypeKind::TraitAlias,
hir::def::DefKind::Fn => TypeKind::Function,
hir::def::DefKind::Const => TypeKind::Const,
hir::def::DefKind::Static => TypeKind::Static,
hir::def::DefKind::Macro(_) => TypeKind::Macro,
_ => TypeKind::Foreign,
}
}
}

impl Clean<Item> for hir::TraitItem<'_> {
fn clean(&self, cx: &DocContext<'_>) -> Item {
let inner = match self.kind {
Expand Down
14 changes: 7 additions & 7 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -836,26 +836,26 @@ pub struct Method {
pub decl: FnDecl,
pub header: hir::FnHeader,
pub defaultness: Option<hir::Defaultness>,
pub all_types: Vec<Type>,
pub ret_types: Vec<Type>,
pub all_types: Vec<(Type, TypeKind)>,
pub ret_types: Vec<(Type, TypeKind)>,
}

#[derive(Clone, Debug)]
pub struct TyMethod {
pub header: hir::FnHeader,
pub decl: FnDecl,
pub generics: Generics,
pub all_types: Vec<Type>,
pub ret_types: Vec<Type>,
pub all_types: Vec<(Type, TypeKind)>,
pub ret_types: Vec<(Type, TypeKind)>,
}

#[derive(Clone, Debug)]
pub struct Function {
pub decl: FnDecl,
pub generics: Generics,
pub header: hir::FnHeader,
pub all_types: Vec<Type>,
pub ret_types: Vec<Type>,
pub all_types: Vec<(Type, TypeKind)>,
pub ret_types: Vec<(Type, TypeKind)>,
}

#[derive(Clone, PartialEq, Eq, Debug, Hash)]
Expand Down Expand Up @@ -1042,7 +1042,7 @@ pub enum PrimitiveType {
Never,
}

#[derive(Clone, Copy, Debug)]
#[derive(Clone, PartialEq, Eq, Hash, Copy, Debug)]
pub enum TypeKind {
Enum,
Function,
Expand Down
40 changes: 31 additions & 9 deletions src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pub fn get_real_types(
arg: &Type,
cx: &DocContext<'_>,
recurse: i32,
) -> FxHashSet<Type> {
) -> FxHashSet<(Type, TypeKind)> {
let arg_s = arg.print().to_string();
let mut res = FxHashSet::default();
if recurse >= 10 {
Expand All @@ -209,7 +209,11 @@ pub fn get_real_types(
if !adds.is_empty() {
res.extend(adds);
} else if !ty.is_full_generic() {
res.insert(ty);
if let Some(did) = ty.def_id() {
if let Some(kind) = cx.tcx.def_kind(did).clean(cx) {
res.insert((ty, kind));
}
}
}
}
}
Expand All @@ -225,22 +229,32 @@ pub fn get_real_types(
if !adds.is_empty() {
res.extend(adds);
} else if !ty.is_full_generic() {
res.insert(ty.clone());
if let Some(did) = ty.def_id() {
if let Some(kind) = cx.tcx.def_kind(did).clean(cx) {
res.insert((ty.clone(), kind));
}
}
}
}
}
}
} else {
res.insert(arg.clone());
if let Some(did) = arg.def_id() {
if let Some(kind) = cx.tcx.def_kind(did).clean(cx) {
res.insert((arg.clone(), kind));
}
}
if let Some(gens) = arg.generics() {
for gen in gens.iter() {
if gen.is_full_generic() {
let adds = get_real_types(generics, gen, cx, recurse + 1);
if !adds.is_empty() {
res.extend(adds);
}
} else {
res.insert(gen.clone());
} else if let Some(did) = gen.def_id() {
if let Some(kind) = cx.tcx.def_kind(did).clean(cx) {
res.insert((gen.clone(), kind));
}
}
}
}
Expand All @@ -256,7 +270,7 @@ pub fn get_all_types(
generics: &Generics,
decl: &FnDecl,
cx: &DocContext<'_>,
) -> (Vec<Type>, Vec<Type>) {
) -> (Vec<(Type, TypeKind)>, Vec<(Type, TypeKind)>) {
let mut all_types = FxHashSet::default();
for arg in decl.inputs.values.iter() {
if arg.type_.is_self_type() {
Expand All @@ -266,15 +280,23 @@ pub fn get_all_types(
if !args.is_empty() {
all_types.extend(args);
} else {
all_types.insert(arg.type_.clone());
if let Some(did) = arg.type_.def_id() {
if let Some(kind) = cx.tcx.def_kind(did).clean(cx) {
all_types.insert((arg.type_.clone(), kind));
}
}
}
}

let ret_types = match decl.output {
FnRetTy::Return(ref return_type) => {
let mut ret = get_real_types(generics, &return_type, cx, 0);
if ret.is_empty() {
ret.insert(return_type.clone());
if let Some(did) = return_type.def_id() {
if let Some(kind) = cx.tcx.def_kind(did).clean(cx) {
ret.insert((return_type.clone(), kind));
}
}
}
ret.into_iter().collect()
}
Expand Down
70 changes: 61 additions & 9 deletions src/librustdoc/html/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ use rustc_span::symbol::{sym, Symbol};
use serde::ser::SerializeSeq;
use serde::{Serialize, Serializer};

use crate::clean::{self, AttributesExt, Deprecation, GetDefId, SelfTy};
use crate::clean::{self, AttributesExt, Deprecation, GetDefId, SelfTy, TypeKind};
use crate::config::{OutputFormat, RenderOptions};
use crate::docfs::{DocFS, ErrorStorage, PathError};
use crate::doctree;
Expand Down Expand Up @@ -302,19 +302,25 @@ impl Serialize for IndexItem {

/// A type used for the search index.
#[derive(Debug)]
struct Type {
struct RenderType {
ty: Option<DefId>,
idx: Option<usize>,
name: Option<String>,
generics: Option<Vec<String>>,
generics: Option<Vec<Generic>>,
}

impl Serialize for Type {
impl Serialize for RenderType {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
if let Some(name) = &self.name {
let mut seq = serializer.serialize_seq(None)?;
seq.serialize_element(&name)?;
if let Some(id) = self.idx {
seq.serialize_element(&id)?;
} else {
seq.serialize_element(&name)?;
}
if let Some(generics) = &self.generics {
seq.serialize_element(&generics)?;
}
Expand All @@ -325,11 +331,32 @@ impl Serialize for Type {
}
}

/// A type used for the search index.
#[derive(Debug)]
struct Generic {
name: String,
defid: Option<DefId>,
idx: Option<usize>,
}

impl Serialize for Generic {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: Serializer,
{
if let Some(id) = self.idx {
serializer.serialize_some(&id)
} else {
serializer.serialize_some(&self.name)
}
}
}

/// Full type of functions/methods in the search index.
#[derive(Debug)]
struct IndexItemFunctionType {
inputs: Vec<Type>,
output: Option<Vec<Type>>,
inputs: Vec<TypeWithKind>,
output: Option<Vec<TypeWithKind>>,
}

impl Serialize for IndexItemFunctionType {
Expand All @@ -340,8 +367,8 @@ impl Serialize for IndexItemFunctionType {
// If we couldn't figure out a type, just write `null`.
let mut iter = self.inputs.iter();
if match self.output {
Some(ref output) => iter.chain(output.iter()).any(|ref i| i.name.is_none()),
None => iter.any(|ref i| i.name.is_none()),
Some(ref output) => iter.chain(output.iter()).any(|ref i| i.ty.name.is_none()),
None => iter.any(|ref i| i.ty.name.is_none()),
} {
serializer.serialize_none()
} else {
Expand All @@ -359,6 +386,31 @@ impl Serialize for IndexItemFunctionType {
}
}

#[derive(Debug)]
pub struct TypeWithKind {
ty: RenderType,
kind: TypeKind,
}

impl From<(RenderType, TypeKind)> for TypeWithKind {
fn from(x: (RenderType, TypeKind)) -> TypeWithKind {
TypeWithKind { ty: x.0, kind: x.1 }
}
}

impl Serialize for TypeWithKind {
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.name)?;
let x: ItemType = self.kind.into();
seq.serialize_element(&x)?;
seq.end()
}
}

thread_local!(static CACHE_KEY: RefCell<Arc<Cache>> = Default::default());
thread_local!(pub static CURRENT_DEPTH: Cell<usize> = Cell::new(0));

Expand Down
43 changes: 28 additions & 15 deletions src/librustdoc/html/render/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::path::{Path, PathBuf};
use serde::Serialize;

use super::{plain_summary_line, shorten, Impl, IndexItem, IndexItemFunctionType, ItemType};
use super::{RenderInfo, Type};
use super::{Generic, RenderInfo, RenderType, TypeWithKind};

/// Indicates where an external crate can be found.
pub enum ExternalLocation {
Expand Down Expand Up @@ -588,17 +588,20 @@ fn build_index(krate: &clean::Crate, cache: &mut Cache) -> String {
let mut lastpathid = 0usize;

for item in search_index {
item.parent_idx = item.parent.map(|defid| {
item.parent_idx = item.parent.and_then(|defid| {
if defid_to_pathid.contains_key(&defid) {
*defid_to_pathid.get(&defid).expect("no pathid")
defid_to_pathid.get(&defid).map(|x| *x)
} else {
let pathid = lastpathid;
defid_to_pathid.insert(defid, pathid);
lastpathid += 1;

let &(ref fqp, short) = paths.get(&defid).unwrap();
crate_paths.push((short, fqp.last().unwrap().clone()));
pathid
if let Some(&(ref fqp, short)) = paths.get(&defid) {
crate_paths.push((short, fqp.last().unwrap().clone()));
Some(pathid)
} else {
None
}
}
});

Expand Down Expand Up @@ -647,20 +650,25 @@ fn get_index_search_type(item: &clean::Item) -> Option<IndexItemFunctionType> {
_ => return None,
};

let inputs =
all_types.iter().map(|arg| get_index_type(&arg)).filter(|a| a.name.is_some()).collect();
let inputs = all_types
.iter()
.map(|(ty, kind)| TypeWithKind::from((get_index_type(&ty), *kind)))
.filter(|a| a.ty.name.is_some())
.collect();
let output = ret_types
.iter()
.map(|arg| get_index_type(&arg))
.filter(|a| a.name.is_some())
.map(|(ty, kind)| TypeWithKind::from((get_index_type(&ty), *kind)))
.filter(|a| a.ty.name.is_some())
.collect::<Vec<_>>();
let output = if output.is_empty() { None } else { Some(output) };

Some(IndexItemFunctionType { inputs, output })
}

fn get_index_type(clean_type: &clean::Type) -> Type {
let t = Type {
fn get_index_type(clean_type: &clean::Type) -> RenderType {
let t = RenderType {
ty: clean_type.def_id(),
idx: None,
name: get_index_type_name(clean_type, true).map(|s| s.to_ascii_lowercase()),
generics: get_generics(clean_type),
};
Expand All @@ -685,12 +693,17 @@ fn get_index_type_name(clean_type: &clean::Type, accept_generic: bool) -> Option
}
}

fn get_generics(clean_type: &clean::Type) -> Option<Vec<String>> {
fn get_generics(clean_type: &clean::Type) -> Option<Vec<Generic>> {
clean_type.generics().and_then(|types| {
let r = types
.iter()
.filter_map(|t| get_index_type_name(t, false))
.map(|s| s.to_ascii_lowercase())
.filter_map(|t| {
if let Some(name) = get_index_type_name(t, false) {
Some(Generic { name: name.to_ascii_lowercase(), defid: t.def_id(), idx: None })
} else {
None
}
})
.collect::<Vec<_>>();
if r.is_empty() { None } else { Some(r) }
})
Expand Down
Loading

0 comments on commit f4c675c

Please sign in to comment.