Merge #5564

5564: SSR: Restrict to current selection if any r=davidlattimore a=davidlattimore

The selection is also used to avoid unnecessary work, but only to the file level. Further restricting unnecessary work is left for later.

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

Author: bors[bot]

crates/ra_ide/src/lib.rs

@@ -510,9 +510,10 @@ impl Analysis {
         query: &str,
         parse_only: bool,
         position: FilePosition,
+        selections: Vec<FileRange>,
     ) -> Cancelable<Result<SourceChange, SsrError>> {
         self.with_db(|db| {
-            let edits = ssr::parse_search_replace(query, parse_only, db, position)?;
+            let edits = ssr::parse_search_replace(query, parse_only, db, position, selections)?;
             Ok(SourceChange::from(edits))
         })
     }

crates/ra_ide/src/ssr.rs

@@ -1,4 +1,4 @@
-use ra_db::FilePosition;
+use ra_db::{FilePosition, FileRange};
 use ra_ide_db::RootDatabase;
 
 use crate::SourceFileEdit;
@@ -24,6 +24,9 @@ use ra_ssr::{MatchFinder, SsrError, SsrRule};
 // Method calls should generally be written in UFCS form. e.g. `foo::Bar::baz($s, $a)` will match
 // `$s.baz($a)`, provided the method call `baz` resolves to the method `foo::Bar::baz`.
 //
+// The scope of the search / replace will be restricted to the current selection if any, otherwise
+// it will apply to the whole workspace.
+//
 // Placeholders may be given constraints by writing them as `${<name>:<constraint1>:<constraint2>...}`.
 //
 // Supported constraints:
