From 1e15bb59c6fd9b56780f19dd1a6fc21cad39a87c Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Sat, 29 Oct 2022 15:10:26 +0200 Subject: [PATCH 1/5] Treat await as an almost unary opeator. Making pipe behave like application seems pretty unlikely. First `->` has deep binary operator behavior, including associating to the left on a->b->c, and mixes in specific ways with other operators. Instead application lives on a different level entirely with no issues of how to treat specially the right-hand-side which is enclosed in parens. The await operator is essentially a unary operator. After all it takes on argument. It is also treated as such in JS. Some expected behaviours are that `await 2 + await 3` just like `-2 + -3` is expected to mean `(await 2) + (await 3)`. The issue raised is with pipe: the desired outcome is that `await a->b` is parsed as `await a->b`. However notice that `- a->b` is parsed as `(-a)->b`. So on one side, one would like to have `await` behave like other unary operators. On the other side one wouldn't. As a compromise, this PR treats `await` as an almost-unary operator. In the sense that it binds more tightly than any "real" binary operator such as `+` and `**`, but not more than `->` and `.`. This introduces some lack of uniformity, and requires special-casing parts of parsing and printing. --- src/res_core.ml | 3 ++- src/res_parens.ml | 16 +++++++++++++-- src/res_parens.mli | 2 +- src/res_printer.ml | 6 ++++-- tests/parsing/grammar/expressions/async.res | 7 ++++++- .../expressions/expected/async.res.txt | 6 +++++- tests/printer/expr/asyncAwait.res | 12 ++++++++++- .../printer/expr/expected/asyncAwait.res.txt | 20 ++++++++++++++----- 8 files changed, 58 insertions(+), 14 deletions(-) diff --git a/src/res_core.ml b/src/res_core.ml index d535487d..e4048594 100644 --- a/src/res_core.ml +++ b/src/res_core.ml @@ -3134,7 +3134,8 @@ and parseAwaitExpression p = let awaitLoc = mkLoc p.Parser.startPos p.endPos in let awaitAttr = makeAwaitAttr awaitLoc in Parser.expect Await p; - let expr = parseUnaryExpr p in + let tokenPrec = Token.precedence MinusGreater in + let expr = parseBinaryExpr ~context:OrdinaryExpr p tokenPrec in { expr with pexp_attributes = awaitAttr :: expr.pexp_attributes; diff --git a/src/res_parens.ml b/src/res_parens.ml index c18b7565..0bbe96f3 100644 --- a/src/res_parens.ml +++ b/src/res_parens.ml @@ -175,7 +175,7 @@ let flattenOperandRhs parentOperator rhs = | _ when ParsetreeViewer.isTernaryExpr rhs -> true | _ -> false -let lazyOrAssertOrAwaitExprRhs expr = +let lazyOrAssertOrAwaitExprRhs ?(inAwait = false) expr = let optBraces, _ = ParsetreeViewer.processBracesAttr expr in match optBraces with | Some ({Location.loc = bracesLoc}, _) -> Braced bracesLoc @@ -186,6 +186,16 @@ let lazyOrAssertOrAwaitExprRhs expr = | _ :: _ -> true | [] -> false -> Parenthesized + | { + pexp_desc = + Pexp_apply ({pexp_desc = Pexp_ident {txt = Longident.Lident operator}}, _); + } + when inAwait + && ParsetreeViewer.isBinaryExpression expr + && ParsetreeViewer.hasAwaitAttribute expr.pexp_attributes + && ParsetreeViewer.operatorPrecedence operator + >= ParsetreeViewer.operatorPrecedence "|." -> + Nothing | expr when ParsetreeViewer.isBinaryExpression expr -> Parenthesized | { pexp_desc = @@ -202,7 +212,9 @@ let lazyOrAssertOrAwaitExprRhs expr = | Pexp_try _ | Pexp_while _ | Pexp_for _ | Pexp_ifthenelse _ ); } -> Parenthesized - | _ when ParsetreeViewer.hasAwaitAttribute expr.pexp_attributes -> + | _ + when (not inAwait) + && ParsetreeViewer.hasAwaitAttribute expr.pexp_attributes -> Parenthesized | _ -> Nothing) diff --git a/src/res_parens.mli b/src/res_parens.mli index cedf98e1..dbbfffe1 100644 --- a/src/res_parens.mli +++ b/src/res_parens.mli @@ -10,7 +10,7 @@ val subBinaryExprOperand : string -> string -> bool val rhsBinaryExprOperand : string -> Parsetree.expression -> bool val flattenOperandRhs : string -> Parsetree.expression -> bool -val lazyOrAssertOrAwaitExprRhs : Parsetree.expression -> kind +val lazyOrAssertOrAwaitExprRhs : ?inAwait:bool -> Parsetree.expression -> kind val fieldExpr : Parsetree.expression -> kind diff --git a/src/res_printer.ml b/src/res_printer.ml index 8179ca0f..600d0be6 100644 --- a/src/res_printer.ml +++ b/src/res_printer.ml @@ -3330,13 +3330,13 @@ and printExpression ~customLayout (e : Parsetree.expression) cmtTbl = if ParsetreeViewer.hasAwaitAttribute e.pexp_attributes then let rhs = match - Parens.lazyOrAssertOrAwaitExprRhs + Parens.lazyOrAssertOrAwaitExprRhs ~inAwait:true { e with pexp_attributes = List.filter (function - | {Location.txt = "res.await" | "ns.braces"}, _ -> false + | {Location.txt = "ns.braces"}, _ -> false | _ -> true) e.pexp_attributes; } @@ -3614,12 +3614,14 @@ and printBinaryExpression ~customLayout (expr : Parsetree.expression) cmtTbl = if isAwait then Doc.concat [ + Doc.lparen; Doc.text "await "; Doc.lparen; leftPrinted; printBinaryOperator ~inlineRhs:false operator; rightPrinted; Doc.rparen; + Doc.rparen; ] else Doc.concat diff --git a/tests/parsing/grammar/expressions/async.res b/tests/parsing/grammar/expressions/async.res index 9f33ef2d..0a499bad 100644 --- a/tests/parsing/grammar/expressions/async.res +++ b/tests/parsing/grammar/expressions/async.res @@ -29,4 +29,9 @@ let async = { let f = isPositive ? (async (a, b) : int => a + b) : async (c, d) : int => c - d let foo = async(~a=34) -let bar = async(~a)=>a+1 \ No newline at end of file +let bar = async(~a)=>a+1 + +let ex1 = await 3 + await 4 +let ex2 = await 3 ** await 4 +let ex3 = await foo->bar(~arg) +let ex4 = await foo.bar.baz diff --git a/tests/parsing/grammar/expressions/expected/async.res.txt b/tests/parsing/grammar/expressions/expected/async.res.txt index eb4424d9..fe86bf4d 100644 --- a/tests/parsing/grammar/expressions/expected/async.res.txt +++ b/tests/parsing/grammar/expressions/expected/async.res.txt @@ -27,4 +27,8 @@ let f = else (((fun c -> fun d -> (c - d : int)))[@res.async ])) [@ns.ternary ]) let foo = async ~a:((34)[@ns.namedArgLoc ]) -let bar = ((fun ~a:((a)[@ns.namedArgLoc ]) -> a + 1)[@res.async ]) \ No newline at end of file +let bar = ((fun ~a:((a)[@ns.namedArgLoc ]) -> a + 1)[@res.async ]) +let ex1 = ((3)[@res.await ]) + ((4)[@res.await ]) +let ex2 = ((3)[@res.await ]) ** ((4)[@res.await ]) +let ex3 = ((foo |. (bar ~arg:((arg)[@ns.namedArgLoc ])))[@res.await ]) +let ex4 = (((foo.bar).baz)[@res.await ]) \ No newline at end of file diff --git a/tests/printer/expr/asyncAwait.res b/tests/printer/expr/asyncAwait.res index efabc856..94349275 100644 --- a/tests/printer/expr/asyncAwait.res +++ b/tests/printer/expr/asyncAwait.res @@ -101,4 +101,14 @@ let aw = (await (server->start))->foo let aw = (@foo (server->start))->foo let foo = async(~a=34) -let bar = async(~a)=>a+1 \ No newline at end of file +let bar = async(~a)=>a+1 + +let a1 = await 3 + await 4 +let a2 = await 3 ** await 4 +let a3 = await foo->bar(~arg) +let a4 = await foo.bar.baz + +let b1 = await (3+4) +let b2 = await (3**4) +let b3 = await (foo->bar(~arg)) +let b4 = await (foo.bar.baz) diff --git a/tests/printer/expr/expected/asyncAwait.res.txt b/tests/printer/expr/expected/asyncAwait.res.txt index 1082a3d8..208b60b8 100644 --- a/tests/printer/expr/expected/asyncAwait.res.txt +++ b/tests/printer/expr/expected/asyncAwait.res.txt @@ -30,14 +30,14 @@ user.data = await fetch() {await weirdReactSuspenseApi} -let inBinaryExpression = (await x)->Js.Promise.resolve + 1 -let inBinaryExpression = (await x)->Js.Promise.resolve + (await y)->Js.Promise.resolve +let inBinaryExpression = await x->Js.Promise.resolve + 1 +let inBinaryExpression = await x->Js.Promise.resolve + await y->Js.Promise.resolve let withCallback = async (. ()) => { - async (. x) => await (x->Js.promise.resolve) + 1 + async (. x) => await x->Js.promise.resolve + 1 } -let () = (await fetch(url))->(await resolve) +let () = await (await fetch(url))->(await resolve) let _ = await (1 + 1) let _ = (await 1) + 1 @@ -119,8 +119,18 @@ let f11 = (. ~x) => (. ~y) => 3 let f12 = @a x => 3 let f13 = (@a @b ~x) => 3 -let aw = await (server->start)->foo +let aw = (await (server->start))->foo let aw = @foo (server->start)->foo let foo = async(~a=34) let bar = async (~a) => a + 1 + +let a1 = (await 3) + (await 4) +let a2 = (await 3) ** (await 4) +let a3 = await foo->bar(~arg) +let a4 = await foo.bar.baz + +let b1 = await (3 + 4) +let b2 = await (3 ** 4) +let b3 = await foo->bar(~arg) +let b4 = await foo.bar.baz From cf914257654998bd0fb62d42b63aa398808ab984 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Mon, 31 Oct 2022 06:19:25 +0100 Subject: [PATCH 2/5] cleanup --- src/res_parens.ml | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/res_parens.ml b/src/res_parens.ml index 0bbe96f3..355418b4 100644 --- a/src/res_parens.ml +++ b/src/res_parens.ml @@ -175,6 +175,10 @@ let flattenOperandRhs parentOperator rhs = | _ when ParsetreeViewer.isTernaryExpr rhs -> true | _ -> false +let binaryOperatorInsideAwaitNeedsParens operator = + ParsetreeViewer.operatorPrecedence operator + < ParsetreeViewer.operatorPrecedence "|." + let lazyOrAssertOrAwaitExprRhs ?(inAwait = false) expr = let optBraces, _ = ParsetreeViewer.processBracesAttr expr in match optBraces with @@ -190,13 +194,10 @@ let lazyOrAssertOrAwaitExprRhs ?(inAwait = false) expr = pexp_desc = Pexp_apply ({pexp_desc = Pexp_ident {txt = Longident.Lident operator}}, _); } - when inAwait - && ParsetreeViewer.isBinaryExpression expr - && ParsetreeViewer.hasAwaitAttribute expr.pexp_attributes - && ParsetreeViewer.operatorPrecedence operator - >= ParsetreeViewer.operatorPrecedence "|." -> - Nothing - | expr when ParsetreeViewer.isBinaryExpression expr -> Parenthesized + when ParsetreeViewer.isBinaryExpression expr -> + if inAwait && not (binaryOperatorInsideAwaitNeedsParens operator) then + Nothing + else Parenthesized | { pexp_desc = Pexp_constraint ({pexp_desc = Pexp_pack _}, {ptyp_desc = Ptyp_package _}); From 69c0416d046d21bf4c732301ddd5840b166ae7ea Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Mon, 31 Oct 2022 06:22:38 +0100 Subject: [PATCH 3/5] Handle binary operator inside await consistently. --- src/res_parens.mli | 1 + src/res_printer.ml | 7 +++++-- tests/printer/expr/expected/asyncAwait.res.txt | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/res_parens.mli b/src/res_parens.mli index dbbfffe1..6f705432 100644 --- a/src/res_parens.mli +++ b/src/res_parens.mli @@ -10,6 +10,7 @@ val subBinaryExprOperand : string -> string -> bool val rhsBinaryExprOperand : string -> Parsetree.expression -> bool val flattenOperandRhs : string -> Parsetree.expression -> bool +val binaryOperatorInsideAwaitNeedsParens : string -> bool val lazyOrAssertOrAwaitExprRhs : ?inAwait:bool -> Parsetree.expression -> kind val fieldExpr : Parsetree.expression -> kind diff --git a/src/res_printer.ml b/src/res_printer.ml index 600d0be6..9925568a 100644 --- a/src/res_printer.ml +++ b/src/res_printer.ml @@ -3612,15 +3612,18 @@ and printBinaryExpression ~customLayout (expr : Parsetree.expression) cmtTbl = in let doc = if isAwait then + let parens = + Res_parens.binaryOperatorInsideAwaitNeedsParens operator + in Doc.concat [ Doc.lparen; Doc.text "await "; - Doc.lparen; + (if parens then Doc.lparen else Doc.nil); leftPrinted; printBinaryOperator ~inlineRhs:false operator; rightPrinted; - Doc.rparen; + (if parens then Doc.rparen else Doc.nil); Doc.rparen; ] else diff --git a/tests/printer/expr/expected/asyncAwait.res.txt b/tests/printer/expr/expected/asyncAwait.res.txt index 208b60b8..89faaa23 100644 --- a/tests/printer/expr/expected/asyncAwait.res.txt +++ b/tests/printer/expr/expected/asyncAwait.res.txt @@ -119,7 +119,7 @@ let f11 = (. ~x) => (. ~y) => 3 let f12 = @a x => 3 let f13 = (@a @b ~x) => 3 -let aw = (await (server->start))->foo +let aw = (await server->start)->foo let aw = @foo (server->start)->foo let foo = async(~a=34) From 60f450826a348d54302b24f42c3ab1ae4af9c909 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Mon, 31 Oct 2022 06:24:42 +0100 Subject: [PATCH 4/5] More tests. --- tests/printer/expr/asyncAwait.res | 1 + tests/printer/expr/expected/asyncAwait.res.txt | 1 + 2 files changed, 2 insertions(+) diff --git a/tests/printer/expr/asyncAwait.res b/tests/printer/expr/asyncAwait.res index 94349275..a410e5ef 100644 --- a/tests/printer/expr/asyncAwait.res +++ b/tests/printer/expr/asyncAwait.res @@ -99,6 +99,7 @@ let f13 = @a @b (~x) => 3 let aw = (await (server->start))->foo let aw = (@foo (server->start))->foo +let aw = (await (3**4))->foo let foo = async(~a=34) let bar = async(~a)=>a+1 diff --git a/tests/printer/expr/expected/asyncAwait.res.txt b/tests/printer/expr/expected/asyncAwait.res.txt index 89faaa23..e9f2b82c 100644 --- a/tests/printer/expr/expected/asyncAwait.res.txt +++ b/tests/printer/expr/expected/asyncAwait.res.txt @@ -121,6 +121,7 @@ let f13 = (@a @b ~x) => 3 let aw = (await server->start)->foo let aw = @foo (server->start)->foo +let aw = (await (3 ** 4))->foo let foo = async(~a=34) let bar = async (~a) => a + 1 From 7e1343ab0945d43f313d76f740040ffcf7122ab5 Mon Sep 17 00:00:00 2001 From: Cristiano Calcagno Date: Mon, 31 Oct 2022 06:26:47 +0100 Subject: [PATCH 5/5] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 064b2d34..5d7f0a10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,7 +50,7 @@ - Fix issue where the JSX key type is not an optional string https://github.com/rescript-lang/syntax/pull/693 - Fix issue where the JSX fragment without children build error https://github.com/rescript-lang/syntax/pull/704 - Fix issue where async as an id cannot be used with application and labelled arguments https://github.com/rescript-lang/syntax/issues/707 - +- Treat await as almost-unary operator weaker than pipe so `await foo->bar` means `await (foo->bar)` https://github.com/rescript-lang/syntax/pull/711 #### :eyeglasses: Spec Compliance