Skip to content

Commit

Permalink
Refactor CLI argument parsing
Browse files Browse the repository at this point in the history 
Closes facebook#465
  • Loading branch information
Joseph Cooper committed May 17, 2024
1 parent f3a2e54 commit 8defaca
Show file tree
Hide file tree
Showing 3 changed files with 79 additions and 103 deletions.
30 changes: 21 additions & 9 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 args: Array<String>
) {
companion object {
@JvmStatic
Expand Down Expand Up @@ -69,9 +69,14 @@ class Main(
}
}

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


fun run(): Int {
val processArgs = ParsedArgs.processArgs(args)
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 +86,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 +112,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 +131,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 +151,7 @@ class Main(
return alreadyFormatted
}

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

fun exitFatal(message: String, returnCode: Int): Nothing {
err.println(message)
exitProcess(returnCode)
}

}

35 changes: 19 additions & 16 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,33 @@ 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(
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
}

117 changes: 39 additions & 78 deletions core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,13 @@ package com.facebook.ktfmt.cli
import com.facebook.ktfmt.format.Formatter
import com.facebook.ktfmt.format.FormattingOptions
import com.google.common.truth.Truth.assertThat
import java.io.ByteArrayOutputStream
import java.io.FileNotFoundException
import java.io.PrintStream
import kotlin.io.path.createTempDirectory
import kotlin.test.assertFailsWith
import org.junit.After
import org.junit.Test
import org.junit.runner.RunWith
import org.junit.runners.JUnit4
import java.io.FileNotFoundException
import kotlin.io.path.createTempDirectory
import kotlin.test.assertFailsWith

@Suppress("FunctionNaming")
@RunWith(JUnit4::class)
Expand All @@ -41,152 +39,115 @@ class ParsedArgsTest {
}

@Test
fun `files to format are returned and unknown flags are reported`() {
val (parsed, out) = parseTestOptions("foo.kt", "--unknown")

assertThat(parsed.fileNames).containsExactly("foo.kt")
assertThat(out.toString()).isEqualTo("Unexpected option: --unknown\n")
fun `unknown flags return an error`() {
val result = ParsedArgs.parseOptions(arrayOf("--unknown"))
assertThat(result).isInstanceOf(ParseResult.Error::class.java)
}

@Test
fun `files to format are returned and flags starting with @ are reported`() {
val (parsed, out) = parseTestOptions("foo.kt", "@unknown")

assertThat(parsed.fileNames).containsExactly("foo.kt")
assertThat(out.toString()).isEqualTo("Unexpected option: @unknown\n")
fun `unknown flags starting with '@' return an error`() {
val result = ParsedArgs.parseOptions(arrayOf("@unknown"))
assertThat(result).isInstanceOf(ParseResult.Error::class.java)
}

@Test
fun `parseOptions uses default values when args are empty`() {
val (parsed, _) = parseTestOptions("foo.kt")
val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("foo.kt")))

val formattingOptions = parsed.formattingOptions
assertThat(formattingOptions.style).isEqualTo(FormattingOptions.Style.FACEBOOK)
assertThat(formattingOptions.maxWidth).isEqualTo(100)
assertThat(formattingOptions.blockIndent).isEqualTo(2)
assertThat(formattingOptions.continuationIndent).isEqualTo(4)
assertThat(formattingOptions.removeUnusedImports).isTrue()
assertThat(formattingOptions.debuggingPrintOpsAfterFormatting).isFalse()

assertThat(parsed.dryRun).isFalse()
assertThat(parsed.setExitIfChanged).isFalse()
assertThat(parsed.stdinName).isNull()
val defaultFormattingOptions = FormattingOptions()
assertThat(formattingOptions).isEqualTo(defaultFormattingOptions)
}

@Test
fun `parseOptions recognizes --dropbox-style and rejects unknown flags`() {
val (parsed, out) = parseTestOptions("--dropbox-style", "foo.kt", "--unknown")

assertThat(parsed.fileNames).containsExactly("foo.kt")
assertThat(parsed.formattingOptions.blockIndent).isEqualTo(4)
assertThat(parsed.formattingOptions.continuationIndent).isEqualTo(4)
assertThat(out.toString()).isEqualTo("Unexpected option: --unknown\n")
fun `parseOptions recognizes --dropbox-style`() {
val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--dropbox-style", "foo.kt")))
assertThat(parsed.formattingOptions).isEqualTo(Formatter.DROPBOX_FORMAT)
}

@Test
fun `parseOptions recognizes --google-style`() {
val (parsed, _) = parseTestOptions("--google-style", "foo.kt")
val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--google-style", "foo.kt")))
assertThat(parsed.formattingOptions).isEqualTo(Formatter.GOOGLE_FORMAT)
}

@Test
fun `parseOptions recognizes --dry-run`() {
val (parsed, _) = parseTestOptions("--dry-run", "foo.kt")
val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--dry-run", "foo.kt")))
assertThat(parsed.dryRun).isTrue()
}

@Test
fun `parseOptions recognizes -n as --dry-run`() {
val (parsed, _) = parseTestOptions("-n", "foo.kt")
val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("-n", "foo.kt")))
assertThat(parsed.dryRun).isTrue()
}