@@ -56,10 +59,11 @@ pub fn parse_search_replace(
     rule: &str,
     parse_only: bool,
     db: &RootDatabase,
-    position: FilePosition,
+    resolve_context: FilePosition,
+    selections: Vec<FileRange>,
 ) -> Result<Vec<SourceFileEdit>, SsrError> {
     let rule: SsrRule = rule.parse()?;
-    let mut match_finder = MatchFinder::in_context(db, position);
+    let mut match_finder = MatchFinder::in_context(db, resolve_context, selections);
     match_finder.add_rule(rule)?;
     if parse_only {
         return Ok(Vec::new());

crates/ra_ssr/src/lib.rs

@@ -52,6 +52,7 @@ pub struct MatchFinder<'db> {
     sema: Semantics<'db, ra_ide_db::RootDatabase>,
     rules: Vec<ResolvedRule>,
     resolution_scope: resolving::ResolutionScope<'db>,
+    restrict_ranges: Vec<FileRange>,
 }
 
 impl<'db> MatchFinder<'db> {
@@ -60,10 +61,17 @@ impl<'db> MatchFinder<'db> {
     pub fn in_context(
         db: &'db ra_ide_db::RootDatabase,
         lookup_context: FilePosition,
+        mut restrict_ranges: Vec<FileRange>,
     ) -> 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 }
+        MatchFinder {
+            sema: Semantics::new(db),
+            rules: Vec::new(),
+            resolution_scope,
+            restrict_ranges,
+        }
     }
 
     /// Constructs an instance using the start of the first file in `db` as the lookup context.
@@ -79,6 +87,7 @@ impl<'db> MatchFinder<'db> {
             Ok(MatchFinder::in_context(
                 db,
                 FilePosition { file_id: first_file_id, offset: 0.into() },
+                vec![],
             ))
         } else {
             bail!("No files to search");

crates/ra_ssr/src/matching.rs

@@ -706,8 +706,8 @@ mod tests {
         let rule: SsrRule = "foo($x) ==>> bar($x)".parse().unwrap();
         let input = "fn foo() {} fn bar() {} fn main() { foo(1+2); }";
 
-        let (db, position) = crate::tests::single_file(input);
-        let mut match_finder = MatchFinder::in_context(&db, position);
+        let (db, position, selections) = crate::tests::single_file(input);
+        let mut match_finder = MatchFinder::in_context(&db, position, selections);
         match_finder.add_rule(rule).unwrap();
         let matches = match_finder.matches();
         assert_eq!(matches.matches.len(), 1);

crates/ra_ssr/src/resolving.rs

@@ -141,22 +141,22 @@ impl Resolver<'_, '_> {
 impl<'db> ResolutionScope<'db> {
     pub(crate) fn new(
         sema: &hir::Semantics<'db, ra_ide_db::RootDatabase>,
-        lookup_context: FilePosition,
+        resolve_context: FilePosition,
     ) -> ResolutionScope<'db> {
         use ra_syntax::ast::AstNode;
-        let file = sema.parse(lookup_context.file_id);
+        let file = sema.parse(resolve_context.file_id);
         // Find a node at the requested position, falling back to the whole file.
         let node = file
             .syntax()
-            .token_at_offset(lookup_context.offset)
+            .token_at_offset(resolve_context.offset)
             .left_biased()
             .map(|token| token.parent())
             .unwrap_or_else(|| file.syntax().clone());
         let node = pick_node_for_resolution(node);
         let scope = sema.scope(&node);
         ResolutionScope {
             scope,
-            hygiene: hir::Hygiene::new(sema.db, lookup_context.file_id.into()),
+            hygiene: hir::Hygiene::new(sema.db, resolve_context.file_id.into()),
         }
     }
 

crates/ra_ssr/src/search.rs

@@ -5,12 +5,13 @@ use crate::{
     resolving::{ResolvedPath, ResolvedPattern, ResolvedRule},
     Match, MatchFinder,
 };
-use ra_db::FileRange;
+use ra_db::{FileId, FileRange};
 use ra_ide_db::{
     defs::Definition,
     search::{Reference, SearchScope},
 };
 use ra_syntax::{ast, AstNode, SyntaxKind, SyntaxNode};
+use rustc_hash::FxHashSet;
 use test_utils::mark;
 
 /// A cache for the results of find_usages. This is for when we have multiple patterns that have the
@@ -54,11 +55,7 @@ impl<'db> MatchFinder<'db> {
                         mark::hit!(use_declaration_with_braces);
                         continue;
                     }
-                    if let Ok(m) =
-                        matching::get_match(false, rule, &node_to_match, &None, &self.sema)
-                    {
-                        matches_out.push(m);
-                    }
+                    self.try_add_match(rule, &node_to_match, &None, matches_out);
                 }
             }
         }
@@ -121,25 +118,39 @@ impl<'db> MatchFinder<'db> {
         // FIXME: We should ideally have a test that checks that we edit local roots and not library
         // roots. This probably would require some changes to fixtures, since currently everything
         // seems to get put into a single source root.
-        use ra_db::SourceDatabaseExt;
-        use ra_ide_db::symbol_index::SymbolsDatabase;
         let mut files = Vec::new();
-        for &root in self.sema.db.local_roots().iter() {
-            let sr = self.sema.db.source_root(root);
-            files.extend(sr.iter());
-        }
+        self.search_files_do(|file_id| {
+            files.push(file_id);
+        });
         SearchScope::files(&files)
     }
 
     fn slow_scan(&self, rule: &ResolvedRule, matches_out: &mut Vec<Match>) {
-        use ra_db::SourceDatabaseExt;
-        use ra_ide_db::symbol_index::SymbolsDatabase;
-        for &root in self.sema.db.local_roots().iter() {
-            let sr = self.sema.db.source_root(root);
-            for file_id in sr.iter() {
-                let file = self.sema.parse(file_id);
-                let code = file.syntax();
-                self.slow_scan_node(code, rule, &None, matches_out);
+        self.search_files_do(|file_id| {
+            let file = self.sema.parse(file_id);
+            let code = file.syntax();
+            self.slow_scan_node(code, rule, &None, matches_out);
+        })
+    }
+
+    fn search_files_do(&self, mut callback: impl FnMut(FileId)) {
+        if self.restrict_ranges.is_empty() {
+            // Unrestricted search.
+            use ra_db::SourceDatabaseExt;
+            use ra_ide_db::symbol_index::SymbolsDatabase;
+            for &root in self.sema.db.local_roots().iter() {
+                let sr = self.sema.db.source_root(root);
+                for file_id in sr.iter() {
+                    callback(file_id);
+                }
+            }
+        } else {
+            // Search is restricted, deduplicate file IDs (generally only one).
+            let mut files = FxHashSet::default();
+            for range in &self.restrict_ranges {
+                if files.insert(range.file_id) {
+                    callback(range.file_id);
+                }
             }
         }
     }
@@ -154,9 +165,7 @@ impl<'db> MatchFinder<'db> {
         if !is_search_permitted(code) {
             return;
         }
-        if let Ok(m) = matching::get_match(false, rule, &code, restrict_range, &self.sema) {
-            matches_out.push(m);
-        }
+        self.try_add_match(rule, &code, restrict_range, matches_out);
         // If we've got a macro call, we already tried matching it pre-expansion, which is the only
         // way to match the whole macro, now try expanding it and matching the expansion.
         if let Some(macro_call) = ast::MacroCall::cast(code.clone()) {
@@ -178,6 +187,38 @@ impl<'db> MatchFinder<'db> {
             self.slow_scan_node(&child, rule, restrict_range, matches_out);
         }
     }
+
+    fn try_add_match(
+        &self,
+        rule: &ResolvedRule,
+        code: &SyntaxNode,
+        restrict_range: &Option<FileRange>,
+        matches_out: &mut Vec<Match>,
+    ) {
+        if !self.within_range_restrictions(code) {
+            mark::hit!(replace_nonpath_within_selection);
+            return;
+        }
+        if let Ok(m) = matching::get_match(false, rule, code, restrict_range, &self.sema) {
+            matches_out.push(m);
+        }
+    }
+
+    /// Returns whether `code` is within one of our range restrictions if we have any. No range
+    /// restrictions is considered unrestricted and always returns true.
+    fn within_range_restrictions(&self, code: &SyntaxNode) -> bool {
+        if self.restrict_ranges.is_empty() {
+            // There is no range restriction.
+            return true;
+        }
+        let node_range = self.sema.original_range(code);
+        for range in &self.restrict_ranges {
+            if range.file_id == node_range.file_id && range.range.contains_range(node_range.range) {
+                return true;
+            }
+        }
+        false
+    }
 }
 
 /// Returns whether we support matching within `node` and all of its ancestors.