Skip to content

Commit de1558d

Browse files
committed
Patch indentation when removing braces
If the first indentation of the region is greater than the indentation of the enclosing region, we use it to indent the whole region. Otherwise we use the incremented indentation of the enclosing region. ```scala def foo = { x // we replicate indentation of x downward in region y } ``` ```scala def foo = { x // indentation of x is incorrect, we increment enclosing indentation y } ``` A bigger indentation than the required one is permitted except just after a closing brace. ```scala def bar = { x .toString // permitted indentation def foo = { } bar // must be unindented, to not fall into the body of foo } ``` And other bug fixes (see scala#17522)
1 parent d5a7131 commit de1558d

17 files changed

+1132
-318
lines changed

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

Lines changed: 345 additions & 270 deletions
Large diffs are not rendered by default.

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

Lines changed: 57 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,17 @@ object Scanners {
6060
/** the base of a number */
6161
var base: Int = 0
6262

63-
def copyFrom(td: TokenData): Unit = {
63+
def copyFrom(td: TokenData): this.type =
6464
this.token = td.token
6565
this.offset = td.offset
6666
this.lastOffset = td.lastOffset
6767
this.lineOffset = td.lineOffset
6868
this.name = td.name
6969
this.strVal = td.strVal
7070
this.base = td.base
71-
}
71+
this
72+
73+
def saveCopy: TokenData = newTokenData.copyFrom(this)
7274

7375
def isNewLine = token == NEWLINE || token == NEWLINES
7476
def isStatSep = isNewLine || token == SEMI
@@ -86,12 +88,14 @@ object Scanners {
8688

8789
def isOperator =
8890
token == BACKQUOTED_IDENT
89-
|| token == IDENTIFIER && isOperatorPart(name(name.length - 1))
91+
|| token == IDENTIFIER && isOperatorPart(name.last)
9092

9193
def isArrow =
9294
token == ARROW || token == CTXARROW
9395
}
9496

97+
def newTokenData: TokenData = new TokenData {}
98+
9599
abstract class ScannerCommon(source: SourceFile)(using Context) extends CharArrayReader with TokenData {
96100
val buf: Array[Char] = source.content
97101
def nextToken(): Unit
@@ -264,11 +268,10 @@ object Scanners {
264268
if (idx >= 0 && idx <= lastKeywordStart) handleMigration(kwArray(idx))
265269
else IDENTIFIER
266270

267-
def newTokenData: TokenData = new TokenData {}
268-
269271
/** We need one token lookahead and one token history
270272
*/
271273
val next = newTokenData
274+
val last = newTokenData
272275
private val prev = newTokenData
273276

274277
/** The current region. This is initially an Indented region with zero indentation width. */
@@ -385,12 +388,11 @@ object Scanners {
385388
/** Produce next token, filling TokenData fields of Scanner.
386389
*/
387390
def nextToken(): Unit =
388-
val lastToken = token
389-
val lastName = name
390-
adjustSepRegions(lastToken)
391-
getNextToken(lastToken)
392-
if isAfterLineEnd then handleNewLine(lastToken)
393-
postProcessToken(lastToken, lastName)
391+
last.copyFrom(this)
392+
adjustSepRegions(last.token)
393+
getNextToken(last.token)
394+
if isAfterLineEnd then handleNewLine()
395+
postProcessToken()
394396
profile.recordNewToken()
395397
printState()
396398

@@ -433,7 +435,7 @@ object Scanners {
433435
// in backticks and is a binary operator. Hence, `x` is not classified as a
434436
// leading infix operator.
435437
def assumeStartsExpr(lexeme: TokenData) =
436-
(canStartExprTokens.contains(lexeme.token) || lexeme.token == COLONeol)
438+
(canStartExprTokens.contains(lexeme.token) || lexeme.token == COLONfollow)
437439
&& (!lexeme.isOperator || nme.raw.isUnary(lexeme.name))
438440
val lookahead = LookaheadScanner()
439441
lookahead.allowLeadingInfixOperators = false
@@ -483,7 +485,7 @@ object Scanners {
483485
if (nextChar == ch)
484486
recur(idx - 1, ch, n + 1, k)
485487
else {
486-
val k1: IndentWidth => IndentWidth = if (n == 0) k else Conc(_, Run(ch, n))
488+
val k1: IndentWidth => IndentWidth = if (n == 0) k else iw => k(Conc(iw, Run(ch, n)))
487489
recur(idx - 1, nextChar, 1, k1)
488490
}
489491
else recur(idx - 1, ' ', 0, identity)
@@ -523,7 +525,7 @@ object Scanners {
523525
*
524526
* The following tokens can start an indentation region:
525527
*
526-
* : = => <- if then else while do try catch
528+
* : = => <- if then else while do try catch
527529
* finally for yield match throw return with
528530
*
529531
* Inserting an INDENT starts a new indentation region with the indentation of the current
@@ -547,7 +549,7 @@ object Scanners {
547549
* I.e. `a <= b` iff `b.startsWith(a)`. If indentation is significant it is considered an error
548550
* if the current indentation width and the indentation of the current token are incomparable.
549551
*/
550-
def handleNewLine(lastToken: Token) =
552+
def handleNewLine() =
551553
var indentIsSignificant = false
552554
var newlineIsSeparating = false
553555
var lastWidth = IndentWidth.Zero
@@ -576,7 +578,7 @@ object Scanners {
576578
*/
577579
inline def isContinuing =
578580
lastWidth < nextWidth
579-
&& (openParensTokens.contains(token) || lastToken == RETURN)
581+
&& (openParensTokens.contains(token) || last.token == RETURN)
580582
&& !pastBlankLine
581583
&& !migrateTo3
582584
&& !noindentSyntax
@@ -590,12 +592,12 @@ object Scanners {
590592
case _: InString => ()
591593
case r =>
592594
indentIsSignificant = indentSyntax
593-
r.proposeKnownWidth(nextWidth, lastToken)
595+
r.proposeKnownWidth(nextWidth, last.token)
594596
lastWidth = r.knownWidth
595597
newlineIsSeparating = r.isInstanceOf[InBraces]
596598

597599
if newlineIsSeparating
598-
&& canEndStatTokens.contains(lastToken)
600+
&& canEndStatTokens.contains(last.token)
599601
&& canStartStatTokens.contains(token)
600602
&& !isLeadingInfixOperator(nextWidth)
601603
&& !isContinuing
@@ -606,7 +608,7 @@ object Scanners {
606608
|| nextWidth == lastWidth && (indentPrefix == MATCH || indentPrefix == CATCH) && token != CASE then
607609
if currentRegion.isOutermost then
608610
if nextWidth < lastWidth then currentRegion = topLevelRegion(nextWidth)
609-
else if !isLeadingInfixOperator(nextWidth) && !statCtdTokens.contains(lastToken) && lastToken != INDENT then
611+
else if !isLeadingInfixOperator(nextWidth) && !statCtdTokens.contains(last.token) && last.token != INDENT then
610612
currentRegion match
611613
case r: Indented =>
612614
insert(OUTDENT, offset)
@@ -630,15 +632,16 @@ object Scanners {
630632
report.warning("Line is indented too far to the left, or a `}` is missing", sourcePos())
631633

632634
else if lastWidth < nextWidth
633-
|| lastWidth == nextWidth && (lastToken == MATCH || lastToken == CATCH) && token == CASE then
634-
if canStartIndentTokens.contains(lastToken) then
635-
currentRegion = Indented(nextWidth, lastToken, currentRegion)
635+
|| lastWidth == nextWidth && (last.token == MATCH || last.token == CATCH) && token == CASE then
636+
if canStartIndentTokens.contains(last.token) then
637+
currentRegion = Indented(nextWidth, last.token, currentRegion)
636638
insert(INDENT, offset)
637-
else if lastToken == SELFARROW then
639+
else if last.token == SELFARROW then
638640
currentRegion.knownWidth = nextWidth
639641
else if (lastWidth != nextWidth)
640642
val lw = lastWidth
641-
errorButContinue(spaceTabMismatchMsg(lw, nextWidth))
643+
val msg = spaceTabMismatchMsg(lw, nextWidth)
644+
if rewriteToIndent then report.warning(msg) else errorButContinue(msg)
642645
if token != OUTDENT then
643646
handleNewIndentWidth(currentRegion, _.otherIndentWidths += nextWidth)
644647
if next.token == EMPTY then
@@ -702,7 +705,7 @@ object Scanners {
702705
* SEMI + ELSE => ELSE, COLON following id/)/] => COLONfollow
703706
* - Insert missing OUTDENTs at EOF
704707
*/
705-
def postProcessToken(lastToken: Token, lastName: SimpleName): Unit = {
708+
def postProcessToken(): Unit = {
706709
def fuse(tok: Int) = {
707710
token = tok
708711
offset = prev.offset
@@ -738,8 +741,8 @@ object Scanners {
738741
case END =>
739742
if !isEndMarker then token = IDENTIFIER
740743
case COLONop =>
741-
if lastToken == IDENTIFIER && lastName != null && isIdentifierStart(lastName.head)
742-
|| colonEOLPredecessors.contains(lastToken)
744+
if last.token == IDENTIFIER && last.name != null && isIdentifierStart(last.name.head)
745+
|| colonEOLPredecessors.contains(last.token)
743746
then token = COLONfollow
744747
case RBRACE | RPAREN | RBRACKET =>
745748
closeIndented()
@@ -758,6 +761,8 @@ object Scanners {
758761
if endMarkerTokens.contains(lookahead.token)
759762
&& source.offsetToLine(lookahead.offset) == endLine
760763
then
764+
if rewriteToIndent && lookahead.token == MATCH then
765+
patch(Span(offset, offset + 3), "`end`")
761766
lookahead.nextToken()
762767
if lookahead.token == EOF
763768
|| source.offsetToLine(lookahead.offset) > endLine
@@ -1266,6 +1271,7 @@ object Scanners {
12661271
putChar(ch) ; nextRawChar()
12671272
loopRest()
12681273
else
1274+
next.lineOffset = if next.lastOffset < lineStartOffset then lineStartOffset else -1
12691275
finishNamedToken(IDENTIFIER, target = next)
12701276
end loopRest
12711277
setStrVal()
@@ -1312,10 +1318,10 @@ object Scanners {
13121318
}
13131319
end getStringPart
13141320

1315-
private def fetchStringPart(multiLine: Boolean) = {
1321+
private def fetchStringPart(multiLine: Boolean) =
13161322
offset = charOffset - 1
1323+
lineOffset = if lastOffset < lineStartOffset then lineStartOffset else -1
13171324
getStringPart(multiLine)
1318-
}
13191325

13201326
private def isTripleQuote(): Boolean =
13211327
if (ch == '"') {
@@ -1657,21 +1663,31 @@ object Scanners {
16571663
case Run(ch: Char, n: Int)
16581664
case Conc(l: IndentWidth, r: Run)
16591665

1660-
def <= (that: IndentWidth): Boolean = this match {
1661-
case Run(ch1, n1) =>
1662-
that match {
1663-
case Run(ch2, n2) => n1 <= n2 && (ch1 == ch2 || n1 == 0)
1664-
case Conc(l, r) => this <= l
1665-
}
1666-
case Conc(l1, r1) =>
1667-
that match {
1668-
case Conc(l2, r2) => l1 == l2 && r1 <= r2
1669-
case _ => false
1670-
}
1671-
}
1666+
def <= (that: IndentWidth): Boolean = (this, that) match
1667+
case (Run(ch1, n1), Run(ch2, n2)) => n1 <= n2 && (ch1 == ch2 || n1 == 0)
1668+
case (Conc(l1, r1), Conc(l2, r2)) => (l1 == l2 && r1 <= r2) || this <= l2
1669+
case (_, Conc(l2, _)) => this <= l2
1670+
case _ => false
16721671

16731672
def < (that: IndentWidth): Boolean = this <= that && !(that <= this)
16741673

1674+
def >= (that: IndentWidth): Boolean = that <= this
1675+
1676+
def >(that: IndentWidth): Boolean = that < this
1677+
1678+
def size: Int = this match
1679+
case Run(_, n) => n
1680+
case Conc(l, r) => l.size + r.n
1681+
1682+
/** Add one level of indentation (one tab or two spaces depending on the last char) */
1683+
def increment: IndentWidth =
1684+
def incRun(ch: Char, n: Int): Run = ch match
1685+
case ' ' => IndentWidth.Run(' ', n + 2)
1686+
case ch => IndentWidth.Run(ch, n + 1)
1687+
this match
1688+
case Run(ch, n) => incRun(ch, n)
1689+
case Conc(l, Run(ch, n)) => Conc(l, incRun(ch, n))
1690+
16751691
/** Does `this` differ from `that` by not more than a single space? */
16761692
def isClose(that: IndentWidth): Boolean = this match
16771693
case Run(ch1, n1) =>

compiler/src/dotty/tools/dotc/parsing/xml/MarkupParsers.scala

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,8 +324,7 @@ object MarkupParsers {
324324
/** Some try/catch/finally logic used by xLiteral and xLiteralPattern. */
325325
inline private def xLiteralCommon(f: () => Tree, ifTruncated: String => Unit): Tree = {
326326
assert(parser.in.token == Tokens.XMLSTART)
327-
val saved = parser.in.newTokenData
328-
saved.copyFrom(parser.in)
327+
val saved = parser.in.saveCopy
329328
var output: Tree = null.asInstanceOf[Tree]
330329
try output = f()
331330
catch {
@@ -404,7 +403,7 @@ object MarkupParsers {
404403
def escapeToScala[A](op: => A, kind: String): A = {
405404
xEmbeddedBlock = false
406405
val res = saving(parser.in.currentRegion, parser.in.currentRegion = _) {
407-
val lbrace = parser.in.newTokenData
406+
val lbrace = Scanners.newTokenData
408407
lbrace.token = LBRACE
409408
lbrace.offset = parser.in.charOffset - 1
410409
lbrace.lastOffset = parser.in.lastOffset

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

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,13 @@ object Rewrites {
2525
def addPatch(span: Span, replacement: String): Unit =
2626
pbuf += Patch(span, replacement)
2727

28+
def patchOver(span: Span, replacement: String): Unit =
29+
pbuf.indices.reverse.find(i => span.contains(pbuf(i).span)).foreach(pbuf.remove)
30+
pbuf += Patch(span, replacement)
31+
2832
def apply(cs: Array[Char]): Array[Char] = {
2933
val delta = pbuf.map(_.delta).sum
30-
val patches = pbuf.toList.sortBy(_.span.start)
34+
val patches = pbuf.toList.sortBy(p => (p.span.start, p.span.end))
3135
if (patches.nonEmpty)
3236
patches.reduceLeft {(p1, p2) =>
3337
assert(p1.span.end <= p2.span.start, s"overlapping patches in $source: $p1 and $p2")
@@ -71,6 +75,14 @@ object Rewrites {
7175
.addPatch(span, replacement)
7276
)
7377

78+
/** Record a patch that replaces the first patch that it contains */
79+
def patchOver(source: SourceFile, span: Span, replacement: String)(using Context): Unit =
80+
if ctx.reporter != Reporter.NoReporter // NoReporter is used for syntax highlighting
81+
then ctx.settings.rewrite.value.foreach(_.patched
82+
.getOrElseUpdate(source, new Patches(source))
83+
.patchOver(span, replacement)
84+
)
85+
7486
/** Patch position in `ctx.compilationUnit.source`. */
7587
def patch(span: Span, replacement: String)(using Context): Unit =
7688
patch(ctx.compilationUnit.source, span, replacement)

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -82,9 +82,14 @@ class CompilationTests {
8282
compileFile("tests/rewrites/rewrites3x.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
8383
compileFile("tests/rewrites/filtering-fors.scala", defaultOptions.and("-rewrite", "-source", "3.2-migration")),
8484
compileFile("tests/rewrites/refutable-pattern-bindings.scala", defaultOptions.and("-rewrite", "-source", "3.2-migration")),
85-
compileFile("tests/rewrites/i8982.scala", defaultOptions.and("-indent", "-rewrite")),
86-
compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")),
87-
compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")),
85+
compileFile("tests/rewrites/i8982.scala", indentRewrite),
86+
compileFile("tests/rewrites/i9632.scala", indentRewrite),
87+
compileFile("tests/rewrites/i11895.scala", indentRewrite),
88+
compileFile("tests/rewrites/indent-rewrite.scala", indentRewrite),
89+
compileFile("tests/rewrites/indent-comments.scala", indentRewrite),
90+
compileFile("tests/rewrites/indent-mix-tab-space.scala", indentRewrite),
91+
compileFile("tests/rewrites/indent-3-spaces.scala", indentRewrite),
92+
compileFile("tests/rewrites/indent-mix-brace.scala", indentRewrite),
8893
compileFile("tests/rewrites/i12340.scala", unindentOptions.and("-rewrite")),
8994
compileFile("tests/rewrites/i17187.scala", unindentOptions.and("-rewrite")),
9095
compileFile("tests/rewrites/i17399.scala", unindentOptions.and("-rewrite")),

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ object TestConfiguration {
6565

6666
val commonOptions = Array("-indent") ++ checkOptions ++ noCheckOptions ++ yCheckOptions
6767
val defaultOptions = TestFlags(basicClasspath, commonOptions)
68+
val indentRewrite = defaultOptions.and("-rewrite")
6869
val unindentOptions = TestFlags(basicClasspath, Array("-no-indent") ++ checkOptions ++ noCheckOptions ++ yCheckOptions)
6970
val withCompilerOptions =
7071
defaultOptions.withClasspath(withCompilerClasspath).withRunClasspath(withCompilerClasspath)

tests/pos/indent-colons.scala

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,11 @@ object Test23:
152152
val _ = 1 `+`: // ok
153153
x
154154

155+
// leading infix op
156+
val _ = 1
157+
`+` :
158+
x
159+
155160
val r = 1 to:
156161
100
157162

tests/rewrites/indent-3-spaces.check

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Rewrite to indent, keeping 3 spaces as indentation
2+
3+
def m1 =
4+
def m2 =
5+
"" +
6+
"" +
7+
""
8+
m2
9+
10+
def m4 =
11+
def m5 =
12+
def m6 =
13+
val x = ""
14+
x
15+
.apply(0)
16+
.toString
17+
m6
18+
.toString
19+
m5 +
20+
m5
21+
.toString

tests/rewrites/indent-3-spaces.scala

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
// Rewrite to indent, keeping 3 spaces as indentation
2+
3+
def m1 = {
4+
def m2 = {
5+
"" +
6+
"" +
7+
""
8+
}
9+
m2
10+
}
11+
12+
def m4 = {
13+
def m5 = {
14+
def m6 = {
15+
val x = ""
16+
x
17+
.apply(0)
18+
.toString
19+
}
20+
m6
21+
.toString
22+
}
23+
m5 +
24+
m5
25+
.toString
26+
}

0 commit comments

Comments
 (0)