Make commands classes instead of objects (#946)

Making these classes caused native-image to statically initialize
them at build time, which included CLI argument default values
(like working dir).

This turns them back into classes.

Co-authored-by: Islon Scherer <islonscherer@gmail.com>
This commit is contained in:
Daniel Chao
2025-02-11 06:38:19 -08:00
committed by GitHub
parent ad99e4a7f7
commit 7ed710c226
13 changed files with 36 additions and 37 deletions

View File

@@ -22,5 +22,5 @@ import org.pkl.commons.cli.cliMain
/** Main method of the Pkl CLI (command-line evaluator and REPL). */ /** Main method of the Pkl CLI (command-line evaluator and REPL). */
internal fun main(args: Array<String>) { internal fun main(args: Array<String>) {
cliMain { RootCommand.main(args) } cliMain { RootCommand().main(args) }
} }

View File

@@ -25,7 +25,7 @@ import org.pkl.cli.CliImportAnalyzerOptions
import org.pkl.commons.cli.commands.ModulesCommand import org.pkl.commons.cli.commands.ModulesCommand
import org.pkl.commons.cli.commands.single import org.pkl.commons.cli.commands.single
object AnalyzeCommand : class AnalyzeCommand :
NoOpCliktCommand( NoOpCliktCommand(
name = "analyze", name = "analyze",
help = "Commands related to static analysis", help = "Commands related to static analysis",

View File

@@ -27,7 +27,7 @@ import org.pkl.commons.cli.commands.ProjectOptions
import org.pkl.commons.cli.commands.single import org.pkl.commons.cli.commands.single
import org.pkl.core.packages.PackageUri import org.pkl.core.packages.PackageUri
object DownloadPackageCommand : class DownloadPackageCommand :
BaseCommand( BaseCommand(
name = "download-package", name = "download-package",
helpLink = helpLink, helpLink = helpLink,

View File

@@ -24,7 +24,7 @@ import org.pkl.cli.CliEvaluatorOptions
import org.pkl.commons.cli.commands.ModulesCommand import org.pkl.commons.cli.commands.ModulesCommand
import org.pkl.commons.cli.commands.single import org.pkl.commons.cli.commands.single
object EvalCommand : class EvalCommand :
ModulesCommand(name = "eval", help = "Render pkl module(s)", helpLink = helpLink) { ModulesCommand(name = "eval", help = "Render pkl module(s)", helpLink = helpLink) {
private val outputPath: String? by private val outputPath: String? by
option( option(

View File

@@ -33,18 +33,18 @@ import org.pkl.commons.cli.commands.single
private const val NEWLINE = '\u0085' private const val NEWLINE = '\u0085'
object ProjectCommand : class ProjectCommand :
NoOpCliktCommand( NoOpCliktCommand(
name = "project", name = "project",
help = "Run commands related to projects", help = "Run commands related to projects",
epilog = "For more information, visit $helpLink", epilog = "For more information, visit $helpLink",
) { ) {
init { init {
subcommands(ResolveCommand, PackageCommand) subcommands(ResolveCommand(), PackageCommand())
} }
} }
object ResolveCommand : class ResolveCommand :
BaseCommand( BaseCommand(
name = "resolve", name = "resolve",
helpLink = helpLink, helpLink = helpLink,
@@ -74,7 +74,7 @@ object ResolveCommand :
} }
} }
object PackageCommand : class PackageCommand :
BaseCommand( BaseCommand(
name = "package", name = "package",
helpLink = helpLink, helpLink = helpLink,

View File

@@ -21,8 +21,7 @@ import org.pkl.cli.CliRepl
import org.pkl.commons.cli.commands.BaseCommand import org.pkl.commons.cli.commands.BaseCommand
import org.pkl.commons.cli.commands.ProjectOptions import org.pkl.commons.cli.commands.ProjectOptions
object ReplCommand : class ReplCommand : BaseCommand(name = "repl", help = "Start a REPL session", helpLink = helpLink) {
BaseCommand(name = "repl", help = "Start a REPL session", helpLink = helpLink) {
private val projectOptions by ProjectOptions() private val projectOptions by ProjectOptions()
override fun run() { override fun run() {

View File

@@ -23,7 +23,7 @@ import org.pkl.core.Release
internal val helpLink = "${Release.current().documentation.homepage}pkl-cli/index.html#usage" internal val helpLink = "${Release.current().documentation.homepage}pkl-cli/index.html#usage"
object RootCommand : class RootCommand :
NoOpCliktCommand( NoOpCliktCommand(
name = "pkl", name = "pkl",
printHelpOnEmptyArgs = true, printHelpOnEmptyArgs = true,
@@ -41,13 +41,13 @@ object RootCommand :
} }
subcommands( subcommands(
EvalCommand, EvalCommand(),
ReplCommand, ReplCommand(),
ServerCommand, ServerCommand(),
TestCommand, TestCommand(),
ProjectCommand, ProjectCommand(),
DownloadPackageCommand, DownloadPackageCommand(),
AnalyzeCommand, AnalyzeCommand(),
) )
} }
} }

View File

@@ -19,7 +19,7 @@ import com.github.ajalt.clikt.core.CliktCommand
import org.pkl.cli.CliServer import org.pkl.cli.CliServer
import org.pkl.commons.cli.CliBaseOptions import org.pkl.commons.cli.CliBaseOptions
object ServerCommand : class ServerCommand :
CliktCommand( CliktCommand(
name = "server", name = "server",
help = "Run as a server that communicates over standard input/output", help = "Run as a server that communicates over standard input/output",

View File

@@ -26,7 +26,7 @@ import org.pkl.commons.cli.commands.BaseOptions
import org.pkl.commons.cli.commands.ProjectOptions import org.pkl.commons.cli.commands.ProjectOptions
import org.pkl.commons.cli.commands.TestOptions import org.pkl.commons.cli.commands.TestOptions
object TestCommand : class TestCommand :
BaseCommand(name = "test", help = "Run tests within the given module(s)", helpLink = helpLink) { BaseCommand(name = "test", help = "Run tests within the given module(s)", helpLink = helpLink) {
val modules: List<URI> by val modules: List<URI> by
argument(name = "<modules>", help = "Module paths or URIs to evaluate.") argument(name = "<modules>", help = "Module paths or URIs to evaluate.")

View File

@@ -32,28 +32,28 @@ import org.pkl.commons.writeString
import org.pkl.core.Release import org.pkl.core.Release
class CliMainTest { class CliMainTest {
private val rootCmd = RootCommand()
@Test @Test
fun `duplicate CLI option produces meaningful error message`(@TempDir tempDir: Path) { fun `duplicate CLI option produces meaningful error message`(@TempDir tempDir: Path) {
val inputFile = tempDir.resolve("test.pkl").writeString("").toString() val inputFile = tempDir.resolve("test.pkl").writeString("").toString()
assertThatCode { assertThatCode {
RootCommand.parse( rootCmd.parse(
arrayOf("eval", "--output-path", "path1", "--output-path", "path2", inputFile) arrayOf("eval", "--output-path", "path1", "--output-path", "path2", inputFile)
) )
} }
.hasMessage("Invalid value for \"--output-path\": Option cannot be repeated") .hasMessage("Invalid value for \"--output-path\": Option cannot be repeated")
assertThatCode { assertThatCode {
RootCommand.parse(arrayOf("eval", "-o", "path1", "--output-path", "path2", inputFile)) rootCmd.parse(arrayOf("eval", "-o", "path1", "--output-path", "path2", inputFile))
} }
.hasMessage("Invalid value for \"--output-path\": Option cannot be repeated") .hasMessage("Invalid value for \"--output-path\": Option cannot be repeated")
} }
@Test @Test
fun `eval requires at least one file`() { fun `eval requires at least one file`() {
assertThatCode { RootCommand.parse(arrayOf("eval")) } assertThatCode { rootCmd.parse(arrayOf("eval")) }.hasMessage("""Missing argument "<modules>"""")
.hasMessage("""Missing argument "<modules>"""")
} }
// Can't reliably create symlinks on Windows. // Can't reliably create symlinks on Windows.
@@ -74,7 +74,7 @@ class CliMainTest {
val inputFile = tempDir.resolve("test.pkl").writeString(code).toString() val inputFile = tempDir.resolve("test.pkl").writeString(code).toString()
val outputFile = makeSymdir(tempDir, "out", "linkOut").resolve("test.pkl").toString() val outputFile = makeSymdir(tempDir, "out", "linkOut").resolve("test.pkl").toString()
assertThatCode { RootCommand.parse(arrayOf("eval", inputFile, "-o", outputFile)) } assertThatCode { rootCmd.parse(arrayOf("eval", inputFile, "-o", outputFile)) }
.doesNotThrowAnyException() .doesNotThrowAnyException()
} }
@@ -85,24 +85,24 @@ class CliMainTest {
val error = val error =
"""Invalid value for "--multiple-file-output-path": Option is mutually exclusive with -o, --output-path and -x, --expression.""" """Invalid value for "--multiple-file-output-path": Option is mutually exclusive with -o, --output-path and -x, --expression."""
assertThatCode { RootCommand.parse(arrayOf("eval", "-m", testOut, "-x", "x", testIn)) } assertThatCode { rootCmd.parse(arrayOf("eval", "-m", testOut, "-x", "x", testIn)) }
.hasMessage(error) .hasMessage(error)
assertThatCode { RootCommand.parse(arrayOf("eval", "-m", testOut, "-o", "/tmp/test", testIn)) } assertThatCode { rootCmd.parse(arrayOf("eval", "-m", testOut, "-o", "/tmp/test", testIn)) }
.hasMessage(error) .hasMessage(error)
} }
@Test @Test
fun `showing version works`() { fun `showing version works`() {
assertThatCode { RootCommand.parse(arrayOf("--version")) } assertThatCode { rootCmd.parse(arrayOf("--version")) }.hasMessage(Release.current().versionInfo)
.hasMessage(Release.current().versionInfo)
} }
@Test @Test
fun `file paths get parsed into URIs`(@TempDir tempDir: Path) { fun `file paths get parsed into URIs`(@TempDir tempDir: Path) {
RootCommand.parse(arrayOf("eval", makeInput(tempDir, "my file.txt"))) val cmd = RootCommand()
cmd.parse(arrayOf("eval", makeInput(tempDir, "my file.txt")))
val evalCmd = RootCommand.registeredSubcommands().filterIsInstance<EvalCommand>().first() val evalCmd = cmd.registeredSubcommands().filterIsInstance<EvalCommand>().first()
val modules = evalCmd.baseOptions.baseOptions(evalCmd.modules).normalizedSourceModules val modules = evalCmd.baseOptions.baseOptions(evalCmd.modules).normalizedSourceModules
assertThat(modules).hasSize(1) assertThat(modules).hasSize(1)
assertThat(modules[0].path).endsWith("my file.txt") assertThat(modules[0].path).endsWith("my file.txt")
@@ -110,8 +110,7 @@ class CliMainTest {
@Test @Test
fun `invalid URIs are not accepted`() { fun `invalid URIs are not accepted`() {
val ex = val ex = assertThrows<BadParameterValue> { rootCmd.parse(arrayOf("eval", "file:my file.txt")) }
assertThrows<BadParameterValue> { RootCommand.parse(arrayOf("eval", "file:my file.txt")) }
assertThat(ex.message).contains("URI `file:my file.txt` has invalid syntax") assertThat(ex.message).contains("URI `file:my file.txt` has invalid syntax")
} }

View File

@@ -388,7 +388,7 @@ class CliTestRunnerTest {
@Test @Test
fun `no source modules specified has same message as pkl eval`() { fun `no source modules specified has same message as pkl eval`() {
val e1 = assertThrows<CliException> { CliTestRunner(CliBaseOptions(), CliTestOptions()).run() } val e1 = assertThrows<CliException> { CliTestRunner(CliBaseOptions(), CliTestOptions()).run() }
val e2 = assertThrows<MissingArgument> { RootCommand.parse(listOf("eval")) } val e2 = assertThrows<MissingArgument> { RootCommand().parse(listOf("eval")) }
assertThat(e1).hasMessageContaining("Missing argument \"<modules>\"") assertThat(e1).hasMessageContaining("Missing argument \"<modules>\"")
assertThat(e1.message!!.replace("test", "eval")).isEqualTo(e2.helpMessage()) assertThat(e1.message!!.replace("test", "eval")).isEqualTo(e2.helpMessage())
} }

View File

@@ -34,10 +34,10 @@ import org.pkl.core.Release
/** Main method for the Pkldoc CLI. */ /** Main method for the Pkldoc CLI. */
internal fun main(args: Array<String>) { internal fun main(args: Array<String>) {
cliMain { DocCommand.main(args) } cliMain { DocCommand().main(args) }
} }
object DocCommand : class DocCommand :
BaseCommand(name = "pkldoc", helpLink = Release.current().documentation().homepage()) { BaseCommand(name = "pkldoc", helpLink = Release.current().documentation().homepage()) {
private val modules: List<URI> by private val modules: List<URI> by

View File

@@ -23,7 +23,8 @@ import org.pkl.commons.cli.CliException
class CliMainTest { class CliMainTest {
@Test @Test
fun `CLI run test`() { fun `CLI run test`() {
val e = assertThrows<CliException> { DocCommand.parse(arrayOf("foo", "--output-dir", "/tmp")) } val e =
assertThrows<CliException> { DocCommand().parse(arrayOf("foo", "--output-dir", "/tmp")) }
assertThat(e) assertThat(e)
.hasMessageContaining("must contain at least one module named `doc-package-info.pkl`") .hasMessageContaining("must contain at least one module named `doc-package-info.pkl`")
} }