SCIP: Exclude leading/trailing trivia in definition ranges

crates/ide/src/hover/tests.rs

@@ -4145,7 +4145,7 @@ pub mod future {
                                 file_id: FileId(
                                     0,
                                 ),
-                                full_range: 0..110,
+                                full_range: 101..110,
                                 focus_range: 108..109,
                                 name: "S",
                                 kind: Struct,

crates/ide/src/navigation_target.rs

@@ -18,7 +18,7 @@ use ide_db::{
 };
 use stdx::never;
 use syntax::{
-    AstNode, AstPtr, SyntaxNode, TextRange,
+    AstNode, AstPtr, AstToken, SyntaxKind, SyntaxNode, SyntaxToken, TextRange,
     ast::{self, HasName},
 };
 
@@ -448,6 +448,8 @@ where
             NavigationTarget::from_named_with_range(
                 db,
                 src.map(|(full_range, node)| {
+                    let full_range =
+                        node.as_ref().map_or(full_range, |node| range_trim_trivia(node.syntax()));
                     (
                         full_range,
                         node.and_then(|node| {
@@ -502,6 +504,8 @@ impl TryToNav for hir::Impl {
     ) -> Option<UpmappingResult<NavigationTarget>> {
         let db = sema.db;
         let InFile { file_id, value: (full_range, source) } = self.source_with_range(db)?;
+        let full_range =
+            source.as_ref().map_or(full_range, |source| range_trim_trivia(source.syntax()));
 
         Some(
             orig_range_with_focus_r(
@@ -930,6 +934,49 @@ fn orig_range_with_focus(
     )
 }
 
+/// Return the range of `value` without leading/trailing trivia.
+fn range_trim_trivia(value: &SyntaxNode) -> TextRange {
+    let mut first = value.first_child_or_token();
+    while let Some(element) = &first {
+        let Some(token) = element.as_token() else { break };
+        if !is_trivia_not_doc_comment(token) {
+            break;
+        }
+        first = element.next_sibling_or_token();
+    }
+
+    let mut last = value.last_child_or_token();
+    while let Some(element) = &last {
+        let Some(token) = element.as_token() else { break };
+        if !is_trivia(token) {
+            break;
+        }
+        last = element.prev_sibling_or_token();
+    }
+
+    match (first, last) {
+        (Some(first), Some(last)) => {
+            TextRange::new(first.text_range().start(), last.text_range().end())
+        }
+        _ => value.text_range(),
+    }
+}
+
+fn is_trivia_not_doc_comment(token: &SyntaxToken) -> bool {
+    match token.kind() {
+        SyntaxKind::WHITESPACE => true,
+        SyntaxKind::COMMENT => match ast::Comment::cast(token.clone()) {
+            Some(comment) => !comment.is_outer(),
+            None => true,
+        },
+        _ => false,
+    }
+}
+
+fn is_trivia(token: &SyntaxToken) -> bool {
+    matches!(token.kind(), SyntaxKind::WHITESPACE | SyntaxKind::COMMENT)
+}
+
 pub(crate) fn orig_range_with_focus_r(
     db: &RootDatabase,
     hir_file: HirFileId,

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

@@ -948,4 +948,68 @@ pub mod example_mod {
 
         assert_eq!(token.definition_body, Some(expected_range));
     }
+
+    #[test]
+    fn function_enclosing_range_trivia() {
+        let s = "fn first() {}\n// belongs to first\n/// second docs\nfn second() {}";
+
+        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();
+        let token = file
+            .tokens
+            .iter()
+            .filter_map(|(_, token_id)| si.tokens.get(*token_id))
+            .find(|token| token.display_name.as_deref() == Some("second"))
+            .unwrap();
+
+        let definition_body = token.definition_body.unwrap();
+        assert_eq!(
+            definition_body.range.start(),
+            TextSize::new(s.find("/// second docs").unwrap() as u32)
+        );
+        assert_eq!(definition_body.range.end(), TextSize::of(s));
+    }
+
+    #[test]
+    fn const_enclosing_range_trivia() {
+        let s = "const FOO_ONE: i32 = 123; // one\nconst FOO_TWO: i32 = 123; // two";
+
+        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();
+        let token = file
+            .tokens
+            .iter()
+            .filter_map(|(_, token_id)| si.tokens.get(*token_id))
+            .find(|token| token.display_name.as_deref() == Some("FOO_TWO"))
+            .unwrap();
+
+        let definition_body = token.definition_body.unwrap();
+        assert_eq!(
+            definition_body.range.start(),
+            TextSize::new(s.find("const FOO_TWO").unwrap() as u32)
+        );
+        assert_eq!(definition_body.range.end(), TextSize::new(s.find(" // two").unwrap() as u32));
+    }
 }