Skip to content

Commit e355923

Browse files
committed
Patch indentation with -indent -rewrite
Ensure indentation is correct 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 } ```
1 parent 9923e29 commit e355923

15 files changed

+607
-198
lines changed

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

+237-179
Large diffs are not rendered by default.

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

+31-16
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ object Scanners {
483483
if (nextChar == ch)
484484
recur(idx - 1, ch, n + 1, k)
485485
else {
486-
val k1: IndentWidth => IndentWidth = if (n == 0) k else Conc(_, Run(ch, n))
486+
val k1: IndentWidth => IndentWidth = if (n == 0) k else iw => k(Conc(iw, Run(ch, n)))
487487
recur(idx - 1, nextChar, 1, k1)
488488
}
489489
else recur(idx - 1, ' ', 0, identity)
@@ -523,7 +523,7 @@ object Scanners {
523523
*
524524
* The following tokens can start an indentation region:
525525
*
526-
* : = => <- if then else while do try catch
526+
* : = => <- if then else while do try catch
527527
* finally for yield match throw return with
528528
*
529529
* Inserting an INDENT starts a new indentation region with the indentation of the current
@@ -1266,6 +1266,7 @@ object Scanners {
12661266
putChar(ch) ; nextRawChar()
12671267
loopRest()
12681268
else
1269+
next.lineOffset = if next.lastOffset < lineStartOffset then lineStartOffset else -1
12691270
finishNamedToken(IDENTIFIER, target = next)
12701271
end loopRest
12711272
setStrVal()
@@ -1312,10 +1313,10 @@ object Scanners {
13121313
}
13131314
end getStringPart
13141315

1315-
private def fetchStringPart(multiLine: Boolean) = {
1316+
private def fetchStringPart(multiLine: Boolean) =
13161317
offset = charOffset - 1
1318+
lineOffset = if lastOffset < lineStartOffset then lineStartOffset else -1
13171319
getStringPart(multiLine)
1318-
}
13191320

13201321
private def isTripleQuote(): Boolean =
13211322
if (ch == '"') {
@@ -1657,21 +1658,35 @@ object Scanners {
16571658
case Run(ch: Char, n: Int)
16581659
case Conc(l: IndentWidth, r: Run)
16591660

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-
}
1661+
def <= (that: IndentWidth): Boolean = (this, that) match
1662+
case (Run(ch1, n1), Run(ch2, n2)) => n1 <= n2 && (ch1 == ch2 || n1 == 0)
1663+
case (Conc(l1, r1), Conc(l2, r2)) => (l1 == l2 && r1 <= r2) || this <= l2
1664+
case (_, Conc(l2, _)) => this <= l2
1665+
case _ => false
16721666

16731667
def < (that: IndentWidth): Boolean = this <= that && !(that <= this)
16741668

1669+
def >= (that: IndentWidth): Boolean = (this, that) match
1670+
case (Run(ch1, n1), Run(ch2, n2)) => n1 >= n2 && (ch1 == ch2 || n2 == 0)
1671+
case (Conc(l1, r1), Conc(l2, r2)) => (l1 == l2 && r1 >= r2) || l1 >= that
1672+
case (Conc(l1, _), _) => l1 >= that
1673+
case _ => false
1674+
1675+
def >(that: IndentWidth): Boolean = this >= that && !(that >= this)
1676+
1677+
def size: Int = this match
1678+
case Run(_, n) => n
1679+
case Conc(l, r) => l.size + r.n
1680+
1681+
/** Add one level of indentation (one tab or two spaces depending on the last char)**/
1682+
def increment: IndentWidth =
1683+
def incRun(ch: Char, n: Int): Run = ch match
1684+
case ' ' => IndentWidth.Run(' ', n + 2)
1685+
case ch => IndentWidth.Run(ch, n + 1)
1686+
this match
1687+
case Run(ch, n) => incRun(ch, n)
1688+
case Conc(l, Run(ch, n)) => Conc(l, incRun(ch, n))
1689+
16751690
/** Does `this` differ from `that` by not more than a single space? */
16761691
def isClose(that: IndentWidth): Boolean = this match
16771692
case Run(ch1, n1) =>

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

+12
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ 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
3034
val patches = pbuf.toList.sortBy(_.span.start)
@@ -71,6 +75,14 @@ object Rewrites {
7175
.addPatch(span, replacement)
7276
)
7377

78+
/** Record a patch that replaces an inner patch */
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

+8-3
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,14 @@ class CompilationTests {
7777
compileFile("tests/rewrites/rewrites3x.scala", defaultOptions.and("-rewrite", "-source", "future-migration")),
7878
compileFile("tests/rewrites/filtering-fors.scala", defaultOptions.and("-rewrite", "-source", "3.2-migration")),
7979
compileFile("tests/rewrites/refutable-pattern-bindings.scala", defaultOptions.and("-rewrite", "-source", "3.2-migration")),
80-
compileFile("tests/rewrites/i8982.scala", defaultOptions.and("-indent", "-rewrite")),
81-
compileFile("tests/rewrites/i9632.scala", defaultOptions.and("-indent", "-rewrite")),
82-
compileFile("tests/rewrites/i11895.scala", defaultOptions.and("-indent", "-rewrite")),
80+
compileFile("tests/rewrites/i8982.scala", indentRewrite),
81+
compileFile("tests/rewrites/i9632.scala", indentRewrite),
82+
compileFile("tests/rewrites/i11895.scala", indentRewrite),
83+
compileFile("tests/rewrites/indent-rewrite.scala", indentRewrite),
84+
compileFile("tests/rewrites/indent-comments.scala", indentRewrite),
85+
compileFile("tests/rewrites/indent-mix-tab-space.scala", indentRewrite),
86+
compileFile("tests/rewrites/indent-3-spaces.scala", indentRewrite),
87+
compileFile("tests/rewrites/indent-mix-brace.scala", indentRewrite),
8388
compileFile("tests/rewrites/i12340.scala", unindentOptions.and("-rewrite")),
8489
compileFile("tests/rewrites/i17187.scala", unindentOptions.and("-rewrite")),
8590
compileFile("tests/rewrites/i17399.scala", unindentOptions.and("-rewrite")),

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

+1
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/rewrites/indent-3-spaces.check

+21
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

+26
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+
}

tests/rewrites/indent-comments.check

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Rewriting to indent should not remove the comments
2+
class A /** 1 */ : /** 2 */
3+
def m1 = /** 3 */ /** 4 */
4+
val x = if (true)
5+
/** 5 */
6+
"true"
7+
/** 6 */
8+
else
9+
/** 7 */
10+
"false"
11+
/** 8 */
12+
/** 9 */ x
13+
/** 10 */ /** 11 */
14+
/** 12 */

tests/rewrites/indent-comments.scala

+14
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
// Rewriting to indent should not remove the comments
2+
class A /** 1 */ { /** 2 */
3+
def m1 = /** 3 */ { /** 4 */
4+
val x = if (true)
5+
/** 5 */ {
6+
"true"
7+
} /** 6 */
8+
else
9+
{ /** 7 */
10+
"false"
11+
/** 8 */ }
12+
/** 9 */ x
13+
/** 10 */ } /** 11 */
14+
/** 12 */ }

tests/rewrites/indent-mix-brace.check

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// A mix of nested in-brace regions and indented regions
2+
3+
class A:
4+
def m1 =
5+
""
6+
7+
def m2 =
8+
def m3 =
9+
val x = ""
10+
x
11+
m3
12+
13+
class B:
14+
def foo =
15+
def bar =
16+
""
17+
bar

tests/rewrites/indent-mix-brace.scala

+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// A mix of nested in-brace regions and indented regions
2+
3+
class A:
4+
def m1 = {
5+
""
6+
}
7+
8+
def m2 = {
9+
def m3 =
10+
val x = ""
11+
x
12+
m3
13+
}
14+
15+
class B {
16+
def foo =
17+
def bar = {
18+
""
19+
}
20+
bar
21+
}
+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
// Contains an ugly but valid mix of spaces and tabs
2+
// Rewrite to significant indentation syntax
3+
4+
def m1 =
5+
def m2 =
6+
"" +
7+
"" +
8+
""
9+
m2
10+
11+
def m4 =
12+
def m5 =
13+
def m6 =
14+
val x = ""
15+
x
16+
.apply(0)
17+
.toString
18+
m6
19+
.toString
20+
m5 +
21+
m5
22+
.toString
+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Contains an ugly but valid mix of spaces and tabs
2+
// Rewrite to significant indentation syntax
3+
4+
def m1 = {
5+
def m2 = {
6+
"" +
7+
"" +
8+
""
9+
}
10+
m2
11+
}
12+
13+
def m4 = {
14+
def m5 = {
15+
def m6 = {
16+
val x = ""
17+
x
18+
.apply(0)
19+
.toString
20+
}
21+
m6
22+
.toString
23+
}
24+
m5 +
25+
m5
26+
.toString
27+
}

tests/rewrites/indent-rewrite.check

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
// A collection of patterns/bugs found when rewriting the community build to indent
2+
3+
trait A:
4+
5+
class B
6+
// do not remove braces if empty region
7+
class C {
8+
9+
}
10+
// do not remove braces if open brace is not followed by new line
11+
def m1(x: Int) =
12+
{ x
13+
.toString
14+
}
15+
// add indent to pass an argument (fewer braces)
16+
def m2: String =
17+
m1:
18+
5
19+
// indent inner method
20+
def m3: Int =
21+
def seq =
22+
Seq(
23+
"1",
24+
"2"
25+
)
26+
seq
27+
(1)
28+
.toInt
29+
// indent refinement
30+
def m4: Any:
31+
def foo: String
32+
=
33+
new:
34+
def foo: String =
35+
"""
36+
Hello, World!
37+
"""
38+
// indent end marker
39+
end m4
40+
41+
// do not indent if closing braces not followed by new line
42+
def m5: String = {
43+
val x = "Hi"
44+
x
45+
}; def m6(x: String): String =
46+
s"""Bye $x ${
47+
x
48+
}
49+
do not indent in a multiline string"""
50+
def m7 =
51+
val foo = ""
52+
val x = Seq(
53+
s"${foo}",
54+
""
55+
)
56+
57+
// preserve indentation style before 'case'
58+
def m9(o: Option[String]) =
59+
o match
60+
case Some(x) => x
61+
case None => ""
62+
63+
o match
64+
case Some(x) => x
65+
case None => ""
66+
67+
// preserve indent of select after {}
68+
def m8(xs: Seq[String]) =
69+
xs
70+
.filter:
71+
_ => true
72+
.map(s => s * 2)

0 commit comments

Comments
 (0)