From 07ccb12ba85f18e515a33a1dc75b688bf8ea1dbc Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20Muska=C5=82a?= <micmus@whatsapp.com>
Date: Wed, 29 Jan 2025 03:49:13 -0800
Subject: [PATCH] Support trailing commas and semis

They will be just removed when formatting, allowing more "sloppy" code
before reformatting, increasing usability of erlfmt.
---
 src/erlfmt_parse.yrl         |  18 ++++
 test/erlfmt_format_SUITE.erl | 195 +++++++++++++++++++++++++++++------
 2 files changed, 180 insertions(+), 33 deletions(-)

diff --git a/src/erlfmt_parse.yrl b/src/erlfmt_parse.yrl
index ad9d0fb..63e565e 100644
--- a/src/erlfmt_parse.yrl
+++ b/src/erlfmt_parse.yrl
@@ -118,6 +118,7 @@ spec_fun -> atom_or_var_or_macro : '$1'.
 spec_fun -> atom_or_var_or_macro ':' atom_or_var_or_macro : {remote, ?range_anno('$1', '$3'), '$1', '$3'}.
 
 type_sigs -> type_sig : {['$1'], ?anno('$1')}.
+type_sigs -> type_sig ';' : {['$1'], ?anno('$1')}.
 type_sigs -> type_sig ';' type_sigs : {['$1' | ?val('$3')], ?anno('$3')}.
 
 type_sig -> type_argument_list '->' type :
@@ -195,6 +196,7 @@ function -> function_clauses :
     {function, ?anno(hd('$1')), '$1'}.
 
 function_clauses -> function_clause : ['$1'].
+function_clauses -> function_clause ';' : ['$1'].
 function_clauses -> function_clause ';' function_clauses : ['$1' | '$3'].
 
 function_clause -> atom_or_var pat_argument_list clause_guard clause_body :
@@ -292,6 +294,7 @@ list -> '[' ']' : {list, ?range_anno('$1', '$2'), []}.
 list -> '[' list_exprs ']' : {list, ?range_anno('$1', '$3'), '$2'}.
 
 list_exprs -> expr : ['$1'].
+list_exprs -> expr ',' : ['$1'].
 list_exprs -> expr '|' expr : [{cons, ?range_anno('$1', '$3'), '$1', '$3'}].
 list_exprs -> expr ',' list_exprs : ['$1' | '$3'].
 
@@ -299,6 +302,7 @@ binary -> '<<' '>>' : {bin,?range_anno('$1', '$2'),[]}.
 binary -> '<<' bin_elements '>>' : {bin,?range_anno('$1', '$3'),'$2'}.
 
 bin_elements -> bin_element : ['$1'].
+bin_elements -> bin_element ',' : ['$1'].
 bin_elements -> bin_element ',' bin_elements : ['$1'|'$3'].
 
 bin_element -> bit_expr :
@@ -340,6 +344,7 @@ binary_comprehension -> '<<' expr_max '||' lc_exprs '>>' :
     {bc, ?range_anno('$1', '$5'), '$2', '$4'}.
 
 lc_exprs -> lc_expr : ['$1'].
+lc_exprs -> lc_expr ',' : ['$1'].
 lc_exprs -> lc_expr ',' lc_exprs : ['$1'|'$3'].
 
 lc_expr -> expr : '$1'.
@@ -389,6 +394,7 @@ record_tuple -> '{' '}' : {[], ?anno('$2')}.
 record_tuple -> '{' record_fields '}' : {'$2', ?anno('$3')}.
 
 record_fields -> record_field : ['$1'].
+record_fields -> record_field ',' : ['$1'].
 record_fields -> record_field ',' record_fields : ['$1' | '$3'].
 
 record_field -> record_field_name '=' expr : {record_field,?range_anno('$1', '$3'),'$1','$3'}.
@@ -403,6 +409,7 @@ function_call -> expr_max_remote argument_list :
 if_expr -> 'if' if_clauses 'end' : {'if',?range_anno('$1', '$3'),'$2'}.
 
 if_clauses -> if_clause : ['$1'].
+if_clauses -> if_clause ';' : ['$1'].
 if_clauses -> if_clause ';' if_clauses : ['$1' | '$3'].
 
 if_clause -> guard clause_body :
