Coalesce pkl format subcommands into the parent command. (#1263)

pkl-cli/src/main/kotlin/org/pkl/cli/CliFormatterApply.kt

@@ -1,59 +0,0 @@
-/*
- * Copyright © 2025 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.
- * You may obtain a copy of the License at
- *
- *     https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.pkl.cli
-
-import java.io.IOException
-import java.nio.file.Files
-import java.nio.file.Path
-import kotlin.io.path.writeText
-import org.pkl.commons.cli.CliBaseOptions
-import org.pkl.commons.cli.CliException
-import org.pkl.formatter.GrammarVersion
-
-class CliFormatterApply(
-  cliBaseOptions: CliBaseOptions,
-  paths: List<Path>,
-  grammarVersion: GrammarVersion,
-  private val silent: Boolean,
-) : CliFormatterCommand(cliBaseOptions, paths, grammarVersion) {
-
-  override fun doRun() {
-    var status = 0
-
-    for (path in paths()) {
-      val contents = Files.readString(path)
-      val (formatted, stat) = format(path, contents)
-      status = if (status == 0) stat else status
-      if (stat != 0 || contents == formatted) continue
-      if (!silent) {
-        consoleWriter.write(path.toAbsolutePath().toString())
-        consoleWriter.appendLine()
-        consoleWriter.flush()
-      }
-      try {
-        path.writeText(formatted, Charsets.UTF_8)
-      } catch (e: IOException) {
-        consoleWriter.write("Could not overwrite `$path`: ${e.message}")
-        consoleWriter.appendLine()
-        consoleWriter.flush()
-        status = 1
-      }
-    }
-    if (status != 0) {
-      throw CliException("Formatting violations found.", status)
-    }
-  }
-}

pkl-cli/src/main/kotlin/org/pkl/cli/CliFormatterCheck.kt

@@ -1,48 +0,0 @@
-/*
- * Copyright © 2025 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.
- * You may obtain a copy of the License at
- *
- *     https://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.pkl.cli
-
-import java.nio.file.Files
-import java.nio.file.Path
-import org.pkl.commons.cli.CliBaseOptions
-import org.pkl.commons.cli.CliException
-import org.pkl.formatter.GrammarVersion
-
-class CliFormatterCheck(
-  cliBaseOptions: CliBaseOptions,
-  paths: List<Path>,
-  grammarVersion: GrammarVersion,
-) : CliFormatterCommand(cliBaseOptions, paths, grammarVersion) {
-
-  override fun doRun() {
-    var status = 0
-
-    for (path in paths()) {
-      val contents = Files.readString(path)
-      val (formatted, stat) = format(path, contents)
-      status = if (status == 0) stat else status
-      if (contents != formatted) {
-        consoleWriter.write(path.toAbsolutePath().toString())
-        consoleWriter.appendLine()
-        consoleWriter.flush()
-        status = 1
-      }
-    }
-    if (status != 0) {
-      throw CliException("Formatting violations found.", status)
-    }
-  }
-}

pkl-cli/src/main/kotlin/org/pkl/cli/CliFormatterCommand.kt

@@ -15,48 +15,144 @@
  */
 package org.pkl.cli
 
+import java.io.File
+import java.io.IOException
 import java.io.Writer
+import java.nio.file.Files
 import java.nio.file.Path
-import kotlin.io.path.ExperimentalPathApi
+import java.util.stream.Stream
 import kotlin.io.path.extension
 import kotlin.io.path.isDirectory
 import kotlin.io.path.name
-import kotlin.io.path.walk
+import kotlin.io.path.writeText
+import kotlin.math.max
 import org.pkl.commons.cli.CliBaseOptions
 import org.pkl.commons.cli.CliCommand
+import org.pkl.commons.cli.CliTestException
+import org.pkl.core.ModuleSource
+import org.pkl.core.runtime.VmUtils
+import org.pkl.core.util.IoUtils
 import org.pkl.formatter.Formatter
 import org.pkl.formatter.GrammarVersion
 import org.pkl.parser.GenericParserError
 
-abstract class CliFormatterCommand
+class CliFormatterCommand
 @JvmOverloads
 constructor(
-  options: CliBaseOptions,
-  protected val paths: List<Path>,
-  protected val grammarVersion: GrammarVersion,
-  protected val consoleWriter: Writer = System.out.writer(),
-) : CliCommand(options) {
-  protected fun format(file: Path, contents: String): Pair<String, Int> {
-    try {
-      return Formatter().format(contents, grammarVersion) to 0
-    } catch (pe: GenericParserError) {
-      consoleWriter.write("Could not format `$file`: $pe")
-      consoleWriter.appendLine()
-      consoleWriter.flush()
-      return "" to 1
+  private val paths: List<Path>,
+  private val grammarVersion: GrammarVersion,
+  private val overwrite: Boolean,
+  private val diffNameOnly: Boolean,
+  private val silent: Boolean,
+  private val consoleWriter: Writer = System.out.writer(),
+  private val errWriter: Writer = System.err.writer(),
+) : CliCommand(CliBaseOptions()) {
+
+  private fun format(contents: String): String {
+    return Formatter().format(contents, grammarVersion)
+  }
+
+  private fun writeErr(error: String) {
+    errWriter.write(error)
+    errWriter.appendLine()
+    errWriter.flush()
+  }
+
+  private fun write(message: String) {
+    if (silent) return
+    consoleWriter.write(message)
+    consoleWriter.appendLine()
+    consoleWriter.flush()
+  }
+
+  private fun allSources(): Stream<ModuleSource> {
+    return paths.distinct().stream().flatMap { path ->
+      when {
+        path.toString() == "-" -> Stream.of(ModuleSource.text(IoUtils.readString(System.`in`)))
+        path.isDirectory() ->
+          Files.walk(path)
+            .filter { it.extension == "pkl" || it.name == "PklProject" }
+            .map(ModuleSource::path)
+        else -> Stream.of(ModuleSource.path(path))
+      }
     }
   }
 
-  @OptIn(ExperimentalPathApi::class)
-  protected fun paths(): Set<Path> {
-    val allPaths = mutableSetOf<Path>()
-    for (path in paths) {
-      if (path.isDirectory()) {
-        allPaths.addAll(path.walk().filter { it.extension == "pkl" || it.name == "PklProject" })
-      } else {
-        allPaths.add(path)
+  override fun doRun() {
+    val status = Status(SUCCESS)
+
+    handleSources(status)
+
+    when (status.status) {
+      FORMATTING_VIOLATION -> {
+        // using CliTestException instead of CliException because we want full control on how to
+        // print errors
+        throw CliTestException("", status.status)
+      }
+      ERROR -> {
+        if (!silent) {
+          writeErr("An error occurred during formatting.")
+        }
+        throw CliTestException("", status.status)
+      }
+    }
+  }
+
+  private fun handleSources(status: Status) {
+    for (source in allSources()) {
+      val path = if (source.uri == VmUtils.REPL_TEXT_URI) Path.of("-") else Path.of(source.uri)
+      try {
+        val contents =
+          if (source.contents != null) {
+            if (overwrite) {
+              writeErr("Cannot write to stdin.")
+              throw CliTestException("", ERROR)
+            }
+            source.contents!!
+          } else {
+            File(source.uri).readText()
+          }
+
+        val formatted = format(contents)
+        if (contents != formatted) {
+          status.update(FORMATTING_VIOLATION)
+          if (diffNameOnly || overwrite) {
+            // if `--diff-name-only` or `-w` is specified, only write file names
+            write(path.toAbsolutePath().toString())
+          }
+
+          if (overwrite) {
+            path.writeText(formatted, Charsets.UTF_8)
+          }
+        }
+
+        if (!diffNameOnly && !overwrite) {
+          write(formatted)
+        }
+      } catch (pe: GenericParserError) {
+        writeErr("Could not format `$path`: $pe")
+        status.update(ERROR)
+      } catch (e: IOException) {
+        writeErr("IO error while reading `$path`: ${e.message}")
+        status.update(ERROR)
+      }
+    }
+  }
+
+  companion object {
+    private const val SUCCESS = 0
+    private const val FORMATTING_VIOLATION = 11
+    private const val ERROR = 1
+
+    private class Status(var status: Int) {
+      fun update(newStatus: Int) {
+        status =
+          when {
+            status == ERROR -> status
+            newStatus == ERROR -> newStatus
+            else -> max(status, newStatus)
+          }
       }
     }
-    return allPaths
   }
 }

pkl-cli/src/main/kotlin/org/pkl/cli/commands/FormatterCommand.kt

@@ -15,9 +15,8 @@
  */
 package org.pkl.cli.commands
 
+import com.github.ajalt.clikt.core.CliktCommand
 import com.github.ajalt.clikt.core.Context
-import com.github.ajalt.clikt.core.NoOpCliktCommand
-import com.github.ajalt.clikt.core.subcommands
 import com.github.ajalt.clikt.parameters.arguments.argument
 import com.github.ajalt.clikt.parameters.arguments.multiple
 import com.github.ajalt.clikt.parameters.options.default
@@ -26,75 +25,75 @@ import com.github.ajalt.clikt.parameters.options.option
 import com.github.ajalt.clikt.parameters.types.enum
 import com.github.ajalt.clikt.parameters.types.path
 import java.nio.file.Path
-import org.pkl.cli.CliFormatterApply
-import org.pkl.cli.CliFormatterCheck
-import org.pkl.cli.commands.FormatterCheckCommand.Companion.grammarVersionHelp
-import org.pkl.commons.cli.commands.BaseCommand
+import org.pkl.cli.CliFormatterCommand
 import org.pkl.formatter.GrammarVersion
 
-class FormatterCommand : NoOpCliktCommand(name = "format") {
-  override fun help(context: Context) = "Run commands related to formatting"
+class FormatterCommand : CliktCommand(name = "format") {
+  override fun help(context: Context) =
+    """
+    Format or check formatting of Pkl files.
+    
+    Examples:
+    
+    ```
+    # Overwrite all Pkl files inside `my/folder/`, recursively.
+    $ pkl format -w my/folder/
+    
+    # Check formatting of all files, printing filenames with formatting violations to stdout.
+    # Exit with exit code `11` if formatting violations were found.
+    $ pkl format --diff-name-only my/folder/
+    
+    # Format Pkl code from stdin.
+    $ echo "foo = 1" | pkl format -
+    ```
+  """
+      .trimIndent()
 
   override fun helpEpilog(context: Context) = "For more information, visit $helpLink"
 
-  init {
-    subcommands(FormatterCheckCommand(), FormatterApplyCommand())
-  }
-}
-
-class FormatterCheckCommand : BaseCommand(name = "check", helpLink = helpLink) {
-  override val helpString: String =
-    "Check if the given files are properly formatted, printing the file name to stdout in case they are not. Returns non-zero in case of failure."
-
   val paths: List<Path> by
-    argument(name = "paths", help = "Files or directory to check.")
-      .path(mustExist = true, canBeDir = true)
+    argument(name = "paths", help = "Files or directory to check. Use `-` to read from stdin.")
+      .path(mustExist = false, canBeDir = true)
       .multiple()
 
   val grammarVersion: GrammarVersion by
-    option(names = arrayOf("--grammar-version"), help = grammarVersionHelp)
+    option(
+        names = arrayOf("--grammar-version"),
+        help =
+          """
+          The grammar compatibility version to use.$NEWLINE
+          ${GrammarVersion.entries.joinToString("$NEWLINE", prefix = "  ") {
+            val default = if (it == GrammarVersion.latest()) " `(default)`" else ""
+            "`${it.version}`: ${it.versionSpan}$default"
+          }}
+          """
+            .trimIndent(),
+      )
       .enum<GrammarVersion> { "${it.version}" }
       .default(GrammarVersion.latest())
 
-  override fun run() {
-    CliFormatterCheck(baseOptions.baseOptions(emptyList()), paths, grammarVersion).run()
-  }
+  val overwrite: Boolean by
+    option(
+        names = arrayOf("-w", "--write"),
+        help = "Format files in place, overwriting them. Implies `---diff-name-only`.",
+      )
+      .flag(default = false)
 
-  companion object {
-    internal val grammarVersionHelp =
-      """
-      The grammar compatibility version to use.$NEWLINE
-      ${GrammarVersion.entries.joinToString("$NEWLINE", prefix = "  ") {
-        val default = if (it == GrammarVersion.latest()) " `(default)`" else ""
-        "`${it.version}`: ${it.versionSpan}$default"
-      }}
-      """
-        .trimIndent()
-  }
-}
-
-class FormatterApplyCommand : BaseCommand(name = "apply", helpLink = helpLink) {
-  override val helpString: String =
-    "Overwrite all the files in place with the formatted version. Returns non-zero in case of failure."
-
-  val paths: List<Path> by
-    argument(name = "paths", help = "Files or directory to format.")
-      .path(mustExist = true, canBeDir = true)
-      .multiple()
+  val diffNameOnly: Boolean by
+    option(
+        names = arrayOf("--diff-name-only"),
+        help = "Write the path of files with formatting violations to stdout.",
+      )
+      .flag(default = false)
 
   val silent: Boolean by
     option(
         names = arrayOf("-s", "--silent"),
-        help = "Do not write the name of the files that failed formatting to stdout.",
+        help = "Don't write to stdout or stderr. Mutually exclusive with `--diff-name-only`.",
       )
-      .flag()
-
-  val grammarVersion: GrammarVersion by
-    option(names = arrayOf("--grammar-version"), help = grammarVersionHelp)
-      .enum<GrammarVersion> { "${it.version}" }
-      .default(GrammarVersion.latest())
+      .flag(default = false)
 
   override fun run() {
-    CliFormatterApply(baseOptions.baseOptions(emptyList()), paths, grammarVersion, silent).run()
+    CliFormatterCommand(paths, grammarVersion, overwrite, diffNameOnly, silent).run()
   }
 }