Skip to content

Commit

Permalink
ScalafmtDynamic: throw if can't resolve config
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
kitbellew committed Jul 4, 2020
1 parent bd4fa42 commit 257735e
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ final case class ScalafmtDynamic(
codeFormatted
case Left(error) =>
reportError(file, error)
if (error.isInstanceOf[ConfigError]) throw error
code
}
}
Expand Down Expand Up @@ -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)
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =>
Expand Down

0 comments on commit 257735e

Please sign in to comment.