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

Smart linebreak printer for pipe chains #6411

Merged
merged 4 commits into from
Sep 27, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
- A little performance improvement for JSX V4 runtime helper by removing one object allocation for components with key prop. https://github.com/rescript-lang/rescript-compiler/pull/6376
- The error message for "toplevel expressions should evaluate to unit" has been revamped and improved. https://github.com/rescript-lang/rescript-compiler/pull/6407
- Improve "Somewhere wanted" error messages by changing wording and adding more context + suggested solutions to the error messages where appropriate. https://github.com/rescript-lang/rescript-compiler/pull/6410
- Add smart printer for pipe-chains. https://github.com/rescript-lang/rescript-compiler/pull/6411

# 11.0.0-rc.3

Expand Down
4 changes: 2 additions & 2 deletions jscomp/gentype/TranslateStructure.ml
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@ let rec addAnnotationsToTypes_ ~config ~(expr : Typedtree.expression)
let aName =
if aName = "*opt*" then
match arg_label with
| Optional l ->
l
| Optional l -> l
| _ -> "" (* should not happen *)
else aName
in
Expand Down Expand Up @@ -56,6 +55,7 @@ and addAnnotationsToFields ~config (expr : Typedtree.expression)
in
({field with nameJS = name} :: nextFields1, types1)
| _ -> (fields, argTypes)
[@@live]

