Skip to content

Commit

Permalink
Refactor CLI argument parsing (#467)
Browse files Browse the repository at this point in the history 
Summary:
Working towards #391.

I wanted to open this to get feedback on the approach.

I could have chosen to use exceptions, but I find sealed classes much more composable and I already anticipate adding another class to the `ParseResult` hierarchy to represent a situation where a message needs to be shown to the user but isn't an error (`--help` is the obvious example).

Closes #465 more or less as a side-effect.

Pull Request resolved: #467

Reviewed By: strulovich

Differential Revision: D58136233

Pulled By: hick209

fbshipit-source-id: 85662e7b5f19195c4bb4c9c28caa02ae224ede52
  • Loading branch information
@facebook-github-bot
Joseph Cooper authored and facebook-github-bot committed Jun 5, 2024
1 parent 9a916b7 commit 26a24ae
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 103 deletions.
35 changes: 25 additions & 10 deletions core/src/main/java/com/facebook/ktfmt/cli/Main.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Main(
private val input: InputStream,
private val out: PrintStream,
private val err: PrintStream,
args: Array<String>
private val inputArgs: Array<String>
) {
companion object {
@JvmStatic
Expand Down Expand Up @@ -69,9 +69,13 @@ class Main(
}
}

private val parsedArgs: ParsedArgs = ParsedArgs.processArgs(err, args)

fun run(): Int {
val processArgs = ParsedArgs.processArgs(inputArgs)
val parsedArgs =
when (processArgs) {
is ParseResult.Ok -> processArgs.parsedValue
is ParseResult.Error -> exitFatal(processArgs.errorMessage, 1)
}
if (parsedArgs.fileNames.isEmpty()) {
err.println(
"Usage: ktfmt [--dropbox-style | --google-style | --kotlinlang-style] [--dry-run] [--set-exit-if-changed] [--stdin-name=<name>] [--do-not-remove-unused-imports] File1.kt File2.kt ...")
Expand All @@ -81,7 +85,7 @@ class Main(

if (parsedArgs.fileNames.size == 1 && parsedArgs.fileNames[0] == "-") {
return try {
val alreadyFormatted = format(null)
val alreadyFormatted = format(null, parsedArgs)
if (!alreadyFormatted && parsedArgs.setExitIfChanged) 1 else 0
} catch (e: Exception) {
1
Expand All @@ -107,7 +111,7 @@ class Main(
val retval = AtomicInteger(0)
files.parallelStream().forEach {
try {
if (!format(it) && parsedArgs.setExitIfChanged) {
if (!format(it, parsedArgs) && parsedArgs.setExitIfChanged) {
retval.set(1)
}
} catch (e: Exception) {
Expand All @@ -126,17 +130,17 @@ class Main(
* @param file The file to format. If null, the code is read from <stdin>.
* @return true iff input is valid and already formatted.
*/
private fun format(file: File?): Boolean {
val fileName = file?.toString() ?: parsedArgs.stdinName ?: "<stdin>"
private fun format(file: File?, args: ParsedArgs): Boolean {
val fileName = file?.toString() ?: args.stdinName ?: "<stdin>"
try {
val bytes = if (file == null) input else FileInputStream(file)
val code = BufferedReader(InputStreamReader(bytes, UTF_8)).readText()
val formattedCode = Formatter.format(parsedArgs.formattingOptions, code)
val formattedCode = Formatter.format(args.formattingOptions, code)
val alreadyFormatted = code == formattedCode

// stdin
if (file == null) {
if (parsedArgs.dryRun) {
if (args.dryRun) {
if (!alreadyFormatted) {
out.println(fileName)
}
Expand All @@ -146,7 +150,7 @@ class Main(
return alreadyFormatted
}

if (parsedArgs.dryRun) {
if (args.dryRun) {
if (!alreadyFormatted) {
out.println(fileName)
}
Expand All @@ -173,4 +177,15 @@ class Main(
throw e
}
}

/**
* Finishes the process with result `returnCode`.
*
* **WARNING**: If you call this method, this is the last that will happen and no code after it
* will be executed.
*/
private fun exitFatal(message: String, returnCode: Int): Nothing {
err.println(message)
exitProcess(returnCode)
}
}
48 changes: 27 additions & 21 deletions core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package com.facebook.ktfmt.cli
import com.facebook.ktfmt.format.Formatter
import com.facebook.ktfmt.format.FormattingOptions
import java.io.File
import java.io.PrintStream
import java.nio.charset.StandardCharsets.UTF_8

/** ParsedArgs holds the arguments passed to ktfmt on the command-line, after parsing. */
Expand All @@ -39,16 +38,16 @@ data class ParsedArgs(
) {
companion object {

fun processArgs(err: PrintStream, args: Array<String>): ParsedArgs {
fun processArgs(args: Array<String>): ParseResult {
if (args.size == 1 && args[0].startsWith("@")) {
return parseOptions(err, File(args[0].substring(1)).readLines(UTF_8).toTypedArray())
return parseOptions(File(args[0].substring(1)).readLines(UTF_8).toTypedArray())
} else {
return parseOptions(err, args)
return parseOptions(args)
}
}

/** parseOptions parses command-line arguments passed to ktfmt. */
fun parseOptions(err: PrintStream, args: Array<String>): ParsedArgs {
fun parseOptions(args: Array<out String>): ParseResult {
val fileNames = mutableListOf<String>()
var formattingOptions = FormattingOptions()
var dryRun = false
Expand All @@ -64,29 +63,36 @@ data class ParsedArgs(
arg == "--dry-run" || arg == "-n" -> dryRun = true
arg == "--set-exit-if-changed" -> setExitIfChanged = true
arg == "--do-not-remove-unused-imports" -> removeUnusedImports = false
arg.startsWith("--stdin-name") -> stdinName = parseKeyValueArg(err, "--stdin-name", arg)
arg.startsWith("--") -> err.println("Unexpected option: $arg")
arg.startsWith("@") -> err.println("Unexpected option: $arg")
arg.startsWith("--stdin-name=") ->
stdinName =
parseKeyValueArg("--stdin-name", arg)
?: return ParseResult.Error(
"Found option '${arg}', expected '${"--stdin-name"}=<value>'")
arg.startsWith("--") -> return ParseResult.Error("Unexpected option: $arg")
arg.startsWith("@") -> return ParseResult.Error("Unexpected option: $arg")
else -> fileNames.add(arg)
}
}

return ParsedArgs(
fileNames,
formattingOptions.copy(removeUnusedImports = removeUnusedImports),
dryRun,
setExitIfChanged,
stdinName,
)
return ParseResult.Ok(
ParsedArgs(
fileNames,
formattingOptions.copy(removeUnusedImports = removeUnusedImports),
dryRun,
setExitIfChanged,
stdinName,
))
}

private fun parseKeyValueArg(err: PrintStream, key: String, arg: String): String? {
private fun parseKeyValueArg(key: String, arg: String): String? {
val parts = arg.split('=', limit = 2)
if (parts[0] != key || parts.size != 2) {
err.println("Found option '${arg}', expected '${key}=<value>'")
return null
}
return parts[1]
return parts[1].takeIf { parts[0] == key || parts.size == 2 }
}
}
}

sealed interface ParseResult {
data class Ok(val parsedValue: ParsedArgs) : ParseResult

data class Error(val errorMessage: String) : ParseResult
}
Loading

0 comments on commit 26a24ae

Please sign in to comment.