Repository: scala-steward-org/scala-steward
Pull request #1629 (2020-09-29 15:58:36)

Do not print stack trace if Cli.parseArgs fails
We're currently printing a rather unhelpful stack trace if `Cli.parseArgs`
fails. This currently happens if
 - a required argument is missing
 - the value of an argument cannot be parsed
 - `--help` or `--usage` is provided

With this PR no stack traces are printed under these conditions. Error
or help messages are just printed to stderr or stdout and the program
exits with an appropriate status.


Patch:
diff --git a/modules/core/src/main/scala/org/scalasteward/core/Main.scala b/modules/core/src/main/scala/org/scalasteward/core/Main.scala
index f911140e..811a7935 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/Main.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/Main.scala
@@ -17,9 +17,13 @@
 package org.scalasteward.core
 
 import cats.effect.{ExitCode, IO, IOApp}
-import org.scalasteward.core.application.Context
+import org.scalasteward.core.application.{Cli, Context}
 
 object Main extends IOApp {
   override def run(args: List[String]): IO[ExitCode] =
-    Context.create[IO](args).use(_.runF)
+    Cli.parseArgs(args) match {
+      case Cli.ParseResult.Success(args) => Context.create[IO](args).use(_.runF)
+      case Cli.ParseResult.Help(help)    => IO(Console.out.println(help)).as(ExitCode.Success)
+      case Cli.ParseResult.Error(error)  => IO(Console.err.println(error)).as(ExitCode.Error)
+    }
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala
index 8fae55fe..b99688bb 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/application/Cli.scala
@@ -17,30 +17,13 @@
 package org.scalasteward.core.application
 
 import caseapp._
-import caseapp.core.Error.{MalformedValue, Other}
+import caseapp.core.Error.MalformedValue
 import caseapp.core.argparser.{ArgParser, SimpleArgParser}
 import cats.syntax.all._
 import org.http4s.Uri
 import org.http4s.syntax.literals._
-import org.scalasteward.core.application.Cli._
-import org.scalasteward.core.util.ApplicativeThrowable
 import scala.concurrent.duration._
 
-final class Cli[F[_]](implicit F: ApplicativeThrowable[F]) {
-  def parseArgs(args: List[String]): F[Args] =
-    F.fromEither {
-      CaseApp
-        .parseWithHelp[Args](args)
-        .flatMap {
-          case (_, true, _, _)              => Left(Other(CaseApp.helpMessage[Args]))
-          case (_, _, true, _)              => Left(Other(CaseApp.usageMessage[Args]))
-          case (parsed @ Right(_), _, _, _) => parsed
-          case (e @ Left(_), _, _, _)       => e
-        }
-        .leftMap(e => new Throwable(e.message))
-    }
-}
-
 object Cli {
   final case class Args(
       workspace: String,
@@ -70,6 +53,22 @@ object Cli {
 
   final case class EnvVar(name: String, value: String)
 
+  sealed trait ParseResult extends Product with Serializable
+  object ParseResult {
+    final case class Success(args: Args) extends ParseResult
+    final case class Help(help: String) extends ParseResult
+    final case class Error(error: String) extends ParseResult
+  }
+
+  def parseArgs(args: List[String]): ParseResult =
+    CaseApp.parseWithHelp[Args](args) match {
+      case Right((_, true, _, _))        => ParseResult.Help(CaseApp.helpMessage[Args])
+      case Right((_, _, true, _))        => ParseResult.Help(CaseApp.usageMessage[Args])
+      case Right((Right(args), _, _, _)) => ParseResult.Success(args)
+      case Right((Left(error), _, _, _)) => ParseResult.Error(error.message)
+      case Left(error)                   => ParseResult.Error(error.message)
+    }
+
   implicit val envVarArgParser: SimpleArgParser[EnvVar] =
     SimpleArgParser.from[EnvVar]("env-var") { s =>
       s.trim.split('=').toList match {
diff --git a/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala b/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala
index bb76b962..22945475 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/application/Context.scala
@@ -44,12 +44,11 @@ import org.scalasteward.core.vcs.{VCSApiAlg, VCSExtraAlg, VCSRepoAlg, VCSSelecti
 
 object Context {
   def create[F[_]: ConcurrentEffect: ContextShift: Parallel: Timer](
-      args: List[String]
+      args: Cli.Args
   ): Resource[F, StewardAlg[F]] =
     for {
       blocker <- Blocker[F]
-      cliArgs_ <- Resource.liftF(new Cli[F].parseArgs(args))
-      implicit0(config: Config) <- Resource.liftF(Config.create[F](cliArgs_))
+      implicit0(config: Config) <- Resource.liftF(Config.create[F](args))
       implicit0(client: Client[F]) <- AsyncHttpClient.resource[F]()
       implicit0(logger: Logger[F]) <- Resource.liftF(Slf4jLogger.create[F])
       implicit0(httpExistenceClient: HttpExistenceClient[F]) <- HttpExistenceClient.create[F]


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala
index d1cbc7e6..e67fc1dd 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/application/CliTest.scala
@@ -2,17 +2,15 @@ package org.scalasteward.core.application
 
 import org.http4s.syntax.literals._
 import org.scalasteward.core.application.Cli.EnvVar
+import org.scalasteward.core.application.Cli.ParseResult._
 import org.scalatest.EitherValues
 import org.scalatest.funsuite.AnyFunSuite
 import org.scalatest.matchers.should.Matchers
 import scala.concurrent.duration._
 
 class CliTest extends AnyFunSuite with Matchers with EitherValues {
-  type Result[A] = Either[Throwable, A]
-  val cli: Cli[Result] = new Cli[Result]
-
-  test("parseArgs") {
-    cli.parseArgs(
+  test("parseArgs: example") {
+    Cli.parseArgs(
       List(
         List("--workspace", "a"),
         List("--repos-file", "b"),
@@ -27,7 +25,7 @@ class CliTest extends AnyFunSuite with Matchers with EitherValues {
         List("--env-var", "i=j"),
         List("--process-timeout", "30min")
       ).flatten
-    ) shouldBe Right(
+    ) shouldBe Success(
       Cli.Args(
         workspace = "a",
         reposFile = "b",
@@ -44,8 +42,8 @@ class CliTest extends AnyFunSuite with Matchers with EitherValues {
     )
   }
 
-  test("parseArgs minimal version") {
-    cli.parseArgs(
+  test("parseArgs: minimal example") {
+    Cli.parseArgs(
       List(
         List("--workspace", "a"),
         List("--repos-file", "b"),
@@ -53,7 +51,7 @@ class CliTest extends AnyFunSuite with Matchers with EitherValues {
         List("--vcs-login", "e"),
         List("--git-ask-pass", "f")
       ).flatten
-    ) shouldBe Right(
+    ) shouldBe Success(
       Cli.Args(
         workspace = "a",
         reposFile = "b",
@@ -64,36 +62,44 @@ class CliTest extends AnyFunSuite with Matchers with EitherValues {
     )
   }
 
-  test("parseArgs fail if required option not provided") {
-    cli.parseArgs(Nil).isLeft shouldBe true
+  test("parseArgs: fail if required option not provided") {
+    Cli.parseArgs(Nil).asInstanceOf[Error].error should startWith("Required option")
+  }
+
+  test("parseArgs: unrecognized argument") {
+    Cli.parseArgs(List("--foo")).asInstanceOf[Error].error should startWith("Unrecognized")
   }
 
-  test("parseArgs --help") {
-    cli.parseArgs(List("--help")).left.value.getMessage should include("--git-author-email")
+  test("parseArgs: --help") {
+    Cli.parseArgs(List("--help")).asInstanceOf[Help].help should include("--git-author-email")
   }
 
-  test("parseArgs --usage") {
-    cli.parseArgs(List("--usage")).left.value.getMessage should startWith("Usage: args")
+  test("parseArgs: --usage") {
+    Cli.parseArgs(List("--usage")).asInstanceOf[Help].help should startWith("Usage: args")
   }
 
-  test("env-var without equals sign") {
+  test("envVarArgParser: env-var without equals sign") {
     Cli.envVarArgParser(None, "SBT_OPTS").isLeft shouldBe true
   }
 
-  test("env-var with multiple equals signs") {
+  test("envVarArgParser: env-var with multiple equals signs") {
     val value = "-Xss8m -XX:MaxMetaspaceSize=256m"
     Cli.envVarArgParser(None, s"SBT_OPTS=$value") shouldBe Right(EnvVar("SBT_OPTS", value))
   }
 
-  test("valid timeout") {
+  test("finiteDurationArgParser: well-formed duration") {
     Cli.finiteDurationArgParser(None, "30min") shouldBe Right(30.minutes)
   }
 
-  test("malformed timeout") {
+  test("finiteDurationArgParser: malformed duration") {
     Cli.finiteDurationArgParser(None, "xyz").isLeft shouldBe true
   }
 
-  test("malformed timeout (Inf)") {
+  test("finiteDurationArgParser: malformed duration (Inf)") {
     Cli.finiteDurationArgParser(None, "Inf").isLeft shouldBe true
   }
+
+  test("finiteDurationArgParser: previous value") {
+    Cli.finiteDurationArgParser(Some(10.seconds), "20seconds").isLeft shouldBe true
+  }
 }