From 257735e57c549d9cfb5f4eef92f177f8b4ac5c60 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 3 Jul 2020 11:48:51 -0700 Subject: [PATCH] ScalafmtDynamic: throw if can't resolve config Intellij invokes the createSession method directly and doesn't expect the null value we return there, so throw ConfigError instead. Similarly, in the Runner, change the implementation of MyInstanceSession to verify the config first, so that the config parsing exception is not thrown for every file we are trying to format. --- .../scalafmt/cli/ScalafmtDynamicRunner.scala | 21 ++++++++++----- .../scalafmt/dynamic/ScalafmtDynamic.scala | 3 ++- .../org/scalafmt/dynamic/DynamicSuite.scala | 27 +++++++++++++------ 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/scalafmt-cli/src/main/scala/org/scalafmt/cli/ScalafmtDynamicRunner.scala b/scalafmt-cli/src/main/scala/org/scalafmt/cli/ScalafmtDynamicRunner.scala index 11f1b4cc25..797e6382b1 100644 --- a/scalafmt-cli/src/main/scala/org/scalafmt/cli/ScalafmtDynamicRunner.scala +++ b/scalafmt-cli/src/main/scala/org/scalafmt/cli/ScalafmtDynamicRunner.scala @@ -8,6 +8,7 @@ import java.util.function.UnaryOperator import org.scalafmt.CompatCollections.ParConverters._ import org.scalafmt.Error.{MisformattedFile, NoMatchingFiles} +import org.scalafmt.dynamic.ScalafmtDynamicError import org.scalafmt.interfaces.Scalafmt import org.scalafmt.interfaces.ScalafmtSession import org.scalafmt.interfaces.ScalafmtSessionFactory @@ -27,13 +28,17 @@ object ScalafmtDynamicRunner extends ScalafmtRunner { .withReporter(reporter) .withRespectProjectFilters(false) - val session = scalafmtInstance match { - case t: ScalafmtSessionFactory => - val session = t.createSession(options.configPath) - if (session == null) return reporter.getExitCode // XXX: returning - session - case _ => new MyInstanceSession(options, scalafmtInstance) - } + val session = + try { + scalafmtInstance match { + case t: ScalafmtSessionFactory => + t.createSession(options.configPath) + case _ => new MyInstanceSession(options, scalafmtInstance) + } + } catch { + case _: ScalafmtDynamicError.ConfigError => + return reporter.getExitCode // XXX: returning + } def sessionMatcher(x: AbsoluteFile): Boolean = session.matchesProjectFilters(x.jfile.toPath) @@ -79,6 +84,8 @@ object ScalafmtDynamicRunner extends ScalafmtRunner { private final class MyInstanceSession(opts: CliOptions, instance: Scalafmt) extends ScalafmtSession { + // check config first + instance.format(opts.configPath, opts.configPath, "") private val customFiles = if (opts.respectProjectFilters) Seq.empty else opts.customFiles.filter(_.jfile.isFile).map(_.jfile.toPath) diff --git a/scalafmt-dynamic/src/main/scala/org/scalafmt/dynamic/ScalafmtDynamic.scala b/scalafmt-dynamic/src/main/scala/org/scalafmt/dynamic/ScalafmtDynamic.scala index be61684f03..97eb691681 100644 --- a/scalafmt-dynamic/src/main/scala/org/scalafmt/dynamic/ScalafmtDynamic.scala +++ b/scalafmt-dynamic/src/main/scala/org/scalafmt/dynamic/ScalafmtDynamic.scala @@ -75,6 +75,7 @@ final case class ScalafmtDynamic( codeFormatted case Left(error) => reportError(file, error) + if (error.isInstanceOf[ConfigError]) throw error code } } @@ -235,7 +236,7 @@ final case class ScalafmtDynamic( override def createSession(config: Path): ScalafmtSession = resolveConfig(config).fold( - error => { reportError(config, error); null }, + error => { reportError(config, error); throw error }, config => new MySession(config) ) diff --git a/scalafmt-tests/src/test/scala/org/scalafmt/dynamic/DynamicSuite.scala b/scalafmt-tests/src/test/scala/org/scalafmt/dynamic/DynamicSuite.scala index 99288d708f..c92d829b03 100644 --- a/scalafmt-tests/src/test/scala/org/scalafmt/dynamic/DynamicSuite.scala +++ b/scalafmt-tests/src/test/scala/org/scalafmt/dynamic/DynamicSuite.scala @@ -11,9 +11,10 @@ import PositionSyntax._ import scala.collection.mutable import scala.collection.mutable.ListBuffer +import scala.reflect.ClassTag import scala.{meta => m} -import org.scalatest.funsuite.AnyFunSuite +import org.scalatest.funsuite.AnyFunSuite import org.scalafmt.util.DiffAssertions class DynamicSuite extends AnyFunSuite with DiffAssertions { @@ -136,11 +137,19 @@ class DynamicSuite extends AnyFunSuite with DiffAssertions { def assertMissingVersion()(implicit pos: Position): Unit = { out.reset() missingVersions.clear() - val original = "object A" - val obtained = dynamic.format(config, filename, original) + intercept[ScalafmtDynamicError.ConfigMissingVersion] { + dynamic.format(config, filename, "object A") + } assert(out.toString().isEmpty) assert(missingVersions.nonEmpty) - assertNoDiff(obtained, original, "Formatter did not error") + } + def assertThrows[A <: AnyRef: ClassTag]( + code: String = "object A { }" + )(implicit pos: Position): A = { + out.reset() + intercept[A] { + dynamic.format(config, filename, code) + } } def assertError(expected: String)(implicit pos: Position): Unit = { assertError("object A { }", expected) @@ -278,7 +287,7 @@ class DynamicSuite extends AnyFunSuite with DiffAssertions { |version=$latest |""".stripMargin ) - f.assertError( + f.assertThrows[ScalafmtDynamicError.ConfigParseError]( """|error: path/.scalafmt.conf: Invalid config: Invalid field: max. Expected one of version, maxColumn, docstrings, optIn, binPack, continuationIndent, align, spaces, literals, lineEndings, rewriteTokens, rewrite, indentOperator, newlines, runner, indentYieldKeyword, importSelectors, unindentTopLevelOperators, includeCurlyBraceInSelectChains, includeNoParensInSelectChains, assumeStandardLibraryStripMargin, danglingParentheses, poorMansTrailingCommasInConfigStyle, trailingCommas, verticalMultilineAtDefinitionSite, verticalMultilineAtDefinitionSiteArityThreshold, verticalMultiline, onTestFailure, encoding, project |""".stripMargin ) @@ -318,7 +327,7 @@ class DynamicSuite extends AnyFunSuite with DiffAssertions { check("wrong-version") { f => f.setVersion("1.0") - f.assertError( + f.assertThrows[ScalafmtDynamicError.CannotDownload]( """|error: path/.scalafmt.conf: org.scalafmt.dynamic.exceptions.ScalafmtException: failed to download v=1.0 |Caused by: org.scalafmt.dynamic.ScalafmtVersion$InvalidVersionException: Invalid scalafmt version 1.0 |""".stripMargin @@ -354,8 +363,10 @@ class DynamicSuite extends AnyFunSuite with DiffAssertions { check("no-config") { f => Files.delete(f.config) - f.assertError("""|error: path/.scalafmt.conf: Missing config - |""".stripMargin) + f.assertThrows[ScalafmtDynamicError.ConfigDoesNotExist]( + """|error: path/.scalafmt.conf: Missing config + |""".stripMargin + ) } check("intellij-default-config") { f: Format =>