Fix non-idempotent formatting of multi-line binary strings

When a binary contains a string with newlines, string_to_algebra splits
it into multiple lines. On re-parse, the AST represents this as a concat
node instead of a string node. break_behaviour and is_tag only matched
the string form, causing different layout decisions on the second pass.

Fix by checking is_multiline_string in both break_behaviour and is_tag
so that a multi-line string in a bin_element is treated the same way as
the concat that it will become after formatting.

Author: michalmuskala

src/erlfmt_format.erl

@@ -528,8 +528,11 @@ break_behaviour(Meta, Values, Break) ->
     case Break =:= flex_break andalso not ForceOuter of
         true ->
             case Values of
-                [{bin_element, _, {string, _, _}, _, _}] ->
-                    no_break;
+                [{bin_element, _, {string, SMeta, _}, _, _}] ->
+                    case is_multiline_string(SMeta) of
+                        true -> #break{outer = false, inner = false};
+                        false -> no_break
+                    end;
                 [SoleElement] ->
                     case is_container(SoleElement) of
                         true -> no_break;
@@ -550,8 +553,8 @@ break_behaviour(Meta, Values, Break) ->
     end.
 
 %% Allow inlining binary literals
-is_tag({bin, _, [{bin_element, _, {string, _, _}, _, _}]}) ->
-    true;
+is_tag({bin, _, [{bin_element, _, {string, SMeta, _}, _, _}]}) ->
+    not is_multiline_string(SMeta);
 is_tag({bin, _, []}) ->
     true;
 %% Allow inlining argless macros
@@ -565,6 +568,14 @@ is_tag(Expr) ->
 is_container(Expr) ->
     lists:member(element(1, Expr), [tuple, bin, list, map, record, call, macro_call, lc, bc]).
 
+is_multiline_string(Meta) ->
+    Text = erlfmt_scan:get_anno(text, Meta),
+    case string:split(Text, "\n", all) of
+        [_] -> false;
+        [_, "\""] -> false;
+        _ -> true
+    end.
+
 surround_container(no_break, Left, Doc, Right) ->
     concat(Left, Doc, Right);
 surround_container(flex_break, Left, Doc, Right) ->

test/erlfmt_SUITE.erl

@@ -53,6 +53,7 @@
     map_comprehension/1,
     snapshot_simple_comments/1,
     snapshot_big_binary/1,
+    snapshot_big_binary_string/1,
     snapshot_attributes/1,
     snapshot_escript/1,
     snapshot_emulator_args/1,
@@ -163,6 +164,7 @@ groups() ->
         {snapshot_tests, [parallel], [
             snapshot_simple_comments,
             snapshot_big_binary,
+            snapshot_big_binary_string,
             snapshot_attributes,
             snapshot_escript,
             snapshot_emulator_args,
@@ -1068,6 +1070,7 @@ parse_forms(String) ->
 snapshot_simple_comments(Config) -> snapshot_same("simple_comments.erl", Config).
 
 snapshot_big_binary(Config) -> snapshot_same("big_binary.erl", Config).
+snapshot_big_binary_string(Config) -> snapshot_same("big_binary_string.erl", Config).
 
 snapshot_attributes(Config) -> snapshot_same("attributes.erl", Config).
 

test/erlfmt_SUITE_data/big_binary_string.erl

@@ -0,0 +1,9 @@
+-module(big_binary_string).
+
+-export([big_binary_string/0]).
+
+big_binary_string() ->
+    <<
+        "{\"key\": \"value\", \"nested\": {\"items\": [\"item1\", \"item2\", \"item3\"]}\n"
+        "\"description\": \"A somewhat long description that goes on and on and is very verbose\"\n"
+    >>.