Move uses_of() to the index

Author: lionel-

crates/oak_db/src/identifier.rs

@@ -19,13 +19,12 @@ use crate::Name;
 #[derive(Debug, Clone)]
 pub enum Identifier<'db> {
     /// Cursor on a binding use or definition site tracked by the semantic
-    /// index. The range is the use or def site's text range. Its start is
-    /// the offset to pass to `resolve_at`.
+    /// index. The range is the use or def site's text range.
     Variable { name: Name<'db>, range: TextRange },
     /// Cursor on the RHS name of a `$` or `@` extract expression. Member
     /// names are not tracked by the semantic index.
     Member {
-        name: String,
+        name: Name<'db>,
         kind: MemberKind,
         operator_range: TextRange,
         name_range: TextRange,
@@ -71,7 +70,7 @@ impl<'db> Identifier<'db> {
 
         if let Some((name, kind, operator_range, name_range)) = classify_member(&root, snapped) {
             return Some(Identifier::Member {
-                name,
+                name: Name::new(db, name.as_str()),
                 kind,
                 operator_range,
                 name_range,
@@ -84,28 +83,8 @@ impl<'db> Identifier<'db> {
 
 impl<'db> File {
     /// All use-site ranges for `name` in this file, across every scope.
-    ///
-    /// Used as the candidate pool for find-references: each returned range is
-    /// confirmed by calling `resolve_at(range.start())` and checking whether
-    /// its definition set intersects the target.
     pub fn uses_of(self, db: &'db dyn Db, name: Name<'db>) -> Vec<TextRange> {
-        let index = self.semantic_index(db);
-        let name_str = name.text(db);
-        let mut ranges = Vec::new();
-
-        for scope_id in index.scope_ids() {
-            let symbols = index.symbols(scope_id);
-            let Some(symbol_id) = symbols.id(name_str) else {
-                continue;
-            };
-            for (_use_id, use_site) in index.uses(scope_id).iter() {
-                if use_site.symbol() == symbol_id {
-                    ranges.push(use_site.range());
-                }
-            }
-        }
-
-        ranges
+        self.semantic_index(db).uses_of(name.text(db).as_str())
     }
 
     /// All ranges where `name` appears as the RHS of a `$` or `@` with the
@@ -141,6 +120,9 @@ fn classify_member(
     root: &RSyntaxNode,
     offset: TextSize,
 ) -> Option<(String, MemberKind, TextRange, TextRange)> {
+    // Snapping put `offset` at the name's start, which is the operator's end,
+    // so `token_at_offset()` reports `Between(op, name)`. Take the name on the
+    // right side.
     let token = root.token_at_offset(offset).right_biased()?;
     if !is_name_token(&token) {
         return None;
@@ -150,8 +132,7 @@ fn classify_member(
     let selector_node = token.parent()?;
     let extract = RExtractExpression::cast(selector_node.parent()?)?;
 
-    // Token is after the operator -> it's on the RHS, not LHS.
-    // RHS start == op end (adjacent bytes), so use strict <.
+    // Token is after the operator -> it's on the RHS, not LHS
     let op = extract.operator().ok()?;
     if token.text_trimmed_range().start() < op.text_trimmed_range().end() {
         return None;
@@ -201,6 +182,9 @@ fn snap_to_name_at_boundary(root: &RSyntaxNode, offset: TextSize) -> TextSize {
 }
 
 fn is_name_token(token: &RSyntaxToken) -> bool {
+    // FIXME: Right now we're too liberal with strings. We should only match
+    // them when they stand for an identifier, e.g. in the LHS of `<-` or in
+    // function call position.
     matches!(
         token.kind(),
         RSyntaxKind::IDENT | RSyntaxKind::R_STRING_LITERAL

crates/oak_db/src/tests/identifier.rs

@@ -102,7 +102,7 @@ fn test_classify_on_dollar_member() {
             operator_range,
             name_range,
         }) => {
-            assert_eq!(name, "bar");
+            assert_eq!(name.text(&db).as_str(), "bar");
             assert_eq!(kind, MemberKind::Dollar);
             assert_eq!(operator_range, text_range(3, 4));
             assert_eq!(name_range, text_range(4, 7));
@@ -119,7 +119,7 @@ fn test_classify_on_at_member() {
 
     match Identifier::classify(&db, file, offset(4)) {
         Some(Identifier::Member { name, kind, .. }) => {
-            assert_eq!(name, "bar");
+            assert_eq!(name.text(&db).as_str(), "bar");
             assert_eq!(kind, MemberKind::At);
         },
         other => panic!("expected Member, got {other:?}"),

crates/oak_ide/src/find_references.rs

@@ -30,7 +30,9 @@ pub fn find_references(
         Identifier::Variable { range, .. } => {
             find_variable_references(db, file, range.start(), include_declaration)
         },
-        Identifier::Member { name, kind, .. } => find_member_references(db, file, &name, kind),
+        Identifier::Member { name, kind, .. } => {
+            find_member_references(db, file, name.text(db).as_str(), kind)
+        },
     }
 }
 
@@ -56,6 +58,11 @@ fn find_variable_references(
         all_matching_files(db, name.text(db).as_str())
     };
 
+    // Rust-Analyzer does a pure text search across all files, then resolves
+    // each occurrences. We are more aligned with ty: we filter files by a text
+    // search, but then we walk a post-parse tree. ty walks a raw AST, we walk
+    // the index via `uses_of()`. The latter is more to the point.
+
     // A candidate is a reference when it resolves to the same binding as the cursor.
     let target_set: HashSet<Definition<'_>> = target_defs.iter().copied().collect();
 

crates/oak_ide/tests/integration/rename.rs

@@ -139,7 +139,7 @@ fn test_rename_non_renamable_errors() {
     let file = upsert(&mut db, "test.R", "dplyr::mutate\n");
 
     let err = rename(&db, file, offset(7), "x").unwrap_err();
-    assert!(err.to_string().contains("renamable identifier"));
+    assert!(err.to_string().contains("Can't rename identifier"));
 }
 
 #[test]

crates/oak_semantic/src/semantic_index.rs

@@ -188,6 +188,22 @@ impl SemanticIndex {
         Some((scope, id, use_site))
     }
 
+    /// All use-site ranges for `name`, across every scope in the file.
+    pub fn uses_of(&self, name: &str) -> Vec<TextRange> {
+        let mut ranges = Vec::new();
+        for scope_id in self.scope_ids() {
+            let Some(symbol_id) = self.symbols(scope_id).id(name) else {
+                continue;
+            };
+            for (_use_id, use_site) in self.uses(scope_id).iter() {
+                if use_site.symbol() == symbol_id {
+                    ranges.push(use_site.range());
+                }
+            }
+        }
+        ranges
+    }
+
     /// Iterate direct child scopes of `scope`.
     pub fn child_scope_ids(&self, scope_id: ScopeId) -> ChildScopeIdsIter<'_> {
         let descendants = &self.scopes[scope_id].descendants;