From e9c77179122bf389c56fe2f244053101248772c2 Mon Sep 17 00:00:00 2001 From: Jordan Walke Date: Wed, 17 Apr 2019 01:20:50 -0700 Subject: [PATCH 1/7] Slightly better breaking of unary prefix symbols. Summary: Before these would break like: ``` !! myFunction( foo, bar ) ``` But now they break like ``` !!myFunction( foo, bar ) ``` This is still not ideal, but it's an improvement. Also, one downside is that now broken nots are not aligned to two characters. ``` let res = !myFunction( foo, bar ) ``` However, I think that is better than what was there previously. ``` let res = ! myFunction( foo, bar ) ``` This doesn't come up with unary minus, or even unary operators that are of character length two. It's mostly about `!`. I'd say this is still an improvement. To fix the remaining issue seems pretty invasive at the moment. For a better fix, I might suggest a pass that prepends unary operators to function names in certain cases, and then running the printer as if these unary prefixes didn't even exist (in those cases). Test Plan: Reviewers: CC: --- .../errorTests/expected_output/syntaxError.re | 2 +- .../typeCheckedTests/expected_output/unary.re | 88 +++++++++++++++++++ formatTest/typeCheckedTests/input/unary.re | 51 +++++++++++ src/reason-parser/reason_pprint_ast.ml | 6 +- 4 files changed, 143 insertions(+), 4 deletions(-) create mode 100644 formatTest/typeCheckedTests/expected_output/unary.re create mode 100644 formatTest/typeCheckedTests/input/unary.re diff --git a/formatTest/errorTests/expected_output/syntaxError.re b/formatTest/errorTests/expected_output/syntaxError.re index 4f9ab58b4..258b460c8 100644 --- a/formatTest/errorTests/expected_output/syntaxError.re +++ b/formatTest/errorTests/expected_output/syntaxError.re @@ -1,3 +1,3 @@ File "syntaxError.re", line 1, characters 9-10: Error: File "syntaxError.re", line 1, characters 9-10: -Error: Syntax error + Error: Syntax error diff --git a/formatTest/typeCheckedTests/expected_output/unary.re b/formatTest/typeCheckedTests/expected_output/unary.re new file mode 100644 index 000000000..232b8c9e6 --- /dev/null +++ b/formatTest/typeCheckedTests/expected_output/unary.re @@ -0,0 +1,88 @@ +type result('a, 'b) = + | Ok('a) + | Error('b); + +let returnsAResultToBeUnwrapped = (a, b, c) => + switch (a, b, c) { + | (None, Some(i), Some(j)) => Ok((i, j)) + | (Some(i), Some(j), None) => + Ok(("hi", "bye")) + | _ => + Error( + Invalid_argument( + "This is not a valid argument", + ), + ) + }; + +let returnsAnIntegerResultNotWrapped = (a, b, c) => + switch (a, b, c) { + | (None, Some(i), Some(j)) => 0 + | (Some(i), Some(j), None) => (-1) + | _ => 100 + }; + +let returnsABoolReturnValueNoResult = (a, b, c) => + switch (a, b, c) { + | (None, Some(i), Some(j)) => true + | (Some(i), Some(j), None) => false + | _ => false + }; + +let (!!) = res => + switch (res) { + | Ok(o) => o + | Error(e) => raise(e) + }; + +let result = + !!returnsAResultToBeUnwrapped( + Some( + "this realy long string will make things wrap", + ), + Some( + "this realy long string will make things wrap", + ), + Some( + "this realy long string will make things wrap", + ), + ); + +let result = + - returnsAnIntegerResultNotWrapped( + Some( + "this realy long string will make things wrap", + ), + Some( + "this realy long string will make things wrap", + ), + Some( + "this realy long string will make things wrap", + ), + ); + +let result = + + returnsAnIntegerResultNotWrapped( + Some( + "this realy long string will make things wrap", + ), + Some( + "this realy long string will make things wrap", + ), + Some( + "this realy long string will make things wrap", + ), + ); + +let result = + !returnsABoolReturnValueNoResult( + Some( + "this realy long string will make things wrap", + ), + Some( + "this realy long string will make things wrap", + ), + Some( + "this realy long string will make things wrap", + ), + ); diff --git a/formatTest/typeCheckedTests/input/unary.re b/formatTest/typeCheckedTests/input/unary.re new file mode 100644 index 000000000..6b6dd3a02 --- /dev/null +++ b/formatTest/typeCheckedTests/input/unary.re @@ -0,0 +1,51 @@ + + +type result('a, 'b) = Ok('a) | Error('b); + +let returnsAResultToBeUnwrapped = (a, b, c) => switch(a, b, c) { + | (None, Some(i), Some(j)) => Ok((i, j)); + | (Some(i), Some(j), None) => Ok(("hi", "bye")); + | _ => Error(Invalid_argument("This is not a valid argument")) +}; + +let returnsAnIntegerResultNotWrapped = (a, b, c) => switch(a,b, c) { + | (None, Some(i), Some(j)) => 0 + | (Some(i), Some(j), None) => -1 + | _ => 100 +}; + +let returnsABoolReturnValueNoResult = (a, b, c) => switch(a,b, c) { + | (None, Some(i), Some(j)) => true + | (Some(i), Some(j), None) => false + | _ => false +}; + +let (!!) = res => switch(res) { + | Ok(o) => o + | Error(e) => raise(e) +}; + + +let result = !!returnsAResultToBeUnwrapped( + Some("this realy long string will make things wrap"), + Some("this realy long string will make things wrap"), + Some("this realy long string will make things wrap") +); + +let result = -returnsAnIntegerResultNotWrapped( + Some("this realy long string will make things wrap"), + Some("this realy long string will make things wrap"), + Some("this realy long string will make things wrap") +) + +let result = +returnsAnIntegerResultNotWrapped( + Some("this realy long string will make things wrap"), + Some("this realy long string will make things wrap"), + Some("this realy long string will make things wrap") +) + +let result = !returnsABoolReturnValueNoResult( + Some("this realy long string will make things wrap"), + Some("this realy long string will make things wrap"), + Some("this realy long string will make things wrap") +); diff --git a/src/reason-parser/reason_pprint_ast.ml b/src/reason-parser/reason_pprint_ast.ml index a02699219..94443d196 100644 --- a/src/reason-parser/reason_pprint_ast.ml +++ b/src/reason-parser/reason_pprint_ast.ml @@ -4068,7 +4068,7 @@ let printer = object(self:'self) self#ensureContainingRule ~withPrecedence:prec ~reducesAfterRight:rightExpr () ) in SpecificInfixPrecedence - ({reducePrecedence=prec; shiftPrecedence = prec}, LayoutNode (label ~space:forceSpace (atom prefixStr) rightItm)) + ({reducePrecedence=prec; shiftPrecedence = prec}, LayoutNode (label ~indent:0 ~break:`Never ~space:forceSpace (atom prefixStr) rightItm)) | (UnaryPostfix postfixStr, [(Nolabel, leftExpr)]) -> let forceSpace = match leftExpr.pexp_desc with | Pexp_apply (ee, _) -> @@ -4118,7 +4118,7 @@ let printer = object(self:'self) let rightItm = self#unparseResolvedRule ( self#ensureContainingRule ~withPrecedence:prec ~reducesAfterRight:rightExpr () ) in - let expr = label ~space:true (atom printedIdent) rightItm in + let expr = label ~indent:0 ~break:`Never ~space:true (atom printedIdent) rightItm in SpecificInfixPrecedence ({reducePrecedence=prec; shiftPrecedence=Token printedIdent}, LayoutNode expr) | (UnaryMinusPrefix printedIdent as x, [(Nolabel, rightExpr)]) | (UnaryNotPrefix printedIdent as x, [(Nolabel, rightExpr)]) -> @@ -4133,7 +4133,7 @@ let printer = object(self:'self) let rightItm = self#unparseResolvedRule ( self#ensureContainingRule ~withPrecedence:prec ~reducesAfterRight:rightExpr () ) in - let expr = label ~space:forceSpace (atom printedIdent) rightItm in + let expr = label ~break:`Never ~indent:0 ~space:forceSpace (atom printedIdent) rightItm in SpecificInfixPrecedence ({reducePrecedence=prec; shiftPrecedence=Token printedIdent}, LayoutNode expr) (* Will need to be rendered in self#expression as (~-) x y z. *) | (_, _) -> From b0625b532b358b6903009445c5d31aefd72681ca Mon Sep 17 00:00:00 2001 From: Jordan Walke Date: Wed, 17 Apr 2019 01:44:25 -0700 Subject: [PATCH 2/7] Add call to run tests in azure pipelines --- .ci/esy-build-steps.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.ci/esy-build-steps.yml b/.ci/esy-build-steps.yml index 7c63d636e..d73679623 100644 --- a/.ci/esy-build-steps.yml +++ b/.ci/esy-build-steps.yml @@ -11,5 +11,7 @@ steps: - template: utils/publish-build-cache.yml - script: esy x refmt --version displayName: 'esy x refmt --version' + - script: esy x refmt --version + displayName: 'esy x make test' # Run tests or any additional steps here # - script: esy b dune runtest From 47a9e384979d6e6821ef82f7f42c9c006b7ce7cb Mon Sep 17 00:00:00 2001 From: Jordan Walke Date: Wed, 17 Apr 2019 01:51:02 -0700 Subject: [PATCH 3/7] Whoops, actually enable tests on azure --- .ci/esy-build-steps.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/esy-build-steps.yml b/.ci/esy-build-steps.yml index d73679623..be2e7e23c 100644 --- a/.ci/esy-build-steps.yml +++ b/.ci/esy-build-steps.yml @@ -11,7 +11,7 @@ steps: - template: utils/publish-build-cache.yml - script: esy x refmt --version displayName: 'esy x refmt --version' - - script: esy x refmt --version + - script: esy x make test displayName: 'esy x make test' # Run tests or any additional steps here # - script: esy b dune runtest From 92fef509ed52dccf817ee303ec4983bae0704041 Mon Sep 17 00:00:00 2001 From: Jordan Walke Date: Wed, 17 Apr 2019 02:20:44 -0700 Subject: [PATCH 4/7] Add special case for 4.06.0 too - to fix breaking test --- formatTest/errorTests/expected_output/syntaxError.re | 2 +- formatTest/errorTests/expected_output/syntaxError.re.4.06.0 | 3 +++ formatTest/test.sh | 2 ++ 3 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 formatTest/errorTests/expected_output/syntaxError.re.4.06.0 diff --git a/formatTest/errorTests/expected_output/syntaxError.re b/formatTest/errorTests/expected_output/syntaxError.re index 258b460c8..4f9ab58b4 100644 --- a/formatTest/errorTests/expected_output/syntaxError.re +++ b/formatTest/errorTests/expected_output/syntaxError.re @@ -1,3 +1,3 @@ File "syntaxError.re", line 1, characters 9-10: Error: File "syntaxError.re", line 1, characters 9-10: - Error: Syntax error +Error: Syntax error diff --git a/formatTest/errorTests/expected_output/syntaxError.re.4.06.0 b/formatTest/errorTests/expected_output/syntaxError.re.4.06.0 new file mode 100644 index 000000000..258b460c8 --- /dev/null +++ b/formatTest/errorTests/expected_output/syntaxError.re.4.06.0 @@ -0,0 +1,3 @@ +File "syntaxError.re", line 1, characters 9-10: +Error: File "syntaxError.re", line 1, characters 9-10: + Error: Syntax error diff --git a/formatTest/test.sh b/formatTest/test.sh index a7b9516dd..f3b97e804 100755 --- a/formatTest/test.sh +++ b/formatTest/test.sh @@ -13,6 +13,8 @@ VERBOSE=${VERBOSE:-} OCAML_VERSION=`echo $(ocaml -version) | egrep -o '[0-9]+.[0-9]+.[0-9]+' | head -1` OCAML_VERSION=${OCAML_VERSION:-"4.02.3"} +echo "Testing with OCaml Version: $OCAML_VERSION" + DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )" REFMT="$DIR/../_build/install/default/bin/refmt" From 4f1b2cffe09ebb35b2836bc5b81ebdea94488731 Mon Sep 17 00:00:00 2001 From: Jordan Walke Date: Wed, 17 Apr 2019 02:34:19 -0700 Subject: [PATCH 5/7] Don't run esy x make test - run esy make test for windows --- .ci/esy-build-steps.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/esy-build-steps.yml b/.ci/esy-build-steps.yml index be2e7e23c..c92925185 100644 --- a/.ci/esy-build-steps.yml +++ b/.ci/esy-build-steps.yml @@ -12,6 +12,6 @@ steps: - script: esy x refmt --version displayName: 'esy x refmt --version' - script: esy x make test - displayName: 'esy x make test' + displayName: 'esy make test' # Run tests or any additional steps here # - script: esy b dune runtest From 11fdfa847f0f119254f6b83be7fb65493d619f66 Mon Sep 17 00:00:00 2001 From: Jordan Walke Date: Wed, 17 Apr 2019 02:40:13 -0700 Subject: [PATCH 6/7] Syntax error for 4.02.3 --- formatTest/errorTests/expected_output/syntaxError.re | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/formatTest/errorTests/expected_output/syntaxError.re b/formatTest/errorTests/expected_output/syntaxError.re index 4f9ab58b4..258b460c8 100644 --- a/formatTest/errorTests/expected_output/syntaxError.re +++ b/formatTest/errorTests/expected_output/syntaxError.re @@ -1,3 +1,3 @@ File "syntaxError.re", line 1, characters 9-10: Error: File "syntaxError.re", line 1, characters 9-10: -Error: Syntax error + Error: Syntax error From bc7a16e33129d8b27f50596ff9c71e7d4db4b31b Mon Sep 17 00:00:00 2001 From: Jordan Walke Date: Wed, 17 Apr 2019 02:51:52 -0700 Subject: [PATCH 7/7] fix --- .ci/esy-build-steps.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.ci/esy-build-steps.yml b/.ci/esy-build-steps.yml index c92925185..4084ca182 100644 --- a/.ci/esy-build-steps.yml +++ b/.ci/esy-build-steps.yml @@ -11,7 +11,7 @@ steps: - template: utils/publish-build-cache.yml - script: esy x refmt --version displayName: 'esy x refmt --version' - - script: esy x make test + - script: esy make test displayName: 'esy make test' # Run tests or any additional steps here # - script: esy b dune runtest