improvement: Fallback to last successful tokens instead of throwing

It seems if we throw too much this will cause the extension to think metals is unresponsive. Using tokens from the last parsing code should work ok enough in that case.
scalameta · Sep 10, 2024 · 6bb4250 · 6bb4250
1 parent f699b87
commit 6bb4250
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 3 deletions.
diff --git a/metals-bench/src/main/scala/bench/SemanticTokensBench.scala b/metals-bench/src/main/scala/bench/SemanticTokensBench.scala
@@ -2,6 +2,7 @@ package bench
 
 import java.net.URI
 import java.nio.charset.StandardCharsets
+import java.nio.file.Paths
 import java.util.concurrent.TimeUnit
 
 import scala.meta.internal.io.FileIO
@@ -19,6 +20,7 @@ import org.openjdk.jmh.annotations.OutputTimeUnit
 import org.openjdk.jmh.annotations.Param
 import org.openjdk.jmh.annotations.Scope
 import org.openjdk.jmh.annotations.State
+import tests.MetalsTestEnrichments
 
 @State(Scope.Benchmark)
 class SemanticTokensBench extends PcBenchmark {
@@ -84,11 +86,12 @@ class SemanticTokensBench extends PcBenchmark {
 
     val nodes = pc.semanticTokens(vFile).get().asScala.toList
     val isScala3 = ScalaVersions.isScala3Version(pc.scalaVersion())
-
     SemanticTokensProvider.provide(
       nodes,
       vFile,
+      AbsolutePath(Paths.get(vFile.uri)),
       isScala3,
+      MetalsTestEnrichments.emptyTrees,
     )
   }
 

diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala
@@ -558,7 +558,9 @@ class Compilers(
                   SemanticTokensProvider.provide(
                     nodes.asScala.toList,
                     vFile,
+                    path,
                     isScala3,
+                    trees,
                   )
                 } catch {
                   case NonFatal(e) =>

diff --git a/metals/src/main/scala/scala/meta/internal/metals/SemanticTokensProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/SemanticTokensProvider.scala
@@ -3,11 +3,16 @@ package scala.meta.internal.metals
 import scala.annotation.switch
 import scala.annotation.tailrec
 import scala.collection.mutable.ListBuffer
+import scala.util.Failure
+import scala.util.Success
+import scala.util.Try
 import scala.util.matching.Regex
 
 import scala.meta.internal.metals.MetalsEnrichments._
 import scala.meta.internal.parsers.SoftKeywords
+import scala.meta.internal.parsing.Trees
 import scala.meta.internal.pc.SemanticTokens._
+import scala.meta.io.AbsolutePath
 import scala.meta.pc.Node
 import scala.meta.pc.VirtualFileParams
 import scala.meta.tokens._
@@ -23,7 +28,6 @@ object SemanticTokensProvider {
 
   def getTokens(isScala3: Boolean, text: String): Tokens = {
     import scala.meta._
-    // This should throw if something breaks since otherwise if we return empty everything will get unhighlighted
     if (isScala3) {
       implicit val dialect = scala.meta.dialects.Scala3
       text.tokenize.get
@@ -79,15 +83,28 @@ object SemanticTokensProvider {
   def provide(
       nodes: List[Node],
       params: VirtualFileParams,
+      path: AbsolutePath,
       isScala3: Boolean,
+      trees: Trees,
   ): List[Integer] = {
     // no semantic data was available, we can revert to default highlighting
     if (nodes.isEmpty) {
       if (params.uri().toString().isScalaFilename)
         scribe.warn("Could not find semantic tokens for: " + params.uri())
       List.empty[Integer]
     } else {
-      val tokens = getTokens(isScala3, params.text())
+      val tokens = Try(getTokens(isScala3, params.text())) match {
+        case Failure(_) =>
+          /* Try to get tokens from trees as a fallback to avoid
+           * things being unhighlighted too often.
+           */
+          trees
+            .get(path)
+            .map(_.tokens)
+            .getOrElse(Tokens(Array.empty))
+
+        case Success(tokens) => tokens
+      }
       val buffer = ListBuffer.empty[Integer]
 
       var delta = Line(0, 0)

diff --git a/tests/slow/src/test/scala/tests/feature/SemanticTokensScala3ExpectSuite.scala b/tests/slow/src/test/scala/tests/feature/SemanticTokensScala3ExpectSuite.scala
@@ -9,6 +9,7 @@ import scala.meta.internal.metals.{BuildInfo => V}
 import tests.DirectoryExpectSuite
 import tests.ExpectTestCase
 import tests.InputProperties
+import tests.MetalsTestEnrichments
 import tests.TestScala3Compiler
 import tests.TestSemanticTokens
 
@@ -36,7 +37,9 @@ class SemanticTokensScala3ExpectSuite(
           val tokens = SemanticTokensProvider.provide(
             nodes,
             params,
+            file.file,
             isScala3 = true,
+            MetalsTestEnrichments.emptyTrees,
           )
 
           TestSemanticTokens.semanticString(

diff --git a/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala b/tests/unit/src/main/scala/tests/MetalsTestEnrichments.scala
@@ -6,15 +6,21 @@ import scala.collection.mutable.ArrayBuffer
 import scala.{meta => m}
 
 import scala.meta.dialects
+import scala.meta.internal.metals.Buffers
+import scala.meta.internal.metals.BuildTargets
+import scala.meta.internal.metals.EmptyReportContext
 import scala.meta.internal.metals.JdkSources
 import scala.meta.internal.metals.Memory
 import scala.meta.internal.metals.MetalsEnrichments._
 import scala.meta.internal.metals.PositionSyntax._
+import scala.meta.internal.metals.ScalaVersionSelector
 import scala.meta.internal.metals.ScalaVersions
 import scala.meta.internal.metals.SemanticdbDefinition
+import scala.meta.internal.metals.UserConfiguration
 import scala.meta.internal.metals.WorkspaceSources
 import scala.meta.internal.metals.WorkspaceSymbolInformation
 import scala.meta.internal.metals.WorkspaceSymbolProvider
+import scala.meta.internal.parsing.Trees
 import scala.meta.internal.{semanticdb => s}
 import scala.meta.io.AbsolutePath
 import scala.meta.io.Classpath
@@ -38,6 +44,16 @@ import org.eclipse.{lsp4j => l}
  */
 object MetalsTestEnrichments {
 
+  def emptyTrees: Trees = {
+    new Trees(
+      new Buffers(),
+      new ScalaVersionSelector(
+        () => UserConfiguration.default,
+        BuildTargets.empty,
+      ),
+    )(EmptyReportContext)
+  }
+
   implicit class XtensionTestAbsolutePath(path: AbsolutePath) {
     def text: String = Files.readAllLines(path.toNIO).asScala.mkString("\n")
   }

diff --git a/tests/unit/src/test/scala/tests/SemanticTokensExpectSuite.scala b/tests/unit/src/test/scala/tests/SemanticTokensExpectSuite.scala
@@ -27,7 +27,9 @@ class SemanticTokensExpectSuite extends DirectoryExpectSuite("semanticTokens") {
           val tokens = SemanticTokensProvider.provide(
             nodes,
             params,
+            file.file,
             isScala3 = false,
+            MetalsTestEnrichments.emptyTrees,
           )
 
           TestSemanticTokens.semanticString(