Skip to content

Commit

Permalink
Router: ignore line breaks for config style apply
Browse files Browse the repository at this point in the history
For the purposes of this change, config style would mean a newline after
the opening parenthesis and one argument per line. Whether the closing
parenthesis is dangling can be controlled via the danglingParentheses
parameter.

NB: there are other places where config style is checked, they will be
addressed separately.
  • Loading branch information
kitbellew committed Mar 17, 2020
1 parent e8dc539 commit 09bf81f
Show file tree
Hide file tree
Showing 8 changed files with 226 additions and 180 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,7 @@ case class Newlines(
avoidAfterYield: Boolean = true
) {
val reader: ConfDecoder[Newlines] = generic.deriveDecoder(this).noTypos
if (source != Newlines.classic) {
Newlines.warnSourceIsExperimental
require(
source == Newlines.keep || afterCurlyLambda != NewlineCurlyLambda.preserve,
s"newlines: can't use source=${source} and afterCurlyLambda=$afterCurlyLambda"
)
}
if (source != Newlines.classic) Newlines.warnSourceIsExperimental

@inline
def sourceIs(hint: Newlines.SourceHints): Boolean =
Expand All @@ -134,6 +128,9 @@ case class Newlines(
def sourceIn(hints: Newlines.SourceHints*): Boolean =
hints.contains(source)

val sourceIgnored: Boolean =
sourceIn(Newlines.fold, Newlines.unfold)

}

object Newlines {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.scalafmt.config

import scala.collection.mutable
import scala.io.Codec
import scala.meta.Dialect
import scala.util.matching.Regex
Expand Down Expand Up @@ -155,6 +156,22 @@ case class ScalafmtConfig(
project: ProjectFiles = ProjectFiles(),
edition: Edition = Edition.Latest
) {
if (newlines.source != Newlines.classic) {
val errors = new mutable.ArrayBuffer[String]
if (newlines.source != Newlines.keep) {
if (newlines.afterCurlyLambda == NewlineCurlyLambda.preserve)
errors += s"newlines.afterCurlyLambda=${newlines.afterCurlyLambda}"
if (optIn.configStyleArguments && align.openParenCallSite)
errors += s"optIn.configStyleArguments with align.openParenCallSite"
if (optIn.configStyleArguments && align.openParenDefnSite)
errors += s"optIn.configStyleArguments with align.openParenDefnSite"
}
if (errors.nonEmpty) {
val prefix = s"newlines: can't use source=${newlines.source} and ["
throw new IllegalArgumentException(errors.mkString(prefix, ",", "]"))
}
}

private implicit val runnerReader = runner.reader
private implicit val projectReader = project.reader
private implicit val rewriteReader = rewrite.reader
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,13 @@ class FormatOps(val tree: Tree, val initStyle: ScalafmtConfig) {
}

def opensConfigStyle(
ft: => FormatToken,
whenSourceIgnored: Boolean
)(implicit style: ScalafmtConfig): Boolean =
if (style.newlines.sourceIgnored) whenSourceIgnored
else opensConfigStyleClassic(ft)

private def opensConfigStyleClassic(
ft: FormatToken
)(implicit style: ScalafmtConfig): Boolean = {
def opensImplicit =
Expand Down Expand Up @@ -1183,4 +1190,10 @@ class FormatOps(val tree: Tree, val initStyle: ScalafmtConfig) {
case _ => true
}

def getClosingIfEnclosedInMatching(tree: Tree): Option[T] =
tree.tokens.lastOption.filter(matchingOpt(_).contains(tree.tokens.head))

def isEnclosedInMatching(tree: Tree): Boolean =
getClosingIfEnclosedInMatching(tree).isDefined

}
107 changes: 75 additions & 32 deletions scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ class Router(formatOps: FormatOps) {

case FormatToken(T.LeftParen() | T.LeftBracket(), right, between)
if style.optIn.configStyleArguments && isDefnOrCallSite(leftOwner) &&
(opensConfigStyle(formatToken) || {
(opensConfigStyle(formatToken, false) || {
forceConfigStyle(leftOwner) && !styleMap.forcedBinPack(leftOwner)
}) =>
val open = formatToken.left
Expand Down Expand Up @@ -627,29 +627,32 @@ class Router(formatOps: FormatOps) {
val open = formatToken.left
val close = matching(open)
val indent = getApplyIndent(leftOwner)
val (_, args) = getApplyArgs(formatToken, false)
val optimal = leftOwner.tokens.find(_.is[T.Comma]).orElse(Some(close))
val isBracket = open.is[T.LeftBracket]
// TODO(olafur) DRY. Same logic as in default.
val exclude =
if (isBracket)
insideBlock[T.LeftBracket](formatToken, close)
else
insideBlock[T.LeftBrace](formatToken, close)
val excludeRanges: Set[Range] = exclude.flatMap(parensRange)
val unindent = UnindentAtExclude(exclude, Num(-indent.n))
def ignoreBlocks(x: FormatToken): Boolean = {
excludeRanges.exists(_.contains(x.left.end))
}
val noSplitPolicy =
penalizeAllNewlines(close, 3, ignore = ignoreBlocks)
.andThen(unindent)
def baseNoSplit = Split(NoSplit, 0).withIndent(indent, close, Before)
val noSplit =
if (style.newlines.sourceIgnored && style.optIn.configStyleArguments)
baseNoSplit.withSingleLine(close)
else {
val opt = leftOwner.tokens.find(_.is[T.Comma]).orElse(Some(close))
val isBracket = open.is[T.LeftBracket]
// TODO(olafur) DRY. Same logic as in default.
val exclude =
if (isBracket)
insideBlock[T.LeftBracket](formatToken, close)
else
insideBlock[T.LeftBrace](formatToken, close)
val excludeRanges: Set[Range] = exclude.flatMap(parensRange)
val unindent = UnindentAtExclude(exclude, Num(-indent.n))
def ignoreBlocks(x: FormatToken): Boolean = {
excludeRanges.exists(_.contains(x.left.end))
}
val policy =
penalizeAllNewlines(close, 3, ignore = ignoreBlocks)
.andThen(unindent)
baseNoSplit.withOptimalTokenOpt(opt).withPolicy(policy)
}
val nlIndent = if (style.activeForEdition_2020_03) indent else Num(4)
Seq(
Split(NoSplit, 0)
.withOptimalTokenOpt(optimal)
.withPolicy(noSplitPolicy)
.withIndent(indent, close, Before),
noSplit,
Split(Newline, 2).withIndent(nlIndent, close, Before)
)
case FormatToken(T.LeftParen(), T.RightParen(), _) =>
Expand Down Expand Up @@ -679,11 +682,33 @@ class Router(formatOps: FormatOps) {
val isBracket = open.is[T.LeftBracket]
val bracketCoef = if (isBracket) Constants.BracketPenalty else 1

val sourceIgnored = style.newlines.sourceIgnored
val notSingleEnclosedArgument =
sourceIgnored && !(singleArgument && isEnclosedInMatching(args(0)))
val useConfigStyle =
style.optIn.configStyleArguments && notSingleEnclosedArgument

def isExcludedTree(tree: Tree): Boolean = tree match {
case t: Init => t.argss.nonEmpty
case t: Term.Apply => t.args.nonEmpty
case t: Term.ApplyType => t.targs.nonEmpty
case t: Term.Match => t.cases.nonEmpty
case t: Term.New => t.init.argss.nonEmpty
case _: Term.NewAnonymous => true
case _ => false
}

val nestedPenalty = nestedApplies(leftOwner) + lhsPenalty
val excludeRanges =
if (isBracket) insideBlockRanges[T.LeftBracket](tok, close)
else if (style.activeForEdition_2020_03 && multipleArgs)
else if (style.activeForEdition_2020_03 && multipleArgs ||
notSingleEnclosedArgument &&
style.newlines.sourceIs(Newlines.unfold))
Set.empty[Range]
else if (style.newlines.sourceIs(Newlines.fold) &&
singleArgument &&
(!notSingleEnclosedArgument || isExcludedTree(args(0))))
parensRange(args(0).tokens.last).toSet
else insideBlockRanges[T.LeftBrace](tok, close)

val indent = getApplyIndent(leftOwner)
Expand Down Expand Up @@ -761,7 +786,7 @@ class Router(formatOps: FormatOps) {
} && (!handleImplicit || style.newlines.afterImplicitParamListModifier)
val alignTuple = align && isTuple(leftOwner)

val keepConfigStyleSplit =
val keepConfigStyleSplit = !sourceIgnored &&
style.optIn.configStyleArguments && newlines != 0
val splitsForAssign =
if (defnSite || isBracket || keepConfigStyleSplit) None
Expand Down Expand Up @@ -790,7 +815,7 @@ class Router(formatOps: FormatOps) {
}

val noSplitPolicy =
if (wouldDangle || mustDangle && isBracket)
if (wouldDangle || mustDangle && isBracket || useConfigStyle)
SingleLineBlock(close, exclude = excludeRanges)
else if (splitsForAssign.isDefined)
singleLine(3)
Expand Down Expand Up @@ -1278,9 +1303,10 @@ class Router(formatOps: FormatOps) {
)

case FormatToken(open: T.LeftParen, right, _) =>
val isConfig = opensConfigStyle(formatToken)
val isConfig = opensConfigStyle(formatToken, false)
val close = matching(open)
val editionActive = style.activeForEdition_2020_03
val editionActive = style.activeForEdition_2020_03 ||
!style.newlines.sourceIn(Newlines.classic)
def spaceSplitWithoutPolicy = {
val indent: Length = right match {
case T.KwIf() => StateColumn
Expand All @@ -1292,20 +1318,37 @@ class Router(formatOps: FormatOps) {
}
def spaceSplit =
spaceSplitWithoutPolicy.withPolicy(penalizeAllNewlines(close, 1))
def newlineSplit(forceDangle: Boolean) = {
def newlineSplit(cost: Int, forceDangle: Boolean) = {
val shouldDangle = forceDangle ||
editionActive && style.danglingParentheses.callSite
val policy =
if (!shouldDangle) NoPolicy
else newlinesOnlyBeforeClosePolicy(close)
Split(Newline, 0)
Split(Newline, cost)
.withPolicy(policy)
.withIndent(style.continuationIndent.callSite, close, Before)
}
if (isSingleLineComment(right) && editionActive)
Seq(newlineSplit(isConfig))
Seq(newlineSplit(0, isConfig))
else
Seq(if (isConfig) newlineSplit(true) else spaceSplit)
style.newlines.source match {
case Newlines.classic =>
Seq(if (isConfig) newlineSplit(0, true) else spaceSplit)
case Newlines.keep =>
Seq(if (newlines != 0) newlineSplit(0, isConfig) else spaceSplit)
case _ =>
val singleLine = !isSuperfluousParenthesis(open, leftOwner) ||
style.newlines.sourceIs(Newlines.unfold) &&
leftOwner.parent.exists {
case _: Template | _: Defn => false
case _ => true
}
Seq(
if (!singleLine) spaceSplit
else spaceSplitWithoutPolicy.withPolicy(SingleLineBlock(close)),
newlineSplit(10, true)
)
}

// Infix operator.
case tok @ FormatToken(op @ T.Ident(_), right, between)
Expand Down Expand Up @@ -1610,7 +1653,7 @@ class Router(formatOps: FormatOps) {
expire, {
case T.RightBrace() => true
case close @ T.RightParen()
if opensConfigStyle(tokens(matching(close))) =>
if opensConfigStyle(tokens(matching(close)), true) =>
// Example:
// def x = foo(
// 1
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,13 +124,13 @@ case class Split(
def withPenalty(penalty: Int): Split =
if (isIgnored) this else copy(cost = cost + penalty)

def withIndent(length: Length, expire: Token, expiresOn: ExpiresOn): Split =
length match {
case Num(0) => this
case _ =>
if (isIgnored) this
else copy(indents = Indent(length, expire, expiresOn) +: indents)
}
def withIndent(length: => Length, expire: => Token, when: ExpiresOn): Split =
if (isIgnored) this
else
length match {
case Num(0) => this
case x => copy(indents = Indent(x, expire, when) +: indents)
}

override def toString = {
val prefix = tag match {
Expand Down
Loading

0 comments on commit 09bf81f

Please sign in to comment.