ConfParsed: don't mask parsing/retrieval errors

scalameta · Nov 4, 2023 · a2785b8 · a2785b8
1 parent 630322d
commit a2785b8
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 51 deletions.
diff --git a/scalafmt-cli/src/main/scala/org/scalafmt/cli/Cli.scala b/scalafmt-cli/src/main/scala/org/scalafmt/cli/Cli.scala
@@ -98,36 +98,39 @@ object Cli {
   private def getProposedConfigVersion(options: CliOptions): String =
     s"version = $stableVersion"
 
+  private type MaybeRunner = Either[String, ScalafmtRunner]
+
   private def findRunner(
       options: CliOptions
-  ): Either[String, ScalafmtRunner] = {
+  ): MaybeRunner = options.hoconOpt.fold[MaybeRunner] {
+    Left(s"""error: missing Scalafmt configuration file.
+      |Consider creating '${options.getProposedConfigFile}' with the following:
+      |${getProposedConfigVersion(options)}
+      |""".stripMargin)
+  } {
     // Run format using
     // - `scalafmt-dynamic` if the specified `version` setting doesn't match build version.
     // - `scalafmt-core` if the specified `version` setting match with build version
     //   (or if the `version` is not specified).
-    val versionOpt = options.hoconOpt.map(x => Right(x.version)).getOrElse {
-      Left(s"""error: missing Scalafmt configuration file.
-        |Consider creating '${options.getProposedConfigFile}' with the following:
+    _.version.fold[MaybeRunner] {
+      val where = options.configStr match {
+        case None =>
+          options.canonicalConfigFile
+            .fold(options.getProposedConfigFile)(_.get)
+            .toString
+        case _ => "--config-str option"
+      }
+      Left(s"""error: missing Scalafmt version.
+        |Consider adding the following to $where:
         |${getProposedConfigVersion(options)}
         |""".stripMargin)
-    }
-    versionOpt.flatMap {
-      case None =>
-        val where = options.configStr match {
-          case None =>
-            options.canonicalConfigFile
-              .fold(options.getProposedConfigFile)(_.get)
-              .toString
-          case _ => "--config-str option"
-        }
-        Left(s"""error: missing Scalafmt version.
-          |Consider adding the following to $where:
-          |${getProposedConfigVersion(options)}
-          |""".stripMargin)
-      case Some(`stableVersion`) =>
+    } {
+      case Left(error) =>
+        Left(s"error: invalid configuration: ${error}")
+      case Right(`stableVersion`) =>
         options.common.debug.println(s"Using core runner [$stableVersion]")
         Right(ScalafmtCoreRunner)
-      case Some(v) if isNativeImage =>
+      case Right(v) if isNativeImage =>
         Left(
           s"""error: invalid Scalafmt version.
             |
@@ -141,7 +144,7 @@ object Cli {
             |Scalafmt automatically installs and invokes the correct version of Scalafmt when running on the JVM.
             |""".stripMargin
         )
-      case Some(v) =>
+      case Right(v) =>
         options.common.debug.println(s"Using dynamic runner [$v]")
         Right(ScalafmtDynamicRunner)
     }

diff --git a/scalafmt-cli/src/main/scala/org/scalafmt/cli/CliOptions.scala b/scalafmt-cli/src/main/scala/org/scalafmt/cli/CliOptions.scala
@@ -4,7 +4,7 @@ import java.io.{InputStream, OutputStream, PrintStream}
 import java.nio.file.{Files, NoSuchFileException, Path}
 
 import metaconfig.Configured
-import org.scalafmt.config.{ConfParsed, ScalafmtConfig}
+import org.scalafmt.config.{ConfParsed, ScalafmtConfig, ScalafmtConfigException}
 import org.scalafmt.sysops.{AbsoluteFile, GitOps, OsSpecific}
 
 import scala.io.Codec
@@ -182,10 +182,18 @@ case class CliOptions(
       case _ => filters.mkString("(", "|", ")").r
     }
 
-  private def getHoconValueOpt[A](f: ConfParsed => Option[A]): Option[A] =
-    hoconOpt.flatMap(f)
+  private def getHoconValueOpt[A](
+      f: ConfParsed => Option[Either[String, A]]
+  ): Option[A] =
+    hoconOpt.flatMap(f).map {
+      case Right(x) => x
+      case Left(x) => throw new ScalafmtConfigException(x)
+    }
 
-  private def getHoconValue[A](default: A, f: ConfParsed => Option[A]): A =
+  private def getHoconValue[A](
+      default: A,
+      f: ConfParsed => Option[Either[String, A]]
+  ): A =
     getHoconValueOpt[A](f).getOrElse(default)
 
   private[cli] def isGit: Boolean =

diff --git a/scalafmt-config/shared/src/main/scala/org/scalafmt/config/ConfParsed.scala b/scalafmt-config/shared/src/main/scala/org/scalafmt/config/ConfParsed.scala
@@ -10,33 +10,40 @@ import org.scalafmt.config.PlatformConfig._
 
 class ConfParsed(val conf: Configured[Conf]) extends AnyVal {
 
-  def getHoconValueOpt[A](
+  def getHoconValue[A](
       path: String*
-  )(implicit ev: ConfDecoderEx[A]): Option[A] = {
-    val nestedConf = conf.andThen(_.getNestedConf(path: _*))
-    nestedConf.andThen(ev.read(None, _)).map(Some(_)).getOrElse(None)
-  }
-
-  def getHoconValue[A: ConfDecoderEx](default: A, path: String*): A =
-    getHoconValueOpt[A](path: _*).getOrElse(default)
-
-  def isGit: Option[Boolean] =
-    getHoconValueOpt[Boolean]("project", "git")
-
-  def fatalWarnings: Option[Boolean] =
-    getHoconValueOpt[Boolean]("runner", "fatalWarnings")
-
-  def ignoreWarnings: Option[Boolean] =
-    getHoconValueOpt[Boolean]("runner", "ignoreWarnings")
-
-  def onTestFailure: Option[String] =
-    getHoconValueOpt[String]("onTestFailure")
-
-  def encoding: Option[Codec] =
-    getHoconValueOpt[String]("encoding").flatMap(x => Try(Codec(x)).toOption)
-
-  def version: Option[String] =
-    getHoconValueOpt[String]("version")
+  )(implicit ev: ConfDecoderEx[A]): Option[Either[String, A]] =
+//    conf.andThen(_.getNestedConf(path: _*)).andThen(ev.read(None, _)) match {
+//      case Configured.Ok(x) => Some(Right(x))
+//      case Configured.NotOk(x) =>
+//        if (x.isMissingField) None else Some(Left(x.msg))
+//    }
+    conf
+      .andThen(_.getNestedConf(path: _*))
+      .andThen(ev.read(None, _))
+      .fold[Option[Either[String, A]]](x =>
+        if (x.isMissingField) None else Some(Left(x.msg))
+      )(x => Some(Right(x)))
+
+  def isGit: Option[Either[String, Boolean]] =
+    getHoconValue[Boolean]("project", "git")
+
+  def fatalWarnings: Option[Either[String, Boolean]] =
+    getHoconValue[Boolean]("runner", "fatalWarnings")
+
+  def ignoreWarnings: Option[Either[String, Boolean]] =
+    getHoconValue[Boolean]("runner", "ignoreWarnings")
+
+  def onTestFailure: Option[Either[String, String]] =
+    getHoconValue[String]("onTestFailure")
+
+  def encoding: Option[Either[String, Codec]] =
+    getHoconValue[String]("encoding").map {
+      _.flatMap(x => Try(Codec(x)).toEither.left.map(_.getMessage))
+    }
+
+  def version: Option[Either[String, String]] =
+    getHoconValue[String]("version")
 
 }