fix: Fix import_map::search_dependencies getting confused by assoc and non assoc items with the same name

This commit is contained in:
Lukas Wirth 2023-12-12 15:44:27 +01:00
parent 3aa6306728
commit ca995d765d
3 changed files with 66 additions and 36 deletions

View file

@ -9,6 +9,7 @@ use indexmap::IndexMap;
use itertools::Itertools; use itertools::Itertools;
use rustc_hash::{FxHashSet, FxHasher}; use rustc_hash::{FxHashSet, FxHasher};
use smallvec::SmallVec; use smallvec::SmallVec;
use stdx::format_to;
use triomphe::Arc; use triomphe::Arc;
use crate::{ use crate::{
@ -53,13 +54,25 @@ pub struct ImportMap {
fst: fst::Map<Vec<u8>>, fst: fst::Map<Vec<u8>>,
} }
#[derive(Copy, Clone, PartialEq, Eq)] #[derive(Copy, Clone, Debug, PartialEq, Eq, Ord, PartialOrd)]
enum IsTraitAssocItem { enum IsTraitAssocItem {
Yes, Yes,
No, No,
} }
impl ImportMap { impl ImportMap {
pub fn dump(&self, db: &dyn DefDatabase) -> String {
let mut out = String::new();
for (k, v) in self.map.iter() {
format_to!(out, "{:?} ({:?}) -> ", k, v.1);
for v in &v.0 {
format_to!(out, "{}:{:?}, ", v.name.display(db.upcast()), v.container);
}
format_to!(out, "\n");
}
out
}
pub(crate) fn import_map_query(db: &dyn DefDatabase, krate: CrateId) -> Arc<Self> { pub(crate) fn import_map_query(db: &dyn DefDatabase, krate: CrateId) -> Arc<Self> {
let _p = profile::span("import_map_query"); let _p = profile::span("import_map_query");
@ -68,26 +81,31 @@ impl ImportMap {
let mut importables: Vec<_> = map let mut importables: Vec<_> = map
.iter() .iter()
// We've only collected items, whose name cannot be tuple field. // We've only collected items, whose name cannot be tuple field.
.flat_map(|(&item, (info, _))| { .flat_map(|(&item, (info, is_assoc))| {
info.iter() info.iter().map(move |info| {
.map(move |info| (item, info.name.as_str().unwrap().to_ascii_lowercase())) (item, *is_assoc, info.name.as_str().unwrap().to_ascii_lowercase())
})
}) })
.collect(); .collect();
importables.sort_by(|(_, lhs_name), (_, rhs_name)| lhs_name.cmp(rhs_name)); importables.sort_by(|(_, l_is_assoc, lhs_name), (_, r_is_assoc, rhs_name)| {
lhs_name.cmp(rhs_name).then_with(|| l_is_assoc.cmp(r_is_assoc))
});
importables.dedup(); importables.dedup();
// Build the FST, taking care not to insert duplicate values. // Build the FST, taking care not to insert duplicate values.
let mut builder = fst::MapBuilder::memory(); let mut builder = fst::MapBuilder::memory();
let iter = let iter = importables
importables.iter().enumerate().dedup_by(|(_, (_, lhs)), (_, (_, rhs))| lhs == rhs); .iter()
for (start_idx, (_, name)) in iter { .enumerate()
.dedup_by(|(_, (_, _, lhs)), (_, (_, _, rhs))| lhs == rhs);
for (start_idx, (_, _, name)) in iter {
let _ = builder.insert(name, start_idx as u64); let _ = builder.insert(name, start_idx as u64);
} }
Arc::new(ImportMap { Arc::new(ImportMap {
map, map,
fst: builder.into_map(), fst: builder.into_map(),
importables: importables.into_iter().map(|(item, _)| item).collect(), importables: importables.into_iter().map(|(item, _, _)| item).collect(),
}) })
} }
@ -332,20 +350,20 @@ impl Query {
} }
/// Checks whether the import map entry matches the query. /// Checks whether the import map entry matches the query.
fn import_matches( fn import_matches(&self, import: &ImportInfo, enforce_lowercase: bool) -> bool {
&self,
db: &dyn DefDatabase,
import: &ImportInfo,
enforce_lowercase: bool,
) -> bool {
let _p = profile::span("import_map::Query::import_matches"); let _p = profile::span("import_map::Query::import_matches");
// FIXME: Can we get rid of the alloc here? // FIXME: Can we get rid of the alloc here?
let mut input = import.name.display(db.upcast()).to_string(); let input = import.name.to_smol_str();
let mut _s_slot;
let case_insensitive = enforce_lowercase || !self.case_sensitive; let case_insensitive = enforce_lowercase || !self.case_sensitive;
if case_insensitive { let input = if case_insensitive {
input.make_ascii_lowercase(); _s_slot = String::from(input);
} _s_slot.make_ascii_lowercase();
&*_s_slot
} else {
&*input
};
let query_string = if case_insensitive { &self.lowercased } else { &self.query }; let query_string = if case_insensitive { &self.lowercased } else { &self.query };
@ -355,7 +373,7 @@ impl Query {
SearchMode::Fuzzy => { SearchMode::Fuzzy => {
let mut input_chars = input.chars(); let mut input_chars = input.chars();
for query_char in query_string.chars() { for query_char in query_string.chars() {
if input_chars.find(|&it| it == query_char).is_none() { if !input_chars.any(|it| it == query_char) {
return false; return false;
} }
} }
@ -376,6 +394,7 @@ pub fn search_dependencies(
let _p = profile::span("search_dependencies").detail(|| format!("{query:?}")); let _p = profile::span("search_dependencies").detail(|| format!("{query:?}"));
let graph = db.crate_graph(); let graph = db.crate_graph();
let import_maps: Vec<_> = let import_maps: Vec<_> =
graph[krate].dependencies.iter().map(|dep| db.import_map(dep.crate_id)).collect(); graph[krate].dependencies.iter().map(|dep| db.import_map(dep.crate_id)).collect();
@ -390,22 +409,28 @@ pub fn search_dependencies(
let mut res = FxHashSet::default(); let mut res = FxHashSet::default();
let mut common_importable_data_scratch = vec![]; let mut common_importable_data_scratch = vec![];
// FIXME: Improve this, its rather unreadable and does duplicate amount of work
while let Some((_, indexed_values)) = stream.next() { while let Some((_, indexed_values)) = stream.next() {
for &IndexedValue { index, value } in indexed_values { for &IndexedValue { index, value } in indexed_values {
let import_map = &import_maps[index]; let import_map = &import_maps[index];
let importables @ [importable, ..] = &import_map.importables[value as usize..] else { let importables = &import_map.importables[value as usize..];
// Find the first item in this group that has a matching assoc mode and slice the rest away
let Some(importable) =
importables.iter().position(|it| query.matches_assoc_mode(import_map.map[it].1))
else {
continue;
};
let importables @ [importable, ..] = &importables[importable..] else {
continue; continue;
}; };
let &(ref importable_data, is_trait_assoc_item) = &import_map.map[importable]; // Fetch all the known names of this importable item (to handle import aliases/renames)
if !query.matches_assoc_mode(is_trait_assoc_item) {
continue;
}
common_importable_data_scratch.extend( common_importable_data_scratch.extend(
importable_data import_map.map[importable]
.0
.iter() .iter()
.filter(|&info| query.import_matches(db, info, true)) .filter(|&info| query.import_matches(info, true))
// Name shared by the importable items in this group. // Name shared by the importable items in this group.
.map(|info| info.name.to_smol_str()), .map(|info| info.name.to_smol_str()),
); );
@ -419,6 +444,7 @@ pub fn search_dependencies(
common_importable_data_scratch.drain(..).flat_map(|common_importable_name| { common_importable_data_scratch.drain(..).flat_map(|common_importable_name| {
// Add the items from this name group. Those are all subsequent items in // Add the items from this name group. Those are all subsequent items in
// `importables` whose name match `common_importable_name`. // `importables` whose name match `common_importable_name`.
importables importables
.iter() .iter()
.copied() .copied()
@ -434,11 +460,8 @@ pub fn search_dependencies(
.filter(move |item| { .filter(move |item| {
!query.case_sensitive || { !query.case_sensitive || {
// we've already checked the common importables name case-insensitively // we've already checked the common importables name case-insensitively
let &(ref import_infos, assoc_mode) = &import_map.map[item]; let &(ref import_infos, _) = &import_map.map[item];
query.matches_assoc_mode(assoc_mode) import_infos.iter().any(|info| query.import_matches(info, false))
&& import_infos
.iter()
.any(|info| query.import_matches(db, info, false))
} }
}) })
}); });

View file

@ -36,7 +36,8 @@ pub fn items_with_name<'a>(
NameToImport::Prefix(exact_name, case_sensitive) NameToImport::Prefix(exact_name, case_sensitive)
| NameToImport::Exact(exact_name, case_sensitive) => { | NameToImport::Exact(exact_name, case_sensitive) => {
let mut local_query = symbol_index::Query::new(exact_name.clone()); let mut local_query = symbol_index::Query::new(exact_name.clone());
let mut external_query = import_map::Query::new(exact_name); let mut external_query =
import_map::Query::new(exact_name).assoc_search_mode(assoc_item_search);
if prefix { if prefix {
local_query.prefix(); local_query.prefix();
external_query = external_query.prefix(); external_query = external_query.prefix();

View file

@ -32,7 +32,10 @@ fn integrated_highlighting_benchmark() {
let workspace_to_load = project_root(); let workspace_to_load = project_root();
let file = "./crates/rust-analyzer/src/config.rs"; let file = "./crates/rust-analyzer/src/config.rs";
let cargo_config = CargoConfig::default(); let cargo_config = CargoConfig {
sysroot: Some(project_model::RustLibSource::Discover),
..CargoConfig::default()
};
let load_cargo_config = LoadCargoConfig { let load_cargo_config = LoadCargoConfig {
load_out_dirs_from_check: true, load_out_dirs_from_check: true,
with_proc_macro_server: ProcMacroServerChoice::None, with_proc_macro_server: ProcMacroServerChoice::None,
@ -85,7 +88,10 @@ fn integrated_completion_benchmark() {
let workspace_to_load = project_root(); let workspace_to_load = project_root();
let file = "./crates/hir/src/lib.rs"; let file = "./crates/hir/src/lib.rs";
let cargo_config = CargoConfig::default(); let cargo_config = CargoConfig {
sysroot: Some(project_model::RustLibSource::Discover),
..CargoConfig::default()
};
let load_cargo_config = LoadCargoConfig { let load_cargo_config = LoadCargoConfig {
load_out_dirs_from_check: true, load_out_dirs_from_check: true,
with_proc_macro_server: ProcMacroServerChoice::None, with_proc_macro_server: ProcMacroServerChoice::None,