Further refactoring on CLI

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

@@ -32,6 +32,14 @@ import java.nio.charset.StandardCharsets.UTF_8
 import java.util.concurrent.atomic.AtomicInteger
 import kotlin.system.exitProcess
 
+private const val EXIT_CODE_FAILURE = 1
+
+private const val EXIT_CODE_SUCCESS = 0
+
+private const val USAGE =
+    """Usage: ktfmt [--style=dropbox|google|kotlinlang] [--dry-run] [--set-exit-if-changed] [--stdin-name=<name>] [--do-not-remove-unused-imports] File1.kt File2.kt ...
+Or: ktfmt @file"""
+
 class Main(
     private val input: InputStream,
     private val out: PrintStream,
@@ -56,10 +64,6 @@ class Main(
       }
       val result = mutableListOf<File>()
       for (arg in args) {
-        if (arg == "-") {
-          error(
-              "Error: '-', which causes ktfmt to read from stdin, should not be mixed with file name")
-        }
         result.addAll(
             File(arg).walkTopDown().filter {
               it.isFile && (it.extension == "kt" || it.extension == "kts")
@@ -74,48 +78,46 @@ class Main(
     val parsedArgs =
         when (processArgs) {
           is ParseResult.Ok -> processArgs.parsedValue
-          is ParseResult.Error -> exitFatal(processArgs.errorMessage, 1)
+          is ParseResult.Error -> {
+            err.println(processArgs.errorMessage)
+            return EXIT_CODE_FAILURE
+          }
         }
     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 ...")
-      err.println("Or: ktfmt @file")
-      return 1
+      err.println(USAGE)
+      return EXIT_CODE_FAILURE
     }
 
     if (parsedArgs.fileNames.size == 1 && parsedArgs.fileNames[0] == "-") {
+      // Format code read from stdin
       return try {
         val alreadyFormatted = format(null, parsedArgs)
-        if (!alreadyFormatted && parsedArgs.setExitIfChanged) 1 else 0
+        if (!alreadyFormatted && parsedArgs.setExitIfChanged)
+            EXIT_CODE_FAILURE
+        else
+            EXIT_CODE_SUCCESS
       } catch (e: Exception) {
-        1
+        e.printStackTrace(err)
+        EXIT_CODE_FAILURE
       }
-    } else if (parsedArgs.stdinName != null) {
-      err.println("Error: --stdin-name can only be used with stdin")
-      return 1
-    }
-
-    val files: List<File>
-    try {
-      files = expandArgsToFileNames(parsedArgs.fileNames)
-    } catch (e: java.lang.IllegalStateException) {
-      err.println(e.message)
-      return 1
     }
 
+    val files: List<File> = expandArgsToFileNames(parsedArgs.fileNames)
+
     if (files.isEmpty()) {
       err.println("Error: no .kt files found")
-      return 1
+      return EXIT_CODE_FAILURE
     }
 
-    val retval = AtomicInteger(0)
+    val retval = AtomicInteger(EXIT_CODE_SUCCESS)
     files.parallelStream().forEach {
       try {
         if (!format(it, parsedArgs) && parsedArgs.setExitIfChanged) {
-          retval.set(1)
+          retval.set(EXIT_CODE_FAILURE)
         }
       } catch (e: Exception) {
-        retval.set(1)
+        e.printStackTrace(err)
+        retval.set(EXIT_CODE_FAILURE)
       }
     }
     return retval.get()
@@ -177,15 +179,4 @@ 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)
-  }
 }

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

@@ -76,6 +76,18 @@ data class ParsedArgs(
         }
       }
 
+      if (fileNames.contains("-")) {
+        // We're reading from stdin
+        if (fileNames.size > 1) {
+          val filesExceptStdin = fileNames - "-"
+          return ParseResult.Error(
+              "Cannot read from stdin and files in same run. Found stdin specifier '-'" +
+                  " and files ${filesExceptStdin.joinToString(", ")} ")
+        }
+      } else if (stdinName != null) {
+        return ParseResult.Error("--stdin-name can only be specified when reading from stdin")
+      }
+
       return ParseResult.Ok(
           ParsedArgs(
               fileNames,

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

@@ -104,16 +104,6 @@ class MainTest {
         .containsExactly(foo1, bar1, foo2, bar2)
   }
 
-  @Test
-  fun `expandArgsToFileNames - a dash is an error`() {
-    try {
-      Main.expandArgsToFileNames(listOf(root.resolve("foo.bar").toString(), File("-").toString()))
-      fail("expected exception, but nothing was thrown")
-    } catch (e: IllegalStateException) {
-      assertThat(e.message).contains("Error")
-    }
-  }
-
   @Test
   fun `Using '-' as the filename formats an InputStream`() {
     val code = "fun    f1 (  ) :    Int =    0"
@@ -470,27 +460,6 @@ class MainTest {
     assertThat(exitCode).isEqualTo(1)
   }
 
-  @Test
-  fun `--stdin-name can only be used with stdin`() {
-    val code = """fun f () =    println( "hello, world" )"""
-    val file = root.resolve("foo.kt")
-    file.writeText(code, UTF_8)
-
-    val exitCode =
-        Main(
-                emptyInput,
-                PrintStream(out),
-                PrintStream(err),
-                arrayOf("--stdin-name=bar.kt", file.toString()))
-            .run()
-
-    assertThat(file.readText()).isEqualTo(code)
-    assertThat(out.toString(UTF_8)).isEmpty()
-    assertThat(err.toString(testCharset))
-        .isEqualTo("Error: --stdin-name can only be used with stdin\n")
-    assertThat(exitCode).isEqualTo(1)
-  }
-
   @Test
   fun `Always use UTF8 encoding (stdin, stdout)`() {
     val code = """fun f () =    println( "hello, world" )"""

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

@@ -105,22 +105,34 @@ class ParsedArgsTest {
 
   @Test
   fun `parseOptions recognizes --stdin-name`() {
-    val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--stdin-name=my/foo.kt")))
+    val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--stdin-name=my/foo.kt", "-")))
     assertThat(parsed.stdinName).isEqualTo("my/foo.kt")
   }
 
   @Test
   fun `parseOptions accepts --stdin-name with empty value`() {
-    val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--stdin-name=")))
+    val parsed = assertSucceeds(ParsedArgs.parseOptions(arrayOf("--stdin-name=", "-")))
     assertThat(parsed.stdinName).isEqualTo("")
   }
 
   @Test
-  fun `parseOptions --stdin-name without value`() {
+  fun `parseOptions rejects --stdin-name without value`() {
     val parseResult = ParsedArgs.parseOptions(arrayOf("--stdin-name"))
     assertThat(parseResult).isInstanceOf(ParseResult.Error::class.java)
   }
 
+  @Test
+  fun `parseOptions rejects '-' and files at the same time`() {
+    val parseResult = ParsedArgs.parseOptions(arrayOf("-", "File.kt"))
+    assertThat(parseResult).isInstanceOf(ParseResult.Error::class.java)
+  }
+
+  @Test
+  fun `parseOptions rejects --stdin-name when not reading from stdin`() {
+    val parseResult = ParsedArgs.parseOptions(arrayOf("--stdin-name=foo","file1.kt"))
+    assertThat(parseResult).isInstanceOf(ParseResult.Error::class.java)
+  }
+
   @Test
   fun `processArgs use the @file option with non existing file`() {
     val e =