Skip to content

Commit

Permalink
Router: treat multi-line comment before NL as SLC
Browse files Browse the repository at this point in the history
Since we now can rewrite single-line comments as multi-line, the latter
need to be treated the same, or idempotence will arise.
  • Loading branch information
kitbellew committed Dec 29, 2021
1 parent 67fd690 commit 331c78a
Show file tree
Hide file tree
Showing 15 changed files with 113 additions and 95 deletions.
5 changes: 5 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,10 @@ the token).
> [ScalaFiddle Playgroud](https://scalameta.org/docs/trees/scalafiddle.html) or
> [AST Explorer](https://scalameta.org/docs/trees/astexplorer.html).
> The special code `//` is used for single-line comments. Also, since v3.3.1, this
> includes multi-line comments `/* ... */` which do not themselves contain newlines
> but are followed by one (i.e., can trivially be changed to a `//` comment).
```scala mdoc:scalafmt
align.tokens = [{
code = "=>"
Expand Down Expand Up @@ -1971,6 +1975,7 @@ class Engine[TD, EI, PD, Q, P, A](
This flag tries to avoid a newline if the line would overflow only because of
trailing single-line comment (one which starts with `//`).
Also, since v3.3.1, this includes a trailing `/* ... */` without embedded breaks.

```scala mdoc:scalafmt
maxColumn = 40
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,13 @@ package org.scalafmt.internal
* Used by [[Policy]] to enforce non-local formatting.
*/
case class Decision(formatToken: FormatToken, splits: Seq[Split]) {
import org.scalafmt.util.TokenOps._

@inline def noNewlines: Seq[Split] =
Decision.noNewlineSplits(splits)

@inline def onlyNewlinesWithFallback(default: => Split): Seq[Split] =
Decision.onlyNewlinesWithFallback(splits, default)

def forceNewline: Seq[Split] =
if (isAttachedSingleLineComment(formatToken))
splits
else
onlyNewlinesWithoutFallback

def onlyNewlinesWithoutFallback: Seq[Split] =
onlyNewlineSplits

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package org.scalafmt.internal
import org.scalafmt.Error.UnexpectedTree
import org.scalafmt.config.{
BinPack,
Comments,
Newlines,
ScalafmtConfig,
ScalafmtRunner,
Expand Down Expand Up @@ -193,9 +192,8 @@ class FormatOps(
_: T.Equals if isInfixRhs(start) || !startsNewBlockOnRight(start) =>
None
case c: T.Comment
if start.noBreak &&
(style.comments.wrap ne Comments.Wrap.trailing) &&
(!start.left.is[T.LeftParen] || isSingleLineComment(c)) =>
if start.noBreak && (!start.left.is[T.LeftParen] ||
tokens.isBreakAfterRight(start)) =>
Some(c)
case _ => Some(start.left)
}
Expand All @@ -211,9 +209,7 @@ class FormatOps(
_: T.Equals =>
None
case _: T.RightParen if start.left.is[T.LeftParen] => None
case c: T.Comment
if isSingleLineComment(c) && start.noBreak &&
(style.comments.wrap ne Comments.Wrap.trailing) =>
case c: T.Comment if start.noBreak && tokens.isBreakAfterRight(start) =>
Some(c)
case _ if start.noBreak && isInfix => None
case _ => Some(start.left)
Expand Down Expand Up @@ -484,7 +480,7 @@ class FormatOps(
getOptimalTokenFor(tokens(token))

def getOptimalTokenFor(ft: FormatToken): T =
if (isAttachedSingleLineComment(ft)) ft.right else ft.left
if (tokens.isAttachedCommentThenBreak(ft)) ft.right else ft.left

def insideInfixSplit(
app: InfixApp,
Expand Down Expand Up @@ -982,9 +978,10 @@ class FormatOps(
case lb: T.LeftBrace if template.self.tokens.isEmpty =>
Policy.after(lb) {
// Force template to be multiline.
case d @ Decision(FormatToken(`lb`, right, _), _)
if !right.is[T.RightBrace] => // corner case, body is {}
d.forceNewline
case d @ Decision(ftd @ FormatToken(`lb`, right, _), s)
if !right.is[T.RightBrace] && // corner case, body is {}
!tokens.isAttachedCommentThenBreak(ftd) =>
d.onlyNewlinesWithoutFallback
}
case _ => NoPolicy
}
Expand Down Expand Up @@ -1183,8 +1180,8 @@ class FormatOps(

val paramGroupSplitter = Policy.on(lastParen) {
// If this is a class, then don't dangle the last paren unless the line ends with a comment
case Decision(FormatToken(previous, `lastParen`, _), _)
if shouldNotDangle && !isSingleLineComment(previous) =>
case Decision(ftd @ FormatToken(_, `lastParen`, _), _)
if shouldNotDangle && !isLeftCommentThenBreak(ftd) =>
Seq(Split(NoSplit, 0))
// Indent separators `)(` and `](` by `indentSep`
case Decision(t @ FormatToken(_, rp @ RightParenOrBracket(), _), _)
Expand Down Expand Up @@ -1213,10 +1210,10 @@ class FormatOps(
Split(NoSplit.orNL(!shouldAddNewline), 0)
.withIndent(indentParam, close2, ExpiresOn.Before)
)
case Decision(FormatToken(soft.ImplicitOrUsing(), r, _), _)
case Decision(ftd @ FormatToken(soft.ImplicitOrUsing(), _, _), _)
if (style.newlines.forceAfterImplicitParamListModifier ||
style.verticalMultiline.newlineAfterImplicitKW) &&
!isSingleLineComment(r) =>
!tokens.isRightCommentThenBreak(ftd) =>
Seq(Split(Newline, 0))
}

Expand Down Expand Up @@ -1321,9 +1318,9 @@ class FormatOps(
spaceOk: Boolean
)(implicit style: ScalafmtConfig): Modification =
ft.right match {
case c: T.Comment =>
val isDetachedSlc = ft.hasBreak && isSingleLineComment(c)
if (isDetachedSlc || next(ft).leftHasNewline) null else Space
case _: T.Comment =>
val isDetachedSlc = ft.hasBreak && tokens.isBreakAfterRight(ft)
if (isDetachedSlc || ft.rightHasNewline) null else Space
case _ =>
Space(style.spaces.inParentheses && spaceOk)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ case class FormatToken(left: Token, right: Token, meta: FormatToken.Meta) {
@inline def hasBlankLine: Boolean = FormatToken.hasBlankLine(newlinesBetween)

@inline def leftHasNewline = meta.left.hasNL
@inline def rightHasNewline = meta.right.hasNL
@inline def hasBreakOrEOF: Boolean = hasBreak || right.is[Token.EOF]

/** A format token is uniquely identified by its left token.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,22 @@ class FormatTokens(leftTok2tok: Map[TokenOps.TokenHash, Int])(
@inline
def tokenBefore(trees: Seq[Tree]): FormatToken = tokenBefore(trees.head)

@inline
def isBreakAfterRight(ft: FormatToken): Boolean =
next(ft).hasBreakOrEOF

@inline
def isRightCommentThenBreak(ft: FormatToken): Boolean =
ft.right.is[Token.Comment] && isBreakAfterRight(ft)

@inline
def isRightLikeSingleLineComment(ft: FormatToken): Boolean =
isRightCommentThenBreak(ft) && !ft.rightHasNewline

@inline
def isAttachedCommentThenBreak(ft: FormatToken): Boolean =
ft.noBreak && isRightCommentThenBreak(ft)

}

object FormatTokens {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -590,7 +590,7 @@ class FormatWriter(formatOps: FormatOps) {

def formatComment(implicit sb: StringBuilder): Unit = {
val text = tok.meta.left.text
if (isSingleLineComment(text))
if (text.startsWith("//"))
new FormatSlc(text).format
else if (text == "/**/")
sb.append(text)
Expand Down Expand Up @@ -1113,7 +1113,7 @@ class FormatWriter(formatOps: FormatOps) {
else if (alignContainer ne container) {
val pos1 = alignContainer.pos
val pos2 = container.pos
if (isSingleLineComment(ft.right)) {
if (tokens.isRightLikeSingleLineComment(ft)) {
if (pos1.end >= tokens.prevNonCommentSameLine(ft).left.end)
appendCandidate()
} else if (pos2.start <= pos1.start && pos2.end >= pos1.end) {
Expand Down Expand Up @@ -1261,7 +1261,7 @@ class FormatWriter(formatOps: FormatOps) {

def getAlignContainer(implicit fl: FormatLocation): Option[Tree] = {
val ft = fl.formatToken
val slc = isSingleLineComment(ft.right)
val slc = tokens.isRightLikeSingleLineComment(ft)
val code = if (slc) "//" else ft.meta.right.text

fl.style.alignMap.get(code).flatMap { matchers =>
Expand Down Expand Up @@ -1559,8 +1559,8 @@ class FormatWriter(formatOps: FormatOps) {
// skip checking if row1 and row2 matches if both of them continues to a single line of comment
// in order to vertical align adjacent single lines of comment.
// see: https://github.com/scalameta/scalafmt/issues/1242
val slc1 = isSingleLineComment(row1.floc.formatToken.right)
val slc2 = isSingleLineComment(row2.floc.formatToken.right)
val slc1 = tokens.isRightLikeSingleLineComment(row1.floc.formatToken)
val slc2 = tokens.isRightLikeSingleLineComment(row2.floc.formatToken)
if (slc1 || slc2) slc1 && slc2
else
(row1.alignContainer eq row2.alignContainer) &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,8 @@ class Router(formatOps: FormatOps) {
val newlineBeforeClosingCurly = decideNewlinesOnlyBeforeClose(close)

val mustDangleForTrailingCommas = getMustDangleForTrailingCommas(close)
val mustUseNL = newlines != 0 && isSingleLineComment(right)
val mustUseNL = newlines != 0 &&
tokens.isRightCommentThenBreak(formatToken)
val newlinePolicy = style.importSelectors match {
case ImportSelectors.singleLine if mustUseNL =>
policy
Expand Down Expand Up @@ -397,7 +398,7 @@ class Router(formatOps: FormatOps) {
case t => getLastNonTrivialToken(t) -> ExpiresOn.Before
}

val hasSingleLineComment = isSingleLineComment(right)
val hasSingleLineComment = tokens.isRightCommentThenBreak(formatToken)
val indent = // don't indent if the body is empty `{ x => }`
if (isEmptyFunctionBody(leftOwner) && !right.is[T.Comment]) 0
else if (leftOwner.is[Template]) 0 // { applied the indent
Expand Down Expand Up @@ -1107,10 +1108,10 @@ class Router(formatOps: FormatOps) {
else {
val penalizeOpens = bracketPenalty.fold(Policy.noPolicy) { p =>
Policy.before(close) {
case Decision(FormatToken(o: T.LeftBracket, r, m), s)
case Decision(ftd @ FormatToken(o: T.LeftBracket, _, m), s)
if isDefnSite(m.leftOwner) &&
!styleMap.at(o).binPack.defnSite(o).isNever =>
if (isSingleLineComment(r)) s
if (tokens.isRightCommentThenBreak(ftd)) s
else s.map(x => if (x.isNL) x.withPenalty(p) else x)
}
}
Expand Down Expand Up @@ -1386,18 +1387,19 @@ class Router(formatOps: FormatOps) {
Split(NoSplit, 0)
)
// These are mostly filtered out/modified by policies.
case ft @ FormatToken(lc: T.Comma, c: T.Comment, _) =>
case ft @ FormatToken(lc: T.Comma, _: T.Comment, _) =>
val nextFt = next(ft)
if (ft.hasBlankLine) Seq(Split(NewlineT(isDouble = true), 0))
else if (isSingleLineComment(c) || ft.meta.right.hasNL)
else if (nextFt.hasBreak || ft.meta.right.hasNL)
Seq(Split(Space.orNL(newlines), 0))
else if (newlines == 0) {
val endFt = nextNonCommentSameLine(next(ft))
val endFt = nextNonCommentSameLine(nextFt)
val useSpaceOnly = endFt.hasBreak ||
rightIsCloseDelimToAddTrailingComma(lc, endFt)
Seq(Split(Space, 0), Split(useSpaceOnly, 1)(Newline))
} else if (
!style.comments.willWrap &&
rightIsCloseDelimToAddTrailingComma(lc, nextNonComment(next(ft)))
rightIsCloseDelimToAddTrailingComma(lc, nextNonComment(nextFt))
) Seq(Split(Space, 0), Split(Newline, 1))
else Seq(Split(Newline, 0))
case FormatToken(_: T.Comma, right, _) if leftOwner.isNot[Template] =>
Expand Down Expand Up @@ -1991,7 +1993,7 @@ class Router(formatOps: FormatOps) {
.withPolicy(decideNewlinesOnlyBeforeClose(close), !shouldDangle)
.withIndent(style.indent.callSite, close, Before)
}
if (isSingleLineComment(right))
if (tokens.isRightCommentThenBreak(formatToken))
Seq(newlineSplit(0, isConfig))
else
style.newlines.source match {
Expand Down Expand Up @@ -2045,7 +2047,7 @@ class Router(formatOps: FormatOps) {
val bodyBlock = isCaseBodyABlock(arrowFt, owner)
def defaultPolicy = decideNewlinesOnlyAfterToken(postArrowFt.left)
val policy =
if (bodyBlock || isAttachedSingleLineComment(arrowFt)) NoPolicy
if (bodyBlock || tokens.isAttachedCommentThenBreak(arrowFt)) NoPolicy
else if (isCaseBodyEnclosedAsBlock(postArrowFt, owner)) {
val postParenFt = nextNonCommentSameLine(next(postArrowFt))
val lparen = postParenFt.left
Expand Down Expand Up @@ -2462,14 +2464,13 @@ class Router(formatOps: FormatOps) {
}
formatToken match {
case FormatToken(_: T.BOF, _, _) => splits
case FormatToken(_, c: T.Comment, _) if isSingleLineComment(c) =>
def adapted =
if (withNoIndent(formatToken)) splits.map(_.withNoIndent)
else splits
if (formatToken.hasBreak) splitsAsNewlines(adapted)
else splits.map(_.withMod(Space))
case ft @ FormatToken(_, c: T.Comment, _)
if tokens.isBreakAfterRight(ft) =>
if (ft.noBreak) splits.map(_.withMod(Space))
else if (!rhsIsCommentedOut(ft)) splitsAsNewlines(splits)
else splitsAsNewlines(splits.map(_.withNoIndent))
// Only newlines after inline comments.
case FormatToken(c: T.Comment, _, _) if isSingleLineComment(c) =>
case ft @ FormatToken(_: T.Comment, _, _) if ft.hasBreak =>
splitsAsNewlines(splits)
case _ => splits
}
Expand Down Expand Up @@ -2569,7 +2570,7 @@ class Router(formatOps: FormatOps) {
}

def baseSpaceSplit(implicit fileLine: FileLine) =
Split(Space, 0).notIf(isSingleLineComment(ft.right))
Split(tokens.isRightCommentThenBreak(ft), 0)(Space)
def twoBranches(implicit fileLine: FileLine) =
baseSpaceSplit
.withOptimalToken(optimal)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import scala.meta.tokens.Token

import org.scalafmt.config.Comments
import org.scalafmt.config.ScalafmtConfig
import org.scalafmt.util.TokenOps
import org.scalafmt.util.TreeOps._

/** A partial formatting solution up to splits.length number of tokens.
Expand Down Expand Up @@ -191,7 +190,7 @@ final case class State(
else prev.getOverflowPenalty(split, defaultOverflowPenalty + 1)
} else if (
style.newlines.avoidForSimpleOverflowSLC &&
TokenOps.isSingleLineComment(ft.right)
tokens.isRightCommentThenBreak(ft)
) {
result(0, prevActive)
} else if (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ object Imports extends RewriteFactory {
ctx.tokenTraverser.findAtOrBefore(ctx.getIndex(tok) - 1) {
case _: Token.LF =>
if (hadLf) Some(true) else { hadLf = true; None }
case t: Token.Comment if TokenOps.isSingleLineComment(t) =>
case t: Token.Comment if TokenOps.isSingleLineIfComment(t) =>
slc.prepend(t); hadLf = false; None
case Whitespace() => None
case _ => if (!hadLf && !slc.isEmpty) slc.remove(0); Some(false)
Expand All @@ -393,7 +393,7 @@ object Imports extends RewriteFactory {
protected final def getCommentAfter(tok: Token): Option[Token] =
ctx.tokenTraverser.findAtOrAfter(ctx.getIndex(tok) + 1) {
case _: Token.LF => Some(false)
case t: Token.Comment if TokenOps.isSingleLineComment(t) =>
case t: Token.Comment if TokenOps.isSingleLineIfComment(t) =>
Some(true)
case Whitespace() | _: Token.Comma => None
case _ => Some(false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import scala.meta.tokens.Token
import org.scalafmt.config.ScalafmtConfig
import org.scalafmt.internal.FormatToken
import org.scalafmt.internal.FormatTokens
import org.scalafmt.util.TokenOps

import metaconfig._

Expand Down Expand Up @@ -74,7 +73,7 @@ private class PreferCurlyFors(ftoks: FormatTokens)
case _: Token.Semicolon
if !style.rewrite.preferCurlyFors.removeTrailingSemicolonsOnly || {
val next = ftoks.next(ft)
next.hasBreak || TokenOps.isSingleLineComment(next.right)
next.hasBreak || ftoks.isRightCommentThenBreak(next)
} =>
ft.meta.rightOwner match {
case _: Term.For | _: Term.ForYield => removeToken
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ object PolicyOps {
noSyntaxNL: Boolean = false
)(implicit fileLine: FileLine)
extends Policy.Clause {
import TokenOps.isSingleLineComment
import TokenOps.isLeftCommentThenBreak
override val noDequeue: Boolean = true
override def toString: String = "SLB:" + super.toString
override val f: Policy.Pf = {
case Decision(ft, s)
if !(ft.right.is[T.EOF] || okSLC && isSingleLineComment(ft.left)) =>
if !(ft.right.is[T.EOF] || okSLC && isLeftCommentThenBreak(ft)) =>
if (noSyntaxNL && ft.leftHasNewline) Seq.empty else s.filterNot(_.isNL)
}
}
Expand Down
Loading

0 comments on commit 331c78a

Please sign in to comment.