Skip to content

Commit 214e277

Browse files
DuhemmDuhemm
committed
Fix go to definition with overridden methods
When doing go to definition on a reference, we are supposed to look for the symbol under the cursor, and the symbols that override it in the same source file. We were previously looking only in the tree of the first definition, and would not inspect the rest of the sourcefile.
1 parent 9d12693 commit 214e277

File tree

5 files changed

+123
-45
lines changed

5 files changed

+123
-45
lines changed

‎compiler/src/dotty/tools/dotc/interactive/Interactive.scala

Lines changed: 79 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,12 +51,52 @@ object Interactive {
5151
def enclosingTree(path: List[Tree])(implicit ctx: Context): Tree =
5252
path.dropWhile(!_.symbol.exists).headOption.getOrElse(tpd.EmptyTree)
5353

54-
/** The source symbol of the closest enclosing tree with a symbol containing position `pos`.
54+
/**
55+
* The source symbol that is the closest to `path`.
56+
*
57+
* @param path The path to the tree whose symbol to extract.
58+
* @return The source symbol that is the closest to `path`.
59+
*
60+
* @see sourceSymbol
61+
*/
62+
def enclosingSourceSymbol(path: List[Tree])(implicit ctx: Context): Symbol = {
63+
val sym = path match {
64+
// For a named arg, find the target `DefDef` and jump to the param
65+
case NamedArg(name, _) :: Apply(fn, _) :: _ =>
66+
val funSym = fn.symbol
67+
if (funSym.name == StdNames.nme.copy
68+
&& funSym.is(Synthetic)
69+
&& funSym.owner.is(CaseClass)) {
70+
funSym.owner.info.member(name).symbol
71+
} else {
72+
val classTree = funSym.topLevelClass.asClass.rootTree
73+
tpd.defPath(funSym, classTree).lastOption.flatMap {
74+
case DefDef(_, _, paramss, _, _) =>
75+
paramss.flatten.find(_.name == name).map(_.symbol)
76+
}.getOrElse(fn.symbol)
77+
}
78+
79+
case _ =>
80+
enclosingTree(path).symbol
81+
}
82+
Interactive.sourceSymbol(sym)
83+
}
84+
85+
/**
86+
* The source symbol that is the closest to the path to `pos` in `trees`.
87+
*
88+
* Computes the path from the tree with position `pos` in `trees`, and extract it source
89+
* symbol.
5590
*
56-
* @see sourceSymbol
91+
* @param trees The trees in which to look for a path to `pos`.
92+
* @param pos That target position of the path.
93+
* @return The source symbol that is the closest to the computed path.
94+
*
95+
* @see sourceSymbol
5796
*/
58-
def enclosingSourceSymbol(trees: List[SourceTree], pos: SourcePosition)(implicit ctx: Context): Symbol =
59-
sourceSymbol(enclosingTree(trees, pos).symbol)
97+
def enclosingSourceSymbol(trees: List[SourceTree], pos: SourcePosition)(implicit ctx: Context): Symbol = {
98+
enclosingSourceSymbol(pathTo(trees, pos))
99+
}
60100

61101
/** A symbol related to `sym` that is defined in source code.
62102
*
@@ -411,4 +451,39 @@ object Interactive {
411451
/** The first tree in the path that is a definition. */
412452
def enclosingDefinitionInPath(path: List[Tree])(implicit ctx: Context): Tree =
413453
path.find(_.isInstanceOf[DefTree]).getOrElse(EmptyTree)
454+
455+
/**
456+
* Find the definitions of the symbol at the end of `path`.
457+
*
458+
* @param path The path to the symbol for which we want the definitions.
459+
* @param driver The driver responsible for `path`.
460+
* @return The definitions for the symbol at the end of `path`.
461+
*/
462+
def findDefinitions(path: List[Tree], driver: InteractiveDriver)(implicit ctx: Context): List[SourceTree] = {
463+
val sym = enclosingSourceSymbol(path)
464+
if (sym == NoSymbol) Nil
465+
else {
466+
val enclTree = enclosingTree(path)
467+
468+
val (trees, include) =
469+
if (enclTree.isInstanceOf[MemberDef])
470+
(driver.allTreesContaining(sym.name.sourceModuleName.toString),
471+
Include.definitions | Include.overriding | Include.overridden)
472+
else sym.topLevelClass match {
473+
case cls: ClassSymbol =>
474+
val trees = Option(cls.sourceFile).map(InteractiveDriver.toUri) match {
475+
case Some(uri) if driver.openedTrees.contains(uri) =>
476+
driver.openedTrees(uri)
477+
case _ => // Symbol comes from the classpath
478+
SourceTree.fromSymbol(cls).toList
479+
}
480+
(trees, Include.definitions | Include.overriding)
481+
case _ =>
482+
(Nil, 0)
483+
}
484+
485+
findTreesMatching(trees, include, sym)
486+
}
487+
}
488+
414489
}

‎compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import java.util.zip._
1111
import scala.collection._
1212
import scala.io.Codec
1313

14-
import dotty.tools.io.{ ClassPath, ClassRepresentation, PlainFile, VirtualFile }
14+
import dotty.tools.io.{ AbstractFile, ClassPath, ClassRepresentation, PlainFile, VirtualFile }
1515

