Merge #6587

bors[bot] · MarijnS95 · web-flow · commit ac123ac9e41c · 2021-01-04T11:14:40.000Z
6587: SSR: Support statement matching and replacing r=davidlattimore a=MarijnS95 For #3186 Hi! This is a smaller initial patchset that came up while working on support for statement lists (and my first time working on RA 😁). It has me stuck on trailing semicolons for which I hope to receive some feedback. Matching (and replacing) `let` bindings with a trailing semicolon works fine, but trying to omit these (to make patterns more ergonomic) turns out more complex than expected. The "optional trailing semicolon solution" implemented in this PR is ugly because `Matcher::attempt_match_token` should only consume a trailing `;` when parsing `let` bindings to prevent other code from breaking. That at the same time has a nasty side-effect of `;` ending up in the matched code: any replacements on that should include the trailing semicolon as well even if it was not in the pattern. A better example is in the tests: https://github.com/rust-analyzer/rust-analyzer/blob/3ae1649c24a689473b874c331f5f176e5839978e/crates/ssr/src/tests.rs#L178-L184 The end result to achieve is (I guess) allowing replacement of let bindings without trailing semicolon like `let x = $a ==>> let x = 1` (but including them on both sides is still fine), and should make replacement in a macro call (where `foo!(let a = 2;)` for a `$x:stmt` is invalid syntax) possible as well. That should allow to enable/fix these tests: https://github.com/rust-analyzer/rust-analyzer/blob/3ae1649c24a689473b874c331f5f176e5839978e/crates/ssr/src/tests.rs#L201-L214 A possible MVP of this PR might be to drop this optional `;' handling entirely and only allow an SSR pattern/template with semicolons on either side. Co-authored-by: Marijn Suijten <marijn@traverseresearch.nl>
diff --git a/crates/parser/src/grammar.rs b/crates/parser/src/grammar.rs
@@ -66,6 +66,10 @@ pub(crate) mod fragments {
         expressions::stmt(p, expressions::StmtWithSemi::No)
     }
 
+    pub(crate) fn stmt_optional_semi(p: &mut Parser) {
+        expressions::stmt(p, expressions::StmtWithSemi::Optional)
+    }
+
     pub(crate) fn opt_visibility(p: &mut Parser) {
         let _ = super::opt_visibility(p);
     }
