Skip to content

Commit 0af780e

Browse files
VeykrilVeykril
committed
Fix symbol_index::Query::search_maps not adhering to case-sensitivity correctly
1 parent c3a29e5 commit 0af780e

File tree

4 files changed

+133
-55
lines changed

4 files changed

+133
-55
lines changed

‎crates/hir-def/src/import_map.rs

+23-3
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,7 @@ fn search_maps(
406406
})
407407
// we put all entries with the same lowercased name in a row, so stop once we find a
408408
// different name in the importables
409+
// FIXME: Consider putting a range into the value: u64 as (u32, u32)?
409410
.take_while(|&(_, info, _)| {
410411
info.name.to_smol_str().as_bytes().eq_ignore_ascii_case(&key)
411412
})
@@ -417,13 +418,32 @@ fn search_maps(
417418
return true;
418419
}
419420
let name = info.name.to_smol_str();
421+
// FIXME: Deduplicate this from ide-db
420422
match query.search_mode {
421-
SearchMode::Exact => name == query.query,
422-
SearchMode::Prefix => name.starts_with(&query.query),
423+
SearchMode::Exact => !query.case_sensitive || name == query.query,
424+
SearchMode::Prefix => {
425+
query.query.len() <= name.len() && {
426+
let prefix = &name[..query.query.len() as usize];
427+
if query.case_sensitive {
428+
prefix == query.query
429+
} else {
430+
prefix.eq_ignore_ascii_case(&query.query)
431+
}
432+
}
433+
}
423434
SearchMode::Fuzzy => {
424435
let mut name = &*name;
425436
query.query.chars().all(|query_char| {
426-
match name.match_indices(query_char).next() {
437+
let m = if query.case_sensitive {
438+
name.match_indices(query_char).next()
439+
} else {
440+
name.match_indices([
441+
query_char,
442+
query_char.to_ascii_uppercase(),
443+
])
444+
.next()
445+
};
446+
match m {
427447
Some((index, _)) => {
428448
name = &name[index + 1..];
429449
true

‎crates/hir/src/symbols.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ impl<'a> SymbolCollector<'a> {
163163
}
164164

165165
// Record renamed imports.
166-
// In case it imports multiple items under different namespaces we just pick one arbitrarily
166+
// FIXME: In case it imports multiple items under different namespaces we just pick one arbitrarily
167167
// for now.
168168
for id in scope.imports() {
169169
let loc = id.import.lookup(self.db.upcast());

‎crates/ide-db/src/items_locator.rs

+10-16
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,9 @@ pub fn items_with_name<'a>(
3636
NameToImport::Prefix(exact_name, case_sensitive)
3737
| NameToImport::Exact(exact_name, case_sensitive) => {
3838
let mut local_query = symbol_index::Query::new(exact_name.clone());
39+
local_query.assoc_search_mode(assoc_item_search);
3940
let mut external_query =
40-
// import_map::Query::new(exact_name).assoc_search_mode(assoc_item_search);
41-
import_map::Query::new(exact_name);
41+
import_map::Query::new(exact_name).assoc_search_mode(assoc_item_search);
4242
if prefix {
4343
local_query.prefix();
4444
external_query = external_query.prefix();
@@ -55,6 +55,7 @@ pub fn items_with_name<'a>(
5555
NameToImport::Fuzzy(fuzzy_search_string, case_sensitive) => {
5656
let mut local_query = symbol_index::Query::new(fuzzy_search_string.clone());
5757
local_query.fuzzy();
58+
local_query.assoc_search_mode(assoc_item_search);
5859

5960
let mut external_query = import_map::Query::new(fuzzy_search_string.clone())
6061
.fuzzy()
@@ -73,13 +74,12 @@ pub fn items_with_name<'a>(
7374
local_query.limit(limit);
7475
}
7576

76-
find_items(sema, krate, assoc_item_search, local_query, external_query)
77+
find_items(sema, krate, local_query, external_query)
7778
}
7879

7980
fn find_items<'a>(
8081
sema: &'a Semantics<'_, RootDatabase>,
8182
krate: Crate,
82-
assoc_item_search: AssocSearchMode,
8383
local_query: symbol_index::Query,
8484
external_query: import_map::Query,
8585
) -> impl Iterator<Item = ItemInNs> + 'a {
@@ -97,18 +97,12 @@ fn find_items<'a>(
9797
});
9898

9999
// Query the local crate using the symbol index.
100-
let local_results = local_query
101-
.search(&symbol_index::crate_symbols(db, krate))
102-
.into_iter()
103-
.filter(move |candidate| match assoc_item_search {
104-
AssocSearchMode::Include => true,
105-
AssocSearchMode::Exclude => !candidate.is_assoc,
106-
AssocSearchMode::AssocItemsOnly => candidate.is_assoc,
107-
})
108-
.map(|local_candidate| match local_candidate.def {
100+
let mut local_results = Vec::new();
101+
local_query.search(&symbol_index::crate_symbols(db, krate), |local_candidate| {
102+
local_results.push(match local_candidate.def {
109103
hir::ModuleDef::Macro(macro_def) => ItemInNs::Macros(macro_def),
110104
def => ItemInNs::from(def),
111-
});
112-
113-
external_importables.chain(local_results)
105+
})
106+
});
107+
local_results.into_iter().chain(external_importables)
114108
}

‎crates/ide-db/src/symbol_index.rs

+99-35
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@ use base_db::{
3131
salsa::{self, ParallelDatabase},
3232
SourceDatabaseExt, SourceRootId, Upcast,
3333
};
34-
use fst::{self, Streamer};
34+
use fst::{self, raw::IndexedValue, Automaton, Streamer};
3535
use hir::{
3636
db::HirDatabase,
37+
import_map::AssocSearchMode,
3738
symbols::{FileSymbol, SymbolCollector},
3839
Crate, Module,
3940
};
@@ -57,6 +58,7 @@ pub struct Query {
5758
only_types: bool,
5859
libs: bool,
5960
mode: SearchMode,
61+
assoc_mode: AssocSearchMode,
6062
case_sensitive: bool,
6163
limit: usize,
6264
}
@@ -70,6 +72,7 @@ impl Query {
7072
only_types: false,
7173
libs: false,
7274
mode: SearchMode::Fuzzy,
75+
assoc_mode: AssocSearchMode::Include,
7376
case_sensitive: false,
7477
limit: usize::max_value(),
7578
}
@@ -95,6 +98,11 @@ impl Query {
9598
self.mode = SearchMode::Prefix;
9699
}
97100

101+
/// Specifies whether we want to include associated items in the result.
102+
pub fn assoc_search_mode(&mut self, assoc_mode: AssocSearchMode) {
103+
self.assoc_mode = assoc_mode;
104+
}
105+
98106
pub fn case_sensitive(&mut self) {
99107
self.case_sensitive = true;
100108
}
@@ -225,7 +233,9 @@ pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec<FileSymbol> {
225233
indices.iter().flat_map(|indices| indices.iter().cloned()).collect()
226234
};
227235

228-
query.search(&indices)
236+
let mut res = vec![];
237+
query.search(&indices, |f| res.push(f.clone()));
238+
res
229239
}
230240

231241
#[derive(Default)]
@@ -285,6 +295,7 @@ impl SymbolIndex {
285295
builder.insert(key, value).unwrap();
286296
}
287297

298+
// FIXME: fst::Map should ideally have a way to shrink the backing buffer without the unwrap dance
288299
let map = fst::Map::new({
289300
let mut buf = builder.into_inner().unwrap();
290301
buf.shrink_to_fit();
@@ -317,61 +328,114 @@ impl SymbolIndex {
317328
}
318329

319330
impl Query {
320-
pub(crate) fn search(self, indices: &[Arc<SymbolIndex>]) -> Vec<FileSymbol> {
331+
pub(crate) fn search<'sym>(
332+
self,
333+
indices: &'sym [Arc<SymbolIndex>],
334+
cb: impl FnMut(&'sym FileSymbol),
335+
) {
321336
let _p = profile::span("symbol_index::Query::search");
322337
let mut op = fst::map::OpBuilder::new();
323-
for file_symbols in indices.iter() {
324-
let automaton = fst::automaton::Subsequence::new(&self.lowercased);
325-
op = op.add(file_symbols.map.search(automaton))
338+
match self.mode {
339+
SearchMode::Exact => {
340+
let automaton = fst::automaton::Str::new(&self.lowercased);
341+
342+
for index in indices.iter() {
343+
op = op.add(index.map.search(&automaton));
344+
}
345+
self.search_maps(&indices, op.union(), cb)
346+
}
347+
SearchMode::Fuzzy => {
348+
let automaton = fst::automaton::Subsequence::new(&self.lowercased);
349+
350+
for index in indices.iter() {
351+
op = op.add(index.map.search(&automaton));
352+
}
353+
self.search_maps(&indices, op.union(), cb)
354+
}
355+
SearchMode::Prefix => {
356+
let automaton = fst::automaton::Str::new(&self.lowercased).starts_with();
357+
358+
for index in indices.iter() {
359+
op = op.add(index.map.search(&automaton));
360+
}
361+
self.search_maps(&indices, op.union(), cb)
362+
}
326363
}
327-
let mut stream = op.union();
328-
let mut res = Vec::new();
364+
}
365+
366+
fn search_maps<'sym>(
367+
&self,
368+
indices: &'sym [Arc<SymbolIndex>],
369+
mut stream: fst::map::Union<'_>,
370+
mut cb: impl FnMut(&'sym FileSymbol),
371+
) {
329372
while let Some((_, indexed_values)) = stream.next() {
330-
for indexed_value in indexed_values {
331-
let symbol_index = &indices[indexed_value.index];
332-
let (start, end) = SymbolIndex::map_value_to_range(indexed_value.value);
373+
for &IndexedValue { index, value } in indexed_values {
374+
let symbol_index = &indices[index];
375+
let (start, end) = SymbolIndex::map_value_to_range(value);
333376

334377
for symbol in &symbol_index.symbols[start..end] {
335-
if self.only_types
378+
let non_type_for_type_only_query = self.only_types
336379
&& !matches!(
337380
symbol.def,
338381
hir::ModuleDef::Adt(..)
339382
| hir::ModuleDef::TypeAlias(..)
340383
| hir::ModuleDef::BuiltinType(..)
341384
| hir::ModuleDef::TraitAlias(..)
342385
| hir::ModuleDef::Trait(..)
343-
)
344-
{
386+
);
387+
if non_type_for_type_only_query || !self.matches_assoc_mode(symbol.is_assoc) {
345388
continue;
346389
}
347-
let skip = match self.mode {
348-
SearchMode::Fuzzy => {
349-
self.case_sensitive
350-
&& self.query.chars().any(|c| !symbol.name.contains(c))
390+
// FIXME: Deduplicate this from hir-def
391+
let matches = match self.mode {
392+
SearchMode::Exact if self.case_sensitive => symbol.name == self.query,
393+
SearchMode::Exact => symbol.name.eq_ignore_ascii_case(&self.query),
394+
SearchMode::Prefix => {
395+
self.query.len() <= symbol.name.len() && {
396+
let prefix = &symbol.name[..self.query.len() as usize];
397+
if self.case_sensitive {
398+
prefix == self.query
399+
} else {
400+
prefix.eq_ignore_ascii_case(&self.query)
401+
}
402+
}
351403
}
352-
SearchMode::Exact => symbol.name != self.query,
353-
SearchMode::Prefix if self.case_sensitive => {
354-
!symbol.name.starts_with(&self.query)
404+
SearchMode::Fuzzy => {
405+
let mut name = &*symbol.name;
406+
self.query.chars().all(|query_char| {
407+
let m = if self.case_sensitive {
408+
name.match_indices(query_char).next()
409+
} else {
410+
name.match_indices([
411+
query_char,
412+
query_char.to_ascii_uppercase(),
413+
])
414+
.next()
415+
};
416+
match m {
417+
Some((index, _)) => {
418+
name = &name[index + 1..];
419+
true
420+
}
421+
None => false,
422+
}
423+
})
355424
}
356-
SearchMode::Prefix => symbol
357-
.name
358-
.chars()
359-
.zip(self.lowercased.chars())
360-
.all(|(n, q)| n.to_lowercase().next() == Some(q)),
361425
};
362-
363-
if skip {
364-
continue;
365-
}
366-
367-
res.push(symbol.clone());
368-
if res.len() >= self.limit {
369-
return res;
426+
if matches {
427+
cb(symbol);
370428
}
371429
}
372430
}
373431
}
374-
res
432+
}
433+
434+
fn matches_assoc_mode(&self, is_trait_assoc_item: bool) -> bool {
435+
match (is_trait_assoc_item, self.assoc_mode) {
436+
(true, AssocSearchMode::Exclude) | (false, AssocSearchMode::AssocItemsOnly) => false,
437+
_ => true,
438+
}
375439
}
376440
}
377441

0 commit comments

Comments
 (0)