Skip to content

Commit

Permalink
RedundantBraces: remove inner braces in argclause
Browse files Browse the repository at this point in the history
Instead of removing outer braces after checking that an inner expression
will provide own, let's keep outer braces and remove them in the inner
if appropriate.
  • Loading branch information
kitbellew committed Dec 22, 2024
1 parent 211c8af commit 2980545
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 68 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,8 @@ class RedundantBraces(implicit val ftoks: FormatTokens)
// single-arg apply of a partial function or optionally any arg
// a({ case b => c; d }) change to a { case b => c; d }
case _: Term.PartialFunction => replaceWithNextBrace()
case _: Term.Block =>
val ok = getBlockNestedPartialFunction(arg).nonEmpty
case b: Term.Block =>
val ok = getTreeSingleExpr(b).is[Term.PartialFunction]
if (ok) replaceWithNextBrace() else null
case f: Term.FunctionTerm =>
getBlockToReplaceAsFuncBodyInSingleArgApply(ta, f) match {
Expand Down Expand Up @@ -297,11 +297,18 @@ class RedundantBraces(implicit val ftoks: FormatTokens)
owner match {
case t: Term.FunctionTerm if t.tokens.last.is[T.RightBrace] =>
if (!okToRemoveFunctionInApplyOrInit(t)) null else removeToken
case _: Term.PartialFunction =>
case t: Term.PartialFunction =>
val pft = ftoks.prevNonComment(ft)
pft.left match {
case _: T.LeftParen if isLeftParenReplacedWithBraceOnLeft(pft) =>
removeToken
case _: T.LeftBrace
if owner.parent.contains(pft.leftOwner) && okLineSpan(t) &&
findTreeWithParentEx(owner) {
case p: Term.Block => Some(p)
case _: Term.ArgClause => None
case _ => Some(null)
}.isEmpty => removeToken
case _ => null
}
case t: Term.Block => t.parent match {
Expand Down Expand Up @@ -361,7 +368,18 @@ class RedundantBraces(implicit val ftoks: FormatTokens)
case _ => true
}) &&
(style.dialect.allowSignificantIndentation ||
okComment(ft) && !elseAfterRightBraceThenpOnLeft) =>
(t.parent match {
case Some(_: Term.Block) => true
case Some(_: Term.ArgClause) =>
val pft = ftoks.prevNonComment(left.ft)
pft.left match {
case _: T.LeftParen =>
isLeftParenReplacedWithBraceOnLeft(pft)
case _: T.LeftBrace => true
case _ => false
}
case _ => false
}) || okComment(ft) && !elseAfterRightBraceThenpOnLeft) =>
(left, removeToken)
case _ => null
}
Expand Down Expand Up @@ -514,16 +532,9 @@ class RedundantBraces(implicit val ftoks: FormatTokens)
if (isSeqMulti(t.values)) okToRemoveBlockWithinApply(b)
else ftoks.getDelimsIfEnclosed(t).exists { case (ldelim, _) =>
def isArgDelimRemoved = session.isRemovedOnLeft(ldelim, ok = true)
def hasExpressionWithBraces: Boolean =
// there's a nested expression with braces that will be kept
getSingleStatIfLineSpanOk(b)
.exists(getBlockNestedPartialFunction(_).isDefined)
ldelim.left match {
case lb: T.LeftBrace =>
// either arg clause has separate braces, or we guarantee braces
(ft.right ne lb) && !isArgDelimRemoved || hasExpressionWithBraces
case _: T.LeftParen => isLeftParenReplacedWithBraceOnLeft(ldelim) ||
hasExpressionWithBraces
case lb: T.LeftBrace => (ft.right ne lb) && !isArgDelimRemoved // arg clause has separate braces
case _: T.LeftParen => isLeftParenReplacedWithBraceOnLeft(ldelim) // this brace replaces paren
case _ => false
}
}
Expand Down Expand Up @@ -673,15 +684,6 @@ class RedundantBraces(implicit val ftoks: FormatTokens)
innerOk(b)(stat) && !b.parent.exists(keepForParent)
}

