diff options
author | David Lattimore <[email protected]> | 2020-07-24 11:53:48 +0100 |
---|---|---|
committer | David Lattimore <[email protected]> | 2020-07-24 12:34:00 +0100 |
commit | 3dac31fe80b9d7279e87b94615b0d55805e83412 (patch) | |
tree | d172c092d9ca93c182b2d88c30c3086a9ea797b0 /crates/ra_ssr | |
parent | 8d09ab86edfc01405fd0045bef82e0642efd5f01 (diff) |
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.
Diffstat (limited to 'crates/ra_ssr')
-rw-r--r-- | crates/ra_ssr/src/lib.rs | 8 | ||||
-rw-r--r-- | crates/ra_ssr/src/matching.rs | 42 | ||||
-rw-r--r-- | crates/ra_ssr/src/resolving.rs | 18 | ||||
-rw-r--r-- | crates/ra_ssr/src/search.rs | 65 | ||||
-rw-r--r-- | crates/ra_ssr/src/tests.rs | 58 |
5 files changed, 166 insertions, 25 deletions
diff --git a/crates/ra_ssr/src/lib.rs b/crates/ra_ssr/src/lib.rs index 5a71d4f31..2fb326b45 100644 --- a/crates/ra_ssr/src/lib.rs +++ b/crates/ra_ssr/src/lib.rs | |||
@@ -202,8 +202,12 @@ impl<'db> MatchFinder<'db> { | |||
202 | // For now we ignore rules that have a different kind than our node, otherwise | 202 | // For now we ignore rules that have a different kind than our node, otherwise |
203 | // we get lots of noise. If at some point we add support for restricting rules | 203 | // we get lots of noise. If at some point we add support for restricting rules |
204 | // to a particular kind of thing (e.g. only match type references), then we can | 204 | // to a particular kind of thing (e.g. only match type references), then we can |
205 | // relax this. | 205 | // relax this. We special-case expressions, since function calls can match |
206 | if rule.pattern.node.kind() != node.kind() { | 206 | // method calls. |
207 | if rule.pattern.node.kind() != node.kind() | ||
208 | && !(ast::Expr::can_cast(rule.pattern.node.kind()) | ||
209 | && ast::Expr::can_cast(node.kind())) | ||
210 | { | ||
207 | continue; | 211 | continue; |
208 | } | 212 | } |
209 | out.push(MatchDebugInfo { | 213 | out.push(MatchDebugInfo { |
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> { | |||
189 | } | 189 | } |
190 | return Ok(()); | 190 | return Ok(()); |
191 | } | 191 | } |
192 | // Non-placeholders. | 192 | // We allow a UFCS call to match a method call, provided they resolve to the same function. |
193 | if let Some(pattern_function) = self.rule.pattern.ufcs_function_calls.get(pattern) { | ||
194 | if let (Some(pattern), Some(code)) = | ||
195 | (ast::CallExpr::cast(pattern.clone()), ast::MethodCallExpr::cast(code.clone())) | ||
196 | { | ||
197 | return self.attempt_match_ufcs(phase, &pattern, &code, *pattern_function); | ||
198 | } | ||
199 | } | ||
193 | if pattern.kind() != code.kind() { | 200 | if pattern.kind() != code.kind() { |
194 | fail_match!( | 201 | fail_match!( |
195 | "Pattern had a `{}` ({:?}), code had `{}` ({:?})", | 202 | "Pattern had `{}` ({:?}), code had `{}` ({:?})", |
196 | pattern.text(), | 203 | pattern.text(), |
197 | pattern.kind(), | 204 | pattern.kind(), |
198 | code.text(), | 205 | code.text(), |
@@ -514,6 +521,37 @@ impl<'db, 'sema> Matcher<'db, 'sema> { | |||
514 | Ok(()) | 521 | Ok(()) |
515 | } | 522 | } |
516 | 523 | ||
524 | fn attempt_match_ufcs( | ||
525 | &self, | ||
526 | phase: &mut Phase, | ||
527 | pattern: &ast::CallExpr, | ||
528 | code: &ast::MethodCallExpr, | ||
529 | pattern_function: hir::Function, | ||
530 | ) -> Result<(), MatchFailed> { | ||
531 | use ast::ArgListOwner; | ||
532 | let code_resolved_function = self | ||
533 | .sema | ||
534 | .resolve_method_call(code) | ||
535 | .ok_or_else(|| match_error!("Failed to resolve method call"))?; | ||
536 | if pattern_function != code_resolved_function { | ||
537 | fail_match!("Method call resolved to a different function"); | ||
538 | } | ||
539 | // Check arguments. | ||
540 | let mut pattern_args = pattern | ||
541 | .arg_list() | ||
542 | .ok_or_else(|| match_error!("Pattern function call has no args"))? | ||
543 | .args(); | ||
544 | self.attempt_match_opt(phase, pattern_args.next(), code.expr())?; | ||
545 | let mut code_args = | ||
546 | code.arg_list().ok_or_else(|| match_error!("Code method call has no args"))?.args(); | ||
547 | loop { | ||
548 | match (pattern_args.next(), code_args.next()) { | ||
549 | (None, None) => return Ok(()), | ||
550 | (p, c) => self.attempt_match_opt(phase, p, c)?, | ||
551 | } | ||
552 | } | ||
553 | } | ||
554 | |||
517 | fn get_placeholder(&self, element: &SyntaxElement) -> Option<&Placeholder> { | 555 | fn get_placeholder(&self, element: &SyntaxElement) -> Option<&Placeholder> { |
518 | only_ident(element.clone()).and_then(|ident| self.rule.get_placeholder(&ident)) | 556 | only_ident(element.clone()).and_then(|ident| self.rule.get_placeholder(&ident)) |
519 | } | 557 | } |
diff --git a/crates/ra_ssr/src/resolving.rs b/crates/ra_ssr/src/resolving.rs index 63fd217ce..75f556785 100644 --- a/crates/ra_ssr/src/resolving.rs +++ b/crates/ra_ssr/src/resolving.rs | |||
@@ -18,10 +18,12 @@ pub(crate) struct ResolvedPattern { | |||
18 | pub(crate) node: SyntaxNode, | 18 | pub(crate) node: SyntaxNode, |
19 | // Paths in `node` that we've resolved. | 19 | // Paths in `node` that we've resolved. |
20 | pub(crate) resolved_paths: FxHashMap<SyntaxNode, ResolvedPath>, | 20 | pub(crate) resolved_paths: FxHashMap<SyntaxNode, ResolvedPath>, |
21 | pub(crate) ufcs_function_calls: FxHashMap<SyntaxNode, hir::Function>, | ||
21 | } | 22 | } |
22 | 23 | ||
23 | pub(crate) struct ResolvedPath { | 24 | pub(crate) struct ResolvedPath { |
24 | pub(crate) resolution: hir::PathResolution, | 25 | pub(crate) resolution: hir::PathResolution, |
26 | /// The depth of the ast::Path that was resolved within the pattern. | ||
25 | pub(crate) depth: u32, | 27 | pub(crate) depth: u32, |
26 | } | 28 | } |
27 | 29 | ||
@@ -64,10 +66,26 @@ impl Resolver<'_, '_> { | |||
64 | fn resolve_pattern_tree(&self, pattern: SyntaxNode) -> Result<ResolvedPattern, SsrError> { | 66 | fn resolve_pattern_tree(&self, pattern: SyntaxNode) -> Result<ResolvedPattern, SsrError> { |
65 | let mut resolved_paths = FxHashMap::default(); | 67 | let mut resolved_paths = FxHashMap::default(); |
66 | self.resolve(pattern.clone(), 0, &mut resolved_paths)?; | 68 | self.resolve(pattern.clone(), 0, &mut resolved_paths)?; |
69 | let ufcs_function_calls = resolved_paths | ||
70 | .iter() | ||
71 | .filter_map(|(path_node, resolved)| { | ||
72 | if let Some(grandparent) = path_node.parent().and_then(|parent| parent.parent()) { | ||
73 | if grandparent.kind() == SyntaxKind::CALL_EXPR { | ||
74 | if let hir::PathResolution::AssocItem(hir::AssocItem::Function(function)) = | ||
75 | &resolved.resolution | ||
76 | { | ||
77 | return Some((grandparent, *function)); | ||
78 | } | ||
79 | } | ||
80 | } | ||
81 | None | ||
82 | }) | ||
83 | .collect(); | ||
67 | Ok(ResolvedPattern { | 84 | Ok(ResolvedPattern { |
68 | node: pattern, | 85 | node: pattern, |
69 | resolved_paths, | 86 | resolved_paths, |
70 | placeholders_by_stand_in: self.placeholders_by_stand_in.clone(), | 87 | placeholders_by_stand_in: self.placeholders_by_stand_in.clone(), |
88 | ufcs_function_calls, | ||
71 | }) | 89 | }) |
72 | } | 90 | } |
73 | 91 | ||
diff --git a/crates/ra_ssr/src/search.rs b/crates/ra_ssr/src/search.rs index 141e7d026..bcf0f0468 100644 --- a/crates/ra_ssr/src/search.rs +++ b/crates/ra_ssr/src/search.rs | |||
@@ -46,35 +46,58 @@ impl<'db> MatchFinder<'db> { | |||
46 | usage_cache: &mut UsageCache, | 46 | usage_cache: &mut UsageCache, |
47 | matches_out: &mut Vec<Match>, | 47 | matches_out: &mut Vec<Match>, |
48 | ) { | 48 | ) { |
49 | if let Some(first_path) = pick_path_for_usages(pattern) { | 49 | if let Some(resolved_path) = pick_path_for_usages(pattern) { |
50 | let definition: Definition = first_path.resolution.clone().into(); | 50 | let definition: Definition = resolved_path.resolution.clone().into(); |
51 | for reference in self.find_usages(usage_cache, definition) { | 51 | for reference in self.find_usages(usage_cache, definition) { |
52 | let file = self.sema.parse(reference.file_range.file_id); | 52 | if let Some(node_to_match) = self.find_node_to_match(resolved_path, reference) { |
53 | if let Some(path) = self.sema.find_node_at_offset_with_descend::<ast::Path>( | 53 | if !is_search_permitted_ancestors(&node_to_match) { |
54 | file.syntax(), | 54 | mark::hit!(use_declaration_with_braces); |
55 | reference.file_range.range.start(), | 55 | continue; |
56 | ) { | 56 | } |
57 | if let Some(node_to_match) = self | 57 | if let Ok(m) = |
58 | .sema | 58 | matching::get_match(false, rule, &node_to_match, &None, &self.sema) |
59 | .ancestors_with_macros(path.syntax().clone()) | ||
60 | .skip(first_path.depth as usize) | ||
61 | .next() | ||
62 | { | 59 | { |
63 | if !is_search_permitted_ancestors(&node_to_match) { | 60 | matches_out.push(m); |
64 | mark::hit!(use_declaration_with_braces); | ||
65 | continue; | ||
66 | } | ||
67 | if let Ok(m) = | ||
68 | matching::get_match(false, rule, &node_to_match, &None, &self.sema) | ||
69 | { | ||
70 | matches_out.push(m); | ||
71 | } | ||
72 | } | 61 | } |
73 | } | 62 | } |
74 | } | 63 | } |
75 | } | 64 | } |
76 | } | 65 | } |
77 | 66 | ||
67 | fn find_node_to_match( | ||
68 | &self, | ||
69 | resolved_path: &ResolvedPath, | ||
70 | reference: &Reference, | ||
71 | ) -> Option<SyntaxNode> { | ||
72 | let file = self.sema.parse(reference.file_range.file_id); | ||
73 | let depth = resolved_path.depth as usize; | ||
74 | let offset = reference.file_range.range.start(); | ||
75 | if let Some(path) = | ||
76 | self.sema.find_node_at_offset_with_descend::<ast::Path>(file.syntax(), offset) | ||
77 | { | ||
78 | self.sema.ancestors_with_macros(path.syntax().clone()).skip(depth).next() | ||
79 | } else if let Some(path) = | ||
80 | self.sema.find_node_at_offset_with_descend::<ast::MethodCallExpr>(file.syntax(), offset) | ||
81 | { | ||
82 | // If the pattern contained a path and we found a reference to that path that wasn't | ||
83 | // itself a path, but was a method call, then we need to adjust how far up to try | ||
84 | // matching by how deep the path was within a CallExpr. The structure would have been | ||
85 | // CallExpr, PathExpr, Path - i.e. a depth offset of 2. We don't need to check if the | ||
86 | // path was part of a CallExpr because if it wasn't then all that will happen is we'll | ||
87 | // fail to match, which is the desired behavior. | ||
88 | const PATH_DEPTH_IN_CALL_EXPR: usize = 2; | ||
89 | if depth < PATH_DEPTH_IN_CALL_EXPR { | ||
90 | return None; | ||
91 | } | ||
92 | self.sema | ||
93 | .ancestors_with_macros(path.syntax().clone()) | ||
94 | .skip(depth - PATH_DEPTH_IN_CALL_EXPR) | ||
95 | .next() | ||
96 | } else { | ||
97 | None | ||
98 | } | ||
99 | } | ||
100 | |||
78 | fn find_usages<'a>( | 101 | fn find_usages<'a>( |
79 | &self, | 102 | &self, |
80 | usage_cache: &'a mut UsageCache, | 103 | usage_cache: &'a mut UsageCache, |
diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs index f564c6129..b38807c0f 100644 --- a/crates/ra_ssr/src/tests.rs +++ b/crates/ra_ssr/src/tests.rs | |||
@@ -827,3 +827,61 @@ fn use_declaration_with_braces() { | |||
827 | "]], | 827 | "]], |
828 | ) | 828 | ) |
829 | } | 829 | } |
830 | |||
831 | #[test] | ||
832 | fn ufcs_matches_method_call() { | ||
833 | let code = r#" | ||
834 | struct Foo {} | ||
835 | impl Foo { | ||
836 | fn new(_: i32) -> Foo { Foo {} } | ||
837 | fn do_stuff(&self, _: i32) {} | ||
838 | } | ||
839 | struct Bar {} | ||
840 | impl Bar { | ||
841 | fn new(_: i32) -> Bar { Bar {} } | ||
842 | fn do_stuff(&self, v: i32) {} | ||
843 | } | ||
844 | fn main() { | ||
845 | let b = Bar {}; | ||
846 | let f = Foo {}; | ||
847 | b.do_stuff(1); | ||
848 | f.do_stuff(2); | ||
849 | Foo::new(4).do_stuff(3); | ||
850 | // Too many / too few args - should never match | ||
851 | f.do_stuff(2, 10); | ||
852 | f.do_stuff(); | ||
853 | } | ||
854 | "#; | ||
855 | assert_matches("Foo::do_stuff($a, $b)", code, &["f.do_stuff(2)", "Foo::new(4).do_stuff(3)"]); | ||
856 | // The arguments needs special handling in the case of a function call matching a method call | ||
857 | // and the first argument is different. | ||
858 | assert_matches("Foo::do_stuff($a, 2)", code, &["f.do_stuff(2)"]); | ||
859 | assert_matches("Foo::do_stuff(Foo::new(4), $b)", code, &["Foo::new(4).do_stuff(3)"]); | ||
860 | |||
861 | assert_ssr_transform( | ||
862 | "Foo::do_stuff(Foo::new($a), $b) ==>> Bar::new($b).do_stuff($a)", | ||
863 | code, | ||
864 | expect![[r#" | ||
865 | struct Foo {} | ||
866 | impl Foo { | ||
867 | fn new(_: i32) -> Foo { Foo {} } | ||
868 | fn do_stuff(&self, _: i32) {} | ||
869 | } | ||
870 | struct Bar {} | ||
871 | impl Bar { | ||
872 | fn new(_: i32) -> Bar { Bar {} } | ||
873 | fn do_stuff(&self, v: i32) {} | ||
874 | } | ||
875 | fn main() { | ||
876 | let b = Bar {}; | ||
877 | let f = Foo {}; | ||
878 | b.do_stuff(1); | ||
879 | f.do_stuff(2); | ||
880 | Bar::new(3).do_stuff(4); | ||
881 | // Too many / too few args - should never match | ||
882 | f.do_stuff(2, 10); | ||
883 | f.do_stuff(); | ||
884 | } | ||
885 | "#]], | ||
886 | ); | ||
887 | } | ||