From cd2f98359bdb504c20ba8f0b2c71b4e614699b1b Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 14 Nov 2018 16:46:30 +0100 Subject: [PATCH 1/7] Look only in source trees in `workspace/symbol` We were inspecting all trees on the classpath for every project, which was not efficient and lead to duplicate results. This commit changes the implementation so that the language server inspects only the source trees of every project. --- .../languageserver/DottyLanguageServer.scala | 2 +- .../dotty/tools/languageserver/SymbolTest.scala | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index d4d38f38a4d5..9aaac9d3a6a9 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -470,7 +470,7 @@ class DottyLanguageServer extends LanguageServer drivers.values.toList.flatMap { driver => implicit val ctx = driver.currentCtx - val trees = driver.allTrees + val trees = driver.sourceTreesContaining(query) val defs = Interactive.namedTrees(trees, nameSubstring = query) defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source))) }.asJava diff --git a/language-server/test/dotty/tools/languageserver/SymbolTest.scala b/language-server/test/dotty/tools/languageserver/SymbolTest.scala index e9e6318b8e38..b03a4edc7a4d 100644 --- a/language-server/test/dotty/tools/languageserver/SymbolTest.scala +++ b/language-server/test/dotty/tools/languageserver/SymbolTest.scala @@ -39,4 +39,19 @@ class SymbolTest { .symbol("Foo", (m1 to m2).symInfo("Foo", SymbolKind.Module), (m3 to m4).symInfo("Foo", SymbolKind.Class)) } + + @Test def multipleProjects0: Unit = { + val p0 = Project.withSources( + code"""class ${m1}Foo${m2}""" + ) + + val p1 = Project.dependingOn(p0).withSources( + code"""class ${m3}Bar${m4} extends Foo""" + ) + + withProjects(p0, p1) + .symbol("Foo", (m1 to m2).symInfo("Foo", SymbolKind.Class)) + + + } } From 54656b9ab95723e850ef890b42c54ab0e193d2b3 Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 14 Nov 2018 17:01:50 +0100 Subject: [PATCH 2/7] Exclude local symbols in `workspace/symbol` --- .../src/dotty/tools/dotc/interactive/Interactive.scala | 8 -------- .../dotty/tools/languageserver/DottyLanguageServer.scala | 5 ++++- .../test/dotty/tools/languageserver/SymbolTest.scala | 9 ++++++++- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index b33d6c0407e1..27afc50b31ac 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -317,14 +317,6 @@ object Interactive { else namedTrees(trees, include, matchSymbol(_, sym, include)) - /** Find named trees with a non-empty position whose name contains `nameSubstring` in `trees`. - */ - def namedTrees(trees: List[SourceTree], nameSubstring: String) - (implicit ctx: Context): List[SourceTree] = { - val predicate: NameTree => Boolean = _.name.toString.contains(nameSubstring) - namedTrees(trees, Include.empty, predicate) - } - /** Find named trees with a non-empty position satisfying `treePredicate` in `trees`. * * @param includeReferences If true, include references and not just definitions diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 9aaac9d3a6a9..83b0e558a91a 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -22,6 +22,7 @@ import core._, core.Decorators.{sourcePos => _, _} import Comments._, Constants._, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ import classpath.ClassPathEntries import reporting._, reporting.diagnostic.{Message, MessageContainer, messages} +import transform.SymUtils.decorateSymbol import typer.Typer import util.{Set => _, _} import interactive._, interactive.InteractiveDriver._ @@ -466,12 +467,14 @@ class DottyLanguageServer extends LanguageServer override def symbol(params: WorkspaceSymbolParams) = computeAsync { cancelToken => val query = params.getQuery + def predicate(implicit ctx: Context): NameTree => Boolean = + tree => tree.symbol.exists && !tree.symbol.isLocal && tree.name.toString.contains(query) drivers.values.toList.flatMap { driver => implicit val ctx = driver.currentCtx val trees = driver.sourceTreesContaining(query) - val defs = Interactive.namedTrees(trees, nameSubstring = query) + val defs = Interactive.namedTrees(trees, includeReferences = false, predicate) defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source))) }.asJava } diff --git a/language-server/test/dotty/tools/languageserver/SymbolTest.scala b/language-server/test/dotty/tools/languageserver/SymbolTest.scala index b03a4edc7a4d..c586c5c17aba 100644 --- a/language-server/test/dotty/tools/languageserver/SymbolTest.scala +++ b/language-server/test/dotty/tools/languageserver/SymbolTest.scala @@ -51,7 +51,14 @@ class SymbolTest { withProjects(p0, p1) .symbol("Foo", (m1 to m2).symInfo("Foo", SymbolKind.Class)) + } - + @Test def noLocalSymbols: Unit = { + code"""object O { + def foo = { + val hello = 0 + } + }""".withSource + .symbol("hello") } } From 7fd0c1dbddf437d878e6919339758347dd2f038b Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Wed, 14 Nov 2018 18:01:31 +0100 Subject: [PATCH 3/7] Hide local or synthetic symbols in documentSymbol --- .../dotty/tools/languageserver/DottyLanguageServer.scala | 8 ++++++-- .../dotty/tools/languageserver/DocumentSymbolTest.scala | 6 ++++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 83b0e558a91a..0f06fc0ca13c 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -457,8 +457,12 @@ class DottyLanguageServer extends LanguageServer implicit val ctx = driver.currentCtx val uriTrees = driver.openedTrees(uri) + val predicate = (tree: NameTree) => { + val sym = tree.symbol + sym.exists && !sym.isLocal && !sym.isPrimaryConstructor && !sym.is(Synthetic) + } - val defs = Interactive.namedTrees(uriTrees, Include.empty, _ => true) + val defs = Interactive.namedTrees(uriTrees, Include.empty, predicate) (for { d <- defs if !isWorksheetWrapper(d) info <- symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source)) @@ -474,7 +478,7 @@ class DottyLanguageServer extends LanguageServer implicit val ctx = driver.currentCtx val trees = driver.sourceTreesContaining(query) - val defs = Interactive.namedTrees(trees, includeReferences = false, predicate) + val defs = Interactive.namedTrees(trees, Include.empty, predicate) defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source))) }.asJava } diff --git a/language-server/test/dotty/tools/languageserver/DocumentSymbolTest.scala b/language-server/test/dotty/tools/languageserver/DocumentSymbolTest.scala index a19137865b76..b27fbf0a6e40 100644 --- a/language-server/test/dotty/tools/languageserver/DocumentSymbolTest.scala +++ b/language-server/test/dotty/tools/languageserver/DocumentSymbolTest.scala @@ -38,4 +38,10 @@ class DocumentSymbolTest { .documentSymbol(m1, (m1 to m2).symInfo("Foo", SymbolKind.Module), (m3 to m4).symInfo("Foo", SymbolKind.Class)) } + + @Test def documentSymbolSynthetic: Unit = { + code"""case class ${m1}Foo${m2}(${m3}x${m4}: Int)""".withSource + .documentSymbol(m1, (m1 to m2).symInfo("Foo", SymbolKind.Class), + (m3 to m4).symInfo("x", SymbolKind.Field, "Foo")) + } } From 1c8eca501676d2a91e880ead4d20dc5176b28f1c Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 29 Nov 2018 11:19:48 +0100 Subject: [PATCH 4/7] Simplify predicates in `symbol`, `documentSymbol` Some checks were redundant with the checks that are performed in `namedTrees`, and can be removed. Also, exclude primary constructors in `symbol`. --- .../dotty/tools/languageserver/DottyLanguageServer.scala | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index 0f06fc0ca13c..a2c613adcc30 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -459,7 +459,7 @@ class DottyLanguageServer extends LanguageServer val uriTrees = driver.openedTrees(uri) val predicate = (tree: NameTree) => { val sym = tree.symbol - sym.exists && !sym.isLocal && !sym.isPrimaryConstructor && !sym.is(Synthetic) + !sym.isLocal && !sym.isPrimaryConstructor } val defs = Interactive.namedTrees(uriTrees, Include.empty, predicate) @@ -471,8 +471,10 @@ class DottyLanguageServer extends LanguageServer override def symbol(params: WorkspaceSymbolParams) = computeAsync { cancelToken => val query = params.getQuery - def predicate(implicit ctx: Context): NameTree => Boolean = - tree => tree.symbol.exists && !tree.symbol.isLocal && tree.name.toString.contains(query) + def predicate(implicit ctx: Context): NameTree => Boolean = { tree => + val sym = tree.symbol + !sym.isLocal && !sym.isPrimaryConstructor && tree.name.toString.contains(query) + } drivers.values.toList.flatMap { driver => implicit val ctx = driver.currentCtx From b40313ad0f1c0e1c6363eb8e7f10644acc806beb Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 29 Nov 2018 14:25:33 +0100 Subject: [PATCH 5/7] Exclude primary constructors in `namedTrees` We never want to match them, so it's easier to exclude them here rather than always add a check in the predicate function. --- .../src/dotty/tools/dotc/interactive/Interactive.scala | 4 ++-- .../dotty/tools/languageserver/DottyLanguageServer.scala | 7 ++----- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index 27afc50b31ac..bfb42b1a5395 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -338,6 +338,7 @@ object Interactive { val tree = utree.asInstanceOf[tpd.NameTree] if (tree.symbol.exists && !tree.symbol.is(Synthetic) + && !tree.symbol.isPrimaryConstructor && tree.pos.exists && !tree.pos.isZeroExtent && (include.isReferences || isDefinition(tree)) @@ -373,8 +374,7 @@ object Interactive { )(implicit ctx: Context): List[SourceTree] = { val linkedSym = symbol.linkedClass val fullPredicate: NameTree => Boolean = tree => - ( !tree.symbol.isPrimaryConstructor - && (includes.isDefinitions || !Interactive.isDefinition(tree)) + ( (includes.isDefinitions || !Interactive.isDefinition(tree)) && ( Interactive.matchSymbol(tree, symbol, includes) || ( includes.isLinkedClass && linkedSym.exists diff --git a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala index a2c613adcc30..f2578f978b82 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -457,10 +457,7 @@ class DottyLanguageServer extends LanguageServer implicit val ctx = driver.currentCtx val uriTrees = driver.openedTrees(uri) - val predicate = (tree: NameTree) => { - val sym = tree.symbol - !sym.isLocal && !sym.isPrimaryConstructor - } + val predicate = (tree: NameTree) => !tree.symbol.isLocal val defs = Interactive.namedTrees(uriTrees, Include.empty, predicate) (for { @@ -473,7 +470,7 @@ class DottyLanguageServer extends LanguageServer val query = params.getQuery def predicate(implicit ctx: Context): NameTree => Boolean = { tree => val sym = tree.symbol - !sym.isLocal && !sym.isPrimaryConstructor && tree.name.toString.contains(query) + !sym.isLocal && tree.name.toString.contains(query) } drivers.values.toList.flatMap { driver => From 8213555b7c63320f5daf45acdbbcadb40aee7e6c Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Thu, 29 Nov 2018 16:06:27 +0100 Subject: [PATCH 6/7] Add `Include.local` This include flag means that local symbols and trees should be inspected. --- .../tools/dotc/interactive/Interactive.scala | 38 +++++++++++++------ .../languageserver/DottyLanguageServer.scala | 14 ++----- .../tools/languageserver/ReferencesTest.scala | 10 +++++ 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index bfb42b1a5395..b6feab31a269 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -8,6 +8,7 @@ import scala.collection._ import ast.{NavigateAST, Trees, tpd, untpd} import core._, core.Decorators.{sourcePos => _, _} import Contexts._, Flags._, Names._, NameOps._, Symbols._, Trees._, Types._ +import transform.SymUtils.decorateSymbol import util.Positions._, util.SourceFile, util.SourcePosition import core.Denotations.SingleDenotation import NameKinds.SimpleNameKind @@ -33,6 +34,7 @@ object Interactive { def isDefinitions: Boolean = (bits & definitions.bits) != 0 def isLinkedClass: Boolean = (bits & linkedClass.bits) != 0 def isImports: Boolean = (bits & imports.bits) != 0 + def isLocal: Boolean = (bits & local.bits) != 0 } /** The empty set */ @@ -59,6 +61,9 @@ object Interactive { /** Include imports in the results */ val imports: Set = Set(1 << 5) + /** Include local symbols, inspect local trees */ + val local: Set = Set(1 << 6) + /** All the flags */ val all: Set = Set(~0) } @@ -321,12 +326,25 @@ object Interactive { * * @param includeReferences If true, include references and not just definitions */ - def namedTrees(trees: List[SourceTree], include: Include.Set, treePredicate: NameTree => Boolean) - (implicit ctx: Context): List[SourceTree] = safely { + def namedTrees(trees: List[SourceTree], + include: Include.Set, + treePredicate: NameTree => Boolean = util.common.alwaysTrue + )(implicit ctx: Context): List[SourceTree] = safely { val buf = new mutable.ListBuffer[SourceTree] def traverser(source: SourceFile) = { new untpd.TreeTraverser { + private def handle(utree: untpd.NameTree): Unit = { + val tree = utree.asInstanceOf[tpd.NameTree] + if (tree.symbol.exists + && !tree.symbol.is(Synthetic) + && !tree.symbol.isPrimaryConstructor + && tree.pos.exists + && !tree.pos.isZeroExtent + && (include.isReferences || isDefinition(tree)) + && treePredicate(tree)) + buf += SourceTree(tree, source) + } override def traverse(tree: untpd.Tree)(implicit ctx: Context) = { tree match { case imp: untpd.Import if include.isImports && tree.hasType => @@ -334,16 +352,11 @@ object Interactive { val selections = tpd.importSelections(tree) traverse(imp.expr) selections.foreach(traverse) + case utree: untpd.ValOrDefDef if tree.hasType => + handle(utree) + if (include.isLocal) traverseChildren(tree) case utree: untpd.NameTree if tree.hasType => - val tree = utree.asInstanceOf[tpd.NameTree] - if (tree.symbol.exists - && !tree.symbol.is(Synthetic) - && !tree.symbol.isPrimaryConstructor - && tree.pos.exists - && !tree.pos.isZeroExtent - && (include.isReferences || isDefinition(tree)) - && treePredicate(tree)) - buf += SourceTree(tree, source) + handle(utree) traverseChildren(tree) case tree: untpd.Inlined => traverse(tree.call) @@ -474,6 +487,7 @@ object Interactive { 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]) @@ -492,7 +506,7 @@ object Interactive { (Nil, Include.empty) } - findTreesMatching(trees, include, sym) + 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 f2578f978b82..a8dd44db60fb 100644 --- a/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala +++ b/language-server/src/dotty/tools/languageserver/DottyLanguageServer.scala @@ -22,7 +22,6 @@ import core._, core.Decorators.{sourcePos => _, _} import Comments._, Constants._, Contexts._, Flags._, Names._, NameOps._, Symbols._, SymDenotations._, Trees._, Types._ import classpath.ClassPathEntries import reporting._, reporting.diagnostic.{Message, MessageContainer, messages} -import transform.SymUtils.decorateSymbol import typer.Typer import util.{Set => _, _} import interactive._, interactive.InteractiveDriver._ @@ -312,7 +311,7 @@ class DottyLanguageServer extends LanguageServer val includes = { val includeDeclaration = params.getContext.isIncludeDeclaration - Include.references | Include.overriding | Include.imports | + Include.references | Include.overriding | Include.imports | Include.local | (if (includeDeclaration) Include.definitions else Include.empty) } @@ -457,9 +456,8 @@ class DottyLanguageServer extends LanguageServer implicit val ctx = driver.currentCtx val uriTrees = driver.openedTrees(uri) - val predicate = (tree: NameTree) => !tree.symbol.isLocal - val defs = Interactive.namedTrees(uriTrees, Include.empty, predicate) + val defs = Interactive.namedTrees(uriTrees, Include.empty) (for { d <- defs if !isWorksheetWrapper(d) info <- symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source)) @@ -468,16 +466,12 @@ class DottyLanguageServer extends LanguageServer override def symbol(params: WorkspaceSymbolParams) = computeAsync { cancelToken => val query = params.getQuery - def predicate(implicit ctx: Context): NameTree => Boolean = { tree => - val sym = tree.symbol - !sym.isLocal && tree.name.toString.contains(query) - } drivers.values.toList.flatMap { driver => implicit val ctx = driver.currentCtx val trees = driver.sourceTreesContaining(query) - val defs = Interactive.namedTrees(trees, Include.empty, predicate) + val defs = Interactive.namedTrees(trees, Include.empty, _.name.toString.contains(query)) defs.flatMap(d => symbolInfo(d.tree.symbol, d.namePos, positionMapperFor(d.source))) }.asJava } @@ -505,7 +499,7 @@ class DottyLanguageServer extends LanguageServer val predicates = definitions.map(Interactive.implementationFilter(_)(ctx)) tree => predicates.exists(_(tree)) } - val matches = Interactive.namedTrees(trees, Include.empty, predicate)(ctx) + val matches = Interactive.namedTrees(trees, Include.local, predicate)(ctx) matches.map(tree => location(tree.namePos(ctx), positionMapperFor(tree.source))) } }.toList diff --git a/language-server/test/dotty/tools/languageserver/ReferencesTest.scala b/language-server/test/dotty/tools/languageserver/ReferencesTest.scala index bfe811b2751e..b99947bfe9bb 100644 --- a/language-server/test/dotty/tools/languageserver/ReferencesTest.scala +++ b/language-server/test/dotty/tools/languageserver/ReferencesTest.scala @@ -346,4 +346,14 @@ class ReferencesTest { .references(m11 to m12, List(m9 to m10, m11 to m12), withDecl = false) } + @Test def referenceInsideLocalMember: Unit = { + withSources( + code"""object A { + | val ${m1}foo${m2} = 0 + | def fizz = println(${m3}foo${m4}) + |}""" + ).references(m1 to m2, List(m1 to m2, m3 to m4), withDecl = true) + .references(m1 to m2, List(m3 to m4), withDecl = false) + } + } From e86b1b8641a9bb2c073700836aef9b1416d0b6aa Mon Sep 17 00:00:00 2001 From: Martin Duhem Date: Fri, 30 Nov 2018 07:01:26 +0100 Subject: [PATCH 7/7] Fix outdated doc --- compiler/src/dotty/tools/dotc/interactive/Interactive.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala index b6feab31a269..78ee8aabfc9e 100644 --- a/compiler/src/dotty/tools/dotc/interactive/Interactive.scala +++ b/compiler/src/dotty/tools/dotc/interactive/Interactive.scala @@ -324,7 +324,10 @@ object Interactive { /** Find named trees with a non-empty position satisfying `treePredicate` in `trees`. * - * @param includeReferences If true, include references and not just definitions + * @param trees The trees to inspect. + * @param include Whether to include references, definitions, etc. + * @param treePredicate An additional predicate that the trees must match. + * @return The trees with a non-empty position satisfying `treePredicate`. */ def namedTrees(trees: List[SourceTree], include: Include.Set,