Recover from missing slots in delimited parsing

Author: Veykril

crates/hir-expand/src/fixup.rs

@@ -631,6 +631,20 @@ fn foo () {a . b ; bar () ;}
         )
     }
 
+    #[test]
+    fn extraneous_comma() {
+        check(
+            r#"
+fn foo() {
+    bar(,);
+}
+"#,
+            expect![[r#"
+fn foo () {__ra_fixup ;}
+"#]],
+        )
+    }
+
     #[test]
     fn fixup_if_1() {
         check(

crates/parser/src/grammar.rs

@@ -393,11 +393,26 @@ fn delimited(
     bra: SyntaxKind,
     ket: SyntaxKind,
     delim: SyntaxKind,
+    unexpected_delim_message: impl Fn() -> String,
     first_set: TokenSet,
     mut parser: impl FnMut(&mut Parser<'_>) -> bool,
 ) {
     p.bump(bra);
     while !p.at(ket) && !p.at(EOF) {
+        if p.at(delim) {
+            // Recover if an argument is missing and only got a delimiter,
+            // e.g. `(a, , b)`.
+
+            // Wrap the erroneous delimiter in an error node so that fixup logic gets rid of it.
+            // FIXME: Ideally this should be handled in fixup in a structured way, but our list
+            // nodes currently have no concept of a missing node between two delimiters.
+            // So doing it this way is easier.
+            let m = p.start();
+            p.error(unexpected_delim_message());
+            p.bump(delim);
+            m.complete(p, ERROR);
+            continue;
+        }
         if !parser(p) {
             break;
         }

crates/parser/src/grammar/expressions.rs

@@ -620,30 +620,15 @@ fn arg_list(p: &mut Parser<'_>) {
     // fn main() {
     //     foo(#[attr] 92)
     // }
-    p.bump(T!['(']);
-    while !p.at(T![')']) && !p.at(EOF) {
-        if p.at(T![,]) {
-            // Recover if an argument is missing and only got a delimiter,
-            // e.g. `(a, , b)`.
-            p.error("expected expression");
-            p.bump(T![,]);
-            continue;
-        }
-
-        if expr(p).is_none() {
-            break;
-        }
-        if !p.at(T![,]) {
-            if p.at_ts(EXPR_FIRST.union(ATTRIBUTE_FIRST)) {
-                p.error(format!("expected {:?}", T![,]));
-            } else {
-                break;
-            }
-        } else {
-            p.bump(T![,]);
-        }
-    }
-    p.expect(T![')']);
+    delimited(
+        p,
+        T!['('],
+        T![')'],
+        T![,],
+        || "expected expression".into(),
+        EXPR_FIRST.union(ATTRIBUTE_FIRST),
+        |p| expr(p).is_some(),
+    );
     m.complete(p, ARG_LIST);
 }
 

crates/parser/src/grammar/generic_args.rs

@@ -1,5 +1,7 @@
 use super::*;
 
+// test_err generic_arg_list_recover
+// type T = T<0, ,T>;
 pub(super) fn opt_generic_arg_list(p: &mut Parser<'_>, colon_colon_required: bool) {
     let m;
     if p.at(T![::]) && p.nth(2) == T![<] {
@@ -11,7 +13,15 @@ pub(super) fn opt_generic_arg_list(p: &mut Parser<'_>, colon_colon_required: boo
         return;
     }
 
-    delimited(p, T![<], T![>], T![,], GENERIC_ARG_FIRST, generic_arg);
+    delimited(
+        p,
+        T![<],
+        T![>],
+        T![,],
+        || "expected generic argument".into(),
+        GENERIC_ARG_FIRST,
+        generic_arg,
+    );
     m.complete(p, GENERIC_ARG_LIST);
 }
 

crates/parser/src/grammar/generic_params.rs

@@ -10,16 +10,27 @@ pub(super) fn opt_generic_param_list(p: &mut Parser<'_>) {
 
 // test generic_param_list
 // fn f<T: Clone>() {}
+
+// test_err generic_param_list_recover
+// fn f<T: Clone,, U:, V>() {}
 fn generic_param_list(p: &mut Parser<'_>) {
     assert!(p.at(T![<]));
     let m = p.start();
-    delimited(p, T![<], T![>], T![,], GENERIC_PARAM_FIRST.union(ATTRIBUTE_FIRST), |p| {
-        // test generic_param_attribute
-        // fn foo<#[lt_attr] 'a, #[t_attr] T>() {}
-        let m = p.start();
-        attributes::outer_attrs(p);
-        generic_param(p, m)
-    });
+    delimited(
+        p,
+        T![<],
+        T![>],
+        T![,],
+        || "expected generic parameter".into(),
+        GENERIC_PARAM_FIRST.union(ATTRIBUTE_FIRST),
+        |p| {
+            // test generic_param_attribute
+            // fn foo<#[lt_attr] 'a, #[t_attr] T>() {}
+            let m = p.start();
+            attributes::outer_attrs(p);
+            generic_param(p, m)
+        },
+    );
 
     m.complete(p, GENERIC_PARAM_LIST);
 }

crates/parser/src/grammar/items/adt.rs

@@ -146,28 +146,39 @@ pub(crate) fn record_field_list(p: &mut Parser<'_>) {
 const TUPLE_FIELD_FIRST: TokenSet =
     types::TYPE_FIRST.union(ATTRIBUTE_FIRST).union(VISIBILITY_FIRST);
 
+// test_err tuple_field_list_recovery
+// struct S(struct S;
+// struct S(A,,B);
 fn tuple_field_list(p: &mut Parser<'_>) {
     assert!(p.at(T!['(']));
     let m = p.start();
-    delimited(p, T!['('], T![')'], T![,], TUPLE_FIELD_FIRST, |p| {
-        let m = p.start();
-        // test tuple_field_attrs
-        // struct S (#[attr] f32);
-        attributes::outer_attrs(p);
-        let has_vis = opt_visibility(p, true);
-        if !p.at_ts(types::TYPE_FIRST) {
-            p.error("expected a type");
-            if has_vis {
-                m.complete(p, ERROR);
-            } else {
-                m.abandon(p);
+    delimited(
+        p,
+        T!['('],
+        T![')'],
+        T![,],
+        || "expected tuple field".into(),
+        TUPLE_FIELD_FIRST,
+        |p| {
+            let m = p.start();
+            // test tuple_field_attrs
+            // struct S (#[attr] f32);
+            attributes::outer_attrs(p);
+            let has_vis = opt_visibility(p, true);
+            if !p.at_ts(types::TYPE_FIRST) {
+                p.error("expected a type");
+                if has_vis {
+                    m.complete(p, ERROR);
+                } else {
+                    m.abandon(p);
+                }
+                return false;
             }
-            return false;
-        }
-        types::type_(p);
-        m.complete(p, TUPLE_FIELD);
-        true
-    });
+            types::type_(p);
+            m.complete(p, TUPLE_FIELD);
+            true
+        },
+    );
 
     m.complete(p, TUPLE_FIELD_LIST);
 }

crates/parser/src/grammar/items/use_item.rs

@@ -93,9 +93,16 @@ pub(crate) fn use_tree_list(p: &mut Parser<'_>) {
     // use b;
     // struct T;
     // fn test() {}
-    delimited(p, T!['{'], T!['}'], T![,], USE_TREE_LIST_FIRST_SET, |p: &mut Parser<'_>| {
-        use_tree(p, false) || p.at_ts(USE_TREE_LIST_RECOVERY_SET)
-    });
+    // use {a ,, b};
+    delimited(
+        p,
+        T!['{'],
+        T!['}'],
+        T![,],
+        || "expected use tree".into(),
+        USE_TREE_LIST_FIRST_SET,
+        |p: &mut Parser<'_>| use_tree(p, false) || p.at_ts(USE_TREE_LIST_RECOVERY_SET),
+    );
 
     m.complete(p, USE_TREE_LIST);
 }

crates/parser/test_data/parser/inline/err/0015_arg_list_recovery.rast

@@ -85,7 +85,8 @@ SOURCE_FILE
                       IDENT "a"
               COMMA ","
               WHITESPACE " "
-              COMMA ","
+              ERROR
+                COMMA ","
               WHITESPACE " "
               PATH_EXPR
                 PATH
@@ -101,4 +102,4 @@ error 25: expected identifier
 error 39: expected COMMA
 error 39: expected expression
 error 55: expected expression
-error 68: expected expression
+error 69: expected expression

crates/parser/test_data/parser/inline/err/0026_use_tree_list_err_recovery.rast

@@ -43,4 +43,29 @@ SOURCE_FILE
         L_CURLY "{"
         R_CURLY "}"
   WHITESPACE "\n"
+  USE
+    USE_KW "use"
+    WHITESPACE " "
+    USE_TREE
+      USE_TREE_LIST
+        L_CURLY "{"
+        USE_TREE
+          PATH
+            PATH_SEGMENT
+              NAME_REF
+                IDENT "a"
+        WHITESPACE " "
+        COMMA ","
+        ERROR
+          COMMA ","
+        WHITESPACE " "
+        USE_TREE
+          PATH
+            PATH_SEGMENT
+              NAME_REF
+                IDENT "b"
+        R_CURLY "}"
+    SEMICOLON ";"
+  WHITESPACE "\n"
 error 6: expected R_CURLY
+error 46: expected use tree

crates/parser/test_data/parser/inline/err/0026_use_tree_list_err_recovery.rs

@@ -2,3 +2,4 @@ use {a;
 use b;
 struct T;
 fn test() {}
+use {a ,, b};