Merge pull request #19062 from darichey/scip-fix-module-names

Fix scip indexing of module names

Author: Veykril

crates/ide/src/moniker.rs

@@ -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);
 
@@ -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");
                         }
@@ -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;
 

crates/ide/src/static_index.rs

@@ -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),
         });
@@ -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,
@@ -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);
     }
@@ -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:?}");

crates/rust-analyzer/src/cli/scip.rs

@@ -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);
@@ -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,
@@ -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"));