Unnamed repository; edit this file 'description' to name the repository.
Merge pull request #22595 from Wilfred/scip_ranges
SCIP: Exclude leading/trailing trivia in definition ranges
Chayim Refael Friedman 11 days ago
parent 1c662c6 · parent b1e191f · commit caa8435
-rw-r--r--crates/ide/src/static_index.rs86
-rw-r--r--crates/rust-analyzer/src/cli/scip.rs64
2 files changed, 143 insertions, 7 deletions
diff --git a/crates/ide/src/static_index.rs b/crates/ide/src/static_index.rs
index 37f58bda97..db24c8dd06 100644
--- a/crates/ide/src/static_index.rs
+++ b/crates/ide/src/static_index.rs
@@ -2,6 +2,7 @@
//! read-only code browsers and emitting LSIF
use arrayvec::ArrayVec;
+use either::Either;
use hir::{Crate, Module, Semantics, db::HirDatabase};
use ide_db::{
FileId, FileRange, FxHashMap, FxHashSet, RootDatabase,
@@ -11,7 +12,7 @@ use ide_db::{
famous_defs::FamousDefs,
ra_fixture::RaFixtureConfig,
};
-use syntax::{AstNode, SyntaxNode, SyntaxToken, TextRange};
+use syntax::{AstNode, AstToken, NodeOrToken, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, ast};
use crate::navigation_target::UpmappingResult;
use crate::{
@@ -52,6 +53,22 @@ pub struct TokenStaticData {
///
/// For example, in `fn foo() {}` this is the position from `fn`
/// to the closing brace.
+ ///
+ /// This excludes trivia (whitespace/comments) other than doc
+ /// comments. This differs from LSP, which includes trivia.
+ ///
+ /// SCIP:
+ ///
+ /// > source range of the nearest non-trivial enclosing AST node.
+ ///
+ /// <https://github.com/scip-code/scip/blob/20459645420419b3c2a10d6a9f57436abeeb273b/scip.proto#L747-L796>
+ ///
+ /// LSP:
+ ///
+ /// > range enclosing this symbol not including leading/trailing
+ /// > whitespace but everything else like comments.
+ ///
+ /// <https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification/#locationLink>
pub definition_body: Option<FileRange>,
pub references: Vec<ReferenceData>,
pub moniker: Option<MonikerResult>,
@@ -182,6 +199,7 @@ impl StaticIndex<'_> {
let id = if let Some(it) = self.def_map.get(&def) {
*it
} else {
+ let nav = def.try_to_nav(&sema).map(UpmappingResult::call_site);
let it = self.tokens.insert(TokenStaticData {
documentation: documentation_for_definition(&sema, def, scope_node),
hover: Some(hover_for_definition(
@@ -196,13 +214,14 @@ impl StaticIndex<'_> {
edition,
display_target,
)),
- 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: nav.as_ref().map(|it| FileRange {
+ file_id: it.file_id,
+ range: it.focus_or_full_range(),
+ }),
+ definition_body: nav.as_ref().map(|it| FileRange {
+ file_id: it.file_id,
+ range: definition_range_excluding_trivia(&sema, it.file_id, it.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 }),
references: vec![],
moniker: current_crate.and_then(|cc| def_to_moniker(self.db, def, cc)),
display_name: def
@@ -288,6 +307,59 @@ impl StaticIndex<'_> {
}
}
+fn definition_range_excluding_trivia(
+ sema: &Semantics<'_, RootDatabase>,
+ file_id: FileId,
+ range: TextRange,
+) -> TextRange {
+ let root = sema.parse_guess_edition(file_id).syntax().clone();
+ if range == root.text_range() {
+ return range;
+ }
+ if !root.text_range().contains_range(range) {
+ return range;
+ }
+
+ let element = root.covering_element(range);
+ let tokens = match element {
+ NodeOrToken::Node(node) => Either::Left(node.descendants_with_tokens().filter_map(|it| {
+ let token = it.into_token()?;
+ range.contains_range(token.text_range()).then_some(token)
+ })),
+ NodeOrToken::Token(token) => Either::Right(std::iter::once(token)),
+ };
+
+ let mut first = None;
+ let mut last = None;
+ for token in tokens {
+ if first.is_none() && !is_leading_trivia_excluding_docs(&token) {
+ first = Some(token.clone());
+ }
+ if !is_trailing_trivia(&token) {
+ last = Some(token);
+ }
+ }
+
+ match (first, last) {
+ (Some(first), Some(last)) => {
+ TextRange::new(first.text_range().start(), last.text_range().end())
+ }
+ _ => range,
+ }
+}
+
+fn is_leading_trivia_excluding_docs(token: &SyntaxToken) -> bool {
+ match token.kind() {
+ SyntaxKind::WHITESPACE => true,
+ SyntaxKind::COMMENT => ast::Comment::cast(token.clone()).is_none_or(|it| !it.is_outer()),
+ _ => false,
+ }
+}
+
+fn is_trailing_trivia(token: &SyntaxToken) -> bool {
+ matches!(token.kind(), SyntaxKind::WHITESPACE | SyntaxKind::COMMENT)
+}
+
#[cfg(test)]
mod tests {
use crate::{StaticIndex, fixture};
diff --git a/crates/rust-analyzer/src/cli/scip.rs b/crates/rust-analyzer/src/cli/scip.rs
index d3acbb934d..3d904e7148 100644
--- a/crates/rust-analyzer/src/cli/scip.rs
+++ b/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));
+ }
}