Skip to content

Commit

Permalink
refactor: Inline symbolsForPackage method
Browse files Browse the repository at this point in the history
  • Loading branch information
tgodzik committed Oct 26, 2023
1 parent bb6b8cb commit 82738ec
Show file tree
Hide file tree
Showing 9 changed files with 107 additions and 62 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,7 @@ final case class Indexer(
val jdkSources = JdkSources(userConfig().javaHome)
jdkSources match {
case Right(zip) =>
scribe.info(s"Indexing JDK sources from $zip")
usedJars += zip
addSourceJarSymbols(zip)
case Left(notFound) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,7 @@ class MetalsLspService(
buildTargets,
() => buildClient.ongoingCompilations(),
definitionIndex,
optJavaHome,
() => userConfig,
scalaVersionSelector,
classpathTreeIndex,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
77 changes: 41 additions & 36 deletions metals/src/main/scala/scala/meta/internal/tvp/IndexedSymbols.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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)
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -243,15 +243,16 @@ class FolderTreeViewProvider(
buildTargets: BuildTargets,
compilations: () => TreeViewCompilations,
definitionIndex: GlobalSymbolIndex,
userJavaHome: Option[AbsolutePath],
userConfig: () => UserConfiguration,
scalaVersionSelector: ScalaVersionSelector,
classpath: IndexedSymbols,
) {
def dialectOf(path: AbsolutePath): Option[Dialect] =
scalaVersionSelector.dialectFromBuildTarget(path)
private val maybeUsedJdkVersion =
userJavaHome.flatMap { path =>
JdkVersion.fromReleaseFileString(path)
private def maybeUsedJdkVersion =
JdkSources.defaultJavaHome(userConfig().javaHome).headOption.flatMap {
path =>
JdkVersion.fromReleaseFileString(path)
}
private val isVisible = TrieMap.empty[String, Boolean].withDefaultValue(false)
private val isCollapsedTarget = TrieMap.empty[BuildTargetIdentifier, Boolean]
Expand All @@ -275,7 +276,10 @@ class FolderTreeViewProvider(
path.filename
},
_.toString,
() => buildTargets.allSourceJars,
() => {
scribe.info(s"libraries: ${buildTargets.allSourceJars.toList}")
buildTargets.allSourceJars
},
(path, symbol) => {
val dialect = ScalaVersions.dialectForDependencyJar(path.filename)
classpath.jarSymbols(path, symbol, dialect)
Expand Down Expand Up @@ -308,8 +312,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]] = {
Expand Down Expand Up @@ -404,7 +408,7 @@ class FolderTreeViewProvider(
closestSymbol: SymbolOccurrence,
): Option[List[String]] = {
if (path.isDependencySource(folder.path) || path.isJarFileSystem) {
def jdkSources = JdkSources(userJavaHome.map(_.toString())).toOption
def jdkSources = JdkSources(userConfig().javaHome).toOption
.collect {
case sources
if sources.toString == path.toNIO.getFileSystem().toString() ||
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,9 @@ object JdkSources {
): Either[NoSourcesAvailable, AbsolutePath] = {
val paths = candidates(userJavaHome)
paths.find(_.isFile) match {
case Some(value) => Right(value)
case Some(value) =>
logger.info("Found JDK sources: " + value)
Right(value)
case None => Left(NoSourcesAvailable(paths))
}
}
Expand All @@ -40,9 +42,7 @@ object JdkSources {
s"Failed to parse java home path $str: ${exception.getMessage}"
)
None
case Success(value) =>
if (value.toString().contains("unit")) throw new Exception
Some(value)
case Success(value) => Some(value)
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/src/main/scala/tests/QuickBuild.scala
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ case class QuickBuild(
val javaHome = Option(platformJavaHome)
.map(Paths.get(_))
.orElse(Option(Properties.jdkHome).map(Paths.get(_)))

val tags = if (isTest) Tag.Test :: Nil else Nil

val scalaCompiler =
Expand Down
47 changes: 40 additions & 7 deletions tests/unit/src/main/scala/tests/TestingServer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1821,7 +1821,7 @@ final case class TestingServer(
)
}

val label = parents.iterator
val labelsMap = parents.iterator
.flatMap { r =>
r.nodes.iterator.map { n =>
val icon = Option(n.icon) match {
Expand All @@ -1835,16 +1835,34 @@ final case class TestingServer(
.toMap
.updated("root", "root")

def label(uri: String, default: String): String =
labelsMap.get(uri) match {
/* fallback for JDK sources since we seem to have issues on CI
* with multiple JDK homes and this is just for the label
*/
// case _ if uri.contains("src.zip") =>
// val last = uri.split("src.zip").last
// labelsMap
// .collectFirst {
// case (k, v) if k.endsWith(last) && k.contains("src.zip") => v
// }
// .getOrElse(default)
case None =>
scribe.warn(s"Cannot find label for $uri")
scribe.warn(labelsMap.mkString("\n"))
default
case Some(value) => value
}
val tree = parents
.zip(reveal.uriChain :+ "root")
.foldLeft(PrettyPrintTree.empty) { case (child, (parent, uri)) =>
val realUri =
if (uri.contains("-sources.jar")) uri
else uri.replace(".jar", "-sources.jar")
PrettyPrintTree(
label.getOrElse(realUri.toLowerCase, realUri),
label(realUri.toLowerCase, realUri),
parent.nodes
.map(n => PrettyPrintTree(label(n.nodeUri.toLowerCase)))
.map(n => PrettyPrintTree(label(n.nodeUri.toLowerCase, n.nodeUri)))
.filterNot(t => isIgnored(t.value))
.toList :+ child,
)
Expand Down Expand Up @@ -1879,12 +1897,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(
Expand Down
3 changes: 2 additions & 1 deletion tests/unit/src/test/scala/tests/TreeViewLspSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import scala.meta.io.AbsolutePath
*/
class TreeViewLspSuite extends BaseLspSuite("tree-view") {

scribe.info("Using for tests" + Paths.get(Properties.javaHome).toString())
private val javaVersion =
JdkVersion
.fromReleaseFileString(AbsolutePath(Paths.get(Properties.javaHome)))
Expand Down Expand Up @@ -160,7 +161,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#",
Expand Down

0 comments on commit 82738ec

Please sign in to comment.