(** Recover from expr the renaming annotations on named arguments. *)
let addAnnotationsToFunctionType ~config (expr : Typedtree.expression)
Expand Down
46 changes: 32 additions & 14 deletions jscomp/syntax/src/res_printer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -3527,8 +3527,8 @@ and printBinaryExpression ~state (expr : Parsetree.expression) cmtTbl =
Doc.concat
[spacingBeforeOperator; Doc.text operatorTxt; spacingAfterOperator]
in
let printOperand ~isLhs expr parentOperator =
let rec flatten ~isLhs expr parentOperator =
let printOperand ~isLhs ~isMultiline expr parentOperator =
let rec flatten ~isLhs ~isMultiline expr parentOperator =
if ParsetreeViewer.isBinaryExpression expr then
match expr with
| {
Expand All @@ -3541,7 +3541,7 @@ and printBinaryExpression ~state (expr : Parsetree.expression) cmtTbl =
ParsetreeViewer.flattenableOperators parentOperator operator
&& not (ParsetreeViewer.hasAttributes expr.pexp_attributes)
then
let leftPrinted = flatten ~isLhs:true left operator in
let leftPrinted = flatten ~isLhs:true ~isMultiline left operator in
let rightPrinted =
let rightPrinteableAttrs, rightInternalAttrs =
ParsetreeViewer.partitionPrintableAttributes
Expand Down Expand Up @@ -3585,12 +3585,26 @@ and printBinaryExpression ~state (expr : Parsetree.expression) cmtTbl =
Doc.rparen;
]
else
Doc.concat
[
leftPrinted;
printBinaryOperator ~inlineRhs:false operator;
rightPrinted;
]
match operator with
| ("|." | "|.u") when isMultiline ->
(* If the pipe-chain is written over multiple lines, break automatically
* `let x = a->b->c -> same line, break when line-width exceeded
* `let x = a->
* b->c` -> pipe-chain is written on multiple lines, break the group *)
Doc.breakableGroup ~forceBreak:true
(Doc.concat
[
leftPrinted;
printBinaryOperator ~inlineRhs:false operator;
rightPrinted;
])
| _ ->
Doc.concat
[
leftPrinted;
printBinaryOperator ~inlineRhs:false operator;
rightPrinted;
]
in

let doc =
Expand Down Expand Up @@ -3665,7 +3679,7 @@ and printBinaryExpression ~state (expr : Parsetree.expression) cmtTbl =
| Braced braces -> printBraces doc expr braces
| Nothing -> doc)
in
flatten ~isLhs expr parentOperator
flatten ~isLhs ~isMultiline expr parentOperator
in
match expr.pexp_desc with
| Pexp_apply
Expand All @@ -3679,8 +3693,8 @@ and printBinaryExpression ~state (expr : Parsetree.expression) cmtTbl =
|| ParsetreeViewer.isBinaryExpression rhs
|| printAttributes ~state expr.pexp_attributes cmtTbl <> Doc.nil) ->
let lhsHasCommentBelow = hasCommentBelow cmtTbl lhs.pexp_loc in
let lhsDoc = printOperand ~isLhs:true lhs op in
let rhsDoc = printOperand ~isLhs:false rhs op in
let lhsDoc = printOperand ~isLhs:true ~isMultiline:false lhs op in
let rhsDoc = printOperand ~isLhs:false ~isMultiline:false rhs op in
Doc.group
(Doc.concat
[
Expand All @@ -3697,12 +3711,16 @@ and printBinaryExpression ~state (expr : Parsetree.expression) cmtTbl =
| Pexp_apply
( {pexp_desc = Pexp_ident {txt = Longident.Lident operator}},
[(Nolabel, lhs); (Nolabel, rhs)] ) ->
let isMultiline =
lhs.pexp_loc.loc_start.pos_lnum < rhs.pexp_loc.loc_start.pos_lnum
in

let right =
let operatorWithRhs =
let rhsDoc =
printOperand
~isLhs:(ParsetreeViewer.isRhsBinaryOperator operator)
rhs operator
~isMultiline rhs operator
in
Doc.concat
[
Expand All @@ -3722,7 +3740,7 @@ and printBinaryExpression ~state (expr : Parsetree.expression) cmtTbl =
[
printOperand
~isLhs:(not @@ ParsetreeViewer.isRhsBinaryOperator operator)
lhs operator;
~isMultiline lhs operator;
right;
])
in
Expand Down
9 changes: 6 additions & 3 deletions jscomp/syntax/tests/printer/expr/expected/callback.res.txt
Original file line number Diff line number Diff line change
Expand Up @@ -215,13 +215,16 @@ let f = () => {
</div>
}

myPromise->Js.Promise.then_(value => {
myPromise
->Js.Promise.then_(value => {
fhammerschmidt marked this conversation as resolved.
Show resolved Hide resolved
Js.log(value)
Js.Promise.resolve(value + 2)
}, _)->Js.Promise.then_(value => {
}, _)
->Js.Promise.then_(value => {
Js.log(value)
Js.Promise.resolve(value + 3)
}, _)->Js.Promise.catch(err => {
}, _)
->Js.Promise.catch(err => {
Js.log2("Failure!!", err)
Js.Promise.resolve(-2)
}, _)
Expand Down
33 changes: 33 additions & 0 deletions jscomp/syntax/tests/printer/expr/expected/smartPipe.res.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
let myFunc = (strA, strB) => strA ++ strB

"expr1"->myFunc("expr2")->myFunc("expr3")

"expr1"
->myFunc("expr2")
->myFunc("expr3")

"expr1"
->myFunc("expr2")
->myFunc("expr3")

"expr1"
->myFunc("expr2")
->myFunc("expr3")
->myFunc("expr4")

myPromise->Promise.then(v => {
Js.log(v)
Promise.resolve(v)
})

myPromise->Promise.then(v => {
Js.log(v)
Promise.resolve(v)
})

let messages = React.useMemo(() =>
messagesById
->StringMap.valuesToArray
->Array.filter(ChatMessage.isVisibleInSimpleFilter)
->Array.toSorted(ChatMessage.compareByDateAsc)
, [messagesById])
31 changes: 31 additions & 0 deletions jscomp/syntax/tests/printer/expr/smartPipe.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
let myFunc = (strA, strB) => strA ++ strB

"expr1"->myFunc("expr2")->myFunc("expr3")

"expr1"
->myFunc("expr2")->myFunc("expr3")

"expr1"->myFunc("expr2")
->myFunc("expr3")

"expr1"->myFunc("expr2")
->myFunc("expr3")->myFunc("expr4")

myPromise->Promise.then(v => {
Js.log(v)
Promise.resolve(v)
})

myPromise
->Promise.then(v => {
Js.log(v)
Promise.resolve(v)
})

let messages = React.useMemo(() =>
messagesById
->StringMap.valuesToArray
->Array.filter(ChatMessage.isVisibleInSimpleFilter)
->Array.toSorted(ChatMessage.compareByDateAsc)
, [messagesById])