From fca7c9b9bb24d9a63bf31cbc7cdf083e3ef8be52 Mon Sep 17 00:00:00 2001 From: Albert Meltzer Date: Thu, 2 Dec 2021 09:06:42 -0800 Subject: [PATCH] Additional settings for error handling --- build.sbt | 2 +- .../org/scalafmt/sbt/ErrorHandling.scala | 7 ++++ .../org/scalafmt/sbt/ScalafmtPlugin.scala | 42 +++++++++++++++---- .../scalafmt/sbt/ScalafmtSbtReporter.scala | 32 ++++++++------ .../scalafmt-sbt/sbt/changes/invalid.scala | 3 ++ plugin/src/sbt-test/scalafmt-sbt/sbt/test | 10 +++++ 6 files changed, 74 insertions(+), 22 deletions(-) create mode 100644 plugin/src/main/scala/org/scalafmt/sbt/ErrorHandling.scala create mode 100644 plugin/src/sbt-test/scalafmt-sbt/sbt/changes/invalid.scala diff --git a/build.sbt b/build.sbt index b203696..b8dd41c 100644 --- a/build.sbt +++ b/build.sbt @@ -34,7 +34,7 @@ inThisBuild( onLoadMessage := s"Welcome to sbt-scalafmt ${version.value}" skip in publish := true -val scalafmtVersion = "3.2.0" +val scalafmtVersion = "3.2.1" lazy val plugin = project .enablePlugins(SbtPlugin) .settings( diff --git a/plugin/src/main/scala/org/scalafmt/sbt/ErrorHandling.scala b/plugin/src/main/scala/org/scalafmt/sbt/ErrorHandling.scala new file mode 100644 index 0000000..4e1f377 --- /dev/null +++ b/plugin/src/main/scala/org/scalafmt/sbt/ErrorHandling.scala @@ -0,0 +1,7 @@ +package org.scalafmt.sbt + +private[sbt] class ErrorHandling( + val logOnEachError: Boolean, + val failOnErrors: Boolean, + val detailedErrorEnabled: Boolean +) diff --git a/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala b/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala index ec9ed45..386de42 100644 --- a/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala +++ b/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtPlugin.scala @@ -71,6 +71,12 @@ object ScalafmtPlugin extends AutoPlugin { val scalafmtFilter = settingKey[String]( "File filtering mode when running scalafmt." ) + val scalafmtLogOnEachError = settingKey[Boolean]( + "Enables logging on an error." + ) + val scalafmtFailOnErrors = settingKey[Boolean]( + "Controls whether to fail in case of formatting errors." + ) } import autoImport._ @@ -127,13 +133,13 @@ object ScalafmtPlugin extends AutoPlugin { resolvers: Seq[Resolver], currentProject: ResolvedProject, filterMode: String, - detailedErrorEnabled: Boolean + errorHandling: ErrorHandling ) { private val log = taskStreams.log private val reporter = new ScalafmtSbtReporter( log, new OutputStreamWriter(taskStreams.binary()), - detailedErrorEnabled + errorHandling ) private val scalafmtSession = { @@ -179,18 +185,28 @@ object ScalafmtPlugin extends AutoPlugin { private def withFormattedSources[T](sources: Seq[File])( onFormat: (File, Input, Output) => T - ): Seq[Option[T]] = - sources.map { file => + ): Seq[Option[T]] = { + val res = sources.map { file => val path = file.toPath.toAbsolutePath Try(IO.read(file)) match { case Failure(x) => reporter.error(path, "Failed to read", x) None case Success(x) => - val output = scalafmtSession.format(path, x) - Some(onFormat(file, x, output)) + val output = scalafmtSession.formatOrError(path, x) + /* no need to report on exception since for all errors + * reporter.error would have been called already */ + Option(output.value).map(o => onFormat(file, x, o)) } } + val bad = res.count(_ eq None) + if (bad != 0) { + val err = s"scalafmt: failed for $bad sources" + if (errorHandling.failOnErrors) throw new MessageOnlyException(err) + log.error(err) + } + res + } def formatTrackedSources( cacheStoreFactory: CacheStoreFactory, @@ -413,7 +429,11 @@ object ScalafmtPlugin extends AutoPlugin { fullResolvers.value, thisProject.value, scalafmtFilter.value, - scalafmtDetailedError.value + new ErrorHandling( + scalafmtLogOnEachError.value, + scalafmtFailOnErrors.value, + scalafmtDetailedError.value + ) ) func(files, session) } @@ -453,7 +473,11 @@ object ScalafmtPlugin extends AutoPlugin { fullResolvers.value, thisProject.value, "", - scalafmtDetailedError.value + new ErrorHandling( + scalafmtLogOnEachError.value, + scalafmtFailOnErrors.value, + scalafmtDetailedError.value + ) ).formatSources(absFiles) } ) @@ -480,6 +504,8 @@ object ScalafmtPlugin extends AutoPlugin { Seq( scalafmtFilter := "", scalafmtOnCompile := false, + scalafmtLogOnEachError := false, + scalafmtFailOnErrors := true, scalafmtDetailedError := false ) diff --git a/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtSbtReporter.scala b/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtSbtReporter.scala index effee5d..048d13a 100644 --- a/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtSbtReporter.scala +++ b/plugin/src/main/scala/org/scalafmt/sbt/ScalafmtSbtReporter.scala @@ -12,7 +12,7 @@ import org.scalafmt.interfaces.ScalafmtReporter class ScalafmtSbtReporter( log: Logger, out: OutputStreamWriter, - detailedErrorEnabled: Boolean + errorHandling: ErrorHandling ) extends ScalafmtReporter { import ScalafmtSbtReporter._ @@ -23,25 +23,31 @@ class ScalafmtSbtReporter( error(file, null, e) override def error(file: Path, message: String, e: Throwable): Unit = { - def getMessage() = { + def getMessage(toThrow: Boolean) = { val res = new StringWriter() - res.write("scalafmt: ") - res.write(Option(message).getOrElse("failed")) + if (toThrow) res.write("scalafmt: ") + val nestedMessage = if (e == null) None else Option(e.getMessage) + val messageOpt = Option(message).orElse(nestedMessage) + res.write(messageOpt.getOrElse("failed")) res.write(" [") res.write(file.toString) res.write(']') - if (null != e) { - if (!detailedErrorEnabled) - Option(e.getMessage).foreach { x => - res.write(" ") - res.write(x) - } + if (null != e && !toThrow) { + if (errorHandling.detailedErrorEnabled) + e.printStackTrace(new PrintWriter(res)) + else if (messageOpt ne nestedMessage) nestedMessage.foreach { x => + res.write(": ") + res.write(x) + } } res.toString } - val cause = if (detailedErrorEnabled) e else null - throw new ScalafmtSbtError(getMessage(), cause) + if (errorHandling.logOnEachError) log.error(getMessage(false)) + else if (errorHandling.failOnErrors) { + val cause = if (errorHandling.detailedErrorEnabled) e else null + throw new ScalafmtSbtError(getMessage(true), cause) + } } override def excluded(file: Path): Unit = @@ -57,6 +63,6 @@ class ScalafmtSbtReporter( object ScalafmtSbtReporter { private class ScalafmtSbtError(message: String, cause: Throwable) - extends RuntimeException(message, cause) + extends RuntimeException(message, cause, true, cause != null) } diff --git a/plugin/src/sbt-test/scalafmt-sbt/sbt/changes/invalid.scala b/plugin/src/sbt-test/scalafmt-sbt/sbt/changes/invalid.scala new file mode 100644 index 0000000..9fa2147 --- /dev/null +++ b/plugin/src/sbt-test/scalafmt-sbt/sbt/changes/invalid.scala @@ -0,0 +1,3 @@ +object Test { + foo(a, b // ) invalid +} diff --git a/plugin/src/sbt-test/scalafmt-sbt/sbt/test b/plugin/src/sbt-test/scalafmt-sbt/sbt/test index cd4ee6f..df4ef4c 100644 --- a/plugin/src/sbt-test/scalafmt-sbt/sbt/test +++ b/plugin/src/sbt-test/scalafmt-sbt/sbt/test @@ -176,6 +176,16 @@ $ exec git -C p17 add "src/main/scala/TestBad2.scala" # now commit it, no longer modified $ exec git -C p17 commit -m 'added TestBad2.scala' > p17/scalafmtCheck +# don't filter but fail after all errors +> set p17/scalafmtFilter := ("") +$ copy-file changes/invalid.scala p17/src/main/scala/TestInvalid1.scala +$ copy-file changes/invalid.scala p17/src/main/scala/TestInvalid2.scala +-> p17/scalafmt +# fail after all errors +> set p17/scalafmtLogOnEachError := true +-> p17/scalafmt +> set p17/scalafmtFailOnErrors := false +> p17/scalafmt $ copy-file changes/target/managed.scala project/target/managed.scala $ copy-file changes/x/Something.scala project/x/Something.scala