SSR: Change the way rules are stored internally.

Previously we had:

- Multiple rules
  - Each rule had its pattern parsed as an expression, path etc

This meant that there were two levels at which there could be multiple
rules.

Now we just have multiple rules. If a pattern can parse as more than one
kind of thing, then they get stored as multiple separate rules.

We also now don't have separate fields for the different kinds of things
that a pattern can parse as. This makes adding new kinds of things
simpler.

Previously, add_search_pattern would construct a rule with a dummy
replacement. Now the replacement is an Option. This is slightly cleaner
and also opens the way for parsing the replacement template as the same
kind of thing as the search pattern.

crates/ra_ssr/src/lib.rs

@@ -13,35 +13,27 @@ mod tests;
 
 pub use crate::errors::SsrError;
 pub use crate::matching::Match;
-use crate::matching::{record_match_fails_reasons_scope, MatchFailureReason};
+use crate::matching::MatchFailureReason;
 use hir::Semantics;
+use parsing::SsrTemplate;
 use ra_db::{FileId, FileRange};
-use ra_syntax::{ast, AstNode, SmolStr, SyntaxKind, SyntaxNode, TextRange};
+use ra_syntax::{ast, AstNode, SyntaxNode, TextRange};
 use ra_text_edit::TextEdit;
-use rustc_hash::FxHashMap;
 
 // A structured search replace rule. Create by calling `parse` on a str.
 #[derive(Debug)]
 pub struct SsrRule {
     /// A structured pattern that we're searching for.
-    pattern: SsrPattern,
+    pattern: parsing::RawPattern,
     /// What we'll replace it with.
-    template: parsing::SsrTemplate,
+    template: SsrTemplate,
+    parsed_rules: Vec<parsing::ParsedRule>,
 }
 
 #[derive(Debug)]
 pub struct SsrPattern {
-    raw: parsing::RawSearchPattern,
+    raw: parsing::RawPattern,
-    /// Placeholders keyed by the stand-in ident that we use in Rust source code.
+    parsed_rules: Vec<parsing::ParsedRule>,
-    placeholders_by_stand_in: FxHashMap<SmolStr, parsing::Placeholder>,
-    // We store our search pattern, parsed as each different kind of thing we can look for. As we
-    // traverse the AST, we get the appropriate one of these for the type of node we're on. For many
-    // search patterns, only some of these will be present.
-    expr: Option<SyntaxNode>,
-    type_ref: Option<SyntaxNode>,
-    item: Option<SyntaxNode>,
-    path: Option<SyntaxNode>,
-    pattern: Option<SyntaxNode>,
 }
 
 #[derive(Debug, Default)]
