Skip to content
This repository has been archived by the owner on Sep 3, 2020. It is now read-only.

Commit

Permalink
Be more careful when printing type parameter lists
Browse files Browse the repository at this point in the history
This commit fixes some problems in the code, related to printing type parameter
lists: It is no longer assumed, that the user prefers `A, B` over `A,B`, even
if the original source code uses `A,B`. Also, layout splitting used to mess
with the code, if newlines and comments where involved. This issue is addressed
by adapting a few regular expressions in `LayoutHelper`, as well as by changing
the order in which they are matched.

Note that this commit replaces ", " with "," in quite a few places, since " "
should now be part of the arguments themselves if it is needed. Each of these
changes was motivated by failing unit tests. Since there are still lots of
hard coded ", " in the library, we might see additional replacements of this
type in the future.

Fixes #1002618
  • Loading branch information
mlangc committed Dec 19, 2015
1 parent c5fa88e commit 262f85c
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ trait LayoutHelper {
private val Equals = """(?ms)(.*?=\s?)(.*)""".r
private val ClosingBrace = """(?ms)(.*?)\)(.*)""".r
private val ClosingCurlyBrace = """(?ms)(.*?\}\s*)(\r?\n.*)""".r
private val Comma = """(.*?),(.*)""".r
private val Comma = """(?ms)(.*?),(.*)""".r
private val CommaSpace = """(.*?), (.*)""".r
private val NewLine = """(?ms)(.*?)(\r?\n.*)""".r
private val ImportStatementNewline = """(?ms)(.*)(\r?\n.*?import.*)""".r
Expand Down Expand Up @@ -322,11 +322,10 @@ trait LayoutHelper {
case ImportStatementNewline(l, r) => (l, r, "ImportStatement Newline")
case ImportStatement(l, r) => (l, r, "ImportStatement")
case ClosingCurlyBrace(l, r)=> (l, r, "ClosingCurlyBrace")
case NewLine(l, r) => (l, r, "NewLine")
case CommaSpace(l, r) => (l, r, "CommaSpace")
case Comma(l, r) => (l, r, "Comma")
case Dot(l, r) => (l, r, "Dot")
case OpeningSquareBracket(l, r) => (l, r, "OpeningSquareBracket")
case NewLine(l, r) => (l, r, "NewLine")
case s => (s, "", "NoMatch")
}
}
Expand Down Expand Up @@ -414,8 +413,8 @@ trait LayoutHelper {
* We remove all leading or trailing commas, they always need to be re-introduced by the printers.
*/
def removeLeadingOrTrailingComma(s: String) = {
val CommaAtStart = "(?ms),\\s?(.*)".r
val CommaAtEnd = "(?ms)(.*),\\s?".r
val CommaAtStart = """(?ms),(\s*)""".r
val CommaAtEnd = """(?ms)(\s*),\s*""".r
s match {
case CommaAtStart(rest) => rest
case CommaAtEnd(rest) => rest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ trait PrettyPrinter extends TreePrintingTraversals with AbstractPrinter {
p(impl)
}

Fragment(mods_ + keyword + name) ++ pp(tparams, before = "[", separator = ", ", after = "]") ++ body.ifNotEmpty {
Fragment(mods_ + keyword + name) ++ pp(tparams, before = "[", separator = "," ++ Requisite.Blank, after = "]") ++ body.ifNotEmpty {
case body if body.asText.startsWith("{") =>
Layout(" ") ++ body
case body =>
Expand Down Expand Up @@ -584,7 +584,7 @@ trait PrettyPrinter extends TreePrintingTraversals with AbstractPrinter {
// Finalize this fragment so that the anywhere-requisite gets applied here
// and does not match on ] that might come later (see testNewDefDefWithOriginalContent3
// and testDefDefWithTypeParams).
pp(tparams, before = "[", after = anywhere("]"), separator = ", ").toLayout
pp(tparams, before = "[", after = anywhere("]"), separator = "," ++ Requisite.Blank).toLayout
}

// if there's existing layout, the type parameter's layout might already contain "()"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ trait ReusingPrinter extends TreePrintingTraversals with AbstractPrinter with Sc
}

override def UnApply(tree: UnApply, fun: Tree, args: List[Tree])(implicit ctx: PrintingContext) = {
l ++ p(fun) ++ pp(args, separator = ", ", before = "(", after = ")") ++ r
l ++ p(fun) ++ pp(args, separator = ",", before = "(", after = ")") ++ r
}

override def Match(tree: Match, selector: Tree, cases: List[Tree])(implicit ctx: PrintingContext) = {
Expand Down Expand Up @@ -702,7 +702,7 @@ trait ReusingPrinter extends TreePrintingTraversals with AbstractPrinter with Sc
} else {
val arguments = args.init
val ret = args.last
l ++ pp(arguments, before = "(", separator = ", ", after = Requisite.anywhere(")")) ++ p(ret) ++ r
l ++ pp(arguments, before = "(", separator = ",", after = Requisite.anywhere(")")) ++ p(ret) ++ r
}
}

Expand All @@ -721,7 +721,7 @@ trait ReusingPrinter extends TreePrintingTraversals with AbstractPrinter with Sc
else ("[", "]")
}

l ++ p(tpt) ++ pp(args, before = beforeArgs, separator = ", ", after = afterArgs) ++ r
l ++ p(tpt) ++ pp(args, before = beforeArgs, separator = Requisite.allowSurroundingWhitespace(","), after = afterArgs) ++ r
}
}

Expand Down Expand Up @@ -1023,14 +1023,14 @@ trait ReusingPrinter extends TreePrintingTraversals with AbstractPrinter with Sc
case (x: TypeDef) :: Nil =>
p(x)
case (x: TypeDef) :: (y: TypeDef) :: rest =>
p(x) ++ ", " ++ mergeTypeParameters(y :: rest)
p(x) ++ "," ++ mergeTypeParameters(y :: rest)
case (x: TypeDef) :: rest =>
val (bounds, next) = rest.span(!_.isInstanceOf[TypeDef])
val current = pp(x :: bounds, separator = ": ")
if (next.isEmpty) {
current
} else {
current ++ ", " ++ mergeTypeParameters(next)
current ++ "," ++ mergeTypeParameters(next)
}
case _ =>
EmptyFragment
Expand Down Expand Up @@ -1096,7 +1096,7 @@ trait ReusingPrinter extends TreePrintingTraversals with AbstractPrinter with Sc
this: TreePrinting with PrintingUtils =>

override def SuperConstructorCall(tree: SuperConstructorCall, clazz: global.Tree, args: List[global.Tree])(implicit ctx: PrintingContext) = {
l ++ p(clazz) ++ pp(args, separator = ", ", before = "(", after = ")") ++ r
l ++ p(clazz) ++ pp(args, separator = ",", before = "(", after = ")") ++ r
}

override def Super(tree: Super, qual: Tree, mix: Name)(implicit ctx: PrintingContext) = {
Expand Down

0 comments on commit 262f85c

Please sign in to comment.