Rewrite to no-indent keeps end comment

Author: som-snytt

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -330,7 +330,7 @@ object Parsers {
         else if in.token == END then
           if endSeen then syntaxError("duplicate end marker")
           checkEndMarker(stats)
-          recur(sepSeen, true)
+          recur(sepSeen, endSeen = true)
         else if isStatSeqEnd || in.token == altEnd then
           false
         else if sepSeen || endSeen then
@@ -663,7 +663,6 @@ object Parsers {
       def closingOffset(lineStart: Offset): Offset =
         if in.lineOffset >= 0 && lineStart >= in.lineOffset then in.lineOffset
         else
-          val candidate = source.nextLine(lineStart)   // unused
           val commentStart = skipBlanks(lineStart)
           if testChar(commentStart, '/') && indentWidth < in.indentWidth(commentStart)
           then closingOffset(source.nextLine(lineStart))
@@ -1297,6 +1296,8 @@ object Parsers {
         case _: (ForYield | ForDo) => in.token == FOR
         case _ => false
 
+      def endName = if in.token == IDENTIFIER then in.name.toString else tokenString(in.token)
+
       def matchesAndSetEnd(last: T): Boolean =
         val didMatch = matches(last)
         if didMatch then
@@ -1307,6 +1308,9 @@ object Parsers {
         val start = in.skipToken()
         if stats.isEmpty || !matchesAndSetEnd(stats.last) then
           syntaxError("misaligned end marker", Span(start, in.lastCharOffset))
+        else if overlapsPatch(source, Span(start, start)) then
+          patch(source, Span(start, start), "")
+          patch(source, Span(start, in.lastCharOffset), s"} // end $endName")
         in.token = IDENTIFIER // Leaving it as the original token can confuse newline insertion
         in.nextToken()
     end checkEndMarker
@@ -3846,9 +3850,9 @@ object Parsers {
     def templateStatSeq(): (ValDef, List[Tree]) = checkNoEscapingPlaceholders {
       var self: ValDef = EmptyValDef
       val stats = new ListBuffer[Tree]
-      if (isExprIntro && !isDefIntro(modifierTokens)) {
+      if isExprIntro && !isDefIntro(modifierTokens) then
         val first = expr1()
-        if (in.token == ARROW) {
+        if in.token == ARROW then
           first match {
             case Typed(tree @ This(EmptyTypeIdent), tpt) =>
               self = makeSelfDef(nme.WILDCARD, tpt).withSpan(first.span)
@@ -3859,11 +3863,10 @@ object Parsers {
           }
           in.token = SELFARROW // suppresses INDENT insertion after `=>`
           in.nextToken()
-        }
         else
           stats += first
           statSepOrEnd(stats)
-      }
+      end if
       while
         var empty = false
         if (in.token == IMPORT)
@@ -3880,7 +3883,7 @@ object Parsers {
           empty = true
         statSepOrEnd(stats, empty)
       do ()
-      (self, if (stats.isEmpty) List(EmptyTree) else stats.toList)
+      (self, if stats.isEmpty then List(EmptyTree) else stats.toList)
     }
 
     /** RefineStatSeq    ::=  RefineStat {semi RefineStat}

compiler/src/dotty/tools/dotc/rewrites/Rewrites.scala

@@ -20,13 +20,16 @@ object Rewrites {
     private[Rewrites] val pbuf = new mutable.ListBuffer[Patch]()
 
     def addPatch(span: Span, replacement: String): Unit =
-      pbuf += Patch(span, replacement)
+      pbuf.indexWhere(p => p.span.start == span.start && p.span.end == span.end) match {
+        case i if i >= 0 => pbuf.update(i, Patch(span, replacement))
+        case _           => pbuf += Patch(span, replacement)
+      }
 
     def apply(cs: Array[Char]): Array[Char] = {
       val delta = pbuf.map(_.delta).sum
       val patches = pbuf.toList.sortBy(_.span.start)
       if (patches.nonEmpty)
-        patches reduceLeft {(p1, p2) =>
+        patches.reduceLeft {(p1, p2) =>
           assert(p1.span.end <= p2.span.start, s"overlapping patches in $source: $p1 and $p2")
           p2
         }
@@ -64,11 +67,11 @@ object Rewrites {
    *  given by `span` in `source` by `replacement`
    */
   def patch(source: SourceFile, span: Span, replacement: String)(using Context): Unit =
-    if (ctx.reporter != Reporter.NoReporter) // NoReporter is used for syntax highlighting
-      for (rewrites <- ctx.settings.rewrite.value)
-        rewrites.patched
-          .getOrElseUpdate(source, new Patches(source))
-          .addPatch(span, replacement)
+    if ctx.reporter != Reporter.NoReporter // NoReporter is used for syntax highlighting
+    then ctx.settings.rewrite.value.foreach(_.patched
+         .getOrElseUpdate(source, new Patches(source))
+         .addPatch(span, replacement)
+    )
 
   /** Patch position in `ctx.compilationUnit.source`. */
   def patch(span: Span, replacement: String)(using Context): Unit =
@@ -96,6 +99,3 @@ class Rewrites {
   import Rewrites._
   private val patched = new PatchedFiles
 }
-
-
-

compiler/src/dotty/tools/dotc/util/Spans.scala

@@ -85,6 +85,7 @@ object Spans {
       || containsInner(this, that.end)
       || containsInner(that, this.start)
       || containsInner(that, this.end)
+      || this.start == that.start && this.end == that.end   // exact match in one point
       )
     }
 

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -72,7 +72,8 @@ class CompilationTests {
       compileFile("tests/rewrites/rewrites3x.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
       compileFile("tests/rewrites/i8982.scala", defaultOptions.and("-indent", "-rewrite")),
       compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")),
-      compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite"))
+      compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")),
+      compileFile("tests/rewrites/i12340.scala", unindentOptions.and("-rewrite")),
     ).checkRewrites()
   }
 

compiler/test/dotty/tools/vulpix/ParallelTesting.scala

@@ -978,7 +978,7 @@ trait ParallelTesting extends RunnerOrchestration { self =>
           target.copy(dir = copyToDir(outDir, dir))
       }
 
-      val test = new RewriteTest(copiedTargets, checkFileMap, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite()
+      new RewriteTest(copiedTargets, checkFileMap, times, threadLimit, shouldFail || shouldSuppressOutput).executeTestSuite()
       this
     }
 

compiler/test/dotty/tools/vulpix/TestConfiguration.scala

@@ -60,6 +60,7 @@ object TestConfiguration {
 
   val commonOptions = Array("-indent", "-language:postfixOps") ++ checkOptions ++ noCheckOptions ++ yCheckOptions
   val defaultOptions = TestFlags(basicClasspath, commonOptions)
+  val unindentOptions = TestFlags(basicClasspath, Array("-no-indent") ++ checkOptions ++ noCheckOptions ++ yCheckOptions)
   val withCompilerOptions =
     defaultOptions.withClasspath(withCompilerClasspath).withRunClasspath(withCompilerClasspath)
   lazy val withStagingOptions =

tests/rewrites/i12340.check

@@ -0,0 +1,17 @@
+
+class C {
+  def f = 42
+} // end C
+
+def f(i: Int) = {
+  if i < 42 then
+    println(i)
+  end if
+  i
+} // end f
+
+def g(i: Int) =
+  if i < 42 then
+    println(i)
+  end if
+end g

tests/rewrites/i12340.scala

@@ -0,0 +1,17 @@
+
+class C:
+  def f = 42
+end C
+
+def f(i: Int) =
+  if i < 42 then
+    println(i)
+  end if
+  i
+end f
+
+def g(i: Int) =
+  if i < 42 then
+    println(i)
+  end if
+end g