Preserve line breaks in container literals

Summary: If you write this in Javascript: ``` const foo = { bar: baz; qux: grault; }; ``` then Prettier will respect that you put the key-values on different lines, even though they could all fit on the same line. (This applies to even the case that there is only one key-value pair.) This is desired behavior. Oftentimes we want to put members of a container literal on lines of their own for readability or emphasis. This commit changes it so that we look to see if the members of the container literal appear to be on different lines, and if so, preserves that formatting and keeps them on different lines. Differential Revision: D5503181 fbshipit-source-id: f7d81e2ce909430161035524b0f442d6806b9fe2
facebook · Jul 27, 2017 · b4afb7e · b4afb7e
1 parent fc313dd
commit b4afb7e
Show file tree

Hide file tree

Showing 4 changed files with 130 additions and 38 deletions.
diff --git a/hphp/hack/src/hackfmt/hack_format.ml b/hphp/hack/src/hackfmt/hack_format.ml
@@ -1014,11 +1014,7 @@ let rec transform node =
     let (name, left_b, initializers, right_b) =
       get_collection_literal_expression_children x
     in
-    Fmt [
-      t name;
-      Space;
-      transform_argish ~spaces:true left_b initializers right_b;
-    ]
+    transform_container_literal ~spaces:true name left_b initializers right_b
   | ObjectCreationExpression x ->
     let (kw, obj_type, left_p, arg_list, right_p) =
       get_object_creation_expression_children x
@@ -1036,48 +1032,30 @@ let rec transform node =
     let (kw, left_p, members, right_p) =
       get_array_intrinsic_expression_children x
     in
-    Fmt [
-      t kw;
-      transform_argish left_p members right_p;
-    ]
+    transform_container_literal kw left_p members right_p
   | DarrayIntrinsicExpression x ->
     let (kw, left_p, members, right_p) =
       get_darray_intrinsic_expression_children x in
-    Fmt [
-      t kw;
-      transform_argish left_p members right_p;
-    ]
+    transform_container_literal kw left_p members right_p
   | DictionaryIntrinsicExpression x ->
     let (kw, left_p, members, right_p) =
       get_dictionary_intrinsic_expression_children x
     in
-    Fmt [
-      t kw;
-      transform_argish left_p members right_p;
-    ]
+    transform_container_literal kw left_p members right_p
   | KeysetIntrinsicExpression x ->
     let (kw, left_p, members, right_p) =
       get_keyset_intrinsic_expression_children x
     in
-    Fmt [
-      t kw;
-      transform_argish left_p members right_p;
-    ]
+    transform_container_literal kw left_p members right_p
   | VarrayIntrinsicExpression x ->
     let (kw, left_p, members, right_p) =
       get_varray_intrinsic_expression_children x in
-    Fmt [
-      t kw;
-      transform_argish left_p members right_p;
-    ]
+    transform_container_literal kw left_p members right_p
   | VectorIntrinsicExpression x ->
     let (kw, left_p, members, right_p) =
       get_vector_intrinsic_expression_children x
     in
-    Fmt [
-      t kw;
-      transform_argish left_p members right_p;
-    ]
+    transform_container_literal kw left_p members right_p
   | ElementInitializer x ->
     let (key, arrow, value) = get_element_initializer_children x in
     transform_mapish_entry key arrow value
@@ -1521,8 +1499,13 @@ and nest ?(spaces=false) right_delim nodes =
     transform right_delim;
   ]
 
-and after_each_argument is_last =
-  if is_last then Split else space_split ()
+and after_each_argument ?(force_newlines=false) is_last =
+  if force_newlines
+  then Newline
+  else
+    if is_last
+    then Split
+    else space_split ()
 
 and handle_lambda_body node =
   match syntax node with
@@ -1830,7 +1813,8 @@ and transform_argish_with_return_type left_p params right_p colon ret_type =
     ])
   ]
 
-and transform_argish ?(allow_trailing=true) ?(spaces=false)
+and transform_argish
+    ?(allow_trailing=true) ?(force_newlines=false) ?(spaces=false)
     left_p arg_list right_p =
   (* When there is only one argument, with no surrounding whitespace in the
    * original source, allow that style to be preserved even when there are line
@@ -1871,7 +1855,7 @@ and transform_argish ?(allow_trailing=true) ?(spaces=false)
     | _ -> true
   in
   delimited_nest ~spaces ~split_when_children_split left_p right_p [
-    transform_arg_list ~spaces ~allow_trailing arg_list
+    transform_arg_list ~spaces ~allow_trailing ~force_newlines arg_list
   ]
 
 and transform_braced_item left_p item right_p =
@@ -1908,9 +1892,10 @@ and transform_braced_item_with_trailer left_p item comma right_p =
        trailing commas in all these places reach end-of-life. *)
     [transform_trailing_comma ~allow_trailing:false item comma]
 
-and transform_arg_list ?(allow_trailing=true) ?(spaces=false) items =
+and transform_arg_list
+    ?(allow_trailing=true) ?(force_newlines=false) ?(spaces=false) items =
   handle_possible_list items
