From a32d1f0c33adc6a0e8b88017b657f2a70d6fd1de Mon Sep 17 00:00:00 2001 From: Kacper Korban Date: Thu, 3 Apr 2025 19:18:24 +0200 Subject: [PATCH 1/3] pc: Properly adjust indentation when inlining blocks Co-Authored-By: Katarzyna Marek <26606662+kasiaMarek@users.noreply.github.com> Co-Authored-By: Robert Marek <64277069+susuro@users.noreply.github.com> --- .../tools/pc/PcInlineValueProviderImpl.scala | 127 +++++++++++++++++- .../pc/tests/edit/InlineValueSuite.scala | 46 +++++++ 2 files changed, 166 insertions(+), 7 deletions(-) diff --git a/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProviderImpl.scala b/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProviderImpl.scala index fc4b53e60bbd..6563c458f53f 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProviderImpl.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProviderImpl.scala @@ -14,6 +14,7 @@ import dotty.tools.dotc.core.Contexts.Context import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.core.Symbols.Symbol +import dotty.tools.dotc.util.SourceFile import dotty.tools.dotc.interactive.Interactive import dotty.tools.dotc.interactive.InteractiveDriver import dotty.tools.dotc.util.SourcePosition @@ -25,8 +26,66 @@ import org.eclipse.lsp4j as l final class PcInlineValueProviderImpl( driver: InteractiveDriver, val params: OffsetParams -) extends WithSymbolSearchCollector[Option[Occurence]](driver, params) - with InlineValueProvider: +) extends WithSymbolSearchCollector[Option[Occurence]](driver, params): + + // We return a result or an error + def getInlineTextEdits(): Either[String, List[l.TextEdit]] = + defAndRefs() match { + case Right((defn, refs)) => + val edits = + if (defn.shouldBeRemoved) { + val defEdit = definitionTextEdit(defn) + val refsEdits = refs.map(referenceTextEdit(defn)) + defEdit :: refsEdits + } else refs.map(referenceTextEdit(defn)) + Right(edits) + case Left(error) => Left(error) + } + + private def referenceTextEdit( + definition: Definition + )(ref: Reference): l.TextEdit = + if (definition.requiresBrackets && ref.requiresBrackets) + new l.TextEdit( + ref.range, + s"""(${ref.rhs})""" + ) + else new l.TextEdit(ref.range, ref.rhs) + + private def definitionTextEdit(definition: Definition): l.TextEdit = + new l.TextEdit( + extend( + definition.rangeOffsets.start, + definition.rangeOffsets.end, + definition.range + ), + "" + ) + + private def extend( + startOffset: Int, + endOffset: Int, + range: l.Range + ): l.Range = { + val (startWithSpace, endWithSpace): (Int, Int) = + extendRangeToIncludeWhiteCharsAndTheFollowingNewLine( + text + )(startOffset, endOffset) + val startPos = new l.Position( + range.getStart.getLine, + range.getStart.getCharacter - (startOffset - startWithSpace) + ) + val endPos = + if (endWithSpace - 1 >= 0 && text(endWithSpace - 1) == '\n') + new l.Position(range.getEnd.getLine + 1, 0) + else + new l.Position( + range.getEnd.getLine, + range.getEnd.getCharacter + endWithSpace - endOffset + ) + + new l.Range(startPos, endPos) + } val position: l.Position = pos.toLsp.getStart().nn @@ -41,7 +100,7 @@ final class PcInlineValueProviderImpl( Some(Occurence(tree, parent, adjustedPos)) case _ => None - override def defAndRefs(): Either[String, (Definition, List[Reference])] = + def defAndRefs(): Either[String, (Definition, List[Reference])] = val newctx = driver.currentCtx.fresh.setCompilationUnit(unit) val allOccurences = result().flatten for @@ -60,7 +119,6 @@ final class PcInlineValueProviderImpl( val defPos = definition.tree.sourcePos val defEdit = Definition( defPos.toLsp, - adjustRhs(definition.tree.rhs.sourcePos), RangeOffset(defPos.start, defPos.end), definitionRequiresBrackets(definition.tree.rhs)(using newctx), deleteDefinition @@ -70,6 +128,25 @@ final class PcInlineValueProviderImpl( end for end defAndRefs + extension (t : untpd.Tree) + def isIndentationSensitive: Boolean = t match + case blck : untpd.Block => + val untpd.Block(stats, expr) = blck + (stats :+ expr).head.span != blck.span + case _ => false + + private def stripIndentPrefix(rhs: String, refIndent: String, defIndent: String): String = + val rhsLines = rhs.split("\n").toList + rhsLines match + case h :: t => + val noPrefixH = h.stripPrefix(refIndent) + if noPrefixH.startsWith("{") then + noPrefixH ++ t.map(refIndent ++ _.stripPrefix(defIndent)).mkString("\n","\n", "") + else + ((" " ++ h) :: t).map(refIndent ++ _.stripPrefix(defIndent)).mkString("\n", "\n", "") + case Nil => + rhsLines.map(refIndent ++ _.stripPrefix(defIndent)).mkString("\n", "\n", "") + private def definitionRequiresBrackets(tree: Tree)(using Context): Boolean = NavigateAST .untypedPath(tree.span) @@ -103,6 +180,7 @@ final class PcInlineValueProviderImpl( end referenceRequiresBrackets private def adjustRhs(pos: SourcePosition) = + //TODO(kπ) doc def extend(point: Int, acceptedChar: Char, step: Int): Int = val newPoint = point + step if newPoint > 0 && newPoint < text.length && text( @@ -139,7 +217,7 @@ final class PcInlineValueProviderImpl( .exists(e => e.isTerm) def allreferences = allOccurences.filterNot(_.isDefn) def inlineAll() = - makeRefsEdits(allreferences, symbols).map((true, _)) + makeRefsEdits(allreferences, symbols, definition).map((true, _)) if definition.tree.sourcePos.toLsp.encloses(position) then if defIsLocal then inlineAll() else Left(Errors.notLocal) else @@ -150,14 +228,28 @@ final class PcInlineValueProviderImpl( ref <- list .find(_.pos.toLsp.encloses(position)) .toRight(Errors.didNotFindReference) - refEdits <- makeRefsEdits(List(ref), symbols) + refEdits <- makeRefsEdits(List(ref), symbols, definition) yield (false, refEdits) end if end getReferencesToInline + extension (pos: SourcePosition) + def startColumnIndentPadding: String = { + val source = pos.source + val offset = pos.start + var idx = source.startOfLine(offset) + val pad = new StringBuilder + while (idx != offset && idx < source.content().length && source.content()(idx).isWhitespace) { + pad.append(if (idx < source.content().length && source.content()(idx) == '\t') '\t' else ' ') + idx += 1 + } + pad.result() + } + private def makeRefsEdits( refs: List[Occurence], - symbols: Set[Symbol] + symbols: Set[Symbol], + definition: DefinitionTree ): Either[String, List[Reference]] = val newctx = driver.currentCtx.fresh.setCompilationUnit(unit) def buildRef(occurrence: Occurence): Either[String, Reference] = @@ -178,6 +270,11 @@ final class PcInlineValueProviderImpl( Right( Reference( occurrence.pos.toLsp, + stripIndentPrefix( + adjustRhs(definition.tree.rhs.sourcePos), + occurrence.tree.startPos.startColumnIndentPadding, + definition.tree.startPos.startColumnIndentPadding + ), occurrence.parent.map(p => RangeOffset(p.sourcePos.start, p.sourcePos.end) ), @@ -205,3 +302,19 @@ case class Occurence(tree: Tree, parent: Option[Tree], pos: SourcePosition): case _ => false case class DefinitionTree(tree: ValDef, pos: SourcePosition) + +case class RangeOffset(start: Int, end: Int) + +case class Definition( + range: l.Range, + rangeOffsets: RangeOffset, + requiresBrackets: Boolean, + shouldBeRemoved: Boolean +) + +case class Reference( + range: l.Range, + rhs: String, + parentOffsets: Option[RangeOffset], + requiresBrackets: Boolean +) diff --git a/presentation-compiler/test/dotty/tools/pc/tests/edit/InlineValueSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/edit/InlineValueSuite.scala index cee0aada9f86..46ddb8368c80 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/edit/InlineValueSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/edit/InlineValueSuite.scala @@ -407,6 +407,52 @@ class InlineValueSuite extends BaseCodeActionSuite with CommonMtagsEnrichments: InlineErrors.variablesAreShadowed("T.a") ) + @Test def `i7137` = + checkEdit( + """|object O { + | def foo = { + | val newValue = + | val x = true + | x + | val xx =new<>alue + | } + |} + |""".stripMargin, + """|object O { + | def foo = { + | val xx = + | val x = true + | x + | } + |} + |""".stripMargin + ) + + @Test def `i7137a` = + checkEdit( + """|object O { + | def foo = { + | val newValue = { + | val x = true + | x + | } + | def bar = + | val xx = new<>alue + | } + |} + |""".stripMargin, + """|object O { + | def foo = { + | def bar = + | val xx = { + | val x = true + | x + | } + | } + |} + |""".stripMargin + ) + def checkEdit( original: String, expected: String, From 9a043d5f991c9800df72cfe611b720c6954964e9 Mon Sep 17 00:00:00 2001 From: Kacper Korban Date: Mon, 7 Apr 2025 11:47:15 +0200 Subject: [PATCH 2/3] Cleanup PcInlineValueProvider a bit after i7137 - fix handling non-multiline edits - rename adjustRhs to extendWithSurroundingParens - adjust a failing test after the changes --- ...Impl.scala => PcInlineValueProvider.scala} | 26 +++++++------------ .../tools/pc/ScalaPresentationCompiler.scala | 2 +- .../pc/tests/edit/InlineValueSuite.scala | 3 ++- 3 files changed, 12 insertions(+), 19 deletions(-) rename presentation-compiler/src/main/dotty/tools/pc/{PcInlineValueProviderImpl.scala => PcInlineValueProvider.scala} (94%) diff --git a/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProviderImpl.scala b/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProvider.scala similarity index 94% rename from presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProviderImpl.scala rename to presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProvider.scala index 6563c458f53f..c54ac6013cf9 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProviderImpl.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProvider.scala @@ -23,7 +23,7 @@ import dotty.tools.pc.IndexedContext.Result import org.eclipse.lsp4j as l -final class PcInlineValueProviderImpl( +final class PcInlineValueProvider( driver: InteractiveDriver, val params: OffsetParams ) extends WithSymbolSearchCollector[Option[Occurence]](driver, params): @@ -128,24 +128,17 @@ final class PcInlineValueProviderImpl( end for end defAndRefs - extension (t : untpd.Tree) - def isIndentationSensitive: Boolean = t match - case blck : untpd.Block => - val untpd.Block(stats, expr) = blck - (stats :+ expr).head.span != blck.span - case _ => false - private def stripIndentPrefix(rhs: String, refIndent: String, defIndent: String): String = val rhsLines = rhs.split("\n").toList rhsLines match + case h :: Nil => rhs case h :: t => val noPrefixH = h.stripPrefix(refIndent) if noPrefixH.startsWith("{") then noPrefixH ++ t.map(refIndent ++ _.stripPrefix(defIndent)).mkString("\n","\n", "") else ((" " ++ h) :: t).map(refIndent ++ _.stripPrefix(defIndent)).mkString("\n", "\n", "") - case Nil => - rhsLines.map(refIndent ++ _.stripPrefix(defIndent)).mkString("\n", "\n", "") + case Nil => rhs private def definitionRequiresBrackets(tree: Tree)(using Context): Boolean = NavigateAST @@ -179,13 +172,12 @@ final class PcInlineValueProviderImpl( end referenceRequiresBrackets - private def adjustRhs(pos: SourcePosition) = - //TODO(kπ) doc + private def extendWithSurroundingParens(pos: SourcePosition) = + /** Move `point` by `step` as long as the character at `point` is `acceptedChar` */ def extend(point: Int, acceptedChar: Char, step: Int): Int = val newPoint = point + step - if newPoint > 0 && newPoint < text.length && text( - newPoint - ) == acceptedChar + if newPoint > 0 && newPoint < text.length && + text(newPoint) == acceptedChar then extend(newPoint, acceptedChar, step) else point val adjustedStart = extend(pos.start, '(', -1) @@ -271,7 +263,7 @@ final class PcInlineValueProviderImpl( Reference( occurrence.pos.toLsp, stripIndentPrefix( - adjustRhs(definition.tree.rhs.sourcePos), + extendWithSurroundingParens(definition.tree.rhs.sourcePos), occurrence.tree.startPos.startColumnIndentPadding, definition.tree.startPos.startColumnIndentPadding ), @@ -293,7 +285,7 @@ final class PcInlineValueProviderImpl( ) end makeRefsEdits -end PcInlineValueProviderImpl +end PcInlineValueProvider case class Occurence(tree: Tree, parent: Option[Tree], pos: SourcePosition): def isDefn = diff --git a/presentation-compiler/src/main/dotty/tools/pc/ScalaPresentationCompiler.scala b/presentation-compiler/src/main/dotty/tools/pc/ScalaPresentationCompiler.scala index dc53525480c3..0359e21dae89 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/ScalaPresentationCompiler.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/ScalaPresentationCompiler.scala @@ -355,7 +355,7 @@ case class ScalaPresentationCompiler( val empty: Either[String, List[l.TextEdit]] = Right(List()) (compilerAccess .withInterruptableCompiler(empty, params.token()) { pc => - new PcInlineValueProviderImpl(pc.compiler(), params) + new PcInlineValueProvider(pc.compiler(), params) .getInlineTextEdits() }(params.toQueryContext)) .thenApply { diff --git a/presentation-compiler/test/dotty/tools/pc/tests/edit/InlineValueSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/edit/InlineValueSuite.scala index 46ddb8368c80..541340bd3f38 100644 --- a/presentation-compiler/test/dotty/tools/pc/tests/edit/InlineValueSuite.scala +++ b/presentation-compiler/test/dotty/tools/pc/tests/edit/InlineValueSuite.scala @@ -228,7 +228,8 @@ class InlineValueSuite extends BaseCodeActionSuite with CommonMtagsEnrichments: | for { | i <- List(1,2,3) | } yield i + 1 - |val b = (for { + |val b = ( + | for { | i <- List(1,2,3) | } yield i + 1).map(_ + 1) |}""".stripMargin From 0ba141373371c0da9bc738ddf32023817143aaf6 Mon Sep 17 00:00:00 2001 From: Kacper Korban Date: Mon, 7 Apr 2025 16:44:03 +0200 Subject: [PATCH 3/3] Sort imports --- .../src/main/dotty/tools/pc/PcInlineValueProvider.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProvider.scala b/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProvider.scala index c54ac6013cf9..eafac513c7e1 100644 --- a/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProvider.scala +++ b/presentation-compiler/src/main/dotty/tools/pc/PcInlineValueProvider.scala @@ -14,9 +14,9 @@ import dotty.tools.dotc.core.Contexts.Context import dotty.tools.dotc.core.Flags.* import dotty.tools.dotc.core.StdNames import dotty.tools.dotc.core.Symbols.Symbol -import dotty.tools.dotc.util.SourceFile import dotty.tools.dotc.interactive.Interactive import dotty.tools.dotc.interactive.InteractiveDriver +import dotty.tools.dotc.util.SourceFile import dotty.tools.dotc.util.SourcePosition import dotty.tools.pc.utils.InteractiveEnrichments.* import dotty.tools.pc.IndexedContext.Result