From 715479689fe1c46a98978885364e5dea5ffe598d Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Tue, 9 Oct 2018 20:23:08 +0100 Subject: [PATCH 1/4] Preserve function body braces This is an alternative to #1826 implementing just the brace preservation code, to make it easier to be reviewed. I think this is now in a better place to get into the codebase given that we have `Reason_attributes` outside of `Reason_pprint_ast` and that we now remove our own attributes before printing to OCaml. I think we can probably refactor this code further in future PRs, as well as add other stylistic preservation changes. --- .../expected_output/attributes.re | 20 +++- .../expected_output/fastPipe.re | 55 ++++----- .../typeCheckedTests/expected_output/jsx.re | 22 ++-- .../typeCheckedTests/expected_output/oo.re | 48 +++++--- .../expected_output/reasonComments.re | 15 ++- .../expected_output/sequences.re | 16 ++- .../expected_output/trailing.re | 6 +- .../expected_output/basicStructures.re | 25 ++-- .../unit_tests/expected_output/extensions.re | 69 +++++++---- .../unit_tests/expected_output/fastPipe.re | 8 +- .../unit_tests/expected_output/infix.re | 5 +- .../unit_tests/expected_output/jsx_functor.re | 9 +- .../unit_tests/expected_output/modules.re | 39 +++++-- .../expected_output/modules_no_semi.re | 39 +++++-- .../expected_output/polymorphism.re | 3 +- .../unit_tests/expected_output/syntax.re | 11 +- src/reason-parser/reason_attributes.ml | 9 ++ src/reason-parser/reason_heuristics.ml | 7 ++ src/reason-parser/reason_parser.mly | 8 +- src/reason-parser/reason_pprint_ast.ml | 110 +++++++++++++----- 20 files changed, 358 insertions(+), 166 deletions(-) diff --git a/formatTest/typeCheckedTests/expected_output/attributes.re b/formatTest/typeCheckedTests/expected_output/attributes.re index 42de2836a..3e82b9b93 100644 --- a/formatTest/typeCheckedTests/expected_output/attributes.re +++ b/formatTest/typeCheckedTests/expected_output/attributes.re @@ -99,7 +99,11 @@ let x = [@attrEverything] (true && false); /** * How attribute parsings respond to other syntactic constructs. */ -let add = a => [@onRet] a; +let add = a => + [@onRet] + { + a; + }; let add = a => [@onRet] a; let add = [@onEntireFunction] (a => a); @@ -342,8 +346,18 @@ type classAttributesOnKeys = { . [@bs.set] key1: string, /* The follow two are the same */ - [@bs.get null] key2: [@onType2] Js.t(int), - [@bs.get null] key3: [@onType2] Js.t(int), + [@bs.get + { + null; + } + ] + key2: [@onType2] Js.t(int), + [@bs.get + { + null; + } + ] + key3: [@onType2] Js.t(int), key4: Js.t([@justOnInt] int), }; diff --git a/formatTest/typeCheckedTests/expected_output/fastPipe.re b/formatTest/typeCheckedTests/expected_output/fastPipe.re index ed8cbc2ab..36433e308 100644 --- a/formatTest/typeCheckedTests/expected_output/fastPipe.re +++ b/formatTest/typeCheckedTests/expected_output/fastPipe.re @@ -26,8 +26,9 @@ let c = true; let t3: bool = !a->b->c; /* parse fast pipe with underscore application correct */ -let doStuff = (a: int, b: int, c: int): int => +let doStuff = (a: int, b: int, c: int): int => { a + 2 * b + 3 * c; +}; let (|.) = (a, f) => f(a); @@ -69,18 +70,16 @@ let saveStatus = Pristine; let t7: string = - { - ( - switch (saveStatus) { - | Pristine => [0] - | Saved => [1] - | Saving => [2] - | Unsaved => [3] - } - ) - ->Foo.map(Foo.plusOne) - ->Foo.toString - } + {( + switch (saveStatus) { + | Pristine => [0] + | Saved => [1] + | Saving => [2] + | Unsaved => [3] + } + ) + ->Foo.map(Foo.plusOne) + ->Foo.toString} ; let genItems = f => List.map(f, items); @@ -99,23 +98,19 @@ let foo = xs => List.concat([xs, xs]); let t10: string = - { - blocks - ->foo - ->Foo.map(Foo.plusOne) - ->Foo.toString - } + {blocks + ->foo + ->Foo.map(Foo.plusOne) + ->Foo.toString} ; let t11: string = - { - blocks - ->foo - ->Foo.map(Foo.plusOne) - ->Foo.map(Foo.plusOne) - ->Foo.toString - } + {blocks + ->foo + ->Foo.map(Foo.plusOne) + ->Foo.map(Foo.plusOne) + ->Foo.toString} ; let title = "los pilares de la tierra"; @@ -164,11 +159,9 @@ module FooLabeled = { let t14: string = - { - items - ->FooLabeled.map(~f=FooLabeled.plusOne) - ->FooLabeled.toString - } + {items + ->FooLabeled.map(~f=FooLabeled.plusOne) + ->FooLabeled.toString} ; let c = (a, b) => a + b; diff --git a/formatTest/typeCheckedTests/expected_output/jsx.re b/formatTest/typeCheckedTests/expected_output/jsx.re index e34e49bd9..247be1464 100644 --- a/formatTest/typeCheckedTests/expected_output/jsx.re +++ b/formatTest/typeCheckedTests/expected_output/jsx.re @@ -102,29 +102,32 @@ module Namespace = { }; module Optional1 = { - let createElement = (~required, ~children, ()) => + let createElement = (~required, ~children, ()) => { switch (required) { | Some(a) => {displayName: a} | None => {displayName: "nope"} }; + }; }; module Optional2 = { let createElement = - (~optional=?, ~children, ()) => + (~optional=?, ~children, ()) => { switch (optional) { | Some(a) => {displayName: a} | None => {displayName: "nope"} }; + }; }; module DefaultArg = { let createElement = - (~default=Some("foo"), ~children, ()) => + (~default=Some("foo"), ~children, ()) => { switch (default) { | Some(a) => {displayName: a} | None => {displayName: "nope"} }; + }; }; module LotsOfArguments = { @@ -175,8 +178,9 @@ let notReallyJSX = (~foo, ~bar, children) => { displayName: "test", }; -let fakeRender = (el: component) => +let fakeRender = (el: component) => { el.displayName; +}; /* end of setup */ @@ -384,7 +388,7 @@ let asd2 = [@foo] [@JSX] video(~test=false, 10); let div = (~children) => 1; [@JSX] ((() => div)())(~children=[]); -let myFun = () => +let myFun = () => { <> ; +}; -let myFun = () => <> ; +let myFun = () => { + <> ; +}; -let myFun = () => +let myFun = () => { <> ; +}; /** * Children should wrap without forcing attributes to. diff --git a/formatTest/typeCheckedTests/expected_output/oo.re b/formatTest/typeCheckedTests/expected_output/oo.re index cd0d1ff27..22b2b6d4a 100644 --- a/formatTest/typeCheckedTests/expected_output/oo.re +++ b/formatTest/typeCheckedTests/expected_output/oo.re @@ -14,12 +14,18 @@ class virtual stack ('a) (init) = { Some(hd); | [] => None }; - pub push = hd => v = [hd, ...v]; - initializer ( - print_string("initializing object") - ); - pub explicitOverrideTest = a => a + 1; - pri explicitOverrideTest2 = a => a + 1; + pub push = hd => { + v = [hd, ...v]; + }; + initializer { + print_string("initializing object"); + }; + pub explicitOverrideTest = a => { + a + 1; + }; + pri explicitOverrideTest2 = a => { + a + 1; + }; }; let tmp = { @@ -50,10 +56,12 @@ class virtual stackWithAttributes ('a) (init) = { Some(hd); | [] => None }; - pub push = hd => v = [hd, ...v]; - initializer ( - print_string("initializing object") - ); + pub push = hd => { + v = [hd, ...v]; + }; + initializer { + print_string("initializing object"); + }; }; class extendedStack ('a) (init) = { @@ -67,9 +75,15 @@ class extendedStackAcknowledgeOverride (init) = { inherit (class stack('a))(init); val dummy = (); - pub implementMe = i => i + 1; - pub! explicitOverrideTest = a => a + 2; - pri! explicitOverrideTest2 = a => a + 2; + pub implementMe = i => { + i + 1; + }; + pub! explicitOverrideTest = a => { + a + 2; + }; + pri! explicitOverrideTest2 = a => { + a + 2; + }; }; let inst = (new extendedStack)([1, 2]); @@ -133,8 +147,12 @@ let anonClosedObject: { x: int, y: int, } = { - pub x = 0; - pub y = 0 + pub x = { + 0; + }; + pub y = { + 0; + } }; let onlyHasX = {pub x = 0}; diff --git a/formatTest/typeCheckedTests/expected_output/reasonComments.re b/formatTest/typeCheckedTests/expected_output/reasonComments.re index 93a76fab0..9aa1de9d9 100644 --- a/formatTest/typeCheckedTests/expected_output/reasonComments.re +++ b/formatTest/typeCheckedTests/expected_output/reasonComments.re @@ -94,8 +94,9 @@ let myFunction = /* First arg */ withFirstArg, /* Second Arg */ andSecondArg, - ) => - withFirstArg + andSecondArg; /* After Semi */ + ) => { + withFirstArg + andSecondArg; +}; /* After Semi */ type point = { x: string, /* x field */ @@ -300,7 +301,11 @@ type intPair2 = ( int, ); -let result = /**/ (2 + 3); +let result = + /**/ + { + 2 + 3; + }; /* This is not yet idempotent */ /* { */ @@ -333,7 +338,9 @@ let blahCurriedX = x => | Black(x) => 0 /* After black */ | Green(x) => 0; /* After second green */ /* On next line after blahCurriedX def */ -let name_equal = (x, y) => x == y; +let name_equal = (x, y) => { + x == y; +}; let equal = (i1, i2) => i1.contents === i2.contents && true; /* most unlikely first */ diff --git a/formatTest/typeCheckedTests/expected_output/sequences.re b/formatTest/typeCheckedTests/expected_output/sequences.re index 831e4c1f0..72b5ad545 100644 --- a/formatTest/typeCheckedTests/expected_output/sequences.re +++ b/formatTest/typeCheckedTests/expected_output/sequences.re @@ -23,8 +23,16 @@ let twenty = 20; * printed in reduced form because sequences are a *parse* time construct. * To ensure these are parsed correctly, adding to an integer. */ -let result = 0 + twenty; -let result = 0 + twenty; +let result = + 0 + + { + twenty; + }; +let result = + 0 + + { + twenty; + }; let result = 0 + twenty; let unitValue = (); @@ -56,7 +64,9 @@ let cannotPunASingleFieldRecord = { }; let fourty = 20 + cannotPunASingleFieldRecord.number; -let thisIsASequenceNotPunedRecord = number; +let thisIsASequenceNotPunedRecord = { + number; +}; let fourty = 20 + thisIsASequenceNotPunedRecord; type recordType = { diff --git a/formatTest/typeCheckedTests/expected_output/trailing.re b/formatTest/typeCheckedTests/expected_output/trailing.re index 22bcd7513..67833739a 100644 --- a/formatTest/typeCheckedTests/expected_output/trailing.re +++ b/formatTest/typeCheckedTests/expected_output/trailing.re @@ -172,9 +172,9 @@ class virtual ]; pub virtual implementMe: (int, int) => (int, int); - initializer ( - print_string("initializing object") - ); + initializer { + print_string("initializing object"); + }; }; class extendedStack diff --git a/formatTest/unit_tests/expected_output/basicStructures.re b/formatTest/unit_tests/expected_output/basicStructures.re index ba3bdc9e6..80f698268 100644 --- a/formatTest/unit_tests/expected_output/basicStructures.re +++ b/formatTest/unit_tests/expected_output/basicStructures.re @@ -1,7 +1,8 @@ /* Copyright (c) 2015-present, Facebook, Inc. All rights reserved. */ -let run = () => +let run = () => { TestUtils.printSection("Basic Structures"); +}; while (something) { print_string("You're in a while loop"); @@ -233,12 +234,14 @@ let result = } else { print_string("b >= a"); }; - } else if ((a, b) => - if (a > b) { - print_string("b < a"); - } else { - print_string("a <= b"); - }) { + } else if ({ + (a, b) => + if (a > b) { + print_string("b < a"); + } else { + print_string("a <= b"); + }; + }) { print_string( "That could never possibly type check", ); @@ -528,9 +531,13 @@ let myTuple: myTupleType = myTuple; let myTuple: myTupleType = (one: int, two: int); /* Now functions that accept a single argument being a tuple look familiar */ -let addValues = (a: int, b: int) => a + b; +let addValues = (a: int, b: int) => { + a + b; +}; -let addValues = (a: int, b: int) => a + b; +let addValues = (a: int, b: int) => { + a + b; +}; let myFunction = (a: int, b: int): int => a + b; diff --git a/formatTest/unit_tests/expected_output/extensions.re b/formatTest/unit_tests/expected_output/extensions.re index d698af6a9..2aebb54cb 100644 --- a/formatTest/unit_tests/expected_output/extensions.re +++ b/formatTest/unit_tests/expected_output/extensions.re @@ -29,25 +29,37 @@ let x = { | Some(x) => assert(false) | None => () }; - try%extend (raise(Not_found)) { + try%extend ( + { + raise(Not_found); + } + ) { | Not_found => () | Invalid_argument(msg) => prerr_endline(msg) }; }; -let x = if%extend (true) {1} else {2}; +let x = { + if%extend (true) {1} else {2}; +}; -let x = +let x = { switch%extend (None) { | Some(x) => assert(false) | None => () }; +}; -let x = - try%extend (raise(Not_found)) { +let x = { + try%extend ( + { + raise(Not_found); + } + ) { | Not_found => () | Invalid_argument(msg) => prerr_endline(msg) }; +}; /* At structure level */ @@ -108,41 +120,50 @@ let x = /* With two extensions, alone */ -let x = [%extend1 +let x = { + %extend1 try%extend2 () { | _ => () - } -]; + }; +}; -let x = [%extend1 +let x = { + %extend1 switch%extend2 () { | _ => () - } -]; + }; +}; -let x = [%extend1 - if%extend2 (true) {1} else {2} -]; +let x = { + %extend1 + if%extend2 (true) {1} else {2}; +}; -let x = [%extend1 +let x = { + %extend1 for%extend2 (i in 1 to 10) { (); - } -]; + }; +}; -let x = [%extend1 +let x = { + %extend1 while%extend2 (false) { (); - } -]; + }; +}; -let x = [%extend1 [%extend2 () => ()]]; +let x = { + %extend1 + [%extend2 () => ()]; +}; -let x = [%extend1 +let x = { + %extend1 fun%extend2 | None => () - | Some(1) => () -]; + | Some(1) => (); +}; /* With two extensions, first in sequence */ diff --git a/formatTest/unit_tests/expected_output/fastPipe.re b/formatTest/unit_tests/expected_output/fastPipe.re index 21ea6d7eb..b2f0a54b9 100644 --- a/formatTest/unit_tests/expected_output/fastPipe.re +++ b/formatTest/unit_tests/expected_output/fastPipe.re @@ -198,11 +198,9 @@ foo##bar );
- { - items - ->Belt.Array.map(ReasonReact.string) - ->ReasonReact.array - } + {items + ->Belt.Array.map(ReasonReact.string) + ->ReasonReact.array}
; a->(b->c); diff --git a/formatTest/unit_tests/expected_output/infix.re b/formatTest/unit_tests/expected_output/infix.re index 839145b60..4cb54e0e0 100644 --- a/formatTest/unit_tests/expected_output/infix.re +++ b/formatTest/unit_tests/expected_output/infix.re @@ -1108,14 +1108,15 @@ let server = { body |> Cohttp_lwt_body.to_string >|= ( - body => + body => { Printf.sprintf( "okokok", uri, meth, headers, body, - ) + ); + } ) >>= ( body => diff --git a/formatTest/unit_tests/expected_output/jsx_functor.re b/formatTest/unit_tests/expected_output/jsx_functor.re index 653a76856..9127c674b 100644 --- a/formatTest/unit_tests/expected_output/jsx_functor.re +++ b/formatTest/unit_tests/expected_output/jsx_functor.re @@ -3,13 +3,15 @@ type elt = | Group(list(elt)); module X = { - let createElement = (~children=[], ()) => + let createElement = (~children=[], ()) => { Text("x"); + }; }; module Y = { - let createElement = (~children=[], ()) => + let createElement = (~children=[], ()) => { Text("y"); + }; }; module M = @@ -18,7 +20,7 @@ module M = Y: (module type of Y), ) => { let createElement = - (~name="M", ~id=0, ~children=[], ()) => + (~name="M", ~id=0, ~children=[], ()) => { Group( [ Text(name), @@ -28,6 +30,7 @@ module M = ] @ children, ); + }; }; let _ = diff --git a/formatTest/unit_tests/expected_output/modules.re b/formatTest/unit_tests/expected_output/modules.re index 47349833b..c6bc5defb 100644 --- a/formatTest/unit_tests/expected_output/modules.re +++ b/formatTest/unit_tests/expected_output/modules.re @@ -1,7 +1,8 @@ /* Copyright (c) 2015-present, Facebook, Inc. All rights reserved. */ -let run = () => +let run = () => { TestUtils.printSection("Modules"); +}; /** * Modules: @@ -515,24 +516,42 @@ module M = { module N = { open M; - let z = M.(34); + let z = { + M.(34); + }; let z = { open M; 34; 35; }; + let z = { + M.(34, 35); + }; let z = M.(34, 35); let z = M.(34, 35); - let z = M.(34, 35); - let z = M.{}; + let z = { + M.{}; + }; let z = M.{}; let z = M.{}; - let z = M.{x: 10}; - let z = M.[foo, bar]; - let z = M.[foo, bar]; - let z = M.{x: 10, y: 20}; - let z = M.(M2.(value)); - let z = M.(M2.value); + let z = { + M.{x: 10}; + }; + let z = { + M.[foo, bar]; + }; + let z = { + M.[foo, bar]; + }; + let z = { + M.{x: 10, y: 20}; + }; + let z = { + M.(M2.(value)); + }; + let z = { + M.(M2.value); + }; let z = { open! M; 34; diff --git a/formatTest/unit_tests/expected_output/modules_no_semi.re b/formatTest/unit_tests/expected_output/modules_no_semi.re index eebd44d7a..28c7e2b73 100644 --- a/formatTest/unit_tests/expected_output/modules_no_semi.re +++ b/formatTest/unit_tests/expected_output/modules_no_semi.re @@ -1,7 +1,8 @@ /* Copyright (c) 2015-present, Facebook, Inc. All rights reserved. */ -let run = () => +let run = () => { TestUtils.printSection("Modules"); +}; /** * Modules: @@ -515,24 +516,42 @@ module M = { module N = { open M; - let z = M.(34); + let z = { + M.(34); + }; let z = { open M; 34; 35; }; + let z = { + M.(34, 35); + }; let z = M.(34, 35); let z = M.(34, 35); - let z = M.(34, 35); - let z = M.{}; + let z = { + M.{}; + }; let z = M.{}; let z = M.{}; - let z = M.{x: 10}; - let z = M.[foo, bar]; - let z = M.[foo, bar]; - let z = M.{x: 10, y: 20}; - let z = M.(M2.(value)); - let z = M.(M2.value); + let z = { + M.{x: 10}; + }; + let z = { + M.[foo, bar]; + }; + let z = { + M.[foo, bar]; + }; + let z = { + M.{x: 10, y: 20}; + }; + let z = { + M.(M2.(value)); + }; + let z = { + M.(M2.value); + }; let z = { open! M; 34; diff --git a/formatTest/unit_tests/expected_output/polymorphism.re b/formatTest/unit_tests/expected_output/polymorphism.re index 8d1149241..c14249c32 100644 --- a/formatTest/unit_tests/expected_output/polymorphism.re +++ b/formatTest/unit_tests/expected_output/polymorphism.re @@ -1,7 +1,8 @@ /* Copyright (c) 2015-present, Facebook, Inc. All rights reserved. */ -let run = () => +let run = () => { TestUtils.printSection("Polymorphism"); +}; type myType('a) = list('a); type myTwoParamType('a, 'b) = ('a, 'b); diff --git a/formatTest/unit_tests/expected_output/syntax.re b/formatTest/unit_tests/expected_output/syntax.re index 6529819b5..cf0c4643e 100644 --- a/formatTest/unit_tests/expected_output/syntax.re +++ b/formatTest/unit_tests/expected_output/syntax.re @@ -384,8 +384,12 @@ let onlyDoingThisTopLevelLetToBypassTopLevelSequence = { type hasA = {a: int}; let a = 10; let returnsASequenceExpressionWithASingleIdentifier = - () => a; -let thisReturnsA = () => a; + () => { + a; +}; +let thisReturnsA = () => { + a; +}; let thisReturnsAAsWell = () => a; let recordVal: int = thisReturnsARecord().a; @@ -992,8 +996,9 @@ let match = "match"; let method = "method"; let foo = - (x, ~x as bar, ~z, ~foo as bar, ~foo as z) => + (x, ~x as bar, ~z, ~foo as bar, ~foo as z) => { bar + 2; +}; let zzz = myFunc(1, 2, [||]); diff --git a/src/reason-parser/reason_attributes.ml b/src/reason-parser/reason_attributes.ml index d3a9c88e2..73937fbe1 100644 --- a/src/reason-parser/reason_attributes.ml +++ b/src/reason-parser/reason_attributes.ml @@ -38,6 +38,9 @@ let rec partitionAttributes ?(partDoc=false) ?(allowUncurry=true) attrs : attrib | (({txt="reason.raw_literal"}, _) as attr) :: atTl -> let partition = partitionAttributes ~partDoc ~allowUncurry atTl in {partition with literalAttrs=attr::partition.literalAttrs} + | (({txt="reason.preserve_braces"}, _) as attr) :: atTl -> + let partition = partitionAttributes ~partDoc ~allowUncurry atTl in + {partition with literalAttrs=attr::partition.literalAttrs} | atHd :: atTl -> let partition = partitionAttributes ~partDoc ~allowUncurry atTl in {partition with stdAttrs=atHd::partition.stdAttrs} @@ -56,3 +59,9 @@ let extract_raw_literal attrs = in loop [] attrs +let is_preserve_braces_attr ({txt}, _) = + txt = "reason.preserve_braces" + +let has_preserve_braces_attrs literalAttrs = + (List.filter is_preserve_braces_attr literalAttrs) != [] + diff --git a/src/reason-parser/reason_heuristics.ml b/src/reason-parser/reason_heuristics.ml index 95115d4a2..bf80980e7 100644 --- a/src/reason-parser/reason_heuristics.ml +++ b/src/reason-parser/reason_heuristics.ml @@ -102,6 +102,13 @@ let isFastPipe e = match Ast_404.Parsetree.(e.pexp_desc) with {pexp_desc = Pexp_ident({txt = Longident.Lident("|.")})}, _ ) -> true + (* | Pexp_apply( + {pexp_desc = Pexp_apply( + {pexp_desc = Pexp_ident({txt = Longident.Lident("|.")})}, + _ + )}, + _ + ) -> Printf.eprintf "YEP\n%!";true *) | _ -> false let isUnderscoreApplication expr = diff --git a/src/reason-parser/reason_parser.mly b/src/reason-parser/reason_parser.mly index f5c3c5e6d..bd39e137e 100644 --- a/src/reason-parser/reason_parser.mly +++ b/src/reason-parser/reason_parser.mly @@ -1089,6 +1089,12 @@ let package_type_of_module_type pmty = | _ -> err pmty.pmty_loc "only module type identifier and 'with type' constraints are supported" + +let add_brace_attr expr = + let label = Location.mknoloc "reason.preserve_braces" in + let payload = PStr [] in + {expr with pexp_attributes= (label, payload) :: expr.pexp_attributes } + %} @@ -2458,7 +2464,7 @@ class_type_declaration_details: braced_expr: mark_position_exp ( LBRACE seq_expr RBRACE - { $2 } + { add_brace_attr $2 } | LBRACE as_loc(seq_expr) error { syntax_error_exp $2.loc "SyntaxError in block" } | LBRACE DOTDOTDOT expr_optional_constraint COMMA? RBRACE diff --git a/src/reason-parser/reason_pprint_ast.ml b/src/reason-parser/reason_pprint_ast.ml index 4258c6129..57a15ce68 100644 --- a/src/reason-parser/reason_pprint_ast.ml +++ b/src/reason-parser/reason_pprint_ast.ml @@ -1747,8 +1747,23 @@ let partitionFinalWrapping listTester wrapFinalItemSetting x = else Some (List.rev revEverythingButLast, last) +let should_preserve_requested_braces expr = + let {literalAttrs} = partitionAttributes expr.pexp_attributes in + match expr.pexp_desc with + | Pexp_ifthenelse _ + | Pexp_try _ -> false + | _ -> Reason_attributes.has_preserve_braces_attrs literalAttrs + let semiTerminated term = makeList [term; atom ";"] +let makeBreakableBlock ?(inline=false) letItems = + makeList + ~break:(if inline then Always else Always_rec) + ~inline:(true, inline) + ~wrap:("{", "}") + ~postSpace:true + ~sep:(if inline then NoSep else (SepFinal (";", ";"))) + letItems (* postSpace is so that when comments are interleaved, we still use spacing rules. *) let makeLetSequence letItems = @@ -1771,14 +1786,14 @@ let makeLetSequenceSingleLine letItems = letItems (* postSpace is so that when comments are interleaved, we still use spacing rules. *) -let makeUnguardedLetSequence letItems = +let makeUnguardedLetSequence ?(sep=(Layout.SepFinal (";", ";"))) letItems = makeList ~break:Always_rec ~inline:(true, true) ~wrap:("", "") ~indent:0 ~postSpace:true - ~sep:(SepFinal (";", ";")) + ~sep letItems let formatSimpleAttributed x y = @@ -3578,13 +3593,31 @@ let printer = object(self:'self) in source_map ~loc:e.pexp_loc itm - method simplifyUnparseExpr ?(inline=false) ?(wrap=("(", ")")) x = - match self#unparseExprRecurse x with + method simplifyUnparseExpr + ?(skip_brace_check=false) + ?(inline_precedence=false) + ?(inline=false) + ?(wrap=("(", ")")) + x = + match self#unparseExprRecurse ~skip_brace_check ~inline x with | SpecificInfixPrecedence (_, itm) -> - formatPrecedence ~inline ~wrap ~loc:x.pexp_loc (self#unparseResolvedRule itm) + formatPrecedence + ~inline:inline_precedence + ~wrap + ~loc:x.pexp_loc + (self#unparseResolvedRule itm) | FunctionApplication itms -> - formatPrecedence ~inline ~wrap ~loc:x.pexp_loc (formatAttachmentApplication applicationFinalWrapping None (itms, Some x.pexp_loc)) - | PotentiallyLowPrecedence itm -> formatPrecedence ~inline ~wrap ~loc:x.pexp_loc itm + formatPrecedence + ~inline:inline_precedence + ~wrap + ~loc:x.pexp_loc + (formatAttachmentApplication applicationFinalWrapping None (itms, Some x.pexp_loc)) + | PotentiallyLowPrecedence itm -> + formatPrecedence + ~inline:inline_precedence + ~wrap + ~loc:x.pexp_loc + itm | Simple itm -> itm @@ -3593,7 +3626,6 @@ let printer = object(self:'self) | InfixTree _ as infixTree -> formatComputedInfixChain (computeInfixChain infixTree) - method unparseExprApplicationItems x = match self#unparseExprRecurse x with | SpecificInfixPrecedence (_, wrappedRule) -> @@ -3843,7 +3875,7 @@ let printer = object(self:'self) | _ -> x - method unparseExprRecurse x = + method unparseExprRecurse ?(skip_brace_check=false) ?(inline=false) x = let x = self#process_underscore_application x in (* If there are any attributes, render unary like `(~-) x [@ppx]`, and infix like `(+) x y [@attr]` *) @@ -3855,9 +3887,9 @@ let printer = object(self:'self) (* If there's any attributes, recurse without them, then apply them to the ends of functions, or simplify infix printings then append. *) if stdAttrs != [] then - let withoutVisibleAttrs = {x with pexp_attributes=(arityAttrs @ jsxAttrs)} in + let withoutVisibleAttrs = {x with pexp_attributes=(literalAttrs @ arityAttrs @ jsxAttrs)} in let attributesAsList = (List.map self#attribute stdAttrs) in - let itms = match self#unparseExprRecurse withoutVisibleAttrs with + let itms = match self#unparseExprRecurse ~skip_brace_check ~inline withoutVisibleAttrs with | SpecificInfixPrecedence ({reducePrecedence}, wrappedRule) -> let itm = self#unparseResolvedRule wrappedRule in (match reducePrecedence with @@ -3877,7 +3909,7 @@ let printer = object(self:'self) (List.concat [attributesAsList; itms]) ] else - match self#simplest_expression x with + match self#simplest_expression ~skip_brace_check ~inline x with | Some se -> Simple se | None -> match x.pexp_desc with @@ -4346,7 +4378,7 @@ let printer = object(self:'self) | {pexp_desc = Pexp_fun _ } -> self#formatPexpFun ~prefix:(atom "...") ~wrap:("{", "}") expr | _ -> - let childLayout = self#simplifyUnparseExpr ~wrap:("{", "}") expr in + let childLayout = self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{", "}") expr in makeList ~break:Never [atom "..."; childLayout] in processArguments tail processedAttrs (Some [dotdotdotChild]) @@ -4356,7 +4388,9 @@ let printer = object(self:'self) | Pexp_ident ident when isPunnedJsxArg lbl ident -> makeList ~break:Layout.Never [atom "?"; atom lbl] | _ -> - label (makeList ~break:Layout.Never [atom lbl; atom "=?"]) (self#simplifyUnparseExpr ~wrap:("{","}") expression) in + label + (makeList ~break:Layout.Never [atom lbl; atom "=?"]) + (self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{","}") expression) in processArguments tail (nextAttr :: processedAttrs) children | (Labelled lbl, expression) :: tail -> @@ -4365,7 +4399,7 @@ let printer = object(self:'self) | Pexp_ident ident when isPunnedJsxArg lbl ident -> atom lbl | _ when isJSXComponent expression -> label (atom (lbl ^ "=")) - (makeList ~break:IfNeed ~wrap:("{", "}") [(self#simplifyUnparseExpr expression)]) + (makeList ~break:IfNeed ~wrap:("{", "}") [self#simplifyUnparseExpr ~skip_brace_check:true expression]) | Pexp_open (_, lid, e) when self#isSeriesOfOpensFollowedByNonSequencyExpression expression -> label (makeList [atom lbl; @@ -4393,7 +4427,7 @@ let printer = object(self:'self) let lhs = makeList [atom lbl; atom "="] in let rhs = (match printedStringAndFixityExpr eFun with | Infix str when requireNoSpaceFor str -> self#unparseExpr expression - | _ -> self#simplifyUnparseExpr ~wrap:("{","}") expression) + | _ -> self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{","}") expression) in label lhs rhs | Pexp_record _ | Pexp_construct _ @@ -4404,11 +4438,11 @@ let printer = object(self:'self) | Pexp_function _ -> label (makeList [atom lbl; atom "="]) - (self#simplifyUnparseExpr ~wrap:("{","}") expression) + (self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{","}") expression) | Pexp_fun _ -> let propName = makeList [atom lbl; atom "="] in self#formatPexpFun ~wrap:("{", "}") ~prefix:propName expression - | _ -> makeList ([atom lbl; atom "="; self#simplifyUnparseExpr ~wrap:("{","}") expression]) + | _ -> makeList ([atom lbl; atom "="; self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{","}") expression]) in processArguments tail (nextAttr :: processedAttrs) children | [] -> (processedAttrs, children) @@ -4769,7 +4803,8 @@ let printer = object(self:'self) method curriedPatternsAndReturnVal x = let uncurried = try Hashtbl.find uncurriedTable x.pexp_loc with | Not_found -> false in let rec extract_args xx = - if xx.pexp_attributes != [] then + let {stdAttrs} = partitionAttributes ~allowUncurry:false xx.pexp_attributes in + if stdAttrs != [] then ([], xx) else match xx.pexp_desc with (* label * expression option * pattern * expression *) @@ -4828,6 +4863,7 @@ let printer = object(self:'self) nonVarifiedType = same_ast_modulo_varification_and_extensions polyType nonVarifiedType && for_all2' string_equal typeVars leadingAbstractVars + (* Reinterpret this as a pattern constraint since we don't currently have a way to disambiguate. There is currently a way to disambiguate a parsing from Ppat_constraint vs. Pexp_constraint. Currently (and consistent with @@ -4935,7 +4971,7 @@ let printer = object(self:'self) | ([], _) -> (* simple let binding, e.g. `let number = 5` *) (* let f = (. a, b) => a + b; *) - let appTerms = self#unparseExprApplicationItems expr in + let appTerms = self#unparseExprApplicationItems expr in self#formatSimplePatternBinding prefixText patternList None appTerms | (_::_, _) -> let (argsWithConstraint, actualReturn) = self#normalizeFunctionArgsConstraint argsList return in @@ -5210,7 +5246,8 @@ let printer = object(self:'self) * list containing the location indicating start/end of the "let-item" and * its layout. *) let rec processLetList acc expr = - match (expr.pexp_attributes, expr.pexp_desc) with + let {stdAttrs} = partitionAttributes expr.pexp_attributes in + match (stdAttrs, expr.pexp_desc) with | ([], Pexp_let (rf, l, e)) -> (* For "letList" bindings, the start/end isn't as simple as with * module value bindings. For "let lists", the sequences were formed @@ -5285,6 +5322,11 @@ let printer = object(self:'self) let layout = source_map ~loc e1Layout in processLetList ((loc, layout)::acc) e2 | _ -> + let without_preserve_braces_attrs = + List.filter (fun x -> not (Reason_attributes.is_preserve_braces_attr x)) expr.pexp_attributes + in + let expr = { expr with pexp_attributes = without_preserve_braces_attrs } + in match expression_not_immediate_extension_sugar expr with | Some (extension, {pexp_attributes = []; pexp_desc = Pexp_let (rf, l, e)}) -> let bindingsLayout = self#bindings ~extension (rf, l) in @@ -5963,10 +6005,12 @@ let printer = object(self:'self) | _ -> raise (Invalid_argument "bs.obj only accepts a record. You've passed something else")) | _ -> assert false - method simplest_expression x = + method simplest_expression ?(skip_brace_check=false) ?(inline=false) x = let {stdAttrs; jsxAttrs} = partitionAttributes x.pexp_attributes in if stdAttrs != [] then None + else if not skip_brace_check && should_preserve_requested_braces x then + Some (makeBreakableBlock ~inline (self#letList x)) else let item = match x.pexp_desc with @@ -6106,14 +6150,13 @@ let printer = object(self:'self) let raw_literal, _ = extract_raw_literal x.pexp_attributes in self#formatChildren remaining (self#constant ?raw_literal constant :: processedRev) | {pexp_desc = Pexp_construct ({txt = Lident "::"}, Some {pexp_desc = Pexp_tuple children} )} as x :: remaining -> - begin match x.pexp_attributes with - | ({txt="JSX"}, PStr []) :: _ -> - begin match self#simplest_expression x with - | Some r -> self#formatChildren remaining (r :: processedRev) - | None -> self#formatChildren (remaining @ children) processedRev - end - | _ -> self#formatChildren (remaining @ children) processedRev - end + let {jsxAttrs} = partitionAttributes x.pexp_attributes in + if jsxAttrs != [] then + match self#simplest_expression x with + | Some r -> self#formatChildren remaining (r :: processedRev) + | None -> self#formatChildren (remaining @ children) processedRev + else + self#formatChildren (remaining @ children) processedRev | ({pexp_desc = Pexp_apply _} as e) :: remaining -> let child = (* Fast pipe behaves differently according to the expression on the @@ -6127,17 +6170,20 @@ let printer = object(self:'self) then self#formatFastPipe e else - self#simplifyUnparseExpr ~wrap:("{", "}") e + self#simplifyUnparseExpr ~inline:true ~wrap:("{", "}") e in self#formatChildren remaining (child::processedRev) | {pexp_desc = Pexp_ident li} :: remaining -> self#formatChildren remaining (self#longident_loc li :: processedRev) | {pexp_desc = Pexp_construct ({txt = Lident "[]"}, None)} :: remaining -> self#formatChildren remaining processedRev | {pexp_desc = Pexp_match _ } as head :: remaining -> + self#formatChildren + remaining + (self#simplifyUnparseExpr ~inline_precedence:true ~inline:true ~wrap:("{", "}") head :: processedRev) + | head :: remaining -> self#formatChildren remaining (self#simplifyUnparseExpr ~inline:true ~wrap:("{", "}") head :: processedRev) - | head :: remaining -> self#formatChildren remaining (self#simplifyUnparseExpr ~wrap:("{", "}") head :: processedRev) | [] -> match processedRev with | [] -> None | _::_ -> Some (List.rev processedRev) From c7182e4e5c0129e8583da932826e23a0de2b1587 Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Tue, 9 Oct 2018 21:03:36 +0100 Subject: [PATCH 2/4] Rebase and fix --- src/reason-parser/reason_attributes.ml | 9 +++++++++ src/reason-parser/reason_pprint_ast.ml | 6 ++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/reason-parser/reason_attributes.ml b/src/reason-parser/reason_attributes.ml index 73937fbe1..784c6cb6a 100644 --- a/src/reason-parser/reason_attributes.ml +++ b/src/reason-parser/reason_attributes.ml @@ -59,6 +59,15 @@ let extract_raw_literal attrs = in loop [] attrs +let without_literal_attrs attrs = + let rec loop acc = function + | attr :: rest when (partitionAttributes [attr]).literalAttrs != [] -> + loop acc rest + | [] -> List.rev acc + | attr :: rest -> loop (attr :: acc) rest + in + loop [] attrs + let is_preserve_braces_attr ({txt}, _) = txt = "reason.preserve_braces" diff --git a/src/reason-parser/reason_pprint_ast.ml b/src/reason-parser/reason_pprint_ast.ml index 57a15ce68..6208959be 100644 --- a/src/reason-parser/reason_pprint_ast.ml +++ b/src/reason-parser/reason_pprint_ast.ml @@ -4370,7 +4370,9 @@ let printer = object(self:'self) processArguments tail processedAttrs (self#formatChildren components []) | (Labelled "children", expr) :: tail -> let dotdotdotChild = match expr with - | {pexp_attributes = []; pexp_desc = Pexp_apply (funExpr, args)} when printedStringAndFixityExpr funExpr == Normal -> + | {pexp_desc = Pexp_apply (funExpr, args)} + when printedStringAndFixityExpr funExpr == Normal && + Reason_attributes.without_literal_attrs expr.pexp_attributes == [] -> begin match (self#formatFunAppl ~prefix:(atom "...") ~wrap:("{", "}") ~jsxAttrs:[] ~args ~funExpr ~applicationExpr:expr ()) with | [x] -> x | xs -> makeList xs @@ -4408,7 +4410,7 @@ let printer = object(self:'self) (self#formatNonSequencyExpression e) | Pexp_apply ({pexp_desc = Pexp_ident _} as funExpr, args) when printedStringAndFixityExpr funExpr == Normal && - expression.pexp_attributes == [] -> + Reason_attributes.without_literal_attrs expression.pexp_attributes == [] -> let lhs = makeList [atom lbl; atom "="] in begin match ( self#formatFunAppl From b0950086416b244ef7c06fc3b0635421c804a73b Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Wed, 10 Oct 2018 13:10:38 +0100 Subject: [PATCH 3/4] address review comments --- src/reason-parser/reason_heuristics.ml | 7 ------- src/reason-parser/reason_pprint_ast.ml | 29 +++++++++++++------------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/src/reason-parser/reason_heuristics.ml b/src/reason-parser/reason_heuristics.ml index bf80980e7..95115d4a2 100644 --- a/src/reason-parser/reason_heuristics.ml +++ b/src/reason-parser/reason_heuristics.ml @@ -102,13 +102,6 @@ let isFastPipe e = match Ast_404.Parsetree.(e.pexp_desc) with {pexp_desc = Pexp_ident({txt = Longident.Lident("|.")})}, _ ) -> true - (* | Pexp_apply( - {pexp_desc = Pexp_apply( - {pexp_desc = Pexp_ident({txt = Longident.Lident("|.")})}, - _ - )}, - _ - ) -> Printf.eprintf "YEP\n%!";true *) | _ -> false let isUnderscoreApplication expr = diff --git a/src/reason-parser/reason_pprint_ast.ml b/src/reason-parser/reason_pprint_ast.ml index 6208959be..bed3b6e6e 100644 --- a/src/reason-parser/reason_pprint_ast.ml +++ b/src/reason-parser/reason_pprint_ast.ml @@ -1756,15 +1756,6 @@ let should_preserve_requested_braces expr = let semiTerminated term = makeList [term; atom ";"] -let makeBreakableBlock ?(inline=false) letItems = - makeList - ~break:(if inline then Always else Always_rec) - ~inline:(true, inline) - ~wrap:("{", "}") - ~postSpace:true - ~sep:(if inline then NoSep else (SepFinal (";", ";"))) - letItems - (* postSpace is so that when comments are interleaved, we still use spacing rules. *) let makeLetSequence letItems = makeList @@ -3595,26 +3586,25 @@ let printer = object(self:'self) method simplifyUnparseExpr ?(skip_brace_check=false) - ?(inline_precedence=false) ?(inline=false) ?(wrap=("(", ")")) x = match self#unparseExprRecurse ~skip_brace_check ~inline x with | SpecificInfixPrecedence (_, itm) -> formatPrecedence - ~inline:inline_precedence + ~inline ~wrap ~loc:x.pexp_loc (self#unparseResolvedRule itm) | FunctionApplication itms -> formatPrecedence - ~inline:inline_precedence + ~inline ~wrap ~loc:x.pexp_loc (formatAttachmentApplication applicationFinalWrapping None (itms, Some x.pexp_loc)) | PotentiallyLowPrecedence itm -> formatPrecedence - ~inline:inline_precedence + ~inline ~wrap ~loc:x.pexp_loc itm @@ -6012,7 +6002,16 @@ let printer = object(self:'self) if stdAttrs != [] then None else if not skip_brace_check && should_preserve_requested_braces x then - Some (makeBreakableBlock ~inline (self#letList x)) + let layout = + makeList + ~break:(if inline then Always else Always_rec) + ~inline:(true, inline) + ~wrap:("{", "}") + ~postSpace:true + ~sep:(if inline then NoSep else (SepFinal (";", ";"))) + (self#letList x) + in + Some layout else let item = match x.pexp_desc with @@ -6181,7 +6180,7 @@ let printer = object(self:'self) | {pexp_desc = Pexp_match _ } as head :: remaining -> self#formatChildren remaining - (self#simplifyUnparseExpr ~inline_precedence:true ~inline:true ~wrap:("{", "}") head :: processedRev) + (self#simplifyUnparseExpr ~inline:true ~wrap:("{", "}") head :: processedRev) | head :: remaining -> self#formatChildren remaining From 373ed5f64d2f2a389e5a507270178dc5b1bf7efa Mon Sep 17 00:00:00 2001 From: Antonio Nuno Monteiro Date: Thu, 11 Oct 2018 14:27:08 +0100 Subject: [PATCH 4/4] Adding so many optional laballed to `unparseExprRecurse` can't scale --- src/reason-parser/reason_pprint_ast.ml | 79 ++++++++++++++------------ 1 file changed, 43 insertions(+), 36 deletions(-) diff --git a/src/reason-parser/reason_pprint_ast.ml b/src/reason-parser/reason_pprint_ast.ml index bed3b6e6e..886262d7c 100644 --- a/src/reason-parser/reason_pprint_ast.ml +++ b/src/reason-parser/reason_pprint_ast.ml @@ -1747,13 +1747,6 @@ let partitionFinalWrapping listTester wrapFinalItemSetting x = else Some (List.rev revEverythingButLast, last) -let should_preserve_requested_braces expr = - let {literalAttrs} = partitionAttributes expr.pexp_attributes in - match expr.pexp_desc with - | Pexp_ifthenelse _ - | Pexp_try _ -> false - | _ -> Reason_attributes.has_preserve_braces_attrs literalAttrs - let semiTerminated term = makeList [term; atom ";"] (* postSpace is so that when comments are interleaved, we still use spacing rules. *) @@ -2299,6 +2292,9 @@ let printer = object(self:'self) val pipe = false val semi = false + val inline_braces = false + val preserve_braces = true + (* *Mutable state* in the printer to keep track of all comments * Used when whitespace needs to be interleaved. * The printing algorithm needs to take the comments into account in between @@ -2335,6 +2331,10 @@ let printer = object(self:'self) method reset_pipe = {} method reset = {} + method inline_braces = {} + method dont_preserve_braces = {} + method reset_request_braces = {} + method longident = function | Lident s -> (protectIdentifier s) @@ -3584,12 +3584,8 @@ let printer = object(self:'self) in source_map ~loc:e.pexp_loc itm - method simplifyUnparseExpr - ?(skip_brace_check=false) - ?(inline=false) - ?(wrap=("(", ")")) - x = - match self#unparseExprRecurse ~skip_brace_check ~inline x with + method simplifyUnparseExpr ?(inline=false) ?(wrap=("(", ")")) x = + match self#unparseExprRecurse x with | SpecificInfixPrecedence (_, itm) -> formatPrecedence ~inline @@ -3865,7 +3861,7 @@ let printer = object(self:'self) | _ -> x - method unparseExprRecurse ?(skip_brace_check=false) ?(inline=false) x = + method unparseExprRecurse x = let x = self#process_underscore_application x in (* If there are any attributes, render unary like `(~-) x [@ppx]`, and infix like `(+) x y [@attr]` *) @@ -3879,7 +3875,7 @@ let printer = object(self:'self) if stdAttrs != [] then let withoutVisibleAttrs = {x with pexp_attributes=(literalAttrs @ arityAttrs @ jsxAttrs)} in let attributesAsList = (List.map self#attribute stdAttrs) in - let itms = match self#unparseExprRecurse ~skip_brace_check ~inline withoutVisibleAttrs with + let itms = match self#unparseExprRecurse withoutVisibleAttrs with | SpecificInfixPrecedence ({reducePrecedence}, wrappedRule) -> let itm = self#unparseResolvedRule wrappedRule in (match reducePrecedence with @@ -3899,7 +3895,7 @@ let printer = object(self:'self) (List.concat [attributesAsList; itms]) ] else - match self#simplest_expression ~skip_brace_check ~inline x with + match self#simplest_expression x with | Some se -> Simple se | None -> match x.pexp_desc with @@ -4370,7 +4366,7 @@ let printer = object(self:'self) | {pexp_desc = Pexp_fun _ } -> self#formatPexpFun ~prefix:(atom "...") ~wrap:("{", "}") expr | _ -> - let childLayout = self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{", "}") expr in + let childLayout = self#dont_preserve_braces#simplifyUnparseExpr ~wrap:("{", "}") expr in makeList ~break:Never [atom "..."; childLayout] in processArguments tail processedAttrs (Some [dotdotdotChild]) @@ -4382,7 +4378,7 @@ let printer = object(self:'self) | _ -> label (makeList ~break:Layout.Never [atom lbl; atom "=?"]) - (self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{","}") expression) in + (self#dont_preserve_braces#simplifyUnparseExpr ~wrap:("{","}") expression) in processArguments tail (nextAttr :: processedAttrs) children | (Labelled lbl, expression) :: tail -> @@ -4391,7 +4387,8 @@ let printer = object(self:'self) | Pexp_ident ident when isPunnedJsxArg lbl ident -> atom lbl | _ when isJSXComponent expression -> label (atom (lbl ^ "=")) - (makeList ~break:IfNeed ~wrap:("{", "}") [self#simplifyUnparseExpr ~skip_brace_check:true expression]) + (makeList ~break:IfNeed ~wrap:("{", "}") + [self#dont_preserve_braces#simplifyUnparseExpr expression]) | Pexp_open (_, lid, e) when self#isSeriesOfOpensFollowedByNonSequencyExpression expression -> label (makeList [atom lbl; @@ -4419,7 +4416,7 @@ let printer = object(self:'self) let lhs = makeList [atom lbl; atom "="] in let rhs = (match printedStringAndFixityExpr eFun with | Infix str when requireNoSpaceFor str -> self#unparseExpr expression - | _ -> self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{","}") expression) + | _ -> self#dont_preserve_braces#simplifyUnparseExpr ~wrap:("{","}") expression) in label lhs rhs | Pexp_record _ | Pexp_construct _ @@ -4430,11 +4427,15 @@ let printer = object(self:'self) | Pexp_function _ -> label (makeList [atom lbl; atom "="]) - (self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{","}") expression) + (self#dont_preserve_braces#simplifyUnparseExpr ~wrap:("{","}") expression) | Pexp_fun _ -> let propName = makeList [atom lbl; atom "="] in self#formatPexpFun ~wrap:("{", "}") ~prefix:propName expression - | _ -> makeList ([atom lbl; atom "="; self#simplifyUnparseExpr ~skip_brace_check:true ~wrap:("{","}") expression]) + | _ -> makeList [ + atom lbl; + atom "="; + self#dont_preserve_braces#simplifyUnparseExpr ~wrap:("{","}") expression + ] in processArguments tail (nextAttr :: processedAttrs) children | [] -> (processedAttrs, children) @@ -5238,7 +5239,7 @@ let printer = object(self:'self) * list containing the location indicating start/end of the "let-item" and * its layout. *) let rec processLetList acc expr = - let {stdAttrs} = partitionAttributes expr.pexp_attributes in + let {stdAttrs; arityAttrs; jsxAttrs} = partitionAttributes ~allowUncurry:false expr.pexp_attributes in match (stdAttrs, expr.pexp_desc) with | ([], Pexp_let (rf, l, e)) -> (* For "letList" bindings, the start/end isn't as simple as with @@ -5314,10 +5315,7 @@ let printer = object(self:'self) let layout = source_map ~loc e1Layout in processLetList ((loc, layout)::acc) e2 | _ -> - let without_preserve_braces_attrs = - List.filter (fun x -> not (Reason_attributes.is_preserve_braces_attr x)) expr.pexp_attributes - in - let expr = { expr with pexp_attributes = without_preserve_braces_attrs } + let expr = { expr with pexp_attributes = (arityAttrs @ stdAttrs @ jsxAttrs) } in match expression_not_immediate_extension_sugar expr with | Some (extension, {pexp_attributes = []; pexp_desc = Pexp_let (rf, l, e)}) -> @@ -5338,7 +5336,7 @@ let printer = object(self:'self) (expr.pexp_loc, layout)::acc in let es = processLetList [] expr in - (* Interleave whitespace between the "let-items" when appropiate *) + (* Interleave whitespace between the "let-items" when appropriate *) groupAndPrint ~xf:(fun (_, layout) -> layout) ~getLoc:(fun (loc, _) -> loc) @@ -5997,18 +5995,27 @@ let printer = object(self:'self) | _ -> raise (Invalid_argument "bs.obj only accepts a record. You've passed something else")) | _ -> assert false - method simplest_expression ?(skip_brace_check=false) ?(inline=false) x = + method should_preserve_requested_braces expr = + let {literalAttrs} = partitionAttributes expr.pexp_attributes in + match expr.pexp_desc with + | Pexp_ifthenelse _ + | Pexp_try _ -> false + | _ -> + preserve_braces && + Reason_attributes.has_preserve_braces_attrs literalAttrs + + method simplest_expression x = let {stdAttrs; jsxAttrs} = partitionAttributes x.pexp_attributes in if stdAttrs != [] then None - else if not skip_brace_check && should_preserve_requested_braces x then + else if self#should_preserve_requested_braces x then let layout = makeList - ~break:(if inline then Always else Always_rec) - ~inline:(true, inline) + ~break:(if inline_braces then Always else Always_rec) + ~inline:(true, inline_braces) ~wrap:("{", "}") ~postSpace:true - ~sep:(if inline then NoSep else (SepFinal (";", ";"))) + ~sep:(if inline_braces then NoSep else (SepFinal (";", ";"))) (self#letList x) in Some layout @@ -6171,7 +6178,7 @@ let printer = object(self:'self) then self#formatFastPipe e else - self#simplifyUnparseExpr ~inline:true ~wrap:("{", "}") e + self#inline_braces#simplifyUnparseExpr ~inline:true ~wrap:("{", "}") e in self#formatChildren remaining (child::processedRev) | {pexp_desc = Pexp_ident li} :: remaining -> @@ -6180,11 +6187,11 @@ let printer = object(self:'self) | {pexp_desc = Pexp_match _ } as head :: remaining -> self#formatChildren remaining - (self#simplifyUnparseExpr ~inline:true ~wrap:("{", "}") head :: processedRev) + (self#inline_braces#simplifyUnparseExpr ~inline:true ~wrap:("{", "}") head :: processedRev) | head :: remaining -> self#formatChildren remaining - (self#simplifyUnparseExpr ~inline:true ~wrap:("{", "}") head :: processedRev) + (self#inline_braces#simplifyUnparseExpr ~inline:true ~wrap:("{", "}") head :: processedRev) | [] -> match processedRev with | [] -> None | _::_ -> Some (List.rev processedRev)