Implement #863: vertical-multiline for type params

core/src/main/scala/org/scalafmt/internal/FormatOps.scala

@@ -22,12 +22,16 @@ import scala.meta.tokens.Tokens
 import org.scalafmt.Error.CaseMissingArrow
 import org.scalafmt.config.ScalafmtConfig
 import org.scalafmt.internal.ExpiresOn.Left
+import org.scalafmt.internal.ExpiresOn.Right
 import org.scalafmt.internal.Length.Num
 import org.scalafmt.internal.Policy.NoPolicy
+import org.scalafmt.util.CtorModifier
 import org.scalafmt.util.StyleMap
 import org.scalafmt.util.TokenOps
 import org.scalafmt.util.TreeOps
 import org.scalafmt.util.Whitespace
+import org.scalafmt.util.Modifier
+import org.scalafmt.util.RightParenOrBracket
 import org.scalafmt.util.logger
 
 /**
@@ -165,7 +169,7 @@ class FormatOps(val tree: Tree, val initStyle: ScalafmtConfig) {
   @tailrec
   final def findFirst(start: FormatToken, end: Token)(
       f: FormatToken => Boolean): Option[FormatToken] = {
-    if (start.left.start < end.start) None
+    if (start.left.start > end.start) None
     else if (f(start)) Some(start)
     else {
       val next_ = next(start)
@@ -679,4 +683,123 @@ class FormatOps(val tree: Tree, val initStyle: ScalafmtConfig) {
     }
   }
 
+  /**
+    * Implementation for `verticalMultilineAtDefinitionSite`
+    */
+  def verticalMultiline(owner: Tree, ft: FormatToken)(
+      implicit style: ScalafmtConfig): Seq[Split] = {
+
+    val FormatToken(open, r, _) = ft
+    val close = matching(open)
+    val indentParam = Num(style.continuationIndent.defnSite)
+    val indentSep = Num((indentParam.n - 2).max(0))
+    val isBracket = open.is[LeftBracket]
+
+    @tailrec
+    def loop(token: Token): Option[Token] = {
+      leftTok2tok(matching(token)) match {
+        case FormatToken(RightParenOrBracket(), l @ LeftParen(), _) =>
+          loop(l)
+        // This case only applies to classes
+        case f @ FormatToken(RightBracket(), mod, _) if mod.is[Modifier] =>
+          loop(next(f).right)
+        case FormatToken(r @ RightParenOrBracket(), _, _) => Some(r)
+        case _ => None
+      }
+    }
+
+    // find the last param on the defn so that we can apply our `policy`
+    // till the end.
+    val lastParen = loop(open).get
+
+    val mixedParams = {
+      owner match {
+        case cls: meta.Defn.Class =>
+          cls.tparams.nonEmpty && cls.ctor.paramss.nonEmpty
+        case _ => false
+      }
+    }
+
+    val shouldNotDangle =
+      owner.is[meta.Ctor.Primary] ||
+        owner.is[meta.Defn.Class] ||
+        owner.is[meta.Defn.Trait]
+
+    // Since classes and defs aren't the same (see below), we need to
+    // create two (2) OneArgOneLineSplit when dealing with classes. One
+    // deals with the type params and the other with the value params.
+    val oneLinePerArg = {
+      val base = OneArgOneLineSplit(open)
+      if (mixedParams) {
+        val afterTypes = leftTok2tok(matchingParentheses(hash(open)))
+        // Try to find the first paren. If found, then we are dealing with
+        // a class with type AND value params. Otherwise it is a class with
+        // just type params.
+        findFirst(afterTypes, lastParen)(t => t.left.is[LeftParen])
+          .fold(base)(t => base.merge(OneArgOneLineSplit(t.left)))
+      } else base
+    }
+
+    // DESNOTE(2017-03-28, pjrt) Classes and defs aren't the same.
+    // For defs, type params and value param have the same `owners`. However
+    // this is not the case for classes. Type params have the class itself
+    // as the owner, but value params have the Ctor as the owner, so a
+    // simple check isn't enough. Instead we check against the owner of the
+    // `lastParen` as well, which will be the same as the value param's
+    // owner.
+    val valueParamsOwner = owners(lastParen)
+    @inline def ownerCheck(rp: Token): Boolean = {
+      val rpOwner = owners(rp)
+      rpOwner == owner || rpOwner == valueParamsOwner
+    }
+
+    val paramGroupSplitter: PartialFunction[Decision, Decision] = {
+      // If this is a class, then don't dangle the last paren
+      case Decision(t @ FormatToken(_, rp @ RightParenOrBracket(), _), _)
+          if shouldNotDangle && rp == lastParen =>
+        val split = Split(NoSplit, 0)
+        Decision(t, Seq(split))
+      // Indent seperators `)(` and `](` by `indentSep`
+      case Decision(t @ FormatToken(_, rp @ RightParenOrBracket(), _), _)
+          if ownerCheck(rp) =>
+        val split = Split(Newline, 0).withIndent(indentSep, rp, Left)
+        Decision(t, Seq(split))
+      // Do NOT Newline the first param after the split, unless we have a
+      // mixed-params case with a Ctor modifier.
+      // `] private (`
+      case Decision(t @ FormatToken(open2 @ LeftParen(), _, _), _) =>
+        val close2 = matchingParentheses(hash(open2))
+        val prevT = prev(t).left
+        val mod =
+          if (mixedParams && owners(prevT).is[CtorModifier]) Newline
+          else NoSplit
+        Decision(t,
+                 Seq(
+                   Split(mod, 0)
+                     .withIndent(indentParam, close2, Right)
+                 ))
+    }
+
+    // Our policy is a combination of OneArgLineSplit and a custom splitter
+    // for parameter groups.
+    val policy = oneLinePerArg.merge(paramGroupSplitter, lastParen.end)
+
+    val firstIndent =
+      if (r.is[RightParen]) indentSep // An empty param group
+      else indentParam
+
+    val singleLineExpire =
+      if (isBracket) close // If we can fit the type params, make it so
+      else lastParen // If we can fit all in one block, make it so
+
+    Seq(
+      Split(NoSplit, 0)
+        .withPolicy(SingleLineBlock(singleLineExpire)),
+      Split(Newline, 1) // Otherwise split vertically
+        .withIndent(firstIndent, close, Right)
+        .withPolicy(policy)
+    )
+
+  }
+
 }

