Merge #5639

5639: SSR: Allow `self` in patterns. r=jonas-schievink a=davidlattimore

It's now consistent with other variables in that if the pattern references self, only the `self` in scope where the rule is invoked will be accepted. Since `self` doesn't work the same as other paths, this is implemented by restricting the search to just the current function. Prior to this change (since path resolution was implemented), having self in a pattern would just result in no matches.

Co-authored-by: David Lattimore <[email protected]>

4 files changed, 68 insertions, 6 deletions

diff --git a/crates/ra_ssr/src/lib.rs b/crates/ra_ssr/src/lib.rs
index 73abfecb2..c780b460a 100644
--- a/crates/ra_ssr/src/lib.rs
+++ b/crates/ra_ssr/src/lib.rs
@@ -66,12 +66,7 @@ impl<'db> MatchFinder<'db> {
         restrict_ranges.retain(|range| !range.range.is_empty());
         let sema = Semantics::new(db);
         let resolution_scope = resolving::ResolutionScope::new(&sema, lookup_context);
-        MatchFinder {
-            sema: Semantics::new(db),
-            rules: Vec::new(),
-            resolution_scope,
-            restrict_ranges,
-        }
+        MatchFinder { sema, rules: Vec::new(), resolution_scope, restrict_ranges }
     }
 
     /// Constructs an instance using the start of the first file in `db` as the lookup context.
diff --git a/crates/ra_ssr/src/resolving.rs b/crates/ra_ssr/src/resolving.rs
index 6f62000f4..df60048eb 100644
--- a/crates/ra_ssr/src/resolving.rs
+++ b/crates/ra_ssr/src/resolving.rs
@@ -11,6 +11,7 @@ use test_utils::mark;
 pub(crate) struct ResolutionScope<'db> {
     scope: hir::SemanticsScope<'db>,
     hygiene: hir::Hygiene,
+    node: SyntaxNode,
 }
 
 pub(crate) struct ResolvedRule {
@@ -25,6 +26,7 @@ pub(crate) struct ResolvedPattern {
     // Paths in `node` that we've resolved.
     pub(crate) resolved_paths: FxHashMap<SyntaxNode, ResolvedPath>,
     pub(crate) ufcs_function_calls: FxHashMap<SyntaxNode, hir::Function>,
+    pub(crate) contains_self: bool,
 }
 
 pub(crate) struct ResolvedPath {
@@ -68,6 +70,7 @@ struct Resolver<'a, 'db> {
 
 impl Resolver<'_, '_> {
     fn resolve_pattern_tree(&self, pattern: SyntaxNode) -> Result<ResolvedPattern, SsrError> {
+        use ra_syntax::{SyntaxElement, T};
         let mut resolved_paths = FxHashMap::default();
         self.resolve(pattern.clone(), 0, &mut resolved_paths)?;
         let ufcs_function_calls = resolved_paths
@@ -85,11 +88,17 @@ impl Resolver<'_, '_> {
                 None
             })
             .collect();
+        let contains_self =
+            pattern.descendants_with_tokens().any(|node_or_token| match node_or_token {
+                SyntaxElement::Token(t) => t.kind() == T![self],
+                _ => false,
+            });
         Ok(ResolvedPattern {
             node: pattern,
             resolved_paths,
             placeholders_by_stand_in: self.placeholders_by_stand_in.clone(),
             ufcs_function_calls,
+            contains_self,
         })
     }
 
@@ -101,6 +110,10 @@ impl Resolver<'_, '_> {
     ) -> Result<(), SsrError> {
         use ra_syntax::ast::AstNode;
         if let Some(path) = ast::Path::cast(node.clone()) {
+            if is_self(&path) {
+                // Self cannot be resolved like other paths.
+                return Ok(());
+            }
             // Check if this is an appropriate place in the path to resolve. If the path is
             // something like `a::B::<i32>::c` then we want to resolve `a::B`. If the path contains
             // a placeholder. e.g. `a::$b::c` then we want to resolve `a`.
@@ -157,9 +170,15 @@ impl<'db> ResolutionScope<'db> {
         ResolutionScope {
             scope,
             hygiene: hir::Hygiene::new(sema.db, resolve_context.file_id.into()),
+            node,
         }
     }
 
+    /// Returns the function in which SSR was invoked, if any.
+    pub(crate) fn current_function(&self) -> Option<SyntaxNode> {
+        self.node.ancestors().find(|node| node.kind() == SyntaxKind::FN).map(|node| node.clone())
+    }
+
     fn resolve_path(&self, path: &ast::Path) -> Option<hir::PathResolution> {
         let hir_path = hir::Path::from_src(path.clone(), &self.hygiene)?;
         // First try resolving the whole path. This will work for things like
@@ -186,6 +205,10 @@ impl<'db> ResolutionScope<'db> {
     }
 }
 
+fn is_self(path: &ast::Path) -> bool {
+    path.segment().map(|segment| segment.self_token().is_some()).unwrap_or(false)
+}
+
 /// Returns a suitable node for resolving paths in the current scope. If we create a scope based on
 /// a statement node, then we can't resolve local variables that were defined in the current scope
 /// (only in parent scopes). So we find another node, ideally a child of the statement where local
diff --git a/crates/ra_ssr/src/search.rs b/crates/ra_ssr/src/search.rs
index 213dc494f..85ffa2ac2 100644
--- a/crates/ra_ssr/src/search.rs
+++ b/crates/ra_ssr/src/search.rs
@@ -33,6 +33,15 @@ impl<'db> MatchFinder<'db> {
         usage_cache: &mut UsageCache,
         matches_out: &mut Vec<Match>,
     ) {
+        if rule.pattern.contains_self {
+            // If the pattern contains `self` we restrict the scope of the search to just the
+            // current method. No other method can reference the same `self`. This makes the
+            // behavior of `self` consistent with other variables.
+            if let Some(current_function) = self.resolution_scope.current_function() {
+                self.slow_scan_node(&current_function, rule, &None, matches_out);
+            }
+            return;
+        }
         if pick_path_for_usages(&rule.pattern).is_none() {
             self.slow_scan(rule, matches_out);
             return;
diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs
index 2ae03c64c..d483640df 100644
--- a/crates/ra_ssr/src/tests.rs
+++ b/crates/ra_ssr/src/tests.rs
@@ -1044,3 +1044,38 @@ fn replace_nonpath_within_selection() {
             }"#]],
     );
 }
+
+#[test]
+fn replace_self() {
+    // `foo(self)` occurs twice in the code, however only the first occurrence is the `self` that's
+    // in scope where the rule is invoked.
+    assert_ssr_transform(
+        "foo(self) ==>> bar(self)",
+        r#"
+        struct S1 {}
+        fn foo(_: &S1) {}
+        fn bar(_: &S1) {}
+        impl S1 {
+            fn f1(&self) {
+                foo(self)<|>
+            }
+            fn f2(&self) {
+                foo(self)
+            }
+        }
+        "#,
+        expect![[r#"
+            struct S1 {}
+            fn foo(_: &S1) {}
+            fn bar(_: &S1) {}
+            impl S1 {
+                fn f1(&self) {
+                    bar(self)
+                }
+                fn f2(&self) {
+                    foo(self)
+                }
+            }
+        "#]],
+    );
+}