From 209a2984e1f7c2915611271aaf5cb1ea6afec18a Mon Sep 17 00:00:00 2001 From: Kasper Kondzielski Date: Wed, 27 Sep 2023 10:11:29 +0200 Subject: [PATCH] Add code action to convert single line comment into multiline comment (#5633) * Create code action to convert comments * Fix case when comment does not start from the beginning of the line * Remove redundant test * Fix imports * Add negative test case * Fix case with indented part of comment * Add test for mixed style comments * Add test for mixed style comments * Further improvements * Polish code * Simplify more * Add yet another test case * Improve more * Add more test cases * Apply review suggestions * Apply review suggestions * Add support for /r/r * Don't suggest converting scala-cli directives * Don't include scala-cli directives during cluster expansion --------- Co-authored-by: ghostbuster91 --- .../codeactions/CodeActionProvider.scala | 1 + .../ConvertCommentCodeAction.scala | 161 +++++++++++ .../MillifyScalaCliDependencyCodeAction.scala | 15 +- .../ConvertSingleLineCommentLspSuite.scala | 273 ++++++++++++++++++ 4 files changed, 444 insertions(+), 6 deletions(-) create mode 100644 metals/src/main/scala/scala/meta/internal/metals/codeactions/ConvertCommentCodeAction.scala create mode 100644 tests/unit/src/test/scala/tests/codeactions/ConvertSingleLineCommentLspSuite.scala diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala index 13b3be7bff6..b3f7475b8a9 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/CodeActionProvider.scala @@ -58,6 +58,7 @@ final class CodeActionProvider( new FlatMapToForComprehensionCodeAction(trees, buffers), new MillifyDependencyCodeAction(buffers), new MillifyScalaCliDependencyCodeAction(buffers), + new ConvertCommentCodeAction(buffers), ) def codeActions( diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/ConvertCommentCodeAction.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/ConvertCommentCodeAction.scala new file mode 100644 index 00000000000..29b32154f5c --- /dev/null +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/ConvertCommentCodeAction.scala @@ -0,0 +1,161 @@ +package scala.meta.internal.metals.codeactions + +import scala.concurrent.ExecutionContext +import scala.concurrent.Future + +import scala.meta.internal.metals.Buffers +import scala.meta.internal.metals.MetalsEnrichments._ +import scala.meta.internal.metals.codeactions.CodeAction +import scala.meta.internal.metals.codeactions.MillifyScalaCliDependencyCodeAction._ +import scala.meta.internal.parsing.Trees +import scala.meta.io.AbsolutePath +import scala.meta.pc.CancelToken +import scala.meta.tokens.Token +import scala.meta.tokens.Tokens + +import org.eclipse.{lsp4j => l} + +class ConvertCommentCodeAction(buffers: Buffers) extends CodeAction { + + override def kind: String = l.CodeActionKind.RefactorRewrite + + override def contribute(params: l.CodeActionParams, token: CancelToken)( + implicit ec: ExecutionContext + ): Future[Seq[l.CodeAction]] = Future { + val path = params.getTextDocument().getUri().toAbsolutePath + val range = params.getRange() + val couldBeScalaCli = path.isScalaScript || path.isScala + + (for { + content <- buffers.get(path) + tokens <- tokenizeIfNotRangeSelection(range, content) + contentLines = content.split("\\r?(\\n|\\r)").toVector + codeAction <- createActionIfPossible( + path, + tokens, + range, + isSingleLineComment(contentLines), + isLikelyScalaCliDirective(contentLines, couldBeScalaCli), + ) + } yield codeAction).toList + } + + private def tokenizeIfNotRangeSelection(range: l.Range, content: String) = { + Option + .when(range.getStart == range.getEnd)( + Trees.defaultTokenizerDialect(content).tokenize + ) + .flatMap(_.toOption) + } + private def isLikelyScalaCliDirective( + contentLines: Vector[String], + couldBeScalaCli: Boolean, + )(t: Token) = { + t match { + case tc: Token.Comment => + val currentLine = contentLines(tc.pos.startLine) + couldBeScalaCli && isScalaCliUsingDirectiveComment(currentLine) + case _ => false + } + } + + private def isSingleLineComment( + contentLines: Vector[String] + )( + t: Token + ): Boolean = t match { + case tc: Token.Comment => + val currentLine = contentLines(t.pos.startLine) + + val tokenIsSingleLine = tc.pos.startLine == tc.pos.endLine + val tokenStartsWithDoubleSlash = + currentLine.slice(tc.pos.startColumn, tc.pos.startColumn + 2) == "//" + + tokenIsSingleLine && + tokenStartsWithDoubleSlash + case _ => false + } + + private def createActionIfPossible( + path: AbsolutePath, + tokens: Tokens, + range: l.Range, + isSingleLineComment: Token => Boolean, + isLikelyScalaCliDirective: Token => Boolean, + ): Option[l.CodeAction] = { + val indexOfLineTokenUnderCursor = tokens.lastIndexWhere(t => + t.pos.encloses(range) + && isSingleLineComment(t) + && !isLikelyScalaCliDirective(t) + ) + if (indexOfLineTokenUnderCursor != -1) { + // tokens that are strictly before cursor, i.e. they end before cursor position + val tokensBeforeCursor = tokens.take(indexOfLineTokenUnderCursor) + // token under the cursor + following tokens + val tokensAfterCursor = tokens.drop(indexOfLineTokenUnderCursor) + val textEdit = createTextEdit( + tokensBeforeCursor, + tokensAfterCursor, + range, + isLikelyScalaCliDirective, + ) + Some( + CodeActionBuilder.build( + title = ConvertCommentCodeAction.Title, + kind = this.kind, + changes = List(path -> List(textEdit)), + ) + ) + } else { + None + } + } + + private def createTextEdit( + tokensBeforeCursor: Tokens, + tokensAfterCursor: Tokens, + range: l.Range, + isLikelyScalaCliDirective: Token => Boolean, + ) = { + val commentBeforeCursor = collectContinuousComments( + tokens = tokensBeforeCursor.reverse, + isLikelyScalaCliDirective, + ).reverse + + val commentAfterCursor = collectContinuousComments( + tokens = tokensAfterCursor, + isLikelyScalaCliDirective, + ) + val commentStart = commentBeforeCursor.headOption + .map(_.pos.toLsp.getStart) + .getOrElse(tokensAfterCursor.head.pos.toLsp.getStart) + val commentEnd = commentAfterCursor.lastOption + .map(_.pos.toLsp.getEnd()) + .getOrElse(range.getEnd()) + + val commentTokens = commentBeforeCursor ++ commentAfterCursor + val replaceText = + commentTokens + .map(_.value.trim()) // TODO replace with strip once we drop jdk 8 + .mkString("/* ", "\n * ", " */") + val pos = new l.Range(commentStart, commentEnd) + new l.TextEdit(pos, replaceText) + } + + private def collectContinuousComments( + tokens: Seq[Token], + isLikelyScalaCliDirective: Token => Boolean, + ) = { + tokens + .takeWhile { + case t: Token.Trivia if !isLikelyScalaCliDirective(t) => true + case _ => false + } + .collect { case t: Token.Comment => t } + .toVector + } +} + +object ConvertCommentCodeAction { + val Title: String = "Convert to multiline comment" +} diff --git a/metals/src/main/scala/scala/meta/internal/metals/codeactions/MillifyScalaCliDependencyCodeAction.scala b/metals/src/main/scala/scala/meta/internal/metals/codeactions/MillifyScalaCliDependencyCodeAction.scala index 03439418283..a67c18beef9 100644 --- a/metals/src/main/scala/scala/meta/internal/metals/codeactions/MillifyScalaCliDependencyCodeAction.scala +++ b/metals/src/main/scala/scala/meta/internal/metals/codeactions/MillifyScalaCliDependencyCodeAction.scala @@ -118,12 +118,6 @@ object MillifyScalaCliDependencyCodeAction { private val dependencyIdentifiers = Set("dep", "lib", "plugin") private val sbtDependencyDelimiters = Set("%", "%%", "%%%") - private def isScalaCliUsingDirectiveComment(text: String): Boolean = - text.split(" ").filterNot(_.isEmpty).toList match { - case "//>" :: "using" :: _ => true - case _ => false - } - private def actionTitle(millStyleDependency: String): String = s"""Convert to "$millStyleDependency"""" @@ -134,4 +128,13 @@ object MillifyScalaCliDependencyCodeAction { val replacementText: String = s"//> using $dependencyIdentifier \"$millStyleDependency\"" } + + private[codeactions] def isScalaCliUsingDirectiveComment( + text: String + ): Boolean = + text.split(" ").filterNot(_.isEmpty).toList match { + case "//>" :: "using" :: _ => true + case _ => false + } + } diff --git a/tests/unit/src/test/scala/tests/codeactions/ConvertSingleLineCommentLspSuite.scala b/tests/unit/src/test/scala/tests/codeactions/ConvertSingleLineCommentLspSuite.scala new file mode 100644 index 00000000000..60a8701c379 --- /dev/null +++ b/tests/unit/src/test/scala/tests/codeactions/ConvertSingleLineCommentLspSuite.scala @@ -0,0 +1,273 @@ +package tests.codeactions + +import scala.meta.internal.metals.codeactions.ConvertCommentCodeAction + +class ConvertSingleLineCommentLspSuite + extends BaseCodeActionLspSuite("convertComment") { + + check( + "convert single line comment to block comment", + """val a = 1 + | + |// <<>>comment middle + | + |val b = 2 + |""".stripMargin, + ConvertCommentCodeAction.Title, + """val a = 1 + | + |/* comment middle */ + | + |val b = 2 + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert single line comment to block comment if it starts with a whitespace", + """val a = 1 + | + | // <<>>comment middle + | + |val b = 2 + |""".stripMargin, + ConvertCommentCodeAction.Title, + """val a = 1 + | + | /* comment middle */ + | + |val b = 2 + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert single line comments to block comment when part of it is indented", + """// comment start + | // <<>>comment middle + | + |val b = 2 + |""".stripMargin, + ConvertCommentCodeAction.Title, + """/* comment start + | * comment middle */ + | + |val b = 2 + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "mixed style comments gets merged", + """// <<>>comment start + |/* comment middle */ + | + |""".stripMargin, + ConvertCommentCodeAction.Title, + """/* comment start + | * comment middle */ + | + |""".stripMargin, + fileName = "script.sc", + ) + + checkNoAction( + "show no action when line comment is inside block comment", + """| + |/* comment //<<>> start */ + | + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert single line comment to block comment if it starts with var decl", + """val a = 1 + | + |val b = 2 // <<>>comment middle + |""".stripMargin, + ConvertCommentCodeAction.Title, + """val a = 1 + | + |val b = 2 /* comment middle */ + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert single line comment to block comment if it includes a block comment", + """val a = 1 + | + |// <<>> /* comment middle */ + |""".stripMargin, + ConvertCommentCodeAction.Title, + """val a = 1 + | + |/* /* comment middle */ */ + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert single line comment to block comment if the line starts with a block comment", + """val a = 1 + | + |/* comment start */ //<<>> comment end + |""".stripMargin, + ConvertCommentCodeAction.Title, + """val a = 1 + | + |/* comment start + | * comment end */ + |""".stripMargin, + fileName = "script.sc", + ) + + checkNoAction( + "show no action when cursor is before line comment", + """| + |<<>> // start + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert single line comment to block comment if it is defined between method params", + """def foo( + | name: String, // another comment + | age: Int, //<<>> important comment + | amount: Int + |) + |""".stripMargin, + ConvertCommentCodeAction.Title, + """def foo( + | name: String, // another comment + | age: Int, /* important comment */ + | amount: Int + |)""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert the whole comment cluster expanded in both ways", + """val a = 1 + |// comment start 1 + |// comment start 2 + |// <<>>comment middle + |// comment end 1 + |// comment end 2 + |val b = 2 + |""".stripMargin, + ConvertCommentCodeAction.Title, + """val a = 1 + |/* comment start 1 + | * comment start 2 + | * comment middle + | * comment end 1 + | * comment end 2 */ + |val b = 2 + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert the whole comment cluster expanded in both ways -- additional white spaces", + """val a = 1 + |// comment start 1 + | // comment start 2 + |// <<>>comment middle + |// comment end 1 + | // comment end 2 + |val b = 2 + |""".stripMargin, + ConvertCommentCodeAction.Title, + """val a = 1 + |/* comment start 1 + | * comment start 2 + | * comment middle + | * comment end 1 + | * comment end 2 */ + |val b = 2 + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert the whole comment cluster expanded in both ways -- mixed style comments", + """val a = 1 + |// comment start 1 + | /* comment start 2 */ + |// <<>>comment middle + |/* comment end 1 */ + | // comment end 2 + |val b = 2 + |""".stripMargin, + ConvertCommentCodeAction.Title, + """val a = 1 + |/* comment start 1 + | * comment start 2 + | * comment middle + | * comment end 1 + | * comment end 2 */ + |val b = 2 + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "convert the whole comment cluster expanded in both ways -- mixed with other tokens", + """val a = 1 + |// comment start 1 + | /* comment start 2 */ val b = 1 + |// <<>>comment middle + |/* comment end 1 */ val c = 2 + | // comment end 2 + |""".stripMargin, + ConvertCommentCodeAction.Title, + """|val a = 1 + |// comment start 1 + | /* comment start 2 */ val b = 1 + |/* comment middle + | * comment end 1 */ val c = 2 + | // comment end 2 + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "should not consume scala-cli directives when expanding comment cluster", + """//> using scala 3.3.1 + |//<<>>my class + |class A + |""".stripMargin, + ConvertCommentCodeAction.Title, + """//> using scala 3.3.1 + |/* my class */ + |class A + |""".stripMargin, + fileName = "script.sc", + ) + + check( + "show action if line comment is the first line in the file", + """// <<>>comment middle""", + ConvertCommentCodeAction.Title, + """/* comment middle */""", + fileName = "script.sc", + ) + + checkNoAction( + "should not show action when cursor is after block comment", + """|val a = 1 + |/* comment middle */ <<>> + |val b = 2""".stripMargin, + fileName = "script2.sc", + ) + + checkNoAction( + "should not show action when a comment is scala-cli directive", + """|//> <<>>using scala 3.3.3 + |val b = 2""".stripMargin, + fileName = "script2.sc", + ) +}