@Test
fun `parseOptions recognizes --set-exit-if-changed`() {
val (parsed, _) = parseTestOptions("--set-exit-if-changed", "foo.kt")
val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--set-exit-if-changed", "foo.kt")))
assertThat(parsed.setExitIfChanged).isTrue()
}

@Test
fun `parseOptions defaults to removing imports`() {
val (parsed, _) = parseTestOptions("foo.kt")
val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("foo.kt")))
assertThat(parsed.formattingOptions.removeUnusedImports).isTrue()
}

@Test
fun `parseOptions recognizes --do-not-remove-unused-imports to removing imports`() {
val (parsed, _) = parseTestOptions("--do-not-remove-unused-imports", "foo.kt")
assertThat(parsed.formattingOptions.removeUnusedImports).isFalse()
}

@Test
fun `parseOptions handles dropbox style and --do-not-remove-unused-imports`() {
val (parsed, _) =
parseTestOptions("--do-not-remove-unused-imports", "--dropbox-style", "foo.kt")
assertThat(parsed.formattingOptions.removeUnusedImports).isFalse()
assertThat(parsed.formattingOptions.style).isEqualTo(FormattingOptions.Style.DROPBOX)
}

@Test
fun `parseOptions handles google style and --do-not-remove-unused-imports`() {
val (parsed, _) = parseTestOptions("--do-not-remove-unused-imports", "--google-style", "foo.kt")
val parsed =
assertSucceeds(ParsedArgs.parseOptions(arrayOf("--do-not-remove-unused-imports", "foo.kt")))
assertThat(parsed.formattingOptions.removeUnusedImports).isFalse()
assertThat(parsed.formattingOptions.style).isEqualTo(FormattingOptions.Style.GOOGLE)
}

@Test
fun `parseOptions --stdin-name`() {
val (parsed, _) = parseTestOptions("--stdin-name=my/foo.kt")
fun `parseOptions recognizes --stdin-name`() {
val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--stdin-name=my/foo.kt")))
assertThat(parsed.stdinName).isEqualTo("my/foo.kt")
}

@Test
fun `parseOptions --stdin-name with empty value`() {
val (parsed, _) = parseTestOptions("--stdin-name=")
fun `parseOptions accepts --stdin-name with empty value`() {
val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--stdin-name=")))
assertThat(parsed.stdinName).isEqualTo("")
}

@Test
fun `parseOptions --stdin-name without value`() {
val (parsed, out) = parseTestOptions("--stdin-name")
assertThat(out).isEqualTo("Found option '--stdin-name', expected '--stdin-name=<value>'\n")
assertThat(parsed.stdinName).isNull()
}

@Test
fun `parseOptions --stdin-name prefix`() {
val (parsed, out) = parseTestOptions("--stdin-namea")
assertThat(out).isEqualTo("Found option '--stdin-namea', expected '--stdin-name=<value>'\n")
assertThat(parsed.stdinName).isNull()
}
@Test
fun `parseOptions --stdin-name without value`() {
val parseResult = ParsedArgs.parseOptions(arrayOf("--stdin-name"))
assertThat(parseResult).isInstanceOf(ParseResult.Error::class.java)
}

@Test
fun `processArgs use the @file option with non existing file`() {
val out = ByteArrayOutputStream()

val e =
assertFailsWith<FileNotFoundException> {
ParsedArgs.processArgs(PrintStream(out), arrayOf("@non-existing-file"))
ParsedArgs.processArgs(arrayOf("@non-existing-file"))
}
assertThat(e.message).contains("non-existing-file (No such file or directory)")
}

@Test
fun `processArgs use the @file option with file containing arguments`() {
val out = ByteArrayOutputStream()
val file = root.resolve("existing-file")
file.writeText("--google-style\n--dry-run\n--set-exit-if-changed\nFile1.kt\nFile2.kt\n")

val parsed = ParsedArgs.processArgs(PrintStream(out), arrayOf("@" + file.absolutePath))
val result = ParsedArgs.processArgs(arrayOf("@" + file.absolutePath))
assertThat(result).isInstanceOf(ParseResult.Ok::class.java)

val parsed = (result as ParseResult.Ok).parsedValue

assertThat(parsed.formattingOptions).isEqualTo(Formatter.GOOGLE_FORMAT)
assertThat(parsed.dryRun).isTrue()
assertThat(parsed.setExitIfChanged).isTrue()
assertThat(parsed.fileNames).containsExactlyElementsIn(listOf("File1.kt", "File2.kt"))
}

private fun parseTestOptions(vararg args: String): Pair<ParsedArgs, String> {
val out = ByteArrayOutputStream()
return Pair(ParsedArgs.parseOptions(PrintStream(out), arrayOf(*args)), out.toString())
private fun assertSucceeds(parseResult: ParseResult): ParsedArgs {
assertThat(parseResult).isInstanceOf(ParseResult.Ok::class.java)
return (parseResult as ParseResult.Ok).parsedValue
}
}

0 comments on commit 8defaca

Please sign in to comment.