Respect client capabilities for completion item snippets (#1057)

Respect client capabilities for completion item snippets
scalameta · Nov 11, 2019 · 86a09f3 · 86a09f3
2 parents b3ea857 + 03c3f68
commit 86a09f3
Show file tree

Hide file tree

Showing 9 changed files with 149 additions and 20 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
@@ -520,6 +520,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
@@ -394,7 +394,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
@@ -70,6 +70,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
@@ -44,6 +44,8 @@ class CompletionProvider(
     )
     val pos = unit.position(params.offset)
     val isSnippet = isSnippetEnabled(pos, params.text())
+    val clientSupportsSnippets =
+      compiler.metalsConfig.isCompletionSnippetsEnabled()
     val (i, completion, editRange, query) = safeCompletionsAt(pos)
     val start = inferIdentStart(pos, params.text())
     val end = inferIdentEnd(pos, params.text())
@@ -88,13 +90,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 +113,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 +157,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/Completions.scala b/mtags/src/main/scala/scala/meta/internal/pc/Completions.scala
@@ -22,6 +22,9 @@ import scala.collection.immutable.Nil
  */
 trait Completions { this: MetalsGlobal =>
 
+  val clientSupportsSnippets: Boolean =
+    metalsConfig.isCompletionSnippetsEnabled()
+
   /**
    * A member for symbols on the classpath that are not in scope, produced via workspace/symbol.
    */
@@ -768,11 +771,11 @@ trait Completions { this: MetalsGlobal =>
           if (text.charAt(lit.pos.start - 1) != 's')
             List(new l.TextEdit(lit.pos.withEnd(lit.pos.start).toLSP, "s"))
           else Nil
-        val dolarEdits = for {
+        val dollarEdits = for {
           i <- lit.pos.start to (lit.pos.end - CURSOR.length())
           if text.charAt(i) == '$' && i != interpolator.dollar
         } yield new l.TextEdit(pos.source.position(i).withEnd(i).toLSP, "$")
-        interpolatorEdit ++ dolarEdits
+        interpolatorEdit ++ dollarEdits
       }
 
       def newText(sym: Symbol): String = {
@@ -796,6 +799,7 @@ trait Completions { this: MetalsGlobal =>
 
       val filter: String =
         text.substring(lit.pos.start, pos.point - interpolator.name.length)
+
       override def contribute: List[Member] = {
         metalsScopeMembers(pos).collect {
           case s: ScopeMember
@@ -988,7 +992,7 @@ trait Completions { this: MetalsGlobal =>
           allParams.exists(param => param.name.startsWith(prefix))
         val isExplicitlyCalled = suffix.startsWith(prefix)
         val hasParamsToFill = allParams.count(!_.hasDefault) > 1
-        if ((shouldShow || isExplicitlyCalled) && hasParamsToFill) {
+        if ((shouldShow || isExplicitlyCalled) && hasParamsToFill && clientSupportsSnippets) {
           val editText = allParams.zipWithIndex
             .collect {
               case (param, index) if !param.hasDefault =>
@@ -1206,7 +1210,11 @@ trait Completions { this: MetalsGlobal =>
         private def signature = printer.defaultMethodSignature()
         private def edit = new l.TextEdit(
           range,
-          s"$filterText$signature = $${0:???}"
+          if (clientSupportsSnippets) {
+            s"$filterText$signature = $${0:???}"
+          } else {
+            s"$filterText$signature = ???"
+          }
         )
       }
 
@@ -1311,7 +1319,11 @@ trait Completions { this: MetalsGlobal =>
           "match",
           new l.TextEdit(
             editRange,
-            "match {\n\tcase$0\n}"
+            if (clientSupportsSnippets) {
+              "match {\n\tcase$0\n}"
+            } else {
+              "match"
+            }
           ),
           completionsSymbol("match"),
           label = Some("match"),
@@ -1325,7 +1337,11 @@ trait Completions { this: MetalsGlobal =>
               tail
                 .map(_.edit.getNewText())
                 .mkString(
-                  s"match {\n\t${head.edit.getNewText} $$0\n\t",
+                  if (clientSupportsSnippets) {
+                    s"match {\n\t${head.edit.getNewText} $$0\n\t"
+                  } else {
+                    s"match {\n\t${head.edit.getNewText}\n\t"
+                  },
                   "\n\t",
                   "\n}"
                 )
@@ -1460,7 +1476,10 @@ trait Completions { this: MetalsGlobal =>
         if (definitions.isTupleType(parents.selector)) {
           result += new TextEditMember(
             "case () =>",
-            new l.TextEdit(editRange, "case ($0) =>"),
+            new l.TextEdit(
+              editRange,
+              if (clientSupportsSnippets) "case ($0) =>" else "case () =>"
+            ),
             parents.selector.typeSymbol,
             label = Some(s"case ${parents.selector} =>"),
             command = metalsConfig.parameterHintsCommand().asScala
@@ -1518,8 +1537,10 @@ trait Completions { this: MetalsGlobal =>
         val label = s"case $pattern =>"
         new TextEditMember(
           filterText = label,
-          edit =
-            new l.TextEdit(editRange, label + (if (isSnippet) " $0" else "")),
+          edit = new l.TextEdit(
+            editRange,
+            label + (if (isSnippet && clientSupportsSnippets) " $0" else "")
+          ),
           sym = sym,
           label = Some(label),
           additionalTextEdits = autoImports
@@ -1534,7 +1555,8 @@ trait Completions { this: MetalsGlobal =>
           s"case _: $name",
           new l.TextEdit(
             editRange,
-            if (isSnippet) s"case $${0:_}: $name$suffix => "
+            if (isSnippet && clientSupportsSnippets)
+              s"case $${0:_}: $name$suffix => "
             else s"case _: $name$suffix =>"
           ),
           sym,

diff --git a/mtags/src/main/scala/scala/meta/internal/pc/MetalsGlobal.scala b/mtags/src/main/scala/scala/meta/internal/pc/MetalsGlobal.scala
@@ -573,11 +573,11 @@ class MetalsGlobal(
 
     def snippetCursor: String = sym.paramss match {
       case Nil =>
-        "$0"
+        if (clientSupportsSnippets) "$0" else ""
       case Nil :: Nil =>
-        "()$0"
+        if (clientSupportsSnippets) "()$0" else "()"
       case _ =>
-        "($0)"
+        if (clientSupportsSnippets) "($0)" else ""
     }
 
     def isDefined: Boolean =

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

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