Do not print stack trace if Cli.parseArgs fails (#1629)

* 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.

* Test unrecognized argument

* Test finiteDurationArgParser with previous value

Author: fthomas

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)
+    }
 }

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 {

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]

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,16 +42,16 @@ 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"),
         List("--git-author-email", "d"),
         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
+  }
 }