Skip to content

fix: Fix scip indexing of module names #19062

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 17 additions & 5 deletions crates/ide/src/moniker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,10 @@ fn def_to_non_local_moniker(
definition: Definition,
from_crate: Crate,
) -> Option<Moniker> {
let module = definition.module(db)?;
let module = match definition {
Definition::Module(module) if module.is_crate_root() => module,
_ => definition.module(db)?,
};
let krate = module.krate();
let edition = krate.edition(db);

Expand Down Expand Up @@ -322,12 +325,18 @@ fn def_to_non_local_moniker(
name: name.display(db, edition).to_string(),
desc: def_to_kind(db, def).into(),
});
} else if reverse_description.is_empty() {
// Don't allow the last descriptor to be absent.
return None;
} else {
match def {
Definition::Module(module) if module.is_crate_root() => {}
Definition::Module(module) if module.is_crate_root() => {
// only include `crate` namespace by itself because we prefer
// `rust-analyzer cargo foo . bar/` over `rust-analyzer cargo foo . crate/bar/`
if reverse_description.is_empty() {
reverse_description.push(MonikerDescriptor {
name: "crate".to_owned(),
desc: MonikerDescriptorKind::Namespace,
});
}
}
_ => {
tracing::error!(?def, "Encountered enclosing definition with no name");
}
Expand All @@ -340,6 +349,9 @@ fn def_to_non_local_moniker(
};
def = next_def;
}
if reverse_description.is_empty() {
return None;
}
reverse_description.reverse();
let description = reverse_description;

Expand Down
37 changes: 26 additions & 11 deletions crates/ide/src/static_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,10 @@ impl StaticIndex<'_> {
.unwrap();
// hovers
let sema = hir::Semantics::new(self.db);
let tokens_or_nodes = sema.parse_guess_edition(file_id).syntax().clone();
let root = sema.parse_guess_edition(file_id).syntax().clone();
let edition =
sema.attach_first_edition(file_id).map(|it| it.edition()).unwrap_or(Edition::CURRENT);
let tokens = tokens_or_nodes.descendants_with_tokens().filter_map(|it| match it {
let tokens = root.descendants_with_tokens().filter_map(|it| match it {
syntax::NodeOrToken::Node(_) => None,
syntax::NodeOrToken::Token(it) => Some(it),
});
Expand All @@ -194,24 +194,19 @@ impl StaticIndex<'_> {
)
});
let mut result = StaticIndexedFile { file_id, inlay_hints, folds, tokens: vec![] };
for token in tokens {
let range = token.text_range();
let node = token.parent().unwrap();
let def = match get_definition(&sema, token.clone()) {
Some(it) => it,
None => continue,
};

let mut add_token = |def: Definition, range: TextRange, scope_node: &SyntaxNode| {
let id = if let Some(it) = self.def_map.get(&def) {
*it
} else {
let it = self.tokens.insert(TokenStaticData {
documentation: documentation_for_definition(&sema, def, &node),
documentation: documentation_for_definition(&sema, def, scope_node),
hover: Some(hover_for_definition(
&sema,
file_id,
def,
None,
&node,
scope_node,
None,
false,
&hover_config,
Expand Down Expand Up @@ -240,6 +235,22 @@ impl StaticIndex<'_> {
},
});
result.tokens.push((range, id));
};

if let Some(module) = sema.file_to_module_def(file_id) {
let def = Definition::Module(module);
let range = root.text_range();
add_token(def, range, &root);
}

for token in tokens {
let range = token.text_range();
let node = token.parent().unwrap();
let def = match get_definition(&sema, token.clone()) {
Some(it) => it,
None => continue,
};
add_token(def, range, &node);
}
self.files.push(result);
}
Expand Down Expand Up @@ -300,6 +311,10 @@ mod tests {
let mut range_set: FxHashSet<_> = ranges.iter().map(|it| it.0).collect();
for f in s.files {
for (range, _) in f.tokens {
if range.start() == TextSize::from(0) {
// ignore whole file range corresponding to module definition
continue;
}
let it = FileRange { file_id: f.file_id, range };
if !range_set.contains(&it) {
panic!("additional range {it:?}");
Expand Down
18 changes: 10 additions & 8 deletions crates/rust-analyzer/src/cli/scip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -444,14 +444,14 @@ impl SymbolGenerator {
MonikerResult::Moniker(moniker) => TokenSymbols {
symbol: scip::symbol::format_symbol(moniker_to_symbol(moniker)),
enclosing_symbol: None,
is_inherent_impl: moniker
.identifier
.description
.get(moniker.identifier.description.len() - 2)
.is_some_and(|descriptor| {
is_inherent_impl: match &moniker.identifier.description[..] {
// inherent impls are represented as impl#[SelfType]
[.., descriptor, _] => {
descriptor.desc == MonikerDescriptorKind::Type
&& descriptor.name == "impl"
}),
}
_ => false,
},
},
MonikerResult::Local { enclosing_moniker } => {
let local_symbol = scip::types::Symbol::new_local(local_count);
Expand Down Expand Up @@ -549,7 +549,9 @@ mod test {
continue;
}
for &(range, id) in &file.tokens {
if range.contains(offset - TextSize::from(1)) {
// check if cursor is within token, ignoring token for the module defined by the file (whose range is the whole file)
if range.start() != TextSize::from(0) && range.contains(offset - TextSize::from(1))
{
let token = si.tokens.get(id).unwrap();
found_symbol = match token.moniker.as_ref() {
None => None,
Expand Down Expand Up @@ -885,7 +887,7 @@ pub mod example_mod {
);

let file = si.files.first().unwrap();
let (_, token_id) = file.tokens.first().unwrap();
let (_, token_id) = file.tokens.get(1).unwrap(); // first token is file module, second is `bar`
let token = si.tokens.get(*token_id).unwrap();

assert_eq!(token.documentation.as_ref().map(|d| d.as_str()), Some("foo"));
Expand Down
Loading