@tailrec
private def getBlockNestedPartialFunction(
tree: Tree,
): Option[Term.PartialFunction] = tree match {
case x: Term.PartialFunction => Some(x)
case Term.Block(x :: Nil) => getBlockNestedPartialFunction(x)
case _ => None
}

private def braceSeparatesTwoXmlTokens(implicit ft: FT): Boolean = ft.left
.is[T.Xml.End] && ftoks.next(ft).right.is[T.Xml.Start]

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,13 +515,13 @@ object TreeOps {
}

@tailrec
def getTreeSingleExpr(tree: Tree): Option[Tree] = tree match {
def getTreeSingleExpr(tree: Tree): Option[Term] = tree match {
case t: Term.Block => t.stats match {
case stat :: Nil => getTreeSingleExpr(stat)
case _ => None
}
case _: Defn => None
case t => Some(t)
case t: Term => Some(t)
case _ => None
}

def isTreeSingleExpr(tree: Tree): Boolean = getTreeSingleExpr(tree).isDefined
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1135,12 +1135,24 @@ object a {
}
>>>
object a {
val foo = bar { case x => y }
val foo = bar { case x => y }
val foo = bar { case x => y }
val foo = bar.baz { case x => y }
val foo = bar.baz { case x => y }
val foo = bar.baz { case x => y }
val foo = bar { case x =>
y
}
val foo = bar { case x =>
y
}
val foo = bar { case x =>
y
}
val foo = bar.baz { case x =>
y
}
val foo = bar.baz { case x =>
y
}
val foo = bar.baz { case x =>
y
}
}
<<< single-block partial function with block one-arg apply, with comments
object a {
Expand Down Expand Up @@ -1210,14 +1222,14 @@ object a {
// c6
}
val foo = bar { // c1
{ // c2
// c3
{ // c4
case x => y
// c5
} // c6
// c7
} // c8
// c2
// c3
{ // c4
case x => y
// c5
} // c6
// c7
// c8
}
val foo = bar.baz { // c1
{ // c2
Expand All @@ -1234,14 +1246,14 @@ object a {
// c6
}
val foo = bar.baz { // c1
{ // c2
// c3
{ // c4
case x => y
// c5
} // c6
// c7
} // c8
// c2
// c3
{ // c4
case x => y
// c5
} // c6
// c7
// c8
}
}
<<< single-block partial function with block one-arg apply, with comments and parens
Expand Down Expand Up @@ -1309,16 +1321,15 @@ object a {
// c4
} // c5
} // c6
val foo = bar( // c1
{ // c2
// c3
{ // c4
case x => y
// c5
} // c6
// c7
} // c8
)
val foo = bar { // c1
// c2
// c3
{ // c4
case x => y
// c5
} // c6
// c7
} // c8
val foo = bar.baz { // c1
// c2
case x => y
Expand All @@ -1331,16 +1342,15 @@ object a {
// c4
} // c5
} // c6
val foo = bar.baz( // c1
{ // c2
// c3
{ // c4
case x => y
// c5
} // c6
// c7
} // c8
)
val foo = bar.baz { // c1
// c2
// c3
{ // c4
case x => y
// c5
} // c6
// c7
} // c8
}
<<< init-only secondary ctors
class a(vi: Int, vs: String) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ class FormatTests extends FunSuite with CanRunTests with FormatAssertions {
val explored = Debug.explored.get()
logger.debug(s"Total explored: $explored")
if (!onlyUnit && !onlyManual)
assertEquals(explored, 1202529, "total explored")
assertEquals(explored, 1202551, "total explored")
val results = debugResults.result()
// TODO(olafur) don't block printing out test results.
// I don't want to deal with scalaz's Tasks :'(
Expand Down

0 comments on commit 2980545

Please sign in to comment.