Skip to content

Commit

Permalink
PrettyPrinter bug fix on Term.Block
Browse files Browse the repository at this point in the history
  • Loading branch information
@RCMartins
RCMartins committed May 27, 2023
1 parent d95145b commit b420975
Show file tree
Hide file tree
Showing 2 changed files with 232 additions and 2 deletions.
77 changes: 75 additions & 2 deletions scalameta/trees/shared/src/main/scala/scala/meta/internal/prettyprinters/TreeSyntax.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,65 @@ object TreeSyntax {
}
!isOnlyChildOfOnlyChild(t)
}
def guessNeedsLineSep(t: Template): Boolean =
t.stats.isEmpty
@annotation.tailrec
def guessNeedsLineSep(t: Stat): Boolean = t match {
case _: Decl.Val => true
case _: Decl.Var => true
case _: Decl.Def => true
case _: Decl.Type => false
case Defn.Val(_, _, _, rhs) => guessNeedsLineSep(rhs)
case Defn.Var.After_4_7_2(_, _, _, body) => guessNeedsLineSep(body)
case Defn.Def.After_4_7_3(_, _, _, _, body) => guessNeedsLineSep(body)
case _: Defn.Macro => true
case _: Defn.Type => true
case Defn.Class.After_4_6_0(_, _, _, _, template) => guessNeedsLineSep(template)
case Defn.Trait.After_4_6_0(_, _, _, _, template) => guessNeedsLineSep(template)
case Defn.Object(_, _, template) => guessNeedsLineSep(template)
case _: Term.This => true
case _: Term.Super => unreachable
case _: Term.Name => true
case _: Term.Select => true
case Term.ApplyUnary(_, arg) => guessNeedsLineSep(arg)
case _: Term.Apply => true
case _: Term.ApplyType => true
case Term.ApplyInfix.After_4_6_0(_, _, _, Term.ArgClause(term :: Nil, _)) =>
guessNeedsLineSep(term)
case _: Term.ApplyInfix => true
case Term.Assign(_, rhs) => guessNeedsLineSep(rhs)
case Term.Return(expr) => guessNeedsLineSep(expr)
case Term.Throw(expr) => guessNeedsLineSep(expr)
case _: Term.Ascribe => true
case _: Term.Annotate => false
case _: Term.Tuple => true
case _: Term.Block => false
case Term.If.After_4_4_0(_, thenp, Lit.Unit(), _) => guessNeedsLineSep(thenp)
case Term.If.After_4_4_0(_, _, elsep, _) => guessNeedsLineSep(elsep)
case _: Term.Match => false
case Term.Try(_, _, Some(term)) => guessNeedsLineSep(term)
case Term.Try(_, _ :: _, _) => false
case Term.Try(expr, Nil, None) => guessNeedsLineSep(expr)
case Term.TryWithHandler(_, _, Some(term)) => guessNeedsLineSep(term)
case Term.TryWithHandler(_, term, _) => guessNeedsLineSep(term)
case Term.Function.After_4_6_0(_, body) => guessNeedsLineSep(body)
case _: Term.PartialFunction => false
case Term.While(_, body) => guessNeedsLineSep(body)
case _: Term.Do => false
case Term.For(_, body) => guessNeedsLineSep(body)
case Term.ForYield(_, body) => guessNeedsLineSep(body)
case _: Term.New => true
case _: Term.NewAnonymous => false
case _: Term.Placeholder => unreachable
case _: Term.Eta => false
case _: Term.Repeated => false
case _: Term.Param => unreachable
case _: Term.Interpolate => true
case _: Term.Xml => true
case _: Import => false
case _: Lit => true
case _ => false
}

