Refactor CLI argument parsing

core/src/main/java/com/facebook/ktfmt/cli/Main.kt

@@ -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
@@ -69,9 +69,14 @@ 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 ...")
@@ -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
@@ -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) {
@@ -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)
           }
@@ -146,7 +151,7 @@ class Main(
         return alreadyFormatted
       }
 
-      if (parsedArgs.dryRun) {
+      if (args.dryRun) {
         if (!alreadyFormatted) {
           out.println(fileName)
         }
@@ -173,4 +178,11 @@ class Main(
       throw e
     }
   }
+
+  fun exitFatal(message: String, returnCode: Int): Nothing {
+    err.println(message)
+    exitProcess(returnCode)
+  }
+
 }
+

core/src/main/java/com/facebook/ktfmt/cli/ParsedArgs.kt

@@ -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. */
@@ -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
@@ -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
+}
+

core/src/test/java/com/facebook/ktfmt/cli/ParsedArgsParameterizedTest.kt

@@ -0,0 +1,87 @@
+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 org.junit.Test
+import org.junit.runner.RunWith
+import org.junit.runners.Parameterized
+import org.junit.runners.Parameterized.Parameters
+
+@RunWith(Parameterized::class)
+class ParsedArgsParameterizedTest(
+    // Description is unused in tests
+    //  only included here to fulfil contract of @Parameterized test runner
+    private val description: String,
+    private val expectedResult: ParseResult,
+    private val inputArgs: Array<String>
+) {
+
+  @Test
+  fun `combination of args gives expected result`() {
+    val testResult = ParsedArgs.parseOptions(inputArgs)
+    assertThat(testResult).isEqualTo(expectedResult)
+  }
+
+  companion object {
+
+
+    /**
+    Creates the list of parameters for ParsedArgsParameterizedTest
+    Each instance of the parameters is an array containing:
+      - a description string, which junit outputs to the console
+      - the expected result of parsing,
+      - an array of the input arguments
+     */
+    @JvmStatic
+    @Parameters(name = "{index}: {0} - parseOptions({1})={2}")
+    fun testData(): Iterable<Array<Any>> =
+        arrayListOf(
+            testCase(
+                description = "Parses multiple args successfully",
+                expectedParseResult =
+                    parseResultOk(
+                        fileNames = listOf("File.kt"),
+                        formattingOptions = Formatter.GOOGLE_FORMAT,
+                        dryRun = true,
+                        setExitIfChanged = true),
+                "--google-style",
+                "--dry-run",
+                "--set-exit-if-changed",
+                "File.kt"),
+            testCase(
+                description = "Last style in args wins",
+                expectedParseResult =
+                    parseResultOk(
+                        fileNames = listOf("File.kt"),
+                        formattingOptions = Formatter.DROPBOX_FORMAT),
+                "--google-style",
+                "--dropbox-style",
+                "File.kt"),
+            testCase(
+                description = "Error when parsing multiple args and one is unknown",
+                expectedParseResult = ParseResult.Error("Unexpected option: @unknown"),
+                "@unknown",
+                "--google-style"))
+
+    private fun testCase(
+        description: String,
+        expectedParseResult: ParseResult,
+        vararg inputArgs: String
+    ) = arrayOf(description, expectedParseResult, inputArgs)
+
+    private fun parseResultOk(
+        fileNames: List<String> = emptyList(),
+        formattingOptions: FormattingOptions = FormattingOptions(),
+        dryRun: Boolean = false,
+        setExitIfChanged: Boolean = false,
+        removedUnusedImports: Boolean = true,
+        stdinName: String? = null
+    ): ParseResult.Ok {
+      val returnedFormattingOptions =
+          formattingOptions.copy(removeUnusedImports = removedUnusedImports)
+      return ParseResult.Ok(
+          ParsedArgs(fileNames, returnedFormattingOptions, dryRun, setExitIfChanged, stdinName))
+    }
+  }
+}