core/src/main/scala/org/scalafmt/internal/Policy.scala

@@ -16,6 +16,18 @@ case class Policy(
     noDequeue: Boolean = false,
     isSingleLine: Boolean = false)(implicit val line: sourcecode.Line) {
 
+  def merge(other: PartialFunction[Decision, Decision],
+            newExpire: Int): Policy =
+    Policy(f.orElse(other), newExpire)
+
+  def merge(other: Policy, newExpire: Int): Policy =
+    merge(other.f, newExpire)
+
+  def merge(other: Policy): Policy = {
+    val newExpire = expire.max(other.expire)
+    merge(other, newExpire)
+  }
+
   def andThen(other: Policy): Policy = {
     if (this.f == Policy.emptyPf) other
     else this.andThen(other.f)

core/src/main/scala/org/scalafmt/internal/Router.scala

@@ -26,11 +26,13 @@ import org.scalafmt.internal.Length.StateColumn
 import org.scalafmt.internal.Policy.NoPolicy
 import org.scalafmt.util.`:parent:`
 import org.scalafmt.util.Delim
+import org.scalafmt.util.CtorModifier
 import org.scalafmt.util.InfixApplication
 import org.scalafmt.util.Keyword
 import org.scalafmt.util.Literal
 import org.scalafmt.util.LoggerOps
 import org.scalafmt.util.Modifier
+import org.scalafmt.util.RightParenOrBracket
 import org.scalafmt.util.SelfAnnotation
 import org.scalafmt.util.SomeInterpolate
 import org.scalafmt.util.TokenOps
@@ -420,71 +422,11 @@ class Router(formatOps: FormatOps) {
         }
 
       // Parameter opening for one parameter group. This format works
-      // on a group-by-group basis.
-      case FormatToken(open @ LeftParen(), r, between)
+      // on the WHOLE defnSite (via policies)
+      case ft @ FormatToken((LeftParen() | LeftBracket()), _, _)
           if style.verticalMultilineAtDefinitionSite &&
-            isDefnSite(leftOwner) =>
-        val close = matchingParentheses(hash(open))
-        val indentParam = Num(style.continuationIndent.defnSite)
-        val indentSep = Num((indentParam.n - 2).max(0))
-
-        @tailrec
-        def loop(token: Token): Option[Token] =
-          leftTok2tok(matchingParentheses(hash(token))) match {
-            case FormatToken(RightParen(), l @ LeftParen(), _) => loop(l)
-            case FormatToken(r @ RightParen(), _, _) => Some(r)
-            case _ => None
-          }
-        // find the last param on the def
-        // so that we can apply our `policy` till
-        // the end.
-        val lastParen = loop(open).get
-
-        // If this is a class, we need to do some different things
-        // We only care about the Primary Ctor since other Ctors will
-        // be picked up by the `def` definition
-        val isCtor = leftOwner.is[meta.Ctor.Primary]
-
-        // Our policy is a combination of OneArgLineSplit and a custom splitter
-        // for parameter groups.
-        val oneLinePerArg = OneArgOneLineSplit(open).f
-        val paramGroupSplitter: PartialFunction[Decision, Decision] = {
-          case Decision(t @ FormatToken(_, rp @ RightParen(), _), _)
-              if rp == lastParen && isCtor =>
-            val split = Split(NoSplit, 0)
-            Decision(t, Seq(split))
-          // Indent seperators `)(` by `indentSep`
-          case Decision(t @ FormatToken(_, rp @ RightParen(), _), _)
-              if owners(rp) == leftOwner =>
-            val split = Split(Newline, 0).withIndent(indentSep, rp, Left)
-            Decision(t, Seq(split))
-          // Do NOT Newline the first param after the split `)(`. But let
-          // following ones get newlined by the oneLinePerArg policy.
-          case Decision(t @ FormatToken(open2 @ LeftParen(), _, _), _) =>
-            val close2 = matchingParentheses(hash(open2))
-            Decision(t,
-                     Seq(
-                       Split(NoSplit, 0)
-                         .withIndent(indentParam, close2, Right)
-                     ))
-        }
-
-        val policy =
-          Policy(oneLinePerArg.orElse(paramGroupSplitter), lastParen.end)
-
-        val firstIndent =
-          if (r.is[RightParen]) // An empty param group
-            indentSep
-          else
-            indentParam
-
-        Seq(
-          Split(NoSplit, 0) // If it fits in one block, make it so
-            .withPolicy(SingleLineBlock(lastParen)),
-          Split(Newline, 1) // Otherwise split vertically
-            .withIndent(firstIndent, close, Right)
-            .withPolicy(policy)
-        )
+            isDefnSiteWithParams(leftOwner) =>
+        verticalMultiline(leftOwner, ft)(style)
 
       // Term.Apply and friends
       case FormatToken(LeftParen() | LeftBracket(), right, between)

core/src/main/scala/org/scalafmt/util/TokenClasses.scala

@@ -77,3 +77,17 @@ object Trivia {
     token.is[Whitespace] || token.is[Comment]
   }
 }
+
+@classifier
+trait LeftParenOrBracket
+object LeftParenOrBracket {
+  def unapply(tok: Token): Boolean =
+    tok.is[LeftParen] || tok.is[LeftBracket]
+}
+
+@classifier
+trait RightParenOrBracket
+object RightParenOrBracket {
+  def unapply(tok: Token): Boolean =
+    tok.is[RightParen] || tok.is[RightBracket]
+}

core/src/main/scala/org/scalafmt/util/TreeClasses.scala

@@ -11,3 +11,9 @@ object SomeInterpolate {
     tree.is[Term.Interpolate] || tree.is[Pat.Interpolate]
   }
 }
