Router: fix closing paren if spaces.inParentheses

Currently, for space.inParentheses, one case is excluded for the opening paren (if/while/etc.), but a completely different one (isDefnOrCallSite) applies to the closing paren. Let's fix that.
scalameta · Oct 16, 2020 · b768f88 · b768f88
1 parent 354df7c
commit b768f88
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 13 deletions.
diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala
@@ -1918,12 +1918,16 @@ class Router(formatOps: FormatOps) {
         Seq(
           Split(NoSplit, 0)
         )
-      case FormatToken(_, T.RightParen(), _) =>
-        val mod =
-          Space(style.spaces.inParentheses && isDefnOrCallSite(rightOwner))
-        Seq(
-          Split(mod, 0)
-        )
+      case FormatToken(_, close: T.RightParen, _) =>
+        def modNoNL = {
+          def allowSpace = rightOwner match {
+            case _: Term.If | _: Term.While | _: Term.For | _: Term.ForYield =>
+              isLastToken(close, rightOwner)
+            case _ => true
+          }
+          Space(style.spaces.inParentheses && allowSpace)
+        }
+        Seq(Split(modNoNL, 0))
 
       case FormatToken(left, _: T.KwCatch | _: T.KwFinally, _)
           if style.newlines.alwaysBeforeElseAfterCurlyIf

diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala
@@ -345,10 +345,17 @@ object TreeOps {
   def isSuperfluousParenthesis(open: LeftParen, owner: Tree): Boolean =
     !isTuple(owner) && owner.tokens.headOption.contains(open)
 
-  def isFirstOrLastToken(token: Token, owner: Tree): Boolean = {
-    owner.tokens.headOption.contains(token) ||
+  @inline
+  def isFirstOrLastToken(token: Token, owner: Tree): Boolean =
+    isFirstToken(token, owner) || isLastToken(token, owner)
+
+  @inline
+  def isFirstToken(token: Token, owner: Tree): Boolean =
+    owner.tokens.headOption.contains(token)
+
+  @inline
+  def isLastToken(token: Token, owner: Tree): Boolean =
     owner.tokens.lastOption.contains(token)
-  }
 
   def isCallSite(tree: Tree)(implicit style: ScalafmtConfig): Boolean =
     tree match {

diff --git a/scalafmt-tests/src/test/resources/newlines/source_keep.stat b/scalafmt-tests/src/test/resources/newlines/source_keep.stat
@@ -3156,11 +3156,11 @@ a match {
 >>>
 a match {
   case ( Root / "foo" | Root / "bar" |
-      Root / "baz" / _ / "qux") =>
-    ( if (foo) ( foo) + bar
-      else foo + ( bar)) + ( baz)
+      Root / "baz" / _ / "qux" ) =>
+    ( if (foo) ( foo ) + bar
+      else foo + ( bar ) ) + ( baz )
   case ( Root / "foo" | Root / "bar" |
-      Root / "baz" / _ / "qux") => (
+      Root / "baz" / _ / "qux" ) => (
     foo
   )
 }