@@ -53,7 +45,7 @@ pub struct SsrMatches {
 pub struct MatchFinder<'db> {
     /// Our source of information about the user's code.
     sema: Semantics<'db, ra_ide_db::RootDatabase>,
-    rules: Vec<SsrRule>,
+    rules: Vec<parsing::ParsedRule>,
 }
 
 impl<'db> MatchFinder<'db> {
@@ -61,14 +53,17 @@ impl<'db> MatchFinder<'db> {
         MatchFinder { sema: Semantics::new(db), rules: Vec::new() }
     }
 
+    /// Adds a rule to be applied. The order in which rules are added matters. Earlier rules take
+    /// precedence. If a node is matched by an earlier rule, then later rules won't be permitted to
+    /// match to it.
     pub fn add_rule(&mut self, rule: SsrRule) {
-        self.rules.push(rule);
+        self.add_parsed_rules(rule.parsed_rules);
     }
 
     /// Adds a search pattern. For use if you intend to only call `find_matches_in_file`. If you
     /// intend to do replacement, use `add_rule` instead.
     pub fn add_search_pattern(&mut self, pattern: SsrPattern) {
-        self.add_rule(SsrRule { pattern, template: "()".parse().unwrap() })
+        self.add_parsed_rules(pattern.parsed_rules);
     }
 
     pub fn edits_for_file(&self, file_id: FileId) -> Option<TextEdit> {
@@ -115,6 +110,14 @@ impl<'db> MatchFinder<'db> {
         res
     }
 
+    fn add_parsed_rules(&mut self, parsed_rules: Vec<parsing::ParsedRule>) {
+        // FIXME: This doesn't need to be a for loop, but does in a subsequent commit. Justify it
+        // being a for-loop.
+        for parsed_rule in parsed_rules {
+            self.rules.push(parsed_rule);
+        }
+    }
+
     fn find_matches(
         &self,
         code: &SyntaxNode,
@@ -177,8 +180,13 @@ impl<'db> MatchFinder<'db> {
             }
             if node_range.range == range.range {
                 for rule in &self.rules {
-                    let pattern =
+                    // For now we ignore rules that have a different kind than our node, otherwise
-                        rule.pattern.tree_for_kind_with_reason(node.kind()).map(|p| p.clone());
+                    // we get lots of noise. If at some point we add support for restricting rules
+                    // to a particular kind of thing (e.g. only match type references), then we can
+                    // relax this.
+                    if rule.pattern.kind() != node.kind() {
+                        continue;
+                    }
                     out.push(MatchDebugInfo {
                         matched: matching::get_match(true, rule, &node, restrict_range, &self.sema)
                             .map_err(|e| MatchFailureReason {
@@ -186,7 +194,7 @@ impl<'db> MatchFinder<'db> {
                                     "Match failed, but no reason was given".to_owned()
                                 }),
                             }),
-                        pattern,
+                        pattern: rule.pattern.clone(),
                         node: node.clone(),
                     });
                 }
@@ -209,9 +217,8 @@ impl<'db> MatchFinder<'db> {
 
 pub struct MatchDebugInfo {
     node: SyntaxNode,
-    /// Our search pattern parsed as the same kind of syntax node as `node`. e.g. expression, item,
+    /// Our search pattern parsed as an expression or item, etc
-    /// etc. Will be absent if the pattern can't be parsed as that kind.
+    pattern: SyntaxNode,
-    pattern: Result<SyntaxNode, MatchFailureReason>,
     matched: Result<Match, MatchFailureReason>,
 }
 
@@ -228,29 +235,12 @@ impl std::fmt::Debug for MatchDebugInfo {
             self.node
         )?;
         writeln!(f, "========= PATTERN ==========")?;
-        match &self.pattern {
+        writeln!(f, "{:#?}", self.pattern)?;
-            Ok(pattern) => {
-                writeln!(f, "{:#?}", pattern)?;
-            }
-            Err(err) => {
-                writeln!(f, "{}", err.reason)?;
-            }
-        }
         writeln!(f, "============================")?;
         Ok(())
     }
 }
 
-impl SsrPattern {
-    fn tree_for_kind_with_reason(
-        &self,
-        kind: SyntaxKind,
-    ) -> Result<&SyntaxNode, MatchFailureReason> {
-        record_match_fails_reasons_scope(true, || self.tree_for_kind(kind))
-            .map_err(|e| MatchFailureReason { reason: e.reason.unwrap() })
-    }
-}
-
 impl SsrMatches {
     /// Returns `self` with any nested matches removed and made into top-level matches.
     pub fn flattened(self) -> SsrMatches {

crates/ra_ssr/src/matching.rs

@@ -2,8 +2,8 @@
 //! process of matching, placeholder values are recorded.
 
 use crate::{
-    parsing::{Constraint, NodeKind, Placeholder, SsrTemplate},
+    parsing::{Constraint, NodeKind, ParsedRule, Placeholder, SsrTemplate},
-    SsrMatches, SsrPattern, SsrRule,
+    SsrMatches,
 };
 use hir::Semantics;
 use ra_db::FileRange;
@@ -50,7 +50,7 @@ pub struct Match {
     pub(crate) ignored_comments: Vec<ast::Comment>,
     // A copy of the template for the rule that produced this match. We store this on the match for
     // if/when we do replacement.
-    pub(crate) template: SsrTemplate,
+    pub(crate) template: Option<SsrTemplate>,
 }
 
 /// Represents a `$var` in an SSR query.
@@ -86,7 +86,7 @@ pub(crate) struct MatchFailed {
 /// parent module, we don't populate nested matches.
 pub(crate) fn get_match(
     debug_active: bool,
-    rule: &SsrRule,
+    rule: &ParsedRule,
     code: &SyntaxNode,
     restrict_range: &Option<FileRange>,
     sema: &Semantics<ra_ide_db::RootDatabase>,
@@ -102,7 +102,7 @@ struct Matcher<'db, 'sema> {
     /// If any placeholders come from anywhere outside of this range, then the match will be
     /// rejected.
     restrict_range: Option<FileRange>,
-    rule: &'sema SsrRule,
+    rule: &'sema ParsedRule,
 }
 
 /// Which phase of matching we're currently performing. We do two phases because most attempted
@@ -117,15 +117,14 @@ enum Phase<'a> {
 
 impl<'db, 'sema> Matcher<'db, 'sema> {
     fn try_match(
-        rule: &'sema SsrRule,
+        rule: &ParsedRule,
         code: &SyntaxNode,
         restrict_range: &Option<FileRange>,
         sema: &'sema Semantics<'db, ra_ide_db::RootDatabase>,
     ) -> Result<Match, MatchFailed> {
         let match_state = Matcher { sema, restrict_range: restrict_range.clone(), rule };
-        let pattern_tree = rule.pattern.tree_for_kind(code.kind())?;
         // First pass at matching, where we check that node types and idents match.
-        match_state.attempt_match_node(&mut Phase::First, &pattern_tree, code)?;
+        match_state.attempt_match_node(&mut Phase::First, &rule.pattern, code)?;
         match_state.validate_range(&sema.original_range(code))?;
         let mut the_match = Match {
             range: sema.original_range(code),
@@ -136,7 +135,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> {
         };
         // Second matching pass, where we record placeholder matches, ignored comments and maybe do
         // any other more expensive checks that we didn't want to do on the first pass.
-        match_state.attempt_match_node(&mut Phase::Second(&mut the_match), &pattern_tree, code)?;
+        match_state.attempt_match_node(&mut Phase::Second(&mut the_match), &rule.pattern, code)?;
         Ok(the_match)
     }
 
@@ -444,8 +443,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> {
     }
 
     fn get_placeholder(&self, element: &SyntaxElement) -> Option<&Placeholder> {
-        only_ident(element.clone())
+        only_ident(element.clone()).and_then(|ident| self.rule.get_placeholder(&ident))
-            .and_then(|ident| self.rule.pattern.placeholders_by_stand_in.get(ident.text()))
     }
 }
 
@@ -510,28 +508,6 @@ impl PlaceholderMatch {
     }
 }
 
-impl SsrPattern {
-    pub(crate) fn tree_for_kind(&self, kind: SyntaxKind) -> Result<&SyntaxNode, MatchFailed> {
-        let (tree, kind_name) = if ast::Expr::can_cast(kind) {
-            (&self.expr, "expression")
-        } else if ast::TypeRef::can_cast(kind) {
-            (&self.type_ref, "type reference")
-        } else if ast::ModuleItem::can_cast(kind) {
-            (&self.item, "item")
-        } else if ast::Path::can_cast(kind) {
-            (&self.path, "path")
-        } else if ast::Pat::can_cast(kind) {
-            (&self.pattern, "pattern")
-        } else {
-            fail_match!("Matching nodes of kind {:?} is not supported", kind);
-        };
-        match tree {
-            Some(tree) => Ok(tree),
-            None => fail_match!("Pattern cannot be parsed as a {}", kind_name),
-        }
-    }
-}
-
 impl NodeKind {
     fn matches(&self, node: &SyntaxNode) -> Result<(), MatchFailed> {
         let ok = match self {

crates/ra_ssr/src/parsing.rs

@@ -7,17 +7,24 @@
 
 use crate::errors::bail;
 use crate::{SsrError, SsrPattern, SsrRule};
-use ra_syntax::{ast, AstNode, SmolStr, SyntaxKind, T};
+use ra_syntax::{ast, AstNode, SmolStr, SyntaxKind, SyntaxNode, SyntaxToken, T};
 use rustc_hash::{FxHashMap, FxHashSet};
 use std::str::FromStr;
 
+#[derive(Debug)]
+pub(crate) struct ParsedRule {
+    pub(crate) placeholders_by_stand_in: FxHashMap<SmolStr, Placeholder>,
+    pub(crate) pattern: SyntaxNode,
+    pub(crate) template: Option<SsrTemplate>,
+}
+
 #[derive(Clone, Debug)]
 pub(crate) struct SsrTemplate {
     pub(crate) tokens: Vec<PatternElement>,
 }
 
 #[derive(Debug)]
-pub(crate) struct RawSearchPattern {
+pub(crate) struct RawPattern {
     tokens: Vec<PatternElement>,
 }
 
@@ -54,6 +61,50 @@ pub(crate) struct Token {
     pub(crate) text: SmolStr,
 }
 
+impl ParsedRule {
+    fn new(
+        pattern: &RawPattern,
+        template: Option<&SsrTemplate>,
+    ) -> Result<Vec<ParsedRule>, SsrError> {
+        let raw_pattern = pattern.as_rust_code();
+        let mut builder = RuleBuilder {
+            placeholders_by_stand_in: pattern.placeholders_by_stand_in(),
+            rules: Vec::new(),
+        };
+        builder.try_add(ast::Expr::parse(&raw_pattern), template);
+        builder.try_add(ast::TypeRef::parse(&raw_pattern), template);
+        builder.try_add(ast::ModuleItem::parse(&raw_pattern), template);
+        builder.try_add(ast::Path::parse(&raw_pattern), template);
+        builder.try_add(ast::Pat::parse(&raw_pattern), template);
+        builder.build()
+    }
+}
+
+struct RuleBuilder {
+    placeholders_by_stand_in: FxHashMap<SmolStr, Placeholder>,
+    rules: Vec<ParsedRule>,
+}
+
+impl RuleBuilder {
+    fn try_add<T: AstNode>(&mut self, pattern: Result<T, ()>, template: Option<&SsrTemplate>) {
+        match pattern {
+            Ok(pattern) => self.rules.push(ParsedRule {
+                placeholders_by_stand_in: self.placeholders_by_stand_in.clone(),
+                pattern: pattern.syntax().clone(),
+                template: template.cloned(),
+            }),
+            _ => {}
+        }
+    }
+
+    fn build(self) -> Result<Vec<ParsedRule>, SsrError> {
+        if self.rules.is_empty() {
+            bail!("Pattern is not a valid Rust expression, type, item, path or pattern");
+        }
+        Ok(self.rules)
+    }
+}
+
 impl FromStr for SsrRule {
     type Err = SsrError;
 
@@ -68,21 +119,24 @@ impl FromStr for SsrRule {
         if it.next().is_some() {
             return Err(SsrError("More than one delimiter found".into()));
         }
-        let rule = SsrRule { pattern: pattern.parse()?, template: template.parse()? };
+        let raw_pattern = pattern.parse()?;
+        let raw_template = template.parse()?;
+        let parsed_rules = ParsedRule::new(&raw_pattern, Some(&raw_template))?;
+        let rule = SsrRule { pattern: raw_pattern, template: raw_template, parsed_rules };
         validate_rule(&rule)?;
         Ok(rule)
     }
 }
 
-impl FromStr for RawSearchPattern {
+impl FromStr for RawPattern {
     type Err = SsrError;
 
-    fn from_str(pattern_str: &str) -> Result<RawSearchPattern, SsrError> {
+    fn from_str(pattern_str: &str) -> Result<RawPattern, SsrError> {
-        Ok(RawSearchPattern { tokens: parse_pattern(pattern_str)? })
+        Ok(RawPattern { tokens: parse_pattern(pattern_str)? })
     }
 }
 
-impl RawSearchPattern {
+impl RawPattern {
     /// Returns this search pattern as Rust source code that we can feed to the Rust parser.
     fn as_rust_code(&self) -> String {
         let mut res = String::new();
@@ -95,7 +149,7 @@ impl RawSearchPattern {
         res
     }
 
-    fn placeholders_by_stand_in(&self) -> FxHashMap<SmolStr, Placeholder> {
+    pub(crate) fn placeholders_by_stand_in(&self) -> FxHashMap<SmolStr, Placeholder> {
         let mut res = FxHashMap::default();
         for t in &self.tokens {
             if let PatternElement::Placeholder(placeholder) = t {
@@ -106,30 +160,22 @@ impl RawSearchPattern {
     }
 }
 
+impl ParsedRule {
+    pub(crate) fn get_placeholder(&self, token: &SyntaxToken) -> Option<&Placeholder> {
+        if token.kind() != SyntaxKind::IDENT {
+            return None;
+        }
+        self.placeholders_by_stand_in.get(token.text())
+    }
+}
+
 impl FromStr for SsrPattern {
     type Err = SsrError;
 
     fn from_str(pattern_str: &str) -> Result<SsrPattern, SsrError> {
-        let raw: RawSearchPattern = pattern_str.parse()?;
+        let raw_pattern = pattern_str.parse()?;
-        let raw_str = raw.as_rust_code();
+        let parsed_rules = ParsedRule::new(&raw_pattern, None)?;
-        let res = SsrPattern {
+        Ok(SsrPattern { raw: raw_pattern, parsed_rules })
-            expr: ast::Expr::parse(&raw_str).ok().map(|n| n.syntax().clone()),
-            type_ref: ast::TypeRef::parse(&raw_str).ok().map(|n| n.syntax().clone()),
-            item: ast::ModuleItem::parse(&raw_str).ok().map(|n| n.syntax().clone()),
-            path: ast::Path::parse(&raw_str).ok().map(|n| n.syntax().clone()),
-            pattern: ast::Pat::parse(&raw_str).ok().map(|n| n.syntax().clone()),
-            placeholders_by_stand_in: raw.placeholders_by_stand_in(),
-            raw,
-        };
-        if res.expr.is_none()
-            && res.type_ref.is_none()
-            && res.item.is_none()
-            && res.path.is_none()
-            && res.pattern.is_none()
-        {
-            bail!("Pattern is not a valid Rust expression, type, item, path or pattern");
-        }
-        Ok(res)
     }
 }
 
@@ -173,7 +219,7 @@ fn parse_pattern(pattern_str: &str) -> Result<Vec<PatternElement>, SsrError> {
 /// pattern didn't define.
 fn validate_rule(rule: &SsrRule) -> Result<(), SsrError> {
     let mut defined_placeholders = FxHashSet::default();
-    for p in &rule.pattern.raw.tokens {
+    for p in &rule.pattern.tokens {
         if let PatternElement::Placeholder(placeholder) = p {
             defined_placeholders.insert(&placeholder.ident);
         }
@@ -316,7 +362,7 @@ mod tests {
         }
         let result: SsrRule = "foo($a, $b) ==>> bar($b, $a)".parse().unwrap();
         assert_eq!(
-            result.pattern.raw.tokens,
+            result.pattern.tokens,
             vec![
                 token(SyntaxKind::IDENT, "foo"),
                 token(T!['('], "("),

crates/ra_ssr/src/replacing.rs

@@ -31,7 +31,11 @@ fn matches_to_edit_at_offset(
 
 fn render_replace(match_info: &Match, file_src: &str) -> String {
     let mut out = String::new();
-    for r in &match_info.template.tokens {
+    let template = match_info
+        .template
+        .as_ref()
+        .expect("You called MatchFinder::edits after calling MatchFinder::add_search_pattern");
+    for r in &template.tokens {
         match r {
             PatternElement::Token(t) => out.push_str(t.text.as_str()),
             PatternElement::Placeholder(p) => {