Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the precedence of fast pipe in the presence of function applic… #2078

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions formatTest/unit_tests/expected_output/fastPipe.re
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ bar->f->g->h;
compilation
->Plugin.buildAssets
->Js.Json.stringify
->Node.Fs.writeFileAsUtf8Sync(_, path);
->Node.Fs.writeFileAsUtf8Sync(
_,
path,
);

foo->bar->baz >>= monadicFunction |> bind;

Expand Down Expand Up @@ -53,12 +56,12 @@ event->(target##value[0]);

event->target(foo);

event->target(foo);

(event->target)(foo);
event->(target(foo));

event->target(foo);

event->(target(foo));

foo->bar := baz;

foo->bar === baz;
Expand Down
2 changes: 1 addition & 1 deletion src/reason-parser/reason_parser.mly
Original file line number Diff line number Diff line change
Expand Up @@ -3105,7 +3105,7 @@ parenthesized_expr:
| E as_loc(SHARPEQUAL) simple_expr
{ let op = { $2 with txt = "#=" } in
mkinfixop $1 (mkoperator op) $3 }
| E as_loc(MINUSGREATER) simple_expr
| E as_loc(MINUSGREATER) simple_expr_no_call
{ mkinfixop $1 (mkoperator {$2 with txt = "|."}) $3 }
| as_loc(mod_longident) DOT LPAREN MODULE module_expr COLON package_type RPAREN
{ let loc = mklocation $symbolstartpos $endpos in
Expand Down
9 changes: 4 additions & 5 deletions src/reason-parser/reason_pprint_ast.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3542,10 +3542,8 @@ let printer = object(self:'self)
let funApplExpr = formatAttachmentApplication applicationFinalWrapping None (itms, Some reducesAfterRight.pexp_loc)
in
(* Little hack: need to print parens for the `bar` application in e.g.
`foo->other##(bar(baz))`. The exception to this is the fast pipe
operator, which binds tighter to function application. *)
if higherPrecedenceThan withPrecedence (Custom "prec_functionAppl") &&
withPrecedence <> (Token fastPipeToken)
`foo->other##(bar(baz))` or `foo->other->(bar(baz))`. *)
if higherPrecedenceThan withPrecedence (Custom "prec_functionAppl")
then LayoutNode (formatPrecedence ~loc:reducesAfterRight.pexp_loc funApplExpr)
else LayoutNode funApplExpr
| PotentiallyLowPrecedence itm -> LayoutNode (formatPrecedence ~loc:reducesAfterRight.pexp_loc itm)
Expand Down Expand Up @@ -7195,6 +7193,7 @@ let printer = object(self:'self)
makeTup ~uncurried (List.map self#label_x_expression_param params)

method formatFunAppl ~jsxAttrs ~args ~funExpr ~applicationExpr ?(uncurried=false) () =
let funExpr = Reason_ast.processFastPipe funExpr in
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to processFastPipe of funExpr because we only processed applicationExpr in unparseExprRecurse and it may have bailed (e.g. if applicationExpr is an underscore application pattern).

let uncurriedApplication = uncurried in
(* If there was a JSX attribute BUT JSX component wasn't detected,
that JSX attribute needs to be pretty printed so it doesn't get
Expand All @@ -7213,7 +7212,7 @@ let printer = object(self:'self)
let formattedFunExpr = match funExpr.pexp_desc with
(* fast pipe chain or sharpop chain as funExpr, no parens needed, we know how to parse *)
| Pexp_apply ({pexp_desc = Pexp_ident {txt = Lident s}}, _)
when requireNoSpaceFor s && s <> fastPipeToken ->
when requireNoSpaceFor s ->
self#unparseExpr funExpr
| _ -> self#simplifyUnparseExpr funExpr
in
Expand Down