@@ -412,6 +419,7 @@ case_expr -> 'case' expr 'of' cr_clauses 'end' :
     {'case', ?range_anno('$1', '$5'), '$2', '$4'}.
 
 cr_clauses -> cr_clause : ['$1'].
+cr_clauses -> cr_clause ';' : ['$1'].
 cr_clauses -> cr_clause ';' cr_clauses : ['$1' | '$3'].
 
 maybe_expr -> 'maybe' exprs 'end' :
@@ -461,6 +469,7 @@ integer_or_var_or_macro -> var : '$1'.
 integer_or_var_or_macro -> macro_call_none : '$1'.
 
 fun_clauses -> fun_clause : ['$1'].
+fun_clauses -> fun_clause ';' : ['$1'].
 fun_clauses -> fun_clause ';' fun_clauses : ['$1' | '$3'].
 
 fun_clause -> pat_argument_list clause_guard clause_body :
@@ -488,6 +497,7 @@ try_catch -> 'after' exprs 'end' :
         {none, ?anno('$3'), '$1', '$2'}.
 
 try_clauses -> try_clause : ['$1'].
+try_clauses -> try_clause ';' : ['$1'].
 try_clauses -> try_clause ';' try_clauses : ['$1' | '$3'].
 
 try_clause -> pat_expr clause_guard clause_body :
@@ -573,30 +583,38 @@ type_argument_list -> '(' ')' : {[], ?range_anno('$1', '$2')}.
 type_argument_list -> '(' types ')' : {'$2', ?range_anno('$1', '$3')}.
 
 anno_exprs -> expr : {['$1'], ?anno('$1')}.
+anno_exprs -> expr ',' : {['$1'], ?anno('$1')}.
 anno_exprs -> expr ',' anno_exprs : {['$1' | ?val('$3')], ?anno('$3')}.
 
 exprs -> expr : ['$1'].
+exprs -> expr ',' : ['$1'].
 exprs -> expr ',' exprs : ['$1' | '$3'].
 
 pat_exprs -> pat_expr : ['$1'].
+pat_exprs -> pat_expr ',' : ['$1'].
 pat_exprs -> pat_expr ',' pat_exprs : ['$1' | '$3'].
 
 anno_types -> type : {['$1'], ?anno('$1')}.
 anno_types -> type ',' anno_types : {['$1' | ?val('$3')], ?range_anno('$1', '$3')}.
 
 types -> type : ['$1'].
+types -> type ',' : ['$1'].
 types -> type ',' types : ['$1' | '$3'].
 
 macro_exprs -> macro_expr : ['$1'].
+macro_exprs -> macro_expr ',' : ['$1'].
 macro_exprs -> macro_expr ',' macro_exprs : ['$1' | '$3'].
 
 vars -> var : ['$1'].
+vars -> var ',' : ['$1'].
 vars -> var ',' vars : ['$1' | '$3'].
 
 guard -> guard_or : {guard_or, ?range_anno(hd(?val('$1')), '$1'), ?val('$1')}.
 
 guard_or -> anno_exprs :
     {[{guard_and, ?range_anno(hd(?val('$1')), '$1'), ?val('$1')}], ?anno('$1')}.
+guard_or -> anno_exprs ';' :
+    {[{guard_and, ?range_anno(hd(?val('$1')), '$1'), ?val('$1')}], ?anno('$1')}.
 guard_or -> anno_exprs ';' guard_or :
     {[{guard_and, ?range_anno(hd(?val('$1')), '$1'), ?val('$1')} | ?val('$3')], ?anno('$3')}.
 
diff --git a/test/erlfmt_format_SUITE.erl b/test/erlfmt_format_SUITE.erl
index 7fa0b0f..b24ee7c 100644
--- a/test/erlfmt_format_SUITE.erl
+++ b/test/erlfmt_format_SUITE.erl
@@ -1223,7 +1223,8 @@ tuple(Config) when is_list(Config) ->
         "            line(expr_to_algebra(Value), comments_to_algebra(Comments))\n"
         "        )\n"
         "    ])}\n"
