From 214e277d677d90f5e9cc26a2c6ec5f894d28614b Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 4 Oct 2018 19:37:09 +0200 Subject: [PATCH] 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. --- .../tools/dotc/interactive/Interactive.scala | 83 ++++++++++++++++++- .../dotc/interactive/InteractiveDriver.scala | 5 +- .../languageserver/DottyLanguageServer.scala | 41 +-------- .../tools/languageserver/DefinitionTest.scala | 35 ++++++++ .../tools/languageserver/util/Code.scala | 4 + 5 files changed, 123 insertions(+), 45 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 46b8369c04b5..7ecf89c32e03 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -51,12 +51,52 @@ object Interactive { def enclosingTree(path: List[Tree])(implicit ctx: Context): Tree = path.dropWhile(!_.symbol.exists).headOption.getOrElse(tpd.EmptyTree) - /** The source symbol of the closest enclosing tree with a symbol containing position `pos`. + /** + * The source symbol that is the closest to `path`. + * + * @param path The path to the tree whose symbol to extract. + * @return The source symbol that is the closest to `path`. + * + * @see sourceSymbol + */ + def enclosingSourceSymbol(path: List[Tree])(implicit ctx: Context): Symbol = { + val sym = path match { + // For a named arg, find the target `DefDef` and jump to the param + case NamedArg(name, _) :: Apply(fn, _) :: _ => + val funSym = fn.symbol + if (funSym.name == StdNames.nme.copy + && funSym.is(Synthetic) + && funSym.owner.is(CaseClass)) { + funSym.owner.info.member(name).symbol + } else { + val classTree = funSym.topLevelClass.asClass.rootTree + tpd.defPath(funSym, classTree).lastOption.flatMap { + case DefDef(_, _, paramss, _, _) => + paramss.flatten.find(_.name == name).map(_.symbol) + }.getOrElse(fn.symbol) + } + + case _ => + enclosingTree(path).symbol + } + Interactive.sourceSymbol(sym) + } + + /** + * The source symbol that is the closest to the path to `pos` in `trees`. + * + * Computes the path from the tree with position `pos` in `trees`, and extract it source + * symbol. * - * @see sourceSymbol + * @param trees The trees in which to look for a path to `pos`. + * @param pos That target position of the path. + * @return The source symbol that is the closest to the computed path. + * + * @see sourceSymbol */ - def enclosingSourceSymbol(trees: List[SourceTree], pos: SourcePosition)(implicit ctx: Context): Symbol = - sourceSymbol(enclosingTree(trees, pos).symbol) + def enclosingSourceSymbol(trees: List[SourceTree], pos: SourcePosition)(implicit ctx: Context): Symbol = { + enclosingSourceSymbol(pathTo(trees, pos)) + } /** A symbol related to `sym` that is defined in source code. * @@ -411,4 +451,39 @@ object Interactive { /** The first tree in the path that is a definition. */ def enclosingDefinitionInPath(path: List[Tree])(implicit ctx: Context): Tree = path.find(_.isInstanceOf[DefTree]).getOrElse(EmptyTree) + + /** + * Find the definitions of the symbol at the end of `path`. + * + * @param path The path to the symbol for which we want the definitions. + * @param driver The driver responsible for `path`. + * @return The definitions for the symbol at the end of `path`. + */ + def findDefinitions(path: List[Tree], driver: InteractiveDriver)(implicit ctx: Context): List[SourceTree] = { + val sym = enclosingSourceSymbol(path) + if (sym == NoSymbol) Nil + else { + val enclTree = enclosingTree(path) + + val (trees, include) = + if (enclTree.isInstanceOf[MemberDef]) + (driver.allTreesContaining(sym.name.sourceModuleName.toString), + Include.definitions | Include.overriding | Include.overridden) + else sym.topLevelClass match { + case cls: ClassSymbol => + val trees = Option(cls.sourceFile).map(InteractiveDriver.toUri) match { + case Some(uri) if driver.openedTrees.contains(uri) => + driver.openedTrees(uri) + case _ => // Symbol comes from the classpath + SourceTree.fromSymbol(cls).toList + } + (trees, Include.definitions | Include.overriding) + case _ => + (Nil, 0) + } + + findTreesMatching(trees, include, sym) + } + } + } diff --git a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala index 281bbe04fbb1..5ea200f3e03a 100644 --- a/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala +++ b/compiler/src/dotty/tools/dotc/interactive/InteractiveDriver.scala @@ -11,7 +11,7 @@ import java.util.zip._ import scala.collection._ import scala.io.Codec -import dotty.tools.io.{ ClassPath, ClassRepresentation, PlainFile, VirtualFile } +import dotty.tools.io.{ AbstractFile, ClassPath, ClassRepresentation, PlainFile, VirtualFile } import ast.{Trees, tpd} import core._, core.Decorators._ @@ -265,6 +265,7 @@ class InteractiveDriver(val settings: List[String]) extends Driver { } object InteractiveDriver { - def toUri(source: SourceFile): URI = Paths.get(source.file.path).toUri + def toUri(file: AbstractFile): URI = Paths.get(file.path).toUri + def toUri(source: SourceFile): URI = toUri(source.file) } diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 63812f48ade4..bbc991861b68 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -269,46 +269,9 @@ class DottyLanguageServer extends LanguageServer val pos = sourcePosition(driver, uri, params.getPosition) val path = Interactive.pathTo(driver.openedTrees(uri), pos) - val enclTree = Interactive.enclosingTree(path) - - val sym = { - val sym = path match { - // For a named arg, find the target `DefDef` and jump to the param - case NamedArg(name, _) :: Apply(fn, _) :: _ => - val funSym = fn.symbol - if (funSym.name == StdNames.nme.copy - && funSym.is(Synthetic) - && funSym.owner.is(CaseClass)) { - funSym.owner.info.member(name).symbol - } else { - val classTree = funSym.topLevelClass.asClass.rootTree - tpd.defPath(funSym, classTree).lastOption.flatMap { - case DefDef(_, _, paramss, _, _) => - paramss.flatten.find(_.name == name).map(_.symbol) - }.getOrElse(fn.symbol) - } - - case _ => - enclTree.symbol - } - Interactive.sourceSymbol(sym) - } - if (sym == NoSymbol) Nil.asJava - else { - val (trees, include) = - if (enclTree.isInstanceOf[MemberDef]) - (driver.allTreesContaining(sym.name.sourceModuleName.toString), - Include.overriding | Include.overridden) - else sym.topLevelClass match { - case cls: ClassSymbol => - (SourceTree.fromSymbol(cls).toList, Include.overriding) - case _ => - (Nil, Include.overriding) - } - val defs = Interactive.namedTrees(trees, include, sym) - defs.flatMap(d => location(d.namePos, positionMapperFor(d.source))).asJava - } + val definitions = Interactive.findDefinitions(path, driver).toList + definitions.flatMap(d => location(d.namePos, positionMapperFor(d.source))).asJava } override def references(params: ReferenceParams) = computeAsync { cancelToken => diff --git a/language-server/test/dotty/tools/languageserver/DefinitionTest.scala b/language-server/test/dotty/tools/languageserver/DefinitionTest.scala index 4520e9bbb9cc..5b9274b358d9 100644 --- a/language-server/test/dotty/tools/languageserver/DefinitionTest.scala +++ b/language-server/test/dotty/tools/languageserver/DefinitionTest.scala @@ -54,6 +54,41 @@ class DefinitionTest { .definition(m5 to m6, List(m1 to m2)) } + @Test def goToOverriddenDef: Unit = { + code"""trait T { + def ${m1}foo${m2}(x: String): Unit + } + trait T2 extends T { + def ${m3}foo${m4}(x: String): Unit = println(x) + } + class T3 extends T { + def ${m5}foo${m6}(x: String): Unit = println(x) + } + class C4 extends T3 { + override def ${m7}foo${m8}(x: String): Unit = println(x) + } + object O { + def hello(obj: T): Unit = { + obj.${m9}foo${m10}("a") + obj match { + case c4: C4 => c4.${m11}foo${m12}("b") + case t3: T3 => t3.${m13}foo${m14}("c") + case t2: T2 => t2.${m15}foo${m16}("d") + case t: T => t.${m17}foo${m18}("e") + } + } + }""".withSource + .definition(m1 to m2, List(m1 to m2, m3 to m4, m5 to m6, m7 to m8)) + .definition(m3 to m4, List(m1 to m2, m3 to m4)) + .definition(m5 to m6, List(m1 to m2, m5 to m6, m7 to m8)) + .definition(m7 to m8, List(m1 to m2, m5 to m6, m7 to m8)) + .definition(m9 to m10, List(m1 to m2, m3 to m4, m5 to m6, m7 to m8)) + .definition(m11 to m12, List(m7 to m8)) + .definition(m13 to m14, List(m5 to m6, m7 to m8)) + .definition(m15 to m16, List(m3 to m4)) + .definition(m17 to m18, List(m1 to m2, m3 to m4, m5 to m6, m7 to m8)) + } + @Test def goToDefNamedArgOverload: Unit = { code"""object Foo { diff --git a/language-server/test/dotty/tools/languageserver/util/Code.scala b/language-server/test/dotty/tools/languageserver/util/Code.scala index aca2279beb8c..9fe5009d5422 100644 --- a/language-server/test/dotty/tools/languageserver/util/Code.scala +++ b/language-server/test/dotty/tools/languageserver/util/Code.scala @@ -26,6 +26,10 @@ object Code { val m12 = new CodeMarker("m12") val m13 = new CodeMarker("m13") val m14 = new CodeMarker("m14") + val m15 = new CodeMarker("m15") + val m16 = new CodeMarker("m16") + val m17 = new CodeMarker("m17") + val m18 = new CodeMarker("m18") implicit class CodeHelper(val sc: StringContext) extends AnyVal {