// Branches
implicit def syntaxTree[T <: Tree]: Syntax[T] = Syntax {
Expand Down Expand Up @@ -398,10 +457,24 @@ object TreeSyntax {
case t: Term.Tuple => m(SimpleExpr1, s("(", r(t.args, ", "), ")"))
case t: Term.Block =>
import Term.{Block, Function}
def block(pre: Show.Result, x: Seq[Stat]) = {
def requireNewLines(stats: List[Stat]): List[Show.Result] =
stats
.foldLeft((false, List.empty[Show.Result])) {
case ((req, acc), block: Block) => (false, r(if (req) EOL else "", i(block)) :: acc)
case ((_, acc), stat) => (guessNeedsLineSep(stat), i(stat) :: acc)
}
._2
.reverse
def block(pre: Show.Result, x: List[Stat]) = {
val res =
if (pre == Show.None && x.isEmpty) s("{}")
else s("{", w(" ", pre), r(x.map(i(_))), n("}"))
else
s(
"{",
w(" ", pre),
r(if (dialect.allowSignificantIndentation) x.map(i(_)) else requireNewLines(x)),
n("}")
)
m(SimpleExpr, res)
}
t match {
Expand Down
157 changes: 157 additions & 0 deletions tests/shared/src/test/scala/scala/meta/tests/prettyprinters/SyntacticSuite.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1933,4 +1933,161 @@ class SyntacticSuite extends scala.meta.tests.parsers.ParseSuite {
assertSyntax(tree, "\"\"\"\n\\\"\\\"\\\"\"\"\"")(tree)
}

test("#2046 new line separator required before some Term.Block") {
assertEquals(
blockStat("{val a = 1; {b}; val c = 1}").syntax,
s"""
|{
| val a = 1
|
| {
| b
| }
| val c = 1
|}
""".trim.stripMargin.replace("\n", EOL)
)
assertEquals(
blockStat("{val a = 1; {b}; {c}}").syntax,
s"""
|{
| val a = 1
|
| {
| b
| }
| {
| c
| }
|}
""".trim.stripMargin.replace("\n", EOL)
)
assertEquals(
blockStat("{{a}; val b = 1; val c = 2; {d}}").syntax,
s"""
|{
| {
| a
| }
| val b = 1
| val c = 2
|
| {
| d
| }
|}
""".trim.stripMargin.replace("\n", EOL)
)

def testBlock(t: Stat, expectedStr: String, needSep: Boolean): Unit = {
def expected(sep: String): String =
s"""
|{
|${expectedStr.split("\n").map(" " + _).mkString("", "\n", sep)}
| {
| a
| }
|}
""".trim.stripMargin.replace("\n", EOL)
val newBlock = q"$t;{a}"
assertEquals(
newBlock.syntax,
expected(sep = if (needSep) "\n" else "")
)
assertEquals(
newBlock.structure,
templStat(newBlock.syntax).structure
)
// If it compiles it cannot produce the same tree, otherwise the separator has unnecessary
if (needSep)
scala.util.Try(templStat(expected(sep = ""))).toOption.foreach { statNoSep =>
assertNotEquals(
newBlock.structure,
statNoSep.structure
)
}
}

def sep(t: Stat, expected: String): Unit = testBlock(t, expected, needSep = true)
def noSep(t: Stat, expected: String): Unit = testBlock(t, expected, needSep = false)

def declTest(f: String => Unit): Unit = Seq("val", "var", "def").foreach(f)
def classTest(f: String => Unit): Unit = Seq("class", "object", "trait").foreach(f)

declTest(k => sep(templStat(s"$k foo: Int"), s"$k foo: Int"))
noSep(templStat("type foo"), "type foo")
declTest(k => sep(templStat(s"$k foo: Int = 1"), s"$k foo: Int = 1"))
declTest(k => noSep(templStat(s"$k foo: Int = {1}"), s"$k foo: Int = {\n 1\n}"))
sep(templStat("def a = macro someMacro"), "def a = macro someMacro")
sep(templStat("type foo = Int"), "type foo = Int")
classTest(k => sep(templStat(s"$k Foo"), s"$k Foo"))
classTest(k => noSep(templStat(s"$k Foo {val foo = 1}"), s"$k Foo { val foo = 1 }"))
classTest(k => sep(templStat(s"$k Foo extends Bar"), s"$k Foo extends Bar"))
classTest(k =>
sep(
templStat(s"$k Foo extends {val foo = 1} with Bar"),
s"$k Foo extends { val foo = 1 } with Bar"
)
)
classTest(k =>
noSep(
templStat(s"$k Foo extends {val foo = 1} with Bar {val bar = 2}"),
s"$k Foo extends { val foo = 1 } with Bar { val bar = 2 }"
)
)
sep(templStat("this"), "this")
sep(templStat("Foo"), "Foo")
sep(templStat("Foo.bar"), "Foo.bar")
sep(templStat("Foo.bar"), "Foo.bar")
sep(templStat("-10"), "-10")
noSep(templStat("-{10}"), "-{\n 10\n}")
sep(templStat("foo(bar)"), "foo(bar)")
sep(templStat("foo[Bar]"), "foo[Bar]")
sep(templStat("foo + bar"), "foo + bar")
noSep(templStat("foo + {bar}"), "foo + {\n bar\n}")
// sep(templStat("foo + (a, b)"), "foo + (a, b)") TODO bug #3138
sep(templStat("foo = bar"), "foo = bar")
noSep(templStat("foo = {bar}"), "foo = {\n bar\n}")
sep(templStat("return foo"), "return foo")
noSep(templStat("return {foo}"), "return {\n foo\n}")
sep(templStat("throw foo"), "throw foo")
noSep(templStat("throw {foo}"), "throw {\n foo\n}")
sep(templStat("foo: Int"), "foo: Int")
noSep(templStat("foo: @annotation"), "foo: @annotation")
sep(templStat("(foo, bar)"), "(foo, bar)")
noSep(templStat("{foo}"), "{\n foo\n}")
sep(templStat("if (cond) foo"), "if (cond) foo")
noSep(templStat("if (cond) {foo}"), "if (cond) {\n foo\n}")
sep(templStat("if (cond) foo else bar"), "if (cond) foo else bar")
noSep(templStat("if (cond) foo else {bar}"), "if (cond) foo else {\n bar\n}")
noSep(templStat("foo match { case _ => () }"), "foo match {\n case _ => ()\n}")
sep(templStat("try foo finally bar"), "try foo finally bar")
noSep(templStat("try foo finally {bar}"), "try foo finally {\n bar\n}")
noSep(templStat("try foo catch { case _ => () }"), "try foo catch {\n case _ => ()\n}")
sep(templStat("try foo"), "try foo")
noSep(templStat("try {foo}"), "try {\n foo\n}")
sep(templStat("try foo catch bar finally baz"), "try foo catch bar finally baz")
noSep(templStat("try foo catch bar finally {baz}"), "try foo catch bar finally {\n baz\n}")
sep(templStat("try foo catch bar"), "try foo catch bar")
// sep(templStat("try foo catch {bar}"), "try foo catch {\n bar\n}") TODO bug #3136
sep(templStat("val func = foo => bar"), "val func = foo => bar")
noSep(templStat("val func = { case foo => bar }"), "val func = {\n case foo => bar\n}")
sep(templStat("while (foo) bar"), "while (foo) bar")
noSep(templStat("while (foo) {bar}"), "while (foo) {\n bar\n}")
noSep(templStat("do foo while (bar)"), "do foo while (bar)")
sep(templStat("for (foo <- bar) baz"), "for (foo <- bar) baz")
noSep(templStat("for (foo <- bar) {baz}"), "for (foo <- bar) {\n baz\n}")
sep(templStat("for (foo <- bar) yield baz"), "for (foo <- bar) yield baz")
noSep(templStat("for (foo <- bar) yield {baz}"), "for (foo <- bar) yield {\n baz\n}")
sep(templStat("new Foo"), "new Foo")
noSep(templStat("foo _"), "foo _")
// Term.Repeated can only be in a block by itself, otherwise is invalid syntax
sep(templStat("foo { bar: _* }"), "foo {\n bar: _*\n}")
sep(templStat("s\"foo\""), "s\"foo\"")
sep(templStat("<h1>{Foo}</h1>"), "<h1>{Foo}</h1>")
noSep(templStat("import foo.Bar"), "import foo.Bar")
Seq("true", "'a'", "1.0d", "1.0f", "1", "1L", "null", "\"foo\"", "'foo", "()")
.foreach { lit => sep(templStat(lit), lit) }
}

}

0 comments on commit b420975

Please sign in to comment.