Respect client capabilities for completion item snippets

scalameta · Nov 10, 2019 · 2c40a21 · 2c40a21
1 parent f54ca49
commit 2c40a21
Show file tree

Hide file tree

Showing 8 changed files with 116 additions and 9 deletions.
diff --git a/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala b/metals/src/main/scala/scala/meta/internal/metals/Compilers.scala
@@ -6,6 +6,7 @@ import ch.epfl.scala.bsp4j.ScalaBuildTarget
 import ch.epfl.scala.bsp4j.ScalacOptionsItem
 import java.util.Collections
 import java.util.concurrent.ScheduledExecutorService
+import org.eclipse.lsp4j.InitializeParams
 import org.eclipse.lsp4j.CompletionItem
 import org.eclipse.lsp4j.CompletionList
 import org.eclipse.lsp4j.CompletionParams
@@ -40,7 +41,8 @@ class Compilers(
     search: SymbolSearch,
     embedded: Embedded,
     statusBar: StatusBar,
-    sh: ScheduledExecutorService
+    sh: ScheduledExecutorService,
+    initializeParams: Option[InitializeParams]
 )(implicit ec: ExecutionContextExecutorService)
     extends Cancelable {
   val plugins = new CompilerPlugins()
@@ -247,7 +249,11 @@ class Compilers(
       .withExecutorService(ec)
       .withScheduledExecutorService(sh)
       .withConfiguration(
-        config.compilers.copy(_symbolPrefixes = userConfig().symbolPrefixes)
+        config.compilers.copy(
+          _symbolPrefixes = userConfig().symbolPrefixes,
+          isCompletionSnippetsEnabled =
+            initializeParams.supportsCompletionSnippets
+        )
       )
 
   def newCompiler(

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsEnrichments.scala
@@ -509,6 +509,16 @@ object MetalsEnrichments
         )
       } yield hierarchicalDocumentSymbolSupport.booleanValue).getOrElse(false)
 
