Upgrade to Scalameta v3.7 #275

olafurpg · 2018-04-09T17:49:43Z

This PR upgrades from Scalameta v2.1.7 from January this year to Scalameta v3.7.3 that was released this morning. The SemanticDB schema went through aggressive changes that primarily involved

Renamings to adopt more LSP-friendly names. TextDocument instead of Document, SymbolInformation instead of Denotation, SymbolOccurrence instead of ResolvedName (inspired by Index-While-Building in clang).
Change Position.{start,end} offsets to Range.{startLine, startCharacter, endLine, endCharacter} to align with LSP conventions.

In addition, the following changes in semanticdb-scalac directly affect Metals

Denotation.signature is deprecated in favor of SymbolInformation.tpe, which affects HoverProvider that used the old signature to show a readable signature. We are blocked by Metap should expose API to pretty-print single SymbolInformation.tpe scalameta#1479 to produce human readable output for SymbolInformation.tpe.
-P:semanticdb:denotations:definitions is now default, meaning that scalafix depends on metalsSetup in order to process the dependency classpath.

This PR is primarily about upgrading and making sure the functionality works OK. I'd like to get this merged asap to avoid merge conflicts.

TODOs:

xeno-by · 2018-04-09T19:53:30Z

I will start working on scalameta/scalameta#1479 today or tomorrow at the latest.

ShaneDelmore · 2018-04-10T04:18:52Z

metals/src/main/scala/scala/meta/metals/Semanticdbs.scala

