Introduce next break fits rules

Summary: This introduces rules to not break containers if only the last argument is multiline. This applies for all map, list, tuple, record, and block (`begin`) expressions. As containers we generally consider everything that "wraps around" expressions, besides mentioned elements, it's also, for example, function arguments.
iamirosein · Apr 17, 2020 · 1bb385c · 1bb385c
1 parent 5bd8468
commit 1bb385c
Show file tree

Hide file tree

Showing 3 changed files with 101 additions and 38 deletions.
diff --git a/src/erlfmt_format.erl b/src/erlfmt_format.erl
@@ -24,6 +24,7 @@
     document_flush/1,
     document_choice/2,
     document_single_line/1,
+    document_prepend/2,
     document_reduce/2,
     document_fail/0
 ]).
@@ -32,6 +33,8 @@
 
 -define(PARENLESS_ATTRIBUTE, [type, opaque, spec, callback]).
 
+-define(NEXT_BREAK_FITS, [map, list, tuple, record, block]).
+
 -spec form_to_algebra(erlfmt_parse:abstract_form()) -> erlfmt_algebra:document().
 form_to_algebra({function, Meta, Clauses}) ->
     Doc = document_combine(clauses_to_algebra(Clauses), document_text(".")),
@@ -188,9 +191,15 @@ combine_all(Docs) ->
 combine_nested(Head, Doc) ->
     combine_newline(Head, document_combine(document_spaces(?INDENT), Doc)).
 
+prepend_comma_space(D1, D2) ->
+    wrap_prepend(D1, document_text(", "), D2).
+
 wrap(Left, Doc, Right) ->
     document_combine(Left, document_combine(Doc, Right)).
 
+wrap_prepend(Left, Doc, Right) ->
+    document_prepend(Left, document_prepend(Doc, Right)).
+
 wrap_in_parens(Doc) -> wrap(document_text("("), Doc, document_text(")")).
 
 wrap_nested(Left, Doc, Right) ->
@@ -248,6 +257,8 @@ binary_operand_to_algebra(Op, {op, Meta, Op, Left, Right}, Indent) ->
 binary_operand_to_algebra(_ParentOp, Expr, _Indent) ->
     expr_to_algebra(Expr).
 
+container_to_algebra([], Left, Right) ->
+    document_combine(Left, Right);
 container_to_algebra(Values, Left, Right) ->
     {Single, Multi} = container_to_algebra_pair(Values, Left, Right),
     document_choice(Single, Multi).
@@ -256,25 +267,35 @@ container_to_algebra_pair([], Left, Right) ->
     Doc = document_combine(Left, Right),
     {Doc, Doc};
 container_to_algebra_pair(Values, Left, Right) ->
-    {Horizontal, Vertical} = container_values_to_algebra_pair(Values),
-    {wrap(Left, Horizontal, Right), wrap_nested(Left, Vertical, Right)}.
+    {Horizontal, LastFits, Vertical} = container_values_to_algebra_pair(Values),
+    HorizontalD = wrap(Left, Horizontal, Right),
+    VerticalD = document_choice(
+        wrap_prepend(Left, LastFits, Right),
+        wrap_nested(Left, Vertical, Right)
+    ),
+    {HorizontalD, VerticalD}.
 
 %% standalone comments are always trailing, but can appear as only expressions
 container_values_to_algebra_pair([{comment, _, _} | _] = Comments) ->
-    {document_fail(), comments_to_algebra(Comments)};
+    {document_fail(), document_fail(), comments_to_algebra(Comments)};
 container_values_to_algebra_pair([Expr | [{comment, _, _} | _] = Comments]) ->
     CommentsD = comments_to_algebra(Comments),
     ExprD = expr_to_algebra(Expr),
-    {document_fail(), combine_newline(ExprD, CommentsD)};
+    {document_fail(), document_fail(), combine_newline(ExprD, CommentsD)};
 container_values_to_algebra_pair([Expr]) ->
     ExprD = expr_to_algebra(Expr),
