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 bbf3cd26..2a26adb1 100644 --- a/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt +++ b/pkl-formatter/src/main/kotlin/org/pkl/formatter/Builder.kt @@ -614,7 +614,7 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe 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 trailingNode = if (endsWithClosingCurlyBrace(lastParam.last())) Empty else line() val lastNodes = formatGenericWithGen(lastParam, sep, null) if (normalParams.isEmpty()) { group(Group(newId(), lastNodes), trailingNode) @@ -639,42 +639,43 @@ internal class Builder(sourceText: String, private val grammarVersion: GrammarVe return false } - private tailrec fun endsWithClosingBracket(node: Node): Boolean { + private tailrec fun endsWithClosingCurlyBrace(node: Node): Boolean { return if (node.children.isNotEmpty()) { - endsWithClosingBracket(node.children.last()) + endsWithClosingCurlyBrace(node.children.last()) } else { - node.isTerminal("}") || node.type.isAffix + node.isTerminal("}") } } /** - * Only considered trailing lamdba if there is only one lambda/new expr/amends expr in the list. + * Tells if an argument list has a trailing lambda, new expr, or amends expr. * - * E.g. avoid formatting `toMap()` weirdly: - * ``` - * foo.toMap( - * (it) -> makeSomeKey(it), - * (it) -> makeSomeValue(it), - * ) - * ``` + * Only considered trailing lamdba if: + * 1. There is only one lambda/new expr/amends expr in the list. E.g. avoid formatting `toMap()` + * weirdly: ``` foo.toMap( (it) -> makeSomeKey(it), (it) -> makeSomeValue(it), ) ``` + * 2. The lambda does not have leading or trailing line comment. */ private fun hasTrailingLambda(argList: Node): Boolean { val children = argList.firstProperChild()?.children ?: return false - var seenArg = false - var ret = false + var seenLambda = false + if (children.last().type == NodeType.LINE_COMMENT) { + return false + } for (i in children.lastIndex downTo 0) { val child = children[i] if (!child.isProper()) continue - if (child.type in SAME_LINE_EXPRS) { - if (seenArg) { + if (!seenLambda) { + if (child.type !in SAME_LINE_EXPRS) { return false - } else { - seenArg = true - ret = true } + // preceded by line comment + if (children.getOrNull(i - 1)?.type == NodeType.LINE_COMMENT) return false + seenLambda = true + } else if (child.type in SAME_LINE_EXPRS) { + return false } } - return ret + return true } private fun pairArguments(nodes: List): List { diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/input/method-call-trailing-lambdas.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/input/method-call-trailing-lambdas.pkl index 8da81f2a..b2ba01f4 100644 --- a/pkl-formatter/src/test/files/FormatterSnippetTests/input/method-call-trailing-lambdas.pkl +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/input/method-call-trailing-lambdas.pkl @@ -19,3 +19,27 @@ res4 = foo.foldLeft( res5 = ifNonNull((it) -> someFunctionCall(it as SomeReaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaalyLongTypeName)) res6 = ifNonNull((it) -> someFunctionCall(it as SomeReaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaalyLongTypeNameLonger)) + +res7 = + qux(bar, (a) -> + hello // some comment + ) + +res8 = qux( + // some comment + (a) -> bar +) + +res9 = + qux(bar, new { + bar = 1 + } // some comment + ) + +res10 = (a) -> hello // some comment + +res11 = + qux(bar, (a) -> + hello + // some comment + ) diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/output/line-breaks.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/output/line-breaks.pkl index cfce478f..656f9776 100644 --- a/pkl-formatter/src/test/files/FormatterSnippetTests/output/line-breaks.pkl +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/output/line-breaks.pkl @@ -76,8 +76,9 @@ local function render(currentIndent: String) = items: List = allItems - .filter((item) -> - badItems.containsKey(item) // some line comment - || item.tags.toList().findOrNull((it) -> it.type == "bookmark") != null // some other line comment + .filter( + (item) -> + badItems.containsKey(item) // some line comment + || item.tags.toList().findOrNull((it) -> it.type == "bookmark") != null // some other line comment ) .sortBy((item) -> item.name) diff --git a/pkl-formatter/src/test/files/FormatterSnippetTests/output/method-call-trailing-lambdas.pkl b/pkl-formatter/src/test/files/FormatterSnippetTests/output/method-call-trailing-lambdas.pkl index 376c97d3..91a283c0 100644 --- a/pkl-formatter/src/test/files/FormatterSnippetTests/output/method-call-trailing-lambdas.pkl +++ b/pkl-formatter/src/test/files/FormatterSnippetTests/output/method-call-trailing-lambdas.pkl @@ -25,3 +25,32 @@ res6 = ifNonNull((it) -> someFunctionCall(it as SomeReaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaalyLongTypeNameLonger) ) + +res7 = + qux( + bar, + (a) -> hello // some comment + ) + +res8 = + qux( + // some comment + (a) -> bar, + ) + +res9 = + qux( + bar, + new { + bar = 1 + } // some comment + ) + +res10 = (a) -> hello // some comment + +res11 = + qux( + bar, + (a) -> hello + // some comment + ) diff --git a/pkl-parser/src/main/java/org/pkl/parser/GenericParser.java b/pkl-parser/src/main/java/org/pkl/parser/GenericParser.java index c8457cda..ff6c74f2 100644 --- a/pkl-parser/src/main/java/org/pkl/parser/GenericParser.java +++ b/pkl-parser/src/main/java/org/pkl/parser/GenericParser.java @@ -77,8 +77,8 @@ public class GenericParser { } var lastImport = parseImportDecl(); imports.add(lastImport); - // keep trailling affixes as part of the import - while (lookahead.isAffix() && lastImport.span.sameLine(spanLookahead)) { + // keep trailing affixes as part of the import + while (lookahead.isAffix() && lastImport.span.isSameLine(spanLookahead)) { imports.add(makeAffix(next())); } if (!isImport()) break; @@ -662,7 +662,7 @@ public class GenericParser { if (operator.getPrec() < minPrecedence) break; // `-` and `[]` must be in the same line as the left operand and have no semicolons inbetween if ((operator == Operator.MINUS || operator == Operator.SUBSCRIPT) - && (fullOpToken.hasSemicolon || !expr.span.sameLine(fullOpToken.tk.span))) break; + && (fullOpToken.hasSemicolon || !expr.span.isSameLine(fullOpToken.tk.span))) break; var children = new ArrayList(); children.add(expr); ff(children); diff --git a/pkl-parser/src/main/java/org/pkl/parser/syntax/generic/FullSpan.java b/pkl-parser/src/main/java/org/pkl/parser/syntax/generic/FullSpan.java index 319ba1f2..f53f1774 100644 --- a/pkl-parser/src/main/java/org/pkl/parser/syntax/generic/FullSpan.java +++ b/pkl-parser/src/main/java/org/pkl/parser/syntax/generic/FullSpan.java @@ -15,8 +15,6 @@ */ package org.pkl.parser.syntax.generic; -import org.pkl.parser.Span; - public record FullSpan( int charIndex, int length, int lineBegin, int colBegin, int lineEnd, int colEnd) { @@ -30,11 +28,7 @@ public record FullSpan( end.colEnd); } - public Span toSpan() { - return new Span(charIndex, length); - } - - public boolean sameLine(FullSpan other) { + public boolean isSameLine(FullSpan other) { return lineEnd == other.lineBegin; }