-    ).
+    ),
+    ?assertFormat("{1,}\n", "{1}\n").
 
 %% tagged vs untagged tuples
 untagged_tuple(Config) when is_list(Config) ->
@@ -1698,7 +1699,8 @@ list(Config) when is_list(Config) ->
         "    % comment below\n"
         "    | Rest\n"
         "].\n"
-    ).
+    ),
+    ?assertFormat("[1,]\n", "[1]\n").
 
 binary(Config) when is_list(Config) ->
     ?assertFormat("<< >>", "<<>>\n"),
@@ -1730,7 +1732,8 @@ binary(Config) when is_list(Config) ->
         "        333\n"
         "    >>\n"
         ">>\n"
-    ).
+    ),
+    ?assertFormat("<<1,>>\n", "<<1>>\n").
 
 map_create(Config) when is_list(Config) ->
     ?assertFormat("#{\n}", "#{}\n"),
@@ -1765,22 +1768,6 @@ map_create(Config) when is_list(Config) ->
         "#{\n"
         "    ?FOO\n"
         "}\n"
-    ).
-
-map_update(Config) when is_list(Config) ->
-    ?assertFormat("X # {\n}", "X#{}\n"),
-    ?assertSame("#{}#{}\n"),
-    ?assertSame("#{}#{}#{}\n"),
-    ?assertSame("(X#foo.bar)#{}\n"),
-    ?assertSame("(catch 1)#{}\n"),
-    ?assertSame("X#{A => B, C := D}\n"),
-    ?assertFormat(
-        "X#{11 => 22, 33 => 44}",
-        "X#{\n"
-        "    11 => 22,\n"
-        "    33 => 44\n"
-        "}\n",
-        15
     ),
     ?assertFormat(
         "#{55 => 66, 77 => 88}#{11 => 22, 33 => 44}",
@@ -1802,7 +1789,25 @@ map_update(Config) when is_list(Config) ->
         "        e\n"
         "    )\n"
         "}\n"
-    ).
+    ),
+    ?assertFormat("#{a => 1,}\n", "#{a => 1}\n").
+
+map_update(Config) when is_list(Config) ->
+    ?assertFormat("X # {\n}", "X#{}\n"),
+    ?assertSame("#{}#{}\n"),
+    ?assertSame("#{}#{}#{}\n"),
+    ?assertSame("(X#foo.bar)#{}\n"),
+    ?assertSame("(catch 1)#{}\n"),
+    ?assertSame("X#{A => B, C := D}\n"),
+    ?assertFormat(
+        "X#{11 => 22, 33 => 44}",
+        "X#{\n"
+        "    11 => 22,\n"
+        "    33 => 44\n"
+        "}\n",
+        15
+    ),
+    ?assertFormat("X#{a => 1,}\n", "X#{a => 1}\n").
 
 record_create(Config) when is_list(Config) ->
     ?assertFormat("#foo{\n}", "#foo{}\n"),
@@ -1862,7 +1867,8 @@ record_create(Config) when is_list(Config) ->
         "    c = 3\n"
         "}\n",
         15
-    ).
+    ),
+    ?assertFormat("#foo{a = 1,}\n", "#foo{a = 1}\n").
 
 record_update(Config) when is_list(Config) ->
     ?assertFormat("X #foo {\n}", "X#foo{}\n"),
@@ -1890,7 +1896,8 @@ record_update(Config) when is_list(Config) ->
         15
     ),
     ?assertSame("X#?FOO{}\n"),
-    ?assertSame("X?FOO{}\n").
+    ?assertSame("X?FOO{}\n"),
+    ?assertFormat("X#foo{a = 1,}\n", "X#foo{a = 1}\n").
 
 record_index(Config) when is_list(Config) ->
     ?assertSame("#foo.bar\n"),
@@ -2145,7 +2152,8 @@ list_comprehension(Config) when is_list(Config) ->
         "    Value\n"
         " || {string, _, Value} <- ValuesR\n"
         "]).\n"
-    ).
+    ),
+    ?assertFormat("[A || A <- List,]\n", "[A || A <- List]\n").
 
 map_comprehension(Config) when is_list(Config) ->
     ?assertFormat("#{X=>X||X<-List}", "#{X => X || X <- List}\n"),
