From d47b381a87f30a82fc031acf2901ab3b61abdd4a Mon Sep 17 00:00:00 2001 From: Meriam Lachkar Date: Thu, 4 Mar 2021 22:51:48 +0100 Subject: [PATCH] Only add patches to files with errors in Scala 3 To avoid computing unecessary patches --- .gitignore | 1 + .../interfaces/FileWithErrorReporter.java | 23 ++++++++++++ .../compiler/interfaces/QuietReporter.java | 3 +- .../compiler/interfaces/Scala3Compiler.java | 11 ++++++ .../src/main/scala/migrate/ScalaMigrat.scala | 37 ++++++++++++++----- .../test/scala/migrate/MigrationSuite.scala | 13 ++++--- 6 files changed, 73 insertions(+), 15 deletions(-) create mode 100644 interfaces/compiler/src/main/java/compiler/interfaces/FileWithErrorReporter.java diff --git a/.gitignore b/.gitignore index 53c0ec86..cbf2889c 100644 --- a/.gitignore +++ b/.gitignore @@ -28,3 +28,4 @@ out/ .metals/ .vscode/ project/metals.sbt +project/project/metals.sbt diff --git a/interfaces/compiler/src/main/java/compiler/interfaces/FileWithErrorReporter.java b/interfaces/compiler/src/main/java/compiler/interfaces/FileWithErrorReporter.java new file mode 100644 index 00000000..d8eaffad --- /dev/null +++ b/interfaces/compiler/src/main/java/compiler/interfaces/FileWithErrorReporter.java @@ -0,0 +1,23 @@ +package compiler.interfaces; + +import dotty.tools.dotc.core.Contexts.Context; +import dotty.tools.dotc.reporting.Diagnostic; +import dotty.tools.dotc.reporting.Reporter; + +import java.util.HashSet; + +// we want to get the paths of all files that have errors +public class FileWithErrorReporter extends Reporter { + HashSet filesWithErrors = new HashSet(); + + public void doReport(Diagnostic dia, Context ctx) { + if (dia.level() == Diagnostic.ERROR) { + String filePath = dia.pos().source().path(); + filesWithErrors.add(filePath); + } + } + + public String[] getFilesWithErrors() { + return filesWithErrors.stream().toArray(String[]::new); + } +} diff --git a/interfaces/compiler/src/main/java/compiler/interfaces/QuietReporter.java b/interfaces/compiler/src/main/java/compiler/interfaces/QuietReporter.java index 95c72108..cd273d1e 100644 --- a/interfaces/compiler/src/main/java/compiler/interfaces/QuietReporter.java +++ b/interfaces/compiler/src/main/java/compiler/interfaces/QuietReporter.java @@ -3,7 +3,8 @@ import dotty.tools.dotc.core.Contexts.Context; import dotty.tools.dotc.reporting.Diagnostic; import dotty.tools.dotc.reporting.Reporter; +import java.util.*; public class QuietReporter extends Reporter { - public void doReport(Diagnostic d, Context ctx) {} + public void doReport(Diagnostic dia, Context ctx) {} } diff --git a/interfaces/compiler/src/main/java/compiler/interfaces/Scala3Compiler.java b/interfaces/compiler/src/main/java/compiler/interfaces/Scala3Compiler.java index 27fe15e3..07f99478 100644 --- a/interfaces/compiler/src/main/java/compiler/interfaces/Scala3Compiler.java +++ b/interfaces/compiler/src/main/java/compiler/interfaces/Scala3Compiler.java @@ -10,6 +10,8 @@ import scala.collection.immutable.List; import scala.runtime.AbstractFunction1; +import java.util.HashSet; + public class Scala3Compiler { private Compiler compiler; private Context context; @@ -50,4 +52,13 @@ public void compileAndReport(List units, Logger r) throws Compi throw new CompilationException(reporter.allErrors().mkString("\n")); } } + + public String[] compileAndReportFilesWithErrors(List units) { + List sources = units.map(toSourceFile); + FileWithErrorReporter reporter = new FileWithErrorReporter(); + Context freshContext = context.fresh().setReporter(reporter); + Run run = compiler.newRun(freshContext); + run.compileSources(sources); + return reporter.getFilesWithErrors(); + } } diff --git a/migrate/src/main/scala/migrate/ScalaMigrat.scala b/migrate/src/main/scala/migrate/ScalaMigrat.scala index 97c2d5df..40be0dfa 100644 --- a/migrate/src/main/scala/migrate/ScalaMigrat.scala +++ b/migrate/src/main/scala/migrate/ScalaMigrat.scala @@ -30,13 +30,23 @@ class ScalaMigrat(scalafixSrv: ScalafixService) { for { compiler <- setupScala3Compiler(scala3Classpath, scala3ClassDirectory, scala3CompilerOptions) (scalaFiles, javaFiles) = unmanagedSources.partition(_.value.endsWith("scala")) - initialFileToMigrate <- - buildMigrationFiles(scalaFiles) - _ <- compileInScala3(initialFileToMigrate, javaFiles ++ managedSources, compiler) - initialFileToMigrate <- - buildMigrationFiles(unmanagedSources) - _ <- compileInScala3(initialFileToMigrate, managedSources, compiler) - migratedFiles = initialFileToMigrate.map(f => (f.source, f.migrate(compiler))).toMap + // first try to compile without adding any patch + filesWithErr = compileInScala3AndGetFilesWithErrors(unmanagedSources ++ managedSources, compiler) + migratedFiles <- if (filesWithErr.isEmpty) { + scribe.info("The project compiles successfully in Scala 3") + Success(Map[AbsolutePath, FileMigrationState.FinalState]()) + } else { + val filesWithoutErrors = scalaFiles.diff(filesWithErr) + for { + initialFileToMigrate <- buildMigrationFiles(filesWithErr) + _ <- compileInScala3( + initialFileToMigrate, + filesWithoutErrors ++ javaFiles ++ managedSources, + compiler + ) + migratedFiles = initialFileToMigrate.map(f => (f.source, f.migrate(compiler))).toMap + } yield migratedFiles + } } yield migratedFiles } @@ -100,16 +110,24 @@ class ScalaMigrat(scalafixSrv: ScalafixService) { for { cuUnmanagedSources <- migrationFiles.map(_.previewAllPatches()).sequence cuManagedSources = managedSources.map(path => new CompilationUnit(path.value, FileUtils.read(path))) - _ <- timeAndLog(Try(compiler.compileAndReport(cuManagedSources.toList ++ cuUnmanagedSources.toList, reporter))) { + _ <- timeAndLog(Try(compiler.compileAndReport((cuUnmanagedSources ++ cuManagedSources).toList, reporter))) { case (finiteDuration, Success(_)) => scribe.info(s"Successfully compiled with scala 3 in $finiteDuration") case (_, Failure(e)) => scribe.info(s"""|Compilation with scala 3 failed. |Please fix the errors above.""".stripMargin) } - } yield () + private def compileInScala3AndGetFilesWithErrors( + files: Seq[AbsolutePath], + compiler: Scala3Compiler + ): Seq[AbsolutePath] = { + val compilationUnits = files.map(path => new CompilationUnit(path.value, FileUtils.read(path))) + val res = compiler.compileAndReportFilesWithErrors(compilationUnits.toList).toSeq + res.map(AbsolutePath.from(_)).sequence.getOrElse(Nil) + } + private def buildMigrationFiles(unmanagedSources: Seq[AbsolutePath]): Try[Seq[FileMigrationState.Initial]] = if (unmanagedSources.isEmpty) Success(Seq()) else @@ -137,6 +155,7 @@ class ScalaMigrat(scalafixSrv: ScalafixService) { } yield fileToMigrate } + object ScalaMigrat { def migrateScalacOptions(scalacOptions: Seq[String]): MigratedScalacOptions = { val sanitized = ScalacOption.sanitizeScalacOption(scalacOptions) diff --git a/migrate/src/test/scala/migrate/MigrationSuite.scala b/migrate/src/test/scala/migrate/MigrationSuite.scala index 65373049..062acc51 100644 --- a/migrate/src/test/scala/migrate/MigrationSuite.scala +++ b/migrate/src/test/scala/migrate/MigrationSuite.scala @@ -1,12 +1,12 @@ package migrate import scala.util.Failure +import scala.util.Success import scala.util.Try import migrate.Values._ import migrate.internal.FileMigrationState import migrate.utils.FileUtils._ -import migrate.utils.ScalaExtensions._ import org.scalatest.funsuite.AnyFunSuiteLike import scalafix.testkit.DiffAssertions @@ -38,9 +38,12 @@ class MigrationSuite extends AnyFunSuiteLike with DiffAssertions { filetoMigrate: AbsolutePath, migratedFiles: Map[AbsolutePath, FileMigrationState.FinalState] ): Try[String] = - migratedFiles.get(filetoMigrate).toTry(new Exception(s"Cannot find $filetoMigrate")).flatMap { - case FileMigrationState.Failed(_, cause) => Failure(cause) - case f: FileMigrationState.Succeeded => f.newFileContent - } + migratedFiles + .get(filetoMigrate) + .map { + case FileMigrationState.Failed(_, cause) => Failure(cause) + case f: FileMigrationState.Succeeded => f.newFileContent + } + .getOrElse(Success(read(filetoMigrate))) }