From 0aba1e227f27dc08fb1aeff6d1d23600b9dbc33e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Mon, 9 Apr 2018 17:36:03 +0200 Subject: [PATCH 01/25] Upgrade build dependency --- sbt-metals/src/main/scala/scala/meta/sbt/MetalsPlugin.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sbt-metals/src/main/scala/scala/meta/sbt/MetalsPlugin.scala b/sbt-metals/src/main/scala/scala/meta/sbt/MetalsPlugin.scala index 656ea9bf40e..5189fba9e08 100644 --- a/sbt-metals/src/main/scala/scala/meta/sbt/MetalsPlugin.scala +++ b/sbt-metals/src/main/scala/scala/meta/sbt/MetalsPlugin.scala @@ -40,7 +40,7 @@ package scala.meta.sbt { def supportedScalaVersions = List(scala212, scala211) - def semanticdbVersion = "2.1.7" + def semanticdbVersion = "3.7.2" val metalsBuildInfo = taskKey[Map[String, String]]( "List of key/value pairs for build information such as classpath/sourceDirectories" From 633bbc298281da14fc3d17615040f757201d0471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Mon, 9 Apr 2018 19:23:04 +0200 Subject: [PATCH 02/25] Fix all compile errors --- build.sbt | 4 +- .../{metaserver.proto => metals.proto} | 2 +- .../languageserver/InputEnrichments.scala | 12 +- .../main/scala/scala/meta/metals/Linter.scala | 2 +- .../scala/meta/metals/MetalsServices.scala | 14 +- .../meta/metals/ScalametaEnrichments.scala | 31 +++- .../scala/scala/meta/metals/Semanticdbs.scala | 67 ++++--- .../scala/meta/metals/mtags/JavaMtags.scala | 49 ++--- .../scala/scala/meta/metals/mtags/Mtags.scala | 40 ++-- .../meta/metals/mtags/MtagsIndexer.scala | 62 ++++--- .../scala/meta/metals/mtags/ScalaMtags.scala | 54 +++--- .../meta/metals/providers/HoverProvider.scala | 175 +----------------- .../meta/metals/search/DocumentIndex.scala | 6 +- .../metals/search/InMemoryDocumentIndex.scala | 8 +- .../metals/search/InMemorySymbolIndex.scala | 68 ++++--- .../metals/search/InMemorySymbolIndexer.scala | 4 +- .../metals/search/InverseSymbolIndexer.scala | 4 +- .../meta/metals/search/SymbolIndex.scala | 4 +- .../meta/metals/search/SymbolIndexer.scala | 5 +- .../scala/meta/metals/storage/Bytes.scala | 8 +- .../languageserver/ScalafixEnrichments.scala | 5 +- project/plugins.sbt | 6 +- 22 files changed, 268 insertions(+), 362 deletions(-) rename metals/src/main/protobuf/{metaserver.proto => metals.proto} (98%) diff --git a/build.sbt b/build.sbt index f5cc078d0c7..beedfeb4ebf 100644 --- a/build.sbt +++ b/build.sbt @@ -79,7 +79,7 @@ lazy val V = new { val scala211 = MetalsPlugin.scala211 val scala212 = MetalsPlugin.scala212 val scalameta = MetalsPlugin.semanticdbVersion - val scalafix = "0.5.7" + val scalafix = "0.6.0-M1" val enumeratum = "1.5.12" val circe = "0.9.0" val cats = "1.0.1" @@ -149,7 +149,7 @@ lazy val metals = project "io.github.soc" % "directories" % "5", // for cache location "me.xdrop" % "fuzzywuzzy" % "1.1.9", // for workspace/symbol "org.fusesource.leveldbjni" % "leveldbjni-all" % "1.8", // for caching classpath index - "org.scalameta" %% "semanticdb-scalac" % V.scalameta cross CrossVersion.full, + "org.scalameta" % "interactive" % V.scalameta cross CrossVersion.full, "org.scalameta" %% "testkit" % V.scalameta % Test ) ) diff --git a/metals/src/main/protobuf/metaserver.proto b/metals/src/main/protobuf/metals.proto similarity index 98% rename from metals/src/main/protobuf/metaserver.proto rename to metals/src/main/protobuf/metals.proto index 0c998785fea..a83ec9eda6f 100644 --- a/metals/src/main/protobuf/metaserver.proto +++ b/metals/src/main/protobuf/metals.proto @@ -8,7 +8,7 @@ message SymbolData { // The Position where this symbol is defined. Position definition = 2; map references = 3; - int64 flags = 4; + int32 kind = 4; string name = 5; string signature = 6; // Planned for Scalameta v2.2, see https://github.com/scalameta/scalameta/milestone/9 diff --git a/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala b/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala index b5634a7a18a..02ed3e97276 100644 --- a/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala +++ b/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala @@ -4,6 +4,7 @@ import org.langmeta.lsp import org.langmeta.inputs.Input import org.langmeta.inputs.Position import scala.meta.metals.{index => i} +import scala.meta.internal.semanticdb3 object InputEnrichments { implicit class XtensionPositionOffset(val pos: Position) extends AnyVal { @@ -24,13 +25,12 @@ object InputEnrichments { } } implicit class XtensionInputOffset(val input: Input) extends AnyVal { - def toIndexRange(start: Int, end: Int): i.Range = { - val pos = Position.Range(input, start, end) + def toIndexRange(r: semanticdb3.Range): i.Range = { i.Range( - startLine = pos.startLine, - startColumn = pos.startColumn, - endLine = pos.endLine, - endColumn = pos.endColumn + startLine = r.startLine, + startColumn = r.startCharacter, + endLine = r.endLine, + endColumn = r.endCharacter ) } diff --git a/metals/src/main/scala/scala/meta/metals/Linter.scala b/metals/src/main/scala/scala/meta/metals/Linter.scala index d0360f4c8dc..0f5e3742296 100644 --- a/metals/src/main/scala/scala/meta/metals/Linter.scala +++ b/metals/src/main/scala/scala/meta/metals/Linter.scala @@ -106,7 +106,7 @@ class Linter(configuration: Observable[Configuration], cwd: AbsolutePath)( case Parsed.Success(tree) => val ctx = RuleCtx.applyInternal(tree, config) val patches = rule.fixWithNameInternal(ctx) - Patch.lintMessagesInternal(patches, ctx).map(_.toLSP) + Patch.lintMessagesInternal(patches, ctx, index).map(_.toLSP) } // megaCache needs to die, if we forget this we will read stale diff --git a/metals/src/main/scala/scala/meta/metals/MetalsServices.scala b/metals/src/main/scala/scala/meta/metals/MetalsServices.scala index bc28bc3715d..efbfc87d2a6 100644 --- a/metals/src/main/scala/scala/meta/metals/MetalsServices.scala +++ b/metals/src/main/scala/scala/meta/metals/MetalsServices.scala @@ -44,12 +44,12 @@ import monix.reactive.Observable import monix.reactive.Observer import monix.reactive.OverflowStrategy import org.langmeta.inputs.Input -import org.langmeta.internal.semanticdb.XtensionDatabase -import org.langmeta.internal.semanticdb.schema +import scala.meta.internal.semanticdb3 +import scala.{meta => m} import org.langmeta.io.AbsolutePath import org.langmeta.languageserver.InputEnrichments._ import org.langmeta.lsp.LanguageClient -import org.langmeta.semanticdb +import org.langmeta.internal.semanticdb._ class MetalsServices( cwd: AbsolutePath, @@ -87,7 +87,7 @@ class MetalsServices( val diagnosticsProvider = new DiagnosticsProvider(configurationPublisher, cwd) val scalacProvider = new ScalacProvider - val interactiveSemanticdbs: Observable[semanticdb.Database] = + val interactiveSemanticdbs: Observable[m.Database] = sourceChangePublisher .debounce(FiniteDuration(1, "s")) .flatMap { input => @@ -97,9 +97,9 @@ class MetalsServices( .executeOn(presentationCompilerScheduler) } else Observable.empty } - val interactiveSchemaSemanticdbs: Observable[schema.Database] = + val interactiveSchemaSemanticdbs: Observable[semanticdb3.TextDocuments] = interactiveSemanticdbs.flatMap(db => Observable(db.toSchema(cwd))) - val metaSemanticdbs: Observable[semanticdb.Database] = + val metaSemanticdbs: Observable[m.Database] = Observable.merge( fileSystemSemanticdbsPublisher.map(_.toDb(sourcepath = None)), interactiveSemanticdbs @@ -565,7 +565,7 @@ object MetalsServices extends LazyLogging { def fileSystemSemanticdbStream(cwd: AbsolutePath)( implicit scheduler: Scheduler - ): (Observer.Sync[AbsolutePath], Observable[schema.Database]) = { + ): (Observer.Sync[AbsolutePath], Observable[semanticdb3.TextDocuments]) = { val (subscriber, publisher) = multicast[AbsolutePath]() val semanticdbPublisher = publisher .map(path => Semanticdbs.loadFromFile(semanticdbPath = path, cwd)) diff --git a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala index 4c4fd47cf0a..1a2d161f875 100644 --- a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala +++ b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala @@ -8,12 +8,14 @@ import org.langmeta.lsp.Diagnostic import org.langmeta.lsp.Location import org.langmeta.lsp.Position import scala.meta.metals.{index => i} -import org.langmeta.internal.semanticdb.{schema => s} +import scala.meta.internal.semanticdb3 import scala.{meta => m} import org.langmeta.lsp.SymbolKind import org.langmeta.{lsp => l} import org.langmeta.internal.io.FileIO import org.langmeta.io.AbsolutePath +import org.langmeta.internal.semanticdb._ +import scala.meta.internal.semanticdb3.SymbolInformation.Kind // Extension methods for convenient reuse of data conversions between // scala.meta._ and language.types._ @@ -35,6 +37,7 @@ object ScalametaEnrichments { case m.Severity.Info => l.DiagnosticSeverity.Information case m.Severity.Warning => l.DiagnosticSeverity.Warning case m.Severity.Error => l.DiagnosticSeverity.Error + case m.Severity.Hint => l.DiagnosticSeverity.Hint } } @@ -50,7 +53,7 @@ object ScalametaEnrichments { case d: Defn.Var => d.decltpe case _ => None } - tpeOpt.filter(_.is[Type.Function]).nonEmpty + tpeOpt.exists(_.is[Type.Function]) } // NOTE: we care only about descendants of Decl, Defn and Pkg[.Object] (see documentSymbols implementation) @@ -150,6 +153,14 @@ object ScalametaEnrichments { def toLanguageServerUri: String = "file:" + path.toString() } implicit class XtensionPositionRangeLSP(val pos: m.Position) extends AnyVal { + def toSchemaRange: semanticdb3.Range = { + semanticdb3.Range( + startLine = pos.startLine, + startCharacter = pos.startColumn, + endLine = pos.endLine, + endCharacter = pos.endColumn + ) + } def toIndexRange: i.Range = i.Range( startLine = pos.startLine, startColumn = pos.startColumn, @@ -312,11 +323,23 @@ object ScalametaEnrichments { } } - implicit class XtensionSchemaDocument(val document: s.Document) + implicit class XtensionSchemaDocument(val document: semanticdb3.TextDocument) extends AnyVal { /** Returns scala.meta.Document from protobuf schema.Document */ def toMetaDocument: m.Document = - s.Database(document :: Nil).toDb(None).documents.head + semanticdb3.TextDocuments(document :: Nil).toDb(None).documents.head + } + + implicit class XtensionIntAsSymbolKind(val flags: Int) { + def hasOneOfFlags(flags: Long): Boolean = + (this.flags & flags) != 0L + def toSymbolKind: SymbolKind = + if (hasOneOfFlags(Kind.CLASS.value)) + SymbolKind.Class + else if (hasOneOfFlags(Kind.TRAIT.value | Kind.INTERFACE.value)) + SymbolKind.Interface + else + SymbolKind.Module } } diff --git a/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala b/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala index 5ef8e55e6fb..d0b753a43d1 100644 --- a/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala +++ b/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala @@ -5,7 +5,7 @@ import scala.meta.interactive.InteractiveSemanticdb import scala.meta.metals.compiler.ScalacProvider import scala.meta.metals.compiler.CompilerEnrichments._ import scala.meta.parsers.ParseException -import scala.meta.semanticdb +import scala.{meta => m} import scala.meta.tokenizers.TokenizeException import scala.tools.nsc.interactive.Global import scala.tools.nsc.reporters.StoreReporter @@ -14,9 +14,10 @@ import scala.{meta => m} import com.typesafe.scalalogging.LazyLogging import org.langmeta.inputs.Input import org.langmeta.internal.io.PathIO -import org.langmeta.internal.semanticdb.schema.Database +import scala.meta.internal.semanticdb3 import org.langmeta.io.AbsolutePath import org.langmeta.io.RelativePath +import scala.meta.internal.semanticdb3.SymbolOccurrence object Semanticdbs extends LazyLogging { @@ -28,7 +29,7 @@ object Semanticdbs extends LazyLogging { def toSemanticdb( input: Input.VirtualFile, scalacProvider: ScalacProvider - ): Option[semanticdb.Database] = + ): Option[m.Database] = for { compiler <- scalacProvider.getCompiler(input) } yield toSemanticdb(input, compiler) @@ -36,7 +37,7 @@ object Semanticdbs extends LazyLogging { def toSemanticdb( input: Input.VirtualFile, compiler: Global, - ): semanticdb.Database = { + ): m.Database = { val doc = try { InteractiveSemanticdb.toDocument( compiler = compiler, @@ -54,7 +55,7 @@ object Semanticdbs extends LazyLogging { } toMessageOnlySemanticdb(input, compiler) } - semanticdb.Database(doc.copy(language = "Scala212") :: Nil) + m.Database(doc.copy(language = "Scala212") :: Nil) } def toMessageOnlySemanticdb( @@ -75,30 +76,50 @@ object Semanticdbs extends LazyLogging { }.toList case _ => Nil } - semanticdb.Document(input, "", Nil, messages, Nil, Nil) + m.Document(input, "", Nil, messages, Nil, Nil) } + implicit private val occurrenceOrdering: Ordering[SymbolOccurrence] = + new Ordering[semanticdb3.SymbolOccurrence] { + override def compare(x: SymbolOccurrence, y: SymbolOccurrence): Int = { + if (x.range.isEmpty || x.range.isEmpty) 0 + else { + val byLine = Integer.compare( + x.range.get.startLine, + y.range.get.startLine + ) + if (byLine != 0) { + byLine + } else { + val byCharacter = Integer.compare( + x.range.get.startCharacter, + y.range.get.startCharacter + ) + byCharacter + } + } + } + } + def loadFromFile( semanticdbPath: AbsolutePath, cwd: AbsolutePath - ): Database = { + ): semanticdb3.TextDocuments = { val bytes = Files.readAllBytes(semanticdbPath.toNIO) - val sdb = Database.parseFrom(bytes) - Database( - sdb.documents.map { d => - val filename = s"file:${cwd.resolve(d.filename)}" - logger.info(s"Loading file $filename") - d.withFilename(filename) - .withNames { - // This should be done inside semanticdb-scalac. - val names = d.names.toArray - util.Sorting.quickSort(names)( - Ordering.by(_.position.fold(-1)(_.start)) - ) - names - } - } - ) + val sdb = semanticdb3.TextDocuments.parseFrom(bytes) + val docs = sdb.documents.map { d => + val filename = s"file:${cwd.resolve(d.uri)}" + logger.info(s"Loading file $filename") + d.withUri(filename) + .withOccurrences { + // This should be done inside semanticdb-scalac. + val names = d.occurrences.toArray + util.Sorting.quickSort(names) + names + } + + } + semanticdb3.TextDocuments(docs) } } diff --git a/metals/src/main/scala/scala/meta/metals/mtags/JavaMtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/JavaMtags.scala index 26148ab7a2f..5cc0ad94591 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/JavaMtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/JavaMtags.scala @@ -1,13 +1,8 @@ package scala.meta.metals.mtags import java.io.StringReader -import scala.meta.CLASS -import scala.meta.DEF -import scala.meta.OBJECT -import scala.meta.PACKAGE -import scala.meta.TRAIT -import scala.meta.VAL -import scala.meta.VAR +import scala.meta.internal.semanticdb3.SymbolInformation.Property +import scala.meta.internal.semanticdb3.SymbolInformation.Kind import com.thoughtworks.qdox._ import com.thoughtworks.qdox.model.JavaClass import com.thoughtworks.qdox.model.JavaField @@ -17,6 +12,7 @@ import com.thoughtworks.qdox.model.JavaModel import org.langmeta.inputs.Input import org.langmeta.inputs.Position import org.langmeta.languageserver.InputEnrichments._ +import scala.meta.internal.semanticdb3.Language object JavaMtags { private implicit class XtensionJavaModel(val m: JavaModel) extends AnyVal { @@ -30,7 +26,12 @@ object JavaMtags { val source = builder.addSource(new StringReader(input.value)) if (source.getPackage != null) { source.getPackageName.split("\\.").foreach { p => - term(p, toRangePosition(source.getPackage.lineNumber, p), PACKAGE) + term( + p, + toRangePosition(source.getPackage.lineNumber, p), + Kind.PACKAGE, + 0 + ) } } source.getClasses.forEach(visitClass) @@ -79,14 +80,14 @@ object JavaMtags { def visitClass(cls: JavaClass): Unit = withOwner(owner(cls.isStatic)) { - val flags = if (cls.isInterface) TRAIT else CLASS + val kind = if (cls.isInterface) Kind.INTERFACE else Kind.CLASS val pos = toRangePosition(cls.lineNumber, cls.getName) - if (cls.isEnum) { - term(cls.getName, pos, OBJECT) - } else { - withOwner() { term(cls.getName, pos, OBJECT) } // object - tpe(cls.getName, pos, flags) - } + tpe( + cls.getName, + pos, + kind, + if (cls.isEnum) Property.ENUM.value else 0 + ) visitClasses(cls.getNestedClasses) visitFields(cls.getMethods) visitFields(cls.getFields) @@ -103,17 +104,17 @@ object JavaMtags { case _ => 0 } val pos = toRangePosition(line, name) - val flags: Long = m match { - case c: JavaMethod => DEF - case c: JavaField => - if (c.isFinal || c.isEnumConstant) VAL - else VAR - // TODO(olafur) handle constructos - case _ => 0L + val kind: Kind = m match { + case _: JavaMethod => Kind.METHOD + case _: JavaField => Kind.FIELD + case c: JavaClass => + if (c.isInterface) Kind.INTERFACE + else Kind.CLASS + case _ => Kind.UNKNOWN_KIND } - term(name, pos, flags) + term(name, pos, kind, 0) } - override def language: String = "Java" + override def language: Language = Language.JAVA } } diff --git a/metals/src/main/scala/scala/meta/metals/mtags/Mtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/Mtags.scala index 1d2b39fdf28..969b441abad 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/Mtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/Mtags.scala @@ -25,8 +25,9 @@ import org.langmeta.internal.io.PathIO import org.langmeta.io.AbsolutePath import org.langmeta.io.Fragment import org.langmeta.io.RelativePath -import org.langmeta.internal.semanticdb.schema.Database -import org.langmeta.internal.semanticdb.schema.Document +import scala.meta.internal.semanticdb3.Schema +import scala.meta.internal.semanticdb3.TextDocuments +import scala.meta.internal.semanticdb3.TextDocument /** * Syntactically build a semanticdb index containing only global symbol definition. @@ -51,7 +52,7 @@ object Mtags extends LazyLogging { def index( classpath: List[AbsolutePath], shouldIndex: RelativePath => Boolean = _ => true - )(callback: Document => Unit): Unit = { + )(callback: TextDocument => Unit): Unit = { val fragments = allClasspathFragments(classpath, shouldIndex) val totalIndexedFiles = new AtomicInteger() val totalIndexedLines = new AtomicInteger() @@ -66,10 +67,10 @@ object Mtags extends LazyLogging { } val decimal = new DecimalFormat("###,###") val N = fragments.length - def updateTotalLines(doc: Document): Unit = { + def updateTotalLines(doc: TextDocument): Unit = { // NOTE(olafur) it would be interesting to have separate statistics for // Java/Scala lines/s but that would require a more sophisticated setup. - totalIndexedLines.addAndGet(countLines(doc.contents)) + totalIndexedLines.addAndGet(countLines(doc.text)) } def reportProgress(indexedFiles: Int): Unit = { if (indexedFiles < 100) return @@ -105,24 +106,24 @@ object Mtags extends LazyLogging { ) } - /** Index all documents into a single scala.meta.Database. */ + /** Index all documents into a single scala.meta.internal.semanticdb3.TextDocuments. */ def indexDatabase( classpath: List[AbsolutePath], shouldIndex: RelativePath => Boolean = _ => true - ): Database = { - val buffer = List.newBuilder[Document] + ): TextDocuments = { + val buffer = List.newBuilder[TextDocument] index(classpath, shouldIndex) { doc => buffer += doc } - Database(buffer.result()) + TextDocuments(buffer.result()) } /** Index single Scala or Java source file from memory */ - def index(filename: String, contents: String): Document = + def index(filename: String, contents: String): TextDocument = index(Input.VirtualFile(filename, contents)) /** Index single Scala or Java from disk or zip file. */ - def index(fragment: Fragment): Document = { + def index(fragment: Fragment): TextDocument = { val uri = { // Need special handling because https://github.com/scalameta/scalameta/issues/1163 if (isZip(fragment.base.toNIO.getFileName.toString)) @@ -134,7 +135,7 @@ object Mtags extends LazyLogging { } /** Index single Scala or Java source file from memory */ - def index(input: Input.VirtualFile): Document = { + def index(input: Input.VirtualFile): TextDocument = { logger.trace(s"Indexing ${input.path} with length ${input.value.length}") val indexer: MtagsIndexer = if (isScala(input.path)) ScalaMtags.index(input) @@ -145,14 +146,15 @@ object Mtags extends LazyLogging { ) } val (names, symbols) = indexer.index() - Document( - filename = input.path, - contents = input.value, + TextDocument( + uri = input.path, + text = input.value, language = indexer.language, - names, - Nil, - symbols, - Nil + schema = Schema.SEMANTICDB3, + symbols = symbols, + occurrences = names, + diagnostics = Nil, + synthetics = Nil ) } diff --git a/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala b/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala index 0fd6a36740c..64abd3ce671 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala @@ -2,20 +2,18 @@ package scala.meta.metals.mtags import scala.meta.Name import scala.meta.Term -import scala.meta.PACKAGE import scala.meta.metals.ScalametaEnrichments._ -import org.langmeta.internal.semanticdb.schema.Denotation -import org.langmeta.internal.semanticdb.schema.ResolvedName -import org.langmeta.internal.semanticdb.schema.Position -import org.langmeta.internal.semanticdb.schema.ResolvedSymbol import org.{langmeta => m} import org.langmeta.semanticdb.Signature import org.langmeta.semanticdb.Symbol +import scala.meta.internal.semanticdb3.Language +import scala.meta.internal.{semanticdb3 => s} +import scala.meta.internal.semanticdb3.SymbolInformation.Kind trait MtagsIndexer { - def language: String + def language: Language def indexRoot(): Unit - def index(): (List[ResolvedName], List[ResolvedSymbol]) = { + def index(): (List[s.SymbolOccurrence], List[s.SymbolInformation]) = { indexRoot() names.result() -> symbols.result() } @@ -32,16 +30,21 @@ trait MtagsIndexer { currentOwner = old result } - def term(name: String, pos: m.Position, flags: Long): Unit = - addSignature(Signature.Term(name), pos, flags) - def term(name: Term.Name, flags: Long): Unit = - addSignature(Signature.Term(name.value), name.pos, flags) - def param(name: Name, flags: Long): Unit = - addSignature(Signature.TermParameter(name.value), name.pos, flags) - def tpe(name: String, pos: m.Position, flags: Long): Unit = - addSignature(Signature.Type(name), pos, flags) - def tpe(name: Name, flags: Long): Unit = - addSignature(Signature.Type(name.value), name.pos, flags) + def term(name: String, pos: m.Position, kind: Kind, properties: Int): Unit = + addSignature(Signature.Term(name), pos, kind, properties) + def term(name: Term.Name, kind: Kind, properties: Int): Unit = + addSignature(Signature.Term(name.value), name.pos, kind, properties) + def param(name: Name, kind: Kind, properties: Int): Unit = + addSignature( + Signature.TermParameter(name.value), + name.pos, + kind, + properties + ) + def tpe(name: String, pos: m.Position, kind: Kind, properties: Int): Unit = + addSignature(Signature.Type(name), pos, kind, properties) + def tpe(name: Name, kind: Kind, properties: Int): Unit = + addSignature(Signature.Type(name.value), name.pos, kind, properties) def pkg(ref: Term): Unit = ref match { case Name(name) => currentOwner = symbol(Signature.Term(name)) @@ -49,23 +52,30 @@ trait MtagsIndexer { pkg(qual) currentOwner = symbol(Signature.Term(name)) } - private val names = List.newBuilder[ResolvedName] - private val symbols = List.newBuilder[ResolvedSymbol] + private val names = List.newBuilder[s.SymbolOccurrence] + private val symbols = List.newBuilder[s.SymbolInformation] private def addSignature( signature: Signature, definition: m.Position, - flags: Long + kind: s.SymbolInformation.Kind, + properties: Int ): Unit = { currentOwner = symbol(signature) val syntax = currentOwner.syntax - names += ResolvedName( - Some(Position(definition.start, definition.end)), + val role = + if (kind.isPackage) s.SymbolOccurrence.Role.REFERENCE + else s.SymbolOccurrence.Role.REFERENCE + names += s.SymbolOccurrence( + range = Some(definition.toSchemaRange), syntax, - isDefinition = (flags & PACKAGE) == 0 + role ) - symbols += ResolvedSymbol( - syntax, - Some(Denotation(flags, signature.name, "", Nil)) + symbols += s.SymbolInformation( + symbol = syntax, + language = language, + kind = kind, + properties = properties, + name = signature.name ) } private def symbol(signature: Signature): Symbol.Global = diff --git a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala index c3fa837c7c7..307d710728a 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala @@ -2,52 +2,62 @@ package scala.meta.metals.mtags import scala.meta._ import org.langmeta.inputs.Input +import scala.meta.internal.semanticdb3.SymbolInformation.Kind +import scala.meta.internal.semanticdb3.Language +import scala.meta.internal.semanticdb3.SymbolInformation.Property object ScalaMtags { def index(input: Input.VirtualFile): MtagsIndexer = { val root: Source = input.parse[Source].get new Traverser with MtagsIndexer { - override def language: String = - "Scala212" // TODO(olafur) more accurate dialect + override def language: Language = Language.SCALA override def indexRoot(): Unit = apply(root) override def apply(tree: Tree): Unit = withOwner() { def continue(): Unit = super.apply(tree) def stop(): Unit = () - def pats(ps: List[Pat], flag: Long): Unit = { + def pats(ps: List[Pat], kind: Kind, properties: Int): Unit = { ps.foreach { - case Pat.Var(name) => withOwner() { term(name, flag) } + case Pat.Var(name) => withOwner() { term(name, kind, properties) } case _ => } } tree match { - case t: Source => continue() - case t: Template => continue() + case _: Source => continue() + case _: Template => continue() case t: Pkg => pkg(t.ref); continue() case t: Pkg.Object => - term(t.name, PACKAGEOBJECT); - term("package", t.name.pos, OBJECT); + term(t.name, Kind.PACKAGE_OBJECT, 0); + term("package", t.name.pos, Kind.OBJECT, 0); continue() case t: Defn.Class => - tpe(t.name, CLASS) + tpe(t.name, Kind.CLASS, 0) for { params <- t.ctor.paramss param <- params } withOwner() { - // TODO(olafur) More precise flags, we add VAL here blindly even if - // it's not a val, it might even be a var! - super.param(param.name, VAL | PARAM) + super.param(param.name, Kind.PARAMETER, 0) } continue() - case t: Defn.Trait => tpe(t.name, TRAIT); continue() - case t: Defn.Object => term(t.name, OBJECT); continue() - case t: Defn.Type => tpe(t.name, TYPE); stop() - case t: Decl.Type => tpe(t.name, TYPE); stop() - case t: Defn.Def => term(t.name, DEF); stop() - case t: Decl.Def => term(t.name, DEF); stop() - case t: Defn.Val => pats(t.pats, VAL); stop() - case t: Decl.Val => pats(t.pats, VAL); stop() - case t: Defn.Var => pats(t.pats, VAR); stop() - case t: Decl.Var => pats(t.pats, VAR); stop() + case t: Defn.Trait => + tpe(t.name, Kind.TRAIT, 0); continue() + case t: Defn.Object => + term(t.name, Kind.OBJECT, 0); continue() + case t: Defn.Type => + tpe(t.name, Kind.TYPE, 0); stop() + case t: Decl.Type => + tpe(t.name, Kind.TYPE, 0); stop() + case t: Defn.Def => + term(t.name, Kind.METHOD, 0); stop() + case t: Decl.Def => + term(t.name, Kind.METHOD, 0); stop() + case t: Defn.Val => + pats(t.pats, Kind.METHOD, Property.VAL.value); stop() + case t: Decl.Val => + pats(t.pats, Kind.METHOD, Property.VAL.value); stop() + case t: Defn.Var => + pats(t.pats, Kind.METHOD, Property.VAR.value); stop() + case t: Decl.Var => + pats(t.pats, Kind.METHOD, Property.VAR.value); stop() case _ => stop() } } diff --git a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala index a2ae9d7de5f..685f0c2474e 100644 --- a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala @@ -1,18 +1,11 @@ package scala.meta.metals.providers -import scala.meta.Type import scala.meta.metals.Uri import org.langmeta.lsp.Hover import org.langmeta.lsp.RawMarkedString import scala.meta.metals.search.SymbolIndex import scala.{meta => m} -import scalafix.internal.util.DenotationOps -import scalafix.internal.util.TypeSyntax -import scalafix.rule.RuleCtx -import scalafix.util.SemanticdbIndex -import scala.meta.metals.index.SymbolData import com.typesafe.scalalogging.LazyLogging -import org.langmeta.internal.semanticdb.schema object HoverProvider extends LazyLogging { def empty: Hover = Hover(Nil, None) @@ -27,177 +20,15 @@ object HoverProvider extends LazyLogging { ): Hover = { val result = for { (symbol, _) <- index.findSymbol(uri, line, column) - data <- index.data(symbol) - tpe <- getPrettyDefinition(symbol, data) - document <- index.documentIndex.getDocument(uri) } yield { - val scalafixIndex = SemanticdbIndex.load( - schema.Database(document :: Nil).toDb(None), - m.Sourcepath(Nil), - m.Classpath(Nil) - ) - val prettyTpe = new TypePrinter()(scalafixIndex).apply(tpe) + // TODO: pretty-print SymbolInformation.tpe, blocked by https://github.com/scalameta/scalameta/issues/1479 + val prettyTpe = symbol.syntax Hover( - contents = RawMarkedString(language = "scala", value = prettyTpe.syntax) :: Nil, + contents = RawMarkedString(language = "scala", value = prettyTpe) :: Nil, range = None ) } result.getOrElse(Hover(Nil, None)) } - /** Returns a definition tree for this symbol signature */ - private def getPrettyDefinition( - symbol: m.Symbol, - data: SymbolData - ): Option[m.Tree] = { - val denotation = m.Denotation(data.flags, data.name, data.signature, Nil) - val input = m.Input.Denotation(denotation.signature, symbol) - val mods = getMods(denotation) - val name = m.Term.Name(denotation.name) - val tname = m.Type.Name(denotation.name) - def parsedTpe: Option[Type] = - DenotationOps.defaultDialect(input).parse[m.Type].toOption - if (denotation.isVal) { - parsedTpe.map { tpe => - m.Decl.Val(mods, m.Pat.Var(name) :: Nil, tpe) - } - } else if (denotation.isVar) { - - parsedTpe.collect { - case Type.Method((m.Term.Param(_, _, Some(tpe), _) :: Nil) :: Nil, _) => - m.Decl.Var( - mods, - // TODO(olafur) fix https://github.com/scalameta/scalameta/issues/1100 - m.Pat.Var(m.Term.Name(name.value.stripSuffix("_="))) :: Nil, - tpe - ) - } - } else if (denotation.isDef) { - // turn method types into defs - // TODO(olafur) handle def macros - DenotationOps.defaultDialect(input).parse[m.Type].toOption.map { - case m.Type.Lambda(tparams, m.Type.Method(paramss, tpe)) => - m.Decl.Def(mods, name, tparams, paramss, tpe) - case m.Type.Lambda(tparams, tpe) => - m.Decl.Def(mods, name, tparams, Nil, tpe) - case m.Type.Method(paramss, tpe) => - m.Decl.Def(mods, name, Nil, paramss, tpe) - case t => t - } - } else if (denotation.isPackageObject) { - symbol match { - case m.Symbol.Global( - m.Symbol.Global(_, m.Signature.Term(pkg)), - m.Signature.Term("package") - ) => - Some(m.Pkg.Object(mods, m.Term.Name(pkg), Template)) - case _ => - logger.warn(s"Unexpected package object symbol: $symbol") - None - } - } else if (denotation.isType && denotation.isAbstract) { - Some( - m.Decl.Type( - mods.filterNot(_.is[m.Mod.Abstract]), - tname, - Nil, - m.Type.Bounds(None, None) - ) - ) - } else if (denotation.isType) { - parsedTpe.map { - case m.Type.Lambda(tparams, tpe) => - m.Defn.Type(mods, tname, tparams, tpe) - case tpe => - m.Defn.Type(mods, tname, Nil, tpe) - } - } else if (denotation.isObject) { - Some(m.Defn.Object(mods.filterNot(_.is[m.Mod.Final]), name, Template)) - } else if (denotation.isClass) { - Some( - m.Defn.Class( - mods, - tname, - Nil, - m.Ctor.Primary(Nil, m.Name.Anonymous(), Nil), - Template - ) - ) - } else if (denotation.isTrait) { - Some( - m.Defn.Trait( - mods, - tname, - Nil, - m.Ctor.Primary(Nil, m.Name.Anonymous(), Nil), - Template - ) - ) - } else if (denotation.isPackage) { - Some(m.Pkg(name, Nil)) - } else if (!denotation.signature.isEmpty) { - parsedTpe - } else { - Some(m.Type.Name(data.name)) - } - } - - private def getMods(denotation: m.Denotation): List[m.Mod] = { - import denotation._ - import scala.meta._ - val buf = List.newBuilder[m.Mod] - if (isPrivate) buf += mod"private" - if (isProtected) buf += mod"protected" - if (isFinal) buf += mod"final" - if (isAbstract) buf += mod"abstract" - if (isImplicit) buf += mod"implicit" - if (isLazy) buf += mod"lazy" - if (isSealed) buf += mod"sealed" - buf.result() - } - - /** Pretty-prints types in a given tree - * - * Uses the scalafix TypeSyntax, the same one used by ExplicitResultTypes. - * It's quite primitive for now, but the plan is to implement fancy - * stuff in the future like scope aware printing taking into - * account renames etc. - */ - private class TypePrinter(implicit scalafixIndex: SemanticdbIndex) - extends m.Transformer { - private def pretty(tpe: m.Type): m.Tree = { - val printed = - TypeSyntax.prettify(tpe, RuleCtx(m.Lit.Null()), shortenNames = true)._1 - InfixSymbolicTypes.apply(printed) - } - - override def apply(tree: m.Tree): m.Tree = tree match { - case tpe: m.Type => { - pretty(tpe) - } - case _ => super.apply(tree) - } - } - - /** Makes all symbolic type binary operators infix */ - private object InfixSymbolicTypes extends m.Transformer { - private object SymbolicType { - def unapply(arg: m.Type): Option[m.Type.Name] = arg match { - case nme @ m.Type.Name(name) => - if (!name.isEmpty && - !Character.isJavaIdentifierStart(name.charAt(0))) { - Some(nme) - } else None - case _ => None - } - } - override def apply(tree: m.Tree): m.Tree = { - val next = tree match { - case m.Type.Apply(SymbolicType(name), lhs :: rhs :: Nil) => - m.Type.ApplyInfix(lhs, name, rhs) - case t => t - } - super.apply(next) - } - } } diff --git a/metals/src/main/scala/scala/meta/metals/search/DocumentIndex.scala b/metals/src/main/scala/scala/meta/metals/search/DocumentIndex.scala index 783d58130e8..8562ce89b36 100644 --- a/metals/src/main/scala/scala/meta/metals/search/DocumentIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/DocumentIndex.scala @@ -1,9 +1,9 @@ package scala.meta.metals.search import scala.meta.metals.Uri -import org.langmeta.internal.semanticdb.schema.Document +import scala.meta.internal.semanticdb3.TextDocument trait DocumentIndex { - def getDocument(uri: Uri): Option[Document] // should this be future? - def putDocument(uri: Uri, document: Document): Unit + def getDocument(uri: Uri): Option[TextDocument] // should this be future? + def putDocument(uri: Uri, document: TextDocument): Unit } diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala index e1d7ef567bf..89a60c6e9a6 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala @@ -4,15 +4,15 @@ import java.util import java.util.concurrent.ConcurrentHashMap import scala.meta.metals.Uri import com.typesafe.scalalogging.LazyLogging -import org.langmeta.internal.semanticdb.schema.Document +import scala.meta.internal.semanticdb3.TextDocument class InMemoryDocumentIndex( - documents: util.Map[Uri, Document] = new ConcurrentHashMap() + documents: util.Map[Uri, TextDocument] = new ConcurrentHashMap() ) extends DocumentIndex with LazyLogging { - override def getDocument(uri: Uri): Option[Document] = + override def getDocument(uri: Uri): Option[TextDocument] = Option(documents.get(uri)) - override def putDocument(uri: Uri, document: Document): Unit = { + override def putDocument(uri: Uri, document: TextDocument): Unit = { if (!uri.isJar) { logger.info(s"Storing in-memory document for uri $uri") } diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala index 29554633fb1..80dc24d523e 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala @@ -19,16 +19,18 @@ import com.typesafe.scalalogging.LazyLogging import me.xdrop.fuzzywuzzy.FuzzySearch import org.langmeta.inputs.Input import org.langmeta.inputs.Position -import org.langmeta.internal.semanticdb.schema.Database -import org.langmeta.internal.semanticdb.schema.ResolvedName -import org.langmeta.internal.semanticdb.{schema => s} +import scala.meta.internal.semanticdb3 +import scala.meta.internal.semanticdb3.SymbolOccurrence +import scala.meta.internal.semanticdb3.TextDocument +import scala.meta.internal.semanticdb3.TextDocuments import org.langmeta.io.AbsolutePath import org.langmeta.languageserver.InputEnrichments._ -import org.langmeta.semanticdb.SemanticdbEnrichments._ import org.langmeta.semanticdb.Symbol import monix.eval.Task import monix.execution.Scheduler import monix.reactive.Observable +import scala.meta.internal.semanticdb3.SymbolInformation.Kind +import scala.meta.internal.semanticdb3.SymbolInformation.Property import scala.util.control.NonFatal class InMemorySymbolIndex( @@ -44,26 +46,26 @@ class InMemorySymbolIndex( private val indexedJars: ConcurrentHashMap[AbsolutePath, Unit] = new ConcurrentHashMap[AbsolutePath, Unit]() - /** Returns a ResolvedName at the given location */ + /** Returns a SymbolOccurrence at the given location */ def resolveName( uri: Uri, line: Int, column: Int - ): Option[(ResolvedName, TokenEditDistance)] = { + ): Option[(SymbolOccurrence, TokenEditDistance)] = { logger.info(s"resolveName at $uri:$line:$column") for { document <- documentIndex.getDocument(uri) _ = logger.info(s"Found document for $uri") - original = Input.VirtualFile(document.filename, document.contents) + original = Input.VirtualFile(document.uri, document.text) revised = uri.toInput(buffers) (originalPosition, edit) <- { findOriginalPosition(original, revised, line, column) } - name <- document.names.collectFirst { - case name @ ResolvedName(Some(position), symbol, _) if { - val range = original.toIndexRange(position.start, position.end) + name <- document.occurrences.collectFirst { + case name @ SymbolOccurrence(Some(r), symbol, _) if { + val range = original.toIndexRange(r) logger.trace( - s"${document.filename.replaceFirst(".*/", "")} [${range.pretty}] ${symbol}" + s"${document.uri.replaceFirst(".*/", "")} [${range.pretty}] $symbol" ) range.contains(originalPosition) } => @@ -81,7 +83,7 @@ class InMemorySymbolIndex( for { (name, edit) <- resolveName(uri, line, column) symbol = Symbol(name.symbol) - _ = logger.info(s"Matching symbol ${symbol}") + _ = logger.info(s"Matching symbol $symbol") } yield symbol -> edit } @@ -137,7 +139,7 @@ class InMemorySymbolIndex( sourceJarsToIndex.foreach { path => logger.info(s"Indexing classpath entry $path") try { - val database = db.getOrElseUpdate[AbsolutePath, Database]( + val database = db.getOrElseUpdate[AbsolutePath, TextDocuments]( path, () => Mtags.indexDatabase(path :: Nil) ) @@ -153,12 +155,12 @@ class InMemorySymbolIndex( } /** Register this Database to symbol indexer. */ - def indexDatabase(document: s.Database): Effects.IndexSemanticdb = { + def indexDatabase(document: TextDocuments): Effects.IndexSemanticdb = { document.documents.foreach { doc => try indexDocument(doc) catch { case NonFatal(e) => - logger.error(s"Failed to index ${doc.filename}", e) + logger.error(s"Failed to index ${doc.uri}", e) } } Effects.IndexSemanticdb @@ -174,11 +176,11 @@ class InMemorySymbolIndex( * - filename must be formatted as a URI * - names must be sorted */ - def indexDocument(document: s.Document): Effects.IndexSemanticdb = { - val uri = Uri(document.filename) - val input = Input.VirtualFile(document.filename, document.contents) + def indexDocument(document: TextDocument): Effects.IndexSemanticdb = { + val uri = Uri(document.uri) + val input = Input.VirtualFile(document.uri, document.text) documentIndex.putDocument(uri, document) - document.names.foreach { + document.occurrences.foreach { // TODO(olafur) handle local symbols on the fly from a `Document` in go-to-definition // local symbols don't need to be indexed globally, by skipping them we should // def isLocalSymbol(sym: String): Boolean = @@ -186,27 +188,33 @@ class InMemorySymbolIndex( // !sym.endsWith("#") && // !sym.endsWith(")") // be able to minimize the size of the global index significantly. - // case s.ResolvedName(_, sym, _) if isLocalSymbol(sym) => // Do nothing, local symbol. - case s.ResolvedName(Some(s.Position(start, end)), sym, true) => + // case s.SymbolOccurrence(_, sym, _) if isLocalSymbol(sym) => // Do nothing, local symbol. + case SymbolOccurrence(Some(r), sym, SymbolOccurrence.Role.DEFINITION) => symbolIndexer.addDefinition( sym, - i.Position(document.filename, Some(input.toIndexRange(start, end))) + i.Position(document.uri, Some(input.toIndexRange(r))) ) - case s.ResolvedName(Some(s.Position(start, end)), sym, false) => + case SymbolOccurrence(Some(r), sym, SymbolOccurrence.Role.REFERENCE) => symbolIndexer.addReference( - document.filename, - input.toIndexRange(start, end), + document.uri, + input.toIndexRange(r), sym ) case _ => } document.symbols.foreach { - case s.ResolvedSymbol(sym, Some(denot)) => + case denot: semanticdb3.SymbolInformation => + val isField = denot.properties.hasOneOfFlags( + Property.VAL.value | Property.VAR.value + ) + val kind = + if (isField) Kind.FIELD + else denot.kind symbolIndexer.addDenotation( - sym, - denot.flags, - denot.name, - denot.signature + symbol = denot.symbol, + kind = kind.value, + name = denot.name, + signature = denot.symbol // TODO: blocked by https://github.com/scalameta/scalameta/issues/1479 ) case _ => } diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala index 263c9546fe4..25bdc84689a 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala @@ -51,11 +51,11 @@ class InMemorySymbolIndexer( override def addDenotation( symbol: String, - flags: Long, + kind: Int, name: String, signature: String ): Unit = updated(symbol) { index => - index.copy(flags = flags, signature = signature, name = name) + index.copy(kind = kind, signature = signature, name = name) } override def addReference( diff --git a/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala index 3b3ebdcbabc..baca75b16e1 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala @@ -12,7 +12,7 @@ object InverseSymbolIndexer { /** Rebuilds a scala.meta.Database with only names filled out * * @param cwd the working directory to relativize file URIs in the symbol index. - * @param documents store for looking up document contents. + * @param documents store for looking up document text. * @param symbols symbol index, from [[SymbolIndexer.allSymbols]] */ def reconstructDatabase( @@ -34,7 +34,7 @@ object InverseSymbolIndexer { m.Document( m.Input.VirtualFile( key, - documents.getDocument(uri).fold("")(_.contents) + documents.getDocument(uri).fold("")(_.text) ), "Scala212", Nil, diff --git a/metals/src/main/scala/scala/meta/metals/search/SymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/SymbolIndex.scala index 639640033e6..41269e161c0 100644 --- a/metals/src/main/scala/scala/meta/metals/search/SymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/SymbolIndex.scala @@ -8,7 +8,7 @@ import scala.meta.metals.index.SymbolData import org.langmeta.lsp.SymbolInformation import org.langmeta.jsonrpc.JsonRpcClient import com.typesafe.scalalogging.LazyLogging -import org.langmeta.internal.semanticdb.{schema => s} +import scala.meta.internal.semanticdb3 import org.langmeta.io.AbsolutePath import org.langmeta.semanticdb.Symbol import monix.eval.Task @@ -53,7 +53,7 @@ trait SymbolIndex extends LazyLogging { ): Task[Effects.IndexSourcesClasspath] /** Register this Database to symbol indexer. */ - def indexDatabase(document: s.Database): Effects.IndexSemanticdb + def indexDatabase(document: semanticdb3.TextDocuments): Effects.IndexSemanticdb /** Remove any persisted files from index returning to a clean start */ def clearIndex(): Unit diff --git a/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala index 44e3c2d3320..fb2f314ffc6 100644 --- a/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala @@ -52,13 +52,14 @@ trait SymbolIndexer { self => /** * Register metadata about a symbol. * - * @param flags the modifiers of this symbol, see org.langmeta.semanticdb.HasFlags + * @param kind the kind of this symbol, see SymbolInformation.Kind in + * https://github.com/scalameta/scalameta/blob/master/semanticdb/semanticdb3/semanticdb3.md#symbolinformation * @param name the name of the symbol, example "get" for scala.Option.get * @param signature the type signature of this symbol, example "List[T]" for List.tail */ def addDenotation( symbol: String, - flags: Long, + kind: Int, name: String, signature: String ): Unit diff --git a/metals/src/main/scala/scala/meta/metals/storage/Bytes.scala b/metals/src/main/scala/scala/meta/metals/storage/Bytes.scala index 98fa5a917db..4a89f4a4a3d 100644 --- a/metals/src/main/scala/scala/meta/metals/storage/Bytes.scala +++ b/metals/src/main/scala/scala/meta/metals/storage/Bytes.scala @@ -1,8 +1,8 @@ package scala.meta.metals.storage import java.nio.charset.StandardCharsets -import org.langmeta.internal.semanticdb.schema.Database import org.langmeta.io.AbsolutePath +import scala.meta.internal.semanticdb3.TextDocuments trait FromBytes[A] { self => def fromBytes(bytes: Array[Byte]): A @@ -14,8 +14,8 @@ object FromBytes { new String(_, StandardCharsets.UTF_8) implicit val ByteArrayFromBytes: FromBytes[Array[Byte]] = identity[Array[Byte]] - implicit val DatabaseFromBytes: FromBytes[Database] = - bytes => Database.parseFrom(bytes) + implicit val DatabaseFromBytes: FromBytes[TextDocuments] = + bytes => TextDocuments.parseFrom(bytes) } trait ToBytes[A] { self => @@ -28,7 +28,7 @@ object ToBytes { _.getBytes(StandardCharsets.UTF_8) implicit val ByteArrayToBytes: ToBytes[Array[Byte]] = identity[Array[Byte]] - implicit val DatabaseToBytes: ToBytes[Database] = + implicit val DatabaseToBytes: ToBytes[TextDocuments] = _.toByteArray implicit val AbsolutePathToBytes: ToBytes[AbsolutePath] = _.toString().getBytes(StandardCharsets.UTF_8) diff --git a/metals/src/main/scala/scalafix/languageserver/ScalafixEnrichments.scala b/metals/src/main/scala/scalafix/languageserver/ScalafixEnrichments.scala index 831dadd81ef..8c8c121e8d4 100644 --- a/metals/src/main/scala/scalafix/languageserver/ScalafixEnrichments.scala +++ b/metals/src/main/scala/scalafix/languageserver/ScalafixEnrichments.scala @@ -40,9 +40,10 @@ object ScalafixEnrichments { implicit class XtensionPatchLSPObject(val `_`: Patch.type) extends AnyVal { def lintMessagesInternal( patches: Map[RuleName, Patch], - ctx: RuleCtx + ctx: RuleCtx, + index: SemanticdbIndex ): List[LintMessage] = - Patch.lintMessages(patches, ctx) + Patch(patches, ctx, Some(index))._2 } implicit class XtensionRuleLSP(val rule: Rule) extends AnyVal { def fixWithNameInternal(ctx: RuleCtx): Map[RuleName, Patch] = diff --git a/project/plugins.sbt b/project/plugins.sbt index cdc8f0a5678..9fbcebd4049 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -3,10 +3,8 @@ addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.5.7") addSbtPlugin("ch.epfl.scala" % "sbt-release-early" % "2.0.0") addSbtPlugin("io.get-coursier" % "sbt-coursier" % "1.0.0-RC13") addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.7.0") -addSbtPlugin( - "com.thesamet" % "sbt-protoc" % "0.99.18" exclude ("com.trueaccord.scalapb", "protoc-bridge_2.12") -) -libraryDependencies += "com.trueaccord.scalapb" %% "compilerplugin-shaded" % "0.6.6" +addSbtPlugin("com.thesamet" % "sbt-protoc" % "0.99.18") +libraryDependencies += "com.thesamet.scalapb" %% "compilerplugin" % "0.7.1" unmanagedSourceDirectories in Compile += baseDirectory.value.getParentFile / "sbt-metals" / "src" / "main" / "scala" From 0ea9e568aaf435ef5d10480428fbd90cc077a82d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 10:12:54 +0200 Subject: [PATCH 03/25] Fix compile errors in tests --- metals/src/main/scala/scala/meta/metals/Uri.scala | 3 ++- metals/src/test/scala/tests/mtags/BaseMtagsTest.scala | 5 +++-- metals/src/test/scala/tests/mtags/ClasspathMtagsTest.scala | 7 ++++--- metals/src/test/scala/tests/mtags/JavaMtagsTest.scala | 1 + .../scala/tests/provider/DiagnosticsProviderTest.scala | 1 + 5 files changed, 11 insertions(+), 6 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/Uri.scala b/metals/src/main/scala/scala/meta/metals/Uri.scala index f7806e2771c..07837f91d4d 100644 --- a/metals/src/main/scala/scala/meta/metals/Uri.scala +++ b/metals/src/main/scala/scala/meta/metals/Uri.scala @@ -16,7 +16,8 @@ import org.langmeta.io.AbsolutePath * - URI has a lot of methods that return null and we don't use anyways * - URI supports any scheme while we are only interested in a couple schemes * - Both file:///path and file:/path are valid URIs while we only use - * file:///path in this project in order to support storing them as strings. For context, see https://github.com/scalameta/metals/pull/127#issuecomment-351880150 + * file:///path in this project in order to support storing them as strings. + * For context, see https://github.com/scalameta/metals/pull/127#issuecomment-351880150 */ sealed abstract case class Uri(value: String) { // Runtime check because wrapping constructor in Option[Uri] is too cumbersome diff --git a/metals/src/test/scala/tests/mtags/BaseMtagsTest.scala b/metals/src/test/scala/tests/mtags/BaseMtagsTest.scala index 82b060a36e7..9c4a0674aae 100644 --- a/metals/src/test/scala/tests/mtags/BaseMtagsTest.scala +++ b/metals/src/test/scala/tests/mtags/BaseMtagsTest.scala @@ -1,8 +1,9 @@ package tests.mtags import scala.meta.metals.mtags.Mtags -import org.langmeta.internal.semanticdb.schema.Database import tests.MegaSuite +import scala.meta.internal.semanticdb3.TextDocuments +import org.langmeta.internal.semanticdb._ class BaseMtagsTest extends MegaSuite { def checkIgnore( @@ -14,7 +15,7 @@ class BaseMtagsTest extends MegaSuite { } def check(filename: String, original: String, expected: String): Unit = { test(filename) { - val sdb = Database(Mtags.index(filename, original) :: Nil) + val sdb = TextDocuments(Mtags.index(filename, original) :: Nil) val obtained = sdb.toDb(None).documents.head.syntax // println(obtained) assertNoDiff(obtained, expected) diff --git a/metals/src/test/scala/tests/mtags/ClasspathMtagsTest.scala b/metals/src/test/scala/tests/mtags/ClasspathMtagsTest.scala index 7e263cef511..3bf517525e0 100644 --- a/metals/src/test/scala/tests/mtags/ClasspathMtagsTest.scala +++ b/metals/src/test/scala/tests/mtags/ClasspathMtagsTest.scala @@ -3,8 +3,9 @@ package tests.mtags import java.nio.file.Paths import scala.meta.metals.Jars import scala.meta.metals.mtags.Mtags -import org.langmeta.internal.semanticdb.schema.Database import tests.MegaSuite +import org.langmeta.internal.semanticdb._ +import scala.meta.internal.semanticdb3.TextDocuments object ClasspathMtagsTest extends MegaSuite { @@ -31,9 +32,9 @@ object ClasspathMtagsTest extends MegaSuite { path.toNIO.endsWith(Predef) } ) { doc => - val path = Paths.get(doc.filename).getFileName.toString + val path = Paths.get(doc.uri).getFileName.toString val underline = "-" * path.length - val mdoc = Database(doc :: Nil).toDb(None).documents.head.toString() + val mdoc = TextDocuments(doc :: Nil).toDb(None).documents.head.toString() docs += s"""$path |$underline diff --git a/metals/src/test/scala/tests/mtags/JavaMtagsTest.scala b/metals/src/test/scala/tests/mtags/JavaMtagsTest.scala index 7c2469fa3f2..54f0e4e0aa6 100644 --- a/metals/src/test/scala/tests/mtags/JavaMtagsTest.scala +++ b/metals/src/test/scala/tests/mtags/JavaMtagsTest.scala @@ -3,6 +3,7 @@ package tests.mtags import java.nio.file.Paths import scala.meta.metals.compiler.CompilerConfig import scala.meta.metals.mtags.Mtags +import org.langmeta.internal.semanticdb._ object JavaMtagsTest extends BaseMtagsTest { check( diff --git a/metals/src/test/scala/tests/provider/DiagnosticsProviderTest.scala b/metals/src/test/scala/tests/provider/DiagnosticsProviderTest.scala index 711376023a7..bbe08a86e51 100644 --- a/metals/src/test/scala/tests/provider/DiagnosticsProviderTest.scala +++ b/metals/src/test/scala/tests/provider/DiagnosticsProviderTest.scala @@ -23,6 +23,7 @@ object DiagnosticsProviderTest extends CompilerSuite with LazyLogging { val logFile = tmp.resolve("metals.log").toFile val out = new PrintStream(new FileOutputStream(logFile)) val scalafixConfPath = ".customScalafixConfPath" + val config = Observable.now( Configuration( scalac = Configuration.Scalac( From 3431fb7792706bcaf7bcf4941b0581a5272fac29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 13:49:31 +0200 Subject: [PATCH 04/25] Fix workspace/symbol by removing protobuf generation. SymbolData was intended to be persisted to disk, but we never made use of it. The reason I removed the protobuf generation was in order to add `SymbolData.info: SymbolInformation` to avoid untyped programming again `flags: Int`. For example, workspace/symbol did not work because there was a broken bitmask somewhere. `SymbolInformation.{kind,properties}` are well documented and should be used instead of the old unspecified flags. --- .../main/scala/org/langmeta/lsp/Types.scala | 19 ++++- metals/src/main/protobuf/metals.proto | 74 +++++++++---------- .../languageserver/InputEnrichments.scala | 10 --- .../meta/metals/ScalametaEnrichments.scala | 50 +++++++++++-- .../scala/scala/meta/metals/Semanticdbs.scala | 13 ++-- .../scala/meta/metals/index/SymbolData.scala | 25 +++++++ .../scala/meta/metals/index/package.scala | 16 ++++ .../providers/DocumentHighlightProvider.scala | 6 +- .../metals/search/InMemorySymbolIndex.scala | 37 ++++------ .../metals/search/InMemorySymbolIndexer.scala | 31 ++++---- .../metals/search/InverseSymbolIndexer.scala | 25 +++++-- .../meta/metals/search/SymbolIndexer.scala | 19 ++--- .../metals/search/TokenEditDistance.scala | 25 ++++--- .../provider/DiagnosticsProviderTest.scala | 2 +- 14 files changed, 224 insertions(+), 128 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/metals/index/SymbolData.scala create mode 100644 metals/src/main/scala/scala/meta/metals/index/package.scala diff --git a/lsp4s/src/main/scala/org/langmeta/lsp/Types.scala b/lsp4s/src/main/scala/org/langmeta/lsp/Types.scala index 05245b39620..002d12c3951 100644 --- a/lsp4s/src/main/scala/org/langmeta/lsp/Types.scala +++ b/lsp4s/src/main/scala/org/langmeta/lsp/Types.scala @@ -14,7 +14,24 @@ import io.circe.generic.JsonCodec /** * A range in a text document. */ -@JsonCodec case class Range(start: Position, end: Position) +@JsonCodec case class Range(start: Position, end: Position) { + def startLine = start.line + def startColumn = start.character + def endLine = end.line + def endColumn = end.character +} + +object Range { + def apply( + startLine: Int, + startColumn: Int, + endLine: Int, + endColumn: Int + ): Range = Range( + start = Position(startLine, startColumn), + end = Position(endLine, endColumn), + ) +} /** * Represents a location inside a resource, such as a line diff --git a/metals/src/main/protobuf/metals.proto b/metals/src/main/protobuf/metals.proto index a83ec9eda6f..9230de20c50 100644 --- a/metals/src/main/protobuf/metals.proto +++ b/metals/src/main/protobuf/metals.proto @@ -2,40 +2,40 @@ syntax = "proto3"; package scala.meta.metals.index; -// All of the metadata associated with a single symbol definition -message SymbolData { - string symbol = 1; - // The Position where this symbol is defined. - Position definition = 2; - map references = 3; - int32 kind = 4; - string name = 5; - string signature = 6; - // Planned for Scalameta v2.2, see https://github.com/scalameta/scalameta/milestone/9 - // string docstring = 7; // javadoc/scaladoc - // string overrides = 8; // parent symbol that this symbol overrides -} - -// A standalone position used by "Go to definition" when jumping to a symbol -// definition. -message Position { - // can be a local file, entry inside local zip file, http url, or anything - // else. - string uri = 1; - Range range = 2; -} - -// A hack to workaround the fact that protobuf can't -// encode `map` -message Ranges { - repeated Range ranges = 1; -} - -// A slim range position, the filename is embedded in the -// key of `references: map` -message Range { - int32 startLine = 1; - int32 startColumn = 2; - int32 endLine = 3; - int32 endColumn = 4; -} +//// All of the metadata associated with a single symbol definition +//message SymbolData { +// string symbol = 1; +// // The Position where this symbol is defined. +// Position definition = 2; +// map references = 3; +// int32 kind = 4; +// string name = 5; +// string signature = 6; +// // Planned for Scalameta v2.2, see https://github.com/scalameta/scalameta/milestone/9 +// // string docstring = 7; // javadoc/scaladoc +// // string overrides = 8; // parent symbol that this symbol overrides +//} +// +//// A standalone position used by "Go to definition" when jumping to a symbol +//// definition. +//message Position { +// // can be a local file, entry inside local zip file, http url, or anything +// // else. +// string uri = 1; +// Range range = 2; +//} +// +//// A hack to workaround the fact that protobuf can't +//// encode `map` +//message Ranges { +// repeated Range ranges = 1; +//} +// +//// A slim range position, the filename is embedded in the +//// key of `references: map` +//message Range { +// int32 startLine = 1; +// int32 startColumn = 2; +// int32 endLine = 3; +// int32 endColumn = 4; +//} diff --git a/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala b/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala index 02ed3e97276..116f790e857 100644 --- a/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala +++ b/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala @@ -48,16 +48,6 @@ object InputEnrichments { ) } - /** Returns a scala.meta.Position from an index range. */ - def toPosition(range: i.Range): Position = { - toPosition( - range.startLine, - range.startColumn, - range.endLine, - range.endColumn - ) - } - def toOffset(pos: lsp.Position): Int = toOffset(pos.line, pos.character) diff --git a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala index 1a2d161f875..221c80ae1f1 100644 --- a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala +++ b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala @@ -16,6 +16,7 @@ import org.langmeta.internal.io.FileIO import org.langmeta.io.AbsolutePath import org.langmeta.internal.semanticdb._ import scala.meta.internal.semanticdb3.SymbolInformation.Kind +import scala.meta.internal.semanticdb3.SymbolInformation.Property // Extension methods for convenient reuse of data conversions between // scala.meta._ and language.types._ @@ -117,12 +118,12 @@ object ScalametaEnrichments { } implicit class XtensionIndexPosition(val pos: i.Position) extends AnyVal { def pretty: String = - s"${pos.uri.replaceFirst(".*/", "")} [${pos.range.map(_.pretty).getOrElse("")}]" + s"${pos.uri.replaceFirst(".*/", "")} [${pos.range.pretty}]" def toLocation: Location = { l.Location( pos.uri, - pos.range.get.toRange + pos.range.toRange ) } } @@ -314,9 +315,9 @@ object ScalametaEnrichments { val defPosition = if (withDefinition) data.definition else None val refPositions = for { - (uri, rangeSet) <- data.references - range <- rangeSet.ranges - } yield i.Position(uri, Some(range)) + (uri, ranges) <- data.references + range <- ranges + } yield i.Position(uri.value, range) (defPosition.toSet ++ refPositions.toSet) .filterNot { _.uri.startsWith("jar:file") } // definition may refer to a jar @@ -331,6 +332,45 @@ object ScalametaEnrichments { semanticdb3.TextDocuments(document :: Nil).toDb(None).documents.head } + implicit class XtensionSymbolInformation( + val info: semanticdb3.SymbolInformation + ) { + import Property._ + def isOneOf(kind: Kind*): Boolean = { + kind.contains(info.kind) + } + def has(prop1: Property, prop2: Property, props: Property*): Boolean = + has(prop1) || has(prop2) || props.exists(has) + def has(prop: Property): Boolean = + info.properties.hasOneOfFlags(prop.value) + def toSymbolKind: SymbolKind = info.kind match { + case Kind.OBJECT | Kind.PACKAGE_OBJECT => + SymbolKind.Module + case Kind.CLASS => + if (has(ENUM)) SymbolKind.Enum + else SymbolKind.Class + case Kind.PACKAGE => + SymbolKind.Package + case Kind.TRAIT | Kind.INTERFACE => + SymbolKind.Interface + case Kind.METHOD | Kind.LOCAL => + if (has(VAL)) SymbolKind.Constant + else if (has(VAR)) SymbolKind.Variable + else SymbolKind.Method + case Kind.CONSTRUCTOR => + SymbolKind.Constructor + case Kind.FIELD => + SymbolKind.Field + case Kind.TYPE => + SymbolKind.Class // ??? + case Kind.PARAMETER | Kind.SELF_PARAMETER | Kind.TYPE_PARAMETER => + SymbolKind.Variable // ??? + case kind @ Kind.UNKNOWN_KIND => + throw new IllegalArgumentException( + s"Unsupported kind $kind in SymbolInformation: ${info.toProtoString}" + ) + } + } implicit class XtensionIntAsSymbolKind(val flags: Int) { def hasOneOfFlags(flags: Long): Boolean = (this.flags & flags) != 0L diff --git a/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala b/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala index d0b753a43d1..95aee5c4276 100644 --- a/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala +++ b/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala @@ -82,18 +82,21 @@ object Semanticdbs extends LazyLogging { implicit private val occurrenceOrdering: Ordering[SymbolOccurrence] = new Ordering[semanticdb3.SymbolOccurrence] { override def compare(x: SymbolOccurrence, y: SymbolOccurrence): Int = { - if (x.range.isEmpty || x.range.isEmpty) 0 + if (x.range.isEmpty) 0 + else if (y.range.isEmpty) 0 else { + val a = x.range.get + val b = y.range.get val byLine = Integer.compare( - x.range.get.startLine, - y.range.get.startLine + a.startLine, + b.startLine ) if (byLine != 0) { byLine } else { val byCharacter = Integer.compare( - x.range.get.startCharacter, - y.range.get.startCharacter + a.startCharacter, + b.startCharacter ) byCharacter } diff --git a/metals/src/main/scala/scala/meta/metals/index/SymbolData.scala b/metals/src/main/scala/scala/meta/metals/index/SymbolData.scala new file mode 100644 index 00000000000..b98555d7c28 --- /dev/null +++ b/metals/src/main/scala/scala/meta/metals/index/SymbolData.scala @@ -0,0 +1,25 @@ +package scala.meta.metals.index + +import org.langmeta.{lsp => l} +import scala.meta.metals.Uri +import scala.meta.internal.{semanticdb3 => s} + +case class SymbolData( + symbol: String, + definition: Option[l.Location], + references: Map[Uri, Seq[l.Range]], + info: Option[s.SymbolInformation] +) + +//message SymbolData { +// string symbol = 1; +// // The Position where this symbol is defined. +// Position definition = 2; +// map references = 3; +// int32 kind = 4; +// string name = 5; +// string signature = 6; +// // Planned for Scalameta v2.2, see https://github.com/scalameta/scalameta/milestone/9 +// // string docstring = 7; // javadoc/scaladoc +// // string overrides = 8; // parent symbol that this symbol overrides +//} diff --git a/metals/src/main/scala/scala/meta/metals/index/package.scala b/metals/src/main/scala/scala/meta/metals/index/package.scala new file mode 100644 index 00000000000..b9c069ad707 --- /dev/null +++ b/metals/src/main/scala/scala/meta/metals/index/package.scala @@ -0,0 +1,16 @@ +package scala.meta.metals + +package object index { + + // TODO: remove these forwarders + @deprecated("Use org.langmeta.lsp.Range instead", "v0.1") + type Range = org.langmeta.lsp.Range + @deprecated("Use org.langmeta.lsp.Range instead", "v0.1") + val Range = org.langmeta.lsp.Range + + @deprecated("Use org.langmeta.lsp.Location instead", "v0.1") + type Position = org.langmeta.lsp.Location + @deprecated("Use org.langmeta.lsp.Location instead", "v0.1") + val Position = org.langmeta.lsp.Location + +} diff --git a/metals/src/main/scala/scala/meta/metals/providers/DocumentHighlightProvider.scala b/metals/src/main/scala/scala/meta/metals/providers/DocumentHighlightProvider.scala index 709a5a9d046..02b5fa3d87d 100644 --- a/metals/src/main/scala/scala/meta/metals/providers/DocumentHighlightProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/providers/DocumentHighlightProvider.scala @@ -18,12 +18,12 @@ object DocumentHighlightProvider extends LazyLogging { logger.info(s"Document highlight in $uri") for { data <- symbolIndex.findReferences(uri, position.line, position.character) - _ = logger.info(s"Highlighting symbol `${data.name}: ${data.signature}`") + _ = logger.info(s"Highlighting symbol ${data.symbol}") pos <- data.referencePositions(withDefinition = true) if pos.uri == uri.value - _ = logger.debug(s"Found highlight at [${pos.range.get.pretty}]") + _ = logger.debug(s"Found highlight at [${pos.range.pretty}]") // TODO(alexey) add DocumentHighlightKind: Text (default), Read, Write - } yield DocumentHighlight(pos.range.get.toRange) + } yield DocumentHighlight(pos.range.toRange) } } diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala index 80dc24d523e..1d3aadeeed9 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala @@ -192,56 +192,45 @@ class InMemorySymbolIndex( case SymbolOccurrence(Some(r), sym, SymbolOccurrence.Role.DEFINITION) => symbolIndexer.addDefinition( sym, - i.Position(document.uri, Some(input.toIndexRange(r))) + i.Position(document.uri, input.toIndexRange(r)) ) case SymbolOccurrence(Some(r), sym, SymbolOccurrence.Role.REFERENCE) => symbolIndexer.addReference( - document.uri, + Uri(document.uri), input.toIndexRange(r), sym ) case _ => } - document.symbols.foreach { - case denot: semanticdb3.SymbolInformation => - val isField = denot.properties.hasOneOfFlags( - Property.VAL.value | Property.VAR.value - ) - val kind = - if (isField) Kind.FIELD - else denot.kind - symbolIndexer.addDenotation( - symbol = denot.symbol, - kind = kind.value, - name = denot.name, - signature = denot.symbol // TODO: blocked by https://github.com/scalameta/scalameta/issues/1479 - ) - case _ => - } + document.symbols.foreach(symbolIndexer.addSymbolInformation) Effects.IndexSemanticdb } override def workspaceSymbols(query: String): List[SymbolInformation] = { import scala.meta.metals.ScalametaEnrichments._ - import scala.meta.semanticdb._ val result = symbolIndexer.allSymbols.toIterator .withFilter { symbol => symbol.definition.isDefined && symbol.definition.get.uri .startsWith("file:") } .collect { - case i.SymbolData(sym, Some(pos), _, flags, name, _) - if flags.hasOneOfFlags(CLASS | TRAIT | OBJECT) && { + case i.SymbolData(sym, Some(pos), _, Some(info)) + if info.isOneOf( + Kind.CLASS, + Kind.OBJECT, + Kind.TRAIT, + Kind.INTERFACE + ) && { // NOTE(olafur) fuzzy-wuzzy doesn't seem to do a great job // for camelcase searches like "DocSymPr" when looking for // "DocumentSymbolProvider. We should try and port something // like https://blog.forrestthewoods.com/reverse-engineering-sublime-text-s-fuzzy-match-4cffeed33fdb // instead. - FuzzySearch.partialRatio(query, name) >= 90 + FuzzySearch.partialRatio(query, info.name) >= 90 } => SymbolInformation( - name, - flags.toSymbolKind, + info.name, + info.toSymbolKind, pos.toLocation, Some(sym.stripPrefix("_root_.")) ) diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala index 25bdc84689a..44832f542ae 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala @@ -5,10 +5,11 @@ import java.util.function.UnaryOperator import scala.collection.concurrent.TrieMap import scala.meta.metals.index.Position import scala.meta.metals.index.Range -import scala.meta.metals.index.Ranges import scala.meta.metals.index.SymbolData import com.typesafe.scalalogging.LazyLogging import org.langmeta.semanticdb.Symbol +import scala.meta.internal.semanticdb3 +import scala.meta.metals.Uri class InMemorySymbolIndexer( // simplest thing I could think of to get something off the ground. @@ -49,28 +50,32 @@ class InMemorySymbolIndexer( index.copy(definition = Some(position)) } - override def addDenotation( - symbol: String, - kind: Int, - name: String, - signature: String - ): Unit = updated(symbol) { index => - index.copy(kind = kind, signature = signature, name = name) + override def addSymbolInformation( + info: semanticdb3.SymbolInformation + ): Unit = updated(info.symbol) { index => + index.copy(info = Some(info)) } override def addReference( - filename: String, // TODO(olafur) change to java.net.URI? + uri: Uri, // TODO(olafur) change to java.net.URI? range: Range, symbol: String // TODO(olafur) move to first argument? ): Unit = updated(symbol) { index => - val ranges = index.references.getOrElse(filename, Ranges()) - val newRanges = ranges.addRanges(range) - val newReferences = index.references + (filename -> newRanges) + val ranges = index.references.getOrElse(uri, Nil) + val newRanges = range +: ranges + val newReferences = index.references.updated(uri, newRanges) index.copy(references = newReferences) } private def newValue(symbol: String) = - new AtomicReference(SymbolData(symbol = symbol)) + new AtomicReference( + SymbolData( + symbol = symbol, + definition = None, + references = Map.empty, + info = None + ) + ) private def updated(symbol: String)(f: SymbolData => SymbolData): Unit = { val value = symbols.getOrElseUpdate(symbol, newValue(symbol)) diff --git a/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala index baca75b16e1..0d443776742 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala @@ -6,6 +6,7 @@ import scala.meta.metals.{index => i} import scala.{meta => m} import org.langmeta.io.AbsolutePath import org.langmeta.languageserver.InputEnrichments._ +import scala.meta.internal.semanticdb3.SymbolOccurrence object InverseSymbolIndexer { @@ -48,24 +49,34 @@ object InverseSymbolIndexer { uri: Uri, symbol: String, range: i.Range, - definition: Boolean + role: SymbolOccurrence.Role ): Unit = { val doc = get(uri) val pos = doc.input.toPosition(range) val rs = - m.ResolvedName(pos, m.Symbol(symbol), isDefinition = definition) + m.ResolvedName(pos, m.Symbol(symbol), isDefinition = role.isDefinition) val newDoc = doc.copy(names = rs :: doc.names) db(doc.input.syntax) = newDoc } symbols.foreach { symbol => symbol.definition.collect { - case i.Position(Uri(uri), Some(range)) => - handleResolvedName(uri, symbol.symbol, range, definition = true) + case i.Position(Uri(uri), range) => + handleResolvedName( + uri, + symbol.symbol, + range, + SymbolOccurrence.Role.DEFINITION + ) } symbol.references.collect { - case (Uri(uri), ranges) => - ranges.ranges.foreach { range => - handleResolvedName(uri, symbol.symbol, range, definition = false) + case (uri, ranges) => + ranges.foreach { range => + handleResolvedName( + uri, + symbol.symbol, + range, + SymbolOccurrence.Role.REFERENCE + ) } } } diff --git a/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala index fb2f314ffc6..8b2d34dc6ec 100644 --- a/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala @@ -2,6 +2,8 @@ package scala.meta.metals.search import scala.meta.metals.index._ import org.langmeta.semanticdb.Symbol +import scala.meta.internal.semanticdb3 +import scala.meta.metals.Uri /** * A key/value store with String keys (by symbol syntax) and @@ -52,28 +54,23 @@ trait SymbolIndexer { self => /** * Register metadata about a symbol. * - * @param kind the kind of this symbol, see SymbolInformation.Kind in + * @param info The information about this symbol * https://github.com/scalameta/scalameta/blob/master/semanticdb/semanticdb3/semanticdb3.md#symbolinformation - * @param name the name of the symbol, example "get" for scala.Option.get - * @param signature the type signature of this symbol, example "List[T]" for List.tail */ - def addDenotation( - symbol: String, - kind: Int, - name: String, - signature: String + def addSymbolInformation( + info: semanticdb3.SymbolInformation ): Unit /** * Reguster a reference/call-site to this symbol. * - * @param filename must be URI, can either be file on local disk or entry - * in jar/zip. + * @param uri must be URI, can either be file on local disk or entry + * in jar/zip. * @param range start/end offset where this symbol is referenced. * @param symbol */ def addReference( - filename: String, + uri: Uri, range: Range, symbol: String ): Unit diff --git a/metals/src/main/scala/scala/meta/metals/search/TokenEditDistance.scala b/metals/src/main/scala/scala/meta/metals/search/TokenEditDistance.scala index 07df0e56e29..9c04159de12 100644 --- a/metals/src/main/scala/scala/meta/metals/search/TokenEditDistance.scala +++ b/metals/src/main/scala/scala/meta/metals/search/TokenEditDistance.scala @@ -8,6 +8,7 @@ import com.typesafe.scalalogging.LazyLogging import difflib._ import difflib.myers.Equalizer import org.langmeta.languageserver.InputEnrichments._ +import scala.meta.metals.Uri sealed trait EmptyResult object EmptyResult { @@ -21,9 +22,11 @@ object EmptyResult { final class TokenEditDistance private (matching: Array[MatchingToken]) { private val isEmpty: Boolean = matching.length == 0 - private val ThisUri: String = originalInput match { - case Input.VirtualFile(uri, _) => uri - case _ => originalInput.syntax + private val ThisUri: Uri = originalInput match { + case Input.VirtualFile(uri, _) => Uri(uri) + case Input.String(_) => Uri("file:///string") + case Input.None => Uri("file:///none") + case _ => throw new IllegalArgumentException(originalInput.syntax) } private def originalInput: Input = if (isEmpty) Input.None @@ -65,11 +68,11 @@ final class TokenEditDistance private (matching: Array[MatchingToken]) { /** Convert the reference positions to match the revised input. */ def toRevisedReferences(data: i.SymbolData): i.SymbolData = { val referencesAdjusted = data.references.get(ThisUri) match { - case Some(i.Ranges(ranges)) => - val newRanges = ranges.collect { case RevisedRange(range) => range } - val newData = data.copy( - references = data.references + (ThisUri -> i.Ranges(newRanges)) - ) + case Some(ranges) => + val newRanges = + ranges.collect { case RevisedRange(range) => range } + val newData = + data.copy(references = data.references + (ThisUri -> newRanges)) newData case _ => data } @@ -79,7 +82,7 @@ final class TokenEditDistance private (matching: Array[MatchingToken]) { /** Convert the definition position to match the revised input. */ def toRevisedDefinition(data: i.SymbolData): i.SymbolData = { data.definition match { - case Some(i.Position(ThisUri, Some(range))) => + case Some(i.Position(uri, range)) if uri == ThisUri.value => toRevised(range.startLine, range.startColumn) match { case Left(EmptyResult.NoMatch) => data.copy(definition = None) case Left(EmptyResult.Unchanged) => data @@ -87,8 +90,8 @@ final class TokenEditDistance private (matching: Array[MatchingToken]) { val newData = data.copy( definition = Some( i.Position( - ThisUri, - Some(newPos.toIndexRange) + ThisUri.value, + newPos.toIndexRange ) ) ) diff --git a/metals/src/test/scala/tests/provider/DiagnosticsProviderTest.scala b/metals/src/test/scala/tests/provider/DiagnosticsProviderTest.scala index bbe08a86e51..b807b51dc1d 100644 --- a/metals/src/test/scala/tests/provider/DiagnosticsProviderTest.scala +++ b/metals/src/test/scala/tests/provider/DiagnosticsProviderTest.scala @@ -23,7 +23,7 @@ object DiagnosticsProviderTest extends CompilerSuite with LazyLogging { val logFile = tmp.resolve("metals.log").toFile val out = new PrintStream(new FileOutputStream(logFile)) val scalafixConfPath = ".customScalafixConfPath" - + val config = Observable.now( Configuration( scalac = Configuration.Scalac( From 4ec168fcbc039cf8fe13d23159b32eb2ee510419 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 14:20:39 +0200 Subject: [PATCH 05/25] Add note on package objects for workspace/symbol --- .../main/scala/scala/meta/metals/providers/RenameProvider.scala | 2 +- .../scala/scala/meta/metals/search/InMemorySymbolIndex.scala | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/metals/src/main/scala/scala/meta/metals/providers/RenameProvider.scala b/metals/src/main/scala/scala/meta/metals/providers/RenameProvider.scala index c94c93edb6a..d1038118e1c 100644 --- a/metals/src/main/scala/scala/meta/metals/providers/RenameProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/providers/RenameProvider.scala @@ -45,7 +45,7 @@ object RenameProvider extends LazyLogging { false } } - position <- reference.referencePositions(true) + position <- reference.referencePositions(withDefinition = true) } yield { TextEdit(position.toLocation.range, newName) } diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala index 1d3aadeeed9..bc8c510a38b 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala @@ -220,6 +220,8 @@ class InMemorySymbolIndex( Kind.OBJECT, Kind.TRAIT, Kind.INTERFACE + // Blocked by https://github.com/scalameta/scalameta/issues/1484, package object has name "package". + // Kind.PACKAGE_OBJECT ) && { // NOTE(olafur) fuzzy-wuzzy doesn't seem to do a great job // for camelcase searches like "DocSymPr" when looking for From 17380e82e4d5759c80f32d7f5f76af7c0577a461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 15:37:52 +0200 Subject: [PATCH 06/25] Compute definition on the fly for local symbols. SemanticDB v3 changed the encoding for local symbols so that they are only unique within a single source file but can have conflicts with local symbols in other source files. This commit refactors the indexing pipeline to not index local symbols and compute symbol data for local symbols on the fly instead. This refactoring should lower memory usage as well. --- .../meta/metals/ScalametaEnrichments.scala | 52 +++++++++- .../meta/metals/search/BinarySearch.scala | 16 ++-- .../metals/search/InMemoryDocumentIndex.scala | 14 ++- .../metals/search/InMemorySymbolIndex.scala | 94 +++++++++++-------- .../metals/search/InMemorySymbolIndexer.scala | 1 + .../meta/metals/search/SymbolIndex.scala | 23 ++++- 6 files changed, 151 insertions(+), 49 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala index 221c80ae1f1..0982edca279 100644 --- a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala +++ b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala @@ -17,6 +17,7 @@ import org.langmeta.io.AbsolutePath import org.langmeta.internal.semanticdb._ import scala.meta.internal.semanticdb3.SymbolInformation.Kind import scala.meta.internal.semanticdb3.SymbolInformation.Property +import scala.meta.metals.search.SymbolInformationsBySymbol // Extension methods for convenient reuse of data conversions between // scala.meta._ and language.types._ @@ -324,9 +325,54 @@ object ScalametaEnrichments { } } + implicit class XtensionSchemaRange(val r: semanticdb3.Range) { + def toLSP: l.Range = l.Range( + startLine = r.startLine, + startColumn = r.startCharacter, + endLine = r.endLine, + endColumn = r.endCharacter + ) + def toLocation(uri: String): l.Location = { + l.Location(uri, toLSP) + } + } implicit class XtensionSchemaDocument(val document: semanticdb3.TextDocument) extends AnyVal { + def computeSymbolDataForLocalSymbol( + symbol: String + ): Option[i.SymbolData] = { + info(symbol).map { info => + val uri = Uri(document.uri) + var definition: Option[Location] = None + val references = List.newBuilder[l.Range] + document.occurrences.foreach { o => + if (o.symbol == symbol && o.range.isDefined) { + if (o.role.isDefinition) { + definition = Some(o.range.get.toLocation(document.uri)) + } else if (o.role.isReference) { + references += o.range.get.toLSP + } + } + } + i.SymbolData( + symbol = symbol, + definition = definition, + references = Map(uri -> references.result()), + info = Some(info) + ) + } + } + + def info(symbol: String): Option[semanticdb3.SymbolInformation] = { + document.symbols match { + case s: SymbolInformationsBySymbol => + s.lookupSymbol(symbol) + case _ => + document.symbols.find(_.symbol == symbol) + } + } + /** Returns scala.meta.Document from protobuf schema.Document */ def toMetaDocument: m.Document = semanticdb3.TextDocuments(document :: Nil).toDb(None).documents.head @@ -334,7 +380,7 @@ object ScalametaEnrichments { implicit class XtensionSymbolInformation( val info: semanticdb3.SymbolInformation - ) { + ) extends AnyVal { import Property._ def isOneOf(kind: Kind*): Boolean = { kind.contains(info.kind) @@ -371,7 +417,8 @@ object ScalametaEnrichments { ) } } - implicit class XtensionIntAsSymbolKind(val flags: Int) { + + implicit class XtensionIntAsSymbolKind(val flags: Int) extends AnyVal { def hasOneOfFlags(flags: Long): Boolean = (this.flags & flags) != 0L def toSymbolKind: SymbolKind = @@ -382,4 +429,5 @@ object ScalametaEnrichments { else SymbolKind.Module } + } diff --git a/metals/src/main/scala/scala/meta/metals/search/BinarySearch.scala b/metals/src/main/scala/scala/meta/metals/search/BinarySearch.scala index 0dddb3de500..421583b7b54 100644 --- a/metals/src/main/scala/scala/meta/metals/search/BinarySearch.scala +++ b/metals/src/main/scala/scala/meta/metals/search/BinarySearch.scala @@ -8,6 +8,9 @@ object BinarySearch { case object Equal extends ComparisonResult case object Smaller extends ComparisonResult + def array[T](arr: Array[T], compare: T => ComparisonResult): Option[T] = + apply[T](i => arr(i), compare, arr.length) + /** * Binary search using a custom compare function. * @@ -15,7 +18,7 @@ object BinarySearch { * by a custom mapping function, you must search by an element of the same * type as the elements of the Seq. * - * @param array Must be sorted according to compare function so that for all + * @param lookup Must be sorted according to compare function so that for all * i > j, compare(array(i), array(i)) == Greater. * @param compare Callback used at every guess index to determine whether * the search element has been found, or whether to search @@ -23,21 +26,22 @@ object BinarySearch { * @return The first element where compare(element) == Equal. There is no guarantee * which element is chosen if many elements return Equal. */ - def array[T]( - array: Array[T], - compare: T => ComparisonResult + def apply[T]( + lookup: Int => T, + compare: T => ComparisonResult, + length: Int ): Option[T] = { @tailrec def loop(lo: Int, hi: Int): Option[T] = if (lo > hi) None else { val mid = lo + (hi - lo) / 2 - val curr = array(mid) + val curr = lookup(mid) compare(curr) match { case Greater => loop(lo, mid - 1) case Equal => Some(curr) case Smaller => loop(mid + 1, hi) } } - loop(0, array.length - 1) + loop(0, length - 1) } } diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala index 89a60c6e9a6..c782a54727b 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala @@ -4,10 +4,11 @@ import java.util import java.util.concurrent.ConcurrentHashMap import scala.meta.metals.Uri import com.typesafe.scalalogging.LazyLogging +import scala.meta.internal.semanticdb3.SymbolInformation import scala.meta.internal.semanticdb3.TextDocument class InMemoryDocumentIndex( - documents: util.Map[Uri, TextDocument] = new ConcurrentHashMap() + documents: util.Map[Uri, TextDocument] = new ConcurrentHashMap(), ) extends DocumentIndex with LazyLogging { override def getDocument(uri: Uri): Option[TextDocument] = @@ -19,3 +20,14 @@ class InMemoryDocumentIndex( documents.put(uri, document) } } + +class SymbolInformationsBySymbol(infos: util.HashMap[String, SymbolInformation]) + extends Seq[SymbolInformation] { + def lookupSymbol(symbol: String): Option[SymbolInformation] = Option(infos.get(symbol)) + import scala.collection.JavaConverters._ + override def length: Int = infos.size() + override def iterator: Iterator[SymbolInformation] = + infos.values().iterator().asScala + override def apply(idx: Int): SymbolInformation = + throw new UnsupportedOperationException("SymbolInformationsBySymbol.apply") +} diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala index bc8c510a38b..06c4a1ded3d 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala @@ -16,6 +16,7 @@ import org.langmeta.lsp.SymbolInformation import org.langmeta.jsonrpc.JsonRpcClient import scala.meta.metals.{index => i} import com.typesafe.scalalogging.LazyLogging +import java.util import me.xdrop.fuzzywuzzy.FuzzySearch import org.langmeta.inputs.Input import org.langmeta.inputs.Position @@ -30,7 +31,6 @@ import monix.eval.Task import monix.execution.Scheduler import monix.reactive.Observable import scala.meta.internal.semanticdb3.SymbolInformation.Kind -import scala.meta.internal.semanticdb3.SymbolInformation.Property import scala.util.control.NonFatal class InMemorySymbolIndex( @@ -91,12 +91,11 @@ class InMemorySymbolIndex( def definitionData( symbol: Symbol ): Option[SymbolData] = { - (symbol :: symbol.definitionAlternative) - .collectFirst { - case symbolIndexer(data) if data.definition.nonEmpty => - logger.info(s"Found definition symbol ${data.symbol}") - data - } + (symbol :: symbol.definitionAlternative).collectFirst { + case symbolIndexer(data) if data.definition.nonEmpty => + logger.info(s"Found definition symbol ${data.symbol}") + data + } } def data(symbol: Symbol): Option[SymbolData] = @@ -106,13 +105,12 @@ class InMemorySymbolIndex( def referencesData( symbol: Symbol ): List[SymbolData] = { - (symbol :: symbol.referenceAlternatives) - .collect { - case symbolIndexer(data) => - if (data.symbol != symbol.syntax) - logger.info(s"Adding alternative references ${data.symbol}") - data - } + (symbol :: symbol.referenceAlternatives).collect { + case symbolIndexer(data) => + if (data.symbol != symbol.syntax) + logger.info(s"Adding alternative references ${data.symbol}") + data + } } def indexDependencyClasspath( @@ -179,30 +177,52 @@ class InMemorySymbolIndex( def indexDocument(document: TextDocument): Effects.IndexSemanticdb = { val uri = Uri(document.uri) val input = Input.VirtualFile(document.uri, document.text) - documentIndex.putDocument(uri, document) - document.occurrences.foreach { - // TODO(olafur) handle local symbols on the fly from a `Document` in go-to-definition - // local symbols don't need to be indexed globally, by skipping them we should - // def isLocalSymbol(sym: String): Boolean = - // !sym.endsWith(".") && - // !sym.endsWith("#") && - // !sym.endsWith(")") - // be able to minimize the size of the global index significantly. - // case s.SymbolOccurrence(_, sym, _) if isLocalSymbol(sym) => // Do nothing, local symbol. - case SymbolOccurrence(Some(r), sym, SymbolOccurrence.Role.DEFINITION) => - symbolIndexer.addDefinition( - sym, - i.Position(document.uri, input.toIndexRange(r)) - ) - case SymbolOccurrence(Some(r), sym, SymbolOccurrence.Role.REFERENCE) => - symbolIndexer.addReference( - Uri(document.uri), - input.toIndexRange(r), - sym - ) - case _ => + val locals = new util.HashMap[String, semanticdb3.SymbolInformation]() + document.symbols.foreach { info => + if (info.kind.isLocal) { + locals.put(info.symbol, info) + } else { + symbolIndexer.addSymbolInformation(info) + } + } + val documentWithOnlyLocalSymbols = + document.copy(symbols = new SymbolInformationsBySymbol(locals)) + documentIndex.putDocument(uri, documentWithOnlyLocalSymbols) + document.occurrences.foreach { occurence => + val isGlobal = locals.get(occurence.symbol) == null + if (isGlobal) { + occurence match { + // TODO(olafur) handle local symbols on the fly from a `Document` in go-to-definition + // local symbols don't need to be indexed globally, by skipping them we should + // def isLocalSymbol(sym: String): Boolean = + // !sym.endsWith(".") && + // !sym.endsWith("#") && + // !sym.endsWith(")") + // be able to minimize the size of the global index significantly. + // case s.SymbolOccurrence(_, sym, _) if isLocalSymbol(sym) => // Do nothing, local symbol. + case SymbolOccurrence( + Some(r), + sym, + SymbolOccurrence.Role.DEFINITION + ) => + symbolIndexer.addDefinition( + sym, + i.Position(document.uri, input.toIndexRange(r)) + ) + case SymbolOccurrence( + Some(r), + sym, + SymbolOccurrence.Role.REFERENCE + ) => + symbolIndexer.addReference( + Uri(document.uri), + input.toIndexRange(r), + sym + ) + case _ => + } + } } - document.symbols.foreach(symbolIndexer.addSymbolInformation) Effects.IndexSemanticdb } diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala index 44832f542ae..26ef59cfd10 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala @@ -53,6 +53,7 @@ class InMemorySymbolIndexer( override def addSymbolInformation( info: semanticdb3.SymbolInformation ): Unit = updated(info.symbol) { index => + require(!info.kind.isLocal, "Local symbols should not be globally indexed!") index.copy(info = Some(info)) } diff --git a/metals/src/main/scala/scala/meta/metals/search/SymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/SymbolIndex.scala index 41269e161c0..bcacce2441d 100644 --- a/metals/src/main/scala/scala/meta/metals/search/SymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/SymbolIndex.scala @@ -14,6 +14,7 @@ import org.langmeta.semanticdb.Symbol import monix.eval.Task import monix.execution.Scheduler import monix.reactive.Observable +import scala.meta.metals.ScalametaEnrichments._ trait SymbolIndex extends LazyLogging { @@ -30,7 +31,11 @@ trait SymbolIndex extends LazyLogging { def findDefinition(path: Uri, line: Int, column: Int): Option[SymbolData] = for { (symbol, edit) <- findSymbol(path, line, column) - data <- definitionData(symbol) + data <- { + val result = definitionData(symbol) + if (result.isEmpty) localData(path, symbol.syntax) + else result + } } yield edit.toRevisedDefinition(data) /** Returns symbol data for this exact Symbol */ @@ -42,9 +47,19 @@ trait SymbolIndex extends LazyLogging { def findReferences(path: Uri, line: Int, column: Int): List[SymbolData] = for { (symbol, edit) <- findSymbol(path, line, column).toList - data <- referencesData(symbol) + data <- { + val result = referencesData(symbol) + if (result.isEmpty) localData(path, symbol.syntax).toList + else result + } } yield edit.toRevisedReferences(data) + def localData(path: Uri, symbol: String): Option[SymbolData] = + for { + doc <- documentIndex.getDocument(path) + data <- doc.computeSymbolDataForLocalSymbol(symbol) + } yield data + /** Returns symbol definitions in this workspace */ def workspaceSymbols(query: String): List[SymbolInformation] @@ -53,7 +68,9 @@ trait SymbolIndex extends LazyLogging { ): Task[Effects.IndexSourcesClasspath] /** Register this Database to symbol indexer. */ - def indexDatabase(document: semanticdb3.TextDocuments): Effects.IndexSemanticdb + def indexDatabase( + document: semanticdb3.TextDocuments + ): Effects.IndexSemanticdb /** Remove any persisted files from index returning to a clean start */ def clearIndex(): Unit From 5eba30ca28c6d125d61b2a8a878e7931de910232 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 16:40:53 +0200 Subject: [PATCH 07/25] Fix indexing for local symbols. It seems that there are - occurrencess that have no associated symbol information - local symbol informations with non-local kind --- .../scala/meta/metals/ScalametaEnrichments.scala | 7 +++++++ .../meta/metals/search/InMemorySymbolIndex.scala | 6 ++++-- .../meta/metals/search/InMemorySymbolIndexer.scala | 12 +++++++++--- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala index 0982edca279..001a249cd83 100644 --- a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala +++ b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala @@ -382,6 +382,13 @@ object ScalametaEnrichments { val info: semanticdb3.SymbolInformation ) extends AnyVal { import Property._ + + def isLocal: Boolean = { + info.kind.isLocal || + // Workaround for https://github.com/scalameta/scalameta/issues/1486 + info.symbol.startsWith("local") + } + def isOneOf(kind: Kind*): Boolean = { kind.contains(info.kind) } diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala index 06c4a1ded3d..388d45885d8 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala @@ -179,7 +179,7 @@ class InMemorySymbolIndex( val input = Input.VirtualFile(document.uri, document.text) val locals = new util.HashMap[String, semanticdb3.SymbolInformation]() document.symbols.foreach { info => - if (info.kind.isLocal) { + if (info.isLocal) { locals.put(info.symbol, info) } else { symbolIndexer.addSymbolInformation(info) @@ -189,7 +189,9 @@ class InMemorySymbolIndex( document.copy(symbols = new SymbolInformationsBySymbol(locals)) documentIndex.putDocument(uri, documentWithOnlyLocalSymbols) document.occurrences.foreach { occurence => - val isGlobal = locals.get(occurence.symbol) == null + val isGlobal = + locals.get(occurence.symbol) == null && + !occurence.symbol.startsWith("local") if (isGlobal) { occurence match { // TODO(olafur) handle local symbols on the fly from a `Document` in go-to-definition diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala index 26ef59cfd10..b5df2fb4fcb 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala @@ -58,17 +58,22 @@ class InMemorySymbolIndexer( } override def addReference( - uri: Uri, // TODO(olafur) change to java.net.URI? + uri: Uri, range: Range, - symbol: String // TODO(olafur) move to first argument? + symbol: String ): Unit = updated(symbol) { index => + require( + !symbol.startsWith("local"), + s"Can't index local symbol '$symbol' in uri $uri" + ) val ranges = index.references.getOrElse(uri, Nil) val newRanges = range +: ranges val newReferences = index.references.updated(uri, newRanges) index.copy(references = newReferences) } - private def newValue(symbol: String) = + private def newValue(symbol: String) = { + require(!symbol.startsWith("local"), symbol) new AtomicReference( SymbolData( symbol = symbol, @@ -77,6 +82,7 @@ class InMemorySymbolIndexer( info = None ) ) + } private def updated(symbol: String)(f: SymbolData => SymbolData): Unit = { val value = symbols.getOrElseUpdate(symbol, newValue(symbol)) From afa835e83ab8d782b646865d098fadef3760b477 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 16:41:43 +0200 Subject: [PATCH 08/25] Don't index message-only documents. When a file contains deprecation messages, semanticdb-scalac appends an empty TextDocument containing only the diagnostics section. --- .../meta/metals/search/InMemorySymbolIndex.scala | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala index 388d45885d8..108e5361034 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala @@ -153,10 +153,15 @@ class InMemorySymbolIndex( } /** Register this Database to symbol indexer. */ - def indexDatabase(document: TextDocuments): Effects.IndexSemanticdb = { - document.documents.foreach { doc => - try indexDocument(doc) - catch { + def indexDatabase(documents: TextDocuments): Effects.IndexSemanticdb = { + documents.documents.foreach { doc => + try { + // semanticdb-scalac emits additional empty TextDocument with deprecation warning diagnostics. + val isOnlyDiagnostics = doc.diagnostics.nonEmpty && doc.occurrences.isEmpty + if (!isOnlyDiagnostics) { + indexDocument(doc) + } + } catch { case NonFatal(e) => logger.error(s"Failed to index ${doc.uri}", e) } From b37d6348d591a953e5526022b6add584c4fae035 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 17:19:47 +0200 Subject: [PATCH 09/25] Fix mtags tests --- .../scala/meta/metals/mtags/JavaMtags.scala | 6 +- .../meta/metals/mtags/MtagsIndexer.scala | 6 +- .../scala/meta/metals/mtags/ScalaMtags.scala | 1 + .../scala/tests/mtags/JavaMtagsTest.scala | 103 ++++++-------- .../scala/tests/mtags/ScalaMtagsTest.scala | 130 +++++++++--------- .../refactoring/RemoveUnusedImportsTest.scala | 3 +- 6 files changed, 116 insertions(+), 133 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/mtags/JavaMtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/JavaMtags.scala index 5cc0ad94591..56769a2823a 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/JavaMtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/JavaMtags.scala @@ -14,6 +14,7 @@ import org.langmeta.inputs.Position import org.langmeta.languageserver.InputEnrichments._ import scala.meta.internal.semanticdb3.Language +// TODO, emit correct method overload symbols https://github.com/scalameta/metals/issues/281 object JavaMtags { private implicit class XtensionJavaModel(val m: JavaModel) extends AnyVal { def lineNumber: Int = m.getLineNumber - 1 @@ -79,7 +80,7 @@ object JavaMtags { else classes.forEach(visitClass) def visitClass(cls: JavaClass): Unit = - withOwner(owner(cls.isStatic)) { + withOwner(owner) { val kind = if (cls.isInterface) Kind.INTERFACE else Kind.CLASS val pos = toRangePosition(cls.lineNumber, cls.getName) tpe( @@ -91,11 +92,10 @@ object JavaMtags { visitClasses(cls.getNestedClasses) visitFields(cls.getMethods) visitFields(cls.getFields) - visitFields(cls.getEnumConstants) } def visitMember[T <: JavaMember](m: T): Unit = - withOwner(owner(m.isStatic)) { + withOwner(owner) { val name = m.getName val line = m match { case c: JavaMethod => c.lineNumber diff --git a/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala b/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala index 64abd3ce671..905dc8666be 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala @@ -20,9 +20,7 @@ trait MtagsIndexer { private val root: Symbol.Global = Symbol.Global(Symbol.None, Signature.Term("_root_")) var currentOwner: Symbol.Global = root - def owner(isStatic: Boolean): Symbol.Global = - if (isStatic) currentOwner.toTerm - else currentOwner + def owner = currentOwner def withOwner[A](owner: Symbol.Global = currentOwner)(thunk: => A): A = { val old = currentOwner currentOwner = owner @@ -64,7 +62,7 @@ trait MtagsIndexer { val syntax = currentOwner.syntax val role = if (kind.isPackage) s.SymbolOccurrence.Role.REFERENCE - else s.SymbolOccurrence.Role.REFERENCE + else s.SymbolOccurrence.Role.DEFINITION names += s.SymbolOccurrence( range = Some(definition.toSchemaRange), syntax, diff --git a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala index 307d710728a..f8818225e5d 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala @@ -6,6 +6,7 @@ import scala.meta.internal.semanticdb3.SymbolInformation.Kind import scala.meta.internal.semanticdb3.Language import scala.meta.internal.semanticdb3.SymbolInformation.Property +// TODO, emit correct method overload symbols https://github.com/scalameta/metals/issues/282 object ScalaMtags { def index(input: Input.VirtualFile): MtagsIndexer = { val root: Source = input.parse[Source].get diff --git a/metals/src/test/scala/tests/mtags/JavaMtagsTest.scala b/metals/src/test/scala/tests/mtags/JavaMtagsTest.scala index 54f0e4e0aa6..676afd35814 100644 --- a/metals/src/test/scala/tests/mtags/JavaMtagsTest.scala +++ b/metals/src/test/scala/tests/mtags/JavaMtagsTest.scala @@ -18,18 +18,16 @@ object JavaMtagsTest extends BaseMtagsTest { |Java | |Names: - |[8..9): a => _root_.a. - |[10..11): b => _root_.a.b. - |[23..24): A <= _root_.a.b.A. - |[23..24): A <= _root_.a.b.A# - |[43..44): a <= _root_.a.b.A#a. + |[8..9): a => a. + |[10..11): b => a.b. + |[23..24): A <= a.b.A# + |[43..44): a <= a.b.A#a. | |Symbols: - |_root_.a. => package a - |_root_.a.b. => package b - |_root_.a.b.A# => trait A - |_root_.a.b.A#a. => def a - |_root_.a.b.A. => object A + |a. => javadefined package a + |a.b. => javadefined package b + |a.b.A# => javadefined interface A + |a.b.A#a. => javadefined method a |""".stripMargin ) @@ -48,24 +46,18 @@ object JavaMtagsTest extends BaseMtagsTest { |Java | |Names: - |[7..8): B <= _root_.B. - |[7..8): B <= _root_.B# - |[18..19): c <= _root_.B.c. - |[53..54): d <= _root_.B#d. - |[76..77): E <= _root_.B#E. - |[76..77): E <= _root_.B#E# - |[103..104): F <= _root_.B.F. - |[103..104): F <= _root_.B.F# + |[7..8): B <= B# + |[18..19): c <= B#c. + |[53..54): d <= B#d. + |[76..77): E <= B#E# + |[103..104): F <= B#F# | |Symbols: - |_root_.B# => class B - |_root_.B#E# => class E - |_root_.B#E. => object E - |_root_.B#d. => def d - |_root_.B. => object B - |_root_.B.F# => class F - |_root_.B.F. => object F - |_root_.B.c. => def c + |B# => javadefined class B + |B#E# => javadefined class E + |B#F# => javadefined class F + |B#c. => javadefined method c + |B#d. => javadefined method d """.stripMargin ) @@ -83,18 +75,14 @@ object JavaMtagsTest extends BaseMtagsTest { |Java | |Names: - |[6..7): G <= _root_.G. - |[12..13): H <= _root_.G.H. - |[12..13): H <= _root_.G.H. - |[17..18): I <= _root_.G.I. - |[17..18): I <= _root_.G.I. + |[6..7): G <= G# + |[12..13): H <= G#H. + |[17..18): I <= G#I. | |Symbols: - |_root_.G. => object G - |_root_.G.H. => val H - |_root_.G.H. => val H - |_root_.G.I. => val I - |_root_.G.I. => val I + |G# => javadefined enum class G + |G#H. => javadefined field H + |G#I. => javadefined field I |""".stripMargin ) @@ -110,14 +98,12 @@ object JavaMtagsTest extends BaseMtagsTest { |Java | |Names: - |[14..15): J <= _root_.J. - |[14..15): J <= _root_.J# - |[46..51): FIELD <= _root_.J.FIELD. + |[14..15): J <= J# + |[46..51): FIELD <= J#FIELD. | |Symbols: - |_root_.J# => class J - |_root_.J. => object J - |_root_.J.FIELD. => val FIELD + |J# => javadefined class J + |J#FIELD. => javadefined field FIELD """.stripMargin ) @@ -141,16 +127,15 @@ object JavaMtagsTest extends BaseMtagsTest { |Java | |Names: - |[8..9): k => _root_.k. - |[28..29): K <= _root_.k.K. - |[28..29): K <= _root_.k.K# - |[36..37): l <= _root_.k.K.l. + |[8..9): k => k. + |[28..29): K <= k.K. + |[28..29): K <= k.K# + |[36..37): l <= k.K.l. | |Symbols: - |_root_.k. => package k - |_root_.k.K# => trait K - |_root_.k.K#m. => def m - |_root_.k.K. => object K + |k. => javadefined package k + |k.K# => javadefined interface K + |k.K#m. => javadefined method m """.stripMargin ) @@ -174,18 +159,16 @@ object JavaMtagsTest extends BaseMtagsTest { |Java | |Names: - |[219..223): java => _root_.java. - |[224..226): io => _root_.java.io. - |[260..277): DefaultFileSystem <= _root_.java.io.DefaultFileSystem. - |[260..277): DefaultFileSystem <= _root_.java.io.DefaultFileSystem# - |[387..400): getFileSystem <= _root_.java.io.DefaultFileSystem.getFileSystem. + |[219..223): java => java. + |[224..226): io => java.io. + |[260..277): DefaultFileSystem <= java.io.DefaultFileSystem# + |[387..400): getFileSystem <= java.io.DefaultFileSystem#getFileSystem. | |Symbols: - |_root_.java. => package java - |_root_.java.io. => package io - |_root_.java.io.DefaultFileSystem# => class DefaultFileSystem - |_root_.java.io.DefaultFileSystem. => object DefaultFileSystem - |_root_.java.io.DefaultFileSystem.getFileSystem. => def getFileSystem + |java. => javadefined package java + |java.io. => javadefined package io + |java.io.DefaultFileSystem# => javadefined class DefaultFileSystem + |java.io.DefaultFileSystem#getFileSystem. => javadefined method getFileSystem """.stripMargin assertNoDiff(obtained, expected) } diff --git a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala index b021389be94..009a4637578 100644 --- a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala +++ b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala @@ -20,35 +20,35 @@ object ScalaMtagsTest extends BaseMtagsTest { """.stripMargin, """ |Language: - |Scala212 + |Scala | |Names: - |[22..23): D <= _root_.a.b.c.D. - |[33..34): e <= _root_.a.b.c.D.e. - |[61..62): f <= _root_.a.b.c.D.f. - |[74..75): g <= _root_.a.b.c.D.g. - |[89..90): H <= _root_.a.b.c.D.H# - |[97..98): x <= _root_.a.b.c.D.H#x. - |[114..115): I <= _root_.a.b.c.D.I# - |[127..128): x <= _root_.a.b.c.D.I#x. - |[143..144): y <= _root_.a.b.c.D.I#y. - |[159..160): z <= _root_.a.b.c.D.I#z. - |[181..182): J <= _root_.a.b.c.D.J. - |[189..190): k <= _root_.a.b.c.D.J.k. + |[22..23): D <= a.b.c.D. + |[33..34): e <= a.b.c.D.e. + |[61..62): f <= a.b.c.D.f. + |[74..75): g <= a.b.c.D.g. + |[89..90): H <= a.b.c.D.H# + |[97..98): x <= a.b.c.D.H#x. + |[114..115): I <= a.b.c.D.I# + |[127..128): x <= a.b.c.D.I#x. + |[143..144): y <= a.b.c.D.I#y. + |[159..160): z <= a.b.c.D.I#z. + |[181..182): J <= a.b.c.D.J. + |[189..190): k <= a.b.c.D.J.k. | |Symbols: - |_root_.a.b.c.D. => object D - |_root_.a.b.c.D.H# => class H - |_root_.a.b.c.D.H#x. => def x - |_root_.a.b.c.D.I# => trait I - |_root_.a.b.c.D.I#x. => def x - |_root_.a.b.c.D.I#y. => val y - |_root_.a.b.c.D.I#z. => var z - |_root_.a.b.c.D.J. => object J - |_root_.a.b.c.D.J.k. => def k - |_root_.a.b.c.D.e. => def e - |_root_.a.b.c.D.f. => val f - |_root_.a.b.c.D.g. => var g + |a.b.c.D. => object D + |a.b.c.D.H# => class H + |a.b.c.D.H#x. => method x + |a.b.c.D.I# => trait I + |a.b.c.D.I#x. => method x + |a.b.c.D.I#y. => val method y + |a.b.c.D.I#z. => var method z + |a.b.c.D.J. => object J + |a.b.c.D.J.k. => method k + |a.b.c.D.e. => method e + |a.b.c.D.f. => val method f + |a.b.c.D.g. => var method g """.stripMargin ) @@ -61,17 +61,17 @@ object ScalaMtagsTest extends BaseMtagsTest { """.stripMargin, """ |Language: - |Scala212 + |Scala | |Names: - |[16..17): K <= _root_.K. - |[16..17): K <= _root_.K.package. - |[26..27): l <= _root_.K.package.l. + |[16..17): K <= K. + |[16..17): K <= K.package. + |[26..27): l <= K.package.l. | |Symbols: - |_root_.K. => packageobject K - |_root_.K.package. => object package - |_root_.K.package.l. => def l + |K. => packageobject K + |K.package. => object package + |K.package.l. => method l """.stripMargin ) @@ -87,29 +87,29 @@ object ScalaMtagsTest extends BaseMtagsTest { """.stripMargin, """ |Language: - |Scala212 + |Scala | |Names: - |[8..12): pats <= _root_.pats. - |[21..22): o <= _root_.pats.o. - |[24..25): p <= _root_.pats.p. - |[36..37): q <= _root_.pats.q. - |[39..40): r <= _root_.pats.r. - |[52..53): s <= _root_.pats.s. - |[55..56): t <= _root_.pats.t. - |[67..68): v <= _root_.pats.v. - |[70..71): w <= _root_.pats.w. + |[8..12): pats <= pats. + |[21..22): o <= pats.o. + |[24..25): p <= pats.p. + |[36..37): q <= pats.q. + |[39..40): r <= pats.r. + |[52..53): s <= pats.s. + |[55..56): t <= pats.t. + |[67..68): v <= pats.v. + |[70..71): w <= pats.w. | |Symbols: - |_root_.pats. => object pats - |_root_.pats.o. => val o - |_root_.pats.p. => val p - |_root_.pats.q. => val q - |_root_.pats.r. => val r - |_root_.pats.s. => var s - |_root_.pats.t. => var t - |_root_.pats.v. => var v - |_root_.pats.w. => var w + |pats. => object pats + |pats.o. => val method o + |pats.p. => val method p + |pats.q. => val method q + |pats.r. => val method r + |pats.s. => var method s + |pats.t. => var method t + |pats.v. => var method v + |pats.w. => var method w """.stripMargin ) @@ -123,17 +123,17 @@ object ScalaMtagsTest extends BaseMtagsTest { """.stripMargin, """ |Language: - |Scala212 + |Scala | |Names: - |[7..10): Tpe <= _root_.Tpe# - |[20..21): M <= _root_.Tpe#M# - |[29..30): N <= _root_.Tpe#N# + |[7..10): Tpe <= Tpe# + |[20..21): M <= Tpe#M# + |[29..30): N <= Tpe#N# | |Symbols: - |_root_.Tpe# => trait Tpe - |_root_.Tpe#M# => type M - |_root_.Tpe#N# => type N + |Tpe# => trait Tpe + |Tpe#M# => type M + |Tpe#N# => type N """.stripMargin ) @@ -142,17 +142,17 @@ object ScalaMtagsTest extends BaseMtagsTest { "case class A(a: Int, b: String)", """ |Language: - |Scala212 + |Scala | |Names: - |[11..12): A <= _root_.A# - |[13..14): a <= _root_.A#(a) - |[21..22): b <= _root_.A#(b) + |[11..12): A <= A# + |[13..14): a <= A#(a) + |[21..22): b <= A#(b) | |Symbols: - |_root_.A# => class A - |_root_.A#(a) => val param a - |_root_.A#(b) => val param b + |A# => class A + |A#(a) => param a + |A#(b) => param b |""".stripMargin ) } diff --git a/metals/src/test/scala/tests/refactoring/RemoveUnusedImportsTest.scala b/metals/src/test/scala/tests/refactoring/RemoveUnusedImportsTest.scala index d4f47c8c3f3..3470a955c9f 100644 --- a/metals/src/test/scala/tests/refactoring/RemoveUnusedImportsTest.scala +++ b/metals/src/test/scala/tests/refactoring/RemoveUnusedImportsTest.scala @@ -9,7 +9,8 @@ import tests.CompilerSuite // This test suite is not supposed to test the actual scalafix implementation, // it is only supposed to check that the conversion from scalafix.Patch to // languageserver.TextEdit is accurate. -object RemoveUnusedImportsTest extends CompilerSuite { +// Disabled until scalafix 0.6.0 +class RemoveUnusedImportsTest extends CompilerSuite { def check(name: String, original: String, expectedEdits: String): Unit = { test(name) { val uri = Uri.file(name) From d413039699fa560f7ebd6e258781b3b28b126bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 22:39:31 +0200 Subject: [PATCH 10/25] Fix remaining test cases. --- .../meta/metals/ScalametaEnrichments.scala | 2 +- .../scala/scala/meta/metals/Semanticdbs.scala | 2 +- .../metals/search/InverseSymbolIndexer.scala | 4 +- .../scala/tests/search/SymbolIndexTest.scala | 55 +++++++++++-------- 4 files changed, 36 insertions(+), 27 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala index 001a249cd83..1fa3d79384c 100644 --- a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala +++ b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala @@ -241,7 +241,7 @@ object ScalametaEnrichments { ) => Symbol.Global( Symbol.Global(owner, Signature.Type(signature.name)), - param + Signature.Method(param.name, "()") ) } diff --git a/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala b/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala index 95aee5c4276..965faf39eb8 100644 --- a/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala +++ b/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala @@ -111,7 +111,7 @@ object Semanticdbs extends LazyLogging { val bytes = Files.readAllBytes(semanticdbPath.toNIO) val sdb = semanticdb3.TextDocuments.parseFrom(bytes) val docs = sdb.documents.map { d => - val filename = s"file:${cwd.resolve(d.uri)}" + val filename = cwd.resolve(d.uri).toURI.toString logger.info(s"Loading file $filename") d.withUri(filename) .withOccurrences { diff --git a/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala index 0d443776742..3566a1ec8c2 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala @@ -24,7 +24,6 @@ object InverseSymbolIndexer { // Reconstruct an m.Database from the symbol index and asserts that the // reconstructed database is identical to the original semanticdbs that // built the symbol index. - // TODO(olafur) handle local symbols when we stop indexing them. val db = mutable.Map.empty[String, m.Document] def get(uri: Uri) = { val key = if (uri.isFile) { @@ -37,7 +36,7 @@ object InverseSymbolIndexer { key, documents.getDocument(uri).fold("")(_.text) ), - "Scala212", + "Scala", Nil, Nil, Nil, @@ -51,6 +50,7 @@ object InverseSymbolIndexer { range: i.Range, role: SymbolOccurrence.Role ): Unit = { + if (symbol.startsWith("local")) return val doc = get(uri) val pos = doc.input.toPosition(range) val rs = diff --git a/metals/src/test/scala/tests/search/SymbolIndexTest.scala b/metals/src/test/scala/tests/search/SymbolIndexTest.scala index 8888af142e4..de14bb46442 100644 --- a/metals/src/test/scala/tests/search/SymbolIndexTest.scala +++ b/metals/src/test/scala/tests/search/SymbolIndexTest.scala @@ -114,6 +114,12 @@ object SymbolIndexTest extends MegaSuite with LazyLogging { )( expected: Location* ): Unit = { + def pretty(location: Location): String = + location.uri + ":" + + location.range.startLine + "L" + + location.range.startColumn + "C-" + + location.range.endLine + "L" + + location.range.endColumn + "C" val (symbol, _) = index .findSymbol(path.UserTestUri, line, column) .getOrElse( @@ -123,16 +129,13 @@ object SymbolIndexTest extends MegaSuite with LazyLogging { ) val dataList = index.referencesData(symbol) if (dataList.isEmpty) fail(s"References not found for term ${symbol}") - // TODO: use `dataList` to test expected alternatives val found = for { data <- dataList pos <- data.referencePositions(withDefinition) } yield pos.toLocation - - val missingLocations = found.toSet diff expected.toSet - assert(missingLocations.isEmpty) - val unexpectedLocations = expected.toSet diff found.toSet - assert(unexpectedLocations.isEmpty) + val expectedPretty = expected.map(pretty).sorted.mkString("\n") + val foundPretty = found.map(pretty).sorted.mkString("\n") + assertNoDiff(foundPretty, expectedPretty) } def ref( @@ -141,7 +144,7 @@ object SymbolIndexTest extends MegaSuite with LazyLogging { end: (Int, Int) ): Location = l.Location( - s"file:${path.toString}", + path.toURI.toString, Range( Position(start._1, start._2), l.Position(end._1, end._2) @@ -151,38 +154,39 @@ object SymbolIndexTest extends MegaSuite with LazyLogging { "definition" - { "<>(...)" - assertSymbolDefinition(path.UserReferenceLine, 17)( - "_root_.a.User.", - "_root_.a.User#" + "a.User.", + "a.User#" ) "User.<>(...)" - assertSymbolDefinition(3, 22)( - "_root_.a.User.apply(Ljava/lang/String;I)La/User;.", - "_root_.a.User#" + "a.User.apply(String,Int).", + "a.User#" ) "User.<>(...)" - assertSymbolDefinition(4, 9)( - "_root_.a.User#copy(Ljava/lang/String;I)La/User;.", - "_root_.a.User#" + "a.User#copy(String,Int).", + "a.User#" ) "User.apply(<> ...)" - assertSymbolDefinition(3, 28)( - "_root_.a.User.apply(Ljava/lang/String;I)La/User;.(name)", - "_root_.a.User#(name)" + "a.User.apply(String,Int).(name)", + "a.User#name()." ) "user.copy(<> = ...)" - assertSymbolDefinition(4, 14)( - "_root_.a.User#copy(Ljava/lang/String;I)La/User;.(age)", - "_root_.a.User#(age)" + "a.User#copy(String,Int).(age)", + "a.User#age()." ) } "classpath" - { "<>(...)" - // ScalaMtags - assertSymbolFound(5, 5)("_root_.scala.collection.immutable.List.") + assertSymbolFound(5, 5)("scala.collection.immutable.List.") "<>.create(...)" - // JavaMtags - assertSymbolFound(8, 19)("_root_.scala.runtime.CharRef.") + assertSymbolFound(8, 19)("scala.runtime.CharRef#") } + // Test failures here are impossible to understand, needs rewriting. "references" - { "<>(...)" - assertSymbolReferences(3, 17, withDefinition = true)( @@ -214,7 +218,7 @@ object SymbolIndexTest extends MegaSuite with LazyLogging { )(implicit path: utest.framework.TestPath): Unit = { while (s.tickOne()) () val result = metals.symbolIndex.workspaceSymbols(path.value.last) - val obtained = result.toIterator.map(_.name).mkString("\n") + val obtained = result.map(_.name).sorted.mkString("\n") assertNoDiff(obtained, expected.mkString("\n")) } "EmptyResult" - checkQuery() @@ -233,7 +237,12 @@ object SymbolIndexTest extends MegaSuite with LazyLogging { ) ) val slimDocuments = complete.documents.map { d => - d.copy(messages = Nil, synthetics = Nil, symbols = Nil) + d.copy( + messages = Nil, + synthetics = Nil, + symbols = Nil, + names = d.names.filterNot(_.symbol.isInstanceOf[Symbol.Local]) + ) } m.Database(slimDocuments) } @@ -265,8 +274,8 @@ object SymbolIndexTest extends MegaSuite with LazyLogging { val newUser = user.copy(value = "// leading comment\n" + user.value) metals.buffers.changed(newUser) assertSymbolDefinition(path.UserReferenceLine + 1, 17)( - "_root_.a.User.", - "_root_.a.User#" + "a.User.", + "a.User#" ) } } From 15b667ab51a26ee91bcfc2a0d8a2148c4627075e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 22:49:06 +0200 Subject: [PATCH 11/25] Fix remaining deprecation warnings --- .../languageserver/InputEnrichments.scala | 11 +++++------ .../scala/meta/metals/ScalametaEnrichments.scala | 16 +++++++++------- .../scala/scala/meta/metals/index/package.scala | 16 ---------------- .../meta/metals/search/InMemorySymbolIndex.scala | 3 ++- .../metals/search/InMemorySymbolIndexer.scala | 6 +++--- .../metals/search/InverseSymbolIndexer.scala | 5 +++-- .../scala/meta/metals/search/SymbolIndexer.scala | 4 +++- .../meta/metals/search/TokenEditDistance.scala | 7 ++++--- 8 files changed, 29 insertions(+), 39 deletions(-) delete mode 100644 metals/src/main/scala/scala/meta/metals/index/package.scala diff --git a/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala b/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala index 116f790e857..ea7c14cdfd5 100644 --- a/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala +++ b/metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala @@ -1,9 +1,8 @@ package org.langmeta.languageserver -import org.langmeta.lsp +import org.langmeta.{lsp => l} import org.langmeta.inputs.Input import org.langmeta.inputs.Position -import scala.meta.metals.{index => i} import scala.meta.internal.semanticdb3 object InputEnrichments { @@ -25,8 +24,8 @@ object InputEnrichments { } } implicit class XtensionInputOffset(val input: Input) extends AnyVal { - def toIndexRange(r: semanticdb3.Range): i.Range = { - i.Range( + def toIndexRange(r: semanticdb3.Range): l.Range = { + l.Range( startLine = r.startLine, startColumn = r.startCharacter, endLine = r.endLine, @@ -39,7 +38,7 @@ object InputEnrichments { Position.Range(input, offset, offset) /** Returns a scala.meta.Position from an index range. */ - def toPosition(range: lsp.Range): Position = { + def toPosition(range: l.Range): Position = { toPosition( range.start.line, range.start.character, @@ -48,7 +47,7 @@ object InputEnrichments { ) } - def toOffset(pos: lsp.Position): Int = + def toOffset(pos: l.Position): Int = toOffset(pos.line, pos.character) /** Returns an offset for this input */ diff --git a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala index 1fa3d79384c..e1f1a61f0e1 100644 --- a/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala +++ b/metals/src/main/scala/scala/meta/metals/ScalametaEnrichments.scala @@ -117,7 +117,7 @@ object ScalametaEnrichments { case _ => new String(input.chars) } } - implicit class XtensionIndexPosition(val pos: i.Position) extends AnyVal { + implicit class XtensionIndexPosition(val pos: l.Location) extends AnyVal { def pretty: String = s"${pos.uri.replaceFirst(".*/", "")} [${pos.range.pretty}]" @@ -128,7 +128,7 @@ object ScalametaEnrichments { ) } } - implicit class XtensionIndexRange(val range: i.Range) extends AnyVal { + implicit class XtensionIndexRange(val range: l.Range) extends AnyVal { def pretty: String = f"${range.startLine}%3d:${range.startColumn}%3d|${range.endLine}%3d:${range.endColumn}%3d" def toRange: l.Range = l.Range( @@ -163,7 +163,7 @@ object ScalametaEnrichments { endCharacter = pos.endColumn ) } - def toIndexRange: i.Range = i.Range( + def toIndexRange: l.Range = l.Range( startLine = pos.startLine, startColumn = pos.startColumn, endLine = pos.endLine, @@ -312,13 +312,13 @@ object ScalametaEnrichments { /** Returns reference positions for the given symbol index data * @param withDefinition if set to `true` will include symbol definition location */ - def referencePositions(withDefinition: Boolean): Set[i.Position] = { + def referencePositions(withDefinition: Boolean): Set[l.Location] = { val defPosition = if (withDefinition) data.definition else None val refPositions = for { (uri, ranges) <- data.references range <- ranges - } yield i.Position(uri.value, range) + } yield l.Location(uri.value, range) (defPosition.toSet ++ refPositions.toSet) .filterNot { _.uri.startsWith("jar:file") } // definition may refer to a jar @@ -410,6 +410,8 @@ object ScalametaEnrichments { if (has(VAL)) SymbolKind.Constant else if (has(VAR)) SymbolKind.Variable else SymbolKind.Method + case Kind.MACRO => + SymbolKind.Method case Kind.CONSTRUCTOR => SymbolKind.Constructor case Kind.FIELD => @@ -418,9 +420,9 @@ object ScalametaEnrichments { SymbolKind.Class // ??? case Kind.PARAMETER | Kind.SELF_PARAMETER | Kind.TYPE_PARAMETER => SymbolKind.Variable // ??? - case kind @ Kind.UNKNOWN_KIND => + case unknown @ (Kind.UNKNOWN_KIND | Kind.Unrecognized(_)) => throw new IllegalArgumentException( - s"Unsupported kind $kind in SymbolInformation: ${info.toProtoString}" + s"Unsupported kind $unknown in SymbolInformation: ${info.toProtoString}" ) } } diff --git a/metals/src/main/scala/scala/meta/metals/index/package.scala b/metals/src/main/scala/scala/meta/metals/index/package.scala deleted file mode 100644 index b9c069ad707..00000000000 --- a/metals/src/main/scala/scala/meta/metals/index/package.scala +++ /dev/null @@ -1,16 +0,0 @@ -package scala.meta.metals - -package object index { - - // TODO: remove these forwarders - @deprecated("Use org.langmeta.lsp.Range instead", "v0.1") - type Range = org.langmeta.lsp.Range - @deprecated("Use org.langmeta.lsp.Range instead", "v0.1") - val Range = org.langmeta.lsp.Range - - @deprecated("Use org.langmeta.lsp.Location instead", "v0.1") - type Position = org.langmeta.lsp.Location - @deprecated("Use org.langmeta.lsp.Location instead", "v0.1") - val Position = org.langmeta.lsp.Location - -} diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala index 108e5361034..c6cd550879b 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala @@ -24,6 +24,7 @@ import scala.meta.internal.semanticdb3 import scala.meta.internal.semanticdb3.SymbolOccurrence import scala.meta.internal.semanticdb3.TextDocument import scala.meta.internal.semanticdb3.TextDocuments +import org.langmeta.{lsp => l} import org.langmeta.io.AbsolutePath import org.langmeta.languageserver.InputEnrichments._ import org.langmeta.semanticdb.Symbol @@ -214,7 +215,7 @@ class InMemorySymbolIndex( ) => symbolIndexer.addDefinition( sym, - i.Position(document.uri, input.toIndexRange(r)) + l.Location(document.uri, input.toIndexRange(r)) ) case SymbolOccurrence( Some(r), diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala index b5df2fb4fcb..d1ba3418f31 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndexer.scala @@ -3,10 +3,10 @@ package scala.meta.metals.search import java.util.concurrent.atomic.AtomicReference import java.util.function.UnaryOperator import scala.collection.concurrent.TrieMap -import scala.meta.metals.index.Position -import scala.meta.metals.index.Range import scala.meta.metals.index.SymbolData import com.typesafe.scalalogging.LazyLogging +import org.langmeta.lsp.Location +import org.langmeta.lsp.Range import org.langmeta.semanticdb.Symbol import scala.meta.internal.semanticdb3 import scala.meta.metals.Uri @@ -43,7 +43,7 @@ class InMemorySymbolIndexer( override def addDefinition( symbol: String, - position: Position + position: Location ): Unit = updated(symbol) { index => // NOTE(olafur): Here we override the previous definition, in some cases, // we should accummulate them, for example non-pure JS/JVM/Native projects. diff --git a/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala index 3566a1ec8c2..a11ec02cb0d 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InverseSymbolIndexer.scala @@ -3,6 +3,7 @@ package scala.meta.metals.search import scala.collection.mutable import scala.meta.metals.Uri import scala.meta.metals.{index => i} +import org.langmeta.{lsp => l} import scala.{meta => m} import org.langmeta.io.AbsolutePath import org.langmeta.languageserver.InputEnrichments._ @@ -47,7 +48,7 @@ object InverseSymbolIndexer { def handleResolvedName( uri: Uri, symbol: String, - range: i.Range, + range: l.Range, role: SymbolOccurrence.Role ): Unit = { if (symbol.startsWith("local")) return @@ -60,7 +61,7 @@ object InverseSymbolIndexer { } symbols.foreach { symbol => symbol.definition.collect { - case i.Position(Uri(uri), range) => + case l.Location(Uri(uri), range) => handleResolvedName( uri, symbol.symbol, diff --git a/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala b/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala index 8b2d34dc6ec..8d39874beff 100644 --- a/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/search/SymbolIndexer.scala @@ -1,5 +1,7 @@ package scala.meta.metals.search +import org.langmeta.lsp.Location +import org.langmeta.lsp.Range import scala.meta.metals.index._ import org.langmeta.semanticdb.Symbol import scala.meta.internal.semanticdb3 @@ -48,7 +50,7 @@ trait SymbolIndexer { self => */ def addDefinition( symbol: String, - position: Position + position: Location ): Unit /** diff --git a/metals/src/main/scala/scala/meta/metals/search/TokenEditDistance.scala b/metals/src/main/scala/scala/meta/metals/search/TokenEditDistance.scala index 9c04159de12..c8434109941 100644 --- a/metals/src/main/scala/scala/meta/metals/search/TokenEditDistance.scala +++ b/metals/src/main/scala/scala/meta/metals/search/TokenEditDistance.scala @@ -3,6 +3,7 @@ package scala.meta.metals.search import scala.annotation.tailrec import scala.meta._ import scala.meta.metals.ScalametaEnrichments._ +import org.langmeta.{lsp => l} import scala.meta.metals.{index => i} import com.typesafe.scalalogging.LazyLogging import difflib._ @@ -57,7 +58,7 @@ final class TokenEditDistance private (matching: Array[MatchingToken]) { } private object RevisedRange { - def unapply(range: i.Range): Option[i.Range] = + def unapply(range: l.Range): Option[l.Range] = toRevised(range.startLine, range.startColumn) match { case Left(EmptyResult.NoMatch) => None case Left(EmptyResult.Unchanged) => Some(range) @@ -82,14 +83,14 @@ final class TokenEditDistance private (matching: Array[MatchingToken]) { /** Convert the definition position to match the revised input. */ def toRevisedDefinition(data: i.SymbolData): i.SymbolData = { data.definition match { - case Some(i.Position(uri, range)) if uri == ThisUri.value => + case Some(l.Location(uri, range)) if uri == ThisUri.value => toRevised(range.startLine, range.startColumn) match { case Left(EmptyResult.NoMatch) => data.copy(definition = None) case Left(EmptyResult.Unchanged) => data case Right(newPos) => val newData = data.copy( definition = Some( - i.Position( + l.Location( ThisUri.value, newPos.toIndexRange ) From 3f8c0b7f0c981aaa2c494a7f13ecfb4b02a1911d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 22:51:39 +0200 Subject: [PATCH 12/25] Address comment --- .../scala/scalafix/languageserver/ScalafixEnrichments.scala | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/metals/src/main/scala/scalafix/languageserver/ScalafixEnrichments.scala b/metals/src/main/scala/scalafix/languageserver/ScalafixEnrichments.scala index 8c8c121e8d4..59c13b3a826 100644 --- a/metals/src/main/scala/scalafix/languageserver/ScalafixEnrichments.scala +++ b/metals/src/main/scala/scalafix/languageserver/ScalafixEnrichments.scala @@ -42,8 +42,10 @@ object ScalafixEnrichments { patches: Map[RuleName, Patch], ctx: RuleCtx, index: SemanticdbIndex - ): List[LintMessage] = - Patch(patches, ctx, Some(index))._2 + ): List[LintMessage] = { + val (_, lintMessages) = Patch(patches, ctx, Some(index)) + lintMessages + } } implicit class XtensionRuleLSP(val rule: Rule) extends AnyVal { def fixWithNameInternal(ctx: RuleCtx): Map[RuleName, Patch] = From 63487bfc9cdc7f225beca5545c6314e6627f426a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Tue, 10 Apr 2018 23:14:07 +0200 Subject: [PATCH 13/25] Fix CI failures * Disable scalafix * Remove trailing comma * Reformat * Fix scripted tests * Remove unused scalapb build integration --- .travis.yml | 2 +- build.sbt | 7 ---- .../main/scala/org/langmeta/lsp/Types.scala | 2 +- metals/src/main/protobuf/metals.proto | 41 ------------------- .../main/scala/scala/meta/metals/Uri.scala | 2 +- .../metals/search/InMemoryDocumentIndex.scala | 3 +- project/plugins.sbt | 3 -- .../sbt-metals/semanticdb-scalac/test | 4 +- 8 files changed, 7 insertions(+), 57 deletions(-) delete mode 100644 metals/src/main/protobuf/metals.proto diff --git a/.travis.yml b/.travis.yml index 7266267f0f2..caf66216667 100644 --- a/.travis.yml +++ b/.travis.yml @@ -18,7 +18,7 @@ jobs: - sbt ++2.11.12 jsonrpc/test - env: TEST="sbt test" script: - - sbt startServer metalsSetup test scalafixTest + - sbt startServer metalsSetup test - env: TEST="sbt-metals" script: - ./bin/run-scripted.sh diff --git a/build.sbt b/build.sbt index beedfeb4ebf..dfe63a21010 100644 --- a/build.sbt +++ b/build.sbt @@ -12,7 +12,6 @@ inThisBuild( // https://github.com/scala/bug/issues/10448 "-Ywarn-unused-import" ), - scalafixEnabled := false, organization := "org.scalameta", licenses := Seq( "Apache-2.0" -> url("http://www.apache.org/licenses/LICENSE-2.0") @@ -127,11 +126,6 @@ lazy val metals = project .enablePlugins(BuildInfoPlugin) .disablePlugins(ScriptedPlugin) .settings( - PB.targets.in(Compile) := Seq( - scalapb.gen( - flatPackage = true // Don't append filename to package - ) -> sourceManaged.in(Compile).value./("protobuf") - ), fork in Test := true, // required for jni interrop with leveldb. buildInfoKeys := Seq[BuildInfoKey]( "testWorkspaceBaseDirectory" -> @@ -178,7 +172,6 @@ lazy val testWorkspace = project scalacOptions += "-Ywarn-unused-import", scalacOptions -= "-Xlint" ) - .disablePlugins(ScalafixPlugin) lazy val metalsRoot = project .in(file(".")) diff --git a/lsp4s/src/main/scala/org/langmeta/lsp/Types.scala b/lsp4s/src/main/scala/org/langmeta/lsp/Types.scala index 002d12c3951..05e9a87f83b 100644 --- a/lsp4s/src/main/scala/org/langmeta/lsp/Types.scala +++ b/lsp4s/src/main/scala/org/langmeta/lsp/Types.scala @@ -29,7 +29,7 @@ object Range { endColumn: Int ): Range = Range( start = Position(startLine, startColumn), - end = Position(endLine, endColumn), + end = Position(endLine, endColumn) ) } diff --git a/metals/src/main/protobuf/metals.proto b/metals/src/main/protobuf/metals.proto deleted file mode 100644 index 9230de20c50..00000000000 --- a/metals/src/main/protobuf/metals.proto +++ /dev/null @@ -1,41 +0,0 @@ -syntax = "proto3"; - -package scala.meta.metals.index; - -//// All of the metadata associated with a single symbol definition -//message SymbolData { -// string symbol = 1; -// // The Position where this symbol is defined. -// Position definition = 2; -// map references = 3; -// int32 kind = 4; -// string name = 5; -// string signature = 6; -// // Planned for Scalameta v2.2, see https://github.com/scalameta/scalameta/milestone/9 -// // string docstring = 7; // javadoc/scaladoc -// // string overrides = 8; // parent symbol that this symbol overrides -//} -// -//// A standalone position used by "Go to definition" when jumping to a symbol -//// definition. -//message Position { -// // can be a local file, entry inside local zip file, http url, or anything -// // else. -// string uri = 1; -// Range range = 2; -//} -// -//// A hack to workaround the fact that protobuf can't -//// encode `map` -//message Ranges { -// repeated Range ranges = 1; -//} -// -//// A slim range position, the filename is embedded in the -//// key of `references: map` -//message Range { -// int32 startLine = 1; -// int32 startColumn = 2; -// int32 endLine = 3; -// int32 endColumn = 4; -//} diff --git a/metals/src/main/scala/scala/meta/metals/Uri.scala b/metals/src/main/scala/scala/meta/metals/Uri.scala index 07837f91d4d..7a465a91020 100644 --- a/metals/src/main/scala/scala/meta/metals/Uri.scala +++ b/metals/src/main/scala/scala/meta/metals/Uri.scala @@ -17,7 +17,7 @@ import org.langmeta.io.AbsolutePath * - URI supports any scheme while we are only interested in a couple schemes * - Both file:///path and file:/path are valid URIs while we only use * file:///path in this project in order to support storing them as strings. - * For context, see https://github.com/scalameta/metals/pull/127#issuecomment-351880150 + * For context, see https://github.com/scalameta/metals/pull/127#issuecomment-351880150 */ sealed abstract case class Uri(value: String) { // Runtime check because wrapping constructor in Option[Uri] is too cumbersome diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala index c782a54727b..ab0b714ce39 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemoryDocumentIndex.scala @@ -23,7 +23,8 @@ class InMemoryDocumentIndex( class SymbolInformationsBySymbol(infos: util.HashMap[String, SymbolInformation]) extends Seq[SymbolInformation] { - def lookupSymbol(symbol: String): Option[SymbolInformation] = Option(infos.get(symbol)) + def lookupSymbol(symbol: String): Option[SymbolInformation] = + Option(infos.get(symbol)) import scala.collection.JavaConverters._ override def length: Int = infos.size() override def iterator: Iterator[SymbolInformation] = diff --git a/project/plugins.sbt b/project/plugins.sbt index 9fbcebd4049..a79b6cc1db4 100644 --- a/project/plugins.sbt +++ b/project/plugins.sbt @@ -1,10 +1,7 @@ addSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.3.2") -addSbtPlugin("ch.epfl.scala" % "sbt-scalafix" % "0.5.7") addSbtPlugin("ch.epfl.scala" % "sbt-release-early" % "2.0.0") addSbtPlugin("io.get-coursier" % "sbt-coursier" % "1.0.0-RC13") addSbtPlugin("com.eed3si9n" % "sbt-buildinfo" % "0.7.0") -addSbtPlugin("com.thesamet" % "sbt-protoc" % "0.99.18") -libraryDependencies += "com.thesamet.scalapb" %% "compilerplugin" % "0.7.1" unmanagedSourceDirectories in Compile += baseDirectory.value.getParentFile / "sbt-metals" / "src" / "main" / "scala" diff --git a/sbt-metals/src/sbt-test/sbt-metals/semanticdb-scalac/test b/sbt-metals/src/sbt-test/sbt-metals/semanticdb-scalac/test index fe814289cc5..344636d18eb 100644 --- a/sbt-metals/src/sbt-test/sbt-metals/semanticdb-scalac/test +++ b/sbt-metals/src/sbt-test/sbt-metals/semanticdb-scalac/test @@ -3,6 +3,6 @@ $ absent target/ # check that semanticdb-scalac is enabled > compile:compile -$ exists target/scala-2.12/classes/META-INF/semanticdb/src/main/scala/main.semanticdb +$ exists target/scala-2.12/classes/META-INF/semanticdb/src/main/scala/main.scala.semanticdb > test:compile -$ exists target/scala-2.12/test-classes/META-INF/semanticdb/src/test/scala/test.semanticdb +$ exists target/scala-2.12/test-classes/META-INF/semanticdb/src/test/scala/test.scala.semanticdb From 45abbe35c59603f7db7ce5e3a66e74c282ce2e82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Fri, 13 Apr 2018 21:02:05 +0200 Subject: [PATCH 14/25] Upgrade scalafix dependency --- build.sbt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.sbt b/build.sbt index dfe63a21010..efa84b320b9 100644 --- a/build.sbt +++ b/build.sbt @@ -78,7 +78,7 @@ lazy val V = new { val scala211 = MetalsPlugin.scala211 val scala212 = MetalsPlugin.scala212 val scalameta = MetalsPlugin.semanticdbVersion - val scalafix = "0.6.0-M1" + val scalafix = "0.6.0-M5" val enumeratum = "1.5.12" val circe = "0.9.0" val cats = "1.0.1" From df272a6b3bf804fabd9b71898e7e7f2beb6bb189 Mon Sep 17 00:00:00 2001 From: Alessandro Marrella Date: Sun, 15 Apr 2018 14:26:46 +0200 Subject: [PATCH 15/25] Handle method overloads in Scala mtags. --- .../meta/metals/mtags/MtagsIndexer.scala | 2 + .../scala/meta/metals/mtags/ScalaMtags.scala | 45 +++++++++++- .../scala/tests/mtags/ScalaMtagsTest.scala | 71 ++++++++++++++++--- 3 files changed, 106 insertions(+), 12 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala b/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala index 905dc8666be..3768395ee5a 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala @@ -39,6 +39,8 @@ trait MtagsIndexer { kind, properties ) + def method(name: Name, disambiguator: String, kind: Kind, properties: Int): Unit = + addSignature(Signature.Method(name.value, disambiguator), name.pos, kind, properties) def tpe(name: String, pos: m.Position, kind: Kind, properties: Int): Unit = addSignature(Signature.Type(name), pos, kind, properties) def tpe(name: Name, kind: Kind, properties: Int): Unit = diff --git a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala index f8818225e5d..965ba9310a1 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala @@ -22,6 +22,20 @@ object ScalaMtags { case _ => } } + def defTerm(name: Name, paramss: Seq[Seq[Term.Param]]) = { + if (paramss.isEmpty) + super.method(name, "()", Kind.METHOD, 0) + else + for { + params <- paramss + tpes = params.flatMap(_.decltpe) + names = tpes.map(getDisambiguator) + } withOwner() { + val ps = names.mkString("(", ",", ")") + super.method(name, ps, Kind.METHOD, 0) + } + stop() + } tree match { case _: Source => continue() case _: Template => continue() @@ -48,9 +62,9 @@ object ScalaMtags { case t: Decl.Type => tpe(t.name, Kind.TYPE, 0); stop() case t: Defn.Def => - term(t.name, Kind.METHOD, 0); stop() + defTerm(t.name, t.paramss) case t: Decl.Def => - term(t.name, Kind.METHOD, 0); stop() + defTerm(t.name, t.paramss) case t: Defn.Val => pats(t.pats, Kind.METHOD, Property.VAL.value); stop() case t: Decl.Val => @@ -64,4 +78,31 @@ object ScalaMtags { } } } + + private def getDisambiguator(t: Type): String = + t match { + case d: Type.Name => d.value + case d: Type.Select => d.name.value + case d: Type.Project => d.name.value + case d: Type.Singleton => "type" + case d: Type.Apply => getDisambiguator(d.tpe) + case d: Type.Existential => getDisambiguator(d.tpe) + case d: Type.Annotate => getDisambiguator(d.tpe) + case d: Type.ApplyInfix => getDisambiguator(d.op) + case d: Type.Lambda => getDisambiguator(d.tpe) + case d: Type.Method => + d.paramss.flatten + .flatMap(param => param.decltpe) + .map(getDisambiguator) + .mkString(",") + case d: Type.Function => d.params.map(getDisambiguator).mkString(",") + case d: Type.ImplicitFunction => + d.params.map(getDisambiguator).mkString(",") + case d: Type.Tuple => d.args.map(getDisambiguator).mkString(",") + case d: Type.ByName => s"=>${getDisambiguator(d.tpe)}" + case d: Type.Repeated => getDisambiguator(d.tpe) + "*" + case d: Type.With => "{}" + case d: Type.Refine => "{}" + case d => "?" + } } diff --git a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala index 009a4637578..95a3833f96e 100644 --- a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala +++ b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala @@ -24,29 +24,29 @@ object ScalaMtagsTest extends BaseMtagsTest { | |Names: |[22..23): D <= a.b.c.D. - |[33..34): e <= a.b.c.D.e. + |[33..34): e <= a.b.c.D.e(). |[61..62): f <= a.b.c.D.f. |[74..75): g <= a.b.c.D.g. |[89..90): H <= a.b.c.D.H# - |[97..98): x <= a.b.c.D.H#x. + |[97..98): x <= a.b.c.D.H#x(). |[114..115): I <= a.b.c.D.I# - |[127..128): x <= a.b.c.D.I#x. + |[127..128): x <= a.b.c.D.I#x(). |[143..144): y <= a.b.c.D.I#y. |[159..160): z <= a.b.c.D.I#z. |[181..182): J <= a.b.c.D.J. - |[189..190): k <= a.b.c.D.J.k. + |[189..190): k <= a.b.c.D.J.k(). | |Symbols: |a.b.c.D. => object D |a.b.c.D.H# => class H - |a.b.c.D.H#x. => method x + |a.b.c.D.H#x(). => method x |a.b.c.D.I# => trait I - |a.b.c.D.I#x. => method x + |a.b.c.D.I#x(). => method x |a.b.c.D.I#y. => val method y |a.b.c.D.I#z. => var method z |a.b.c.D.J. => object J - |a.b.c.D.J.k. => method k - |a.b.c.D.e. => method e + |a.b.c.D.J.k(). => method k + |a.b.c.D.e(). => method e |a.b.c.D.f. => val method f |a.b.c.D.g. => var method g """.stripMargin @@ -66,12 +66,12 @@ object ScalaMtagsTest extends BaseMtagsTest { |Names: |[16..17): K <= K. |[16..17): K <= K.package. - |[26..27): l <= K.package.l. + |[26..27): l <= K.package.l(). | |Symbols: |K. => packageobject K |K.package. => object package - |K.package.l. => method l + |K.package.l(). => method l """.stripMargin ) @@ -155,4 +155,55 @@ object ScalaMtagsTest extends BaseMtagsTest { |A#(b) => param b |""".stripMargin ) + + check( + "methods.scala", + """ + |abstract class Methods { + | def m1(a: Int, b: String): Int + | def m2(a: A[Int]): Unit + | def m3(a: A.type) + | def m4(a: b.A) + | def m5(a: => A) + | def m6(a: A*) + | def m7(a: A with B) + | def m8(a: {def b:B}) + | def m9(a: () => A) + | def m10(a: (A, B)) + | def m11() + |} + """.stripMargin, + """ + |Language: + |Scala + | + |Names: + |[16..23): Methods <= Methods# + |[32..34): m1 <= Methods#m1(Int,String). + |[65..67): m2 <= Methods#m2(A). + |[91..93): m3 <= Methods#m3(type). + |[111..113): m4 <= Methods#m4(A). + |[128..130): m5 <= Methods#m5(=>A). + |[146..148): m6 <= Methods#m6(A*). + |[162..164): m7 <= Methods#m7({}). + |[184..186): m8 <= Methods#m8({}). + |[207..209): m9 <= Methods#m9(). + |[228..231): m10 <= Methods#m10(A,B). + |[249..252): m11 <= Methods#m11(). + | + |Symbols: + |Methods# => class Methods + |Methods#m1(Int,String). => method m1 + |Methods#m10(A,B). => method m10 + |Methods#m11(). => method m11 + |Methods#m2(A). => method m2 + |Methods#m3(type). => method m3 + |Methods#m4(A). => method m4 + |Methods#m5(=>A). => method m5 + |Methods#m6(A*). => method m6 + |Methods#m7({}). => method m7 + |Methods#m8({}). => method m8 + |Methods#m9(). => method m9 + |""".stripMargin + ) } From bce8dd9094df94d6ba508c7dcad90fb129dd953f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sun, 15 Apr 2018 17:05:42 +0200 Subject: [PATCH 16/25] Handle conflicting method overlaods. --- .../scala/meta/metals/mtags/ScalaMtags.scala | 63 +++++++++++++------ .../scala/tests/mtags/ScalaMtagsTest.scala | 6 ++ 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala index 965ba9310a1..425feddc0df 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala @@ -2,6 +2,7 @@ package scala.meta.metals.mtags import scala.meta._ import org.langmeta.inputs.Input +import scala.collection.mutable import scala.meta.internal.semanticdb3.SymbolInformation.Kind import scala.meta.internal.semanticdb3.Language import scala.meta.internal.semanticdb3.SymbolInformation.Property @@ -22,23 +23,21 @@ object ScalaMtags { case _ => } } - def defTerm(name: Name, paramss: Seq[Seq[Term.Param]]) = { - if (paramss.isEmpty) - super.method(name, "()", Kind.METHOD, 0) - else - for { - params <- paramss - tpes = params.flatMap(_.decltpe) - names = tpes.map(getDisambiguator) - } withOwner() { - val ps = names.mkString("(", ",", ")") - super.method(name, ps, Kind.METHOD, 0) - } - stop() - } tree match { case _: Source => continue() - case _: Template => continue() + case t: Template => + // In case of conflicting disambiguators, append +N to N-th conflict. + // https://github.com/scalameta/scalameta/blob/master/semanticdb/semanticdb3/semanticdb3.md#symbol + val disambiguatorConflicts = + mutable.Map.empty[Signature.Method, Int] + t.stats.foreach { + case t: Defn.Def => + defTerm(disambiguatorConflicts, t.name, t.paramss) + case t: Decl.Def => + defTerm(disambiguatorConflicts, t.name, t.paramss) + case _ => + } + continue() case t: Pkg => pkg(t.ref); continue() case t: Pkg.Object => term(t.name, Kind.PACKAGE_OBJECT, 0); @@ -61,10 +60,6 @@ object ScalaMtags { tpe(t.name, Kind.TYPE, 0); stop() case t: Decl.Type => tpe(t.name, Kind.TYPE, 0); stop() - case t: Defn.Def => - defTerm(t.name, t.paramss) - case t: Decl.Def => - defTerm(t.name, t.paramss) case t: Defn.Val => pats(t.pats, Kind.METHOD, Property.VAL.value); stop() case t: Decl.Val => @@ -76,6 +71,36 @@ object ScalaMtags { case _ => stop() } } + def defTerm( + overloads: mutable.Map[Signature.Method, Int], + name: Name, + paramss: Seq[Seq[Term.Param]] + ): Unit = + withOwner() { + if (paramss.isEmpty) { + super.method(name, "()", Kind.METHOD, 0) + } else { + for { + params <- paramss + tpes = params.flatMap(_.decltpe) + names = tpes.map(getDisambiguator) + } { + val disambiguator = names.mkString(",") + val signature = Signature.Method(name.value, disambiguator) + val counter = overloads.getOrElseUpdate(signature, 0) + overloads(signature) = counter + 1 + val finalDisambiguator: String = + if (counter == 0) disambiguator + else disambiguator + "+" + counter + super.method( + name, + "(" + finalDisambiguator + ")", + Kind.METHOD, + 0 + ) + } + } + } } } diff --git a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala index 95a3833f96e..6b73633260f 100644 --- a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala +++ b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala @@ -171,6 +171,8 @@ object ScalaMtagsTest extends BaseMtagsTest { | def m9(a: () => A) | def m10(a: (A, B)) | def m11() + | def m12(a: b.A) + | def m12(a: c.A) |} """.stripMargin, """ @@ -190,12 +192,16 @@ object ScalaMtagsTest extends BaseMtagsTest { |[207..209): m9 <= Methods#m9(). |[228..231): m10 <= Methods#m10(A,B). |[249..252): m11 <= Methods#m11(). + |[261..264): m12 <= Methods#m12(A). + |[279..282): m12 <= Methods#m12(A+1). | |Symbols: |Methods# => class Methods |Methods#m1(Int,String). => method m1 |Methods#m10(A,B). => method m10 |Methods#m11(). => method m11 + |Methods#m12(A). => method m12 + |Methods#m12(A+1). => method m12 |Methods#m2(A). => method m2 |Methods#m3(type). => method m3 |Methods#m4(A). => method m4 From ed9bb9dd66322b655918d4ab41dba91e494db324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sun, 15 Apr 2018 17:08:28 +0200 Subject: [PATCH 17/25] Emit method symbols for val/var to be compliant with spec --- .../meta/metals/mtags/MtagsIndexer.scala | 14 +++++- .../scala/meta/metals/mtags/ScalaMtags.scala | 5 +- .../scala/tests/mtags/ScalaMtagsTest.scala | 48 +++++++++---------- 3 files changed, 40 insertions(+), 27 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala b/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala index 3768395ee5a..05248d5369a 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala @@ -39,8 +39,18 @@ trait MtagsIndexer { kind, properties ) - def method(name: Name, disambiguator: String, kind: Kind, properties: Int): Unit = - addSignature(Signature.Method(name.value, disambiguator), name.pos, kind, properties) + def method( + name: Name, + disambiguator: String, + kind: Kind, + properties: Int + ): Unit = + addSignature( + Signature.Method(name.value, disambiguator), + name.pos, + kind, + properties + ) def tpe(name: String, pos: m.Position, kind: Kind, properties: Int): Unit = addSignature(Signature.Type(name), pos, kind, properties) def tpe(name: Name, kind: Kind, properties: Int): Unit = diff --git a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala index 425feddc0df..0f369bac97d 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala @@ -19,7 +19,10 @@ object ScalaMtags { def stop(): Unit = () def pats(ps: List[Pat], kind: Kind, properties: Int): Unit = { ps.foreach { - case Pat.Var(name) => withOwner() { term(name, kind, properties) } + case Pat.Var(name) => + withOwner() { + method(name, "()", kind, properties) + } case _ => } } diff --git a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala index 6b73633260f..b100c110c30 100644 --- a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala +++ b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala @@ -25,14 +25,14 @@ object ScalaMtagsTest extends BaseMtagsTest { |Names: |[22..23): D <= a.b.c.D. |[33..34): e <= a.b.c.D.e(). - |[61..62): f <= a.b.c.D.f. - |[74..75): g <= a.b.c.D.g. + |[61..62): f <= a.b.c.D.f(). + |[74..75): g <= a.b.c.D.g(). |[89..90): H <= a.b.c.D.H# |[97..98): x <= a.b.c.D.H#x(). |[114..115): I <= a.b.c.D.I# |[127..128): x <= a.b.c.D.I#x(). - |[143..144): y <= a.b.c.D.I#y. - |[159..160): z <= a.b.c.D.I#z. + |[143..144): y <= a.b.c.D.I#y(). + |[159..160): z <= a.b.c.D.I#z(). |[181..182): J <= a.b.c.D.J. |[189..190): k <= a.b.c.D.J.k(). | @@ -42,13 +42,13 @@ object ScalaMtagsTest extends BaseMtagsTest { |a.b.c.D.H#x(). => method x |a.b.c.D.I# => trait I |a.b.c.D.I#x(). => method x - |a.b.c.D.I#y. => val method y - |a.b.c.D.I#z. => var method z + |a.b.c.D.I#y(). => val method y + |a.b.c.D.I#z(). => var method z |a.b.c.D.J. => object J |a.b.c.D.J.k(). => method k |a.b.c.D.e(). => method e - |a.b.c.D.f. => val method f - |a.b.c.D.g. => var method g + |a.b.c.D.f(). => val method f + |a.b.c.D.g(). => var method g """.stripMargin ) @@ -91,25 +91,25 @@ object ScalaMtagsTest extends BaseMtagsTest { | |Names: |[8..12): pats <= pats. - |[21..22): o <= pats.o. - |[24..25): p <= pats.p. - |[36..37): q <= pats.q. - |[39..40): r <= pats.r. - |[52..53): s <= pats.s. - |[55..56): t <= pats.t. - |[67..68): v <= pats.v. - |[70..71): w <= pats.w. + |[21..22): o <= pats.o(). + |[24..25): p <= pats.p(). + |[36..37): q <= pats.q(). + |[39..40): r <= pats.r(). + |[52..53): s <= pats.s(). + |[55..56): t <= pats.t(). + |[67..68): v <= pats.v(). + |[70..71): w <= pats.w(). | |Symbols: |pats. => object pats - |pats.o. => val method o - |pats.p. => val method p - |pats.q. => val method q - |pats.r. => val method r - |pats.s. => var method s - |pats.t. => var method t - |pats.v. => var method v - |pats.w. => var method w + |pats.o(). => val method o + |pats.p(). => val method p + |pats.q(). => val method q + |pats.r(). => val method r + |pats.s(). => var method s + |pats.t(). => var method t + |pats.v(). => var method v + |pats.w(). => var method w """.stripMargin ) From 3fd72628454d8ec272b73f13264b93fb9abdd580 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sun, 15 Apr 2018 17:10:14 +0200 Subject: [PATCH 18/25] Emit correct disambiguator for singleton type --- .../src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala | 2 +- metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala index 0f369bac97d..029ed2c12a6 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala @@ -112,7 +112,7 @@ object ScalaMtags { case d: Type.Name => d.value case d: Type.Select => d.name.value case d: Type.Project => d.name.value - case d: Type.Singleton => "type" + case d: Type.Singleton => ".type" case d: Type.Apply => getDisambiguator(d.tpe) case d: Type.Existential => getDisambiguator(d.tpe) case d: Type.Annotate => getDisambiguator(d.tpe) diff --git a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala index b100c110c30..b405a498cae 100644 --- a/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala +++ b/metals/src/test/scala/tests/mtags/ScalaMtagsTest.scala @@ -183,7 +183,7 @@ object ScalaMtagsTest extends BaseMtagsTest { |[16..23): Methods <= Methods# |[32..34): m1 <= Methods#m1(Int,String). |[65..67): m2 <= Methods#m2(A). - |[91..93): m3 <= Methods#m3(type). + |[91..93): m3 <= Methods#m3(.type). |[111..113): m4 <= Methods#m4(A). |[128..130): m5 <= Methods#m5(=>A). |[146..148): m6 <= Methods#m6(A*). @@ -203,7 +203,7 @@ object ScalaMtagsTest extends BaseMtagsTest { |Methods#m12(A). => method m12 |Methods#m12(A+1). => method m12 |Methods#m2(A). => method m2 - |Methods#m3(type). => method m3 + |Methods#m3(.type). => method m3 |Methods#m4(A). => method m4 |Methods#m5(=>A). => method m5 |Methods#m6(A*). => method m6 From 7a3e0bfbef7fb72df4093633ad8937acb7a0f96c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 21 Apr 2018 00:46:13 +0200 Subject: [PATCH 19/25] Use scalafix PrettyType for HoverProvider - setup metacp integration - setup symbol table integration - adjust hover expected output --- .../scala/meta/metals/MetalsServices.scala | 13 +- .../main/scala/scala/meta/metals/Uri.scala | 7 +- .../meta/metals/compiler/CompilerConfig.scala | 11 ++ .../meta/metals/compiler/MetacpProvider.scala | 114 ++++++++++++++++++ .../meta/metals/compiler/ScalacProvider.scala | 8 +- .../meta/metals/compiler/SymtabProvider.scala | 35 ++++++ .../meta/metals/providers/HoverProvider.scala | 84 ++++++++++++- .../metals/search/InMemorySymbolIndex.scala | 2 +- .../src/test/scala/tests/CompilerSuite.scala | 7 +- .../provider/BaseHoverProviderTest.scala | 15 ++- .../tests/provider/HoverProviderTest.scala | 14 ++- .../scala/tests/search/BaseIndexTest.scala | 36 ++++-- 12 files changed, 316 insertions(+), 30 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala create mode 100644 metals/src/main/scala/scala/meta/metals/compiler/SymtabProvider.scala diff --git a/metals/src/main/scala/scala/meta/metals/MetalsServices.scala b/metals/src/main/scala/scala/meta/metals/MetalsServices.scala index efbfc87d2a6..70cefb6ed33 100644 --- a/metals/src/main/scala/scala/meta/metals/MetalsServices.scala +++ b/metals/src/main/scala/scala/meta/metals/MetalsServices.scala @@ -50,6 +50,8 @@ import org.langmeta.io.AbsolutePath import org.langmeta.languageserver.InputEnrichments._ import org.langmeta.lsp.LanguageClient import org.langmeta.internal.semanticdb._ +import scala.meta.metals.compiler.MetacpProvider +import scala.meta.metals.compiler.SymtabProvider class MetalsServices( cwd: AbsolutePath, @@ -87,6 +89,13 @@ class MetalsServices( val diagnosticsProvider = new DiagnosticsProvider(configurationPublisher, cwd) val scalacProvider = new ScalacProvider + val metacpProvider = new MetacpProvider + val symtabProvider = + new SymtabProvider( + symbolIndex.documentIndex, + scalacProvider, + metacpProvider + ) val interactiveSemanticdbs: Observable[m.Database] = sourceChangePublisher .debounce(FiniteDuration(1, "s")) @@ -317,6 +326,7 @@ class MetalsServices( if (latestConfig().hover.enabled) { HoverProvider.hover( symbolIndex, + symtabProvider, Uri(params.textDocument), params.position.line, params.position.character @@ -526,10 +536,11 @@ class MetalsServices( object MetalsServices extends LazyLogging { lazy val cacheDirectory: AbsolutePath = { + val cacheVersion = "0.1" val path = AbsolutePath( ProjectDirectories.fromProjectName("metals").projectCacheDir ) - Files.createDirectories(path.toNIO) + Files.createDirectories(path.resolve(cacheVersion).toNIO) path } diff --git a/metals/src/main/scala/scala/meta/metals/Uri.scala b/metals/src/main/scala/scala/meta/metals/Uri.scala index 7a465a91020..1b3ba6471bd 100644 --- a/metals/src/main/scala/scala/meta/metals/Uri.scala +++ b/metals/src/main/scala/scala/meta/metals/Uri.scala @@ -44,11 +44,6 @@ object Uri { def apply(td: VersionedTextDocumentIdentifier): Uri = Uri(td.uri) def apply(path: AbsolutePath): Uri = Uri(path.toURI) def apply(uri: URI): Uri = - if (uri.getScheme == "file") { - // nio.Path.toUri.toString produces file:/// while LSP expected file:/ - new Uri(s"file://${uri.getPath}") {} - } else { - new Uri(uri.toString) {} - } + new Uri(uri.toString) {} def unapply(arg: String): Option[Uri] = Some(Uri(URI.create(arg))) } diff --git a/metals/src/main/scala/scala/meta/metals/compiler/CompilerConfig.scala b/metals/src/main/scala/scala/meta/metals/compiler/CompilerConfig.scala index 7acb06c1863..a285d0d22a7 100644 --- a/metals/src/main/scala/scala/meta/metals/compiler/CompilerConfig.scala +++ b/metals/src/main/scala/scala/meta/metals/compiler/CompilerConfig.scala @@ -60,6 +60,17 @@ case class CompilerConfig( object CompilerConfig extends LazyLogging { private val relativeDir: RelativePath = RelativePath(".metals").resolve("buildinfo") + val empty = CompilerConfig( + sources = Nil, + unmanagedSourceDirectories = Nil, + managedSourceDirectories = Nil, + scalacOptions = Nil, + classDirectory = AbsolutePath.workingDirectory, + dependencyClasspath = Nil, + sourceJars = Nil, + origin = PathIO.workingDirectory, + scalaVersion = ScalaVersion("2.12.4").asInstanceOf[SpecificScalaVersion] + ) def dir(cwd: AbsolutePath): AbsolutePath = cwd.resolve(relativeDir) diff --git a/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala b/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala new file mode 100644 index 00000000000..3d708a1937c --- /dev/null +++ b/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala @@ -0,0 +1,114 @@ +package scala.meta.metals.compiler + +import java.io.File +import java.io.OutputStream +import java.io.PrintStream +import java.nio.file.Files +import java.util +import java.util.Collections +import java.util.function +import scala.meta.AbsolutePath +import scala.meta.Classpath +import scala.meta.cli.Metacp +import scala.meta.metacp +import scala.meta.metacp.Reporter +import scala.meta.metacp.Settings +import scalafix.internal.util.LazySymbolTable +import scalafix.internal.util.SymbolTable + +class MetacpProvider { + private val classpaths = + Collections.synchronizedMap(new util.HashMap[AbsolutePath, AbsolutePath]()) + private val settings = + Settings().withPar(false).withScalaLibrarySynthetics(false) + private val reporter = Reporter() + private val synthetics = Metacp + .process(Settings().withScalaLibrarySynthetics(true), reporter) + .get + .shallow + .head + private val empty = AbsolutePath(Files.createTempDirectory("metals")) + private val processEntry = new function.Function[AbsolutePath, AbsolutePath] { + override def apply(t: AbsolutePath): AbsolutePath = { + val result = Metacp.process( + settings.withClasspath(Classpath(t :: Nil)), + reporter + ) + result match { + case Some(Classpath(entry :: Nil)) => + entry + case _ => + empty + } + } + } + + def process(classpath: Classpath): Classpath = { + Classpath(synthetics :: classpath.shallow.map { entry => + if (entry.isDirectory) entry + else classpaths.computeIfAbsent(entry, processEntry) + }) + } + +} + +object ClasspathOps { + + def bootClasspath: Option[Classpath] = sys.props.collectFirst { + case (k, v) if k.endsWith(".boot.class.path") => Classpath(v) + } + + val devNull = new PrintStream(new OutputStream { + override def write(b: Int): Unit = () + }) + + /** Process classpath with metacp to build semanticdbs of global symbols. **/ + def toMetaClasspath( + sclasspath: Classpath, + cacheDirectory: Option[AbsolutePath] = None, + parallel: Boolean = true, + out: PrintStream = devNull + ): Option[Classpath] = { + val (processed, toProcess) = sclasspath.shallow.partition { path => + path.isDirectory && + path.resolve("META-INF").resolve("semanticdb.semanticidx").isFile + } + val withJDK = Classpath( + bootClasspath.fold(sclasspath.shallow)(_.shallow ::: toProcess) + ) + val default = metacp.Settings() + val settings = default + .withClasspath(withJDK) + .withScalaLibrarySynthetics(true) + .withCacheDir(cacheDirectory.getOrElse(default.cacheDir)) + .withPar(parallel) + val reporter = metacp + .Reporter() + .withOut(devNull) // out prints classpath of proccessed classpath, which is not relevant for scalafix. + .withErr(out) + val mclasspath = scala.meta.cli.Metacp.process(settings, reporter) + mclasspath.map(x => Classpath(x.shallow ++ processed)) + } + + def newSymbolTable( + classpath: Classpath, + cacheDirectory: Option[AbsolutePath] = None, + parallel: Boolean = true, + out: PrintStream = System.out + ): Option[SymbolTable] = { + toMetaClasspath(classpath, cacheDirectory, parallel, out) + .map(new LazySymbolTable(_)) + } + + def getCurrentClasspath: Classpath = { + Thread.currentThread.getContextClassLoader match { + case url: java.net.URLClassLoader => + val classpath = url.getURLs.map(_.getFile).mkString(File.pathSeparator) + Classpath(classpath) + case els => + throw new IllegalStateException( + s"Expected java.net.URLClassLoader, obtained $els" + ) + } + } +} diff --git a/metals/src/main/scala/scala/meta/metals/compiler/ScalacProvider.scala b/metals/src/main/scala/scala/meta/metals/compiler/ScalacProvider.scala index c7ea30a0228..de48ccb6367 100644 --- a/metals/src/main/scala/scala/meta/metals/compiler/ScalacProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/compiler/ScalacProvider.scala @@ -35,26 +35,28 @@ class ScalacProvider()(implicit client: JsonRpcClient) extends LazyLogging { // Looking up by sourceDirectory alone is not enough since // in sbt it's possible to `sources += file("blah")`, which would // not be respected if we only went by directories. - .orElse(compilerBySourceDirectory(uri)) + .orElse(compilerBySourceDirectory(uri).map(_._2)) .map { compiler => compiler.reporter.reset() compiler } } - def compilerBySourceDirectory(uri: Uri): Option[Global] = { + def compilerBySourceDirectory(uri: Uri): Option[(CompilerConfig, Global)] = { val path = uri.toAbsolutePath.toNIO compilerByConfigOrigin.values.collectFirst { case (config, global) if config.sourceDirectories.exists( dir => path.startsWith(dir.toNIO) ) => - global + config -> global } } private val compilerByConfigOrigin = mutable.Map.empty[AbsolutePath, (CompilerConfig, Global)] private val compilerByPath = mutable.Map.empty[Uri, Global] + def configForUri(uri: Uri): Option[CompilerConfig] = + compilerBySourceDirectory(uri).map(_._1) def allCompilerConfigs: Iterable[CompilerConfig] = compilerByConfigOrigin.values.map(_._1) def loadNewCompilerGlobals( diff --git a/metals/src/main/scala/scala/meta/metals/compiler/SymtabProvider.scala b/metals/src/main/scala/scala/meta/metals/compiler/SymtabProvider.scala new file mode 100644 index 00000000000..2b3dc8a5262 --- /dev/null +++ b/metals/src/main/scala/scala/meta/metals/compiler/SymtabProvider.scala @@ -0,0 +1,35 @@ +package scala.meta.metals.compiler + +import org.langmeta.io.Classpath +import scala.meta.internal.semanticdb3.SymbolInformation +import scala.meta.metals.Uri +import scala.meta.metals.search.DocumentIndex +import scalafix.internal.util.LazySymbolTable +import scalafix.internal.util.SymbolTable + +class SymtabProvider( + docs: DocumentIndex, + scalac: ScalacProvider, + metacp: MetacpProvider +) { + def symtab(uri: Uri): SymbolTable = { + val classpath = scalac.configForUri(uri) match { + case Some(config) => + val unprocessed = + Classpath(config.classDirectory :: config.dependencyClasspath) + metacp.process(unprocessed) + case _ => + Classpath(Nil) + } + val globalSymtab = new LazySymbolTable(classpath) + new SymbolTable { + override def info(symbol: String): Option[SymbolInformation] = + globalSymtab.info(symbol).orElse { + for { + doc <- docs.getDocument(uri) + info <- doc.symbols.find(_.symbol == symbol) + } yield info + } + } + } +} diff --git a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala index 685f0c2474e..d0ae98888b5 100644 --- a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala @@ -5,26 +5,104 @@ import org.langmeta.lsp.Hover import org.langmeta.lsp.RawMarkedString import scala.meta.metals.search.SymbolIndex import scala.{meta => m} +import scala.meta.internal.semanticdb3.SymbolInformation.{Property => p} import com.typesafe.scalalogging.LazyLogging +import scala.meta.Tree +import scala.meta._ +import scala.meta.metals.compiler.SymtabProvider +import scala.util.control.NonFatal +import scalafix.internal.util.PrettyType +import scalafix.internal.util.QualifyStrategy +import scala.meta.internal.{semanticdb3 => s} +import scalafix.internal.util.SymbolTable +import scala.meta.internal.semanticdb3.Scala._ object HoverProvider extends LazyLogging { def empty: Hover = Hover(Nil, None) val Template = m.Template(Nil, Nil, m.Self(m.Name.Anonymous(), None), Nil) + private def isSymbolicName(name: String): Boolean = + !Character.isJavaIdentifierStart(name.head) + val EmptyTemplate = template"{}" + val EmptyCtor = q"def this" + + private def toTree( + info: s.SymbolInformation, + symtab: SymbolTable + ): Tree = { + def mods = { + // TODO: Upstream FINAL OBJECT fix to scalafix, + // this method is copy pasted from PrettyType + def is(property: s.SymbolInformation.Property): Boolean = + (info.properties & property.value) != 0 + val buf = List.newBuilder[Mod] + info.accessibility.foreach { accessibility => + // TODO: private[within] + if (accessibility.tag.isPrivate) buf += Mod.Private(Name.Anonymous()) + if (accessibility.tag.isProtected) + buf += Mod.Protected(Name.Anonymous()) + } + if (is(p.SEALED)) buf += Mod.Sealed() + if (info.kind.isClass && is(p.ABSTRACT)) buf += Mod.Abstract() + if (!info.kind.isObject && is(p.FINAL)) buf += Mod.Final() + if (is(p.IMPLICIT)) buf += Mod.Implicit() + if (info.kind.isClass && is(p.CASE)) buf += Mod.Case() + buf.result() + } + if (info.kind.isObject) { + Defn.Object(mods, Term.Name(info.name), EmptyTemplate) + } else if (info.kind.isPackageObject) { + Pkg.Object(mods, Term.Name(info.owner.desc.name), EmptyTemplate) + } else if (info.kind.isPackage) { + val ref = + info.symbol.stripSuffix(".").parse[Term].get.asInstanceOf[Term.Ref] + Pkg(ref, Nil) + } else if (info.kind.isTrait) { + Defn.Trait(mods, Type.Name(info.name), Nil, EmptyCtor, EmptyTemplate) + } else if (info.kind.isClass) { + Defn.Class(mods, Type.Name(info.name), Nil, EmptyCtor, EmptyTemplate) + } else if (info.kind.isLocal || info.symbol.startsWith("local")) { + PrettyType.toType(info.tpe.get, symtab, QualifyStrategy.Readable).tree + } else { + PrettyType.toTree(info, symtab, QualifyStrategy.Readable).tree + } + } + def hover( index: SymbolIndex, + symtabs: SymtabProvider, uri: Uri, line: Int, column: Int ): Hover = { val result = for { (symbol, _) <- index.findSymbol(uri, line, column) + symtab = symtabs.symtab(uri) + info <- symtab.info(symbol.syntax) + tree <- { + try { + Some(toTree(info, symtab)) + } catch { + case NonFatal(e) => + logger.error( + s"""Failed to pretty print symbol $symbol + |Classpath=$symtab + |${info.toProtoString} + |""".stripMargin, + e + ) + None + } + } } yield { - // TODO: pretty-print SymbolInformation.tpe, blocked by https://github.com/scalameta/scalameta/issues/1479 - val prettyTpe = symbol.syntax + val pretty = tree.transform { + case Type.Apply(op: Type.Name, lhs :: rhs :: Nil) + if isSymbolicName(op.value) => + Type.ApplyInfix(lhs, op, rhs) + } Hover( - contents = RawMarkedString(language = "scala", value = prettyTpe) :: Nil, + contents = RawMarkedString(language = "scala", value = pretty.syntax) :: Nil, range = None ) } diff --git a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala index c6cd550879b..59b0f640b56 100644 --- a/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala +++ b/metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala @@ -193,7 +193,7 @@ class InMemorySymbolIndex( } val documentWithOnlyLocalSymbols = document.copy(symbols = new SymbolInformationsBySymbol(locals)) - documentIndex.putDocument(uri, documentWithOnlyLocalSymbols) + documentIndex.putDocument(uri, document) document.occurrences.foreach { occurence => val isGlobal = locals.get(occurence.symbol) == null && diff --git a/metals/src/test/scala/tests/CompilerSuite.scala b/metals/src/test/scala/tests/CompilerSuite.scala index 595e1c377b9..46605de4043 100644 --- a/metals/src/test/scala/tests/CompilerSuite.scala +++ b/metals/src/test/scala/tests/CompilerSuite.scala @@ -2,6 +2,8 @@ package tests import org.langmeta.semanticdb.Document +import java.nio.file.Files +import org.langmeta.io.AbsolutePath import scala.meta.interactive.InteractiveSemanticdb import scala.meta.metals.Uri import scala.meta.metals.compiler.{Cursor, ScalacProvider} @@ -15,6 +17,8 @@ class CompilerSuite extends MegaSuite { Nil ) + val sourceDirectory = AbsolutePath(Files.createTempDirectory("metals")) + def toDocument(name: String, code: String): Document = { InteractiveSemanticdb.toDocument(compiler, code, name, 10000) } @@ -32,7 +36,8 @@ class CompilerSuite extends MegaSuite { carets match { case (start, end) :: Nil => val code = chevrons.replaceAllIn(markup, "$1") - Cursor(Uri.file(filename), code, start) + val uri = Uri(sourceDirectory.resolve(filename)) + Cursor(uri, code, start) case els => throw new IllegalArgumentException( s"Expected one chevron, found ${els.length}" diff --git a/metals/src/test/scala/tests/provider/BaseHoverProviderTest.scala b/metals/src/test/scala/tests/provider/BaseHoverProviderTest.scala index f3f4da8bf84..34d65108970 100644 --- a/metals/src/test/scala/tests/provider/BaseHoverProviderTest.scala +++ b/metals/src/test/scala/tests/provider/BaseHoverProviderTest.scala @@ -7,6 +7,10 @@ import tests.search.BaseIndexTest import scala.meta.metals.Uri import scala.meta.metals.providers.HoverProvider import scala.{meta => m} +import tests.search.BaseIndexTest +import io.circe.syntax._ +import java.nio.file.Files +import org.langmeta.io.AbsolutePath abstract class BaseHoverProviderTest extends BaseIndexTest { @@ -18,12 +22,17 @@ abstract class BaseHoverProviderTest extends BaseIndexTest { targeted( filename, code, { point => - val uri = Uri.file(filename) + val uri = Uri(sourceDirectory.resolve(filename)) val input = m.Input.VirtualFile(uri.value, point.contents) val pos = m.Position.Range(input, point.offset, point.offset) indexInput(uri, point.contents) - val result = - HoverProvider.hover(symbols, uri, pos.startLine, pos.startColumn) + val result = HoverProvider.hover( + symbols, + symtabs, + uri, + pos.startLine, + pos.startColumn + ) f(result) } ) diff --git a/metals/src/test/scala/tests/provider/HoverProviderTest.scala b/metals/src/test/scala/tests/provider/HoverProviderTest.scala index 56b3bb24119..5cacd8333bc 100644 --- a/metals/src/test/scala/tests/provider/HoverProviderTest.scala +++ b/metals/src/test/scala/tests/provider/HoverProviderTest.scala @@ -214,7 +214,7 @@ object HoverProviderTest extends BaseHoverProviderTest { | List(1) match { case <> :: Nil => } |} """.stripMargin, - "val x: Int" + "Int" ) check( @@ -224,7 +224,7 @@ object HoverProviderTest extends BaseHoverProviderTest { | List(1) match { case List(<>) => } |} """.stripMargin, - "val x: Int" + "Int" ) check( @@ -297,6 +297,14 @@ object HoverProviderTest extends BaseHoverProviderTest { | type <>[T] <: Any |} """.stripMargin, - "type F" + "type F[T]" + ) + + check( + "mods", + """ + |sealed trait <>[T] + """.stripMargin, + "sealed trait W" ) } diff --git a/metals/src/test/scala/tests/search/BaseIndexTest.scala b/metals/src/test/scala/tests/search/BaseIndexTest.scala index 79809f53f1d..82db03872af 100644 --- a/metals/src/test/scala/tests/search/BaseIndexTest.scala +++ b/metals/src/test/scala/tests/search/BaseIndexTest.scala @@ -1,19 +1,23 @@ package tests.search +import monix.execution.Scheduler.Implicits.global +import monix.reactive.Observable +import org.langmeta.internal.io.PathIO +import org.langmeta.internal.semanticdb._ +import org.langmeta.jsonrpc.JsonRpcClient import scala.meta.interactive.InteractiveSemanticdb import scala.meta.internal.inputs._ import scala.meta.metals.Buffers import scala.meta.metals.Configuration import scala.meta.metals.Effects import scala.meta.metals.Uri -import org.langmeta.jsonrpc.JsonRpcClient - +import scala.meta.metals.compiler.ClasspathOps +import scala.meta.metals.compiler.CompilerConfig +import scala.meta.metals.compiler.MetacpProvider +import scala.meta.metals.compiler.ScalacProvider +import scala.meta.metals.compiler.SymtabProvider import scala.meta.metals.search.SymbolIndex import scala.{meta => m} -import monix.execution.Scheduler.Implicits.global -import monix.reactive.Observable -import org.langmeta.internal.io.PathIO -import org.langmeta.internal.semanticdb._ import tests.CompilerSuite abstract class BaseIndexTest extends CompilerSuite { @@ -24,10 +28,24 @@ abstract class BaseIndexTest extends CompilerSuite { buffers, Observable.now(Configuration()) ) - def indexDocument(document: m.Document): Effects.IndexSemanticdb = - symbols.indexDatabase( - m.Database(document :: Nil).toSchema(PathIO.workingDirectory) + + val thisClasspath = ClasspathOps.getCurrentClasspath + val scalac = new ScalacProvider() + scalac.loadNewCompilerGlobals( + CompilerConfig.empty.copy( + dependencyClasspath = thisClasspath.shallow, + managedSourceDirectories = sourceDirectory :: Nil ) + ) + val symtabs = new SymtabProvider( + symbols.documentIndex, + scalac, + new MetacpProvider + ) + def indexDocument(document: m.Document): Effects.IndexSemanticdb = { + val sdoc = m.Database(document :: Nil).toSchema(PathIO.workingDirectory) + symbols.indexDatabase(sdoc) + } def indexInput(uri: Uri, code: String): m.Document = { buffers.changed(m.Input.VirtualFile(uri.value, code)) val document = From 525039cda1a88210f88d04d0037b3e003811be58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 21 Apr 2018 01:43:08 +0200 Subject: [PATCH 20/25] Fix scalafix integration. The latest release exposes a better API that made it possible to remove almost all of copy-pasted code from scalafix internals. --- .../main/scala/scala/meta/metals/Uri.scala | 3 +- .../ScalafixPatchEnrichments.scala | 69 ++----------------- .../refactoring/RemoveUnusedImportsTest.scala | 3 +- 3 files changed, 6 insertions(+), 69 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/Uri.scala b/metals/src/main/scala/scala/meta/metals/Uri.scala index 1b3ba6471bd..104835a3da8 100644 --- a/metals/src/main/scala/scala/meta/metals/Uri.scala +++ b/metals/src/main/scala/scala/meta/metals/Uri.scala @@ -5,7 +5,6 @@ import java.nio.file.Path import java.nio.file.Paths import org.langmeta.lsp.TextDocumentIdentifier import org.langmeta.lsp.VersionedTextDocumentIdentifier -import org.langmeta.lsp.VersionedTextDocumentIdentifier import org.langmeta.inputs.Input import org.langmeta.io.AbsolutePath @@ -37,7 +36,7 @@ sealed abstract case class Uri(value: String) { object Uri { def apply(uri: String): Uri = Uri(URI.create(uri)) def file(path: String): Uri = { - val slash = if (path.startsWith("/")) "" else "/" + val slash = if (path.startsWith("/")) "" else "///" Uri(s"file:$slash${path.replace(' ', '-')}") } def apply(td: TextDocumentIdentifier): Uri = Uri(td.uri) diff --git a/metals/src/main/scala/scalafix/languageserver/ScalafixPatchEnrichments.scala b/metals/src/main/scala/scalafix/languageserver/ScalafixPatchEnrichments.scala index e028a2f27df..66362ea4b61 100644 --- a/metals/src/main/scala/scalafix/languageserver/ScalafixPatchEnrichments.scala +++ b/metals/src/main/scala/scalafix/languageserver/ScalafixPatchEnrichments.scala @@ -1,29 +1,14 @@ package scalafix.languageserver -import scala.collection.immutable.Seq -import scala.meta.metals.ScalametaEnrichments._ import org.langmeta.lsp.TextEdit +import scala.meta.metals.ScalametaEnrichments._ import scalafix.SemanticdbIndex -import scalafix.internal.patch.ImportPatchOps -import scalafix.internal.patch.ReplaceSymbolOps import scalafix.internal.util.Failure import scalafix.internal.util.TokenOps -import scalafix.patch.Concat -import scalafix.patch.EmptyPatch -import scalafix.patch.LintPatch import scalafix.patch.Patch import scalafix.patch.TokenPatch -import scalafix.patch.TreePatch.ImportPatch -import scalafix.patch.TreePatch.ReplaceSymbol import scalafix.rule.RuleCtx -// Copy-pasta from scalafix because all of these methods are private. -// We should expose a package private API to get a list of token patches from -// a Patch. -// TODO(olafur): Figure out how to expose a minimal public API in scalafix.Patch -// that supports this use-case. -// All of the copy-paste below could be avoided with a single: -// Patch.toTokenPatches(Patch): Iterable[TokenPatch] object ScalafixPatchEnrichments { implicit class XtensionPatchLSP(val patch: Patch) extends AnyVal { @@ -38,7 +23,8 @@ object ScalafixPatchEnrichments { implicit ctx: RuleCtx, index: SemanticdbIndex ): List[TextEdit] = { - val mergedTokenPatches = tokenPatches(patch) + val mergedTokenPatches = Patch + .treePatchApply(patch) .groupBy(x => TokenOps.hash(x.tok)) .values .map(_.reduce(merge)) @@ -51,56 +37,9 @@ object ScalafixPatchEnrichments { .toList } } - private def tokenPatches( - patch: Patch - )(implicit ctx: RuleCtx, index: SemanticdbIndex): Iterable[TokenPatch] = { - val base = underlying(patch) - val moveSymbol = underlying( - ReplaceSymbolOps.naiveMoveSymbolPatch(base.collect { - case m: ReplaceSymbol => m - }) - ) - val patches = base.filterNot(_.isInstanceOf[ReplaceSymbol]) ++ moveSymbol - val tokenPatches = patches.collect { case e: TokenPatch => e } - val importPatches = patches.collect { case e: ImportPatch => e } - val importTokenPatches = { - val result = ImportPatchOps.superNaiveImportPatchToTokenPatchConverter( - ctx, - importPatches - ) - underlying(result.asPatch) - .collect { - case x: TokenPatch => x - case els => - throw Failure.InvariantFailedException( - s"Expected TokenPatch, got $els" - ) - } - } - importTokenPatches ++ tokenPatches - } - private def underlying(patch: Patch): Seq[Patch] = { - val builder = Seq.newBuilder[Patch] - foreach(patch) { - case _: LintPatch => - case els => - builder += els - } - builder.result() - } - private def foreach(patch: Patch)(f: Patch => Unit): Unit = { - def loop(patch: Patch): Unit = patch match { - case Concat(a, b) => - loop(a) - loop(b) - case EmptyPatch => // do nothing - case els => - f(els) - } - loop(patch) - } import scalafix.patch.TokenPatch._ + // TODO(olafur): make this method public in scalafix API private def merge(a: TokenPatch, b: TokenPatch): TokenPatch = (a, b) match { case (add1: Add, add2: Add) => Add( diff --git a/metals/src/test/scala/tests/refactoring/RemoveUnusedImportsTest.scala b/metals/src/test/scala/tests/refactoring/RemoveUnusedImportsTest.scala index 3470a955c9f..d4f47c8c3f3 100644 --- a/metals/src/test/scala/tests/refactoring/RemoveUnusedImportsTest.scala +++ b/metals/src/test/scala/tests/refactoring/RemoveUnusedImportsTest.scala @@ -9,8 +9,7 @@ import tests.CompilerSuite // This test suite is not supposed to test the actual scalafix implementation, // it is only supposed to check that the conversion from scalafix.Patch to // languageserver.TextEdit is accurate. -// Disabled until scalafix 0.6.0 -class RemoveUnusedImportsTest extends CompilerSuite { +object RemoveUnusedImportsTest extends CompilerSuite { def check(name: String, original: String, expectedEdits: String): Unit = { test(name) { val uri = Uri.file(name) From 1f6ec3927d551f0cf1fdd9ce109925cebec4cc71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 21 Apr 2018 01:57:39 +0200 Subject: [PATCH 21/25] Polish new Hover implementation - Include JDK classpath - Document blocking issues --- .../meta/metals/compiler/MetacpProvider.scala | 65 +++++-------------- .../meta/metals/providers/HoverProvider.scala | 4 ++ .../tests/provider/HoverProviderTest.scala | 13 ++++ 3 files changed, 34 insertions(+), 48 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala b/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala index 3d708a1937c..9325bc50134 100644 --- a/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala @@ -26,7 +26,11 @@ class MetacpProvider { .process(Settings().withScalaLibrarySynthetics(true), reporter) .get .shallow - .head + private val jdk = Metacp + .process(settings.withClasspath(ClasspathOps.bootClasspath), reporter) + .getOrElse { + throw new IllegalArgumentException("Failed to process JDK") + } private val empty = AbsolutePath(Files.createTempDirectory("metals")) private val processEntry = new function.Function[AbsolutePath, AbsolutePath] { override def apply(t: AbsolutePath): AbsolutePath = { @@ -44,61 +48,26 @@ class MetacpProvider { } def process(classpath: Classpath): Classpath = { - Classpath(synthetics :: classpath.shallow.map { entry => + val processed = classpath.shallow.map { entry => if (entry.isDirectory) entry else classpaths.computeIfAbsent(entry, processEntry) - }) + } + Classpath(List(synthetics, jdk.shallow, processed).flatten) } } object ClasspathOps { - def bootClasspath: Option[Classpath] = sys.props.collectFirst { - case (k, v) if k.endsWith(".boot.class.path") => Classpath(v) - } - - val devNull = new PrintStream(new OutputStream { - override def write(b: Int): Unit = () - }) - - /** Process classpath with metacp to build semanticdbs of global symbols. **/ - def toMetaClasspath( - sclasspath: Classpath, - cacheDirectory: Option[AbsolutePath] = None, - parallel: Boolean = true, - out: PrintStream = devNull - ): Option[Classpath] = { - val (processed, toProcess) = sclasspath.shallow.partition { path => - path.isDirectory && - path.resolve("META-INF").resolve("semanticdb.semanticidx").isFile - } - val withJDK = Classpath( - bootClasspath.fold(sclasspath.shallow)(_.shallow ::: toProcess) - ) - val default = metacp.Settings() - val settings = default - .withClasspath(withJDK) - .withScalaLibrarySynthetics(true) - .withCacheDir(cacheDirectory.getOrElse(default.cacheDir)) - .withPar(parallel) - val reporter = metacp - .Reporter() - .withOut(devNull) // out prints classpath of proccessed classpath, which is not relevant for scalafix. - .withErr(out) - val mclasspath = scala.meta.cli.Metacp.process(settings, reporter) - mclasspath.map(x => Classpath(x.shallow ++ processed)) - } - - def newSymbolTable( - classpath: Classpath, - cacheDirectory: Option[AbsolutePath] = None, - parallel: Boolean = true, - out: PrintStream = System.out - ): Option[SymbolTable] = { - toMetaClasspath(classpath, cacheDirectory, parallel, out) - .map(new LazySymbolTable(_)) - } + def bootClasspath: Classpath = + sys.props + .collectFirst { + case (k, v) if k.endsWith(".boot.class.path") => + Classpath( + Classpath(v).shallow.filter(p => Files.exists(p.toNIO)) + ) + } + .getOrElse(Classpath(Nil)) def getCurrentClasspath: Classpath = { Thread.currentThread.getContextClassLoader match { diff --git a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala index d0ae98888b5..ea14a5e12ef 100644 --- a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala @@ -59,10 +59,14 @@ object HoverProvider extends LazyLogging { info.symbol.stripSuffix(".").parse[Term].get.asInstanceOf[Term.Ref] Pkg(ref, Nil) } else if (info.kind.isTrait) { + // TODO: include type parameters Defn.Trait(mods, Type.Name(info.name), Nil, EmptyCtor, EmptyTemplate) } else if (info.kind.isClass) { + // TODO: include type parameters and primary constructor Defn.Class(mods, Type.Name(info.name), Nil, EmptyCtor, EmptyTemplate) } else if (info.kind.isLocal || info.symbol.startsWith("local")) { + // Workaround for https://github.com/scalameta/scalameta/issues/1503 + // In the future we should be able to produce `val x: Int` syntax for local symbols. PrettyType.toType(info.tpe.get, symtab, QualifyStrategy.Readable).tree } else { PrettyType.toTree(info, symtab, QualifyStrategy.Readable).tree diff --git a/metals/src/test/scala/tests/provider/HoverProviderTest.scala b/metals/src/test/scala/tests/provider/HoverProviderTest.scala index 5cacd8333bc..31a15308a72 100644 --- a/metals/src/test/scala/tests/provider/HoverProviderTest.scala +++ b/metals/src/test/scala/tests/provider/HoverProviderTest.scala @@ -1,5 +1,7 @@ package tests.provider +import java.util + object HoverProviderTest extends BaseHoverProviderTest { check( @@ -307,4 +309,15 @@ object HoverProviderTest extends BaseHoverProviderTest { """.stripMargin, "sealed trait W" ) + + check( + "jdk", + """ + |object x { + | val <> = + | new java.util.HashMap[java.lang.Integer, java.lang.String]().entrySet() + |} + """.stripMargin, + "val entry: Set[Map.Entry[Integer, String]]" + ) } From b20a6c9c25c992bef7cb698108335e8827312fe0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 21 Apr 2018 02:23:47 +0200 Subject: [PATCH 22/25] Add more relevant tests for hover --- .../meta/metals/providers/HoverProvider.scala | 16 ++++- .../tests/provider/HoverProviderTest.scala | 67 ++++++++++++++++++- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala index ea14a5e12ef..46e8dffd05b 100644 --- a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala @@ -64,10 +64,22 @@ object HoverProvider extends LazyLogging { } else if (info.kind.isClass) { // TODO: include type parameters and primary constructor Defn.Class(mods, Type.Name(info.name), Nil, EmptyCtor, EmptyTemplate) - } else if (info.kind.isLocal || info.symbol.startsWith("local")) { + } else if (info.symbol == "scala.Any#asInstanceOf().") { + // HACK(olafur) to avoid 'java.util.NoSuchElementException: scala.Any.asInstanceOf(A).[A]' + q"final def asInstanceOf[T]: T" + } else if (info.kind.isLocal || + info.kind.isParameter || + info.symbol.startsWith("local")) { // Workaround for https://github.com/scalameta/scalameta/issues/1503 // In the future we should be able to produce `val x: Int` syntax for local symbols. - PrettyType.toType(info.tpe.get, symtab, QualifyStrategy.Readable).tree + val tpe = info.tpe match { + case Some(t) => + if (t.tag.isMethodType) t.methodType.get.returnType.get + else t + case _ => + throw new IllegalArgumentException(info.toProtoString) + } + PrettyType.toType(tpe, symtab, QualifyStrategy.Readable).tree } else { PrettyType.toTree(info, symtab, QualifyStrategy.Readable).tree } diff --git a/metals/src/test/scala/tests/provider/HoverProviderTest.scala b/metals/src/test/scala/tests/provider/HoverProviderTest.scala index 31a15308a72..d80d431984a 100644 --- a/metals/src/test/scala/tests/provider/HoverProviderTest.scala +++ b/metals/src/test/scala/tests/provider/HoverProviderTest.scala @@ -241,7 +241,7 @@ object HoverProviderTest extends BaseHoverProviderTest { ) check( - "params", + "lambda params", """ |object v { | List(1).foreach(<> => println(x)) @@ -320,4 +320,69 @@ object HoverProviderTest extends BaseHoverProviderTest { """.stripMargin, "val entry: Set[Map.Entry[Integer, String]]" ) + + check( + "param", + """ + |object y { + | def bar(<>: Map[Int, String]) = param + |} + """.stripMargin, + "Map[Int, String]" + ) + + check( + "Any synthetic", + """ + |object z { + | val <>: Any = 2 + |} + """.stripMargin, + "val x: Any" + ) + + check( + "asInstanceOf", + """ + |object z2 { + | val x = 2.<>[Any] + |} + """.stripMargin, + "final def asInstanceOf[T]: T" + ) + + check( + "isInstanceOf", + """ + |object z3 { + | val x = "".<>[String] + |} + """.stripMargin, + "final def isInstanceOf(): Boolean" + ) + + check( + "local", + """ + |object z4 { + | locally { + | val <> = 42 + | } + |} + """.stripMargin, + "Int" + ) + + check( + "local2", + """ + |object z5 { + | val x: Serializable = new Serializable { + | val <> = 42 + | } + |} + """.stripMargin, + "private val x: Int" + ) + } From 317fd60ad12a0fc5d340b8706c55e18a7e10aa67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 21 Apr 2018 02:28:23 +0200 Subject: [PATCH 23/25] Document hover requires metalsSetup --- docs/overview.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/overview.md b/docs/overview.md index 06f2d0233c4..bcf816d0a19 100644 --- a/docs/overview.md +++ b/docs/overview.md @@ -82,8 +82,8 @@ architecture as [Index-While-Building][] in XCode 9. * [x] Symbol outline in the sidebar as you type (`textDocument/documentSymbol`). * [ ] Goto implementation (`textDocument/implementation`) * [ ] Goto type definition (`textDocument/typeDefinition`) -* [x] Show type of symbol at position (`textDocument/hover`). Requires - [semanticdb-scalac](#semanticdb-scalac). +* [x] Show type of symbol at position (`textDocument/hover`). + Requires [`metalsSetup`](#metalssetup). * [ ] Show type of expression at position (`textDocument/hover`). ## Fast completions From 0c9d0fce827d8cf721b41b7432556e3ce1592869 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 21 Apr 2018 02:33:37 +0200 Subject: [PATCH 24/25] Log slightly more helpful error message on failure to parse proto file SemanticDB v2 payloads will now cause an error to be reported.in the logs. --- .../main/scala/scala/meta/metals/Semanticdbs.scala | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala b/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala index 965faf39eb8..e63b0ffa2fb 100644 --- a/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala +++ b/metals/src/main/scala/scala/meta/metals/Semanticdbs.scala @@ -1,5 +1,6 @@ package scala.meta.metals +import com.google.protobuf.InvalidProtocolBufferException import java.nio.file.Files import scala.meta.interactive.InteractiveSemanticdb import scala.meta.metals.compiler.ScalacProvider @@ -109,7 +110,17 @@ object Semanticdbs extends LazyLogging { cwd: AbsolutePath ): semanticdb3.TextDocuments = { val bytes = Files.readAllBytes(semanticdbPath.toNIO) - val sdb = semanticdb3.TextDocuments.parseFrom(bytes) + val sdb = + try { + semanticdb3.TextDocuments.parseFrom(bytes) + } catch { + case _: InvalidProtocolBufferException => + logger.error( + s"Have you upgraded to SemanticDB v3? Error parsing $semanticdbPath" + ) + semanticdb3.TextDocuments() + } + val docs = sdb.documents.map { d => val filename = cwd.resolve(d.uri).toURI.toString logger.info(s"Loading file $filename") @@ -120,7 +131,6 @@ object Semanticdbs extends LazyLogging { util.Sorting.quickSort(names) names } - } semanticdb3.TextDocuments(docs) } From 42765d7effb4a73998347fa6fb448b3c5e13c5ab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=93lafur=20P=C3=A1ll=20Geirsson?= Date: Sat, 21 Apr 2018 15:51:05 +0200 Subject: [PATCH 25/25] Address review feedback and polish. - Document Mtags version number - Replace TODO with tickets - Simplify handling of class info type in hover - Include primary constructor for hover --- .../scala/meta/metals/MetalsServices.scala | 4 +- .../meta/metals/compiler/MetacpProvider.scala | 5 - .../meta/metals/compiler/ScalacProvider.scala | 13 +- .../scala/meta/metals/index/SymbolData.scala | 23 ++-- .../scala/scala/meta/metals/mtags/Mtags.scala | 9 ++ .../meta/metals/providers/HoverProvider.scala | 125 ++++++++++-------- .../ScalafixPatchEnrichments.scala | 2 +- .../tests/provider/HoverProviderTest.scala | 19 ++- 8 files changed, 118 insertions(+), 82 deletions(-) diff --git a/metals/src/main/scala/scala/meta/metals/MetalsServices.scala b/metals/src/main/scala/scala/meta/metals/MetalsServices.scala index 70cefb6ed33..85a59d6c742 100644 --- a/metals/src/main/scala/scala/meta/metals/MetalsServices.scala +++ b/metals/src/main/scala/scala/meta/metals/MetalsServices.scala @@ -52,6 +52,7 @@ import org.langmeta.lsp.LanguageClient import org.langmeta.internal.semanticdb._ import scala.meta.metals.compiler.MetacpProvider import scala.meta.metals.compiler.SymtabProvider +import scala.meta.metals.mtags.Mtags class MetalsServices( cwd: AbsolutePath, @@ -536,11 +537,10 @@ class MetalsServices( object MetalsServices extends LazyLogging { lazy val cacheDirectory: AbsolutePath = { - val cacheVersion = "0.1" val path = AbsolutePath( ProjectDirectories.fromProjectName("metals").projectCacheDir ) - Files.createDirectories(path.resolve(cacheVersion).toNIO) + Files.createDirectories(path.resolve(Mtags.Version).toNIO) path } diff --git a/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala b/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala index 9325bc50134..1136d2d1f87 100644 --- a/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala @@ -1,8 +1,6 @@ package scala.meta.metals.compiler import java.io.File -import java.io.OutputStream -import java.io.PrintStream import java.nio.file.Files import java.util import java.util.Collections @@ -10,11 +8,8 @@ import java.util.function import scala.meta.AbsolutePath import scala.meta.Classpath import scala.meta.cli.Metacp -import scala.meta.metacp import scala.meta.metacp.Reporter import scala.meta.metacp.Settings -import scalafix.internal.util.LazySymbolTable -import scalafix.internal.util.SymbolTable class MetacpProvider { private val classpaths = diff --git a/metals/src/main/scala/scala/meta/metals/compiler/ScalacProvider.scala b/metals/src/main/scala/scala/meta/metals/compiler/ScalacProvider.scala index de48ccb6367..d9286601055 100644 --- a/metals/src/main/scala/scala/meta/metals/compiler/ScalacProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/compiler/ScalacProvider.scala @@ -35,7 +35,9 @@ class ScalacProvider()(implicit client: JsonRpcClient) extends LazyLogging { // Looking up by sourceDirectory alone is not enough since // in sbt it's possible to `sources += file("blah")`, which would // not be respected if we only went by directories. - .orElse(compilerBySourceDirectory(uri).map(_._2)) + .orElse(compilerBySourceDirectory(uri).map { + case (_, global) => global + }) .map { compiler => compiler.reporter.reset() compiler @@ -56,9 +58,14 @@ class ScalacProvider()(implicit client: JsonRpcClient) extends LazyLogging { mutable.Map.empty[AbsolutePath, (CompilerConfig, Global)] private val compilerByPath = mutable.Map.empty[Uri, Global] def configForUri(uri: Uri): Option[CompilerConfig] = - compilerBySourceDirectory(uri).map(_._1) + compilerBySourceDirectory(uri).map { + case (config, _) => config + } + def allCompilerConfigs: Iterable[CompilerConfig] = - compilerByConfigOrigin.values.map(_._1) + compilerByConfigOrigin.values.map { + case (config, _) => config + } def loadNewCompilerGlobals( config: CompilerConfig ): Effects.InstallPresentationCompiler = { diff --git a/metals/src/main/scala/scala/meta/metals/index/SymbolData.scala b/metals/src/main/scala/scala/meta/metals/index/SymbolData.scala index b98555d7c28..13c28a887da 100644 --- a/metals/src/main/scala/scala/meta/metals/index/SymbolData.scala +++ b/metals/src/main/scala/scala/meta/metals/index/SymbolData.scala @@ -4,22 +4,19 @@ import org.langmeta.{lsp => l} import scala.meta.metals.Uri import scala.meta.internal.{semanticdb3 => s} +/** All metadata about a single symbol that is stored in the symbol index. + * + * @param symbol The symbol itself. + * @param definition The location where this symbol is defined, could come from project + * sources via metac or dependency sources via mtags. + * @param references Locations of references to this symbol, come from project + * sources via metac. + * @param info The SemanticDB information about this symbol, contains type, owner, name, + * accessibility, kind, and other juicy metadata. + */ case class SymbolData( symbol: String, definition: Option[l.Location], references: Map[Uri, Seq[l.Range]], info: Option[s.SymbolInformation] ) - -//message SymbolData { -// string symbol = 1; -// // The Position where this symbol is defined. -// Position definition = 2; -// map references = 3; -// int32 kind = 4; -// string name = 5; -// string signature = 6; -// // Planned for Scalameta v2.2, see https://github.com/scalameta/scalameta/milestone/9 -// // string docstring = 7; // javadoc/scaladoc -// // string overrides = 8; // parent symbol that this symbol overrides -//} diff --git a/metals/src/main/scala/scala/meta/metals/mtags/Mtags.scala b/metals/src/main/scala/scala/meta/metals/mtags/Mtags.scala index 969b441abad..109f218e137 100644 --- a/metals/src/main/scala/scala/meta/metals/mtags/Mtags.scala +++ b/metals/src/main/scala/scala/meta/metals/mtags/Mtags.scala @@ -42,6 +42,15 @@ import scala.meta.internal.semanticdb3.TextDocument */ object Mtags extends LazyLogging { + /** Mtags version, used to invalidate caches when mtags is improved. + * + * We cache mtags artifacts to avoid redundant work, cached artifacts + * are keyed by the original jar file path and this version number. + * If we update mtags to, for example, fix a bug then we should bump + * this version number. + */ + val Version = "1" + /** * Build an index from a classpath of -sources.jar * diff --git a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala index 46e8dffd05b..f4e1bacc03e 100644 --- a/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala +++ b/metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala @@ -1,21 +1,22 @@ package scala.meta.metals.providers -import scala.meta.metals.Uri +import com.typesafe.scalalogging.LazyLogging import org.langmeta.lsp.Hover import org.langmeta.lsp.RawMarkedString -import scala.meta.metals.search.SymbolIndex -import scala.{meta => m} -import scala.meta.internal.semanticdb3.SymbolInformation.{Property => p} -import com.typesafe.scalalogging.LazyLogging import scala.meta.Tree import scala.meta._ +import scala.meta.internal.semanticdb3.SymbolInformation.Property +import scala.meta.internal.{semanticdb3 => s} +import scala.meta.internal.semanticdb3.Scala._ +import scala.meta.metals.Uri import scala.meta.metals.compiler.SymtabProvider +import scala.meta.metals.ScalametaEnrichments._ +import scala.meta.metals.search.SymbolIndex import scala.util.control.NonFatal +import scala.{meta => m} import scalafix.internal.util.PrettyType import scalafix.internal.util.QualifyStrategy -import scala.meta.internal.{semanticdb3 => s} import scalafix.internal.util.SymbolTable -import scala.meta.internal.semanticdb3.Scala._ object HoverProvider extends LazyLogging { def empty: Hover = Hover(Nil, None) @@ -30,40 +31,39 @@ object HoverProvider extends LazyLogging { private def toTree( info: s.SymbolInformation, symtab: SymbolTable - ): Tree = { - def mods = { - // TODO: Upstream FINAL OBJECT fix to scalafix, - // this method is copy pasted from PrettyType - def is(property: s.SymbolInformation.Property): Boolean = - (info.properties & property.value) != 0 - val buf = List.newBuilder[Mod] - info.accessibility.foreach { accessibility => - // TODO: private[within] - if (accessibility.tag.isPrivate) buf += Mod.Private(Name.Anonymous()) - if (accessibility.tag.isProtected) - buf += Mod.Protected(Name.Anonymous()) - } - if (is(p.SEALED)) buf += Mod.Sealed() - if (info.kind.isClass && is(p.ABSTRACT)) buf += Mod.Abstract() - if (!info.kind.isObject && is(p.FINAL)) buf += Mod.Final() - if (is(p.IMPLICIT)) buf += Mod.Implicit() - if (info.kind.isClass && is(p.CASE)) buf += Mod.Case() - buf.result() + ): Option[Tree] = { + try { + Some(unsafeToTree(info, symtab)) + } catch { + case NonFatal(e) => + logger.error( + s"""Failed to pretty print symbol ${info.symbol} + |Classpath=$symtab + |${info.toProtoString} + """.stripMargin, + e + ) + None } - if (info.kind.isObject) { - Defn.Object(mods, Term.Name(info.name), EmptyTemplate) - } else if (info.kind.isPackageObject) { - Pkg.Object(mods, Term.Name(info.owner.desc.name), EmptyTemplate) - } else if (info.kind.isPackage) { + } + + private def unsafeToTree( + info: s.SymbolInformation, + symtab: SymbolTable + ): Tree = { + if (info.kind.isPackage) { val ref = info.symbol.stripSuffix(".").parse[Term].get.asInstanceOf[Term.Ref] Pkg(ref, Nil) - } else if (info.kind.isTrait) { - // TODO: include type parameters - Defn.Trait(mods, Type.Name(info.name), Nil, EmptyCtor, EmptyTemplate) - } else if (info.kind.isClass) { - // TODO: include type parameters and primary constructor - Defn.Class(mods, Type.Name(info.name), Nil, EmptyCtor, EmptyTemplate) + } else if (info.kind.isPackageObject) { + // NOTE(olafur) custom handling for name due to https://github.com/scalameta/scalameta/issues/1484 + Pkg.Object(Nil, Term.Name(info.owner.desc.name), EmptyTemplate) + } else if (info.kind.isObject) { + // NOTE(olafur) custom handling for object due to https://github.com/scalameta/scalameta/issues/1480 + val mods = List.newBuilder[Mod] + if (info.has(Property.IMPLICIT)) mods += mod"implicit" + if (info.has(Property.CASE)) mods += mod"case" + Defn.Object(mods.result(), Term.Name(info.name), EmptyTemplate) } else if (info.symbol == "scala.Any#asInstanceOf().") { // HACK(olafur) to avoid 'java.util.NoSuchElementException: scala.Any.asInstanceOf(A).[A]' q"final def asInstanceOf[T]: T" @@ -74,14 +74,43 @@ object HoverProvider extends LazyLogging { // In the future we should be able to produce `val x: Int` syntax for local symbols. val tpe = info.tpe match { case Some(t) => - if (t.tag.isMethodType) t.methodType.get.returnType.get - else t + if (t.tag.isMethodType) { + t.methodType.get.returnType.get + } else { + t + } case _ => throw new IllegalArgumentException(info.toProtoString) } PrettyType.toType(tpe, symtab, QualifyStrategy.Readable).tree } else { - PrettyType.toTree(info, symtab, QualifyStrategy.Readable).tree + val tpe = info.tpe.get + val noDeclarations = + if (tpe.tag.isClassInfoType) { + val classInfoType = tpe.classInfoType.get + // Remove declarations from object/trait/class to reduce noise + val declarations = + if (info.kind.isClass) { + classInfoType.declarations.find { sym => + symtab + .info(sym) + .exists(i => i.kind.isConstructor && i.has(Property.PRIMARY)) + }.toList + } else { + Nil + } + info.copy( + tpe = Some( + tpe.copy( + classInfoType = + Some(classInfoType.withDeclarations(declarations)) + ) + ) + ) + } else { + info + } + PrettyType.toTree(noDeclarations, symtab, QualifyStrategy.Readable).tree } } @@ -96,21 +125,7 @@ object HoverProvider extends LazyLogging { (symbol, _) <- index.findSymbol(uri, line, column) symtab = symtabs.symtab(uri) info <- symtab.info(symbol.syntax) - tree <- { - try { - Some(toTree(info, symtab)) - } catch { - case NonFatal(e) => - logger.error( - s"""Failed to pretty print symbol $symbol - |Classpath=$symtab - |${info.toProtoString} - |""".stripMargin, - e - ) - None - } - } + tree <- toTree(info, symtab) } yield { val pretty = tree.transform { case Type.Apply(op: Type.Name, lhs :: rhs :: Nil) diff --git a/metals/src/main/scala/scalafix/languageserver/ScalafixPatchEnrichments.scala b/metals/src/main/scala/scalafix/languageserver/ScalafixPatchEnrichments.scala index 66362ea4b61..729cf289557 100644 --- a/metals/src/main/scala/scalafix/languageserver/ScalafixPatchEnrichments.scala +++ b/metals/src/main/scala/scalafix/languageserver/ScalafixPatchEnrichments.scala @@ -39,7 +39,7 @@ object ScalafixPatchEnrichments { } import scalafix.patch.TokenPatch._ - // TODO(olafur): make this method public in scalafix API + // TODO(olafur): fix https://github.com/scalacenter/scalafix/issues/709 so we can remove this copy-pasta private def merge(a: TokenPatch, b: TokenPatch): TokenPatch = (a, b) match { case (add1: Add, add2: Add) => Add( diff --git a/metals/src/test/scala/tests/provider/HoverProviderTest.scala b/metals/src/test/scala/tests/provider/HoverProviderTest.scala index d80d431984a..b9511d7623d 100644 --- a/metals/src/test/scala/tests/provider/HoverProviderTest.scala +++ b/metals/src/test/scala/tests/provider/HoverProviderTest.scala @@ -143,9 +143,12 @@ object HoverProviderTest extends BaseHoverProviderTest { check( "class", """ - |class <>(x: Int, y: String) + |class <>(x: Int, y: String) { + | def this(x: Int) = this(x, "") // secondary constructor + | def ignoreme = x + |} """.stripMargin, - "class C" + "class C(x: Int, y: String)" ) check( @@ -307,7 +310,7 @@ object HoverProviderTest extends BaseHoverProviderTest { """ |sealed trait <>[T] """.stripMargin, - "sealed trait W" + "sealed trait W[T]" ) check( @@ -385,4 +388,14 @@ object HoverProviderTest extends BaseHoverProviderTest { "private val x: Int" ) + check( + "implicit object", + """ + |object aa { + | implicit case object <> + |} + """.stripMargin, + "implicit case object Foo" + ) + }