Repository: scalameta/scalameta (scala)

Issue:
ScalametaParser: comment before outdent not part of indented expression
In this example:
```
val (xTranslation, yTranslation) = (isHMirror, isVMirror) match
      case (true, true)  => ((rotationWidth / 2).toInt, (rotationHeight / 2).toInt)  // Top left
      case (true, false) => ((rotationHeight / 2).toInt, -(rotationWidth / 2).toInt) // Bottom left
      case (false, true) => (-(rotationHeight / 2).toInt, (rotationWidth / 2).toInt) // Top right
      case (false, false) => (-(rotationWidth / 2).toInt, -(rotationHeight / 2).toInt) // Bottom right
```
the `// Bottom right` comment *should* be part of the `Term.Match` but it only belongs to `Defn.Val` instead.

Pull request:
Previously, we would outdent immediately after a non-trivial token while ignoring any trailing comments which followed.
Fixes #2516.

Patch:
diff --git a/scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala b/scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala
index 43328dda24..6cc7641d45 100644
--- a/scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala
+++ b/scalameta/parsers/shared/src/main/scala/scala/meta/internal/parsers/ScalametaParser.scala
@@ -389,13 +389,41 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
           case region :: tail if region.isIndented && canEndIndentation(curr.token) =>
             sepRegions = tail
             val outdent = mkOutdentToken(curr.token)
-            curr = TokenRef(outdent, curr.pos, curr.pos, prevPos)
+            val outdentPos = findOutdentPos(prevPos, curr.pos, region)
+            curr = TokenRef(outdent, curr.pos, curr.pos, outdentPos)
             true
           case _ => false
         }
       }
     }
 
