diff --git a/docs/modules/release-notes/pages/0.30.adoc b/docs/modules/release-notes/pages/0.30.adoc index 1a5cabb4..be1c8e04 100644 --- a/docs/modules/release-notes/pages/0.30.adoc +++ b/docs/modules/release-notes/pages/0.30.adoc @@ -25,7 +25,7 @@ To get started, follow xref:pkl-cli:index.adoc#installation[Installation].# [[formatter]] === Formatter -Pkl now has a formatter (https://github.com/apple/pkl/pull/1107[#1107], https://github.com/apple/pkl/pull/1208[#1208], https://github.com/apple/pkl/pull/1211[#1211], https://github.com/apple/pkl/pull/1215[#1215], https://github.com/apple/pkl/pull/1217[#1217], https://github.com/apple/pkl/pull/1235[#1235], https://github.com/apple/pkl/pull/1246[#1246], https://github.com/apple/pkl/pull/1247[#1247], https://github.com/apple/pkl/pull/1252[#1252], https://github.com/apple/pkl/pull/1256[#1256], https://github.com/apple/pkl/pull/1259[#1259], https://github.com/apple/pkl/pull/1260[#1260], https://github.com/apple/pkl/pull/1263[#1263], https://github.com/apple/pkl/pull/1265[#1265], https://github.com/apple/pkl/pull/1266[#1266], https://github.com/apple/pkl/pull/1267[#1267], https://github.com/apple/pkl/pull/1270[#1270], https://github.com/apple/pkl/pull/1271[#1271], https://github.com/apple/pkl/pull/1272[#1272], https://github.com/apple/pkl/pull/1273[#1273], https://github.com/apple/pkl/pull/1274[#1274], https://github.com/apple/pkl/pull/1280[#1280])! +Pkl now has a formatter (https://github.com/apple/pkl/pull/1107[#1107], https://github.com/apple/pkl/pull/1208[#1208], https://github.com/apple/pkl/pull/1211[#1211], https://github.com/apple/pkl/pull/1215[#1215], https://github.com/apple/pkl/pull/1217[#1217], https://github.com/apple/pkl/pull/1235[#1235], https://github.com/apple/pkl/pull/1246[#1246], https://github.com/apple/pkl/pull/1247[#1247], https://github.com/apple/pkl/pull/1252[#1252], https://github.com/apple/pkl/pull/1256[#1256], https://github.com/apple/pkl/pull/1259[#1259], https://github.com/apple/pkl/pull/1260[#1260], https://github.com/apple/pkl/pull/1263[#1263], https://github.com/apple/pkl/pull/1265[#1265], https://github.com/apple/pkl/pull/1266[#1266], https://github.com/apple/pkl/pull/1267[#1267], https://github.com/apple/pkl/pull/1268[#1268], https://github.com/apple/pkl/pull/1270[#1270], https://github.com/apple/pkl/pull/1271[#1271], https://github.com/apple/pkl/pull/1272[#1272], https://github.com/apple/pkl/pull/1273[#1273], https://github.com/apple/pkl/pull/1274[#1274], https://github.com/apple/pkl/pull/1280[#1280])! Pkl's formatter is _canonical_, which means that it has a single set of formatting rules, with (almost) no configuration options. The goal is to eliminate the possibility of formatting debates, which can lead to churn and bike-shedding. diff --git a/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt b/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt index fa5b3216..bbf3cd26 100644 --- a/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt +++ b/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt @@ -299,7 +299,15 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe gatherFacts(node) - val separator: (Node, Node) -> FormatNode? = { prev, next -> + val leadingSeparator: (Node, Node) -> FormatNode? = { prev, next -> + when { + prev.type == NodeType.OPERATOR -> null + next.type == NodeType.OPERATOR -> line() + else -> spaceOrLine() + } + } + + val trailingSeparator: (Node, Node) -> FormatNode? = { prev, next -> when { prev.type == NodeType.OPERATOR -> null next.type == NodeType.OPERATOR -> if (lambdaCount > 1) forceLine() else line() @@ -312,8 +320,9 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe methodCallCount == 1 && isMethodCall(flat.lastProperNode()!!) -> { // lift argument list into its own node val (callChain, argsList) = splitFunctionCallNode(flat) - val leadingNodes = indentAfterFirstNewline(formatGeneric(callChain, separator), true) - val trailingNodes = formatGeneric(argsList, separator) + val leadingNodes = + indentAfterFirstNewline(formatGeneric(callChain, leadingSeparator), true) + val trailingNodes = formatGeneric(argsList, trailingSeparator) val sep = getBaseSeparator(callChain.last(), argsList.first()) if (sep != null) { leadingNodes + sep + trailingNodes @@ -324,11 +333,11 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe methodCallCount > 0 && indexBeforeFirstMethodCall > 0 -> { val leading = flat.subList(0, indexBeforeFirstMethodCall) val trailing = flat.subList(indexBeforeFirstMethodCall, flat.size) - val leadingNodes = indentAfterFirstNewline(formatGeneric(leading, separator), true) - val trailingNodes = formatGeneric(trailing, separator) + val leadingNodes = indentAfterFirstNewline(formatGeneric(leading, leadingSeparator), true) + val trailingNodes = formatGeneric(trailing, trailingSeparator) leadingNodes + line() + trailingNodes } - else -> formatGeneric(flat, separator) + else -> formatGeneric(flat, trailingSeparator) } val shouldGroup = node.children.size == flat.size @@ -585,10 +594,14 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe twoBy2: Boolean = false, ): FormatNode { val children = node.children + val shouldMultiline = shouldMultlineNodes(node) { it.isTerminal(",") } + val sep: (Node, Node) -> FormatNode = { _, _ -> + if (shouldMultiline) forceSpaceyLine() else spaceOrLine() + } return if (twoBy2) { val pairs = pairArguments(children) val nodes = - formatGenericWithGen(pairs, spaceOrLine()) { node, _ -> + formatGenericWithGen(pairs, sep) { node, _ -> if (node.type == NodeType.ARGUMENT_LIST_ELEMENTS) { Group(newId(), formatGeneric(node.children, spaceOrLine())) } else { @@ -597,24 +610,35 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe } Indent(nodes) } else if (hasTrailingLambda) { - // if the args have a trailing expression (lambda, new, amends) group them differently + // if the args have a trailing lambda, group them differently val splitIndex = children.indexOfLast { it.type in SAME_LINE_EXPRS } val normalParams = children.subList(0, splitIndex) val lastParam = children.subList(splitIndex, children.size) val trailingNode = if (endsWithClosingBracket(lastParam.last())) Empty else line() - val lastNodes = formatGeneric(lastParam, spaceOrLine()) + val lastNodes = formatGenericWithGen(lastParam, sep, null) if (normalParams.isEmpty()) { group(Group(newId(), lastNodes), trailingNode) } else { val separator = getSeparator(normalParams.last(), lastParam[0], Space) - val paramNodes = formatGeneric(normalParams, spaceOrLine()) + val paramNodes = formatGenericWithGen(normalParams, sep, null) group(Group(newId(), paramNodes), separator, Group(newId(), lastNodes), trailingNode) } } else { - Indent(formatGeneric(children, spaceOrLine())) + Indent(formatGeneric(children, sep)) } } + private fun shouldMultlineNodes(node: Node, predicate: (Node) -> Boolean): Boolean { + for (idx in 0.. 0) { + return true + } + } + return false + } + private tailrec fun endsWithClosingBracket(node: Node): Boolean { return if (node.children.isNotEmpty()) { endsWithClosingBracket(node.children.last()) @@ -998,19 +1022,21 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe private fun formatBinaryOpExpr(node: Node): FormatNode { val flat = flattenBinaryOperatorExprs(node) + val shouldMultiline = shouldMultlineNodes(node) { it.type == NodeType.OPERATOR } val nodes = formatGeneric(flat) { prev, next -> + val sep = if (shouldMultiline) forceSpaceyLine() else spaceOrLine() if (prev.type == NodeType.OPERATOR) { when (prev.text()) { - "-" -> spaceOrLine() + "-" -> sep else -> Space } } else if (next.type == NodeType.OPERATOR) { when (next.text()) { "-" -> Space - else -> spaceOrLine() + else -> sep } - } else spaceOrLine() + } else sep } val shouldGroup = node.children.size == flat.size @@ -1396,15 +1422,15 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe private fun flattenBinaryOperatorExprs(node: Node, prec: Int): List { val actualOp = node.children.first { it.type == NodeType.OPERATOR }.text() if (prec != Operator.byName(actualOp).prec) return listOf(node) - val res = mutableListOf() - for (child in node.children) { - if (child.type == NodeType.BINARY_OP_EXPR) { - res += flattenBinaryOperatorExprs(child, prec) - } else { - res += child + return buildList { + for (child in node.children) { + if (child.type == NodeType.BINARY_OP_EXPR) { + addAll(flattenBinaryOperatorExprs(child, prec)) + } else { + add(child) + } } } - return res } private fun Node.linesBetween(next: Node): Int = next.span.lineBegin - span.lineEnd diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/input/expr-chain-grouping.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/input/expr-chain-grouping.pkl index 008a922d..74a6496e 100644 --- a/pkl-formatter/src/test/files/FormatterSnippetTests/input/expr-chain-grouping.pkl +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/input/expr-chain-grouping.pkl @@ -29,3 +29,6 @@ res10 = foo.bar /* some comment */ (5) res11 = foo.bar /* some comment */.baz(new { foooooooooooooooooooooooooo = 1; baaaaaaaaaaaaaaaaaaaaar = 2 }).buz +res12 = foo.biz + .bar((it) -> it + 2) + .qux((it) -> it + 1) diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/input/line-breaks3.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/input/line-breaks3.pkl new file mode 100644 index 00000000..729d961a --- /dev/null +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/input/line-breaks3.pkl @@ -0,0 +1,58 @@ +// respect line breaks on: +// * operator chains +// * argument lists in method calls +// +// only look for line breaks on either side of the operator, or the comma. +res1 = + foo + * bar + * baz + +res2 = foo + |> bar + bar + |> baz + +res3 = foo + |> bar + + baz + |> qux + +res4 = foo( + (it) -> it + 1, + (it) -> it + 2 +) + +res5 = foo( + 1, + 2, 3, 4 +) + +// don't respect newlines right after opening paren and closing paren +res6 = foo.bar( + 1, 2, 3 +) + +// only respect newlines on either side of a binary operator (we still want to inline `bar(3)` here). +res7 = foo + bar( + 3 + ) + baz + +// only the newlines after each "entry" are respected +res8 = Map( + 1, 2, + 3, 4 +) + +res9 = Map( + 1, + 2, + 3, 4 +) + +res10 = foo(1 + ,2 + ,3) + +res11 = foo + + bar + + baz diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/output/expr-chain-grouping.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/output/expr-chain-grouping.pkl index 2d95b7fc..2d16936f 100644 --- a/pkl-formatter/src/test/files/FormatterSnippetTests/output/expr-chain-grouping.pkl +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/output/expr-chain-grouping.pkl @@ -63,3 +63,8 @@ res11 = foo.bar /* some comment */ .baz(new { foooooooooooooooooooooooooo = 1; baaaaaaaaaaaaaaaaaaaaar = 2 }) .buz + +res12 = + foo.biz + .bar((it) -> it + 2) + .qux((it) -> it + 1) diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/output/line-breaks3.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/output/line-breaks3.pkl new file mode 100644 index 00000000..6810aaf2 --- /dev/null +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/output/line-breaks3.pkl @@ -0,0 +1,65 @@ +// respect line breaks on: +// * operator chains +// * argument lists in method calls +// +// only look for line breaks on either side of the operator, or the comma. +res1 = + foo + * bar + * baz + +res2 = + foo + |> bar + bar + |> baz + +res3 = + foo + |> bar + + baz + |> qux + +res4 = + foo( + (it) -> it + 1, + (it) -> it + 2, + ) + +res5 = + foo( + 1, + 2, + 3, + 4, + ) + +// don't respect newlines right after opening paren and closing paren +res6 = foo.bar(1, 2, 3) + +// only respect newlines on either side of a binary operator (we still want to inline `bar(3)` here). +res7 = foo + bar(3) + baz + +// only the newlines after each "entry" are respected +res8 = + Map( + 1, 2, + 3, 4, + ) + +res9 = + Map( + 1, 2, + 3, 4, + ) + +res10 = + foo( + 1, + 2, + 3, + ) + +res11 = + foo + + bar + + baz