-    {document_single_line(ExprD), ExprD};
+    case lists:member(element(1, Expr), ?NEXT_BREAK_FITS) andalso no_comments_or_parens(Expr) of
+        true -> {document_single_line(ExprD), ExprD, ExprD};
+        false -> {document_single_line(ExprD), document_fail(), ExprD}
+    end;
 container_values_to_algebra_pair([Expr | Rest]) ->
     ExprD = expr_to_algebra(Expr),
-    {RestSingle, RestMulti} = container_values_to_algebra_pair(Rest),
-    Single = combine_comma_space(document_single_line(ExprD), RestSingle),
+    SingleExprD = document_single_line(ExprD),
+    {RestSingle, RestLastFits, RestMulti} = container_values_to_algebra_pair(Rest),
+    Single = combine_comma_space(SingleExprD, RestSingle),
+    LastFits = prepend_comma_space(SingleExprD, RestLastFits),
     Multi = combine_comma_newline(ExprD, RestMulti),
-    {Single, Multi}.
+    {Single, LastFits, Multi}.
 
 cons_to_algebra(Head, Tail) ->
     HeadD = expr_to_algebra(Head),
@@ -324,7 +345,7 @@ field_to_algebra(Op, Key, Value) ->
 
 comprehension_to_algebra(ExprD, LcExprs, Left, Right) ->
     PipesD = document_text("|| "),
-    {LcExprsSingleD, LcExprsMultiD} = container_values_to_algebra_pair(LcExprs),
+    {LcExprsSingleD, _LcExprsLastFitsD, LcExprsMultiD} = container_values_to_algebra_pair(LcExprs),
     LcExprsD = document_choice(LcExprsSingleD, LcExprsMultiD),
 
     SingleLine =
@@ -558,6 +579,10 @@ try_of_block(Exprs, OfClauses) ->
             )
     end.
 
+no_comments_or_parens(Meta) ->
+    {Pre, Post} = comments(Meta),
+    Pre =:= [] andalso Post =:= [] andalso not erlfmt_scan:get_anno(parens, Meta, false).
+
 maybe_wrap_in_parens(Meta, Doc) ->
     Parens = erlfmt_scan:get_anno(parens, Meta, false),
     if

diff --git a/test/erlfmt_SUITE_data/simple_comments.erl b/test/erlfmt_SUITE_data/simple_comments.erl
@@ -4,13 +4,10 @@
 
 -export_type([foo/0]).
 
--record(
-    foo,
-    {
-        %% comment
-        a
-    }
-).
+-record(foo, {
+    %% comment
+    a
+}).
 
 -define(CONST, const).
 

diff --git a/test/erlfmt_format_SUITE.erl b/test/erlfmt_format_SUITE.erl
@@ -337,23 +337,34 @@ tuple(Config) when is_list(Config) ->
     ?assertFormatExpr("{1,2}", "{1, 2}"),
     ?assertFormatExpr("{1,{2,3}}", "{1, {2, 3}}"),
     ?assertFormatExpr(
-        "{1,{2,3}}",
+        "{long,{2,3,4}}",
         "{\n"
-        "    1,\n"
-        "    {2, 3}\n"
+        "    long,\n"
+        "    {2, 3, 4}\n"
         "}",
-        10
+        15
     ),
     ?assertFormatExpr(
-        "{1,{long,word}}",
+        "{{long, word},{long,word}}",
         "{\n"
-        "    1,\n"
+        "    {\n"
+        "        long,\n"
+        "        word\n"
+        "    },\n"
         "    {\n"
         "        long,\n"
         "        word\n"
         "    }\n"
         "}",
         15
+    ),
+    ?assertFormatExpr(
+        "{short, {long, word}}",
+        "{short, {\n"
+        "    long,\n"
+        "    word\n"
+        "}}",
+        15
     ).
 
 list(Config) when is_list(Config) ->
