RedundantBraces: handle infix expressions

scalameta · Apr 11, 2020 · a479e83 · a479e83
1 parent 2ca0a48
commit a479e83
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 44 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
@@ -10,6 +10,7 @@ import scala.meta._
 import scala.meta.tokens.Token.LeftBrace
 import scala.meta.tokens.Token.RightBrace
 
+import org.scalafmt.util.InfixApp
 import org.scalafmt.util.TreeOps._
 
 object RedundantBraces extends Rewrite {
@@ -169,19 +170,36 @@ class RedundantBraces(implicit ctx: RewriteCtx) extends RewriteSession {
       val close = b.tokens.last
       if (close.is[RightBrace] && check(b)) {
         implicit val builder = Seq.newBuilder[TokenPatch]
-        val ok =
-          if (b.parent.exists(_.is[Term.ApplyInfix])) {
+        val ok = b.parent match {
+          case Some(InfixApp(_)) =>
             /* for infix, we will preserve the block unless the closing brace
              * follows a non-whitespace character on the same line as we don't
              * break lines around infix expressions.
              * we shouldn't join with the previous line (which might also end
              * in a comment), and if we keep the break before the right brace
              * we are removing, that will likely invalidate the expression. */
-            !ctx.onlyWhitespaceBefore(close)
-          } else {
+            val canRemove: ((Token, Option[Token.LF])) => Boolean =
+              if (!ctx.style.newlines.formatInfix) {
+                case (_, lfOpt) => lfOpt.isEmpty
+              }
+              else {
+                case (nonWs, lfOpt) =>
+                  if (nonWs.is[Token.Comment]) lfOpt.isEmpty
+                  else {
+                    lfOpt.foreach(builder += TokenPatch.Remove(_))
+                    true
+                  }
+              }
+            Seq(
+              // ensure can remove opening brace
+              ctx.tokenTraverser.findAfter(open) _,
+              // ensure can remove closing brace
+              ctx.tokenTraverser.findBefore(close) _
+            ).forall(ctx.findNonWhitespaceWith(_).exists(canRemove))
+          case _ =>
             ctx.removeLFToAvoidEmptyLine(close)
             true
-          }
+        }
         if (ok) {
           builder += TokenPatch.Remove(open)
           builder += TokenPatch.Remove(close)

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/Rewrite.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/rewrite/Rewrite.scala
@@ -52,6 +52,20 @@ case class RewriteCtx(
   def onlyWhitespaceBefore(token: Token): Boolean =
     tokenTraverser.findBefore(token)(RewriteCtx.isLFSkipWhitespace).isDefined
 
+  def findNonWhitespaceWith(
+      f: (Token => Option[Boolean]) => Option[Token]
+  ): Option[(Token, Option[LF])] = {
+    var lf: Option[LF] = None
+    val nonWs = f {
+      case t: LF =>
+        if (lf.nonEmpty) Some(false)
+        else { lf = Some(t); None }
+      case Whitespace() => None
+      case _ => Some(true)
+    }
+    nonWs.map((_, lf))
+  }
+
   def getPatchToAvoidEmptyLine(token: Token): Option[TokenPatch] =
     if (!onlyWhitespaceBefore(token)) None
     else

diff --git a/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat b/scalafmt-tests/src/test/resources/rewrite/RedundantBraces.stat
@@ -1,6 +1,7 @@
 rewrite.rules = [RedundantBraces]
 rewrite.redundantBraces.maxLines = 1
 rewrite.redundantBraces.generalExpressions = true
+newlines.afterInfix = some
 <<< basic
 object a {
   def x(i: Int): Int = {
@@ -321,6 +322,8 @@ object a {
   b.c(d => e => { f; g })
 }
 <<< #1027 2.1: parens to braces no
+newlines.afterInfix = keep
+===
 object a {
   b.c (d =>
     e
@@ -427,12 +430,12 @@ class Test extends AnyWordSpec {
 >>>
 class Test extends AnyWordSpec {
   "SUT" when {
-    "statement about test" should {
-      "some test" in {
-        val s = 1
-      }
+      "statement about test" should {
+          "some test" in {
+              val s = 1
+            }
+        }
     }
-  }
 }
 <<< #1633 1.1: multiline infix
     "properly extract the value of www-urlencoded form fields" in {
@@ -446,14 +449,14 @@ class Test extends AnyWordSpec {
     }
 >>>
 "properly extract the value of www-urlencoded form fields" in {
-  println
-  Post("/", urlEncodedForm) ~> {
-    formFields('firstName, "age".as[Int], 'sex.?, "VIP" ? false) {
-      (firstName, age, sex, vip) ⇒
-        complete(firstName + age + sex + vip)
-    }
-  } ~> check(responseAs[String] shouldEqual "Mike42Nonefalse")
-}
+    println
+    Post("/", urlEncodedForm) ~> {
+        formFields('firstName, "age".as[Int], 'sex.?, "VIP" ? false) {
+          (firstName, age, sex, vip) ⇒
+            complete(firstName + age + sex + vip)
+        }
+      } ~> check(responseAs[String] shouldEqual "Mike42Nonefalse")
+  }
 <<< #1633 1.2: func with placeholder and select
 object a {
   def findAccountByAPIKey(apiKey: APIKey) = byKeyCache.get(apiKey) match {
@@ -469,8 +472,8 @@ object a {
     byKeyCache.get(apiKey) match {
       case None =>
         delegate.findAccountByAPIKey(apiKey) map {
-          _ map _ tap (add(apiKey, _)) unsafePerformIO
-        }
+            _ map _ tap (add(apiKey, _)) unsafePerformIO
+          }
     }
 }
 <<< #1633 1.3: func with placeholder and infix
@@ -488,8 +491,8 @@ object a {
     byKeyCache.get(apiKey) match {
       case None =>
         delegate.findAccountByAPIKey(apiKey) map {
-          _ map (_) tap (add(apiKey, _)) unsafePerformIO (foo)
-        }
+            _ map (_) tap (add(apiKey, _)) unsafePerformIO (foo)
+          }
     }
 }
 <<< #1633 1.4: func
@@ -507,8 +510,8 @@ object a {
     byKeyCache.get(apiKey) match {
       case None =>
         delegate.findAccountByAPIKey(apiKey) map { x =>
-          x._1 map x._2 tap (add(apiKey, _)) unsafePerformIO
-        }
+            x._1 map x._2 tap (add(apiKey, _)) unsafePerformIO
+          }
     }
 }
 <<< #1633 1.5: partial func
@@ -526,9 +529,9 @@ object a {
     byKeyCache.get(apiKey) match {
       case None =>
         delegate.findAccountByAPIKey(apiKey) map {
-          case (x, y) =>
-            x map y tap (add(apiKey, _)) unsafePerformIO
-        }
+            case (x, y) =>
+              x map y tap (add(apiKey, _)) unsafePerformIO
+          }
     }
 }
 <<< #1633 2.1: function block in an if-statement
@@ -607,9 +610,7 @@ object a {
 }
 >>>
 object a {
-  1 + {
-    2
-  } + 3
+  1 + 2 + 3
 }
 <<< #1633 4.2: infix with literal and no break before
 object a {
@@ -618,8 +619,7 @@ object a {
 }
 >>>
 object a {
-  1 +
-    2 + 3
+  1 + 2 + 3
 }
 <<< #1633 4.3: infix with literal and comment
 object a {
@@ -630,8 +630,8 @@ object a {
 >>>
 object a {
   1 + {
-    2 // comment
-  } + 3
+      2 // comment
+    } + 3
 }
 <<< #1633 4.4: infix with literal and two comments
 object a {
@@ -643,8 +643,8 @@ object a {
 >>>
 object a {
   1 + {
-    2 // comment
-  } + // comment
+      2 // comment
+    } + // comment
     3
 }
 <<< #1633 4.5: infix with apply
@@ -656,8 +656,8 @@ object a {
 >>>
 object a {
   1 + {
-    a(b)
-  } + 3
+      a(b)
+    } + 3
 }
 <<< #1633 4.6: infix with apply and no break before
 object a {
@@ -667,8 +667,8 @@ object a {
 >>>
 object a {
   1 + {
-    a(b)
-  } + 3
+      a(b)
+    } + 3
 }
 <<< #1633 4.7: infix with apply and comment
 object a {
@@ -679,8 +679,8 @@ object a {
 >>>
 object a {
   1 + {
-    a(b) // comment
-  } + 3
+      a(b) // comment
+    } + 3
 }
 <<< #1633 4.8: infix with apply and two comments
 object a {
@@ -692,8 +692,8 @@ object a {
 >>>
 object a {
   1 + {
-    a(b) // comment
-  } + // comment
+      a(b) // comment
+    } + // comment
     3
 }
 <<< #1633 5.1: verify correct newline removal