aboutsummaryrefslogtreecommitdiff
path: root/crates
diff options
context:
space:
mode:
authorbors[bot] <26634292+bors[bot]@users.noreply.github.com>2020-07-30 12:36:50 +0100
committerGitHub <[email protected]>2020-07-30 12:36:50 +0100
commit9042009b7f1ba0f85e892ac5184fa4542d0c10f5 (patch)
treeb56e925c4f822f5479350883c87b952300995fc6 /crates
parentbe803efb7c7ba257716fcc97c57ecfd07e278b07 (diff)
parentfa1e4113224c119258670538f8c3ca6c8ea8ad1e (diff)
Merge #5567
5567: SSR: Wrap placeholder expansions in parenthesis when necessary r=matklad a=davidlattimore e.g. `foo($a) ==> $a.to_string()` should produce `(1 + 2).to_string()` not `1 + 2.to_string()` We don't yet try to determine if the whole replacement needs to be wrapped in parenthesis. That's harder and I think perhaps less often an issue. Co-authored-by: David Lattimore <[email protected]>
Diffstat (limited to 'crates')
-rw-r--r--crates/ra_ssr/src/replacing.rs106
-rw-r--r--crates/ra_ssr/src/tests.rs25
2 files changed, 111 insertions, 20 deletions
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 @@
3use crate::matching::Var; 3use crate::matching::Var;
4use crate::{resolving::ResolvedRule, Match, SsrMatches}; 4use crate::{resolving::ResolvedRule, Match, SsrMatches};
5use ra_syntax::ast::{self, AstToken}; 5use ra_syntax::ast::{self, AstToken};
6use ra_syntax::{SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextSize}; 6use ra_syntax::{SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize};
7use ra_text_edit::TextEdit; 7use ra_text_edit::TextEdit;
8use rustc_hash::{FxHashMap, FxHashSet};
8 9
9/// Returns a text edit that will replace each match in `matches` with its corresponding replacement 10/// Returns a text edit that will replace each match in `matches` with its corresponding replacement
10/// template. Placeholders in the template will have been substituted with whatever they matched to 11/// template. Placeholders in the template will have been substituted with whatever they matched to
@@ -38,62 +39,79 @@ struct ReplacementRenderer<'a> {
38 file_src: &'a str, 39 file_src: &'a str,
39 rules: &'a [ResolvedRule], 40 rules: &'a [ResolvedRule],
40 rule: &'a ResolvedRule, 41 rule: &'a ResolvedRule,
42 out: String,
43 // Map from a range within `out` to a token in `template` that represents a placeholder. This is
44 // used to validate that the generated source code doesn't split any placeholder expansions (see
45 // below).
46 placeholder_tokens_by_range: FxHashMap<TextRange, SyntaxToken>,
47 // Which placeholder tokens need to be wrapped in parenthesis in order to ensure that when `out`
48 // is parsed, placeholders don't get split. e.g. if a template of `$a.to_string()` results in `1
49 // + 2.to_string()` then the placeholder value `1 + 2` was split and needs parenthesis.
50 placeholder_tokens_requiring_parenthesis: FxHashSet<SyntaxToken>,
41} 51}
42 52
43fn render_replace(match_info: &Match, file_src: &str, rules: &[ResolvedRule]) -> String { 53fn render_replace(match_info: &Match, file_src: &str, rules: &[ResolvedRule]) -> String {
44 let mut out = String::new();
45 let rule = &rules[match_info.rule_index]; 54 let rule = &rules[match_info.rule_index];
46 let template = rule 55 let template = rule
47 .template 56 .template
48 .as_ref() 57 .as_ref()
49 .expect("You called MatchFinder::edits after calling MatchFinder::add_search_pattern"); 58 .expect("You called MatchFinder::edits after calling MatchFinder::add_search_pattern");
50 let renderer = ReplacementRenderer { match_info, file_src, rules, rule }; 59 let mut renderer = ReplacementRenderer {
51 renderer.render_node(&template.node, &mut out); 60 match_info,
61 file_src,
62 rules,
63 rule,
64 out: String::new(),
65 placeholder_tokens_requiring_parenthesis: FxHashSet::default(),
66 placeholder_tokens_by_range: FxHashMap::default(),
67 };
68 renderer.render_node(&template.node);
69 renderer.maybe_rerender_with_extra_parenthesis(&template.node);
52 for comment in &match_info.ignored_comments { 70 for comment in &match_info.ignored_comments {
53 out.push_str(&comment.syntax().to_string()); 71 renderer.out.push_str(&comment.syntax().to_string());
54 } 72 }
55 out 73 renderer.out
56} 74}
57 75
58impl ReplacementRenderer<'_> { 76impl ReplacementRenderer<'_> {
59 fn render_node_children(&self, node: &SyntaxNode, out: &mut String) { 77 fn render_node_children(&mut self, node: &SyntaxNode) {
60 for node_or_token in node.children_with_tokens() { 78 for node_or_token in node.children_with_tokens() {
61 self.render_node_or_token(&node_or_token, out); 79 self.render_node_or_token(&node_or_token);
62 } 80 }
63 } 81 }
64 82
65 fn render_node_or_token(&self, node_or_token: &SyntaxElement, out: &mut String) { 83 fn render_node_or_token(&mut self, node_or_token: &SyntaxElement) {
66 match node_or_token { 84 match node_or_token {
67 SyntaxElement::Token(token) => { 85 SyntaxElement::Token(token) => {
68 self.render_token(&token, out); 86 self.render_token(&token);
69 } 87 }
70 SyntaxElement::Node(child_node) => { 88 SyntaxElement::Node(child_node) => {
71 self.render_node(&child_node, out); 89 self.render_node(&child_node);
72 } 90 }
73 } 91 }
74 } 92 }
75 93
76 fn render_node(&self, node: &SyntaxNode, out: &mut String) { 94 fn render_node(&mut self, node: &SyntaxNode) {
77 use ra_syntax::ast::AstNode; 95 use ra_syntax::ast::AstNode;
78 if let Some(mod_path) = self.match_info.rendered_template_paths.get(&node) { 96 if let Some(mod_path) = self.match_info.rendered_template_paths.get(&node) {
79 out.push_str(&mod_path.to_string()); 97 self.out.push_str(&mod_path.to_string());
80 // Emit everything except for the segment's name-ref, since we already effectively 98 // Emit everything except for the segment's name-ref, since we already effectively
81 // emitted that as part of `mod_path`. 99 // emitted that as part of `mod_path`.
82 if let Some(path) = ast::Path::cast(node.clone()) { 100 if let Some(path) = ast::Path::cast(node.clone()) {
83 if let Some(segment) = path.segment() { 101 if let Some(segment) = path.segment() {
84 for node_or_token in segment.syntax().children_with_tokens() { 102 for node_or_token in segment.syntax().children_with_tokens() {
85 if node_or_token.kind() != SyntaxKind::NAME_REF { 103 if node_or_token.kind() != SyntaxKind::NAME_REF {
86 self.render_node_or_token(&node_or_token, out); 104 self.render_node_or_token(&node_or_token);
87 } 105 }
88 } 106 }
89 } 107 }
90 } 108 }
91 } else { 109 } else {
92 self.render_node_children(&node, out); 110 self.render_node_children(&node);
93 } 111 }
94 } 112 }
95 113
96 fn render_token(&self, token: &SyntaxToken, out: &mut String) { 114 fn render_token(&mut self, token: &SyntaxToken) {
97 if let Some(placeholder) = self.rule.get_placeholder(&token) { 115 if let Some(placeholder) = self.rule.get_placeholder(&token) {
98 if let Some(placeholder_value) = 116 if let Some(placeholder_value) =
99 self.match_info.placeholder_values.get(&Var(placeholder.ident.to_string())) 117 self.match_info.placeholder_values.get(&Var(placeholder.ident.to_string()))
@@ -107,8 +125,23 @@ impl ReplacementRenderer<'_> {
107 range.start(), 125 range.start(),
108 self.rules, 126 self.rules,
109 ); 127 );
128 let needs_parenthesis =
129 self.placeholder_tokens_requiring_parenthesis.contains(token);
110 edit.apply(&mut matched_text); 130 edit.apply(&mut matched_text);
111 out.push_str(&matched_text); 131 if needs_parenthesis {
132 self.out.push('(');
133 }
134 self.placeholder_tokens_by_range.insert(
135 TextRange::new(
136 TextSize::of(&self.out),
137 TextSize::of(&self.out) + TextSize::of(&matched_text),
138 ),
139 token.clone(),
140 );
141 self.out.push_str(&matched_text);
142 if needs_parenthesis {
143 self.out.push(')');
144 }
112 } else { 145 } else {
113 // We validated that all placeholder references were valid before we 146 // We validated that all placeholder references were valid before we
114 // started, so this shouldn't happen. 147 // started, so this shouldn't happen.
@@ -118,7 +151,44 @@ impl ReplacementRenderer<'_> {
118 ); 151 );
119 } 152 }
120 } else { 153 } else {
121 out.push_str(token.text().as_str()); 154 self.out.push_str(token.text().as_str());
155 }
156 }
157
158 // Checks if the resulting code, when parsed doesn't split any placeholders due to different
159 // order of operations between the search pattern and the replacement template. If any do, then
160 // we rerender the template and wrap the problematic placeholders with parenthesis.
161 fn maybe_rerender_with_extra_parenthesis(&mut self, template: &SyntaxNode) {
162 if let Some(node) = parse_as_kind(&self.out, template.kind()) {
163 self.remove_node_ranges(node);
164 if self.placeholder_tokens_by_range.is_empty() {
165 return;
166 }
167 self.placeholder_tokens_requiring_parenthesis =
168 self.placeholder_tokens_by_range.values().cloned().collect();
169 self.out.clear();
170 self.render_node(template);
171 }
172 }
173
174 fn remove_node_ranges(&mut self, node: SyntaxNode) {
175 self.placeholder_tokens_by_range.remove(&node.text_range());
176 for child in node.children() {
177 self.remove_node_ranges(child);
178 }
179 }
180}
181
182fn parse_as_kind(code: &str, kind: SyntaxKind) -> Option<SyntaxNode> {
183 use ra_syntax::ast::AstNode;
184 if ast::Expr::can_cast(kind) {
185 if let Ok(expr) = ast::Expr::parse(code) {
186 return Some(expr.syntax().clone());
187 }
188 } else if ast::Item::can_cast(kind) {
189 if let Ok(item) = ast::Item::parse(code) {
190 return Some(item.syntax().clone());
122 } 191 }
123 } 192 }
193 None
124} 194}
diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs
index f5ffff7cc..a4fa2cb44 100644
--- a/crates/ra_ssr/src/tests.rs
+++ b/crates/ra_ssr/src/tests.rs
@@ -664,7 +664,7 @@ fn replace_binary_op() {
664 assert_ssr_transform( 664 assert_ssr_transform(
665 "$a + $b ==>> $b + $a", 665 "$a + $b ==>> $b + $a",
666 "fn f() {1 + 2 + 3 + 4}", 666 "fn f() {1 + 2 + 3 + 4}",
667 expect![["fn f() {4 + 3 + 2 + 1}"]], 667 expect![[r#"fn f() {4 + (3 + (2 + 1))}"#]],
668 ); 668 );
669} 669}
670 670
@@ -773,12 +773,33 @@ fn preserves_whitespace_within_macro_expansion() {
773 macro_rules! macro1 { 773 macro_rules! macro1 {
774 ($a:expr) => {$a} 774 ($a:expr) => {$a}
775 } 775 }
776 fn f() {macro1!(4 - 3 - 1 * 2} 776 fn f() {macro1!(4 - (3 - 1 * 2)}
777 "#]], 777 "#]],
778 ) 778 )
779} 779}
780 780
781#[test] 781#[test]
782fn add_parenthesis_when_necessary() {
783 assert_ssr_transform(
784 "foo($a) ==>> $a.to_string()",
785 r#"
786 fn foo(_: i32) {}
787 fn bar3(v: i32) {
788 foo(1 + 2);
789 foo(-v);
790 }
791 "#,
792 expect![[r#"
793 fn foo(_: i32) {}
794 fn bar3(v: i32) {
795 (1 + 2).to_string();
796 (-v).to_string();
797 }
798 "#]],
799 )
800}
801
802#[test]
782fn match_failure_reasons() { 803fn match_failure_reasons() {
783 let code = r#" 804 let code = r#"
784 fn bar() {} 805 fn bar() {}