@@ -362,12 +373,12 @@ list(Config) when is_list(Config) ->
     ?assertFormatExpr("[ 1 ,2,3, 4]", "[1, 2, 3, 4]"),
     ?assertFormatExpr("[1,2,3|4]", "[1, 2, 3 | 4]"),
     ?assertFormatExpr(
-        "[1,[2,3]]",
+        "[long,[2,3,4]]",
         "[\n"
-        "    1,\n"
-        "    [2, 3]\n"
+        "    long,\n"
+        "    [2, 3, 4]\n"
         "]",
-        10
+        15
     ),
     ?assertFormatExpr(
         "[11,2|3]",
@@ -385,6 +396,14 @@ list(Config) when is_list(Config) ->
         "    | 33\n"
         "]",
         10
+    ),
+    ?assertFormatExpr(
+        "[short, [long, word]]",
+        "[short, [\n"
+        "    long,\n"
+        "    word\n"
+        "]]",
+        15
     ).
 
 binary(Config) when is_list(Config) ->
@@ -614,15 +633,15 @@ call(Config) when is_list(Config) ->
         25
     ),
     ?assertFormatExpr(
-        "long_name({Very, Long, Expression})",
-        "long_name(\n"
+        "very_very_long_name({Very, Long, Expression})",
+        "very_very_long_name(\n"
         "    {\n"
         "        Very,\n"
         "        Long,\n"
         "        Expression\n"
         "    }\n"
         ")",
-        25
+        20
     ),
     ?assertFormatExpr(
         "long_name({Long, Expression}, AnotherArgument)",
@@ -631,6 +650,15 @@ call(Config) when is_list(Config) ->
         "    AnotherArgument\n"
         ")",
         25
+    ),
+    ?assertFormatExpr(
+        "long_name(Arg, {Very, Long, Expression})",
+        "long_name(Arg, {\n"
+        "    Very,\n"
+        "    Long,\n"
+        "    Expression\n"
+        "})",
+        25
     ).
 
 block(Config) when is_list(Config) ->
@@ -1071,7 +1099,18 @@ attribute(Config) when is_list(Config) ->
     ),
     ?assertFormatForm(
         "-attribute([ExceptionallyLong, Value]).",
+        "-attribute([\n"
+        "    ExceptionallyLong,\n"
+        "    Value\n"
+        "]).",
+        25
+    ),
+    ?assertFormatForm(
+        "-attribute(\n"
+        "           [ ExceptionallyLong % comment\n"
+        "           , Value]).",
         "-attribute(\n"
+        "    % comment\n"
         "    [\n"
         "        ExceptionallyLong,\n"
         "        Value\n"
@@ -1089,15 +1128,12 @@ attribute(Config) when is_list(Config) ->
     ),
     ?assertFormatForm(
         "-record(foo, {a = 1 :: integer(), b :: float(), c  = 2, d}).",
-        "-record(\n"
-        "    foo,\n"
-        "    {\n"
-        "        a = 1 :: integer(),\n"
-        "        b :: float(),\n"
-        "        c = 2,\n"
-        "        d\n"
-        "    }\n"
-        ").",
+        "-record(foo, {\n"
+        "    a = 1 :: integer(),\n"
+        "    b :: float(),\n"
+        "    c = 2,\n"
+        "    d\n"
+        "}).",
         30
     ),
     ?assertSameForm(
@@ -1164,6 +1200,11 @@ attribute(Config) when is_list(Config) ->
         ").",
         40
     ),
+    ?assertSameForm(
+        "-define(FOO(X), begin\n"
+        "    is_atom(X) orelse is_tuple(X)\n"
+        "end)."
+    ),
     ?assertSameForm(
         "-type foo() :: {fun(), fun((...) -> mod:bar()), fun(() -> integer())}."
     ).