@@ -2213,7 +2221,8 @@ map_comprehension(Config) when is_list(Config) ->
         "    X < 10\n"
         "}\n",
         25
-    ).
+    ),
+    ?assertFormat("#{A => A || A := B <- Map,}\n", "#{A => A || A := B <- Map}\n").
 
 binary_comprehension(Config) when is_list(Config) ->
     ?assertFormat("<<X||X<-List>>", "<<X || X <- List>>\n"),
@@ -2259,7 +2268,8 @@ binary_comprehension(Config) when is_list(Config) ->
         "    <<ALong, B>>\n"
         " || ALong = {_, _, _, {B, _}} <- All, lists:member(B, Keep)\n"
         ">>)\n"
-    ).
+    ),
+    ?assertFormat("<<A || <<A>> <= Bin,>>\n", "<<A || <<A>> <= Bin>>\n").
 
 call(Config) when is_list(Config) ->
     ?assertFormat("foo(\n)", "foo()\n"),
@@ -2364,7 +2374,8 @@ call(Config) when is_list(Config) ->
         "    Long,\n"
         "    Arguments\n"
         ")\n"
-    ).
+    ),
+    ?assertFormat("foo(1,)\n", "foo(1)\n").
 
 block(Config) when is_list(Config) ->
     ?assertFormat(
@@ -2566,7 +2577,8 @@ fun_expression(Config) when is_list(Config) ->
         "->\n"
         "    X\n"
         "end\n"
-    ).
+    ),
+    ?assertFormat("fun() -> ok;end.\n", "fun() -> ok end.\n").
 
 case_expression(Config) when is_list(Config) ->
     ?assertFormat(
@@ -2767,6 +2779,14 @@ case_expression(Config) when is_list(Config) ->
         "    ?macro(1);\n"
         "    ?macro(2)\n"
         "end\n"
+    ),
+    ?assertFormat(
+        "case 1 of\n"
+        "    1 -> ok;\n"
+        "end\n",
+        "case 1 of\n"
+        "    1 -> ok\n"
+        "end\n"
     ).
 
 maybe_expression(Config) when is_list(Config) ->
@@ -2811,6 +2831,26 @@ maybe_expression(Config) when is_list(Config) ->
         "    1 -> ok\n"
         "    % comment else after\n"
         "end\n"
+    ),
+    ?assertFormat(
+        "maybe\n"
+        "    1 ?= 1,\n"
+        "end\n",
+        "maybe\n"
+        "    1 ?= 1\n"
+        "end\n"
+    ),
+    ?assertFormat(
+        "maybe\n"
+        "    ok\n"
+        "else\n"
+        "    4 -> ok;\n"
+        "end\n",
+        "maybe\n"
+        "    ok\n"
+        "else\n"
+        "    4 -> ok\n"
+        "end\n"
     ).
 
 receive_expression(Config) when is_list(Config) ->
@@ -3049,6 +3089,14 @@ receive_expression(Config) when is_list(Config) ->
         "        )\n"
         "        %% after after for receive\n"
         "end\n"
+    ),
+    ?assertFormat(
+        "receive\n"
+        "    1 -> ok;\n"
+        "end\n",
+        "receive\n"
+        "    1 -> ok\n"
+        "end\n"
     ).
 
 try_expression(Config) when is_list(Config) ->
@@ -3182,6 +3230,54 @@ try_expression(Config) when is_list(Config) ->
         "    ok\n"
         "    %% after after\n"
         "end\n"
+    ),
+    ?assertFormat(
+        "try\n"
+        "    ok,\n"
+        "after\n"
+        "    Expr\n"
+        "end\n",
+        "try\n"
+        "    ok\n"
+        "after\n"
+        "    Expr\n"
+        "end\n"
+    ),
+    ?assertFormat(
+        "try\n"
+        "    ok\n"
+        "after\n"
+        "    Expr,\n"
+        "end\n",
+        "try\n"
+        "    ok\n"
+        "after\n"
+        "    Expr\n"
+        "end\n"
+    ),
+    ?assertFormat(
+        "try Expr of\n"
+        "    _ -> ok;\n"
+        "after\n"
+        "    Expr\n"
+        "end\n",
+        "try Expr of\n"
+        "    _ -> ok\n"
+        "after\n"
+        "    Expr\n"
+        "end\n"
+    ),
+    ?assertFormat(
+        "try\n"
+        "    ok\n"
+        "catch\n"
+        "    _ -> throw;\n"
+        "end\n",
+        "try\n"
+        "    ok\n"
+        "catch\n"
+        "    _ -> throw\n"
+        "end\n"
     ).
 
 if_expression(Config) when is_list(Config) ->