+@classifier
+trait CtorModifier
+object CtorModifier {
+  def unapply(tree: Tree): Boolean =
+    tree.is[Mod.Private] || tree.is[Mod.Protected]
+}

core/src/main/scala/org/scalafmt/util/TreeOps.scala

@@ -229,6 +229,27 @@ object TreeOps {
     case _ => None
   }
 
+  /**
+    * Returns `true` if the [[Tree]] is a class, trait or def
+    *
+    * For classes this includes primary and secondary Ctors.
+    */
+  def isDefnSiteWithParams(tree: Tree): Boolean = tree match {
+    case _: Decl.Def | _: Defn.Def | _: Defn.Macro | _: Defn.Class |
+        _: Defn.Trait | _: Ctor.Secondary =>
+      true
+    case x: Ctor.Primary if x.parent.exists(_.isInstanceOf[Defn.Class]) =>
+      true
+    case _ => false
+  }
+
+  /**
+    * Returns `true` if the [[Tree]] is a defintion site
+    *
+    * Currently, this includes everything from classes and defs to type
+    * applications
+    * TODO (#867) Type applications shouldn't be here
+    */
   def isDefnSite(tree: Tree): Boolean = tree match {
     case _: Decl.Def | _: Defn.Def | _: Defn.Macro | _: Defn.Class |
         _: Defn.Trait | _: Ctor.Secondary | _: Decl.Type | _: Defn.Type |