diff --git a/metals-bench/src/main/scala/bench/ClasspathOnlySymbolSearch.scala b/metals-bench/src/main/scala/bench/ClasspathOnlySymbolSearch.scala index ceed77f0dae..b9e088381d1 100644 --- a/metals-bench/src/main/scala/bench/ClasspathOnlySymbolSearch.scala +++ b/metals-bench/src/main/scala/bench/ClasspathOnlySymbolSearch.scala @@ -21,6 +21,9 @@ class ClasspathOnlySymbolSearch(classpath: ClasspathSearch) def definition(symbol: String): ju.List[Location] = ju.Collections.emptyList() + override def definitionSourceToplevels(symbol: String): ju.List[String] = + ju.Collections.emptyList() + override def search( query: String, buildTargetIdentifier: String, diff --git a/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala index 03bd45ecaff..f363244d090 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/DefinitionProvider.scala @@ -73,6 +73,17 @@ final class DefinitionProvider( case Some(destination) => destination.locations } + /** + * Returns VirtualFile that contains the definition of + * the given symbol (of semanticdb). + */ + def definitionPathInputFromSymbol( + sym: String + ): Option[Input.VirtualFile] = + index + .definition(Symbol(sym)) + .map(symDef => symDef.path.toInputFromBuffers(buffers)) + def symbolOccurence( source: AbsolutePath, dirtyPosition: TextDocumentPositionParams diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala index d453f3e066a..2576b62b4f6 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala @@ -173,6 +173,7 @@ class MetalsLanguageServer( private var foldingRangeProvider: FoldingRangeProvider = _ private val packageProvider: PackageProvider = new PackageProvider(buildTargets) + private var symbolSearch: MetalsSymbolSearch = _ private var compilers: Compilers = _ var tables: Tables = _ var statusBar: StatusBar = _ @@ -386,6 +387,11 @@ class MetalsLanguageServer( interactiveSemanticdbs.toFileOnDisk ) foldingRangeProvider = FoldingRangeProvider(trees, buffers, params) + symbolSearch = new MetalsSymbolSearch( + symbolDocs, + workspaceSymbols, + definitionProvider + ) compilers = register( new Compilers( workspace, @@ -393,11 +399,7 @@ class MetalsLanguageServer( () => userConfig, buildTargets, buffers, - new MetalsSymbolSearch( - symbolDocs, - workspaceSymbols, - definitionProvider - ), + symbolSearch, embedded, statusBar, sh, @@ -1597,6 +1599,7 @@ class MetalsLanguageServer( semanticDBIndexer.reset() treeView.reset() worksheetProvider.reset() + symbolSearch.reset() buildTargets.addWorkspaceBuildTargets(i.workspaceBuildTargets) buildTargets.addScalacOptions(i.scalacOptions) for { diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsSymbolSearch.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsSymbolSearch.scala index e010046d114..991ca8e7c08 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/MetalsSymbolSearch.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/MetalsSymbolSearch.scala @@ -3,10 +3,14 @@ package scala.meta.internal.metals import java.{util => ju} import org.eclipse.lsp4j.Location import java.util.Optional +import scala.collection.concurrent.TrieMap import ch.epfl.scala.bsp4j.BuildTargetIdentifier import scala.meta.pc.SymbolDocumentation import scala.meta.pc.SymbolSearch import scala.meta.pc.SymbolSearchVisitor +import scala.meta.internal.mtags.Mtags +import scala.meta.io.AbsolutePath +import scala.meta.internal.metals.MetalsEnrichments._ /** * Implementation of SymbolSearch that delegates to WorkspaceSymbolProvider and SymbolDocumentationIndexer. @@ -16,6 +20,16 @@ class MetalsSymbolSearch( wsp: WorkspaceSymbolProvider, defn: DefinitionProvider ) extends SymbolSearch { + // A cache for definitionSourceToplevels. + // The key is an absolutepath to the dependency source file, and + // the value is the list of symbols that the file contains. + private val dependencySourceCache = + new TrieMap[AbsolutePath, ju.List[String]]() + + def reset(): Unit = { + dependencySourceCache.clear() + } + override def documentation(symbol: String): Optional[SymbolDocumentation] = docs.documentation(symbol) @@ -23,6 +37,44 @@ class MetalsSymbolSearch( defn.fromSymbol(symbol) } + /** + * Returns a list of semanticdb symbols in a source file that contains the + * definition of the given symbol. + */ + override def definitionSourceToplevels(symbol: String): ju.List[String] = { + defn + .definitionPathInputFromSymbol(symbol) + .map(input => { + val path = AbsolutePath(input.path) + if (path.isWorkspaceSource(wsp.workspace)) { + // If the source file is a workspace source, retrieve its symbols from + // WorkspaceSymbolProvider so that metals server can reuse its cache. + wsp.inWorkspace + .get(path.toNIO) + .map(symInfo => { + symInfo.symbols + .sortBy(sym => + ( + sym.range.getStart().getLine(), + sym.range.getStart().getCharacter() + ) + ) + .map(_.symbol) + .asJava + }) + .getOrElse( + ju.Collections.emptyList[String]() + ) + } else { + dependencySourceCache.getOrElseUpdate( + path, + Mtags.toplevels(input).asJava + ) + } + }) + .getOrElse(ju.Collections.emptyList()) + } + override def search( query: String, buildTargetIdentifier: String, diff --git a/mtags-interfaces/src/main/java/scala/meta/pc/SymbolSearch.java b/mtags-interfaces/src/main/java/scala/meta/pc/SymbolSearch.java index a1078bb27e9..a38539f9143 100644 --- a/mtags-interfaces/src/main/java/scala/meta/pc/SymbolSearch.java +++ b/mtags-interfaces/src/main/java/scala/meta/pc/SymbolSearch.java @@ -19,6 +19,12 @@ public interface SymbolSearch { */ List definition(String symbol); + /** + * Returns the all symbols in the file where the given symbol is defined + * in declaration order, if any. + */ + List definitionSourceToplevels(String symbol); + /** * Runs fuzzy symbol search for the given query. * diff --git a/mtags/src/main/scala/scala/meta/internal/pc/Completions.scala b/mtags/src/main/scala/scala/meta/internal/pc/Completions.scala index ebfb1be43f9..8ef7717bc5b 100644 --- a/mtags/src/main/scala/scala/meta/internal/pc/Completions.scala +++ b/mtags/src/main/scala/scala/meta/internal/pc/Completions.scala @@ -1297,7 +1297,41 @@ trait Completions { this: MetalsGlobal => val members = ListBuffer.empty[TextEditMember] val importPos = autoImportPosition(pos, text) val context = doLocateImportContext(pos, importPos) + val subclasses = ListBuffer.empty[Symbol] + tpe.typeSymbol.foreachKnownDirectSubClass { sym => + subclasses += sym + } + val subclassesResult = subclasses.result() + + // sort subclasses by declaration order + // see: https://github.com/scalameta/metals-feature-requests/issues/49 + val sortedSubclasses = + if (subclassesResult.forall(_.pos.isDefined)) { + // if all the symbols of subclasses' position is defined + // we can sort those symbols by declaration order + // based on their position information quite cheaply + subclassesResult.sortBy(subclass => + (subclass.pos.line, subclass.pos.column) + ) + } else { + // Read all the symbols in the source that contains + // the definition of the symbol in declaration order + val defnSymbols = search + .definitionSourceToplevels(semanticdbSymbol(tpe.typeSymbol)) + .asScala + if (defnSymbols.length > 0) { + val symbolIdx = defnSymbols.zipWithIndex.toMap + subclassesResult + .sortBy(sym => { + symbolIdx.getOrElse(semanticdbSymbol(sym), -1) + }) + } else { + subclassesResult + } + } + + sortedSubclasses.foreach { sym => val (shortName, edits) = importPos match { case Some(value) => diff --git a/mtags/src/main/scala/scala/meta/internal/pc/EmptySymbolSearch.scala b/mtags/src/main/scala/scala/meta/internal/pc/EmptySymbolSearch.scala index e65a1a42a42..bac2c4bf602 100644 --- a/mtags/src/main/scala/scala/meta/internal/pc/EmptySymbolSearch.scala +++ b/mtags/src/main/scala/scala/meta/internal/pc/EmptySymbolSearch.scala @@ -20,6 +20,10 @@ object EmptySymbolSearch extends SymbolSearch { ju.Collections.emptyList() } + override def definitionSourceToplevels(symbol: String): ju.List[String] = { + ju.Collections.emptyList() + } + override def documentation(symbol: String): Optional[SymbolDocumentation] = Optional.empty() } diff --git a/tests/cross/src/test/scala/tests/pc/CompletionMatchSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionMatchSuite.scala index 6d19e27ca00..f5e274efe08 100644 --- a/tests/cross/src/test/scala/tests/pc/CompletionMatchSuite.scala +++ b/tests/cross/src/test/scala/tests/pc/CompletionMatchSuite.scala @@ -3,6 +3,10 @@ package tests.pc import tests.BaseCompletionSuite object CompletionMatchSuite extends BaseCompletionSuite { + override def beforeAll(): Unit = { + indexScalaLibrary() + } + check( "match", """ @@ -103,6 +107,61 @@ object CompletionMatchSuite extends BaseCompletionSuite { filter = _.contains("exhaustive") ) + checkEdit( + "exhaustive-sorting", + """package sort + |sealed abstract class TestTree + |case class Branch1(t1: TestTree) extends TestTree + |case class Leaf(v: Int) extends TestTree + |case class Branch2(t1: TestTree, t2: TestTree) extends TestTree + |object App { + | null.asInstanceOf[TestTree] matc@@ + |} + |""".stripMargin, + """|package sort + |sealed abstract class TestTree + |case class Branch1(t1: TestTree) extends TestTree + |case class Leaf(v: Int) extends TestTree + |case class Branch2(t1: TestTree, t2: TestTree) extends TestTree + |object App { + | null.asInstanceOf[TestTree] match { + |\tcase Branch1(t1) => $0 + |\tcase Leaf(v) => + |\tcase Branch2(t1, t2) => + |} + |} + |""".stripMargin, + filter = _.contains("exhaustive") + ) + + checkEdit( + "exhaustive-sorting-scalalib", + """package sort + |object App { + | Option(1) matc@@ + |} + |""".stripMargin, + if (!isScala211) + """package sort + |object App { + | Option(1) match { + |\tcase Some(value) => $0 + |\tcase None => + |} + |} + |""".stripMargin + else + """package sort + |object App { + | Option(1) match { + |\tcase Some(x) => $0 + |\tcase None => + |} + |} + |""".stripMargin, + filter = _.contains("exhaustive") + ) + check( "inner-class", """ diff --git a/tests/mtest/src/main/scala/tests/TestingSymbolSearch.scala b/tests/mtest/src/main/scala/tests/TestingSymbolSearch.scala index d90e2aade06..bcfae96b685 100644 --- a/tests/mtest/src/main/scala/tests/TestingSymbolSearch.scala +++ b/tests/mtest/src/main/scala/tests/TestingSymbolSearch.scala @@ -3,6 +3,8 @@ package tests import java.{util => ju} import org.eclipse.lsp4j.Location import java.util.Optional +import java.nio.file.Files +import scala.meta.inputs.Input import scala.meta.internal.metals.ClasspathSearch import scala.meta.internal.metals.Docstrings import scala.meta.internal.metals.WorkspaceSymbolQuery @@ -12,6 +14,7 @@ import scala.meta.pc.SymbolSearchVisitor import scala.meta.internal.mtags.OnDemandSymbolIndex import scala.meta.internal.mtags.GlobalSymbolIndex import scala.meta.internal.mtags.Symbol +import scala.meta.internal.mtags.Mtags /** * Implementation of `SymbolSearch` for testing purposes. @@ -46,6 +49,22 @@ class TestingSymbolSearch( } } + override def definitionSourceToplevels(symbol: String): ju.List[String] = { + index.definition(Symbol(symbol)) match { + case None => + ju.Collections.emptyList() + case Some(value) => + import scala.collection.JavaConverters._ + val filename = value.path.toNIO.getFileName().toString() + val content = new String(Files.readAllBytes(value.path.toNIO)) + val input = Input.VirtualFile( + filename, + content + ) + Mtags.toplevels(input).asJava + } + } + override def search( textQuery: String, buildTargetIdentifier: String, diff --git a/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala b/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala index 55bafeba92a..20a32f5a2af 100644 --- a/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala +++ b/tests/slow/src/test/scala/tests/feature/CompletionCrossLspSuite.scala @@ -11,4 +11,10 @@ object CompletionCrossLspSuite testAsync("basic-213") { basicTest(V.scala213) } + testAsync("match-211") { + matchKeywordTest(V.scala213) + } + testAsync("match-213") { + matchKeywordTest(V.scala213) + } } diff --git a/tests/unit/src/main/scala/tests/BaseCompletionLspSuite.scala b/tests/unit/src/main/scala/tests/BaseCompletionLspSuite.scala index b083232e895..cc80181cd95 100644 --- a/tests/unit/src/main/scala/tests/BaseCompletionLspSuite.scala +++ b/tests/unit/src/main/scala/tests/BaseCompletionLspSuite.scala @@ -2,6 +2,7 @@ package tests import org.eclipse.lsp4j.CompletionList import scala.concurrent.Future +import scala.meta.internal.metals.TextEdits abstract class BaseCompletionLspSuite(name: String) extends BaseLspSuite(name) { @@ -33,6 +34,40 @@ abstract class BaseCompletionLspSuite(name: String) extends BaseLspSuite(name) { } } + def withCompletionEdit( + query: String, + project: Char = 'a', + filter: String => Boolean = _ => true + )( + fn: String => Unit + ): Future[Unit] = { + import scala.collection.JavaConverters._ + val filename = s"$project/src/main/scala/$project/${project.toUpper}.scala" + val text = server + .textContentsOnDisk(filename) + .replaceAllLiterally("// @@", query.replaceAllLiterally("@@", "")) + for { + _ <- server.didChange(filename)(_ => text) + completion <- server.completionList(filename, query) + } yield { + val items = + completion.getItems().asScala.filter(item => filter(item.getLabel)) + val obtained = TextEdits.applyEdits(text, items.head) + fn(obtained) + } + } + + def assertCompletionEdit( + query: String, + expected: String, + project: Char = 'a', + filter: String => Boolean = _ => true + ): Future[Unit] = { + withCompletionEdit(query, project, filter) { obtained => + assertNoDiff(obtained, expected) + } + } + def basicTest(scalaVersion: String): Future[Unit] = { cleanWorkspace() for { @@ -125,4 +160,60 @@ abstract class BaseCompletionLspSuite(name: String) extends BaseLspSuite(name) { ) } yield () } + + def matchKeywordTest(scalaVersion: String): Future[Unit] = { + cleanWorkspace() + for { + _ <- server.initialize( + s"""/metals.json + |{ + | "a": { "scalaVersion": "${scalaVersion}" } + |} + |/a/src/main/scala/a/A.scala + |package a + |object A { + | val x: Option[Int] = Some(1) + | // @@ + |} + |/a/src/main/scala/a/Color.scala + |package a + |abstract sealed class Color + |case object Red extends Color + |case object Blue extends Color + |case object Green extends Color + |""".stripMargin + ) + _ <- server.didOpen("a/src/main/scala/a/A.scala") + _ = assertNoDiagnostics() + // completed exhausted matches should be sorted by declaration order + // https://github.com/scala/scala/blob/cca78e1e18c55e5b0223b9dfa4ac230f7bc6a858/src/library/scala/Option.scala#L513-L527 + _ <- assertCompletionEdit( + "x matc@@", + """|package a + |object A { + | val x: Option[Int] = Some(1) + | x match { + |\tcase Some(value) => + |\tcase None => + |} + |} + |""".stripMargin, + filter = _.contains("exhaustive") + ) + _ <- assertCompletionEdit( + "null.asInstanceOf[Color] matc@@", + """|package a + |object A { + | val x: Option[Int] = Some(1) + | null.asInstanceOf[Color] match { + |\tcase Red => + |\tcase Blue => + |\tcase Green => + |} + |} + |""".stripMargin, + filter = _.contains("exhaustive") + ) + } yield () + } }