-    ~after_each:after_each_argument
+    ~after_each:(after_each_argument ~force_newlines)
     ~handle_last:(transform_last_arg ~allow_trailing)
 
 and transform_possible_comma_list ?(allow_trailing=true) ?(spaces=false)
@@ -1919,6 +1904,18 @@ and transform_possible_comma_list ?(allow_trailing=true) ?(spaces=false)
     transform_arg_list ~spaces ~allow_trailing items
   ]
 
+and transform_container_literal ?(spaces=false) kw left_p members right_p =
+  let force_newlines =
+    let trivia = trailing_trivia left_p in
+    List.exists trivia
+      ~f:(fun x -> Full_fidelity_editable_trivia.kind x = TriviaKind.EndOfLine)
+  in
+  Fmt [
+    transform kw;
+    if spaces then Space else Nothing;
+    transform_argish ~spaces ~force_newlines left_p members right_p;
+  ]
+
 and remove_leading_trivia node =
   match Syntax.leading_token node with
   | None -> [], node

diff --git a/hphp/hack/test/hackfmt/tests/collection_literal_newlines.php b/hphp/hack/test/hackfmt/tests/collection_literal_newlines.php
@@ -0,0 +1,48 @@
+<?hh // strict
+$foo = array('should_stay_on_same_line');
+$foo = array(
+  'should_remain_on_new_line',
+);
+$foo = varray['should_stay_on_same_line'];
+$foo = varray[
+  'should_remain_on_new_line',
+];
+
+$foo = array('should_stay' => 'on_same_line');
+$foo = array(
+  'should_remain' => 'on_new_line',
+);
+$foo = darray['should_stay' => 'on_same_line'];
+$foo = darray[
+  'should_remain' => 'on_new_line',
+];
+
+$foo = Set { 'should_stay_on_same_line' };
+$foo = Set {
+  'should_remain_on_new_line',
+};
+$foo = Vector { 'should_stay_on_same_line' };
+$foo = Vector {
+  'should_remain_on_new_line',
+};
+$foo = Map { 'should_stay' => 'on_same_line' };
+$foo = Map {
+  'should_remain' => 'on_new_line',
+};
+
+$foo = keyset['should_stay_on_same_line'];
+$foo = keyset[
+  'should_remain_on_new_line',
+];
+$foo = vec['should_stay_on_same_line'];
+$foo = vec[
+  'should_remain_on_new_line',
+];
+$foo = dict['should_stay' => 'on_same_line'];
+$foo = dict[
+  'should_remain' => 'on_new_line',
+];
+
+// If there are no elements, there should not be a newline.
+$foo = dict[
+];
diff --git a/hphp/hack/test/hackfmt/tests/collection_literal_newlines.php.exp b/hphp/hack/test/hackfmt/tests/collection_literal_newlines.php.exp
@@ -0,0 +1,47 @@
+<?hh // strict
+$foo = array('should_stay_on_same_line');
+$foo = array(
+  'should_remain_on_new_line',
+);
+$foo = varray['should_stay_on_same_line'];
+$foo = varray[
+  'should_remain_on_new_line',
+];
+
+$foo = array('should_stay' => 'on_same_line');
+$foo = array(
+  'should_remain' => 'on_new_line',
+);
+$foo = darray['should_stay' => 'on_same_line'];
+$foo = darray[
+  'should_remain' => 'on_new_line',
+];
+
+$foo = Set { 'should_stay_on_same_line' };
+$foo = Set {
+  'should_remain_on_new_line',
+};
+$foo = Vector { 'should_stay_on_same_line' };
+$foo = Vector {
+  'should_remain_on_new_line',
+};
+$foo = Map { 'should_stay' => 'on_same_line' };
+$foo = Map {
+  'should_remain' => 'on_new_line',
+};
+
+$foo = keyset['should_stay_on_same_line'];
+$foo = keyset[
+  'should_remain_on_new_line',
+];
+$foo = vec['should_stay_on_same_line'];
+$foo = vec[
+  'should_remain_on_new_line',
+];
+$foo = dict['should_stay' => 'on_same_line'];
+$foo = dict[
+  'should_remain' => 'on_new_line',
+];
+
+// If there are no elements, there should not be a newline.
+$foo = dict[];
diff --git a/hphp/hack/test/hackfmt/tests/xhp_1.php.exp b/hphp/hack/test/hackfmt/tests/xhp_1.php.exp
@@ -15,9 +15,9 @@ final class :MyFancyXHPClass extends :OtherXHPThing {
       <x:column-layout-component
         attr1="attr"
         attr2={
-          FakeClassWithReallyLongAttributeStuff::getCreateSomeStaticData(
-            Map { 'key1' => 'value1' },
-          )
+          FakeClassWithReallyLongAttributeStuff::getCreateSomeStaticData(Map {
+            'key1' => 'value1',
+          })
         }>
         <x:div padding="large"><x:link href="#">Click Here!</x:link></x:div>
         <x:div padding="large">{$link}</x:div>