Merge remote-tracking branch 'origin/master'

7 files changed, 403 insertions, 80 deletions

diff --git a/crates/ra_ssr/src/lib.rs b/crates/ra_ssr/src/lib.rs
index 7014a6ac6..c780b460a 100644
--- a/crates/ra_ssr/src/lib.rs
+++ b/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,12 @@ 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, 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 +82,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");
diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs
index 4862622bd..0f72fea69 100644
--- a/crates/ra_ssr/src/matching.rs
+++ b/crates/ra_ssr/src/matching.rs
@@ -209,7 +209,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> {
         // Some kinds of nodes have special handling. For everything else, we fall back to default
         // matching.
         match code.kind() {
-            SyntaxKind::RECORD_FIELD_LIST => {
+            SyntaxKind::RECORD_EXPR_FIELD_LIST => {
                 self.attempt_match_record_field_list(phase, pattern, code)
             }
             SyntaxKind::TOKEN_TREE => self.attempt_match_token_tree(phase, pattern, code),
@@ -348,8 +348,8 @@ impl<'db, 'sema> Matcher<'db, 'sema> {
                 // separately via comparing what the path resolves to below.
                 self.attempt_match_opt(
                     phase,
-                    pattern_segment.type_arg_list(),
-                    code_segment.type_arg_list(),
+                    pattern_segment.generic_arg_list(),
+                    code_segment.generic_arg_list(),
                 )?;
                 self.attempt_match_opt(
                     phase,
@@ -399,7 +399,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> {
         // Build a map keyed by field name.
         let mut fields_by_name = FxHashMap::default();
         for child in code.children() {
-            if let Some(record) = ast::RecordField::cast(child.clone()) {
+            if let Some(record) = ast::RecordExprField::cast(child.clone()) {
                 if let Some(name) = record.field_name() {
                     fields_by_name.insert(name.text().clone(), child.clone());
                 }
@@ -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);
diff --git a/crates/ra_ssr/src/parsing.rs b/crates/ra_ssr/src/parsing.rs
index 2d6f4e514..f455eb5b7 100644
--- a/crates/ra_ssr/src/parsing.rs
+++ b/crates/ra_ssr/src/parsing.rs
@@ -10,6 +10,7 @@ use crate::{SsrError, SsrPattern, SsrRule};
 use ra_syntax::{ast, AstNode, SmolStr, SyntaxKind, SyntaxNode, T};
 use rustc_hash::{FxHashMap, FxHashSet};
 use std::str::FromStr;
+use test_utils::mark;
 
 #[derive(Debug)]
 pub(crate) struct ParsedRule {
@@ -69,11 +70,8 @@ impl ParsedRule {
             rules: Vec::new(),
         };
         builder.try_add(ast::Expr::parse(&raw_pattern), raw_template.map(ast::Expr::parse));
-        builder.try_add(ast::TypeRef::parse(&raw_pattern), raw_template.map(ast::TypeRef::parse));
-        builder.try_add(
-            ast::ModuleItem::parse(&raw_pattern),
-            raw_template.map(ast::ModuleItem::parse),
-        );
+        builder.try_add(ast::Type::parse(&raw_pattern), raw_template.map(ast::Type::parse));
+        builder.try_add(ast::Item::parse(&raw_pattern), raw_template.map(ast::Item::parse));
         builder.try_add(ast::Path::parse(&raw_pattern), raw_template.map(ast::Path::parse));
         builder.try_add(ast::Pat::parse(&raw_pattern), raw_template.map(ast::Pat::parse));
         builder.build()
@@ -102,14 +100,35 @@ impl RuleBuilder {
         }
     }
 
-    fn build(self) -> Result<Vec<ParsedRule>, SsrError> {
+    fn build(mut self) -> Result<Vec<ParsedRule>, SsrError> {
         if self.rules.is_empty() {
             bail!("Not a valid Rust expression, type, item, path or pattern");
         }
+        // If any rules contain paths, then we reject any rules that don't contain paths. Allowing a
+        // mix leads to strange semantics, since the path-based rules only match things where the
+        // path refers to semantically the same thing, whereas the non-path-based rules could match
+        // anything. Specifically, if we have a rule like `foo ==>> bar` we only want to match the
+        // `foo` that is in the current scope, not any `foo`. However "foo" can be parsed as a
+        // pattern (IDENT_PAT -> NAME -> IDENT). Allowing such a rule through would result in
+        // renaming everything called `foo` to `bar`. It'd also be slow, since without a path, we'd
+        // have to use the slow-scan search mechanism.
+        if self.rules.iter().any(|rule| contains_path(&rule.pattern)) {
+            let old_len = self.rules.len();
+            self.rules.retain(|rule| contains_path(&rule.pattern));
+            if self.rules.len() < old_len {
+                mark::hit!(pattern_is_a_single_segment_path);
+            }
+        }
         Ok(self.rules)
     }
 }
 
+/// Returns whether there are any paths in `node`.
+fn contains_path(node: &SyntaxNode) -> bool {
+    node.kind() == SyntaxKind::PATH
+        || node.descendants().any(|node| node.kind() == SyntaxKind::PATH)
+}
+
 impl FromStr for SsrRule {
     type Err = SsrError;
 
diff --git a/crates/ra_ssr/src/replacing.rs b/crates/ra_ssr/src/replacing.rs
index 4b3f5509c..0943244ff 100644
--- a/crates/ra_ssr/src/replacing.rs
+++ b/crates/ra_ssr/src/replacing.rs
@@ -3,8 +3,9 @@
 use crate::matching::Var;
 use crate::{resolving::ResolvedRule, Match, SsrMatches};
 use ra_syntax::ast::{self, AstToken};
-use ra_syntax::{SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextSize};
+use ra_syntax::{SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize};
 use ra_text_edit::TextEdit;
+use rustc_hash::{FxHashMap, FxHashSet};
 
 /// Returns a text edit that will replace each match in `matches` with its corresponding replacement
 /// template. Placeholders in the template will have been substituted with whatever they matched to
@@ -38,62 +39,79 @@ struct ReplacementRenderer<'a> {
     file_src: &'a str,
     rules: &'a [ResolvedRule],
     rule: &'a ResolvedRule,
+    out: String,
+    // Map from a range within `out` to a token in `template` that represents a placeholder. This is
+    // used to validate that the generated source code doesn't split any placeholder expansions (see
+    // below).
+    placeholder_tokens_by_range: FxHashMap<TextRange, SyntaxToken>,
+    // Which placeholder tokens need to be wrapped in parenthesis in order to ensure that when `out`
+    // is parsed, placeholders don't get split. e.g. if a template of `$a.to_string()` results in `1
+    // + 2.to_string()` then the placeholder value `1 + 2` was split and needs parenthesis.
+    placeholder_tokens_requiring_parenthesis: FxHashSet<SyntaxToken>,
 }
 
 fn render_replace(match_info: &Match, file_src: &str, rules: &[ResolvedRule]) -> String {
-    let mut out = String::new();
     let rule = &rules[match_info.rule_index];
     let template = rule
         .template
         .as_ref()
         .expect("You called MatchFinder::edits after calling MatchFinder::add_search_pattern");
-    let renderer = ReplacementRenderer { match_info, file_src, rules, rule };
-    renderer.render_node(&template.node, &mut out);
+    let mut renderer = ReplacementRenderer {
+        match_info,
+        file_src,
+        rules,
+        rule,
+        out: String::new(),
+        placeholder_tokens_requiring_parenthesis: FxHashSet::default(),
+        placeholder_tokens_by_range: FxHashMap::default(),
+    };
+    renderer.render_node(&template.node);
+    renderer.maybe_rerender_with_extra_parenthesis(&template.node);
     for comment in &match_info.ignored_comments {
-        out.push_str(&comment.syntax().to_string());
+        renderer.out.push_str(&comment.syntax().to_string());
     }
-    out
+    renderer.out
 }
 
 impl ReplacementRenderer<'_> {
-    fn render_node_children(&self, node: &SyntaxNode, out: &mut String) {
+    fn render_node_children(&mut self, node: &SyntaxNode) {
         for node_or_token in node.children_with_tokens() {
-            self.render_node_or_token(&node_or_token, out);
+            self.render_node_or_token(&node_or_token);
         }
     }
 
-    fn render_node_or_token(&self, node_or_token: &SyntaxElement, out: &mut String) {
+    fn render_node_or_token(&mut self, node_or_token: &SyntaxElement) {
         match node_or_token {
             SyntaxElement::Token(token) => {
-                self.render_token(&token, out);
+                self.render_token(&token);
             }
             SyntaxElement::Node(child_node) => {
-                self.render_node(&child_node, out);
+                self.render_node(&child_node);
             }
         }
     }
 
-    fn render_node(&self, node: &SyntaxNode, out: &mut String) {
+    fn render_node(&mut self, node: &SyntaxNode) {
         use ra_syntax::ast::AstNode;
         if let Some(mod_path) = self.match_info.rendered_template_paths.get(&node) {
-            out.push_str(&mod_path.to_string());
+            self.out.push_str(&mod_path.to_string());
             // Emit everything except for the segment's name-ref, since we already effectively
             // emitted that as part of `mod_path`.
             if let Some(path) = ast::Path::cast(node.clone()) {
                 if let Some(segment) = path.segment() {
                     for node_or_token in segment.syntax().children_with_tokens() {
                         if node_or_token.kind() != SyntaxKind::NAME_REF {
-                            self.render_node_or_token(&node_or_token, out);
+                            self.render_node_or_token(&node_or_token);
                         }
                     }
                 }
             }
         } else {
-            self.render_node_children(&node, out);
+            self.render_node_children(&node);
         }
     }
 
-    fn render_token(&self, token: &SyntaxToken, out: &mut String) {
+    fn render_token(&mut self, token: &SyntaxToken) {
         if let Some(placeholder) = self.rule.get_placeholder(&token) {
             if let Some(placeholder_value) =
                 self.match_info.placeholder_values.get(&Var(placeholder.ident.to_string()))
@@ -107,8 +125,23 @@ impl ReplacementRenderer<'_> {
                     range.start(),
                     self.rules,
                 );
+                let needs_parenthesis =
+                    self.placeholder_tokens_requiring_parenthesis.contains(token);
                 edit.apply(&mut matched_text);
-                out.push_str(&matched_text);
+                if needs_parenthesis {
+                    self.out.push('(');
+                }
+                self.placeholder_tokens_by_range.insert(
+                    TextRange::new(
+                        TextSize::of(&self.out),
+                        TextSize::of(&self.out) + TextSize::of(&matched_text),
+                    ),
+                    token.clone(),
+                );
+                self.out.push_str(&matched_text);
+                if needs_parenthesis {
+                    self.out.push(')');
+                }
             } else {
                 // We validated that all placeholder references were valid before we
                 // started, so this shouldn't happen.
@@ -118,7 +151,44 @@ impl ReplacementRenderer<'_> {
                 );
             }
         } else {
-            out.push_str(token.text().as_str());
+            self.out.push_str(token.text().as_str());
+        }
+    }
+
+    // Checks if the resulting code, when parsed doesn't split any placeholders due to different
+    // order of operations between the search pattern and the replacement template. If any do, then
+    // we rerender the template and wrap the problematic placeholders with parenthesis.
+    fn maybe_rerender_with_extra_parenthesis(&mut self, template: &SyntaxNode) {
+        if let Some(node) = parse_as_kind(&self.out, template.kind()) {
+            self.remove_node_ranges(node);
+            if self.placeholder_tokens_by_range.is_empty() {
+                return;
+            }
+            self.placeholder_tokens_requiring_parenthesis =
+                self.placeholder_tokens_by_range.values().cloned().collect();
+            self.out.clear();
+            self.render_node(template);
+        }
+    }
+
+    fn remove_node_ranges(&mut self, node: SyntaxNode) {
+        self.placeholder_tokens_by_range.remove(&node.text_range());
+        for child in node.children() {
+            self.remove_node_ranges(child);
+        }
+    }
+}
+
+fn parse_as_kind(code: &str, kind: SyntaxKind) -> Option<SyntaxNode> {
+    use ra_syntax::ast::AstNode;
+    if ast::Expr::can_cast(kind) {
+        if let Ok(expr) = ast::Expr::parse(code) {
+            return Some(expr.syntax().clone());
+        }
+    } else if ast::Item::can_cast(kind) {
+        if let Ok(item) = ast::Item::parse(code) {
+            return Some(item.syntax().clone());
         }
     }
+    None
 }
diff --git a/crates/ra_ssr/src/resolving.rs b/crates/ra_ssr/src/resolving.rs
index 123bd2bb2..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`.
@@ -141,14 +154,14 @@ 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());
@@ -156,10 +169,16 @@ impl<'db> ResolutionScope<'db> {
         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()),
+            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
@@ -198,7 +221,7 @@ fn pick_node_for_resolution(node: SyntaxNode) -> SyntaxNode {
                 return n;
             }
         }
-        SyntaxKind::LET_STMT | SyntaxKind::BIND_PAT => {
+        SyntaxKind::LET_STMT | SyntaxKind::IDENT_PAT => {
             if let Some(next) = node.next_sibling() {
                 return pick_node_for_resolution(next);
             }
@@ -217,7 +240,7 @@ fn pick_node_for_resolution(node: SyntaxNode) -> SyntaxNode {
 fn path_contains_type_arguments(path: Option<ast::Path>) -> bool {
     if let Some(path) = path {
         if let Some(segment) = path.segment() {
-            if segment.type_arg_list().is_some() {
+            if segment.generic_arg_list().is_some() {
                 mark::hit!(type_arguments_within_path);
                 return true;
             }
diff --git a/crates/ra_ssr/src/search.rs b/crates/ra_ssr/src/search.rs
index bcf0f0468..85ffa2ac2 100644
--- a/crates/ra_ssr/src/search.rs
+++ b/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
@@ -32,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;
@@ -54,11 +64,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 +127,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 +174,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 +196,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.
@@ -196,7 +246,7 @@ fn is_search_permitted(node: &SyntaxNode) -> bool {
     // and the code is `use foo::{baz, bar}`, we'll match `bar`, since it resolves to `foo::bar`.
     // However we'll then replace just the part we matched `bar`. We probably need to instead remove
     // `bar` and insert a new use declaration.
-    node.kind() != SyntaxKind::USE_ITEM
+    node.kind() != SyntaxKind::USE
 }
 
 impl UsageCache {
diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs
index 18ef2506a..d483640df 100644
--- a/crates/ra_ssr/src/tests.rs
+++ b/crates/ra_ssr/src/tests.rs
@@ -1,9 +1,9 @@
 use crate::{MatchFinder, SsrRule};
 use expect::{expect, Expect};
-use ra_db::{salsa::Durability, FileId, FilePosition, SourceDatabaseExt};
+use ra_db::{salsa::Durability, FileId, FilePosition, FileRange, SourceDatabaseExt};
 use rustc_hash::FxHashSet;
 use std::sync::Arc;
-use test_utils::mark;
+use test_utils::{mark, RangeOrOffset};
 
 fn parse_error_text(query: &str) -> String {
     format!("{}", query.parse::<SsrRule>().unwrap_err())
@@ -60,20 +60,32 @@ fn parser_undefined_placeholder_in_replacement() {
 }
 
 /// `code` may optionally contain a cursor marker `<|>`. If it doesn't, then the position will be
-/// the start of the file.
-pub(crate) fn single_file(code: &str) -> (ra_ide_db::RootDatabase, FilePosition) {
+/// the start of the file. If there's a second cursor marker, then we'll return a single range.
+pub(crate) fn single_file(code: &str) -> (ra_ide_db::RootDatabase, FilePosition, Vec<FileRange>) {
     use ra_db::fixture::WithFixture;
     use ra_ide_db::symbol_index::SymbolsDatabase;
-    let (mut db, position) = if code.contains(test_utils::CURSOR_MARKER) {
-        ra_ide_db::RootDatabase::with_position(code)
+    let (mut db, file_id, range_or_offset) = if code.contains(test_utils::CURSOR_MARKER) {
+        ra_ide_db::RootDatabase::with_range_or_offset(code)
     } else {
         let (db, file_id) = ra_ide_db::RootDatabase::with_single_file(code);
-        (db, FilePosition { file_id, offset: 0.into() })
+        (db, file_id, RangeOrOffset::Offset(0.into()))
     };
+    let selections;
+    let position;
+    match range_or_offset {
+        RangeOrOffset::Range(range) => {
+            position = FilePosition { file_id, offset: range.start() };
+            selections = vec![FileRange { file_id, range: range }];
+        }
+        RangeOrOffset::Offset(offset) => {
+            position = FilePosition { file_id, offset };
+            selections = vec![];
+        }
+    }
     let mut local_roots = FxHashSet::default();
     local_roots.insert(ra_db::fixture::WORKSPACE);
     db.set_local_roots_with_durability(Arc::new(local_roots), Durability::HIGH);
-    (db, position)
+    (db, position, selections)
 }
 
 fn assert_ssr_transform(rule: &str, input: &str, expected: Expect) {
@@ -81,8 +93,8 @@ fn assert_ssr_transform(rule: &str, input: &str, expected: Expect) {
 }
 
 fn assert_ssr_transforms(rules: &[&str], input: &str, expected: Expect) {
-    let (db, position) = single_file(input);
-    let mut match_finder = MatchFinder::in_context(&db, position);
+    let (db, position, selections) = single_file(input);
+    let mut match_finder = MatchFinder::in_context(&db, position, selections);
     for rule in rules {
         let rule: SsrRule = rule.parse().unwrap();
         match_finder.add_rule(rule).unwrap();
@@ -112,8 +124,8 @@ fn print_match_debug_info(match_finder: &MatchFinder, file_id: FileId, snippet:
 }
 
 fn assert_matches(pattern: &str, code: &str, expected: &[&str]) {
-    let (db, position) = single_file(code);
-    let mut match_finder = MatchFinder::in_context(&db, position);
+    let (db, position, selections) = single_file(code);
+    let mut match_finder = MatchFinder::in_context(&db, position, selections);
     match_finder.add_search_pattern(pattern.parse().unwrap()).unwrap();
     let matched_strings: Vec<String> =
         match_finder.matches().flattened().matches.iter().map(|m| m.matched_text()).collect();
@@ -124,8 +136,8 @@ fn assert_matches(pattern: &str, code: &str, expected: &[&str]) {
 }
 
 fn assert_no_match(pattern: &str, code: &str) {
-    let (db, position) = single_file(code);
-    let mut match_finder = MatchFinder::in_context(&db, position);
+    let (db, position, selections) = single_file(code);
+    let mut match_finder = MatchFinder::in_context(&db, position, selections);
     match_finder.add_search_pattern(pattern.parse().unwrap()).unwrap();
     let matches = match_finder.matches().flattened().matches;
     if !matches.is_empty() {
@@ -135,8 +147,8 @@ fn assert_no_match(pattern: &str, code: &str) {
 }
 
 fn assert_match_failure_reason(pattern: &str, code: &str, snippet: &str, expected_reason: &str) {
-    let (db, position) = single_file(code);
-    let mut match_finder = MatchFinder::in_context(&db, position);
+    let (db, position, selections) = single_file(code);
+    let mut match_finder = MatchFinder::in_context(&db, position, selections);
     match_finder.add_search_pattern(pattern.parse().unwrap()).unwrap();
     let mut reasons = Vec::new();
     for d in match_finder.debug_where_text_equal(position.file_id, snippet) {
@@ -490,9 +502,10 @@ fn no_match_split_expression() {
 
 #[test]
 fn replace_function_call() {
+    // This test also makes sure that we ignore empty-ranges.
     assert_ssr_transform(
         "foo() ==>> bar()",
-        "fn foo() {} fn bar() {} fn f1() {foo(); foo();}",
+        "fn foo() {<|><|>} fn bar() {} fn f1() {foo(); foo();}",
         expect![["fn foo() {} fn bar() {} fn f1() {bar(); bar();}"]],
     );
 }
@@ -651,7 +664,7 @@ fn replace_binary_op() {
     assert_ssr_transform(
         "$a + $b ==>> $b + $a",
         "fn f() {1 + 2 + 3 + 4}",
-        expect![["fn f() {4 + 3 + 2 + 1}"]],
+        expect![[r#"fn f() {4 + (3 + (2 + 1))}"#]],
     );
 }
 
@@ -760,12 +773,33 @@ fn preserves_whitespace_within_macro_expansion() {
             macro_rules! macro1 {
                 ($a:expr) => {$a}
             }
-            fn f() {macro1!(4 - 3 - 1   *   2}
+            fn f() {macro1!(4 - (3 - 1   *   2)}
             "#]],
     )
 }
 
 #[test]
+fn add_parenthesis_when_necessary() {
+    assert_ssr_transform(
+        "foo($a) ==>> $a.to_string()",
+        r#"
+        fn foo(_: i32) {}
+        fn bar3(v: i32) {
+            foo(1 + 2);
+            foo(-v);
+        }
+        "#,
+        expect![[r#"
+            fn foo(_: i32) {}
+            fn bar3(v: i32) {
+                (1 + 2).to_string();
+                (-v).to_string();
+            }
+        "#]],
+    )
+}
+
+#[test]
 fn match_failure_reasons() {
     let code = r#"
         fn bar() {}
@@ -887,6 +921,45 @@ fn ufcs_matches_method_call() {
 }
 
 #[test]
+fn pattern_is_a_single_segment_path() {
+    mark::check!(pattern_is_a_single_segment_path);
+    // The first function should not be altered because the `foo` in scope at the cursor position is
+    // a different `foo`. This case is special because "foo" can be parsed as a pattern (IDENT_PAT ->
+    // NAME -> IDENT), which contains no path. If we're not careful we'll end up matching the `foo`
+    // in `let foo` from the first function. Whether we should match the `let foo` in the second
+    // function is less clear. At the moment, we don't. Doing so sounds like a rename operation,
+    // which isn't really what SSR is for, especially since the replacement `bar` must be able to be
+    // resolved, which means if we rename `foo` we'll get a name collision.
+    assert_ssr_transform(
+        "foo ==>> bar",
+        r#"
+        fn f1() -> i32 {
+            let foo = 1;
+            let bar = 2;
+            foo
+        }
+        fn f1() -> i32 {
+            let foo = 1;
+            let bar = 2;
+            foo<|>
+        }
+        "#,
+        expect![[r#"
+            fn f1() -> i32 {
+                let foo = 1;
+                let bar = 2;
+                foo
+            }
+            fn f1() -> i32 {
+                let foo = 1;
+                let bar = 2;
+                bar
+            }
+        "#]],
+    );
+}
+
+#[test]
 fn replace_local_variable_reference() {
     // The pattern references a local variable `foo` in the block containing the cursor. We should
     // only replace references to this variable `foo`, not other variables that just happen to have
@@ -922,3 +995,87 @@ fn replace_local_variable_reference() {
         "#]],
     )
 }
+
+#[test]
+fn replace_path_within_selection() {
+    assert_ssr_transform(
+        "foo ==>> bar",
+        r#"
+        fn main() {
+            let foo = 41;
+            let bar = 42;
+            do_stuff(foo);
+            do_stuff(foo);<|>
+            do_stuff(foo);
+            do_stuff(foo);<|>
+            do_stuff(foo);
+        }"#,
+        expect![[r#"
+            fn main() {
+                let foo = 41;
+                let bar = 42;
+                do_stuff(foo);
+                do_stuff(foo);
+                do_stuff(bar);
+                do_stuff(bar);
+                do_stuff(foo);
+            }"#]],
+    );
+}
+
+#[test]
+fn replace_nonpath_within_selection() {
+    mark::check!(replace_nonpath_within_selection);
+    assert_ssr_transform(
+        "$a + $b ==>> $b * $a",
+        r#"
+        fn main() {
+            let v = 1 + 2;<|>
+            let v2 = 3 + 3;
+            let v3 = 4 + 5;<|>
+            let v4 = 6 + 7;
+        }"#,
+        expect![[r#"
+            fn main() {
+                let v = 1 + 2;
+                let v2 = 3 * 3;
+                let v3 = 5 * 4;
+                let v4 = 6 + 7;
+            }"#]],
+    );
+}
+
+#[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)
+                }
+            }
+        "#]],
+    );
+}