+            byLine
+          } else {
+            val byCharacter = Integer.compare(
+              x.range.get.startCharacter,


I find it unfortunate that we always call .get on these. What is the reason that are always sure they are nonEmpty, yet make these optional? Since they are positional maybe this could be modeled better with the concept of pos.MIN and pos.MAX and never have an unpositioned item?

I guard against isEmpty above.

This is what I typically use in these cases:

(x.range, y.range) match { case (None, _) | (_, None) => 0 case (Some(xRange), Some(yRange)) => //... }

or

(x.range, y.range) match { case (Some(xRange), Some(yRange)) => //... case _ => 0 }

gabro · 2018-04-10T07:15:17Z

metals/src/main/scala/scala/meta/metals/Semanticdbs.scala

+  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


Double check on x, you probably meant y.range.isEmpty the second time.

gabro · 2018-04-10T08:19:14Z

metals/src/main/scala/org/langmeta/languageserver/InputEnrichments.scala

+        startLine = r.startLine,
+        startColumn = r.startCharacter,
+        endLine = r.endLine,
+        endColumn = r.endCharacter


these are the cases in which I wish we had a structural type system

gabro · 2018-04-10T08:20:16Z

metals/src/main/scala/scala/meta/metals/MetalsServices.scala

@@ -42,12 +42,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


curiosity: why are semanticdb3 data types internal?

Because otherwise we'd have to provide strong compatibility guarantees for them: https://github.com/scalameta/scalameta/blob/master/COMPATIBILITY.md. I don't think we're fully ready to do this right now.

gabro

Looking great, the new names make everything so much clearer to me (especially SymbolOccurrence vs ResolvedName is a welcome change).

I've found some minor issues and asked some questions. See the comments.

gabro · 2018-04-10T08:29:31Z

metals/src/main/scala/scala/meta/metals/Semanticdbs.scala

+      logger.info(s"Loading file $filename")
+      d.withUri(filename)
+        .withOccurrences {
+          // This should be done inside semanticdb-scalac.


is there an issue to track this? (I know the comment was already there)

gabro · 2018-04-10T08:31:39Z

metals/src/main/scala/scala/meta/metals/mtags/MtagsIndexer.scala

-      Some(Position(definition.start, definition.end)),
+    val role =
+      if (kind.isPackage) s.SymbolOccurrence.Role.REFERENCE
+      else s.SymbolOccurrence.Role.REFERENCE


if and else branches yield the same value

gabro · 2018-04-10T08:37:12Z

metals/src/main/scala/scala/meta/metals/mtags/ScalaMtags.scala

+          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()


Is is right that Defn.Var/Val are all Kind.METHOD?

I've read https://github.com/scalameta/scalameta/blob/master/semanticdb/semanticdb3/semanticdb3.md#symbolinformation-1, it's not clear whether t is the field or the synthetic method here.

The SymbolOccurrence part of the format is still unspecified at this point. For now, I propose that we do whatever semanticdb-scalac does.

gabro · 2018-04-10T08:38:17Z

metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala

        range = None
      )
    }
    result.getOrElse(Hover(Nil, None))
  }

-  /** Returns a definition tree for this symbol signature */
-  private def getPrettyDefinition(


I'm glad to see this gone in favor of a scalameta pretty-printer!

I agree! The current hover output is not helpful but that needs to be fixed upstream.

gabro · 2018-04-10T08:39:38Z

metals/src/main/scala/scala/meta/metals/search/InMemorySymbolIndex.scala

          sym
        )
      case _ =>
    }
    document.symbols.foreach {
-      case s.ResolvedSymbol(sym, Some(denot)) =>
+      case denot: semanticdb3.SymbolInformation =>


s/denot/symbolInfo ?

Or simply info. This is how we name related variables in scalameta/scalameta.

gabro · 2018-04-10T08:41:27Z

metals/src/main/scala/scalafix/languageserver/ScalafixEnrichments.scala

    ): List[LintMessage] =
-      Patch.lintMessages(patches, ctx)
+      Patch(patches, ctx, Some(index))._2


Minor: for readability, I tend to shy away from _N methods in tuples.
Can we do:

val (_, lintMessage) = Patch(patches, ctx, Some(index)) lintMessage

instead?

olafurpg · 2018-04-10T20:52:25Z

Thanks for the comments! The test suite passes and most things seem to work OK except the remaining TODOs

olafurpg · 2018-04-10T21:04:14Z

NB, a couple test suites are still disabled due to blocking issues. Fixing hover tests will require integrating with metacp and make hover depend on metalsSetup

gabro · 2018-04-10T21:12:34Z

metals/src/main/scala/scala/meta/metals/Semanticdbs.scala

+    new Ordering[semanticdb3.SymbolOccurrence] {
+      override def compare(x: SymbolOccurrence, y: SymbolOccurrence): Int = {
+        if (x.range.isEmpty) 0
+        else if (y.range.isEmpty) 0


if (x.range.isEmpty || y.range.isEmpty) 0

?

I made it this way to make it easier to see diff between x.range and y.range

For that reason I like to format such conditions this way:

if ( x.range.isEmpty || y.range.isEmpty ) ...

But scalafmt with changes it to this:

if (x.range.isEmpty || y.range.isEmpty) ...

😒

There's an undocumented align.ifWhileOpenParen = true to override that @laughedelic I agree that's the most annoying part of align = false, this is the indentation style used by Scala.js.

We should probably support config style in if conditions though 🤔

olafurpg · 2018-04-10T21:16:42Z

cc/ @martijnhoekstra FYI the scalapb protobuf generation is no longer part of the build step, it wasn't required.

olafurpg · 2018-04-12T17:51:58Z

Blocked by scalacenter/scalafix#699 with test failures https://gist.github.com/olafurpg/30729248cc54ccafa3802f87850a683e

Help to grind through the reported issues would be appreciated https://github.com/scalameta/scalameta/milestone/26 The tickests are ordered by importance, scalameta/scalameta#1491 is by far most critical

olafurpg

While manually testing the new hover I hit on the problem that classes with @JsonCodec don't have SymbolInformation. I opened scalameta/scalameta#1504 to track this

olafurpg · 2018-04-20T23:45:16Z

metals/src/test/scala/tests/hover/HoverTest.scala

@@ -214,7 +214,7 @@ object HoverTest extends BaseHoverTest {
      |  List(1) match { case <<x>> :: Nil => }
      |}
    """.stripMargin,
-    "val x: Int"
+    "Int"


Blocked by scalameta/scalameta#1503

olafurpg · 2018-04-21T00:26:35Z

Hover seems to be working OK-ish now

olafurpg · 2018-04-21T00:35:46Z

OK, this is ready for another round of review. All blocking issues should be addressed now.

ShaneDelmore

Overall I’m a big 👍 on this PR. Feedback is small, I would always like to see more issues vs TODOs (we called them TODONTs in my last gig 😀 ) and continue to dislike the number of times we call Option.get but that may just be a fact of life with Protobuf.

ShaneDelmore · 2018-04-21T01:10:33Z

metals/src/main/scala/scala/meta/metals/MetalsServices.scala

@@ -493,10 +503,11 @@ class MetalsServices(

 object MetalsServices extends LazyLogging {
  lazy val cacheDirectory: AbsolutePath = {
+    val cacheVersion = "0.1"


Do we just bump this every time we change format?

Correct, I moved this variable to Mtags, added a detailed docstring and changed the number to "1" so upgrades are a simple increment. No need for semantic versioning

ShaneDelmore · 2018-04-21T01:15:17Z

metals/src/main/scala/scala/meta/metals/compiler/MetacpProvider.scala

+        )
+    }
+  }
+}


This whole file was a pleasure to read.

ShaneDelmore · 2018-04-21T01:16:33Z

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))


I saw you and @gabro discussing earlier but don’t remember the result. Do we have a convention to avoid ._N methods? I personally don’t like the readability here as it makes it difficult to see that we are falling back to Global.

Good point, I agree _N methods are hard to read. Updated

this might actually be a decent linter rule "Introduce a case class or use pattern matching instead! Overuse of tuples is a code smell"

ShaneDelmore · 2018-04-21T01:31:34Z

metals/src/main/scala/scalafix/languageserver/ScalafixPatchEnrichments.scala


  import scalafix.patch.TokenPatch._
+  // TODO(olafur): make this method public in scalafix API


How about a ticket for this instead of a todo?

ShaneDelmore · 2018-04-21T01:34:21Z

metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala

      Defn.Trait(mods, Type.Name(info.name), Nil, EmptyCtor, EmptyTemplate)
    } else if (info.kind.isClass) {
+      // TODO: include type parameters and primary constructor


I worry about TODO vs tickets. Consider creating issues for these and referencing the issue in the TODO comment.

ShaneDelmore · 2018-04-21T01:39:15Z

metals/src/main/scala/scala/meta/metals/providers/HoverProvider.scala

-      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


Does this mean if t is tagged as a method type that we know that both get calls will succeed here? Is this more protobuf influence? The number of calls we make to get is a continual source of concern for me.

ShaneDelmore · 2018-04-21T01:40:53Z

metals/src/main/scala/scala/meta/metals/Semanticdbs.scala

+      } catch {
+        case _: InvalidProtocolBufferException =>
+          logger.error(
+            s"Have you upgraded to SemanticDB v3? Error parsing $semanticdbPath"


Does the protobuf have a specific version field we can check?

After SemanticDB v3 there is TextDocument.schema :) I tried to implement a smarter thing to detect old payloads but saw the logic would have to be in the critical path, so I opted to catch exceptions instead.

olafurpg

Thanks for the review @ShaneDelmore I updated the PR and added several more cleanups

olafurpg · 2018-04-21T13:03:50Z

metals/src/main/scala/scala/meta/metals/MetalsServices.scala

@@ -493,10 +503,11 @@ class MetalsServices(

 object MetalsServices extends LazyLogging {
  lazy val cacheDirectory: AbsolutePath = {
+    val cacheVersion = "0.1"


Correct, I moved this variable to Mtags, added a detailed docstring and changed the number to "1" so upgrades are a simple increment. No need for semantic versioning

olafurpg · 2018-04-21T13:08:01Z

metals/src/main/scala/scala/meta/metals/Semanticdbs.scala

+      } catch {
+        case _: InvalidProtocolBufferException =>
+          logger.error(
+            s"Have you upgraded to SemanticDB v3? Error parsing $semanticdbPath"


After SemanticDB v3 there is TextDocument.schema :) I tried to implement a smarter thing to detect old payloads but saw the logic would have to be in the critical path, so I opted to catch exceptions instead.

olafurpg · 2018-04-21T13:08:43Z

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))


Good point, I agree _N methods are hard to read. Updated

olafurpg · 2018-04-21T13:09:57Z

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))


this might actually be a decent linter rule "Introduce a case class or use pattern matching instead! Overuse of tuples is a code smell"

laughedelic · 2018-04-21T22:59:05Z

I switched to this branch, cleaned all cache, did metalsSetup, started server, compiled, restarted server. And it mostly works, but on some symbols I see this log message (and no tooltip):

00:51:56.172 INFO  s.m.m.s.InMemorySymbolIndex - 
Matching symbol scala.meta.metals.MetalsServices#latestConfig.`<error: <none>>`#

And then I hit on this:

00:54:34.352 ERROR s.m.m.p.HoverProvider$ - Failed to pretty print symbol scala.meta.metals.MetalsServices#x$4.configurationPublisher.
Classpath=scala.meta.metals.compiler.SymtabProvider$$anon$1@1b36ab56
symbol: "scala.meta.metals.MetalsServices#x$4.configurationPublisher."
kind: LOCAL
properties: 1024
name: "configurationPublisher"
tpe {
  tag: TYPE_REF
  typeRef {
    symbol: "monix.reactive.Observable#"
    type_arguments {
      tag: TYPE_REF
      typeRef {
        symbol: "scala.meta.metals.Configuration#"
      }
    }
  }
}
accessibility {
  tag: PUBLIC
  symbol: ""
}
owner: "scala.meta.metals.MetalsServices#x$4."
language: SCALA

Trace: `java.util.NoSuchElementException: scala.meta.metals.Configuration#`

java.util.NoSuchElementException: scala.meta.metals.Configuration#
	at scalafix.internal.util.PrettyType.$anonfun$info$1(PrettyType.scala:131)
	at scala.Option.getOrElse(Option.scala:121)
	at scalafix.internal.util.PrettyType.scalafix$internal$util$PrettyType$$info(PrettyType.scala:131)
	at scalafix.internal.util.PrettyType.toType(PrettyType.scala:526)
	at scalafix.internal.util.PrettyType.$anonfun$toType$1(PrettyType.scala:505)
	at scala.collection.Iterator$$anon$10.next(Iterator.scala:448)
	at scala.collection.Iterator.foreach(Iterator.scala:929)
	at scala.collection.Iterator.foreach$(Iterator.scala:929)
	at scala.collection.AbstractIterator.foreach(Iterator.scala:1417)
	at scala.collection.generic.Growable.$plus$plus$eq(Growable.scala:59)
	at scala.collection.generic.Growable.$plus$plus$eq$(Growable.scala:50)
	at scala.collection.mutable.ListBuffer.$plus$plus$eq(ListBuffer.scala:186)
	at scala.collection.mutable.ListBuffer.$plus$plus$eq(ListBuffer.scala:44)
	at scala.collection.TraversableOnce.to(TraversableOnce.scala:310)
	at scala.collection.TraversableOnce.to$(TraversableOnce.scala:308)
	at scala.collection.AbstractIterator.to(Iterator.scala:1417)
	at scala.collection.TraversableOnce.toList(TraversableOnce.scala:294)
	at scala.collection.TraversableOnce.toList$(TraversableOnce.scala:294)
	at scala.collection.AbstractIterator.toList(Iterator.scala:1417)
	at scalafix.internal.util.PrettyType.targs$1(PrettyType.scala:497)
	at scalafix.internal.util.PrettyType.toType(PrettyType.scala:530)
	at scalafix.internal.util.PrettyType$.toType(PrettyType.scala:33)
	at scala.meta.metals.providers.HoverProvider$.unsafeToTree(HoverProvider.scala:85)
	at scala.meta.metals.providers.HoverProvider$.toTree(HoverProvider.scala:36)
	at scala.meta.metals.providers.HoverProvider$.$anonfun$hover$4(HoverProvider.scala:128)
	at scala.Option.flatMap(Option.scala:171)
	at scala.meta.metals.providers.HoverProvider$.$anonfun$hover$3(HoverProvider.scala:127)
	at scala.Option.flatMap(Option.scala:171)
	at scala.meta.metals.providers.HoverProvider$.hover(HoverProvider.scala:125)
	at scala.meta.metals.MetalsServices.$anonfun$services$23(MetalsServices.scala:331)
	at org.langmeta.jsonrpc.Services$$anon$3.$anonfun$handle$8(Services.scala:86)
	at monix.eval.internal.TaskRunLoop$.monix$eval$internal$TaskRunLoop$$loop$1(TaskRunLoop.scala:187)
	at monix.eval.internal.TaskRunLoop$RestartCallback$1.onSuccess(TaskRunLoop.scala:119)
	at monix.eval.Task$.$anonfun$forkedUnit$2(Task.scala:1463)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:748)

olafurpg · 2018-04-21T23:04:29Z

Thanks for trying it out @laughedelic ! That particular "failed to pretty print" error is caused by scalameta/scalameta#1504 The symbol table does not have an entry for Configuration because it's annotated with @JsonCodec. It should be a simple fix in metac.

The first error Matching symbol scala.meta.metals.MetalsServices#latestConfig.<error: ># seems related to the presentation compiler producing error types in code that does not type-check.

olafurpg · 2018-04-21T23:05:24Z

I'm not sure if it's a good idea to tone down the logs to skip such cases 🤔

When a file contains deprecation messages, semanticdb-scalac appends an empty TextDocument containing only the diagnostics section.

* Disable scalafix * Remove trailing comma * Reformat * Fix scripted tests * Remove unused scalapb build integration

- setup metacp integration - setup symbol table integration - adjust hover expected output

The latest release exposes a better API that made it possible to remove almost all of copy-pasted code from scalafix internals.

- Include JDK classpath - Document blocking issues

SemanticDB v2 payloads will now cause an error to be reported.in the logs.

- Document Mtags version number - Replace TODO with tickets - Simplify handling of class info type in hover - Include primary constructor for hover

olafurpg · 2018-05-06T12:42:45Z

I've rebased on top of master. Please refrain from refactoring the codebase to avoid further rebase issues. I haven't been able to address the remaining blocking issues since I've been traveling quite a lot past weeks and the upstream fixes require more focused work than what I can do on an airplane.

olafurpg · 2018-05-06T15:55:56Z

I am fine to keep this PR open longer, I don't mind rebasing on top of master for new changes. My comment above was primarily about refactorings that move stuff around because that unnecessarily conflicts with changes in this PR.

olafurpg · 2018-05-06T15:59:19Z

I don't think we should merge this PR for now. As it is, merging would be a regression and the upgrade doesn't bring immediate benefits feature wise. A better time would be when a stable Scalafix v0.6 is out since then it will arguable be a huge simplification that we no longer have to deal with toDb/toSchema conversions which will be a nice code health improvement.

olafurpg · 2018-05-06T16:03:12Z

The primary tickets blocking this change are

Metac should emit global symbol definitions even if they don't have occurrences scalameta#1504 to support hover+definition for macro annotation classes. Most important, not a trivial fix but totally doable.
Scalacp emits wrong symbols for Java references scalameta#1491 to support hover on method signatures with parameter types that are java inner classes. Appeared frequently when testing against the full scalafix classpath.

olafurpg · 2018-06-20T20:08:10Z

I haven't forgotten this PR! There's been a lot of activity in scalameta/scalameta recently to address the remaining issues blocking this PR. For example, we merged a PR today addressing problem related to incremental compilation scalameta/scalameta#1612 There remain 26 open issues in the v4.0 milestone https://github.com/scalameta/scalameta/milestone/26 and 70 issues have already been closed. The deadline for release is July 9th. I'm gonna close this PR until then :)

olafurpg · 2018-07-03T14:26:53Z

A PR fixing java.util.NoSuchElementException: scala.meta.metals.Configuration# is finally up 🎉 scalameta/scalameta#1656 Scalameta v4.0 is getting closer to being complete 💪

olafurpg mentioned this pull request Apr 9, 2018

let AbsolutePath do the Uri encoding #277

Closed

ShaneDelmore reviewed Apr 10, 2018

View reviewed changes

gabro reviewed Apr 10, 2018

View reviewed changes

gabro requested changes Apr 10, 2018

View reviewed changes

gabro mentioned this pull request Apr 10, 2018

Java Mtags fails to index ParserConstants.java in the JDK #280

Closed

gabro reviewed Apr 10, 2018

View reviewed changes

olafurpg mentioned this pull request Apr 10, 2018

Re-enable scalafix in the build #283

Closed

olafurpg mentioned this pull request Apr 15, 2018

Added disambiguator to methods #286

Closed

olafurpg force-pushed the 3.7 branch from d6d4e41 to 27b3003 Compare April 20, 2018 23:38

olafurpg changed the title ~~WIP Upgrade to Scalameta v3.7~~ Upgrade to Scalameta v3.7 Apr 20, 2018

olafurpg commented Apr 21, 2018

View reviewed changes

olafurpg requested review from xeno-by, gabro, laughedelic and ShaneDelmore April 21, 2018 00:35

ShaneDelmore approved these changes Apr 21, 2018

View reviewed changes

olafurpg mentioned this pull request Apr 21, 2018

load-plugin integration #287

Merged

4 tasks

olafurpg commented Apr 21, 2018

View reviewed changes

olafurpg mentioned this pull request Apr 22, 2018

Emit method overload symbol in Scala mtags #282

Closed

olafurpg and others added 11 commits May 6, 2018 14:20

Don't index message-only documents.

afa835e

When a file contains deprecation messages, semanticdb-scalac appends an empty TextDocument containing only the diagnostics section.

Fix mtags tests

b37d634

Fix remaining test cases.

d413039

Fix remaining deprecation warnings

15b667a

Address comment

3f8c0b7

Fix CI failures

63487bf

* Disable scalafix * Remove trailing comma * Reformat * Fix scripted tests * Remove unused scalapb build integration

Upgrade scalafix dependency

45abbe3

Handle method overloads in Scala mtags.

df272a6

Handle conflicting method overlaods.

bce8dd9

Emit method symbols for val/var to be compliant with spec

ed9bb9d

Emit correct disambiguator for singleton type

3fd7262

olafurpg mentioned this pull request May 6, 2018

Move provider tests into provider package #300

Merged

olafurpg added 7 commits May 6, 2018 14:24

Use scalafix PrettyType for HoverProvider

7a3e0bf

- setup metacp integration - setup symbol table integration - adjust hover expected output

Fix scalafix integration.

525039c

The latest release exposes a better API that made it possible to remove almost all of copy-pasted code from scalafix internals.

Polish new Hover implementation

1f6ec39

- Include JDK classpath - Document blocking issues

Add more relevant tests for hover

b20a6c9

Document hover requires metalsSetup

317fd60

Log slightly more helpful error message on failure to parse proto file

0c9d0fc

SemanticDB v2 payloads will now cause an error to be reported.in the logs.

Address review feedback and polish.

42765d7

- Document Mtags version number - Replace TODO with tickets - Simplify handling of class info type in hover - Include primary constructor for hover

olafurpg force-pushed the 3.7 branch from 2085102 to 42765d7 Compare May 6, 2018 12:40

laughedelic approved these changes May 6, 2018

View reviewed changes

olafurpg closed this Jun 20, 2018

olafurpg mentioned this pull request Jun 29, 2018

Upgrade to Scalameta v4.0 #239

Closed

3 tasks

olafurpg deleted the 3.7 branch November 30, 2018 10:39


		import scalafix.patch.TokenPatch._
		// TODO(olafur): make this method public in scalafix API

Upgrade to Scalameta v3.7 #275

Upgrade to Scalameta v3.7 #275

Conversation

olafurpg commented Apr 9, 2018 • edited Loading

xeno-by commented Apr 9, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabro Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

gabro Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gabro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xeno-by Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg commented Apr 10, 2018

olafurpg commented Apr 10, 2018

gabro Apr 10, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg commented Apr 10, 2018

olafurpg commented Apr 12, 2018 • edited Loading

olafurpg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg commented Apr 21, 2018

olafurpg commented Apr 21, 2018

ShaneDelmore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olafurpg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laughedelic commented Apr 21, 2018

olafurpg commented Apr 21, 2018

olafurpg commented Apr 21, 2018

olafurpg commented May 6, 2018

olafurpg commented May 6, 2018

olafurpg commented May 6, 2018

olafurpg commented May 6, 2018

olafurpg commented Jun 20, 2018

olafurpg commented Jul 3, 2018

olafurpg commented Apr 9, 2018 •

edited

Loading

xeno-by commented Apr 9, 2018 •

edited

Loading

gabro Apr 10, 2018 •

edited

Loading

gabro Apr 10, 2018 •

edited

Loading

xeno-by Apr 10, 2018 •

edited

Loading

gabro Apr 10, 2018 •

edited

Loading

olafurpg commented Apr 12, 2018 •

edited

Loading