Merge #4016

4016: Don't use SyntaxNodePtr::range when determining scope for offset r=matklad a=matklad



bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

crates/ra_hir/src/source_analyzer.rs

@@ -23,7 +23,7 @@ use hir_ty::{
 };
 use ra_syntax::{
     ast::{self, AstNode},
-    SyntaxNode, SyntaxNodePtr, TextUnit,
+    SyntaxNode, TextRange, TextUnit,
 };
 
 use crate::{
@@ -56,7 +56,7 @@ impl SourceAnalyzer {
         let scopes = db.expr_scopes(def);
         let scope = match offset {
             None => scope_for(&scopes, &source_map, node),
-            Some(offset) => scope_for_offset(&scopes, &source_map, node.with_value(offset)),
+            Some(offset) => scope_for_offset(db, &scopes, &source_map, node.with_value(offset)),
         };
         let resolver = resolver_for_scope(db.upcast(), def, scope);
         SourceAnalyzer {
@@ -304,6 +304,7 @@ fn scope_for(
 }
 
 fn scope_for_offset(
+    db: &dyn HirDatabase,
     scopes: &ExprScopes,
     source_map: &BodySourceMap,
     offset: InFile<TextUnit>,
@@ -317,21 +318,63 @@ fn scope_for_offset(
             if source.file_id != offset.file_id {
                 return None;
             }
-            let syntax_node_ptr = source.value.syntax_node_ptr();
+            let root = source.file_syntax(db.upcast());
-            Some((syntax_node_ptr, scope))
+            let node = source.value.to_node(&root);
+            Some((node.syntax().text_range(), scope))
         })
         // find containing scope
-        .min_by_key(|(ptr, _scope)| {
+        .min_by_key(|(expr_range, _scope)| {
             (
-                !(ptr.range().start() <= offset.value && offset.value <= ptr.range().end()),
+                !(expr_range.start() <= offset.value && offset.value <= expr_range.end()),
-                ptr.range().len(),
+                expr_range.len(),
             )
         })
-        .map(|(ptr, scope)| {
+        .map(|(expr_range, scope)| {
-            adjust(scopes, source_map, ptr, offset.file_id, offset.value).unwrap_or(*scope)
+            adjust(db, scopes, source_map, expr_range, offset.file_id, offset.value)
+                .unwrap_or(*scope)
         })
 }
 
+// XXX: during completion, cursor might be outside of any particular
+// expression. Try to figure out the correct scope...
+fn adjust(
+    db: &dyn HirDatabase,
+    scopes: &ExprScopes,
+    source_map: &BodySourceMap,
+    expr_range: TextRange,
+    file_id: HirFileId,
+    offset: TextUnit,
+) -> Option<ScopeId> {
+    let child_scopes = scopes
+        .scope_by_expr()
+        .iter()
+        .filter_map(|(id, scope)| {
+            let source = source_map.expr_syntax(*id).ok()?;
+            // FIXME: correctly handle macro expansion
+            if source.file_id != file_id {
+                return None;
+            }
+            let root = source.file_syntax(db.upcast());
+            let node = source.value.to_node(&root);
+            Some((node.syntax().text_range(), scope))
+        })
+        .filter(|(range, _)| {
+            range.start() <= offset && range.is_subrange(&expr_range) && *range != expr_range
+        });
+
+    child_scopes
+        .max_by(|(r1, _), (r2, _)| {
+            if r2.is_subrange(&r1) {
+                std::cmp::Ordering::Greater
+            } else if r1.is_subrange(&r2) {
+                std::cmp::Ordering::Less
+            } else {
+                r1.start().cmp(&r2.start())
+            }
+        })
+        .map(|(_ptr, scope)| *scope)
+}
+
 pub(crate) fn resolve_hir_path(
     db: &dyn HirDatabase,
     resolver: &Resolver,
@@ -376,41 +419,3 @@ pub(crate) fn resolve_hir_path(
             .map(|def| PathResolution::Macro(def.into()))
     })
 }
-
-// XXX: during completion, cursor might be outside of any particular
-// expression. Try to figure out the correct scope...
-fn adjust(
-    scopes: &ExprScopes,
-    source_map: &BodySourceMap,
-    ptr: SyntaxNodePtr,
-    file_id: HirFileId,
-    offset: TextUnit,
-) -> Option<ScopeId> {
-    let r = ptr.range();
-    let child_scopes = scopes
-        .scope_by_expr()
-        .iter()
-        .filter_map(|(id, scope)| {
-            let source = source_map.expr_syntax(*id).ok()?;
-            // FIXME: correctly handle macro expansion
-            if source.file_id != file_id {
-                return None;
-            }
-            let syntax_node_ptr = source.value.syntax_node_ptr();
-            Some((syntax_node_ptr, scope))
-        })
-        .map(|(ptr, scope)| (ptr.range(), scope))
-        .filter(|(range, _)| range.start() <= offset && range.is_subrange(&r) && *range != r);
-
-    child_scopes
-        .max_by(|(r1, _), (r2, _)| {
-            if r2.is_subrange(&r1) {
-                std::cmp::Ordering::Greater
-            } else if r1.is_subrange(&r2) {
-                std::cmp::Ordering::Less
-            } else {
-                r1.start().cmp(&r2.start())
-            }
-        })
-        .map(|(_ptr, scope)| *scope)
-}