@@ -3225,6 +3321,30 @@ if_expression(Config) when is_list(Config) ->
         "        Expression\n"
         "end\n",
         25
+    ),
+    ?assertFormat(
+        "if\n"
+        "    true -> ok;\n"
+        "end\n",
+        "if\n"
+        "    true -> ok\n"
+        "end\n"
+    ),
+    ?assertFormat(
+        "if\n"
+        "    true, -> ok\n"
+        "end\n",
+        "if\n"
+        "    true -> ok\n"
+        "end\n"
+    ),
+    ?assertFormat(
+        "if\n"
+        "    true; -> ok\n"
+        "end\n",
+        "if\n"
+        "    true -> ok\n"
+        "end\n"
     ).
 
 macro(Config) when is_list(Config) ->
@@ -3250,7 +3370,8 @@ macro(Config) when is_list(Config) ->
         ")\n",
         23
     ),
-    ?assertSame("?macro(Expr when Guard1; Guard2)\n").
+    ?assertSame("?macro(Expr when Guard1; Guard2)\n"),
+    ?assertFormat("?foo(X,)\n", "?foo(X)\n").
 
 function(Config) when is_list(Config) ->
     ?assertSame("f() -> ok.\n"),
@@ -3294,7 +3415,10 @@ function(Config) when is_list(Config) ->
         "bar(X) -> ok. % comment\n",
         "% comment\n"
         "bar(X) -> ok.\n"
-    ).
+    ),
+    ?assertFormat("foo(1,) -> ok.\n", "foo(1) -> ok.\n"),
+    ?assertFormat("foo() -> ok,.\n", "foo() -> ok.\n"),
+    ?assertFormat("foo() -> ok;.\n", "foo() -> ok.\n").
 
 attribute(Config) when is_list(Config) ->
     ?assertFormat("-else .", "-else.\n"),
@@ -3353,7 +3477,8 @@ attribute(Config) when is_list(Config) ->
         "-type str() :: string().\n"
         "\n"
         "-type int() :: integer().\n"
-    ).
+    ),
+    ?assertFormat("-attr(1,2,).\n", "-attr(1, 2).\n").
 
 exportimport(Config) when is_list(Config) ->
     ?assertSame("-export([bar/2, baz/3]).\n"),
@@ -3920,7 +4045,9 @@ spec(Config) when is_list(Config) ->
         "    Long,\n"
         "    Arguments\n"
         ") -> ok.\n"
-    ).
+    ),
+    ?assertFormat("-spec foo(1,) -> ok.\n", "-spec foo(1) -> ok.\n"),
+    ?assertFormat("-spec foo() -> ok;.\n", "-spec foo() -> ok.\n").
 
 define(Config) when is_list(Config) ->
     ?assertSame(
@@ -3965,7 +4092,8 @@ define(Config) when is_list(Config) ->
         ").\n"
     ),
     ?assertSame("-define(ANY_MODE(Mode), Mode when Mode =:= inline; Mode =:= async).\n"),
-    ?assertSame("-define(CATCH, C:E:S).\n").
+    ?assertSame("-define(CATCH, C:E:S).\n"),
+    ?assertFormat("-define(FOO(A,), A).\n", "-define(FOO(A), A).\n").
 
 type(Config) when is_list(Config) ->
     ?assertSame(
@@ -4125,7 +4253,8 @@ type(Config) when is_list(Config) ->
     ),
     ?assertSame(
         "-type foo() :: ?FOO:?BAR().\n"
-    ).
+    ),
+    ?assertFormat("-type foo() :: bar(1,).\n", "-type foo() :: bar(1).\n").
 
 exprs(Config) when is_list(Config) ->
     ?assertSame(