+    def supportsCompletionSnippets: Boolean =
+      (for {
+        params <- initializeParams
+        capabilities <- Option(params.getCapabilities)
+        textDocument <- Option(capabilities.getTextDocument)
+        completion <- Option(textDocument.getCompletion)
+        completionItem <- Option(completion.getCompletionItem)
+        snippetSupport <- Option(completionItem.getSnippetSupport())
+      } yield snippetSupport.booleanValue).getOrElse(false)
+
   }
 
   implicit class XtensionPromise[T](promise: Promise[T]) {

diff --git a/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala b/metals/src/main/scala/scala/meta/internal/metals/MetalsLanguageServer.scala
@@ -381,7 +381,8 @@ class MetalsLanguageServer(
         ),
         embedded,
         statusBar,
-        sh
+        sh,
+        Option(params)
       )
     )
     doctor = new Doctor(

diff --git a/mtags-interfaces/src/main/java/scala/meta/pc/PresentationCompilerConfig.java b/mtags-interfaces/src/main/java/scala/meta/pc/PresentationCompilerConfig.java
@@ -64,6 +64,11 @@ enum OverrideDefFormat {
      */
     boolean isSignatureHelpDocumentationEnabled();
 
+    /**
+     * Returns true if completions can contain snippets.
+     */
+    boolean isCompletionSnippetsEnabled();
+
     /**
      * The maximum delay for requests to respond.
      *

diff --git a/mtags/src/main/scala/scala/meta/internal/pc/CompletionProvider.scala b/mtags/src/main/scala/scala/meta/internal/pc/CompletionProvider.scala
@@ -16,7 +16,8 @@ import org.eclipse.{lsp4j => l}
 
 class CompletionProvider(
     val compiler: MetalsGlobal,
-    params: OffsetParams
+    params: OffsetParams,
+    clientSupportsSnippets: Boolean
 ) {
   import compiler._
 
@@ -88,13 +89,13 @@ class CompletionProvider(
           item.setDetail(detail)
         }
         val templateSuffix =
-          if (!isSnippet) ""
+          if (!isSnippet || !clientSupportsSnippets) ""
           else if (completion.isNew &&
             r.sym.dealiased.requiresTemplateCurlyBraces) " {}"
           else ""
 
         val typeSuffix =
-          if (!isSnippet) ""
+          if (!isSnippet || !clientSupportsSnippets) ""
           else if (completion.isType && r.sym.hasTypeParams) "[$0]"
           else if (completion.isNew && r.sym.hasTypeParams) "[$0]"
           else ""
@@ -111,7 +112,11 @@ class CompletionProvider(
             item.setFilterText(symbolName)
         }
 
-        item.setInsertTextFormat(InsertTextFormat.Snippet)
+        if (clientSupportsSnippets) {
+          item.setInsertTextFormat(InsertTextFormat.Snippet)
+        } else {
+          item.setInsertTextFormat(InsertTextFormat.PlainText)
+        }
 
         r match {
           case i: TextEditMember =>
@@ -151,7 +156,9 @@ class CompletionProvider(
                 case head :: Nil if head.forall(_.isImplicit) =>
                   () // Don't set ($0) snippet for implicit-only params.
                 case _ =>
-                  item.setTextEdit(textEdit(baseLabel + "($0)"))
+                  if (clientSupportsSnippets) {
+                    item.setTextEdit(textEdit(baseLabel + "($0)"))
+                  }
                   metalsConfig
                     .parameterHintsCommand()
                     .asScala

diff --git a/mtags/src/main/scala/scala/meta/internal/pc/PresentationCompilerConfigImpl.scala b/mtags/src/main/scala/scala/meta/internal/pc/PresentationCompilerConfigImpl.scala
@@ -18,6 +18,7 @@ case class PresentationCompilerConfigImpl(
     isCompletionItemDocumentationEnabled: Boolean = true,
     isHoverDocumentationEnabled: Boolean = true,
     isSignatureHelpDocumentationEnabled: Boolean = true,
+    isCompletionSnippetsEnabled: Boolean = true,
     isCompletionItemResolve: Boolean = true,
     timeoutDelay: Long = 20,
     timeoutUnit: TimeUnit = TimeUnit.SECONDS

diff --git a/mtags/src/main/scala/scala/meta/internal/pc/ScalaPresentationCompiler.scala b/mtags/src/main/scala/scala/meta/internal/pc/ScalaPresentationCompiler.scala
@@ -85,7 +85,8 @@ case class ScalaPresentationCompiler(
       params: OffsetParams
   ): CompletableFuture[CompletionList] =
     access.withInterruptableCompiler(emptyCompletion, params.token) { global =>
-      new CompletionProvider(global, params).completions()
+      new CompletionProvider(global, params, config.isCompletionSnippetsEnabled)
+        .completions()
     }
 
   // NOTE(olafur): hover and signature help use a "shared" compiler instance because

diff --git a/tests/cross/src/test/scala/tests/pc/CompletionSnippetNegSuite.scala b/tests/cross/src/test/scala/tests/pc/CompletionSnippetNegSuite.scala
@@ -0,0 +1,76 @@
+package tests.pc
+
+import tests.BaseCompletionSuite
+import scala.meta.pc.PresentationCompilerConfig
+import scala.meta.internal.pc.PresentationCompilerConfigImpl
+
+object CompletionSnippetNegSuite extends BaseCompletionSuite {
+
+  override def config: PresentationCompilerConfig =
+    PresentationCompilerConfigImpl(
+      isCompletionSnippetsEnabled = false
+    )
+
+  checkSnippet(
+    "member",
+    """
+      |object Main {
+      |  List.appl@@
+      |}
+      |""".stripMargin,
+    """|apply
+       |unapplySeq
+       |""".stripMargin,
+    compat = Map(
+      "2.13" ->
+        // the second apply is from scala/collection/BuildFrom#apply(), introduced in 2.13
+        """|apply
+           |unapplySeq
+           |apply
+           |""".stripMargin
+    )
+  )
+
+  checkSnippet(
+    "scope",
+    """
+      |object Main {
+      |  printl@@
+      |
+      |}
+      |""".stripMargin,
+    """|println()
+       |println
+       |""".stripMargin
+  )
+
+  checkSnippet(
+    "java-nullary",
+    """
+      |class Foo {
+      |  override def toString = "Foo"
+      |}
+      |object Main {
+      |  new Foo().toStrin@@
+      |
+      |}
+      |""".stripMargin,
+    // even if `Foo.toString` is nullary, it overrides `Object.toString()`
+    // which is a Java non-nullary method with an empty parameter list.
+    """|toString()
+       |""".stripMargin
+  )
+
+  checkSnippet(
+    "type",
+    s"""|object Main {
+        |  val x: scala.IndexedSe@@
+        |}
+        |""".stripMargin,
+    // It's expected to have two separate results, one for `object IndexedSeq` and one for `type IndexedSeq[T]`.
+    """|IndexedSeq
+       |IndexedSeq
+       |""".stripMargin
+  )
+
+}