1616
import ast.{Trees, tpd}
1717
import core._, core.Decorators._
@@ -265,6 +265,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver {
265265
}
266266

267267
object InteractiveDriver {
268-
def toUri(source: SourceFile): URI = Paths.get(source.file.path).toUri
268+
def toUri(file: AbstractFile): URI = Paths.get(file.path).toUri
269+
def toUri(source: SourceFile): URI = toUri(source.file)
269270
}
270271

‎language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala

Lines changed: 2 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -269,46 +269,9 @@ class DottyLanguageServer extends LanguageServer
269269

270270
val pos = sourcePosition(driver, uri, params.getPosition)
271271
val path = Interactive.pathTo(driver.openedTrees(uri), pos)
272-
val enclTree = Interactive.enclosingTree(path)
273-
274-
val sym = {
275-
val sym = path match {
276-
// For a named arg, find the target `DefDef` and jump to the param
277-
case NamedArg(name, _) :: Apply(fn, _) :: _ =>
278-
val funSym = fn.symbol
279-
if (funSym.name == StdNames.nme.copy
280-
&& funSym.is(Synthetic)
281-
&& funSym.owner.is(CaseClass)) {
282-
funSym.owner.info.member(name).symbol
283-
} else {
284-
val classTree = funSym.topLevelClass.asClass.rootTree
285-
tpd.defPath(funSym, classTree).lastOption.flatMap {
286-
case DefDef(_, _, paramss, _, _) =>
287-
paramss.flatten.find(_.name == name).map(_.symbol)
288-
}.getOrElse(fn.symbol)
289-
}
290-
291-
case _ =>
292-
enclTree.symbol
293-
}
294-
Interactive.sourceSymbol(sym)
295-
}
296272

297-
if (sym == NoSymbol) Nil.asJava
298-
else {
299-
val (trees, include) =
300-
if (enclTree.isInstanceOf[MemberDef])
301-
(driver.allTreesContaining(sym.name.sourceModuleName.toString),
302-
Include.overriding | Include.overridden)
303-
else sym.topLevelClass match {
304-
case cls: ClassSymbol =>
305-
(SourceTree.fromSymbol(cls).toList, Include.overriding)
306-
case _ =>
307-
(Nil, Include.overriding)
308-
}
309-
val defs = Interactive.namedTrees(trees, include, sym)
310-
defs.flatMap(d => location(d.namePos, positionMapperFor(d.source))).asJava
311-
}
273+
val definitions = Interactive.findDefinitions(path, driver).toList
274+
definitions.flatMap(d => location(d.namePos, positionMapperFor(d.source))).asJava
312275
}
313276

314277
override def references(params: ReferenceParams) = computeAsync { cancelToken =>

‎language-server/test/dotty/tools/languageserver/DefinitionTest.scala

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,41 @@ class DefinitionTest {
5454
.definition(m5 to m6, List(m1 to m2))
5555
}
5656

57+
@Test def goToOverriddenDef: Unit = {
58+
code"""trait T {
59+
def ${m1}foo${m2}(x: String): Unit
60+
}
61+
trait T2 extends T {
62+
def ${m3}foo${m4}(x: String): Unit = println(x)
63+
}
64+
class T3 extends T {
65+
def ${m5}foo${m6}(x: String): Unit = println(x)
66+
}
67+
class C4 extends T3 {
68+
override def ${m7}foo${m8}(x: String): Unit = println(x)
69+
}
70+
object O {
71+
def hello(obj: T): Unit = {
72+
obj.${m9}foo${m10}("a")
73+
obj match {
74+
case c4: C4 => c4.${m11}foo${m12}("b")
75+
case t3: T3 => t3.${m13}foo${m14}("c")
76+
case t2: T2 => t2.${m15}foo${m16}("d")
77+
case t: T => t.${m17}foo${m18}("e")
78+
}
79+
}
80+
}""".withSource
81+
.definition(m1 to m2, List(m1 to m2, m3 to m4, m5 to m6, m7 to m8))
82+
.definition(m3 to m4, List(m1 to m2, m3 to m4))
83+
.definition(m5 to m6, List(m1 to m2, m5 to m6, m7 to m8))
84+
.definition(m7 to m8, List(m1 to m2, m5 to m6, m7 to m8))
85+
.definition(m9 to m10, List(m1 to m2, m3 to m4, m5 to m6, m7 to m8))
86+
.definition(m11 to m12, List(m7 to m8))
87+
.definition(m13 to m14, List(m5 to m6, m7 to m8))
88+
.definition(m15 to m16, List(m3 to m4))
89+
.definition(m17 to m18, List(m1 to m2, m3 to m4, m5 to m6, m7 to m8))
90+
}
91+
5792
@Test def goToDefNamedArgOverload: Unit = {
5893

5994
code"""object Foo {

‎language-server/test/dotty/tools/languageserver/util/Code.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ object Code {
2626
val m12 = new CodeMarker("m12")
2727
val m13 = new CodeMarker("m13")
2828
val m14 = new CodeMarker("m14")
29+
val m15 = new CodeMarker("m15")
30+
val m16 = new CodeMarker("m16")
31+
val m17 = new CodeMarker("m17")
32+
val m18 = new CodeMarker("m18")
2933

3034
implicit class CodeHelper(val sc: StringContext) extends AnyVal {
3135

0 commit comments

Comments
 (0)