SSR: Wrap placeholder expansions in parenthesis when necessary
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.
This commit is contained in:
David Lattimore
parent
570fdf26c9
commit
fa1e411322
@ -3,8 +3,9 @@
|
||||
use crate::matching::Var;
|
||||
use crate::{resolving::ResolvedRule, Match, SsrMatches};
|
||||
use ra_syntax::ast::{self, AstToken};
|
||||
use ra_syntax::{SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextSize};
|
||||
use ra_syntax::{SyntaxElement, SyntaxKind, SyntaxNode, SyntaxToken, TextRange, TextSize};
|
||||
use ra_text_edit::TextEdit;
|
||||
use rustc_hash::{FxHashMap, FxHashSet};
|
||||
|
||||
/// Returns a text edit that will replace each match in `matches` with its corresponding replacement
|
||||
/// template. Placeholders in the template will have been substituted with whatever they matched to
|
||||
@ -38,62 +39,79 @@ struct ReplacementRenderer<'a> {
|
||||
file_src: &'a str,
|
||||
rules: &'a [ResolvedRule],
|
||||
rule: &'a ResolvedRule,
|
||||
out: String,
|
||||
// Map from a range within `out` to a token in `template` that represents a placeholder. This is
|
||||
// used to validate that the generated source code doesn't split any placeholder expansions (see
|
||||
// below).
|
||||
placeholder_tokens_by_range: FxHashMap<TextRange, SyntaxToken>,
|
||||
// Which placeholder tokens need to be wrapped in parenthesis in order to ensure that when `out`
|
||||
// is parsed, placeholders don't get split. e.g. if a template of `$a.to_string()` results in `1
|
||||
// + 2.to_string()` then the placeholder value `1 + 2` was split and needs parenthesis.
|
||||
placeholder_tokens_requiring_parenthesis: FxHashSet<SyntaxToken>,
|
||||
}
|
||||
|
||||
fn render_replace(match_info: &Match, file_src: &str, rules: &[ResolvedRule]) -> String {
|
||||
let mut out = String::new();
|
||||
let rule = &rules[match_info.rule_index];
|
||||
let template = rule
|
||||
.template
|
||||
.as_ref()
|
||||
.expect("You called MatchFinder::edits after calling MatchFinder::add_search_pattern");
|
||||
let renderer = ReplacementRenderer { match_info, file_src, rules, rule };
|
||||
renderer.render_node(&template.node, &mut out);
|
||||
let mut renderer = ReplacementRenderer {
|
||||
match_info,
|
||||
file_src,
|
||||
rules,
|
||||
rule,
|
||||
out: String::new(),
|
||||
placeholder_tokens_requiring_parenthesis: FxHashSet::default(),
|
||||
placeholder_tokens_by_range: FxHashMap::default(),
|
||||
};
|
||||
renderer.render_node(&template.node);
|
||||
renderer.maybe_rerender_with_extra_parenthesis(&template.node);
|
||||
for comment in &match_info.ignored_comments {
|
||||
out.push_str(&comment.syntax().to_string());
|
||||
renderer.out.push_str(&comment.syntax().to_string());
|
||||
}
|
||||
out
|
||||
renderer.out
|
||||
}
|
||||
|
||||
impl ReplacementRenderer<'_> {
|
||||
fn render_node_children(&self, node: &SyntaxNode, out: &mut String) {
|
||||
fn render_node_children(&mut self, node: &SyntaxNode) {
|
||||
for node_or_token in node.children_with_tokens() {
|
||||
self.render_node_or_token(&node_or_token, out);
|
||||
self.render_node_or_token(&node_or_token);
|
||||
}
|
||||
}
|
||||
|
||||
fn render_node_or_token(&self, node_or_token: &SyntaxElement, out: &mut String) {
|
||||
fn render_node_or_token(&mut self, node_or_token: &SyntaxElement) {
|
||||
match node_or_token {
|
||||
SyntaxElement::Token(token) => {
|
||||
self.render_token(&token, out);
|
||||
self.render_token(&token);
|
||||
}
|
||||
SyntaxElement::Node(child_node) => {
|
||||
self.render_node(&child_node, out);
|
||||
self.render_node(&child_node);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn render_node(&self, node: &SyntaxNode, out: &mut String) {
|
||||
fn render_node(&mut self, node: &SyntaxNode) {
|
||||
use ra_syntax::ast::AstNode;
|
||||
if let Some(mod_path) = self.match_info.rendered_template_paths.get(&node) {
|
||||
out.push_str(&mod_path.to_string());
|
||||
self.out.push_str(&mod_path.to_string());
|
||||
// Emit everything except for the segment's name-ref, since we already effectively
|
||||
// emitted that as part of `mod_path`.
|
||||
if let Some(path) = ast::Path::cast(node.clone()) {
|
||||
if let Some(segment) = path.segment() {
|
||||
for node_or_token in segment.syntax().children_with_tokens() {
|
||||
if node_or_token.kind() != SyntaxKind::NAME_REF {
|
||||
self.render_node_or_token(&node_or_token, out);
|
||||
self.render_node_or_token(&node_or_token);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
} else {
|
||||
self.render_node_children(&node, out);
|
||||
self.render_node_children(&node);
|
||||
}
|
||||
}
|
||||
|
||||
fn render_token(&self, token: &SyntaxToken, out: &mut String) {
|
||||
fn render_token(&mut self, token: &SyntaxToken) {
|
||||
if let Some(placeholder) = self.rule.get_placeholder(&token) {
|
||||
if let Some(placeholder_value) =
|
||||
self.match_info.placeholder_values.get(&Var(placeholder.ident.to_string()))
|
||||
@ -107,8 +125,23 @@ fn render_token(&self, token: &SyntaxToken, out: &mut String) {
|
||||
range.start(),
|
||||
self.rules,
|
||||
);
|
||||
let needs_parenthesis =
|
||||
self.placeholder_tokens_requiring_parenthesis.contains(token);
|
||||
edit.apply(&mut matched_text);
|
||||
out.push_str(&matched_text);
|
||||
if needs_parenthesis {
|
||||
self.out.push('(');
|
||||
}
|
||||
self.placeholder_tokens_by_range.insert(
|
||||
TextRange::new(
|
||||
TextSize::of(&self.out),
|
||||
TextSize::of(&self.out) + TextSize::of(&matched_text),
|
||||
),
|
||||
token.clone(),
|
||||
);
|
||||
self.out.push_str(&matched_text);
|
||||
if needs_parenthesis {
|
||||
self.out.push(')');
|
||||
}
|
||||
} else {
|
||||
// We validated that all placeholder references were valid before we
|
||||
// started, so this shouldn't happen.
|
||||
@ -118,7 +151,44 @@ fn render_token(&self, token: &SyntaxToken, out: &mut String) {
|
||||
);
|
||||
}
|
||||
} else {
|
||||
out.push_str(token.text().as_str());
|
||||
self.out.push_str(token.text().as_str());
|
||||
}
|
||||
}
|
||||
|
||||
// Checks if the resulting code, when parsed doesn't split any placeholders due to different
|
||||
// order of operations between the search pattern and the replacement template. If any do, then
|
||||
// we rerender the template and wrap the problematic placeholders with parenthesis.
|
||||
fn maybe_rerender_with_extra_parenthesis(&mut self, template: &SyntaxNode) {
|
||||
if let Some(node) = parse_as_kind(&self.out, template.kind()) {
|
||||
self.remove_node_ranges(node);
|
||||
if self.placeholder_tokens_by_range.is_empty() {
|
||||
return;
|
||||
}
|
||||
self.placeholder_tokens_requiring_parenthesis =
|
||||
self.placeholder_tokens_by_range.values().cloned().collect();
|
||||
self.out.clear();
|
||||
self.render_node(template);
|
||||
}
|
||||
}
|
||||
|
||||
fn remove_node_ranges(&mut self, node: SyntaxNode) {
|
||||
self.placeholder_tokens_by_range.remove(&node.text_range());
|
||||
for child in node.children() {
|
||||
self.remove_node_ranges(child);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_as_kind(code: &str, kind: SyntaxKind) -> Option<SyntaxNode> {
|
||||
use ra_syntax::ast::AstNode;
|
||||
if ast::Expr::can_cast(kind) {
|
||||
if let Ok(expr) = ast::Expr::parse(code) {
|
||||
return Some(expr.syntax().clone());
|
||||
}
|
||||
} else if ast::Item::can_cast(kind) {
|
||||
if let Ok(item) = ast::Item::parse(code) {
|
||||
return Some(item.syntax().clone());
|
||||
}
|
||||
}
|
||||
None
|
||||
}
|
||||
|
||||
@ -664,7 +664,7 @@ fn replace_binary_op() {
|
||||
assert_ssr_transform(
|
||||
"$a + $b ==>> $b + $a",
|
||||
"fn f() {1 + 2 + 3 + 4}",
|
||||
expect![["fn f() {4 + 3 + 2 + 1}"]],
|
||||
expect![[r#"fn f() {4 + (3 + (2 + 1))}"#]],
|
||||
);
|
||||
}
|
||||
|
||||
@ -773,11 +773,32 @@ fn f() {macro1!(1 * 2 + 3 + 4}
|
||||
macro_rules! macro1 {
|
||||
($a:expr) => {$a}
|
||||
}
|
||||
fn f() {macro1!(4 - 3 - 1 * 2}
|
||||
fn f() {macro1!(4 - (3 - 1 * 2)}
|
||||
"#]],
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn add_parenthesis_when_necessary() {
|
||||
assert_ssr_transform(
|
||||
"foo($a) ==>> $a.to_string()",
|
||||
r#"
|
||||
fn foo(_: i32) {}
|
||||
fn bar3(v: i32) {
|
||||
foo(1 + 2);
|
||||
foo(-v);
|
||||
}
|
||||
"#,
|
||||
expect![[r#"
|
||||
fn foo(_: i32) {}
|
||||
fn bar3(v: i32) {
|
||||
(1 + 2).to_string();
|
||||
(-v).to_string();
|
||||
}
|
||||
"#]],
|
||||
)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn match_failure_reasons() {
|
||||
let code = r#"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user