From af06ddc84c3a37b0cf284813578497293823a568 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 24 Jul 2020 17:43:48 -0700 Subject: [PATCH 1/2] Add a failing test with removing delims in "if(a)" --- .../resources/rewrite/RedundantBraces-case.stat | 12 ++++++++++++ .../test/resources/rewrite/RedundantBraces.stat | 6 ++++++ .../test/resources/rewrite/RedundantParens.stat | 14 +++++++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-case.stat b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-case.stat index 801ddbc2cc..e64c5e72a4 100644 --- a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-case.stat +++ b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-case.stat @@ -89,3 +89,15 @@ val a = b.c(d => val a = b.c(d => {e}, f =>g match { case h =>}) >>> val a = b.c(d => e, f => g match { case h => }) +<<< #2117 if without a space +object a { + a match { + case a if{a} => + } +} +>>> +test does not parse +object a { + a match { + case a ifa => + ^ diff --git a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat index 146d026150..d8529b7749 100644 --- a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat +++ b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat @@ -851,3 +851,9 @@ object Issue { val _ = f() } } +<<< #2117 maintain delimiter after removing newline in infix +1 op1{ +2 +}op2 3 +>>> +1 op12op2 3 diff --git a/scalafmt-tests/src/test/resources/rewrite/RedundantParens.stat b/scalafmt-tests/src/test/resources/rewrite/RedundantParens.stat index 3bedfc4f2b..430785051c 100644 --- a/scalafmt-tests/src/test/resources/rewrite/RedundantParens.stat +++ b/scalafmt-tests/src/test/resources/rewrite/RedundantParens.stat @@ -282,4 +282,16 @@ object a { val x = for { a <- b } yield a -} \ No newline at end of file +} +<<< #2117 if without a space +object a { + a match { + case a if(((a))) => + } +} +>>> +test does not parse +object a { + a match { + case a ifa => + ^ From fdc6ca30ca42ddaea3b0f20f7f9d8a19fe807f5c Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 24 Jul 2020 20:44:45 -0700 Subject: [PATCH 2/2] Rewrite: replace delimiters with space Parens/braces serve as delimiters (such as in "if(x"), so when we remove them, we must ensure that tokens before and after continue to be clearly separated. For that, let's just replace these delimiters with spaces, as formatter will remove any redundant space later. --- .../scalafmt/rewrite/RedundantBraces.scala | 32 +++++++++---------- .../scalafmt/rewrite/RedundantParens.scala | 14 +++----- .../rewrite/RedundantBraces-case.stat | 6 ++-- .../resources/rewrite/RedundantBraces.stat | 2 +- .../resources/rewrite/RedundantParens.stat | 6 ++-- 5 files changed, 27 insertions(+), 33 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantBraces.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantBraces.scala index 3013cbd19d..b61d72cb1a 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantBraces.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantBraces.scala @@ -78,17 +78,13 @@ class RedundantBraces(implicit ctx: RewriteCtx) extends RewriteSession { arg.value.head == '_' || isLiteralIdentifier(arg) t.parts.tail.zip(t.args).foreach { - case (Lit(value: String), arg @ Term.Name(_)) + case (Lit.String(value), arg: Term.Name) if !isIdentifierAtStart(value) && !shouldTermBeEscaped(arg) => - val openBrace = prevToken(arg.tokens.head) - val closeBrace = nextToken(arg.tokens.head) - (openBrace, closeBrace) match { - case (LeftBrace(), RightBrace()) => - ctx.addPatchSet( - TokenPatch.Remove(openBrace), - TokenPatch.Remove(closeBrace) - ) - case _ => + val open = prevToken(arg.tokens.head) + if (open.is[LeftBrace]) { + val close = nextToken(arg.tokens.head) + if (close.is[RightBrace]) + ctx.addPatchSet(TokenPatch.Remove(open), TokenPatch.Remove(close)) } case _ => } @@ -140,8 +136,7 @@ class RedundantBraces(implicit ctx: RewriteCtx) extends RewriteSession { val lparen = ctx.getMatching(rparen) implicit val builder = Seq.newBuilder[TokenPatch] builder += TokenPatch.Replace(lparen, lbrace.text) - builder += TokenPatch.Remove(lbrace) - builder += TokenPatch.Remove(rparen) + removeBraces(lbrace, rparen) ctx.removeLFToAvoidEmptyLine(rparen) ctx.addPatchSet(builder.result(): _*) } @@ -161,8 +156,7 @@ class RedundantBraces(implicit ctx: RewriteCtx) extends RewriteSession { val lbrace = ctx.getMatching(rbrace) if (lbrace.start <= body.tokens.head.start) { implicit val builder = Seq.newBuilder[TokenPatch] - builder += TokenPatch.Remove(lbrace) - builder += TokenPatch.Remove(rbrace) + removeBraces(lbrace, rbrace) ctx.removeLFToAvoidEmptyLine(rbrace) ctx.addPatchSet(builder.result(): _*) } @@ -207,8 +201,7 @@ class RedundantBraces(implicit ctx: RewriteCtx) extends RewriteSession { true } if (ok) { - builder += TokenPatch.Remove(open) - builder += TokenPatch.Remove(close) + removeBraces(open, close) ctx.addPatchSet(builder.result(): _*) } } @@ -343,4 +336,11 @@ class RedundantBraces(implicit ctx: RewriteCtx) extends RewriteSession { private def getSingleStatIfLineSpanOk(b: Term.Block): Option[Stat] = getBlockSingleStat(b).filter(getTermLineSpan(_) <= settings.maxLines) + private def removeBraces(lbrace: Token, rbrace: Token)(implicit + builder: Rewrite.PatchBuilder + ): Unit = { + builder += TokenPatch.AddLeft(lbrace, " ", keepTok = false) + builder += TokenPatch.AddRight(rbrace, " ", keepTok = false) + } + } diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantParens.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantParens.scala index 63d2ccfb28..83293fbf1f 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantParens.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/RedundantParens.scala @@ -93,21 +93,15 @@ class RedundantParens(implicit ctx: RewriteCtx) extends RewriteSession { val offBeg = minToKeep if (offBeg <= offEnd) { implicit val builder = Seq.newBuilder[TokenPatch] - (offBeg to offEnd).foreach { x => + // replace outer with space, to avoid joining with an adjacent keyword + builder += TokenPatch.AddLeft(toks(beg + offBeg), " ", keepTok = false) + builder += TokenPatch.AddRight(toks(end - offBeg), " ", keepTok = false) + ((offBeg + 1) to offEnd).foreach { x => builder += TokenPatch.Remove(toks(beg + x)) builder += TokenPatch.Remove(toks(end - x)) } ctx.removeLFToAvoidEmptyLine(toks(beg + offBeg), toks(beg + offEnd)) ctx.removeLFToAvoidEmptyLine(toks(end - offEnd), toks(end - offBeg)) - - if (beg > 0) { - toks(beg - 1) match { - case t: Token.KwYield => - builder += TokenPatch.AddRight(t, " ", keepTok = true) - case _ => () - } - } - ctx.addPatchSet(builder.result(): _*) } } diff --git a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-case.stat b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-case.stat index e64c5e72a4..feb169b243 100644 --- a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-case.stat +++ b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces-case.stat @@ -96,8 +96,8 @@ object a { } } >>> -test does not parse object a { a match { - case a ifa => - ^ + case a if a => + } +} diff --git a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat index d8529b7749..933a812542 100644 --- a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat +++ b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat @@ -856,4 +856,4 @@ object Issue { 2 }op2 3 >>> -1 op12op2 3 +1 op1 2 op2 3 diff --git a/scalafmt-tests/src/test/resources/rewrite/RedundantParens.stat b/scalafmt-tests/src/test/resources/rewrite/RedundantParens.stat index 430785051c..9348e5b4a2 100644 --- a/scalafmt-tests/src/test/resources/rewrite/RedundantParens.stat +++ b/scalafmt-tests/src/test/resources/rewrite/RedundantParens.stat @@ -290,8 +290,8 @@ object a { } } >>> -test does not parse object a { a match { - case a ifa => - ^ + case a if a => + } +}