+    private def findOutdentPos(prevPos: Int, currPos: Int, region: SepRegion): Int = {
+      val outdent = region.indent
+      @tailrec
+      def iter(i: Int, pos: Int, indent: Int): Int = {
+        if (i >= currPos) {
+          if (pos < currPos) pos else currPos - 1
+        } else {
+          scannerTokens(i) match {
+            case _: LF | _: FF =>
+              iter(i + 1, if (pos == prevPos) i else pos, 0)
+            case _: Space | _: Tab if indent >= 0 =>
+              iter(i + 1, pos, indent + 1)
+            case Whitespace() =>
+              iter(i + 1, pos, indent)
+            case _: Comment if indent < 0 || outdent <= indent =>
+              iter(i + 1, i + 1, -1)
+            case _ => pos
+          }
+        }
+      }
+
+      val iterPos = 1 + prevPos
+      if (iterPos < currPos) iter(iterPos, prevPos, -1)
+      else if (scannerTokens(currPos).is[EOF]) currPos
+      else prevPos
+    }
+
     @tailrec
     private def nextToken(
         prevPos: Int,
@@ -421,27 +449,38 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
       def mkIndent(pointPos: Int): TokenRef =
         TokenRef(mkIndentToken(curr), prevPos, currPos, pointPos)
 
-      def mkOutdent(pointPos: Int): TokenRef =
+      def mkOutdent(region: SepRegion): TokenRef =
+        mkOutdentTo(region, currPos)
+
+      def mkOutdentTo(region: SepRegion, maxPointPos: Int): TokenRef = {
+        val pointPos = findOutdentPos(prevPos, maxPointPos, region)
         TokenRef(mkOutdentToken(curr), prevPos, currPos, pointPos)
+      }
 
       def currRef: TokenRef = TokenRef(curr, currPos, currPos + 1, currPos)
 
-      def indentationWithinParenRegion: Boolean =
-        sepRegions.headOption.exists(!_.isInstanceOf[RegionParen]) &&
-          sepRegions
+      def indentationWithinParenRegion: Option[SepRegion] = {
+        def isWithinParenRegion =
+          sepRegions.tail
             .collectFirst {
               case RegionParen(_) => true
               case other if !other.isIndented => false
             }
-            .getOrElse(false)
+            .contains(true)
+        sepRegions.headOption.filter(_.isIndented && isWithinParenRegion)
+      }
 
       if (isTrailingComma) {
         nextToken(currPos, currPos + 1, sepRegions)
       } else if (curr.isNot[Trivia]) {
         if (curr.is[LeftParen]) (RegionParen(false) :: sepRegions, currRef)
         else if (curr.is[LeftBracket]) (RegionBracket :: sepRegions, currRef)
-        else if (curr.is[Comma] && indentationWithinParenRegion) {
-          (sepRegions.tail, mkOutdent(prevPos))
+        else if (curr.is[Comma]) {
+          indentationWithinParenRegion.fold {
+            (sepRegions, currRef)
+          } { region =>
+            (sepRegions.tail, mkOutdent(region))
+          }
         } else if (curr.is[LeftBrace]) {
           val indentInBrace = if (isAheadNewLine(currPos)) countIndent(nextPos) else -1
           // After encountering keyword Enum we add artificial '{' on top of stack.
@@ -474,7 +513,7 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
               case (_: RegionBrace | _: RegionEnum) :: xs =>
                 (xs, currRef)
               case x :: xs if x.isIndented =>
-                (xs, mkOutdent(prevPos))
+                (xs, mkOutdent(x))
               case _ :: xs =>
                 nextRegions(xs)
               case Nil =>
@@ -489,15 +528,12 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
           (nextRegions, currRef)
         } else if (curr.is[EOF]) {
           sepRegions match {
-            case x :: xs if x.isIndented =>
-              // do no include trailing comment/whitespace tokens
-              val pointPos = prevPos + 1
-              (xs, mkOutdent(pointPos))
+            case x :: xs if x.isIndented => (xs, mkOutdent(x))
             case other => (other, currRef)
           }
         } else if (curr.is[RightParen]) {
           sepRegions match {
-            case x :: xs if x.isIndented => (xs, mkOutdent(prevPos))
+            case x :: xs if x.isIndented => (xs, mkOutdent(x))
             case RegionParen(_) :: xs => (xs, currRef)
             case _ => (sepRegions, currRef)
           }
@@ -567,7 +603,11 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
           }
         }
 
-        if (dialect.allowSignificantIndentation) {
+        def getIfCanProduceLF =
+          if (canProduceLF) Some((sepRegions, lastWhitespaceToken))
+          else None
+
+        val resOpt = if (dialect.allowSignificantIndentation) {
           val hasLF = lastNewlinePos != -1 || hasMultilineComment
           if (hasLF && next != null && !isLeadingInfixOperator(next)) {
 
@@ -586,16 +626,16 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
              *    foo()
              *    ```
              */
-            def needOutdent: Boolean = {
-              sepRegions.headOption.exists {
-                case r if r.isIndented =>
+            def getOutdentIfNeeded() = sepRegions.headOption
+              .filter { r =>
+                r.isIndented && {
                   // need to check prev.prev in case of `end match`
                   if (nextIndent < r.indent)
                     prev.isNot[CanContinueOnNextLine] || prev.prev.is[soft.KwEnd]
                   else r.closeOnNonCase && next.isNot[KwCase] && nextIndent == r.indent
-                case _ => false
+                }
               }
-            }
+              .map { region => (sepRegions.tail, mkOutdentTo(region, nextPos)) }
 
             /**
              * Indent is needed in the following cases:
@@ -612,8 +652,8 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
              *  Indentation after `:` isn't hadled here.
              *  It's produced manually on the parser level.
              */
-            def needIndent: Boolean = {
-              if (nextIndent >= 0) {
+            def getIndentIfNeeded = {
+              val ok = nextIndent >= 0 && {
 
                 val (currIndent, indentOnArrow) =
                   sepRegions.headOption.fold((0, true))(r => (r.indent, r.indentOnArrow))
@@ -631,25 +671,24 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
                   // check the previous token to avoid infinity loop
                   (!prev.prev.is[soft.KwEnd] && (prev.is[KwMatch] || prev.is[KwCatch])) &&
                   next.is[KwCase] && token.isNot[Indentation.Indent]
-              } else false
+              }
+              if (ok) Some {
+                val region = RegionIndent(nextIndent, prev.is[KwMatch])
+                (region :: sepRegions, mkIndent(lastNewlinePos))
+              }
+              else None
             }
 
-            if (needOutdent)
-              (sepRegions.tail, mkOutdent(lastNewlinePos))
-            else if (needIndent)
-              (RegionIndent(nextIndent, prev.is[KwMatch]) :: sepRegions, mkIndent(lastNewlinePos))
-            else if (canProduceLF) {
-              (sepRegions, lastWhitespaceToken)
-            } else
-              nextToken(prevPos, nextPos, sepRegions)
-          } else nextToken(prevPos, nextPos, sepRegions)
-
+            getOutdentIfNeeded()
+              .orElse { getIndentIfNeeded }
+              .orElse { getIfCanProduceLF }
+          } else None
         } else {
-          if (canProduceLF) {
-            (sepRegions, lastWhitespaceToken)
-          } else {
-            nextToken(prevPos, nextPos, sepRegions)
-          }
+          getIfCanProduceLF
+        }
+        resOpt match {
+          case Some(res) => res
+          case _ => nextToken(prevPos, nextPos, sepRegions)
         }
       }
     }
@@ -866,6 +905,12 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
     def startTokenPos = in.tokenPos
     def endTokenPos = in.prevTokenPos
   }
+  case object StartPosPrev extends StartPos {
+    def startTokenPos = in.prevTokenPos
+  }
+  case object EndPosPreOutdent extends EndPos {
+    def endTokenPos = if (in.token.is[Indentation.Outdent]) in.tokenPos else in.prevTokenPos
+  }
   implicit def intToIndexPos(index: Int): IndexPos = IndexPos(index)
   implicit def tokenToTokenPos(token: Token): TokenPos = TokenPos(token)
   implicit def treeToTreePos(tree: Tree): TreePos = TreePos(tree)
@@ -891,11 +936,12 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
     def skipBy(range: Range, f: Token => Boolean): Int =
       range.dropWhile(i => f(scannerTokens(i))).headOption.getOrElse(range.start)
     @inline def skipTrivia(range: Range): Int = skipBy(range, _.is[Trivia])
+    @inline def skipWhitespace(range: Range): Int = skipBy(range, _.is[Whitespace])
 
     val rangeFromStart = startTokenPos to endTokenPos
     val (start, end) =
       if (rangeFromStart.isEmpty) (startTokenPos, endTokenPos)
-      else (skipTrivia(rangeFromStart), skipTrivia(rangeFromStart.reverse))
+      else (skipTrivia(rangeFromStart), skipWhitespace(rangeFromStart.reverse))
 
     val endExcl = if (start == end && scannerTokens(start).is[Trivia]) end else end + 1
     val pos = TokenStreamPosition(start, endExcl)
@@ -3213,7 +3259,7 @@ class ScalametaParser(input: Input)(implicit dialect: Dialect) { parser =>
     }
   }
 
-  def caseClause(forceSingleExpr: Boolean = false): Case = atPos(in.prevTokenPos, auto) {
+  def caseClause(forceSingleExpr: Boolean = false): Case = atPos(StartPosPrev, EndPosPreOutdent) {
     if (token.isNot[KwCase]) {
       def caseBody() = {
         accept[RightArrow]


Test patch:
diff --git a/tests/shared/src/test/scala-2.13/scala/meta/tests/parsers/dotty/Scala3PositionSuite.scala b/tests/shared/src/test/scala-2.13/scala/meta/tests/parsers/dotty/Scala3PositionSuite.scala
index 47d182c3ff..bd82ce18d3 100644
--- a/tests/shared/src/test/scala-2.13/scala/meta/tests/parsers/dotty/Scala3PositionSuite.scala
+++ b/tests/shared/src/test/scala-2.13/scala/meta/tests/parsers/dotty/Scala3PositionSuite.scala
@@ -507,6 +507,28 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |""".stripMargin
   )
 
+  checkPositions[Stat](
+    """|foo(
+       |  s =>
+       |    fx
+       |    gx(s)
+       |    /* c1 */,
+       |  "yo"
+       |)
+       |""".stripMargin,
+    """|Term.Function s =>
+       |    fx
+       |    gx(s)
+       |    /* c1 */
+       |Term.Param s
+       |Term.Block fx
+       |    gx(s)
+       |    /* c1 */
+       |Term.Apply gx(s)
+       |Lit.String "yo"
+       |""".stripMargin
+  )
+
   checkPositions[Stat](
     """|inline def encodeFlat =
        |    putInt
@@ -592,6 +614,7 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |      // c1
        |    else
        |      gx
+       |      // c2
        |Self   @@val a = 
        |Defn.Val val a = 
        |    if cond then
@@ -599,11 +622,54 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |      // c1
        |    else
        |      gx
+       |      // c2
        |Term.If if cond then
        |      fx
        |      // c1
        |    else
        |      gx
+       |      // c2
+       |Term.Name fx
+       |      // c1
+       |Term.Name gx
+       |      // c2
+       |""".stripMargin
+  )
+
+  checkPositions[Stat](
+    """|object x:
+       |  val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
+       |""".stripMargin,
+    """|Template :
+       |  val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
+       |Self   @@val a =
+       |Defn.Val val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
+       |Term.If if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
+       |Term.Name fx
+       |      // c1
        |""".stripMargin
   )
 
@@ -625,6 +691,7 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |      // c1
        |    else
        |      gx
+       |      // c2
        |Self   @@val a = 
        |Defn.Val val a = 
        |    if cond then
@@ -632,11 +699,129 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |      // c1
        |    else
        |      gx
+       |      // c2
+       |Term.If if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |      // c2
+       |Term.Name fx
+       |      // c1
+       |Term.Name gx
+       |      // c2
+       |""".stripMargin
+  )
+
+  // do not add LF to end of input
+  // this test checks `..$comment$EOF` case
+  checkPositions[Stat](
+    """|object y:
+       |  val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2""".stripMargin,
+    """|Template :
+       |  val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
+       |Self   @@val a =
+       |Defn.Val val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
        |Term.If if cond then
        |      fx
        |      // c1
        |    else
        |      gx
+       |    // c2
+       |Term.Name fx
+       |      // c1
+       |""".stripMargin
+  )
+
+  checkPositions[Stat](
+    """|object y {
+       |  val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
+       |}""".stripMargin,
+    """|Template {
+       |  val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
+       |}
+       |Self   @@val a =
+       |Defn.Val val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
+       |Term.If if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    // c2
+       |Term.Name fx
+       |      // c1
+       |""".stripMargin
+  )
+
+  checkPositions[Stat](
+    """|object y {
+       |  val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    /* c2 */}""".stripMargin,
+    """|Template {
+       |  val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    /* c2 */}
+       |Self   @@val a =
+       |Defn.Val val a =
+       |    if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    /* c2 */
+       |Term.If if cond then
+       |      fx
+       |      // c1
+       |    else
+       |      gx
+       |    /* c2 */
+       |Term.Name fx
+       |      // c1
        |""".stripMargin
   )
 
@@ -666,9 +851,9 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |  case (false, false) => (baz, qux) // c1""".stripMargin,
     """|Pat.Tuple (x, y)
        |Term.Match (foo, bar) match
-       |  case (false, false) => (baz, qux)
+       |  case (false, false) => (baz, qux) // c1
        |Term.Tuple (foo, bar)
-       |Case case (false, false) => (baz, qux)
+       |Case case (false, false) => (baz, qux) // c1
        |Pat.Tuple (false, false)
        |Term.Tuple (baz, qux)
        |""".stripMargin
@@ -680,9 +865,9 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |""".stripMargin,
     """|Pat.Tuple (x, y)
        |Term.Match (foo, bar) match
-       |  case (false, false) => (baz, qux)
+       |  case (false, false) => (baz, qux) // c1
        |Term.Tuple (foo, bar)
-       |Case case (false, false) => (baz, qux)
+       |Case case (false, false) => (baz, qux) // c1
        |Pat.Tuple (false, false)
        |Term.Tuple (baz, qux)
        |""".stripMargin
@@ -696,12 +881,12 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
     """|Pat.Tuple (x, y)
        |Term.Match (foo, bar) match
        |  case (true, false) => (baz, qux) // c1
-       |  case (false, true) => (baz, qux)
+       |  case (false, true) => (baz, qux) // c2
        |Term.Tuple (foo, bar)
-       |Case case (true, false) => (baz, qux)
+       |Case case (true, false) => (baz, qux) // c1
        |Pat.Tuple (true, false)
        |Term.Tuple (baz, qux)
-       |Case case (false, true) => (baz, qux)
+       |Case case (false, true) => (baz, qux) // c2
        |Pat.Tuple (false, true)
        |Term.Tuple (baz, qux)
        |""".stripMargin
@@ -714,12 +899,12 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
     """|Pat.Tuple (x, y)
        |Term.Match (foo, bar) match
        |  case (true, false) => (baz, qux) // c1
-       |  case (false, true) => (baz, qux)
+       |  case (false, true) => (baz, qux) // c2
        |Term.Tuple (foo, bar)
-       |Case case (true, false) => (baz, qux)
+       |Case case (true, false) => (baz, qux) // c1
        |Pat.Tuple (true, false)
        |Term.Tuple (baz, qux)
-       |Case case (false, true) => (baz, qux)
+       |Case case (false, true) => (baz, qux) // c2
        |Pat.Tuple (false, true)
        |Term.Tuple (baz, qux)
        |""".stripMargin
@@ -730,10 +915,10 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |  case (true, false) => (baz, qux) // c1
        |  case (false, true) => (baz, qux) // c2""".stripMargin,
     """|Term.Tuple (foo, bar)
-       |Case case (true, false) => (baz, qux)
+       |Case case (true, false) => (baz, qux) // c1
        |Pat.Tuple (true, false)
        |Term.Tuple (baz, qux)
-       |Case case (false, true) => (baz, qux)
+       |Case case (false, true) => (baz, qux) // c2
        |Pat.Tuple (false, true)
        |Term.Tuple (baz, qux)
        |""".stripMargin
@@ -744,10 +929,10 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |case (true, false) => (baz, qux) // c1
        |case (false, true) => (baz, qux) // c2""".stripMargin,
     """|Term.Tuple (foo, bar)
-       |Case case (true, false) => (baz, qux)
+       |Case case (true, false) => (baz, qux) // c1
        |Pat.Tuple (true, false)
        |Term.Tuple (baz, qux)
-       |Case case (false, true) => (baz, qux)
+       |Case case (false, true) => (baz, qux) // c2
        |Pat.Tuple (false, true)
        |Term.Tuple (baz, qux)
        |""".stripMargin
@@ -759,10 +944,10 @@ class Scala3PositionSuite extends BasePositionSuite(dialects.Scala3) {
        |case (false, true) => (baz, qux) // c2
        |""".stripMargin,
     """|Term.Tuple (foo, bar)
-       |Case case (true, false) => (baz, qux)
+       |Case case (true, false) => (baz, qux) // c1
        |Pat.Tuple (true, false)
        |Term.Tuple (baz, qux)
-       |Case case (false, true) => (baz, qux)
+       |Case case (false, true) => (baz, qux) // c2
        |Pat.Tuple (false, true)
        |Term.Tuple (baz, qux)
        |""".stripMargin
diff --git a/tests/shared/src/test/scala-2.13/scala/meta/tests/tokenizers/TokensPositionSuite.scala b/tests/shared/src/test/scala-2.13/scala/meta/tests/tokenizers/TokensPositionSuite.scala
index 5e7c16c52c..abf599bbaf 100644
--- a/tests/shared/src/test/scala-2.13/scala/meta/tests/tokenizers/TokensPositionSuite.scala
+++ b/tests/shared/src/test/scala-2.13/scala/meta/tests/tokenizers/TokensPositionSuite.scala
@@ -679,6 +679,27 @@ class TokensPositionSuite extends BasePositionSuite(dialects.Scala213) {
        |""".stripMargin
   )
 
+  checkPositions[Stat](
+    """|trait SampleTrait extends A {
+       |  self: X with B with C => // c1
+       |
+       |  def foo: Boolean = true // c2
+       |  // c3
+       |} // c4""".stripMargin,
+    """|Ctor.Primary trait SampleTrait @@extends A {
+       |Template extends A {
+       |  self: X with B with C => // c1
+       |
+       |  def foo: Boolean = true // c2
+       |  // c3
+       |}
+       |Self self: X with B with C
+       |Type.With X with B with C
+       |Type.With X with B
+       |Defn.Def def foo: Boolean = true
+       |""".stripMargin
+  )
+
   checkPositions[Stat](
     """|object A {
        |  private [this] def foo: Int = ???