Skip to content

Commit 632f3cc

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 3f86587 commit 632f3cc

File tree

4 files changed

+130
-45
lines changed

4 files changed

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

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import scala.collection._
1414
import JavaConverters._
1515
import scala.io.Codec
1616

17-
import dotty.tools.io.{ ClassPath, ClassRepresentation, PlainFile, VirtualFile }
17+
import dotty.tools.io.{ AbstractFile, ClassPath, ClassRepresentation, PlainFile, VirtualFile }
1818

1919
import ast.{Trees, tpd}
2020
import core._, core.Decorators._
@@ -269,6 +269,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver {
269269
}
270270

271271
object InteractiveDriver {
272-
def toUri(source: SourceFile) = Paths.get(source.file.path).toUri
272+
def toUri(file: AbstractFile): URI = Paths.get(file.path).toUri
273+
def toUri(source: SourceFile): URI = toUri(source.file)
273274
}
274275

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

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

251251
val pos = sourcePosition(driver, uri, params.getPosition)
252252
val path = Interactive.pathTo(driver.openedTrees(uri), pos)
253-
val enclTree = Interactive.enclosingTree(path)
254-
255-
val sym = {
256-
val sym = path match {
257-
// For a named arg, find the target `DefDef` and jump to the param
258-
case NamedArg(name, _) :: Apply(fn, _) :: _ =>
259-
val funSym = fn.symbol
260-
if (funSym.name == StdNames.nme.copy
261-
&& funSym.is(Synthetic)
262-
&& funSym.owner.is(CaseClass)) {
263-
funSym.owner.info.member(name).symbol
264-
} else {
265-
val classTree = funSym.topLevelClass.asClass.rootTree
266-
tpd.defPath(funSym, classTree).lastOption.flatMap {
267-
case DefDef(_, _, paramss, _, _) =>
268-
paramss.flatten.find(_.name == name).map(_.symbol)
269-
}.getOrElse(fn.symbol)
270-
}
271-
272-
case _ =>
273-
enclTree.symbol
274-
}
275-
Interactive.sourceSymbol(sym)
276-
}
277253

278-
if (sym == NoSymbol) Nil.asJava
279-
else {
280-
val (trees, include) =
281-
if (enclTree.isInstanceOf[MemberDef])
282-
(driver.allTreesContaining(sym.name.sourceModuleName.toString),
283-
Include.overriding | Include.overridden)
284-
else sym.topLevelClass match {
285-
case cls: ClassSymbol =>
286-
(SourceTree.fromSymbol(cls).toList, Include.overriding)
287-
case _ =>
288-
(Nil, Include.overriding)
289-
}
290-
val defs = Interactive.namedTrees(trees, include, sym)
291-
defs.flatMap(d => location(d.namePos)).asJava
292-
}
254+
val definitions = Interactive.findDefinitions(path, driver).toList
255+
definitions.flatMap(d => location(d.namePos)).asJava
293256
}
294257

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

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

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

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

0 commit comments

Comments
 (0)