Cli: make sure version is explicitly specified

scalameta · Nov 9, 2021 · 2ba8ab4 · 2ba8ab4
1 parent 3be3508
commit 2ba8ab4
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 25 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
@@ -3,7 +3,7 @@ package org.scalafmt.cli
 import com.martiansoftware.nailgun.NGContext
 import java.nio.file.{Files, Paths}
 
-import org.scalafmt.Versions
+import org.scalafmt.Versions.{stable => stableVersion}
 import org.scalafmt.sysops.AbsoluteFile
 
 import scala.io.Source
@@ -105,22 +105,23 @@ object Cli {
     // - `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).
-    options.getVersionIfDifferent match {
+    options.getVersionOpt match {
+      case None => Left(s"error: missing version (current $stableVersion)")
+      case Some(`stableVersion`) => Right(ScalafmtCoreRunner)
       case Some(v) if isNativeImage =>
         Left(
           s"""error: invalid Scalafmt version.
             |
-            |This Scalafmt installation has version '${Versions.version}' and the version configured in '${options.configPath}' is '${v}'.
+            |This Scalafmt installation has version '$stableVersion' and the version configured in '${options.configPath}' is '${v}'.
             |To fix this problem, add the following line to .scalafmt.conf:
             |```
-            |version = '${Versions.version}'
+            |version = '$stableVersion'
             |```
             |
             |NOTE: this error happens only when running a native Scalafmt binary.
             |Scalafmt automatically installs and invokes the correct version of Scalafmt when running on the JVM.
             |""".stripMargin
         )
-      case None => Right(ScalafmtCoreRunner)
       case _ => 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,6 @@ import java.io.{InputStream, OutputStream, PrintStream}
 import java.nio.file.{Files, NoSuchFileException, Path}
 
 import metaconfig.{Conf, ConfDecoderEx, ConfDynamic, Configured}
-import org.scalafmt.Versions
 import org.scalafmt.config.{Config, ScalafmtConfig}
 import org.scalafmt.sysops.{AbsoluteFile, GitOps, OsSpecific}
 
@@ -212,7 +211,7 @@ case class CliOptions(
 
   /** Returns None if .scalafmt.conf is not found or version setting is missing.
     */
-  private[cli] def getVersionIfDifferent: Option[String] =
-    getHoconValueOpt[String]("version").filter(_ != Versions.version)
+  private[cli] def getVersionOpt: Option[String] =
+    getHoconValueOpt[String]("version")
 
 }
diff --git a/scalafmt-tests/src/test/scala/org/scalafmt/cli/CliTest.scala b/scalafmt-tests/src/test/scala/org/scalafmt/cli/CliTest.scala
@@ -12,7 +12,7 @@ import java.nio.file.{Files, Path}
 import scala.collection.JavaConverters._
 import munit.FunSuite
 import org.scalafmt.Error.NoMatchingFiles
-import org.scalafmt.Versions
+import org.scalafmt.Versions.{stable => stableVersion}
 import org.scalafmt.cli.FileTestOps._
 import org.scalafmt.config.{Config, ProjectFiles, ScalafmtConfig}
 import org.scalafmt.sysops.{AbsoluteFile, FileOps}
@@ -103,9 +103,9 @@ abstract class AbstractCliTest extends FunSuite {
 
 trait CliTestBehavior { this: AbstractCliTest =>
   def testCli(version: String) {
-    val label = if (version == Versions.version) "core" else "dynamic"
-    val dialectError =
-      if (version == Versions.version) " [dialect default]" else ""
+    val isCore = version == stableVersion
+    val label = if (isCore) "core" else "dynamic"
+    val dialectError = if (isCore) " [dialect default]" else ""
     test(s"scalafmt tmpFile tmpFile2: $label") {
       val originalTmpFile = Files.createTempFile("prefix", ".scala")
       val originalTmpFile2 = Files.createTempFile("prefix2", ".scala")
@@ -739,7 +739,8 @@ trait CliTestBehavior { this: AbstractCliTest =>
     test(s"eof: $label") {
       val in = Files.createTempFile("scalafmt", "Foo.scala")
       Files.write(in, "object A".getBytes(StandardCharsets.UTF_8))
-      val exit = Cli.mainWithOptions(Array(in.toString), baseCliOptions)
+      val args = Array("--config-str", s"""{version="$version"}""", in.toString)
+      val exit = Cli.mainWithOptions(args, baseCliOptions)
       assert(exit.isOk)
       val obtained = new String(Files.readAllBytes(in), StandardCharsets.UTF_8)
       assert(obtained == "object A\n")
@@ -812,11 +813,9 @@ trait CliTestBehavior { this: AbstractCliTest =>
 
 class CliTest extends AbstractCliTest with CliTestBehavior {
   testCli("1.6.0-RC4") // test for runDynamic
-  testCli(Versions.version) // test for runScalafmt
+  testCli(stableVersion) // test for runScalafmt
 
-  test(
-    "Running pre-resolved version of scalafmt if .scalafmt.conf is missing."
-  ) {
+  test("Fail if .scalafmt.conf is missing.") {
     val input =
       s"""|/foo.scala
         |object A {}
@@ -829,16 +828,16 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
           "--debug"
         ) // debug options is needed to output running scalafmt version
       ),
-      assertExit = { exit => assert(exit.isOk, exit) },
+      assertExit = { x => assertEquals(x, ExitCode.UnsupportedVersion, x) },
       assertOut = out => {
-        assert(out.contains(Versions.version))
+        val eol = System.lineSeparator()
+        val msg = s"error: missing version (current $stableVersion)$eol"
+        assertEquals(out, msg + msg) // two invocations
       }
     )
   }
 
-  test(
-    "Running pre-resolved version of scalafmt if `version` setting is missing."
-  ) {
+  test("Fail if `version` setting is missing.") {
     val input =
       s"""|/.scalafmt.conf
         |maxColumn = 10
@@ -854,9 +853,11 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
           "--debug"
         ) // debug options is needed to output running scalafmt version
       ),
-      assertExit = { exit => assert(exit.isOk, exit) },
+      assertExit = { x => assertEquals(x, ExitCode.UnsupportedVersion, x) },
       assertOut = out => {
-        assert(out.contains(Versions.version))
+        val eol = System.lineSeparator()
+        val msg = s"error: missing version (current $stableVersion)$eol"
+        assertEquals(out, msg + msg) // two invocations
       }
     )
   }
@@ -903,13 +904,15 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
         |object A   { }
         |
         |/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn = 2
         |project { includePaths = ["glob:**/bar.scala"] }
         |""".stripMargin
     )
 
     val expected =
       s"""|/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn = 2
         |project { includePaths = ["glob:**/bar.scala"] }
         |
@@ -940,13 +943,15 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
         |object A   { }
         |
         |/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn = 2
         |project { excludePaths = ["glob:**target**"] }
         |""".stripMargin
     )
 
     val expected =
       s"""|/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn = 2
         |project { excludePaths = ["glob:**target**"] }
         |
@@ -977,13 +982,15 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
         |object A   { }
         |
         |/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn = 2
         |project { includeFilters = ["bar"] }
         |""".stripMargin
     )
 
     val expected =
       s"""|/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn = 2
         |project { includeFilters = ["bar"] }
         |
@@ -1016,6 +1023,7 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
         |object A   { }
         |
         |/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn = 2
         |project {
         |  includePaths = $defaultIncludePathsJson
@@ -1026,6 +1034,7 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
 
     val expected =
       s"""|/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn = 2
         |project {
         |  includePaths = $defaultIncludePathsJson
@@ -1061,13 +1070,15 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
         |val  x   =   42
         |```
         |/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn   = 8
         |project.includePaths."+" = ["glob:**.md"]
         |""".stripMargin
     )
     val expected =
       s"""|
         |/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn   = 8
         |project.includePaths."+" = ["glob:**.md"]
         |
@@ -1100,11 +1111,13 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
         |val  x   =   42
         |```
         |/.scalafmt.conf
+        |version = $stableVersion
         |""".stripMargin
     )
     val expected =
       s"""|
         |/.scalafmt.conf
+        |version = $stableVersion
         |
         |/foobar.md
         |# Hello
@@ -1160,7 +1173,7 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
       Array(
         input.toString(),
         "--config-str",
-        s"""{project.includePaths."+" = ["glob:**.md"]}"""
+        s"""{version = "$stableVersion", project.includePaths."+" = ["glob:**.md"]}"""
       )
     )
     Cli.run(options)
@@ -1249,13 +1262,15 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
         |val  x   =   42
         |```
         |/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn   = 8
         |project.includePaths."+" = ["glob:**.md"]
         |""".stripMargin
     )
     val expected =
       s"""|
         |/.scalafmt.conf
+        |version = $stableVersion
         |maxColumn   = 8
         |project.includePaths."+" = ["glob:**.md"]
         |
@@ -1291,6 +1306,7 @@ class CliTest extends AbstractCliTest with CliTestBehavior {
       Console.withErr(err) {
         val options = getConfig(Array("--stdin", "--test"))
         val options2 = options.copy(
+          configStr = Some(s"{version = $stableVersion}"),
           common = options.common.copy(
             in = new ByteArrayInputStream(codeNoEol.getBytes),
             out = Console.out,