From f86080bee97f40736753bc8614eb46e1eae24010 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 3 Jul 2020 11:57:06 -0700 Subject: [PATCH 1/3] Add failing test for select-chain infix indent Also, a few more correct tests, to ensure functionality is preserved. --- .../src/test/resources/default/Select.stat | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/scalafmt-tests/src/test/resources/default/Select.stat b/scalafmt-tests/src/test/resources/default/Select.stat index c0d67fd06e..bf3cbf89c6 100644 --- a/scalafmt-tests/src/test/resources/default/Select.stat +++ b/scalafmt-tests/src/test/resources/default/Select.stat @@ -743,6 +743,67 @@ object a { } } } +<<< #2061 14.1 +preset = default +optIn.breakChainOnFirstMethodDot = false +includeNoParensInSelectChains = true +optIn.breaksInsideChains = true +danglingParentheses.preset = false +=== +object a { + class a { + def a = { + "zip" in { + intercept[IllegalStateException] { + Await.result(Promise.failed[String](f).future zip Promise.successful("foo").future, timeout) + } should ===(f) + } + } + } +} +>>> +object a { + class a { + def a = { + "zip" in { + intercept[IllegalStateException] { + Await.result( + Promise + .failed[String](f).future zip Promise.successful("foo").future, + timeout) + } should ===(f) + } + } + } +} +<<< #2061 14.2 +preset = default +optIn.breakChainOnFirstMethodDot = false +includeNoParensInSelectChains = true +optIn.breaksInsideChains = false +danglingParentheses.preset = false +=== +object a { + class a { + if (!cell + .routerConfig + .isInstanceOf[Pool] && !cell.routerConfig.isInstanceOf[Group]) + throw ActorInitializationException( + "Cluster router actor can only be used with Pool or Group, not with " + + cell.routerConfig.getClass) + } +} +>>> +object a { + class a { + if (!cell + .routerConfig + .isInstanceOf[Pool] && !cell.routerConfig.isInstanceOf[Group]) + throw ActorInitializationException( + "Cluster router actor can only be used with Pool or Group, not with " + + cell.routerConfig.getClass) + } +} <<< #2061 15 preset = default optIn.breakChainOnFirstMethodDot = true @@ -799,3 +860,20 @@ object a { }).headOption } } +<<< curly select chain followed by infix +preset = default +optIn.breakChainOnFirstMethodDot = true +=== +object a { + val reduces = node + .foldDown[List[dag.Reduce]](true) { + case r: dag.Reduce => List(r) + } distinct +} +>>> +object a { + val reduces = node + .foldDown[List[dag.Reduce]](true) { + case r: dag.Reduce => List(r) + } distinct +} From ada2e305fac8d09bb7d96227f5973c37a55fcf51 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Thu, 2 Jul 2020 08:39:33 -0700 Subject: [PATCH 2/3] Split: allow multiple tags required for activation Remove Active and Ignored explicit tags as no longer needed. --- .../org/scalafmt/internal/FormatOps.scala | 13 ++-- .../scala/org/scalafmt/internal/Router.scala | 2 +- .../scala/org/scalafmt/internal/Split.scala | 67 ++++++++++++++----- .../org/scalafmt/internal/SplitTag.scala | 23 ++----- 4 files changed, 62 insertions(+), 43 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala index d879d8312f..742ca59df8 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala @@ -1009,22 +1009,21 @@ class FormatOps(val tree: Tree, baseStyle: ScalafmtConfig) { val owners = chain.fold[Set[Tree]](Set(_), x => x.toSet) val nlPolicy = ctorWithChain(owners, lastToken) val nlOnelineTag = style.binPack.parentConstructors match { - case BinPack.ParentCtors.Oneline => SplitTag.Active + case BinPack.ParentCtors.Oneline => Right(true) case BinPack.ParentCtors.OnelineIfPrimaryOneline => - SplitTag.OnelineWithChain + Left(SplitTag.OnelineWithChain) case BinPack.ParentCtors.Always | BinPack.ParentCtors.Never => - SplitTag.Ignored + Right(false) case BinPack.ParentCtors.MaybeNever => - val isOneline = style.newlines.sourceIs(Newlines.fold) - if (isOneline) SplitTag.Active else SplitTag.Ignored + Right(style.newlines.sourceIs(Newlines.fold)) } val indent = Indent(Num(indentLen), lastToken, ExpiresOn.After) val extendsThenWith = chain.left.exists(_.inits.length > 1) Seq( Split(Space, 0).withSingleLine(lastToken, noSyntaxNL = extendsThenWith), Split(nlMod, 0) - .onlyFor(nlOnelineTag) - .activateFor(nlOnelineTag) + .onlyIf(nlOnelineTag != Right(false)) + .preActivateFor(nlOnelineTag.left.toOption) .withSingleLine(lastToken, noSyntaxNL = extendsThenWith) .withIndent(indent), Split(nlMod, 1).withPolicy(nlPolicy).withIndent(indent) 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 index 819ff9934c..868fc79c99 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala @@ -573,7 +573,7 @@ class Router(formatOps: FormatOps) { val forceNewlineBeforeExtends = Policy(expire) { case Decision(t @ FormatToken(_, _: T.KwExtends, _), s) if t.meta.rightOwner == leftOwner => - s.filter(x => x.isNL && (x.activeTag ne SplitTag.OnelineWithChain)) + s.filter(x => x.isNL && !x.isActiveFor(SplitTag.OnelineWithChain)) } val policyEnd = defnBeforeTemplate(leftOwner).fold(r)(_.tokens.last) val policy = delayedBreakPolicy(policyEnd)(forceNewlineBeforeExtends) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Split.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Split.scala index 52422e21c1..b0b6ec8cd7 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Split.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Split.scala @@ -38,8 +38,8 @@ case class OptimalToken(token: Token, killOnFail: Boolean = false) { case class Split( modExt: ModExt, cost: Int, - tag: SplitTag = SplitTag.Active, - activeTag: SplitTag = SplitTag.Active, + neededTags: Set[SplitTag] = Set.empty, + activeTags: Set[SplitTag] = Set.empty, policy: Policy = NoPolicy, optimalAt: Option[OptimalToken] = None )(implicit val line: sourcecode.Line) { @@ -63,24 +63,44 @@ case class Split( def length: Int = modExt.mod.length @inline - def isIgnored: Boolean = tag eq SplitTag.Ignored + def isIgnored: Boolean = neededTags eq Split.ignoredTags @inline - def isActive: Boolean = tag eq activeTag + def isActive: Boolean = neededTags == activeTags + + @inline + def isActiveFor(splitTag: SplitTag): Boolean = activeTags(splitTag) + + @inline + def isNeededFor(splitTag: SplitTag): Boolean = neededTags(splitTag) + + private def ignored: Split = + if (isIgnored) this else copy(neededTags = Split.ignoredTags) @inline def notIf(flag: Boolean): Split = onlyIf(!flag) - def onlyIf(flag: Boolean): Split = - if (flag || isIgnored) this else copy(tag = SplitTag.Ignored) + @inline + def onlyIf(flag: Boolean): Split = if (flag) this else ignored - def onlyFor(tag: SplitTag, ignore: Boolean = false): Split = - if (isIgnored || ignore || (this.tag eq tag)) this - else if (isActive) copy(tag = tag) - else throw new UnsupportedOperationException("Multiple tags unsupported") + def onlyFor(splitTag: SplitTag, ignore: Boolean = false): Split = + if (isIgnored || ignore || isNeededFor(splitTag)) this + else copy(neededTags = neededTags + splitTag)(line = line) - def activateFor(tag: SplitTag): Split = - if (isIgnored || (this.activeTag eq tag)) this else copy(activeTag = tag) + def activateFor(splitTag: SplitTag): Split = + if (isIgnored || isActiveFor(splitTag)) this + else copy(activeTags = activeTags + splitTag)(line = line) + + def preActivateFor(splitTag: SplitTag): Split = + if (isIgnored) this + else + copy( + activeTags = activeTags + splitTag, + neededTags = neededTags + splitTag + )(line = line) + + def preActivateFor(splitTag: Option[SplitTag]): Split = + if (isIgnored) this else splitTag.fold(this)(preActivateFor) def withOptimalTokenOpt( token: => Option[Token], @@ -206,11 +226,15 @@ case class Split( } override def toString = { - val prefix = tag match { - case SplitTag.Ignored => "!" - case SplitTag.Active => "" - case _ => s"[$tag]" - } + val prefix = + if (isIgnored) "!" + else { + val wantedTags = neededTags.filterNot(activeTags).mkString(",") + val unusedTags = activeTags.filterNot(neededTags).mkString(",") + if (unusedTags.nonEmpty) s"[$wantedTags!$unusedTags]" + else if (wantedTags.nonEmpty) s"[$wantedTags]" + else "" + } val opt = optimalAt.fold("")(", opt=" + _) s"""$prefix${modExt.mod}:${line.value}(cost=$cost, indents=$indentation, $policy$opt)""" } @@ -218,7 +242,14 @@ case class Split( object Split { + private val ignoredTags = Set[SplitTag](null) + def ignored(implicit line: sourcecode.Line) = - Split(ModExt(NoSplit), 0, tag = SplitTag.Ignored) + Split(ModExt(NoSplit), 0).ignored + + def apply(ignore: Boolean, cost: Int)( + modExt: ModExt + )(implicit line: sourcecode.Line): Split = + if (ignore) ignored else Split(modExt, cost) } diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala index ac06105845..2f08f8e846 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala @@ -2,27 +2,16 @@ package org.scalafmt.internal sealed abstract class SplitTag { - def activateOnly(splits: Seq[Split]): Seq[Split] + final def activateOnly(splits: Seq[Split]): Seq[Split] = + splits.map(_.activateFor(this)) } object SplitTag { - abstract class Base extends SplitTag { - override final def activateOnly(splits: Seq[Split]): Seq[Split] = splits - } - - abstract class Custom extends SplitTag { - override final def activateOnly(splits: Seq[Split]): Seq[Split] = - splits.map(_.activateFor(this)).filter(_.isActive) - } - - case object Active extends Base - case object Ignored extends Base - - case object OneArgPerLine extends Custom - case object SelectChainFirstNL extends Custom - case object InfixChainNoNL extends Custom - case object OnelineWithChain extends Custom + case object OneArgPerLine extends SplitTag + case object SelectChainFirstNL extends SplitTag + case object InfixChainNoNL extends SplitTag + case object OnelineWithChain extends SplitTag } From aa5fb8079da65049587a3c6874b23dc174f778eb Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 3 Jul 2020 11:59:43 -0700 Subject: [PATCH 3/3] Router: merge classic select chain rule with other --- .../org/scalafmt/internal/FormatOps.scala | 71 ++----- .../scala/org/scalafmt/internal/Router.scala | 194 ++++++++++-------- .../org/scalafmt/internal/SplitTag.scala | 1 + .../scala/org/scalafmt/util/TokenOps.scala | 12 -- .../scala/org/scalafmt/util/TreeOps.scala | 17 -- .../src/test/resources/default/Select.stat | 6 +- .../resources/newlines/source_classic.stat | 18 +- .../src/test/resources/test/JavaDoc.stat | 2 +- 8 files changed, 139 insertions(+), 182 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala index 742ca59df8..2788604cc6 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala @@ -138,25 +138,6 @@ class FormatOps(val tree: Tree, baseStyle: ScalafmtConfig) { ownersMap.get(hash(tok)).map(tree => tok -> tree) } - object `:chain:` { - def unapply( - tok: Token - )(implicit style: ScalafmtConfig): Option[(Token, Vector[Term.Select])] = { - val ft = tokens(tok) - ft.meta.leftOwner match { - case t: Term.Select => - val (expireTree, nextSelect) = - findLastApplyAndNextSelect(t, style.optIn.encloseClassicChains) - if (!canStartSelectChain(t, nextSelect, expireTree)) None - else { - val chain = getSelectChain(t, expireTree, Vector(t)) - Some(tok -> chain) - } - case _ => None - } - } - } - @inline def prev(tok: FormatToken): FormatToken = tokens(tok, -1) @inline def next(tok: FormatToken): FormatToken = tokens(tok, 1) @@ -465,48 +446,12 @@ class FormatOps(val tree: Tree, baseStyle: ScalafmtConfig) { } } - /** - * Returns last token of select, handles case when select's parent is apply. - * - * For example, in: - * foo.bar[T](1, 2) - * the last token is the final ) - * - * @param dot the dot owned by the select. - */ - def getSelectsLastToken(dot: T.Dot): FormatToken = { - var curr = tokens(dot, 1) - while ( - isOpenApply( - curr.right, - includeCurly = true, - includeNoParens = true - ) && - !statementStarts.contains(hash(curr.right)) - ) { - if (curr.right.is[T.Dot]) { - curr = tokens(curr, 2) - } else { - curr = tokens(matching(curr.right)) - } - } - curr - } - def getOptimalTokenFor(token: Token): Token = getOptimalTokenFor(tokens(token)) def getOptimalTokenFor(ft: FormatToken): Token = if (isAttachedSingleLineComment(ft)) ft.right else ft.left - def getSelectOptimalToken(tree: Tree): Token = { - val lastDotOpt = findLast(tree.tokens)(_.is[T.Dot]) - if (lastDotOpt.isEmpty) - throw new IllegalStateException(s"Missing . in select $tree") - val lastDot = lastDotOpt.get.asInstanceOf[T.Dot] - lastToken(getSelectsLastToken(lastDot).meta.leftOwner) - } - def infixIndent( app: InfixApp, formatToken: FormatToken, @@ -1479,6 +1424,22 @@ class FormatOps(val tree: Tree, baseStyle: ScalafmtConfig) { }) } + /** Checks if an earlier select started the chain */ + @tailrec + final def inSelectChain( + prevSelect: Option[Term.Select], + thisSelect: Term.Select, + lastApply: Tree + )(implicit style: ScalafmtConfig): Boolean = + prevSelect match { + case None => false + case Some(p) if canStartSelectChain(p, Some(thisSelect), lastApply) => + true + case Some(p) => + val prevPrevSelect = findPrevSelect(p, style.encloseSelectChains) + inSelectChain(prevPrevSelect, p, lastApply) + } + @tailrec final def findTokenWith[A]( ft: FormatToken, 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 index 868fc79c99..fb62cb379a 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala @@ -1194,15 +1194,117 @@ class Router(formatOps: FormatOps) { }.isDefined => Seq(Split(NoSplit, 0)) - case t @ FormatToken(left, _: T.Dot, _) - if !style.newlines.sourceIs(Newlines.classic) && - rightOwner.is[Term.Select] => + case t @ FormatToken(left, _: T.Dot, _) if rightOwner.is[Term.Select] => + val enclosed = style.encloseSelectChains val (expireTree, nextSelect) = - findLastApplyAndNextSelect(rightOwner, true) - val prevSelect = findPrevSelect(rightOwner.asInstanceOf[Term.Select]) + findLastApplyAndNextSelect(rightOwner, enclosed) + val thisSelect = rightOwner.asInstanceOf[Term.Select] + val prevSelect = findPrevSelect(thisSelect, enclosed) val expire = lastToken(expireTree) + def breakOnNextDot: Policy = + nextSelect.fold[Policy](Policy.NoPolicy) { tree => + val end = tree.name.tokens.head + Policy(end) { + case Decision(t @ FormatToken(_, _: T.Dot, _), s) + if t.meta.rightOwner eq tree => + val filtered = s.flatMap { x => + val y = x.activateFor(SplitTag.SelectChainSecondNL) + if (y.isActive) Some(y) else None + } + if (filtered.isEmpty) Seq.empty + else { + val minCost = math.max(0, filtered.map(_.cost).min - 1) + filtered.map { x => + val p = + x.policy.filter(!_.isInstanceOf[PenalizeAllNewlines]) + x.copy(cost = x.cost - minCost, policy = p) + } + } + } + } val baseSplits = style.newlines.source match { + case Newlines.classic => + def getNlMod = { + val endSelect = nextSelect.fold(expire)(x => lastToken(x.qual)) + val nlAlt = ModExt(NoSplit).withIndent(-2, endSelect, After) + NewlineT(alt = Some(nlAlt)) + } + + val prevChain = inSelectChain(prevSelect, thisSelect, expireTree) + if (canStartSelectChain(thisSelect, nextSelect, expireTree)) { + val chainExpire = + if (nextSelect.isEmpty) lastToken(thisSelect) + else if (!isEnclosedInMatching(expireTree)) expire + else lastToken(expireTree.tokens.dropRight(1)) + val nestedPenalty = + nestedSelect(rightOwner) + nestedApplies(leftOwner) + // This policy will apply to both the space and newline splits, otherwise + // the newline is too cheap even it doesn't actually prevent other newlines. + val penalizeBreaks = penalizeAllNewlines(chainExpire, 2) + def slbPolicy = + SingleLineBlock( + chainExpire, + getExcludeIf(chainExpire), + penaliseNewlinesInsideTokens = true + ) + val newlinePolicy = breakOnNextDot & penalizeBreaks + val ignoreNoSplit = t.hasBreak && + (left.is[T.Comment] || style.optIn.breakChainOnFirstMethodDot) + val chainLengthPenalty = + if ( + style.newlines.penalizeSingleSelectMultiArgList && + nextSelect.isEmpty + ) { + // penalize by the number of arguments in the rhs open apply. + // I know, it's a bit arbitrary, but my manual experiments seem + // to show that it produces OK output. The key insight is that + // many arguments on the same line can be hard to read. By not + // putting a newline before the dot, we force the argument list + // to break into multiple lines. + splitCallIntoParts.lift(tokens(t, 2).meta.rightOwner) match { + case Some((_, Left(args))) => + Math.max(0, args.length - 1) + case Some((_, Right(argss))) => + Math.max(0, argss.map(_.length).sum - 1) + case _ => 0 + } + } else 0 + // when the flag is on, penalize break, to avoid idempotence issues; + // otherwise, after the break is chosen, the flag prohibits nosplit + val nlBaseCost = + if (style.optIn.breakChainOnFirstMethodDot && t.noBreak) 3 + else 2 + val nlCost = nlBaseCost + nestedPenalty + chainLengthPenalty + val nlMod = getNlMod + Seq( + Split(!prevChain, 1) { // must come first, for backwards compat + if (style.optIn.breaksInsideChains) NoSplit.orNL(t.noBreak) + else nlMod + } + .withPolicy(newlinePolicy) + .onlyFor(SplitTag.SelectChainSecondNL), + Split(ignoreNoSplit, 0)(NoSplit) + .withPolicy(slbPolicy, prevChain) + .andPolicy(penalizeBreaks), + Split(if (ignoreNoSplit) Newline else nlMod, nlCost) + .withPolicy(newlinePolicy) + ) + } else { + val isComment = left.is[T.Comment] + val doBreak = isComment && t.hasBreak + Seq( + Split(!prevChain, 1) { + if (style.optIn.breaksInsideChains) NoSplit.orNL(t.noBreak) + else if (doBreak) Newline + else getNlMod + } + .withPolicy(breakOnNextDot) + .onlyFor(SplitTag.SelectChainSecondNL), + Split(if (doBreak) Newline else Space(isComment), 0) + ) + } + case _ if left.is[T.Comment] => Seq(Split(Space.orNL(t.noBreak), 0)) @@ -1236,7 +1338,7 @@ class Router(formatOps: FormatOps) { ) } - val delayedBreakPolicy = nextSelect.map { tree => + val delayedBreakPolicyOpt = nextSelect.map { tree => Policy(tree.name.tokens.head) { case Decision(t @ FormatToken(_, _: T.Dot, _), s) if t.meta.rightOwner eq tree => @@ -1249,90 +1351,12 @@ class Router(formatOps: FormatOps) { val willBreak = nextNonCommentSameLine(tokens(t, 2)).right.is[T.Comment] val splits = baseSplits.map { s => if (willBreak || s.isNL) s.withIndent(indent) - else s.andPolicyOpt(delayedBreakPolicy) + else s.andFirstPolicyOpt(delayedBreakPolicyOpt) } if (prevSelect.isEmpty) splits else baseSplits ++ splits.map(_.onlyFor(SplitTag.SelectChainFirstNL)) - case FormatToken(T.Ident(name), _: T.Dot, _) if isSymbolicName(name) => - Seq(Split(NoSplit, 0)) - - case FormatToken(_: T.Underscore, _: T.Dot, _) => - Seq(Split(NoSplit, 0)) - - case tok @ FormatToken(left, dot @ T.Dot() `:chain:` chain, _) => - val nestedPenalty = nestedSelect(rightOwner) + nestedApplies(leftOwner) - val optimalToken = getSelectOptimalToken(chain.last) - val expire = - if (chain.length == 1) lastToken(chain.last) - else optimalToken - - def getNewline(ft: FormatToken): NewlineT = { - val (_, nextSelect) = findLastApplyAndNextSelect( - ft.meta.rightOwner, - style.optIn.encloseClassicChains - ) - val endSelect = nextSelect.fold(optimalToken)(x => lastToken(x.qual)) - val nlAlt = ModExt(NoSplit).withIndent(-2, endSelect, After) - NewlineT(alt = Some(nlAlt)) - } - - val breakOnEveryDot = Policy(expire) { - case Decision(t @ FormatToken(_, _: T.Dot, _), _) - if chain.contains(t.meta.rightOwner) => - val mod = - if (style.optIn.breaksInsideChains) - NoSplit.orNL(t.noBreak) - else getNewline(t) - Seq(Split(mod, 1)) - } - val exclude = getExcludeIf(expire) - // This policy will apply to both the space and newline splits, otherwise - // the newline is too cheap even it doesn't actually prevent other newlines. - val penalizeNewlinesInApply = penalizeAllNewlines(expire, 2) - val noSplitPolicy = - SingleLineBlock( - expire, - exclude, - penaliseNewlinesInsideTokens = true - ) & penalizeNewlinesInApply - val newlinePolicy = breakOnEveryDot & penalizeNewlinesInApply - val ignoreNoSplit = - style.optIn.breakChainOnFirstMethodDot && tok.hasBreak - val chainLengthPenalty = - if ( - style.newlines.penalizeSingleSelectMultiArgList && - chain.length < 2 - ) { - // penalize by the number of arguments in the rhs open apply. - // I know, it's a bit arbitrary, but my manual experiments seem - // to show that it produces OK output. The key insight is that - // many arguments on the same line can be hard to read. By not - // putting a newline before the dot, we force the argument list - // to break into multiple lines. - splitCallIntoParts.lift(tokens(tok, 2).meta.rightOwner) match { - case Some((_, Left(args))) => - Math.max(0, args.length - 1) - case Some((_, Right(argss))) => - Math.max(0, argss.map(_.length).sum - 1) - case _ => 0 - } - } else 0 - // when the flag is on, penalize break, to avoid idempotence issues; - // otherwise, after the break is chosen, the flag prohibits nosplit - val nlBaseCost = - if (style.optIn.breakChainOnFirstMethodDot && tok.noBreak) 3 else 2 - val nlCost = nlBaseCost + nestedPenalty + chainLengthPenalty - Seq( - Split(NoSplit, 0) - .notIf(ignoreNoSplit) - .withPolicy(noSplitPolicy), - Split(if (ignoreNoSplit) Newline else getNewline(tok), nlCost) - .withPolicy(newlinePolicy) - .withIndent(2, optimalToken, After) - ) - // ApplyUnary case tok @ FormatToken(T.Ident(_), Literal(), _) if leftOwner == rightOwner => diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala index 2f08f8e846..9f1668ef48 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala @@ -11,6 +11,7 @@ object SplitTag { case object OneArgPerLine extends SplitTag case object SelectChainFirstNL extends SplitTag + case object SelectChainSecondNL extends SplitTag case object InfixChainNoNL extends SplitTag case object OnelineWithChain extends SplitTag diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TokenOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TokenOps.scala index 502e819196..4b3939ae0e 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TokenOps.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TokenOps.scala @@ -90,18 +90,6 @@ object TokenOps { Space(!Character.isLetterOrDigit(lastCharacter) && lastCharacter != '`') } - def isOpenApply( - token: Token, - includeCurly: Boolean = false, - includeNoParens: Boolean = false - ): Boolean = - token match { - case LeftParen() | LeftBracket() => true - case LeftBrace() if includeCurly => true - case Dot() if includeNoParens => true - case _ => false - } - @inline def isSingleLineComment(c: String): Boolean = c.startsWith("//") 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 index 2124c89f5b..ba0aa1f73b 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala @@ -426,23 +426,6 @@ object TreeOps { splitDefnIntoParts.lift(tree) } - @tailrec - def getSelectChain( - child: Tree, - lastApply: Tree, - accum: Vector[Term.Select] - ): Vector[Term.Select] = { - if (child eq lastApply) accum - else - child.parent match { - case Some(parent: Term.Select) => - getSelectChain(parent, lastApply, accum :+ parent) - case Some(parent @ SplitCallIntoParts(`child`, _)) => - getSelectChain(parent, lastApply, accum) - case els => accum - } - } - /** * How many parents of tree are Term.Apply? */ diff --git a/scalafmt-tests/src/test/resources/default/Select.stat b/scalafmt-tests/src/test/resources/default/Select.stat index bf3cbf89c6..b81a6440a8 100644 --- a/scalafmt-tests/src/test/resources/default/Select.stat +++ b/scalafmt-tests/src/test/resources/default/Select.stat @@ -62,7 +62,7 @@ val lastToken = owner.body.tokens.filter { case _: Whitespace | _: Comment => false case _ => true } // edge case, if body is empty expire on arrow. -.lastOption.getOrElse(arrow) + .lastOption.getOrElse(arrow) <<< apply infix has indent val expireAAAAAAAAAAAAAAAAAAAAA = owner.tokens. find(t => t.isInstanceOf[`=`] && owners(t) == owner) @@ -310,7 +310,7 @@ object a { (TypedPipe.from[(Int, Int)](Tsv("in0", (0, 1)), (0, 1)) joinBy TypedPipe.from[(Int, Int)](Tsv("in1", (0, 1)), (0, 1)))(_._1, _._1) //Flatten out to three values: - .toTypedPipe + .toTypedPipe .map { kvw => (kvw._1, kvw._2._1._2, kvw._2._2._2) } .write(TypedText.tsv[(Int, Int, Int)]("out2")) } @@ -875,5 +875,5 @@ object a { val reduces = node .foldDown[List[dag.Reduce]](true) { case r: dag.Reduce => List(r) - } distinct + } distinct } diff --git a/scalafmt-tests/src/test/resources/newlines/source_classic.stat b/scalafmt-tests/src/test/resources/newlines/source_classic.stat index 616069a28c..c0f0674813 100644 --- a/scalafmt-tests/src/test/resources/newlines/source_classic.stat +++ b/scalafmt-tests/src/test/resources/newlines/source_classic.stat @@ -2305,8 +2305,8 @@ class Foo { >>> class Foo { val vv = v.aaa // - // - .bbb + // + .bbb .ccc() val vv = v.aaa // val vv = v.aaa @@ -2323,10 +2323,10 @@ class Foo { >>> class Foo { val vv = v.aaa // - .bbb // - // - .ccc // - .ddd + .bbb // + // + .ccc // + .ddd .eee() } <<< #1334 3: continue chain indent after a comment with apply @@ -2372,7 +2372,7 @@ class Foo { .tail >>> val a: Vector[Array[Double]] = b.c -// similarUserFeatures may not contain the requested user + // similarUserFeatures may not contain the requested user .map { x => similarUserFeatures.get(x) } @@ -2392,8 +2392,8 @@ val a: Vector[Array[Double]] = b.c } >>> val a: Vector[Array[Double]] = b.c -// Only handle first case, others will be fixed on the next pass. -.headOption.a match { + // Only handle first case, others will be fixed on the next pass. + .headOption.a match { case None => case _ => } diff --git a/scalafmt-tests/src/test/resources/test/JavaDoc.stat b/scalafmt-tests/src/test/resources/test/JavaDoc.stat index c640805cae..e56cc8d659 100644 --- a/scalafmt-tests/src/test/resources/test/JavaDoc.stat +++ b/scalafmt-tests/src/test/resources/test/JavaDoc.stat @@ -1288,7 +1288,7 @@ object a { >>> object a { input.read - /** foo */ + /** foo */ .foo { bar } /** baz */ .baz(qux)