diff --git a/crates/parser/src/lib.rs b/crates/parser/src/lib.rs
@@ -88,6 +88,7 @@ pub enum FragmentKind {
     Path,
     Expr,
     Statement,
+    StatementOptionalSemi,
     Type,
     Pattern,
     Item,
@@ -118,6 +119,7 @@ pub fn parse_fragment(
         FragmentKind::Visibility => grammar::fragments::opt_visibility,
         FragmentKind::MetaItem => grammar::fragments::meta_item,
         FragmentKind::Statement => grammar::fragments::stmt,
+        FragmentKind::StatementOptionalSemi => grammar::fragments::stmt_optional_semi,
         FragmentKind::Items => grammar::fragments::macro_items,
         FragmentKind::Statements => grammar::fragments::macro_stmts,
         FragmentKind::Attr => grammar::fragments::attr,
diff --git a/crates/ssr/src/parsing.rs b/crates/ssr/src/parsing.rs
@@ -78,6 +78,7 @@ impl ParsedRule {
         builder.try_add(ast::Item::parse(&raw_pattern), raw_template.map(ast::Item::parse));
         builder.try_add(ast::Path::parse(&raw_pattern), raw_template.map(ast::Path::parse));
         builder.try_add(ast::Pat::parse(&raw_pattern), raw_template.map(ast::Pat::parse));
+        builder.try_add(ast::Stmt::parse(&raw_pattern), raw_template.map(ast::Stmt::parse));
         builder.build()
     }
 }
diff --git a/crates/ssr/src/tests.rs b/crates/ssr/src/tests.rs
@@ -159,6 +159,50 @@ fn assert_match_failure_reason(pattern: &str, code: &str, snippet: &str, expecte
     assert_eq!(reasons, vec![expected_reason]);
 }
 
+#[test]
+fn ssr_let_stmt_in_macro_match() {
+    assert_matches(
+        "let a = 0",
+        r#"
+            macro_rules! m1 { ($a:stmt) => {$a}; }
+            fn f() {m1!{ let a = 0 };}"#,
+        // FIXME: Whitespace is not part of the matched block
+        &["leta=0"],
+    );
+}
+
+#[test]
+fn ssr_let_stmt_in_fn_match() {
+    assert_matches("let $a = 10;", "fn main() { let x = 10; x }", &["let x = 10;"]);
+    assert_matches("let $a = $b;", "fn main() { let x = 10; x }", &["let x = 10;"]);
+}
+
+#[test]
+fn ssr_block_expr_match() {
+    assert_matches("{ let $a = $b; }", "fn main() { let x = 10; }", &["{ let x = 10; }"]);
+    assert_matches("{ let $a = $b; $c }", "fn main() { let x = 10; x }", &["{ let x = 10; x }"]);
+}
+
+#[test]
+fn ssr_let_stmt_replace() {
+    // Pattern and template with trailing semicolon
+    assert_ssr_transform(
+        "let $a = $b; ==>> let $a = 11;",
+        "fn main() { let x = 10; x }",
+        expect![["fn main() { let x = 11; x }"]],
+    );
+}
+
+#[test]
+fn ssr_let_stmt_replace_expr() {
+    // Trailing semicolon should be dropped from the new expression
+    assert_ssr_transform(
+        "let $a = $b; ==>> $b",
+        "fn main() { let x = 10; }",
+        expect![["fn main() { 10 }"]],
+    );
+}
+
 #[test]
 fn ssr_function_to_method() {
     assert_ssr_transform(
diff --git a/crates/syntax/src/lib.rs b/crates/syntax/src/lib.rs
@@ -212,6 +212,13 @@ impl ast::Attr {
     }
 }
 
+impl ast::Stmt {
+    /// Returns `text`, parsed as statement, but only if it has no errors.
+    pub fn parse(text: &str) -> Result<Self, ()> {
+        parsing::parse_text_fragment(text, parser::FragmentKind::StatementOptionalSemi)
+    }
+}
+
 /// Matches a `SyntaxNode` against an `ast` type.
 ///
 /// # Example:
diff --git a/crates/syntax/src/tests.rs b/crates/syntax/src/tests.rs
@@ -102,6 +102,15 @@ fn type_parser_tests() {
     );
 }
 
+#[test]
+fn stmt_parser_tests() {
+    fragment_parser_dir_test(
+        &["parser/fragments/stmt/ok"],
+        &["parser/fragments/stmt/err"],
+        crate::ast::Stmt::parse,
+    );
+}
+
 #[test]
 fn parser_fuzz_tests() {
     for (_, text) in collect_rust_files(&test_data_dir(), &["parser/fuzz-failures"]) {
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_attr.rast b/crates/syntax/test_data/parser/fragments/stmt/err/0000_attr.rast
@@ -0,0 +1 @@
+ERROR
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_attr.rs b/crates/syntax/test_data/parser/fragments/stmt/err/0000_attr.rs
@@ -0,0 +1 @@
+#[foo]
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_multiple_stmts.rast b/crates/syntax/test_data/parser/fragments/stmt/err/0000_multiple_stmts.rast
@@ -0,0 +1 @@
+ERROR
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_multiple_stmts.rs b/crates/syntax/test_data/parser/fragments/stmt/err/0000_multiple_stmts.rs
@@ -0,0 +1 @@
+a(); b(); c()
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_open_parenthesis.rast b/crates/syntax/test_data/parser/fragments/stmt/err/0000_open_parenthesis.rast
@@ -0,0 +1 @@
+ERROR
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_open_parenthesis.rs b/crates/syntax/test_data/parser/fragments/stmt/err/0000_open_parenthesis.rs
@@ -0,0 +1 @@
+(
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_semicolon.rast b/crates/syntax/test_data/parser/fragments/stmt/err/0000_semicolon.rast
@@ -0,0 +1 @@
+ERROR
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_semicolon.rs b/crates/syntax/test_data/parser/fragments/stmt/err/0000_semicolon.rs
@@ -0,0 +1 @@
+;
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_unterminated_expr.rast b/crates/syntax/test_data/parser/fragments/stmt/err/0000_unterminated_expr.rast
@@ -0,0 +1 @@
+ERROR
diff --git a/crates/syntax/test_data/parser/fragments/stmt/err/0000_unterminated_expr.rs b/crates/syntax/test_data/parser/fragments/stmt/err/0000_unterminated_expr.rs
@@ -0,0 +1 @@
+1 +
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_expr.rast b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_expr.rast
@@ -0,0 +1,9 @@
+EXPR_STMT@0..5
+  BIN_EXPR@0..5
+    LITERAL@0..1
+      INT_NUMBER@0..1 "1"
+    WHITESPACE@1..2 " "
+    PLUS@2..3 "+"
+    WHITESPACE@3..4 " "
+    LITERAL@4..5
+      INT_NUMBER@4..5 "1"
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_expr.rs b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_expr.rs
@@ -0,0 +1 @@
+1 + 1
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_expr_block.rast b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_expr_block.rast
@@ -0,0 +1,69 @@
+EXPR_STMT@0..55
+  BLOCK_EXPR@0..55
+    L_CURLY@0..1 "{"
+    WHITESPACE@1..6 "\n    "
+    LET_STMT@6..20
+      LET_KW@6..9 "let"
+      WHITESPACE@9..10 " "
+      IDENT_PAT@10..11
+        NAME@10..11
+          IDENT@10..11 "x"
+      WHITESPACE@11..12 " "
+      EQ@12..13 "="
+      WHITESPACE@13..14 " "
+      CALL_EXPR@14..19
+        PATH_EXPR@14..17
+          PATH@14..17
+            PATH_SEGMENT@14..17
+              NAME_REF@14..17
+                IDENT@14..17 "foo"
+        ARG_LIST@17..19
+          L_PAREN@17..18 "("
+          R_PAREN@18..19 ")"
+      SEMICOLON@19..20 ";"
+    WHITESPACE@20..25 "\n    "
+    LET_STMT@25..39
+      LET_KW@25..28 "let"
+      WHITESPACE@28..29 " "
+      IDENT_PAT@29..30
+        NAME@29..30
+          IDENT@29..30 "y"
+      WHITESPACE@30..31 " "
+      EQ@31..32 "="
+      WHITESPACE@32..33 " "
+      CALL_EXPR@33..38
+        PATH_EXPR@33..36
+          PATH@33..36
+            PATH_SEGMENT@33..36
+              NAME_REF@33..36
+                IDENT@33..36 "bar"
+        ARG_LIST@36..38
+          L_PAREN@36..37 "("
+          R_PAREN@37..38 ")"
+      SEMICOLON@38..39 ";"
+    WHITESPACE@39..44 "\n    "
+    CALL_EXPR@44..53
+      PATH_EXPR@44..46
+        PATH@44..46
+          PATH_SEGMENT@44..46
+            NAME_REF@44..46
+              IDENT@44..46 "Ok"
+      ARG_LIST@46..53
+        L_PAREN@46..47 "("
+        BIN_EXPR@47..52
+          PATH_EXPR@47..48
+            PATH@47..48
+              PATH_SEGMENT@47..48
+                NAME_REF@47..48
+                  IDENT@47..48 "x"
+          WHITESPACE@48..49 " "
+          PLUS@49..50 "+"
+          WHITESPACE@50..51 " "
+          PATH_EXPR@51..52
+            PATH@51..52
+              PATH_SEGMENT@51..52
+                NAME_REF@51..52
+                  IDENT@51..52 "y"
+        R_PAREN@52..53 ")"
+    WHITESPACE@53..54 "\n"
+    R_CURLY@54..55 "}"
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_expr_block.rs b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_expr_block.rs
@@ -0,0 +1,5 @@
+{
+    let x = foo();
+    let y = bar();
+    Ok(x + y)
+}
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_fn_call.rast b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_fn_call.rast
@@ -0,0 +1,11 @@
+EXPR_STMT@0..6
+  CALL_EXPR@0..5
+    PATH_EXPR@0..3
+      PATH@0..3
+        PATH_SEGMENT@0..3
+          NAME_REF@0..3
+            IDENT@0..3 "foo"
+    ARG_LIST@3..5
+      L_PAREN@3..4 "("
+      R_PAREN@4..5 ")"
+  SEMICOLON@5..6 ";"
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_fn_call.rs b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_fn_call.rs
@@ -0,0 +1 @@
+foo();
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_let_stmt.rast b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_let_stmt.rast
@@ -0,0 +1,12 @@
+LET_STMT@0..11
+  LET_KW@0..3 "let"
+  WHITESPACE@3..4 " "
+  IDENT_PAT@4..5
+    NAME@4..5
+      IDENT@4..5 "x"
+  WHITESPACE@5..6 " "
+  EQ@6..7 "="
+  WHITESPACE@7..8 " "
+  LITERAL@8..10
+    INT_NUMBER@8..10 "10"
+  SEMICOLON@10..11 ";"
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_let_stmt.rs b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_let_stmt.rs
@@ -0,0 +1 @@
+let x = 10;
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_macro_let_stmt.rast b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_macro_let_stmt.rast
@@ -0,0 +1,21 @@
+EXPR_STMT@0..18
+  MACRO_CALL@0..17
+    PATH@0..2
+      PATH_SEGMENT@0..2
+        NAME_REF@0..2
+          IDENT@0..2 "m1"
+    BANG@2..3 "!"
+    TOKEN_TREE@3..17
+      L_CURLY@3..4 "{"
+      WHITESPACE@4..5 " "
+      LET_KW@5..8 "let"
+      WHITESPACE@8..9 " "
+      IDENT@9..10 "a"
+      WHITESPACE@10..11 " "
+      EQ@11..12 "="
+      WHITESPACE@12..13 " "
+      INT_NUMBER@13..14 "0"
+      SEMICOLON@14..15 ";"
+      WHITESPACE@15..16 " "
+      R_CURLY@16..17 "}"
+  SEMICOLON@17..18 ";"
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_macro_let_stmt.rs b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_macro_let_stmt.rs
@@ -0,0 +1 @@
+m1!{ let a = 0; };
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_macro_unterminated_let_stmt.rast b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_macro_unterminated_let_stmt.rast
@@ -0,0 +1,21 @@
+EXPR_STMT@0..18
+  MACRO_CALL@0..17
+    PATH@0..2
+      PATH_SEGMENT@0..2
+        NAME_REF@0..2
+          IDENT@0..2 "m1"
+    BANG@2..3 "!"
+    TOKEN_TREE@3..17
+      L_CURLY@3..4 "{"
+      WHITESPACE@4..5 " "
+      LET_KW@5..8 "let"
+      WHITESPACE@8..9 " "
+      IDENT@9..10 "a"
+      WHITESPACE@10..11 " "
+      EQ@11..12 "="
+      WHITESPACE@12..13 " "
+      INT_NUMBER@13..14 "0"
+      SEMICOLON@14..15 ";"
+      WHITESPACE@15..16 " "
+      R_CURLY@16..17 "}"
+  SEMICOLON@17..18 ";"
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_macro_unterminated_let_stmt.rs b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_macro_unterminated_let_stmt.rs
@@ -0,0 +1 @@
+m1!{ let a = 0; };
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_struct_item.rast b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_struct_item.rast
@@ -0,0 +1,22 @@
+STRUCT@0..28
+  STRUCT_KW@0..6 "struct"
+  WHITESPACE@6..7 " "
+  NAME@7..10
+    IDENT@7..10 "Foo"
+  WHITESPACE@10..11 " "
+  RECORD_FIELD_LIST@11..28
+    L_CURLY@11..12 "{"
+    WHITESPACE@12..17 "\n    "
+    RECORD_FIELD@17..25
+      NAME@17..20
+        IDENT@17..20 "bar"
+      COLON@20..21 ":"
+      WHITESPACE@21..22 " "
+      PATH_TYPE@22..25
+        PATH@22..25
+          PATH_SEGMENT@22..25
+            NAME_REF@22..25
+              IDENT@22..25 "u32"
+    COMMA@25..26 ","
+    WHITESPACE@26..27 "\n"
+    R_CURLY@27..28 "}"
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_struct_item.rs b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_struct_item.rs
@@ -0,0 +1,3 @@
+struct Foo {
+    bar: u32,
+}
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_unterminated_fn_call.rast b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_unterminated_fn_call.rast
@@ -0,0 +1,10 @@
+EXPR_STMT@0..5
+  CALL_EXPR@0..5
+    PATH_EXPR@0..3
+      PATH@0..3
+        PATH_SEGMENT@0..3
+          NAME_REF@0..3
+            IDENT@0..3 "foo"
+    ARG_LIST@3..5
+      L_PAREN@3..4 "("
+      R_PAREN@4..5 ")"
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_unterminated_fn_call.rs b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_unterminated_fn_call.rs
@@ -0,0 +1 @@
+foo()
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_unterminated_let_stmt.rast b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_unterminated_let_stmt.rast
@@ -0,0 +1,11 @@
+LET_STMT@0..10
+  LET_KW@0..3 "let"
+  WHITESPACE@3..4 " "
+  IDENT_PAT@4..5
+    NAME@4..5
+      IDENT@4..5 "x"
+  WHITESPACE@5..6 " "
+  EQ@6..7 "="
+  WHITESPACE@7..8 " "
+  LITERAL@8..10
+    INT_NUMBER@8..10 "10"
diff --git a/crates/syntax/test_data/parser/fragments/stmt/ok/0000_unterminated_let_stmt.rs b/crates/syntax/test_data/parser/fragments/stmt/ok/0000_unterminated_let_stmt.rs
@@ -0,0 +1 @@
+let x = 10

Original file line number	Diff line number	Diff line change
`@@ -66,6 +66,10 @@ pub(crate) mod fragments {`
`66`	`66`	`expressions::stmt(p, expressions::StmtWithSemi::No)`
`67`	`67`	`}`
`68`	`68`
	`69`	`+ pub(crate) fn stmt_optional_semi(p: &mut Parser) {`
	`70`	`+ expressions::stmt(p, expressions::StmtWithSemi::Optional)`
	`71`	`+ }`
	`72`	`+`
`69`	`73`	`pub(crate) fn opt_visibility(p: &mut Parser) {`
`70`	`74`	`let _ = super::opt_visibility(p);`
`71`	`75`	`}`
Original file line number	Diff line number	Diff line change
`@@ -78,6 +78,7 @@ impl ParsedRule {`
`78`	`78`	`builder.try_add(ast::Item::parse(&raw_pattern), raw_template.map(ast::Item::parse));`
`79`	`79`	`builder.try_add(ast::Path::parse(&raw_pattern), raw_template.map(ast::Path::parse));`
`80`	`80`	`builder.try_add(ast::Pat::parse(&raw_pattern), raw_template.map(ast::Pat::parse));`
	`81`	`+ builder.try_add(ast::Stmt::parse(&raw_pattern), raw_template.map(ast::Stmt::parse));`
`81`	`82`	`builder.build()`
`82`	`83`	`}`
`83`	`84`	`}`
Original file line number	Diff line number	Diff line change
`@@ -212,6 +212,13 @@ impl ast::Attr {`
`212`	`212`	`}`
`213`	`213`	`}`
`214`	`214`
	`215`	`+impl ast::Stmt {`
	`216`	+ /// Returns `text`, parsed as statement, but only if it has no errors.
	`217`	`+ pub fn parse(text: &str) -> Result<Self, ()> {`
	`218`	`+ parsing::parse_text_fragment(text, parser::FragmentKind::StatementOptionalSemi)`
	`219`	`+ }`
	`220`	`+}`
	`221`	`+`
`215`	`222`	/// Matches a `SyntaxNode` against an `ast` type.
`216`	`223`	`///`
`217`	`224`	`/// # Example:`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,9 @@ @@
 +[email protected]
 +  [email protected]
 +    [email protected]
 +      [email protected] "1"
 +    [email protected] " "
 +    [email protected] "+"
 +    [email protected] " "
 +    [email protected]
 +      [email protected] "1"