From 1fce8b6ba32bebba36d588d07781e9e578845728 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Fri, 3 Jul 2020 12:57:17 +1000 Subject: 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/matching.rs | 42 +++++++++--------------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) (limited to 'crates/ra_ssr/src/matching.rs') diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index 50b29eab2..842f4b6f3 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs @@ -2,8 +2,8 @@ //! process of matching, placeholder values are recorded. use crate::{ - parsing::{Constraint, NodeKind, Placeholder, SsrTemplate}, - SsrMatches, SsrPattern, SsrRule, + parsing::{Constraint, NodeKind, ParsedRule, Placeholder, SsrTemplate}, + SsrMatches, }; use hir::Semantics; use ra_db::FileRange; @@ -50,7 +50,7 @@ pub struct Match { pub(crate) ignored_comments: Vec, // 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, } /// 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, sema: &Semantics, @@ -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, - 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, sema: &'sema Semantics<'db, ra_ide_db::RootDatabase>, ) -> Result { 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()) - .and_then(|ident| self.rule.pattern.placeholders_by_stand_in.get(ident.text())) + only_ident(element.clone()).and_then(|ident| self.rule.get_placeholder(&ident)) } } @@ -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 { -- cgit v1.2.3 From 113abbeefee671266d2d9bebdbd517eb8b802ef8 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Wed, 22 Jul 2020 19:15:19 +1000 Subject: SSR: Parse template as Rust code. This is in preparation for a subsequent commit where we add special handling for paths in the template, allowing them to be qualified differently in different contexts. --- crates/ra_ssr/src/matching.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) (limited to 'crates/ra_ssr/src/matching.rs') diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index 842f4b6f3..486191635 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs @@ -2,7 +2,7 @@ //! process of matching, placeholder values are recorded. use crate::{ - parsing::{Constraint, NodeKind, ParsedRule, Placeholder, SsrTemplate}, + parsing::{Constraint, NodeKind, ParsedRule, Placeholder}, SsrMatches, }; use hir::Semantics; @@ -48,9 +48,7 @@ pub struct Match { pub(crate) matched_node: SyntaxNode, pub(crate) placeholder_values: FxHashMap, pub(crate) ignored_comments: Vec, - // 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: Option, + pub(crate) rule_index: usize, } /// Represents a `$var` in an SSR query. @@ -131,7 +129,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> { matched_node: code.clone(), placeholder_values: FxHashMap::default(), ignored_comments: Vec::new(), - template: rule.template.clone(), + rule_index: rule.index, }; // 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. @@ -591,7 +589,7 @@ mod tests { "1+2" ); - let edit = crate::replacing::matches_to_edit(&matches, input); + let edit = crate::replacing::matches_to_edit(&matches, input, &match_finder.rules); let mut after = input.to_string(); edit.apply(&mut after); assert_eq!(after, "fn foo() {} fn main() { bar(1+2); }"); -- cgit v1.2.3 From a45682ed96f18f962ac403419b4d143d59ba5283 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Wed, 22 Jul 2020 16:23:43 +1000 Subject: Move iteration over all files into the SSR crate The methods `edits_for_file` and `find_matches_in_file` are replaced with just `edits` and `matches`. This simplifies the API a bit, but more importantly it makes it possible in a subsequent commit for SSR to decide to not search all files. --- crates/ra_ssr/src/matching.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'crates/ra_ssr/src/matching.rs') diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index 486191635..064e3a204 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs @@ -570,13 +570,12 @@ mod tests { #[test] fn parse_match_replace() { let rule: SsrRule = "foo($x) ==>> bar($x)".parse().unwrap(); - let input = "fn foo() {} fn main() { foo(1+2); }"; + let input = "fn foo() {} fn bar() {} fn main() { foo(1+2); }"; - use ra_db::fixture::WithFixture; - let (db, file_id) = ra_ide_db::RootDatabase::with_single_file(input); + let (db, _) = crate::tests::single_file(input); let mut match_finder = MatchFinder::new(&db); match_finder.add_rule(rule); - let matches = match_finder.find_matches_in_file(file_id); + let matches = match_finder.matches(); assert_eq!(matches.matches.len(), 1); assert_eq!(matches.matches[0].matched_node.text(), "foo(1+2)"); assert_eq!(matches.matches[0].placeholder_values.len(), 1); @@ -589,9 +588,11 @@ mod tests { "1+2" ); - let edit = crate::replacing::matches_to_edit(&matches, input, &match_finder.rules); + let edits = match_finder.edits(); + assert_eq!(edits.len(), 1); + let edit = &edits[0]; let mut after = input.to_string(); - edit.apply(&mut after); - assert_eq!(after, "fn foo() {} fn main() { bar(1+2); }"); + edit.edit.apply(&mut after); + assert_eq!(after, "fn foo() {} fn bar() {} fn main() { bar(1+2); }"); } } -- cgit v1.2.3 From 02fc3d50ee4d179cc5a443a790544c2a5e439cb0 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Wed, 22 Jul 2020 16:48:12 +1000 Subject: SSR: Refactor to not rely on recursive search for nesting of matches Previously, submatches were handled simply by searching in placeholders for more matches. That only works if we search all nodes in the tree recursively. In a subsequent commit, I intend to make search not always be recursive recursive. This commit prepares for that by finding all matches, even if they overlap, then nesting them and removing overlapping matches. --- crates/ra_ssr/src/matching.rs | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'crates/ra_ssr/src/matching.rs') diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index 064e3a204..005569f6f 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs @@ -49,6 +49,8 @@ pub struct Match { pub(crate) placeholder_values: FxHashMap, pub(crate) ignored_comments: Vec, pub(crate) rule_index: usize, + /// The depth of matched_node. + pub(crate) depth: usize, } /// Represents a `$var` in an SSR query. @@ -130,10 +132,12 @@ impl<'db, 'sema> Matcher<'db, 'sema> { placeholder_values: FxHashMap::default(), ignored_comments: Vec::new(), rule_index: rule.index, + depth: 0, }; // 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), &rule.pattern, code)?; + the_match.depth = sema.ancestors_with_macros(the_match.matched_node.clone()).count(); Ok(the_match) } -- cgit v1.2.3 From 3975952601888d9f77e466c12e8e389748984b33 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Wed, 22 Jul 2020 15:00:28 +1000 Subject: SSR: Pass current file position through to SSR code. In a subsequent commit, it will be used for resolving paths. --- crates/ra_ssr/src/matching.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'crates/ra_ssr/src/matching.rs') diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index 005569f6f..a43d57c34 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs @@ -576,8 +576,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, _) = crate::tests::single_file(input); - let mut match_finder = MatchFinder::new(&db); + let (db, position) = crate::tests::single_file(input); + let mut match_finder = MatchFinder::in_context(&db, position); match_finder.add_rule(rule); let matches = match_finder.matches(); assert_eq!(matches.matches.len(), 1); -- cgit v1.2.3 From 757f755c29e041fd319af466d7d0418f54cb090a Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Wed, 22 Jul 2020 16:46:29 +1000 Subject: SSR: Match paths based on what they resolve to Also render template paths appropriately for their context. --- crates/ra_ssr/src/matching.rs | 106 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 99 insertions(+), 7 deletions(-) (limited to 'crates/ra_ssr/src/matching.rs') diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index a43d57c34..f3cc60c29 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs @@ -2,7 +2,8 @@ //! process of matching, placeholder values are recorded. use crate::{ - parsing::{Constraint, NodeKind, ParsedRule, Placeholder}, + parsing::{Constraint, NodeKind, Placeholder}, + resolving::{ResolvedPattern, ResolvedRule}, SsrMatches, }; use hir::Semantics; @@ -51,6 +52,8 @@ pub struct Match { pub(crate) rule_index: usize, /// The depth of matched_node. pub(crate) depth: usize, + // Each path in the template rendered for the module in which the match was found. + pub(crate) rendered_template_paths: FxHashMap, } /// Represents a `$var` in an SSR query. @@ -86,7 +89,7 @@ pub(crate) struct MatchFailed { /// parent module, we don't populate nested matches. pub(crate) fn get_match( debug_active: bool, - rule: &ParsedRule, + rule: &ResolvedRule, code: &SyntaxNode, restrict_range: &Option, sema: &Semantics, @@ -102,7 +105,7 @@ struct Matcher<'db, 'sema> { /// If any placeholders come from anywhere outside of this range, then the match will be /// rejected. restrict_range: Option, - rule: &'sema ParsedRule, + rule: &'sema ResolvedRule, } /// Which phase of matching we're currently performing. We do two phases because most attempted @@ -117,14 +120,14 @@ enum Phase<'a> { impl<'db, 'sema> Matcher<'db, 'sema> { fn try_match( - rule: &ParsedRule, + rule: &ResolvedRule, code: &SyntaxNode, restrict_range: &Option, sema: &'sema Semantics<'db, ra_ide_db::RootDatabase>, ) -> Result { let match_state = Matcher { sema, restrict_range: restrict_range.clone(), rule }; // First pass at matching, where we check that node types and idents match. - match_state.attempt_match_node(&mut Phase::First, &rule.pattern, code)?; + match_state.attempt_match_node(&mut Phase::First, &rule.pattern.node, code)?; match_state.validate_range(&sema.original_range(code))?; let mut the_match = Match { range: sema.original_range(code), @@ -133,11 +136,19 @@ impl<'db, 'sema> Matcher<'db, 'sema> { ignored_comments: Vec::new(), rule_index: rule.index, depth: 0, + rendered_template_paths: FxHashMap::default(), }; // 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), &rule.pattern, code)?; + match_state.attempt_match_node( + &mut Phase::Second(&mut the_match), + &rule.pattern.node, + code, + )?; the_match.depth = sema.ancestors_with_macros(the_match.matched_node.clone()).count(); + if let Some(template) = &rule.template { + the_match.render_template_paths(template, sema)?; + } Ok(the_match) } @@ -195,6 +206,7 @@ impl<'db, 'sema> Matcher<'db, 'sema> { self.attempt_match_record_field_list(phase, pattern, code) } SyntaxKind::TOKEN_TREE => self.attempt_match_token_tree(phase, pattern, code), + SyntaxKind::PATH => self.attempt_match_path(phase, pattern, code), _ => self.attempt_match_node_children(phase, pattern, code), } } @@ -311,6 +323,64 @@ impl<'db, 'sema> Matcher<'db, 'sema> { Ok(()) } + /// Paths are matched based on whether they refer to the same thing, even if they're written + /// differently. + fn attempt_match_path( + &self, + phase: &mut Phase, + pattern: &SyntaxNode, + code: &SyntaxNode, + ) -> Result<(), MatchFailed> { + if let Some(pattern_resolved) = self.rule.pattern.resolved_paths.get(pattern) { + let pattern_path = ast::Path::cast(pattern.clone()).unwrap(); + let code_path = ast::Path::cast(code.clone()).unwrap(); + if let (Some(pattern_segment), Some(code_segment)) = + (pattern_path.segment(), code_path.segment()) + { + // Match everything within the segment except for the name-ref, which is handled + // separately via comparing what the path resolves to below. + self.attempt_match_opt( + phase, + pattern_segment.type_arg_list(), + code_segment.type_arg_list(), + )?; + self.attempt_match_opt( + phase, + pattern_segment.param_list(), + code_segment.param_list(), + )?; + } + if matches!(phase, Phase::Second(_)) { + let resolution = self + .sema + .resolve_path(&code_path) + .ok_or_else(|| match_error!("Failed to resolve path `{}`", code.text()))?; + if pattern_resolved.resolution != resolution { + fail_match!("Pattern had path `{}` code had `{}`", pattern.text(), code.text()); + } + } + } else { + return self.attempt_match_node_children(phase, pattern, code); + } + Ok(()) + } + + fn attempt_match_opt( + &self, + phase: &mut Phase, + pattern: Option, + code: Option, + ) -> Result<(), MatchFailed> { + match (pattern, code) { + (Some(p), Some(c)) => self.attempt_match_node(phase, &p.syntax(), &c.syntax()), + (None, None) => Ok(()), + (Some(p), None) => fail_match!("Pattern `{}` had nothing to match", p.syntax().text()), + (None, Some(c)) => { + fail_match!("Nothing in pattern to match code `{}`", c.syntax().text()) + } + } + } + /// We want to allow the records to match in any order, so we have special matching logic for /// them. fn attempt_match_record_field_list( @@ -449,6 +519,28 @@ impl<'db, 'sema> Matcher<'db, 'sema> { } } +impl Match { + fn render_template_paths( + &mut self, + template: &ResolvedPattern, + sema: &Semantics, + ) -> Result<(), MatchFailed> { + let module = sema + .scope(&self.matched_node) + .module() + .ok_or_else(|| match_error!("Matched node isn't in a module"))?; + for (path, resolved_path) in &template.resolved_paths { + if let hir::PathResolution::Def(module_def) = resolved_path.resolution { + let mod_path = module.find_use_path(sema.db, module_def).ok_or_else(|| { + match_error!("Failed to render template path `{}` at match location") + })?; + self.rendered_template_paths.insert(path.clone(), mod_path); + } + } + Ok(()) + } +} + impl Phase<'_> { fn next_non_trivial(&mut self, code_it: &mut SyntaxElementChildren) -> Option { loop { @@ -578,7 +670,7 @@ mod tests { let (db, position) = crate::tests::single_file(input); let mut match_finder = MatchFinder::in_context(&db, position); - match_finder.add_rule(rule); + match_finder.add_rule(rule).unwrap(); let matches = match_finder.matches(); assert_eq!(matches.matches.len(), 1); assert_eq!(matches.matches[0].matched_node.text(), "foo(1+2)"); -- cgit v1.2.3 From 3dac31fe80b9d7279e87b94615b0d55805e83412 Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Fri, 24 Jul 2020 20:53:48 +1000 Subject: SSR: Allow function calls to match method calls This differs from how this used to work before I removed it in that: a) It's only one direction. Function calls in the pattern can match method calls in the code, but not the other way around. b) We now check that the function call in the pattern resolves to the same function as the method call in the code. The lack of (b) was the reason I felt the need to remove the feature before. --- crates/ra_ssr/src/matching.rs | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) (limited to 'crates/ra_ssr/src/matching.rs') diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index f3cc60c29..4862622bd 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs @@ -189,10 +189,17 @@ impl<'db, 'sema> Matcher<'db, 'sema> { } return Ok(()); } - // Non-placeholders. + // We allow a UFCS call to match a method call, provided they resolve to the same function. + if let Some(pattern_function) = self.rule.pattern.ufcs_function_calls.get(pattern) { + if let (Some(pattern), Some(code)) = + (ast::CallExpr::cast(pattern.clone()), ast::MethodCallExpr::cast(code.clone())) + { + return self.attempt_match_ufcs(phase, &pattern, &code, *pattern_function); + } + } if pattern.kind() != code.kind() { fail_match!( - "Pattern had a `{}` ({:?}), code had `{}` ({:?})", + "Pattern had `{}` ({:?}), code had `{}` ({:?})", pattern.text(), pattern.kind(), code.text(), @@ -514,6 +521,37 @@ impl<'db, 'sema> Matcher<'db, 'sema> { Ok(()) } + fn attempt_match_ufcs( + &self, + phase: &mut Phase, + pattern: &ast::CallExpr, + code: &ast::MethodCallExpr, + pattern_function: hir::Function, + ) -> Result<(), MatchFailed> { + use ast::ArgListOwner; + let code_resolved_function = self + .sema + .resolve_method_call(code) + .ok_or_else(|| match_error!("Failed to resolve method call"))?; + if pattern_function != code_resolved_function { + fail_match!("Method call resolved to a different function"); + } + // Check arguments. + let mut pattern_args = pattern + .arg_list() + .ok_or_else(|| match_error!("Pattern function call has no args"))? + .args(); + self.attempt_match_opt(phase, pattern_args.next(), code.expr())?; + let mut code_args = + code.arg_list().ok_or_else(|| match_error!("Code method call has no args"))?.args(); + loop { + match (pattern_args.next(), code_args.next()) { + (None, None) => return Ok(()), + (p, c) => self.attempt_match_opt(phase, p, c)?, + } + } + } + fn get_placeholder(&self, element: &SyntaxElement) -> Option<&Placeholder> { only_ident(element.clone()).and_then(|ident| self.rule.get_placeholder(&ident)) } -- cgit v1.2.3