From dc1e35a829b24240044d096b18e1c614a60ed6ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C4=99drzej=20Rochala?= <48657087+rochala@users.noreply.github.com> Date: Thu, 2 Nov 2023 17:42:03 +0100 Subject: [PATCH] Completions should prepend, not replace as it is for Scala 2 (#18803) By default, in Scala 2 completion prepend and only replace if there is exact match. This PR unifies the experience between the Scala versions. It seems more intuitive to work that way. The change will be as follows (the order is: new logic, old logic and scala 2 logic): https://github.com/lampepfl/dotty/assets/48657087/c037c322-5613-4b95-a6e5-090b4e8827b6 In the future, it should be improved by changing implementation to use `InsertReplaceEdit` instead of `TextEdit`. This will allow users to decide which behaviour they prefer, but this has to be first implemented in metals to properly handle the new logic, but for now the key is to unify behaviours. https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#insertReplaceEdit --- .../tools/pc/completions/CompletionPos.scala | 5 +-- .../pc/completions/CompletionProvider.scala | 27 +++++------- .../pc/tests/completion/CompletionSuite.scala | 44 +++++++++++++++++++ 3 files changed, 57 insertions(+), 19 deletions(-) diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionPos.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionPos.scala index 9ce7939c10fa..29699bd05203 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionPos.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionPos.scala @@ -29,10 +29,9 @@ case class CompletionPos( ): def sourcePos: SourcePosition = cursorPos.withSpan(Spans.Span(start, end)) + def stripSuffixEditRange: l.Range = new l.Range(cursorPos.offsetToPos(start), cursorPos.offsetToPos(end)) + def toEditRange: l.Range = cursorPos.withStart(start).withEnd(cursorPos.point).toLsp - def toEditRange: l.Range = - new l.Range(cursorPos.offsetToPos(start), cursorPos.offsetToPos(end)) - end toEditRange end CompletionPos object CompletionPos: diff --git a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala index 78f4affe8c49..323f63050377 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/completions/CompletionProvider.scala @@ -151,10 +151,7 @@ class CompletionProvider( indexedContext: IndexedContext )(using ctx: Context): CompletionItem = val printer = - ShortenedTypePrinter(search, IncludeDefaultParam.ResolveLater)(using - indexedContext - ) - val editRange = completionPos.toEditRange + ShortenedTypePrinter(search, IncludeDefaultParam.ResolveLater)(using indexedContext) // For overloaded signatures we get multiple symbols, so we need // to recalculate the description @@ -165,24 +162,22 @@ class CompletionProvider( val ident = completion.insertText.getOrElse(completion.label) def mkItem( - insertText: String, + newText: String, additionalEdits: List[TextEdit] = Nil, range: Option[LspRange] = None ): CompletionItem = - val nameEdit = new TextEdit( - range.getOrElse(editRange), - insertText - ) + val oldText = params.text.substring(completionPos.start, completionPos.end) + val editRange = if newText.startsWith(oldText) then completionPos.stripSuffixEditRange + else completionPos.toEditRange + + val textEdit = new TextEdit(range.getOrElse(editRange), newText) + val item = new CompletionItem(label) item.setSortText(f"${idx}%05d") item.setDetail(description) - item.setFilterText( - completion.filterText.getOrElse(completion.label) - ) - item.setTextEdit(nameEdit) - item.setAdditionalTextEdits( - (completion.additionalEdits ++ additionalEdits).asJava - ) + item.setFilterText(completion.filterText.getOrElse(completion.label)) + item.setTextEdit(textEdit) + item.setAdditionalTextEdits((completion.additionalEdits ++ additionalEdits).asJava) completion.insertMode.foreach(item.setInsertTextMode) completion diff --git a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala index 213dd7157293..a64a6dfac6a2 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/completion/CompletionSuite.scala @@ -1500,3 +1500,47 @@ class CompletionSuite extends BaseCompletionSuite: |""".stripMargin, ) + @Test def `prepend-instead-of-replace` = + checkEdit( + """|object O: + | printl@@println() + |""".stripMargin, + """|object O: + | printlnprintln() + |""".stripMargin, + assertSingleItem = false + ) + + @Test def `prepend-instead-of-replace-duplicate-word` = + checkEdit( + """|object O: + | println@@println() + |""".stripMargin, + """|object O: + | printlnprintln() + |""".stripMargin, + assertSingleItem = false + ) + + @Test def `replace-when-inside` = + checkEdit( + """|object O: + | print@@ln() + |""".stripMargin, + """|object O: + | println() + |""".stripMargin, + assertSingleItem = false + ) + + @Test def `replace-exact-same` = + checkEdit( + """|object O: + | println@@() + |""".stripMargin, + """|object O: + | println() + |""".stripMargin, + assertSingleItem = false + ) +