From 09435af54fd7e8802633b56a58c051045b168fc3 Mon Sep 17 00:00:00 2001 From: odenix Date: Tue, 7 Apr 2026 14:16:12 -0700 Subject: [PATCH] Improve Formatter API (#1505) - pass `GrammarVersion` to constructor instead of passing it to each `format` method - replace `format(Path): String` with `format(Reader, Appendable)` - instead of picking which overloads besides `format(String): String` might be useful, offer a single generalized method that streams input and output - add `@Throws(IOException::class)` to ensure that Java callers can catch this exception - deprecate old methods --- .../kotlin/org/pkl/cli/CliFormatterCommand.kt | 2 +- .../kotlin/org/pkl/formatter/Formatter.kt | 60 +++++++++++++++---- .../kotlin/org/pkl/formatter/Generator.kt | 9 +-- .../formatter/FormatterSnippetTestsEngine.kt | 21 +++---- .../kotlin/org/pkl/formatter/FormatterTest.kt | 10 +++- 5 files changed, 68 insertions(+), 34 deletions(-) diff --git a/pkl-cli/src/main/kotlin/org/pkl/cli/CliFormatterCommand.kt b/pkl-cli/src/main/kotlin/org/pkl/cli/CliFormatterCommand.kt index 2c5ae3c0..8fccd10a 100644 --- a/pkl-cli/src/main/kotlin/org/pkl/cli/CliFormatterCommand.kt +++ b/pkl-cli/src/main/kotlin/org/pkl/cli/CliFormatterCommand.kt @@ -47,7 +47,7 @@ constructor( private val errWriter: Writer = System.err.writer(), ) : CliCommand(CliBaseOptions()) { private fun format(contents: String): String { - return Formatter().format(contents, grammarVersion) + return Formatter(grammarVersion).format(contents) } private fun writeErrLine(error: String) { diff --git a/pkl-formatter/src/main/kotlin/org/pkl/formatter/Formatter.kt b/pkl-formatter/src/main/kotlin/org/pkl/formatter/Formatter.kt index fbe26678..663fdcb8 100644 --- a/pkl-formatter/src/main/kotlin/org/pkl/formatter/Formatter.kt +++ b/pkl-formatter/src/main/kotlin/org/pkl/formatter/Formatter.kt @@ -15,14 +15,24 @@ */ package org.pkl.formatter +import java.io.IOException +import java.io.Reader import java.nio.file.Files import java.nio.file.Path +import kotlin.jvm.Throws import org.pkl.formatter.ast.ForceLine import org.pkl.formatter.ast.Nodes import org.pkl.parser.GenericParser -/** A formatter for Pkl files that applies canonical formatting rules. */ -class Formatter { +/** + * A formatter for Pkl files that applies canonical formatting rules. + * + * @param grammarVersion grammar compatibility version + */ +class Formatter +@JvmOverloads +constructor(private val grammarVersion: GrammarVersion = GrammarVersion.latest()) { + /** * Formats a Pkl file from the given file path. * @@ -32,8 +42,9 @@ class Formatter { * @throws java.io.IOException if the file cannot be read */ @JvmOverloads + @Deprecated(message = "use format(path.readText()) instead") fun format(path: Path, grammarVersion: GrammarVersion = GrammarVersion.latest()): String { - return format(Files.readString(path), grammarVersion) + return Formatter(grammarVersion).format(Files.readString(path)) } /** @@ -43,16 +54,41 @@ class Formatter { * @param grammarVersion grammar compatibility version * @return the formatted Pkl source code as a string */ - @JvmOverloads - fun format(text: String, grammarVersion: GrammarVersion = GrammarVersion.latest()): String { - val parser = GenericParser() - val builder = Builder(text, grammarVersion) - val gen = Generator() - val ast = parser.parseModule(text) - val formatAst = builder.format(ast) + @Deprecated(message = "use Formatter(grammarVersion).format(text) instead") + fun format(text: String, grammarVersion: GrammarVersion): String { + return Formatter(grammarVersion).format(text) + } + + /** + * Formats the given Pkl source code text. + * + * @param text the Pkl source code to format + * @return the formatted Pkl source code as a string + */ + fun format(text: String): String { + return buildString { format(text, this) } + } + + /** + * Formats the given Pkl source code text. + * + * It is the caller's responsibility to close [input], and, if applicable, [output]. + * + * @param input the Pkl source code to format + * @param output the formatted Pkl source code + * @throws java.io.IOException if an I/O error occurs during reading or writing + */ + @Throws(IOException::class) + fun format(input: Reader, output: Appendable) { + format(input.readText(), output) + } + + private fun format(input: String, output: Appendable) { + val ast = GenericParser().parseModule(input) + val formatAst = Builder(input, grammarVersion).format(ast) // force a line at the end of the file - gen.generate(Nodes(listOf(formatAst, ForceLine))) - return gen.toString() + val nodes = Nodes(listOf(formatAst, ForceLine)) + Generator(output).generate(nodes) } } diff --git a/pkl-formatter/src/main/kotlin/org/pkl/formatter/Generator.kt b/pkl-formatter/src/main/kotlin/org/pkl/formatter/Generator.kt index 18d68147..1d80bc61 100644 --- a/pkl-formatter/src/main/kotlin/org/pkl/formatter/Generator.kt +++ b/pkl-formatter/src/main/kotlin/org/pkl/formatter/Generator.kt @@ -1,5 +1,5 @@ /* - * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2025-2026 Apple Inc. and the Pkl project authors. All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -29,8 +29,7 @@ import org.pkl.formatter.ast.SpaceOrLine import org.pkl.formatter.ast.Text import org.pkl.formatter.ast.Wrap -internal class Generator { - private val buf: StringBuilder = StringBuilder() +internal class Generator(private val buf: Appendable) { private var indent: Int = 0 private var size: Int = 0 private val wrapped: MutableSet = mutableSetOf() @@ -138,10 +137,6 @@ internal class Generator { return if (beforeLast is Text) beforeLast.text.length else 0 } - override fun toString(): String { - return buf.toString() - } - companion object { // max line length const val MAX = 100 diff --git a/pkl-formatter/src/test/kotlin/org/pkl/formatter/FormatterSnippetTestsEngine.kt b/pkl-formatter/src/test/kotlin/org/pkl/formatter/FormatterSnippetTestsEngine.kt index d245a0ac..5e700431 100644 --- a/pkl-formatter/src/test/kotlin/org/pkl/formatter/FormatterSnippetTestsEngine.kt +++ b/pkl-formatter/src/test/kotlin/org/pkl/formatter/FormatterSnippetTestsEngine.kt @@ -1,5 +1,5 @@ /* - * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2025-2026 Apple Inc. and the Pkl project authors. All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ package org.pkl.formatter import java.nio.file.Path import kotlin.io.path.isRegularFile +import kotlin.io.path.readText import kotlin.reflect.KClass import org.pkl.commons.test.InputOutputTestEngine import org.pkl.parser.ParserError @@ -58,16 +59,10 @@ abstract class AbstractFormatterSnippetTestsEngine : InputOutputTestEngine() { class FormatterSnippetTestsEngine : AbstractFormatterSnippetTestsEngine() { override val testClass: KClass<*> = FormatterSnippetTests::class - override fun generateOutputFor(inputFile: Path): Pair { - val formatter = Formatter() - val (success, output) = - try { - val res = formatter.format(inputFile) - true to res - } catch (_: ParserError) { - false to "" - } - - return success to output - } + override fun generateOutputFor(inputFile: Path): Pair = + try { + true to Formatter().format(inputFile.readText()) + } catch (_: ParserError) { + false to "" + } } diff --git a/pkl-formatter/src/test/kotlin/org/pkl/formatter/FormatterTest.kt b/pkl-formatter/src/test/kotlin/org/pkl/formatter/FormatterTest.kt index bbad5981..2bbc9c15 100644 --- a/pkl-formatter/src/test/kotlin/org/pkl/formatter/FormatterTest.kt +++ b/pkl-formatter/src/test/kotlin/org/pkl/formatter/FormatterTest.kt @@ -1,5 +1,5 @@ /* - * Copyright © 2025 Apple Inc. and the Pkl project authors. All rights reserved. + * Copyright © 2025-2026 Apple Inc. and the Pkl project authors. All rights reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -120,4 +120,12 @@ class FormatterTest { assertThat(format(src)).isEqualTo("\n") } } + + @Test + fun `read from Reader and write to Appendable`() { + val input = " x = 42".reader() + val output = StringBuilder() + Formatter().format(input, output) + assertThat(output.toString()).isEqualTo("x = 42\n") + } }