From 36e99263dca7bed1df3518a1af01b8c220373d8a Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Wed, 25 Oct 2023 15:43:14 +0200 Subject: [PATCH] refactor: Inline symbolsForPackage method --- .../meta/internal/tvp/ClasspathTreeView.scala | 5 +- .../meta/internal/tvp/IndexedSymbols.scala | 77 ++++++++++--------- .../internal/tvp/MetalsTreeViewProvider.scala | 8 +- .../src/main/scala/tests/TestingServer.scala | 23 +++++- .../test/scala/tests/TreeViewLspSuite.scala | 2 +- 5 files changed, 68 insertions(+), 47 deletions(-) diff --git a/metals/src/main/scala/scala/meta/internal/tvp/ClasspathTreeView.scala b/metals/src/main/scala/scala/meta/internal/tvp/ClasspathTreeView.scala index a7fee7bb82b..6a81132e02f 100644 --- a/metals/src/main/scala/scala/meta/internal/tvp/ClasspathTreeView.scala +++ b/metals/src/main/scala/scala/meta/internal/tvp/ClasspathTreeView.scala @@ -111,8 +111,9 @@ class ClasspathTreeView[Value, Key]( case k.CLASS => "symbol-class" case k.INTERFACE => "symbol-interface" case k.CONSTRUCTOR => "symbol-method" - case k.METHOD | k.MACRO if (child.properties.isVal) => "symbol-field" - case k.METHOD | k.MACRO if (child.properties.isVar) => "symbol-variable" + case k.METHOD | k.MACRO if (child.properties.isVal) => "symbol-field" + case k.METHOD | k.MACRO if (child.properties.isVar) => + "symbol-variable" case k.METHOD | k.MACRO => "symbol-method" case k.FIELD if (child.properties.isEnum) => "symbol-enum-member" case k.FIELD => "symbol-field" diff --git a/metals/src/main/scala/scala/meta/internal/tvp/IndexedSymbols.scala b/metals/src/main/scala/scala/meta/internal/tvp/IndexedSymbols.scala index 7513287ce2e..5bcacc03c1e 100644 --- a/metals/src/main/scala/scala/meta/internal/tvp/IndexedSymbols.scala +++ b/metals/src/main/scala/scala/meta/internal/tvp/IndexedSymbols.scala @@ -84,10 +84,10 @@ class IndexedSymbols(isStatisticsEnabled: Boolean)(implicit rc: ReportContext) } /** - * We load all symbols for workspace eagerly + * We load all symbols for workspace when semanticdb files are produced. * - * @param in the input file to index - * @param dialect dialect to parse the file with + * @param in the input file to get symbols for + * @param symbol we are looking for * @return list of tree view symbols within the file */ def workspaceSymbols( @@ -142,20 +142,50 @@ class IndexedSymbols(isStatisticsEnabled: Boolean)(implicit rc: ReportContext) } val parsedSymbol = Symbol(symbol) - // if it's a package we'll collect all the children - if (parsedSymbol.isPackage) { + // root package doesn't need to calculate any members, they will be calculated lazily + if (parsedSymbol.isRootPackage) { jarSymbols.values .collect { - // root package doesn't need to calculate any members, they will be calculated lazily - case Left(defn) if parsedSymbol.isRootPackage => + case Left(defn) => Array(toTreeView(defn)) - case Right(list) if parsedSymbol.isRootPackage => list - case cached => - symbolsForPackage(cached, dialect, jarSymbols, parsedSymbol) + case Right(list) => list } .flatten .iterator - } else { + } // If it's a package we'll collect all the children + else if (parsedSymbol.isPackage) { + jarSymbols + .collect { + /* If the package we are looking for is the parent of the current symbol we + * need to check if we have grandchildren and the nodes are exapandable + * on the UI + */ + case (_, Left(toplevel)) + if (toplevel.definitionSymbol.owner == parsedSymbol) => + val children = + members(toplevel.path, dialect).map(toTreeView) + jarSymbols.put( + toplevel.definitionSymbol.value, + Right(children), + ) + children + + /* If this is further down we don't need to resolve it yet as + * as we will check that later when resolving parent package + */ + case (toplevelSymbol, Left(toplevel)) + if toplevelSymbol.startsWith(symbol) => + Array(toTreeView(toplevel)) + + // If it's already calculated then we can just return it + case (toplevelSymbol, Right(allSymbols)) + if toplevelSymbol.startsWith(symbol) => + allSymbols + case _ => Array.empty[TreeViewSymbolInformation] + } + .flatten + .iterator + } else { // if we are looking for a particular symbol then we need to resolve it properly jarSymbols.get(toplevelOwner(Symbol(symbol)).value) match { case Some(Left(toplevelOnly)) => val allSymbols = members(toplevelOnly.path, dialect).map(toTreeView) @@ -172,31 +202,6 @@ class IndexedSymbols(isStatisticsEnabled: Boolean)(implicit rc: ReportContext) } } - private def symbolsForPackage( - cached: Either[TopLevel, AllSymbols], - dialect: Dialect, - jarSymbols: TrieMap[String, Either[TopLevel, AllSymbols]], - symbol: Symbol, - ): Array[TreeViewSymbolInformation] = - cached match { - case Left(toplevel) - if toplevel.definitionSymbol.value.startsWith(symbol.value) => - // we need to check if we have grandchildren and the nodes are exapandable - if (toplevel.definitionSymbol.owner == symbol) { - val children = - members(toplevel.path, dialect).map(toTreeView) - jarSymbols.put( - toplevel.definitionSymbol.value, - Right(children), - ) - children - } else { - Array(toTreeView(toplevel)) - } - case Right(allSymbols) => allSymbols - case _ => Array.empty[TreeViewSymbolInformation] - } - private def members( path: AbsolutePath, dialect: Dialect, diff --git a/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala b/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala index 47df8c9e79c..af184781385 100644 --- a/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/tvp/MetalsTreeViewProvider.scala @@ -198,8 +198,8 @@ class MetalsTreeViewProvider( override def onVisibilityDidChange( params: TreeViewVisibilityDidChangeParams ): Unit = { - val trees = getFolderTreeViewProviders() - trees.foreach{ + val trees = getFolderTreeViewProviders() + trees.foreach { _.setVisible(params.viewId, params.visible) } if (params.visible) { @@ -308,8 +308,8 @@ class FolderTreeViewProvider( }, ) - def setVisible(viewId: String, visibility: Boolean): Unit ={ - isVisible(viewId) = visibility + def setVisible(viewId: String, visibility: Boolean): Unit = { + isVisible(viewId) = visibility } def flushPendingProjectUpdates(): Option[Array[TreeViewNode]] = { diff --git a/tests/unit/src/main/scala/tests/TestingServer.scala b/tests/unit/src/main/scala/tests/TestingServer.scala index 1b3021ea79d..0781015a7ed 100644 --- a/tests/unit/src/main/scala/tests/TestingServer.scala +++ b/tests/unit/src/main/scala/tests/TestingServer.scala @@ -1879,12 +1879,27 @@ final case class TestingServer( Assertions.assertNoDiff(obtained, expected) } - def treeViewVisibilityDidChange(viewId: String, isVisible: Boolean): Future[Unit] = { - fullServer.treeViewVisibilityDidChange(TreeViewVisibilityDidChangeParams(viewId, isVisible)).asScala + def treeViewVisibilityDidChange( + viewId: String, + isVisible: Boolean, + ): Future[Unit] = { + fullServer + .treeViewVisibilityDidChange( + TreeViewVisibilityDidChangeParams(viewId, isVisible) + ) + .asScala } - def treeViewNodeCollapseDidChange(viewId: String, nodeId: String, isCollapsed: Boolean): Future[Unit] = { - fullServer.treeViewNodeCollapseDidChange(TreeViewNodeCollapseDidChangeParams(viewId, nodeId, isCollapsed)).asScala + def treeViewNodeCollapseDidChange( + viewId: String, + nodeId: String, + isCollapsed: Boolean, + ): Future[Unit] = { + fullServer + .treeViewNodeCollapseDidChange( + TreeViewNodeCollapseDidChangeParams(viewId, nodeId, isCollapsed) + ) + .asScala } def findTextInDependencyJars( diff --git a/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala b/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala index f14cf086389..80c7383c8ce 100644 --- a/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala +++ b/tests/unit/src/test/scala/tests/TreeViewLspSuite.scala @@ -160,7 +160,7 @@ class TreeViewLspSuite extends BaseLspSuite("tree-view") { } _ = assertEquals( server.client.workspaceTreeViewChanges, - s"metalsPackages projects-$folder:${server.buildTarget("a")}!/_root_/" + s"metalsPackages projects-$folder:${server.buildTarget("a")}!/_root_/", ) _ = server.assertTreeViewChildren( s"projects-$folder:${server.buildTarget("a")}!/_empty_/Zero#",