Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add patches only to files having error in scala 3 #120

Merged
merged 1 commit into from
Mar 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ out/
.metals/
.vscode/
project/metals.sbt
project/project/metals.sbt
Original file line number Diff line number Diff line change
@@ -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<String> filesWithErrors = new HashSet<String>();

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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -50,4 +52,13 @@ public void compileAndReport(List<CompilationUnit> units, Logger r) throws Compi
throw new CompilationException(reporter.allErrors().mkString("\n"));
}
}

public String[] compileAndReportFilesWithErrors(List<CompilationUnit> units) {
List<SourceFile> 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();
}
}
37 changes: 28 additions & 9 deletions migrate/src/main/scala/migrate/ScalaMigrat.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Comment on lines +41 to +46
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should fix the compilation errors in the filesWithErr but there may be cases where the compiler then find errors in others files, in which case we can loop to add the patches of the new filesWithErr until there is no more error.

Can be done in a V2 after we have found a case in which it happens.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. I have to open an issue to get track of this.

migratedFiles = initialFileToMigrate.map(f => (f.source, f.migrate(compiler))).toMap
} yield migratedFiles
}
} yield migratedFiles
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -137,6 +155,7 @@ class ScalaMigrat(scalafixSrv: ScalafixService) {
} yield fileToMigrate

}

object ScalaMigrat {
def migrateScalacOptions(scalacOptions: Seq[String]): MigratedScalacOptions = {
val sanitized = ScalacOption.sanitizeScalacOption(scalacOptions)
Expand Down
13 changes: 8 additions & 5 deletions migrate/src/test/scala/migrate/MigrationSuite.scala
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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)))

}