Permalink
Browse files

SI-7548 askTypeAt returns the same type whether the source was fully …

…or targeted type-checked

When asking for targeted typecheck, the located tree may have overloaded types
is the source isn't yet fully typechecked (e.g., a select tree for an
overloaded method). This is problematic as it can lead to unknown 'hovers',
broken hyperlinking that suddenly starts working, unresolved ScalaDoc comments,
and similar, in the Scala IDE.

With this commit we are hardening the contract of `askTypeAt` to return the
same type whether the file was fully type-checked or targeted type-checked.
This is done by preventing the typechecker to stop too early if the `located`
tree has an overloaded type. Furthermore, I'm assuming that if `located.tpe`
is of type `OverloadedType`, by letting the compiler carry-on the typechecking,
the `located.tpe` will eventually be resolved to a non-overloaded type. Said
otherwise, I expect the targeted typechecking will always terminate (if my
reasoning isn't sound, please say so).

The test provided with this commit demonstrates the new behavior (the position
used to execute the test is resolved to the `foo` method's call). In fact,
before this commit, executing the test returned the following:

    (x: Int, y: String)Unit <and> (x: String)Unit <and> (x: Int)Unit

Showing that the tree's type is an overloaded type. The ambiguity is fixed by
this commit, and in fact the test's output is now:

    (x: Int)Unit
  • Loading branch information...
1 parent da73950 commit b7509c922f78624a9eba88c3c64054e0d217ecea @dotta dotta committed Nov 22, 2013
@@ -247,16 +247,21 @@ class Global(settings: Settings, _reporter: Reporter, projectName: String = "")
println("something's wrong: no "+context.unit+" in "+result+result.pos)
located = result
}
- throw new TyperResult(located)
+ located.tpe match {
+ case _: OverloadedType => ()
+ case _ => throw new TyperResult(located)
+ }
}
- try {
- checkForMoreWork(old.pos)
- } catch {
- case ex: ValidateException => // Ignore, this will have been reported elsewhere
- debugLog("validate exception caught: "+ex)
- case ex: Throwable =>
- log.flush()
- throw ex
+ else {
+ try {
+ checkForMoreWork(old.pos)
+ } catch {
+ case ex: ValidateException => // Ignore, this will have been reported elsewhere
+ debugLog("validate exception caught: "+ex)
+ case ex: Throwable =>
+ log.flush()
+ throw ex
+ }
}
}
}
@@ -0,0 +1 @@
+(x: Int)Unit
@@ -0,0 +1,17 @@
+import scala.tools.nsc.interactive.tests.InteractiveTest
+
+object Test extends InteractiveTest {
+ override protected def loadSources() { /* don't parse or typecheck sources */ }
+
+ import compiler._
+
+ override def runDefaultTests() {
+ val res = new Response[Tree]
+ val pos = compiler.rangePos(sourceFiles.head, 102,102,102)
+ compiler.askTypeAt(pos, res)
+ res.get match {
+ case Left(tree) => compiler.ask(() => reporter.println(tree.tpe))
+ case Right(ex) => reporter.println(ex)
+ }
+ }
+}
@@ -0,0 +1,7 @@
+object Foo {
+ def foo(x: Int) = {}
+ def foo(x: String) = {}
+ def foo(x: Int, y: String) = {}
+
+ foo(2)
+}

0 comments on commit b7509c9

Please sign in to comment.