Fix bugs with SCIP field enclosing_range

Summary:

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Author: Catherine Gasnier

crates/ide/src/static_index.rs

@@ -232,7 +232,42 @@ impl StaticIndex<'_> {
             let id = if let Some(it) = self.def_map.get(&def) {
                 *it
             } else {
-                let it = self.tokens.insert(TokenStaticData {
+                // For methods/functions, get the actual AST node range to avoid issues where
+                // full_range incorrectly includes the entire impl block.
+                let definition_body = match def {
+                    Definition::Function(_) => {
+                        // Get the function's definition position which works reliably
+                        let def_nav = def.try_to_nav(&sema).map(UpmappingResult::call_site);
+
+                        def_nav.and_then(|nav| {
+                            // Parse the file where the definition is located
+                            // Convert FileId to EditionedFileId for parsing
+                            let editioned_file_id = hir::EditionedFileId::current_edition_guess_origin(self.db, nav.file_id);
+                            let parse = sema.parse(editioned_file_id);
+                            let root = parse.syntax();
+
+                            // Find the token at the definition position
+                            let token = root.token_at_offset(nav.focus_or_full_range().start()).right_biased()?;
+
+                            // Walk up to find the function node
+                            let fn_node = token.parent_ancestors().find_map(syntax::ast::Fn::cast)?;
+
+                            // Use the function node's range
+                            Some(FileRange {
+                                file_id: nav.file_id,
+                                range: fn_node.syntax().text_range(),
+                            })
+                        })
+                    }
+                    _ => {
+                        // For non-functions, use the existing NavigationTarget approach
+                        def.try_to_nav(&sema)
+                            .map(UpmappingResult::call_site)
+                            .map(|it| FileRange { file_id: it.file_id, range: it.full_range })
+                    }
+                };
+
+              let it = self.tokens.insert(TokenStaticData {
                     documentation: documentation_for_definition(&sema, def, scope_node),
                     hover: Some(hover_for_definition(
                         &sema,
@@ -249,10 +284,7 @@ impl StaticIndex<'_> {
                     definition: def.try_to_nav(&sema).map(UpmappingResult::call_site).map(|it| {
                         FileRange { file_id: it.file_id, range: it.focus_or_full_range() }
                     }),
-                    definition_body: def
-                        .try_to_nav(&sema)
-                        .map(UpmappingResult::call_site)
-                        .map(|it| FileRange { file_id: it.file_id, range: it.full_range }),
+                    definition_body,
                     references: vec![],
                     moniker: current_crate.and_then(|cc| def_to_moniker(self.db, def, cc)),
                     display_name: def

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

@@ -189,8 +189,8 @@ impl flags::Scip {
                     symbol_roles |= scip_types::SymbolRole::Definition as i32;
                 }
 
-                let enclosing_range = match token.definition_body {
-                    Some(def_body) if def_body.file_id == file_id => {
+                let enclosing_range = match (is_definition, token.definition_body) {
+                    (true, Some(def_body)) if def_body.file_id == file_id => {
                         text_range_to_scip_range(&line_index, def_body.range)
                     }
                     _ => Vec::new(),
@@ -946,4 +946,60 @@ pub mod example_mod {
 
         assert_eq!(token.definition_body, Some(expected_range));
     }
+
+    #[test]
+    fn method_in_impl_has_enclosing_range() {
+        let s = r#"
+struct Foo;
+impl Foo {
+    fn method(&self) {}
+}
+"#;
+
+        let mut host = AnalysisHost::default();
+        let change_fixture = ChangeFixture::parse(s);
+        host.raw_database_mut().apply_change(change_fixture.change);
+
+        let analysis = host.analysis();
+        let si = StaticIndex::compute(
+            &analysis,
+            VendoredLibrariesConfig::Included {
+                workspace_root: &VfsPath::new_virtual_path("/workspace".to_owned()),
+            },
+        );
+
+        let file = si.files.first().unwrap();
+        // Find the definition token for `method` - it should have a definition_body
+        let method_token = file.tokens.iter().find_map(|(_range, id)| {
+            let token = si.tokens.get(*id).unwrap();
+            // Check if this is the method definition by looking at the display name and kind
+            if token.display_name.as_deref() == Some("method")
+                && token.kind == SymbolInformationKind::Method
+            {
+                Some(id)
+            } else {
+                None
+            }
+        }).expect("Should find method token");
+
+        let token = si.tokens.get(*method_token).unwrap();
+
+        // The definition_body should exist and should be just the method, not the entire impl block
+        let def_body = token.definition_body.expect("Method should have a definition_body");
+
+        // The definition_body should cover just the method
+        // Based on the test output, we can see the actual range is 27..46
+        // Let's verify it's not the entire impl block (which would be larger)
+        let impl_start = s.find("impl Foo").unwrap();
+        let impl_end = s.rfind('}').unwrap() + 1;
+        let impl_range = TextRange::new((impl_start as u32).into(), (impl_end as u32).into());
+
+        // The method body should NOT be the same as the entire impl block
+        assert_ne!(def_body.range, impl_range,
+            "Method enclosing range should not be the entire impl block");
+
+        // The method body should be smaller than the impl block
+        assert!(def_body.range.len() < impl_range.len(),
+            "Method enclosing range should be smaller than the impl block");
+    }
 }