From e865d45904e1f2a60f81fa84288b15c696feee4c Mon Sep 17 00:00:00 2001
From: Yutaro Ohno <yutaro.ono.418@gmail.com>
Date: Fri, 8 Dec 2023 19:05:27 +0900
Subject: [PATCH] fix: Recover from missing argument in call expressions

Previously, when parsing an argument list with a missing argument (e.g.,
`(a, , b)` in `foo(a, , b)`), the parser would stop upon an unexpected
token (at the second comma in the example), resulting in an incorrect
parse tree.

This commit improves error handling in such cases, ensuring a more
accurate parse tree is built.
---
 crates/hir-expand/src/fixup.rs                | 14 --------
 crates/parser/src/grammar/expressions.rs      | 33 ++++++++++++++-----
 .../inline/err/0015_arg_list_recovery.rast    | 27 +++++++++++++++
 .../inline/err/0015_arg_list_recovery.rs      |  1 +
 4 files changed, 53 insertions(+), 22 deletions(-)

diff --git a/crates/hir-expand/src/fixup.rs b/crates/hir-expand/src/fixup.rs
index a3b2c700ff..ed29411a6b 100644
--- a/crates/hir-expand/src/fixup.rs
+++ b/crates/hir-expand/src/fixup.rs
@@ -631,20 +631,6 @@ 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(
diff --git a/crates/parser/src/grammar/expressions.rs b/crates/parser/src/grammar/expressions.rs
index e99c111d39..52a08c4fb1 100644
--- a/crates/parser/src/grammar/expressions.rs
+++ b/crates/parser/src/grammar/expressions.rs
@@ -611,6 +611,7 @@ fn cast_expr(p: &mut Parser<'_>, lhs: CompletedMarker) -> CompletedMarker {
 //     foo(bar::);
 //     foo(bar:);
 //     foo(bar+);
+//     foo(a, , b);
 // }
 fn arg_list(p: &mut Parser<'_>) {
     assert!(p.at(T!['(']));
@@ -619,14 +620,30 @@ fn arg_list(p: &mut Parser<'_>) {
     // fn main() {
     //     foo(#[attr] 92)
     // }
-    delimited(
-        p,
-        T!['('],
-        T![')'],
-        T![,],
-        EXPR_FIRST.union(ATTRIBUTE_FIRST),
-        |p: &mut Parser<'_>| expr(p).is_some(),
-    );
+    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![')']);
     m.complete(p, ARG_LIST);
 }
 
diff --git a/crates/parser/test_data/parser/inline/err/0015_arg_list_recovery.rast b/crates/parser/test_data/parser/inline/err/0015_arg_list_recovery.rast
index 5d0fe859c2..85def9e148 100644
--- a/crates/parser/test_data/parser/inline/err/0015_arg_list_recovery.rast
+++ b/crates/parser/test_data/parser/inline/err/0015_arg_list_recovery.rast
@@ -68,6 +68,32 @@ SOURCE_FILE
                 PLUS "+"
               R_PAREN ")"
           SEMICOLON ";"
+        WHITESPACE "\n    "
+        EXPR_STMT
+          CALL_EXPR
+            PATH_EXPR
+              PATH
+                PATH_SEGMENT
+                  NAME_REF
+                    IDENT "foo"
+            ARG_LIST
+              L_PAREN "("
+              PATH_EXPR
+                PATH
+                  PATH_SEGMENT
+                    NAME_REF
+                      IDENT "a"
+              COMMA ","
+              WHITESPACE " "
+              COMMA ","
+              WHITESPACE " "
+              PATH_EXPR
+                PATH
+                  PATH_SEGMENT
+                    NAME_REF
+                      IDENT "b"
+              R_PAREN ")"
+          SEMICOLON ";"
         WHITESPACE "\n"
         R_CURLY "}"
   WHITESPACE "\n"
@@ -75,3 +101,4 @@ error 25: expected identifier
 error 39: expected COMMA
 error 39: expected expression
 error 55: expected expression
+error 68: expected expression
diff --git a/crates/parser/test_data/parser/inline/err/0015_arg_list_recovery.rs b/crates/parser/test_data/parser/inline/err/0015_arg_list_recovery.rs
index 0e7ac9cc30..175a31f8b5 100644
--- a/crates/parser/test_data/parser/inline/err/0015_arg_list_recovery.rs
+++ b/crates/parser/test_data/parser/inline/err/0015_arg_list_recovery.rs
@@ -2,4 +2,5 @@ fn main() {
     foo(bar::);
     foo(bar:);
     foo(bar+);
+    foo(a, , b);
 }