From 4f88a96637ef0d19a8692f5cbcd6bef476fdb296 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 16 Nov 2018 10:25:14 +0100 Subject: [PATCH 1/2] IDE: Look for overrides in the whole project When doing go-to-definition, we used to look for symbols that override the symbol selected for go to definition only in the same source file as the one that was selected. This commit changes that, as suggested in the review of #5208. --- .../tools/dotc/interactive/Interactive.scala | 60 +++++++++++-------- .../languageserver/DottyLanguageServer.scala | 5 +- .../tools/languageserver/DefinitionTest.scala | 22 +++++++ 3 files changed, 58 insertions(+), 29 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 7309eccba5a8..797bf77f1e8e 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -43,10 +43,7 @@ object Interactive { /** Include trees whose symbol is overridden by `sym` */ val overridden: Set = Set(1 << 0) - /** - * Include trees whose symbol overrides `sym` (but for performance only in same source - * file) - */ + /** Include trees whose symbol overrides `sym` */ val overriding: Set = Set(1 << 1) /** Include references */ @@ -328,34 +325,45 @@ object Interactive { path.find(_.isInstanceOf[DefTree]).getOrElse(EmptyTree) /** - * Find the definitions of the symbol at the end of `path`. + * Find the definitions of the symbol at the end of `path`. In the case of an import node, + * all imported symbols will be considered. * * @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], pos: SourcePosition, driver: InteractiveDriver)(implicit ctx: Context): List[SourceTree] = { - enclosingSourceSymbols(path, pos).flatMap { sym => - val enclTree = enclosingTree(path) - val includeLocal = if (sym.exists && sym.isLocal) Include.local else Include.empty - - 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).flatMap(InteractiveDriver.toUriOption) 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, Include.empty) - } + def findDefinitions(path: List[Tree], pos: SourcePosition, driver: InteractiveDriver): List[SourceTree] = { + implicit val ctx = driver.currentCtx + val enclTree = enclosingTree(path) + val includeOverridden = enclTree.isInstanceOf[MemberDef] + val symbols = enclosingSourceSymbols(path, pos) + val includeExternal = symbols.exists(!_.isLocal) + findDefinitions(symbols, driver, includeOverridden, includeExternal) + } + /** + * Find the definitions of `symbols`. + * + * @param symbols The list of symbols for which to find a definition. + * @param driver The driver responsible for the given symbols. + * @param includeOverridden If true, also include the symbols overridden by any of `symbols`. + * @param includeExternal If true, also look for definitions on the classpath. + * @return The definitions for the symbols in `symbols`, and if `includeOverridden` is set, the + * definitions for the symbols that they override. + */ + def findDefinitions(symbols: List[Symbol], + driver: InteractiveDriver, + includeOverridden: Boolean, + includeExternal: Boolean): List[SourceTree] = { + implicit val ctx = driver.currentCtx + val include = Include.definitions | Include.overriding | + (if (includeOverridden) Include.overridden else Include.empty) + symbols.flatMap { sym => + val name = sym.name.sourceModuleName.toString + val includeLocal = if (sym.exists && sym.isLocal) Include.local else Include.empty + val trees = + if (includeExternal) driver.allTreesContaining(name) + else driver.sourceTreesContaining(name) findTreesMatching(trees, include | includeLocal, sym) } } diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index c9430a122bb8..27b29c5b841e 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -289,10 +289,9 @@ class DottyLanguageServer extends LanguageServer /*isIncomplete = */ false, items.map(completionItem).asJava)) } - /** If cursor is on a reference, show its definition and all overriding definitions in - * the same source as the primary definition. + /** If cursor is on a reference, show its definition and all overriding definitions. * If cursor is on a definition, show this definition together with all overridden - * and overriding definitions (in all sources). + * and overriding definitions. */ override def definition(params: TextDocumentPositionParams) = computeAsync { cancelToken => val uri = new URI(params.getTextDocument.getUri) diff --git a/language-server/test/dotty/tools/languageserver/DefinitionTest.scala b/language-server/test/dotty/tools/languageserver/DefinitionTest.scala index fa71b14384f7..1f0810912f64 100644 --- a/language-server/test/dotty/tools/languageserver/DefinitionTest.scala +++ b/language-server/test/dotty/tools/languageserver/DefinitionTest.scala @@ -322,4 +322,26 @@ class DefinitionTest { .definition(m11 to m12, List(m3 to m4)) } + @Test def definitionShowOverrides: Unit = { + withSources( + code"""class A { def ${m1}foo${m2}: Int = 0 }""", + code"""class B extends A { def ${m3}foo${m4}: Int = 1 }""", + code"""class C extends A { def ${m5}foo${m6}: Int = 2 }""", + code"""class D extends C { def ${m7}foo${m8}: Int = 3 }""", + code"""object O { + val a = new A().${m9}foo${m10} + val b = new B().${m11}foo${m12} + val c = new C().${m13}foo${m14} + val d = new D().${m15}foo${m16} + }""" + ).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(m3 to m4)) + .definition(m13 to m14, List(m5 to m6, m7 to m8)) + .definition(m15 to m16, List(m7 to m8)) + } + } From 72053cacd1bdb65d95e1ef06a9ba6a5d7d2e6f72 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Mon, 19 Nov 2018 07:28:58 +0100 Subject: [PATCH 2/2] IDE: Support multi-project rename Fixes #4994 --- .../languageserver/DottyLanguageServer.scala | 20 +++-- .../tools/languageserver/RenameTest.scala | 81 +++++++++++++++++++ 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 27b29c5b841e..dfcd5c7cf2cf 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -381,18 +381,24 @@ class DottyLanguageServer extends LanguageServer List( RENAME_OVERRIDDEN -> (() => (Include.all, syms.flatMap(s => s :: s.allOverriddenSymbols.toList))), RENAME_NO_OVERRIDDEN -> (() => (Include.all.except(Include.overridden), syms))) - ).get.getOrElse((Include.empty, List.empty[Symbol])) + ).get.getOrElse((Include.empty, Nil)) } else { (Include.all, syms) } val names = allSymbols.map(_.name.sourceModuleName).toSet - val trees = names.flatMap(name => driver.allTreesContaining(name.toString)).toList - allSymbols.flatMap { sym => - Interactive.findTreesMatching(trees, - include, - sym, - t => names.exists(Interactive.sameName(t.name, _))) + val definitions = Interactive.findDefinitions(allSymbols, driver, include.isOverridden, includeExternal = true) + val perProjectInfo = inProjectsSeeing(driver, definitions, allSymbols) + + perProjectInfo.flatMap { (remoteDriver, ctx, definitions) => + definitions.flatMap { definition => + val name = definition.name(ctx).sourceModuleName.toString + val trees = remoteDriver.sourceTreesContaining(name)(ctx) + Interactive.findTreesMatching(trees, + include, + definition, + t => names.exists(Interactive.sameName(t.name, _)))(ctx) + } } } diff --git a/language-server/test/dotty/tools/languageserver/RenameTest.scala b/language-server/test/dotty/tools/languageserver/RenameTest.scala index 63c870de0f03..ec3e51de8349 100644 --- a/language-server/test/dotty/tools/languageserver/RenameTest.scala +++ b/language-server/test/dotty/tools/languageserver/RenameTest.scala @@ -251,4 +251,85 @@ class RenameTest { testRename(m2) } + @Test def renameValMultiProject: Unit = { + def testRename(m: CodeMarker, expectations: Set[CodeRange]) = { + val p0 = Project.withSources( + code"""object A { val ${m1}foo${m2} = 0 }""" + ) + + val p1 = Project.dependingOn(p0).withSources( + code"""object B { val ${m3}bar${m4} = A.${m5}foo${m6} }""" + ) + + val p2 = Project.dependingOn(p1).withSources( + code"""object C { val ${m7}baz${m8} = A.${m9}foo${m10} + B.${m11}bar${m12} }""" + ) + + withProjects(p0, p1, p2).rename(m, "NewName", expectations) + } + + testRename(m1, Set(m1 to m2, m5 to m6, m9 to m10)) + testRename(m5, Set(m1 to m2, m5 to m6, m9 to m10)) + testRename(m9, Set(m1 to m2, m5 to m6, m9 to m10)) + + testRename(m3, Set(m3 to m4, m11 to m12)) + testRename(m11, Set(m3 to m4, m11 to m12)) + + testRename(m7, Set(m7 to m8)) + } + + @Test def renameClassMultiProject: Unit = { + val m21 = new CodeMarker("m21") + val m22 = new CodeMarker("m22") + val m23 = new CodeMarker("m23") + val m24 = new CodeMarker("m24") + val m25 = new CodeMarker("m25") + val m26 = new CodeMarker("m26") + val m27 = new CodeMarker("m27") + val m28 = new CodeMarker("m28") + def testRename(m: CodeMarker, expectations: Set[CodeRange]) = { + val p0 = Project.withSources( + code"""package a + object ${m1}A${m2} { class ${m3}B${m4} }""" + ) + + val p1 = Project.dependingOn(p0).withSources( + code"""package b + import a.${m5}A${m6}.{${m7}B${m8} => ${m9}AB${m10}} + object ${m11}B${m12} { class ${m13}C${m14} extends ${m15}AB${m16} }""" + ) + + val p2 = Project.dependingOn(p1).withSources( + code"""package c + import b.${m17}B${m18}.{${m19}C${m20} => ${m21}BC${m22}} + object ${m23}C${m24} { class ${m25}D${m26} extends ${m27}BC${m28} }""" + ) + + withProjects(p0, p1, p2).rename(m, "NewName", expectations) + } + + testRename(m1, Set(m1 to m2, m5 to m6)) + testRename(m5, Set(m1 to m2, m5 to m6)) + + testRename(m3, Set(m3 to m4, m7 to m8)) + testRename(m7, Set(m3 to m4, m7 to m8)) + + testRename(m9, Set(m9 to m10, m15 to m16)) + testRename(m15, Set(m9 to m10, m15 to m16)) + + testRename(m11, Set(m11 to m12, m17 to m18)) + testRename(m17, Set(m11 to m12, m17 to m18)) + + testRename(m13, Set(m13 to m14, m19 to m20)) + testRename(m19, Set(m13 to m14, m19 to m20)) + + testRename(m21, Set(m21 to m22, m27 to m28)) + testRename(m27, Set(m21 to m22, m27 to m28)) + + testRename(m23, Set(m23 to m24)) + + testRename(m25, Set(m25 to m26)) + + } + }