-
Notifications
You must be signed in to change notification settings - Fork 939
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix unstable existential type names bug.
Fix the problem with unstable names synthesized for existential types (declared with underscore syntax) by renaming type variables to a scheme that is guaranteed to be stable no matter where given the existential type appears. The sheme we use are De Bruijn-like indices that capture both position of type variable declarion within single existential type and nesting level of nested existential type. This way we properly support nested existential types by avoiding name clashes. In general, we can perform renamings like that because type variables declared in existential types are scoped to those types so the renaming operation is local. There's a specs2 unit test covering instability of existential types. The test is included in compiler-interface project and the build definition has been modified to enable building and executing tests in compiler-interface project. Some dependencies has been modified: * compiler-interface project depends on api project for testing (test makes us of SameAPI) * dependency on junit has been introduced because it's needed for `@RunWith` annotation which declares that specs2 unit test should be ran with JUnitRunner SameAPI has been modified to expose a method that allows us to compare two definitions. This commit also adds `ScalaCompilerForUnitTesting` class that allows to compile a piece of Scala code and inspect information recorded callbacks defined in `AnalysisCallback` interface. That class uses existing ConsoleLogger for logging. I considered doing the same for ConsoleReporter. There's LoggingReporter defined which would fit our usecase but it's defined in compile subproject that compiler-interface doesn't depend on so we roll our own. ScalaCompilerForUnit testing uses TestCallback from compiler-interface subproject for recording information passed to callbacks. In order to be able to access TestCallback from compiler-interface subproject I had to tweak dependencies between interface and compiler-interface so test classes from the former are visible in the latter. I also modified the TestCallback itself to accumulate apis in a HashMap instead of a buffer of tuples for easier lookup. An integration test has been added which tests scenario mentioned in #823. This commit fixes #823. Conflicts: project/Util.scala
- Loading branch information
1 parent
7acf567
commit 55110a2
Showing
12 changed files
with
282 additions
and
14 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
42 changes: 42 additions & 0 deletions
42
compile/interface/src/test/scala/xsbt/ExtractAPISpecification.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package xsbt | ||
|
||
import org.junit.runner.RunWith | ||
import xsbti.api.ClassLike | ||
import xsbti.api.Def | ||
import xsbt.api.SameAPI | ||
import org.specs2.mutable.Specification | ||
import org.specs2.runner.JUnitRunner | ||
|
||
@RunWith(classOf[JUnitRunner]) | ||
class ExtractAPISpecification extends Specification { | ||
|
||
"Existential types in method signatures" should { | ||
"have stable names" in { stableExistentialNames } | ||
} | ||
|
||
def stableExistentialNames: Boolean = { | ||
def compileAndGetFooMethodApi(src: String): Def = { | ||
val compilerForTesting = new ScalaCompilerForUnitTesting | ||
val sourceApi = compilerForTesting.compileSrc(src) | ||
val FooApi = sourceApi.definitions().find(_.name() == "Foo").get.asInstanceOf[ClassLike] | ||
val fooMethodApi = FooApi.structure().declared().find(_.name == "foo").get | ||
fooMethodApi.asInstanceOf[Def] | ||
} | ||
val src1 = """ | ||
|class Box[T] | ||
|class Foo { | ||
| def foo: Box[_] = null | ||
| | ||
}""".stripMargin | ||
val fooMethodApi1 = compileAndGetFooMethodApi(src1) | ||
val src2 = """ | ||
|class Box[T] | ||
|class Foo { | ||
| def bar: Box[_] = null | ||
| def foo: Box[_] = null | ||
| | ||
}""".stripMargin | ||
val fooMethodApi2 = compileAndGetFooMethodApi(src2) | ||
SameAPI.apply(fooMethodApi1, fooMethodApi2) | ||
} | ||
} |
71 changes: 71 additions & 0 deletions
71
compile/interface/src/test/scala/xsbt/ScalaCompilerForUnitTesting.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
package xsbt | ||
|
||
import xsbti.compile.SingleOutput | ||
import java.io.File | ||
import _root_.scala.tools.nsc.reporters.ConsoleReporter | ||
import _root_.scala.tools.nsc.Settings | ||
import xsbti._ | ||
import xsbti.api.SourceAPI | ||
import sbt.IO.withTemporaryDirectory | ||
import xsbti.api.ClassLike | ||
import xsbti.api.Definition | ||
import xsbti.api.Def | ||
import xsbt.api.SameAPI | ||
import sbt.ConsoleLogger | ||
|
||
/** | ||
* Provides common functionality needed for unit tests that require compiling | ||
* source code using Scala compiler. | ||
*/ | ||
class ScalaCompilerForUnitTesting { | ||
|
||
/** | ||
* Compiles given source code using Scala compiler and returns API representation | ||
* extracted by ExtractAPI class. | ||
*/ | ||
def compileSrc(src: String): SourceAPI = { | ||
import java.io.FileWriter | ||
withTemporaryDirectory { temp => | ||
val analysisCallback = new TestCallback | ||
val classesDir = new File(temp, "classes") | ||
classesDir.mkdir() | ||
val compiler = prepareCompiler(classesDir, analysisCallback) | ||
val run = new compiler.Run | ||
val srcFile = new File(temp, "Test.scala") | ||
srcFile.createNewFile() | ||
val fw = new FileWriter(srcFile) | ||
fw.write(src) | ||
fw.close() | ||
run.compile(List(srcFile.getAbsolutePath())) | ||
analysisCallback.apis(srcFile) | ||
} | ||
} | ||
|
||
private def prepareCompiler(outputDir: File, analysisCallback: AnalysisCallback): CachedCompiler0#Compiler = { | ||
val args = Array.empty[String] | ||
object output extends SingleOutput { | ||
def outputDirectory: File = outputDir | ||
} | ||
val weakLog = new WeakLog(ConsoleLogger(), ConsoleReporter) | ||
val cachedCompiler = new CachedCompiler0(args, output, weakLog, false) | ||
val settings = cachedCompiler.settings | ||
settings.usejavacp.value = true | ||
val scalaReporter = new ConsoleReporter(settings) | ||
val delegatingReporter = DelegatingReporter(settings, ConsoleReporter) | ||
val compiler = cachedCompiler.compiler | ||
compiler.set(analysisCallback, delegatingReporter) | ||
compiler | ||
} | ||
|
||
private object ConsoleReporter extends Reporter { | ||
def reset(): Unit = () | ||
def hasErrors: Boolean = false | ||
def hasWarnings: Boolean = false | ||
def printWarnings(): Unit = () | ||
def problems: Array[Problem] = Array.empty | ||
def log(pos: Position, msg: String, sev: Severity): Unit = println(msg) | ||
def comment(pos: Position, msg: String): Unit = () | ||
def printSummary(): Unit = () | ||
} | ||
|
||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,19 +1,23 @@ | ||
package xsbti | ||
|
||
import java.io.File | ||
import scala.collection.mutable.ArrayBuffer | ||
import java.io.File | ||
import scala.collection.mutable.ArrayBuffer | ||
import xsbti.api.SourceAPI | ||
|
||
class TestCallback extends AnalysisCallback | ||
{ | ||
val sourceDependencies = new ArrayBuffer[(File, File, Boolean)] | ||
val binaryDependencies = new ArrayBuffer[(File, String, File, Boolean)] | ||
val products = new ArrayBuffer[(File, File, String)] | ||
val apis = new ArrayBuffer[(File, xsbti.api.SourceAPI)] | ||
val apis: scala.collection.mutable.Map[File, SourceAPI] = scala.collection.mutable.Map.empty | ||
|
||
def sourceDependency(dependsOn: File, source: File, inherited: Boolean) { sourceDependencies += ((dependsOn, source, inherited)) } | ||
def binaryDependency(binary: File, name: String, source: File, inherited: Boolean) { binaryDependencies += ((binary, name, source, inherited)) } | ||
def generatedClass(source: File, module: File, name: String) { products += ((source, module, name)) } | ||
|
||
def api(source: File, sourceAPI: xsbti.api.SourceAPI) { apis += ((source, sourceAPI)) } | ||
def api(source: File, sourceAPI: SourceAPI): Unit = { | ||
assert(!apis.contains(source), s"The `api` method should be called once per source file: $source") | ||
apis(source) = sourceAPI | ||
} | ||
def problem(category: String, pos: xsbti.Position, message: String, severity: xsbti.Severity, reported: Boolean) {} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
// checks number of compilation iterations performed since last `clean` run | ||
InputKey[Unit]("check-number-of-compiler-iterations") <<= inputTask { (argTask: TaskKey[Seq[String]]) => | ||
(argTask, compile in Compile) map { (args: Seq[String], a: sbt.inc.Analysis) => | ||
assert(args.size == 1) | ||
val expectedIterationsNumber = args(0).toInt | ||
val allCompilationsSize = a.compilations.allCompilations.size | ||
assert(allCompilationsSize == expectedIterationsNumber, | ||
"allCompilationsSize == %d (expected %d)".format(allCompilationsSize, expectedIterationsNumber)) | ||
} | ||
} |
13 changes: 13 additions & 0 deletions
13
sbt/src/sbt-test/api/unstable-existential-names/changes/Foo1.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package test | ||
|
||
class Box[T] | ||
|
||
class Foo { | ||
/** | ||
* This method shouldn't affect public API of Foo | ||
* but due to instability of synthesized names for | ||
* existentials causes change of `foo` method API. | ||
*/ | ||
private def abc: Box[_] = null | ||
def foo: Box[_] = null | ||
} |
5 changes: 5 additions & 0 deletions
5
sbt/src/sbt-test/api/unstable-existential-names/src/main/scala/Bar.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
package test | ||
|
||
class Bar { | ||
def f: Foo = null //we introduce dependency on Foo | ||
} |
7 changes: 7 additions & 0 deletions
7
sbt/src/sbt-test/api/unstable-existential-names/src/main/scala/Foo.scala
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
package test | ||
|
||
class Box[T] | ||
|
||
class Foo { | ||
def foo: Box[_] = null | ||
} |
Oops, something went wrong.