From 467af611fb5b1add25b36a2127b172240bc141cf Mon Sep 17 00:00:00 2001 From: David Lattimore Date: Mon, 22 Jun 2020 18:15:51 +1000 Subject: SSR: Allow matching of whole macro calls Matching within macro calls is to come later and matching of macro calls within macro calls later still. --- crates/ra_ssr/src/lib.rs | 18 +++---- crates/ra_ssr/src/matching.rs | 105 +++++++++++++++++++++++++++++++++++++++-- crates/ra_ssr/src/replacing.rs | 10 +++- crates/ra_ssr/src/tests.rs | 53 +++++++++++++++++++++ 4 files changed, 173 insertions(+), 13 deletions(-) (limited to 'crates/ra_ssr') diff --git a/crates/ra_ssr/src/lib.rs b/crates/ra_ssr/src/lib.rs index fc716ae82..da26ee669 100644 --- a/crates/ra_ssr/src/lib.rs +++ b/crates/ra_ssr/src/lib.rs @@ -91,14 +91,16 @@ impl<'db> MatchFinder<'db> { if let Ok(mut m) = matching::get_match(false, rule, &code, restrict_range, &self.sema) { // Continue searching in each of our placeholders. for placeholder_value in m.placeholder_values.values_mut() { - // Don't search our placeholder if it's the entire matched node, otherwise we'd - // find the same match over and over until we got a stack overflow. - if placeholder_value.node != *code { - self.find_matches( - &placeholder_value.node, - restrict_range, - &mut placeholder_value.inner_matches, - ); + if let Some(placeholder_node) = &placeholder_value.node { + // Don't search our placeholder if it's the entire matched node, otherwise we'd + // find the same match over and over until we got a stack overflow. + if placeholder_node != code { + self.find_matches( + placeholder_node, + restrict_range, + &mut placeholder_value.inner_matches, + ); + } } } matches_out.matches.push(m); diff --git a/crates/ra_ssr/src/matching.rs b/crates/ra_ssr/src/matching.rs index 265b6d793..bdaba9f1b 100644 --- a/crates/ra_ssr/src/matching.rs +++ b/crates/ra_ssr/src/matching.rs @@ -61,8 +61,9 @@ pub(crate) struct Var(pub String); /// Information about a placeholder bound in a match. #[derive(Debug)] pub(crate) struct PlaceholderMatch { - /// The node that the placeholder matched to. - pub(crate) node: SyntaxNode, + /// The node that the placeholder matched to. If set, then we'll search for further matches + /// within this node. It isn't set when we match tokens within a macro call's token tree. + pub(crate) node: Option, pub(crate) range: FileRange, /// More matches, found within `node`. pub(crate) inner_matches: SsrMatches, @@ -195,6 +196,7 @@ impl<'db, 'sema> MatchState<'db, 'sema> { SyntaxKind::RECORD_FIELD_LIST => { self.attempt_match_record_field_list(match_inputs, pattern, code) } + SyntaxKind::TOKEN_TREE => self.attempt_match_token_tree(match_inputs, pattern, code), _ => self.attempt_match_node_children(match_inputs, pattern, code), } } @@ -340,6 +342,90 @@ impl<'db, 'sema> MatchState<'db, 'sema> { Ok(()) } + /// Outside of token trees, a placeholder can only match a single AST node, whereas in a token + /// tree it can match a sequence of tokens. + fn attempt_match_token_tree( + &mut self, + match_inputs: &MatchInputs, + pattern: &SyntaxNode, + code: &ra_syntax::SyntaxNode, + ) -> Result<(), MatchFailed> { + let mut pattern = PatternIterator::new(pattern).peekable(); + let mut children = code.children_with_tokens(); + while let Some(child) = children.next() { + if let Some(placeholder) = pattern.peek().and_then(|p| match_inputs.get_placeholder(p)) + { + pattern.next(); + let next_pattern_token = pattern + .peek() + .and_then(|p| match p { + SyntaxElement::Token(t) => Some(t.clone()), + SyntaxElement::Node(n) => n.first_token(), + }) + .map(|p| p.text().to_string()); + let first_matched_token = child.clone(); + let mut last_matched_token = child; + // Read code tokens util we reach one equal to the next token from our pattern + // or we reach the end of the token tree. + while let Some(next) = children.next() { + match &next { + SyntaxElement::Token(t) => { + if Some(t.to_string()) == next_pattern_token { + pattern.next(); + break; + } + } + SyntaxElement::Node(n) => { + if let Some(first_token) = n.first_token() { + if Some(first_token.to_string()) == next_pattern_token { + if let Some(SyntaxElement::Node(p)) = pattern.next() { + // We have a subtree that starts with the next token in our pattern. + self.attempt_match_token_tree(match_inputs, &p, &n)?; + break; + } + } + } + } + }; + last_matched_token = next; + } + if let Some(match_out) = &mut self.match_out { + match_out.placeholder_values.insert( + Var(placeholder.ident.to_string()), + PlaceholderMatch::from_range(FileRange { + file_id: self.sema.original_range(code).file_id, + range: first_matched_token + .text_range() + .cover(last_matched_token.text_range()), + }), + ); + } + continue; + } + // Match literal (non-placeholder) tokens. + match child { + SyntaxElement::Token(token) => { + self.attempt_match_token(&mut pattern, &token)?; + } + SyntaxElement::Node(node) => match pattern.next() { + Some(SyntaxElement::Node(p)) => { + self.attempt_match_token_tree(match_inputs, &p, &node)?; + } + Some(SyntaxElement::Token(p)) => fail_match!( + "Pattern has token '{}', code has subtree '{}'", + p.text(), + node.text() + ), + None => fail_match!("Pattern has nothing, code has '{}'", node.text()), + }, + } + } + if let Some(p) = pattern.next() { + fail_match!("Reached end of token tree in code, but pattern still has {:?}", p); + } + Ok(()) + } + fn next_non_trivial(&mut self, code_it: &mut SyntaxElementChildren) -> Option { loop { let c = code_it.next(); @@ -399,7 +485,11 @@ fn recording_match_fail_reasons() -> bool { impl PlaceholderMatch { fn new(node: &SyntaxNode, range: FileRange) -> Self { - Self { node: node.clone(), range, inner_matches: SsrMatches::default() } + Self { node: Some(node.clone()), range, inner_matches: SsrMatches::default() } + } + + fn from_range(range: FileRange) -> Self { + Self { node: None, range, inner_matches: SsrMatches::default() } } } @@ -484,7 +574,14 @@ mod tests { 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); - assert_eq!(matches.matches[0].placeholder_values[&Var("x".to_string())].node.text(), "1+2"); + assert_eq!( + matches.matches[0].placeholder_values[&Var("x".to_string())] + .node + .as_ref() + .unwrap() + .text(), + "1+2" + ); let edit = crate::replacing::matches_to_edit(&matches); let mut after = input.to_string(); diff --git a/crates/ra_ssr/src/replacing.rs b/crates/ra_ssr/src/replacing.rs index 81a5e06a9..5dcde82a2 100644 --- a/crates/ra_ssr/src/replacing.rs +++ b/crates/ra_ssr/src/replacing.rs @@ -24,6 +24,7 @@ fn matches_to_edit_at_offset(matches: &SsrMatches, relative_start: TextSize) -> fn render_replace(match_info: &Match) -> String { let mut out = String::new(); + let match_start = match_info.matched_node.text_range().start(); for r in &match_info.template.tokens { match r { PatternElement::Token(t) => out.push_str(t.text.as_str()), @@ -32,7 +33,14 @@ fn render_replace(match_info: &Match) -> String { match_info.placeholder_values.get(&Var(p.ident.to_string())) { let range = &placeholder_value.range.range; - let mut matched_text = placeholder_value.node.text().to_string(); + let mut matched_text = if let Some(node) = &placeholder_value.node { + node.text().to_string() + } else { + let relative_range = range.checked_sub(match_start).unwrap(); + match_info.matched_node.text().to_string() + [usize::from(relative_range.start())..usize::from(relative_range.end())] + .to_string() + }; let edit = matches_to_edit_at_offset(&placeholder_value.inner_matches, range.start()); edit.apply(&mut matched_text); diff --git a/crates/ra_ssr/src/tests.rs b/crates/ra_ssr/src/tests.rs index 4b747fe18..3ee1e74e9 100644 --- a/crates/ra_ssr/src/tests.rs +++ b/crates/ra_ssr/src/tests.rs @@ -426,6 +426,45 @@ fn match_reordered_struct_instantiation() { assert_no_match("Foo {a: 1, z: 9}", "fn f() {Foo {a: 1}}"); } +#[test] +fn match_macro_invocation() { + assert_matches("foo!($a)", "fn() {foo(foo!(foo()))}", &["foo!(foo())"]); + assert_matches("foo!(41, $a, 43)", "fn() {foo!(41, 42, 43)}", &["foo!(41, 42, 43)"]); + assert_no_match("foo!(50, $a, 43)", "fn() {foo!(41, 42, 43}"); + assert_no_match("foo!(41, $a, 50)", "fn() {foo!(41, 42, 43}"); + assert_matches("foo!($a())", "fn() {foo!(bar())}", &["foo!(bar())"]); +} + +// When matching within a macro expansion, we only allow matches of nodes that originated from +// the macro call, not from the macro definition. +#[test] +fn no_match_expression_from_macro() { + assert_no_match( + "$a.clone()", + r#" + macro_rules! m1 { + () => {42.clone()} + } + fn f1() {m1!()} + "#, + ); +} + +// We definitely don't want to allow matching of an expression that part originates from the +// macro call `42` and part from the macro definition `.clone()`. +#[test] +fn no_match_split_expression() { + assert_no_match( + "$a.clone()", + r#" + macro_rules! m1 { + ($x:expr) => {$x.clone()} + } + fn f1() {m1!(42)} + "#, + ); +} + #[test] fn replace_function_call() { assert_ssr_transform("foo() ==>> bar()", "fn f1() {foo(); foo();}", "fn f1() {bar(); bar();}"); @@ -467,6 +506,20 @@ fn replace_struct_init() { ); } +#[test] +fn replace_macro_invocations() { + assert_ssr_transform( + "try!($a) ==>> $a?", + "fn f1() -> Result<(), E> {bar(try!(foo()));}", + "fn f1() -> Result<(), E> {bar(foo()?);}", + ); + assert_ssr_transform( + "foo!($a($b)) ==>> foo($b, $a)", + "fn f1() {foo!(abc(def() + 2));}", + "fn f1() {foo(def() + 2, abc);}", + ); +} + #[test] fn replace_binary_op() { assert_ssr_transform( -- cgit v1.2.3