diff options
author | David Lattimore <[email protected]> | 2020-07-29 02:44:01 +0100 |
---|---|---|
committer | David Lattimore <[email protected]> | 2020-07-29 06:06:58 +0100 |
commit | cf55806257776baf7db6b02d260bdaa9e851c7d4 (patch) | |
tree | 6827c094a4a27631ee9b1b97d98a8ac9c0a56aac /crates/ra_ssr | |
parent | 5a8124273dd663f7f1ed43b53defc4a2c52dbc12 (diff) |
SSR: Restrict to current selection if any
The selection is also used to avoid unnecessary work, but only to the
file level. Further restricting unnecessary work is left for later.
Diffstat (limited to 'crates/ra_ssr')
-rw-r--r-- | crates/ra_ssr/src/lib.rs | 11 | ||||
-rw-r--r-- | crates/ra_ssr/src/matching.rs | 4 | ||||
-rw-r--r-- | crates/ra_ssr/src/search.rs | 87 | ||||
-rw-r--r-- | crates/ra_ssr/src/tests.rs | 96 |
4 files changed, 155 insertions, 43 deletions
diff --git a/crates/ra_ssr/src/lib.rs b/crates/ra_ssr/src/lib.rs index 7014a6ac6..73abfecb2 100644 --- a/crates/ra_ssr/src/lib.rs +++ b/crates/ra_ssr/src/lib.rs | |||
@@ -52,6 +52,7 @@ pub struct MatchFinder<'db> { | |||
52 | sema: Semantics<'db, ra_ide_db::RootDatabase>, | 52 | sema: Semantics<'db, ra_ide_db::RootDatabase>, |
53 | rules: Vec<ResolvedRule>, | 53 | rules: Vec<ResolvedRule>, |
54 | resolution_scope: resolving::ResolutionScope<'db>, | 54 | resolution_scope: resolving::ResolutionScope<'db>, |
55 | restrict_ranges: Vec<FileRange>, | ||
55 | } | 56 | } |
56 | 57 | ||
57 | impl<'db> MatchFinder<'db> { | 58 | impl<'db> MatchFinder<'db> { |
@@ -60,10 +61,17 @@ impl<'db> MatchFinder<'db> { | |||
60 | pub fn in_context( | 61 | pub fn in_context( |
61 | db: &'db ra_ide_db::RootDatabase, | 62 | db: &'db ra_ide_db::RootDatabase, |
62 | lookup_context: FilePosition, | 63 | lookup_context: FilePosition, |
64 | mut restrict_ranges: Vec<FileRange>, | ||
63 | ) -> MatchFinder<'db> { | 65 | ) -> MatchFinder<'db> { |
66 | restrict_ranges.retain(|range| !range.range.is_empty()); | ||
64 | let sema = Semantics::new(db); | 67 | let sema = Semantics::new(db); |
65 | let resolution_scope = resolving::ResolutionScope::new(&sema, lookup_context); | 68 | let resolution_scope = resolving::ResolutionScope::new(&sema, lookup_context); |
66 | MatchFinder { sema: Semantics::new(db), rules: Vec::new(), resolution_scope } | 69 | MatchFinder { |
70 | sema: Semantics::new(db), | ||
71 | rules: Vec::new(), | ||
72 | resolution_scope, | ||
73 | restrict_ranges, | ||
74 | } | ||
67 | } | 75 | } |
68 | 76 | ||
69 | /// Constructs an instance using the start of the first file in `db` as the lookup context. | 77 | /// Constructs an instance using the start of the first file in `db` as the lookup context. |
@@ -79,6 +87,7 @@ impl<'db> MatchFinder<'db> { | |||
79 | Ok(MatchFinder::in_context( | 87 | Ok(MatchFinder::in_context( |
80 | db, | 88 | db, |
81 | FilePosition { file_id: first_file_id, offset: 0.into() }, | 89 | FilePosition { file_id: first_file_id, offset: 0.into() }, |
90 | vec![], | ||
82 | )) | 91 | )) |
83 | } else { | 92 | } else { |
84 | bail!("No files to search"); | 93 | bail!("No files to search"); |
diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index 4862622bd..c1b66748e 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs | |||
@@ -706,8 +706,8 @@ mod tests { | |||
706 | let rule: SsrRule = "foo($x) ==>> bar($x)".parse().unwrap(); | 706 | let rule: SsrRule = "foo($x) ==>> bar($x)".parse().unwrap(); |
707 | let input = "fn foo() {} fn bar() {} fn main() { foo(1+2); }"; | 707 | let input = "fn foo() {} fn bar() {} fn main() { foo(1+2); }"; |
708 | 708 | ||
709 | let (db, position) = crate::tests::single_file(input); | 709 | let (db, position, selections) = crate::tests::single_file(input); |
710 | let mut match_finder = MatchFinder::in_context(&db, position); | 710 | let mut match_finder = MatchFinder::in_context(&db, position, selections); |
711 | match_finder.add_rule(rule).unwrap(); | 711 | match_finder.add_rule(rule).unwrap(); |
712 | let matches = match_finder.matches(); | 712 | let matches = match_finder.matches(); |
713 | assert_eq!(matches.matches.len(), 1); | 713 | assert_eq!(matches.matches.len(), 1); |
diff --git a/crates/ra_ssr/src/search.rs b/crates/ra_ssr/src/search.rs index bcf0f0468..0f512cb62 100644 --- a/crates/ra_ssr/src/search.rs +++ b/crates/ra_ssr/src/search.rs | |||
@@ -5,12 +5,13 @@ use crate::{ | |||
5 | resolving::{ResolvedPath, ResolvedPattern, ResolvedRule}, | 5 | resolving::{ResolvedPath, ResolvedPattern, ResolvedRule}, |
6 | Match, MatchFinder, | 6 | Match, MatchFinder, |
7 | }; | 7 | }; |
8 | use ra_db::FileRange; | 8 | use ra_db::{FileId, FileRange}; |
9 | use ra_ide_db::{ | 9 | use ra_ide_db::{ |
10 | defs::Definition, | 10 | defs::Definition, |
11 | search::{Reference, SearchScope}, | 11 | search::{Reference, SearchScope}, |
12 | }; | 12 | }; |
13 | use ra_syntax::{ast, AstNode, SyntaxKind, SyntaxNode}; | 13 | use ra_syntax::{ast, AstNode, SyntaxKind, SyntaxNode}; |
14 | use rustc_hash::FxHashSet; | ||
14 | use test_utils::mark; | 15 | use test_utils::mark; |
15 | 16 | ||
16 | /// A cache for the results of find_usages. This is for when we have multiple patterns that have the | 17 | /// 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> { | |||
54 | mark::hit!(use_declaration_with_braces); | 55 | mark::hit!(use_declaration_with_braces); |
55 | continue; | 56 | continue; |
56 | } | 57 | } |
57 | if let Ok(m) = | 58 | self.try_add_match(rule, &node_to_match, &None, matches_out); |
58 | matching::get_match(false, rule, &node_to_match, &None, &self.sema) | ||
59 | { | ||
60 | matches_out.push(m); | ||
61 | } | ||
62 | } | 59 | } |
63 | } | 60 | } |
64 | } | 61 | } |
@@ -121,25 +118,39 @@ impl<'db> MatchFinder<'db> { | |||
121 | // FIXME: We should ideally have a test that checks that we edit local roots and not library | 118 | // FIXME: We should ideally have a test that checks that we edit local roots and not library |
122 | // roots. This probably would require some changes to fixtures, since currently everything | 119 | // roots. This probably would require some changes to fixtures, since currently everything |
123 | // seems to get put into a single source root. | 120 | // seems to get put into a single source root. |
124 | use ra_db::SourceDatabaseExt; | ||
125 | use ra_ide_db::symbol_index::SymbolsDatabase; | ||
126 | let mut files = Vec::new(); | 121 | let mut files = Vec::new(); |
127 | for &root in self.sema.db.local_roots().iter() { | 122 | self.search_files_do(|file_id| { |
128 | let sr = self.sema.db.source_root(root); | 123 | files.push(file_id); |
129 | files.extend(sr.iter()); | 124 | }); |
130 | } | ||
131 | SearchScope::files(&files) | 125 | SearchScope::files(&files) |
132 | } | 126 | } |
133 | 127 | ||
134 | fn slow_scan(&self, rule: &ResolvedRule, matches_out: &mut Vec<Match>) { | 128 | fn slow_scan(&self, rule: &ResolvedRule, matches_out: &mut Vec<Match>) { |
135 | use ra_db::SourceDatabaseExt; | 129 | self.search_files_do(|file_id| { |
136 | use ra_ide_db::symbol_index::SymbolsDatabase; | 130 | let file = self.sema.parse(file_id); |
137 | for &root in self.sema.db.local_roots().iter() { | 131 | let code = file.syntax(); |
138 | let sr = self.sema.db.source_root(root); | 132 | self.slow_scan_node(code, rule, &None, matches_out); |
139 | for file_id in sr.iter() { | 133 | }) |
140 | let file = self.sema.parse(file_id); | 134 | } |
141 | let code = file.syntax(); | 135 | |
142 | self.slow_scan_node(code, rule, &None, matches_out); | 136 | fn search_files_do(&self, mut callback: impl FnMut(FileId)) { |
137 | if self.restrict_ranges.is_empty() { | ||
138 | // Unrestricted search. | ||
139 | use ra_db::SourceDatabaseExt; | ||
140 | use ra_ide_db::symbol_index::SymbolsDatabase; | ||
141 | for &root in self.sema.db.local_roots().iter() { | ||
142 | let sr = self.sema.db.source_root(root); | ||
143 | for file_id in sr.iter() { | ||
144 | callback(file_id); | ||
145 | } | ||
146 | } | ||
147 | } else { | ||
148 | // Search is restricted, deduplicate file IDs (generally only one). | ||
149 | let mut files = FxHashSet::default(); | ||
150 | for range in &self.restrict_ranges { | ||
151 | if files.insert(range.file_id) { | ||
152 | callback(range.file_id); | ||
153 | } | ||
143 | } | 154 | } |
144 | } | 155 | } |
145 | } | 156 | } |
@@ -154,9 +165,7 @@ impl<'db> MatchFinder<'db> { | |||
154 | if !is_search_permitted(code) { | 165 | if !is_search_permitted(code) { |
155 | return; | 166 | return; |
156 | } | 167 | } |
157 | if let Ok(m) = matching::get_match(false, rule, &code, restrict_range, &self.sema) { | 168 | self.try_add_match(rule, &code, restrict_range, matches_out); |
158 | matches_out.push(m); | ||
159 | } | ||
160 | // If we've got a macro call, we already tried matching it pre-expansion, which is the only | 169 | // If we've got a macro call, we already tried matching it pre-expansion, which is the only |
161 | // way to match the whole macro, now try expanding it and matching the expansion. | 170 | // way to match the whole macro, now try expanding it and matching the expansion. |
162 | if let Some(macro_call) = ast::MacroCall::cast(code.clone()) { | 171 | if let Some(macro_call) = ast::MacroCall::cast(code.clone()) { |
@@ -178,6 +187,38 @@ impl<'db> MatchFinder<'db> { | |||
178 | self.slow_scan_node(&child, rule, restrict_range, matches_out); | 187 | self.slow_scan_node(&child, rule, restrict_range, matches_out); |
179 | } | 188 | } |
180 | } | 189 | } |
190 | |||
191 | fn try_add_match( | ||
192 | &self, | ||
193 | rule: &ResolvedRule, | ||
194 | code: &SyntaxNode, | ||
195 | restrict_range: &Option<FileRange>, | ||
196 | matches_out: &mut Vec<Match>, | ||
197 | ) { | ||
198 | if !self.within_range_restrictions(code) { | ||
199 | mark::hit!(replace_nonpath_within_selection); | ||
200 | return; | ||
201 | } | ||
202 | if let Ok(m) = matching::get_match(false, rule, code, restrict_range, &self.sema) { | ||
203 | matches_out.push(m); | ||
204 | } | ||
205 | } | ||
206 | |||
207 | /// Returns whether `code` is within one of our range restrictions if we have any. No range | ||
208 | /// restrictions is considered unrestricted and always returns true. | ||
209 | fn within_range_restrictions(&self, code: &SyntaxNode) -> bool { | ||
210 | if self.restrict_ranges.is_empty() { | ||
211 | // There is no range restriction. | ||
212 | return true; | ||
213 | } | ||
214 | let node_range = self.sema.original_range(code); | ||
215 | for range in &self.restrict_ranges { | ||
216 | if range.file_id == node_range.file_id && range.range.contains_range(node_range.range) { | ||
217 | return true; | ||
218 | } | ||
219 | } | ||
220 | false | ||
221 | } | ||
181 | } | 222 | } |
182 | 223 | ||
183 | /// Returns whether we support matching within `node` and all of its ancestors. | 224 | /// Returns whether we support matching within `node` and all of its ancestors. |
diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs index 18ef2506a..6a44ef378 100644 --- a/crates/ra_ssr/src/tests.rs +++ b/crates/ra_ssr/src/tests.rs | |||
@@ -1,9 +1,9 @@ | |||
1 | use crate::{MatchFinder, SsrRule}; | 1 | use crate::{MatchFinder, SsrRule}; |
2 | use expect::{expect, Expect}; | 2 | use expect::{expect, Expect}; |
3 | use ra_db::{salsa::Durability, FileId, FilePosition, SourceDatabaseExt}; | 3 | use ra_db::{salsa::Durability, FileId, FilePosition, FileRange, SourceDatabaseExt}; |
4 | use rustc_hash::FxHashSet; | 4 | use rustc_hash::FxHashSet; |
5 | use std::sync::Arc; | 5 | use std::sync::Arc; |
6 | use test_utils::mark; | 6 | use test_utils::{mark, RangeOrOffset}; |
7 | 7 | ||
8 | fn parse_error_text(query: &str) -> String { | 8 | fn parse_error_text(query: &str) -> String { |
9 | format!("{}", query.parse::<SsrRule>().unwrap_err()) | 9 | format!("{}", query.parse::<SsrRule>().unwrap_err()) |
@@ -60,20 +60,32 @@ fn parser_undefined_placeholder_in_replacement() { | |||
60 | } | 60 | } |
61 | 61 | ||
62 | /// `code` may optionally contain a cursor marker `<|>`. If it doesn't, then the position will be | 62 | /// `code` may optionally contain a cursor marker `<|>`. If it doesn't, then the position will be |
63 | /// the start of the file. | 63 | /// the start of the file. If there's a second cursor marker, then we'll return a single range. |
64 | pub(crate) fn single_file(code: &str) -> (ra_ide_db::RootDatabase, FilePosition) { | 64 | pub(crate) fn single_file(code: &str) -> (ra_ide_db::RootDatabase, FilePosition, Vec<FileRange>) { |
65 | use ra_db::fixture::WithFixture; | 65 | use ra_db::fixture::WithFixture; |
66 | use ra_ide_db::symbol_index::SymbolsDatabase; | 66 | use ra_ide_db::symbol_index::SymbolsDatabase; |
67 | let (mut db, position) = if code.contains(test_utils::CURSOR_MARKER) { | 67 | let (mut db, file_id, range_or_offset) = if code.contains(test_utils::CURSOR_MARKER) { |
68 | ra_ide_db::RootDatabase::with_position(code) | 68 | ra_ide_db::RootDatabase::with_range_or_offset(code) |
69 | } else { | 69 | } else { |
70 | let (db, file_id) = ra_ide_db::RootDatabase::with_single_file(code); | 70 | let (db, file_id) = ra_ide_db::RootDatabase::with_single_file(code); |
71 | (db, FilePosition { file_id, offset: 0.into() }) | 71 | (db, file_id, RangeOrOffset::Offset(0.into())) |
72 | }; | 72 | }; |
73 | let selections; | ||
74 | let position; | ||
75 | match range_or_offset { | ||
76 | RangeOrOffset::Range(range) => { | ||
77 | position = FilePosition { file_id, offset: range.start() }; | ||
78 | selections = vec![FileRange { file_id, range: range }]; | ||
79 | } | ||
80 | RangeOrOffset::Offset(offset) => { | ||
81 | position = FilePosition { file_id, offset }; | ||
82 | selections = vec![]; | ||
83 | } | ||
84 | } | ||
73 | let mut local_roots = FxHashSet::default(); | 85 | let mut local_roots = FxHashSet::default(); |
74 | local_roots.insert(ra_db::fixture::WORKSPACE); | 86 | local_roots.insert(ra_db::fixture::WORKSPACE); |
75 | db.set_local_roots_with_durability(Arc::new(local_roots), Durability::HIGH); | 87 | db.set_local_roots_with_durability(Arc::new(local_roots), Durability::HIGH); |
76 | (db, position) | 88 | (db, position, selections) |
77 | } | 89 | } |
78 | 90 | ||
79 | fn assert_ssr_transform(rule: &str, input: &str, expected: Expect) { | 91 | fn assert_ssr_transform(rule: &str, input: &str, expected: Expect) { |
@@ -81,8 +93,8 @@ fn assert_ssr_transform(rule: &str, input: &str, expected: Expect) { | |||
81 | } | 93 | } |
82 | 94 | ||
83 | fn assert_ssr_transforms(rules: &[&str], input: &str, expected: Expect) { | 95 | fn assert_ssr_transforms(rules: &[&str], input: &str, expected: Expect) { |
84 | let (db, position) = single_file(input); | 96 | let (db, position, selections) = single_file(input); |
85 | let mut match_finder = MatchFinder::in_context(&db, position); | 97 | let mut match_finder = MatchFinder::in_context(&db, position, selections); |
86 | for rule in rules { | 98 | for rule in rules { |
87 | let rule: SsrRule = rule.parse().unwrap(); | 99 | let rule: SsrRule = rule.parse().unwrap(); |
88 | match_finder.add_rule(rule).unwrap(); | 100 | match_finder.add_rule(rule).unwrap(); |
@@ -112,8 +124,8 @@ fn print_match_debug_info(match_finder: &MatchFinder, file_id: FileId, snippet: | |||
112 | } | 124 | } |
113 | 125 | ||
114 | fn assert_matches(pattern: &str, code: &str, expected: &[&str]) { | 126 | fn assert_matches(pattern: &str, code: &str, expected: &[&str]) { |
115 | let (db, position) = single_file(code); | 127 | let (db, position, selections) = single_file(code); |
116 | let mut match_finder = MatchFinder::in_context(&db, position); | 128 | let mut match_finder = MatchFinder::in_context(&db, position, selections); |
117 | match_finder.add_search_pattern(pattern.parse().unwrap()).unwrap(); | 129 | match_finder.add_search_pattern(pattern.parse().unwrap()).unwrap(); |
118 | let matched_strings: Vec<String> = | 130 | let matched_strings: Vec<String> = |
119 | match_finder.matches().flattened().matches.iter().map(|m| m.matched_text()).collect(); | 131 | 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]) { | |||
124 | } | 136 | } |
125 | 137 | ||
126 | fn assert_no_match(pattern: &str, code: &str) { | 138 | fn assert_no_match(pattern: &str, code: &str) { |
127 | let (db, position) = single_file(code); | 139 | let (db, position, selections) = single_file(code); |
128 | let mut match_finder = MatchFinder::in_context(&db, position); | 140 | let mut match_finder = MatchFinder::in_context(&db, position, selections); |
129 | match_finder.add_search_pattern(pattern.parse().unwrap()).unwrap(); | 141 | match_finder.add_search_pattern(pattern.parse().unwrap()).unwrap(); |
130 | let matches = match_finder.matches().flattened().matches; | 142 | let matches = match_finder.matches().flattened().matches; |
131 | if !matches.is_empty() { | 143 | if !matches.is_empty() { |
@@ -135,8 +147,8 @@ fn assert_no_match(pattern: &str, code: &str) { | |||
135 | } | 147 | } |
136 | 148 | ||
137 | fn assert_match_failure_reason(pattern: &str, code: &str, snippet: &str, expected_reason: &str) { | 149 | fn assert_match_failure_reason(pattern: &str, code: &str, snippet: &str, expected_reason: &str) { |
138 | let (db, position) = single_file(code); | 150 | let (db, position, selections) = single_file(code); |
139 | let mut match_finder = MatchFinder::in_context(&db, position); | 151 | let mut match_finder = MatchFinder::in_context(&db, position, selections); |
140 | match_finder.add_search_pattern(pattern.parse().unwrap()).unwrap(); | 152 | match_finder.add_search_pattern(pattern.parse().unwrap()).unwrap(); |
141 | let mut reasons = Vec::new(); | 153 | let mut reasons = Vec::new(); |
142 | for d in match_finder.debug_where_text_equal(position.file_id, snippet) { | 154 | for d in match_finder.debug_where_text_equal(position.file_id, snippet) { |
@@ -490,9 +502,10 @@ fn no_match_split_expression() { | |||
490 | 502 | ||
491 | #[test] | 503 | #[test] |
492 | fn replace_function_call() { | 504 | fn replace_function_call() { |
505 | // This test also makes sure that we ignore empty-ranges. | ||
493 | assert_ssr_transform( | 506 | assert_ssr_transform( |
494 | "foo() ==>> bar()", | 507 | "foo() ==>> bar()", |
495 | "fn foo() {} fn bar() {} fn f1() {foo(); foo();}", | 508 | "fn foo() {<|><|>} fn bar() {} fn f1() {foo(); foo();}", |
496 | expect![["fn foo() {} fn bar() {} fn f1() {bar(); bar();}"]], | 509 | expect![["fn foo() {} fn bar() {} fn f1() {bar(); bar();}"]], |
497 | ); | 510 | ); |
498 | } | 511 | } |
@@ -922,3 +935,52 @@ fn replace_local_variable_reference() { | |||
922 | "#]], | 935 | "#]], |
923 | ) | 936 | ) |
924 | } | 937 | } |
938 | |||
939 | #[test] | ||
940 | fn replace_path_within_selection() { | ||
941 | assert_ssr_transform( | ||
942 | "foo ==>> bar", | ||
943 | r#" | ||
944 | fn main() { | ||
945 | let foo = 41; | ||
946 | let bar = 42; | ||
947 | do_stuff(foo); | ||
948 | do_stuff(foo);<|> | ||
949 | do_stuff(foo); | ||
950 | do_stuff(foo);<|> | ||
951 | do_stuff(foo); | ||
952 | }"#, | ||
953 | expect![[r#" | ||
954 | fn main() { | ||
955 | let foo = 41; | ||
956 | let bar = 42; | ||
957 | do_stuff(foo); | ||
958 | do_stuff(foo); | ||
959 | do_stuff(bar); | ||
960 | do_stuff(bar); | ||
961 | do_stuff(foo); | ||
962 | }"#]], | ||
963 | ); | ||
964 | } | ||
965 | |||
966 | #[test] | ||
967 | fn replace_nonpath_within_selection() { | ||
968 | mark::check!(replace_nonpath_within_selection); | ||
969 | assert_ssr_transform( | ||
970 | "$a + $b ==>> $b * $a", | ||
971 | r#" | ||
972 | fn main() { | ||
973 | let v = 1 + 2;<|> | ||
974 | let v2 = 3 + 3; | ||
975 | let v3 = 4 + 5;<|> | ||
976 | let v4 = 6 + 7; | ||
977 | }"#, | ||
978 | expect![[r#" | ||
979 | fn main() { | ||
980 | let v = 1 + 2; | ||
981 | let v2 = 3 * 3; | ||
982 | let v3 = 5 * 4; | ||
983 | let v4 = 6 + 7; | ||
984 | }"#]], | ||
985 | ); | ||
986 | } | ||