Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PolicyOps: use newlines.ignoreInSyntax #3613

Merged
merged 2 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -2576,6 +2576,44 @@ newlines.inInterpolation
- `avoid`: attemps to avoid breaks within the spliced code, regardless of line overflow
- `oneline`: formats the splice on a single line, or breaks after `${` if overflows

### `newlines.ignoreInSyntax`

The formatter frequently chooses between adding a newline and continuing the
same line but either prohibiting or heavily discouraging subsequent newlines
_between_ tokens, to fit the rest of the expression on the same line.

However, in many cases and, for historical reasons, intentionally, newlines
_within_ tokens have been frequently ignored, leading to "single-line" blocks
which actually span multiple lines.

This boolean parameter now allows controlling whether to ignore newlines found
in syntax of strings or other possibly multi-line tokens when newlines are
otherwise prohibited or undesirable (such as for single-line formatting).
Comment on lines +2581 to +2591
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tgodzik do you think this explanation makes it a bit clearer?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leading to "single-line" blocks which actually span multiple lines. is a bit unclear to me, though I guess the example here would need to be the best explanation we can get


```scala mdoc:defaults
newlines.ignoreInSyntax
```

> Since v3.7.13. Prior to that, this behaviour was always enabled.

```scala mdoc:scalafmt
newlines.ignoreInSyntax = true
---
// ignores newline in string, pretends everything fits on one line
println(s"""${1}
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an example. contains newline but will be formatted without breaks around open and close parentheses.

""".stripMargin
)
```

```scala mdoc:scalafmt
newlines.ignoreInSyntax = false
---
// detects newline in string, forces proper multi-line formatting
println(s"""${1}
""".stripMargin
)
```

### `optIn.annotationNewlines`

This boolean parameter controls newlines after annotations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ case class Newlines(
afterInfixMaxCountPerFile: Int = 500,
avoidForSimpleOverflow: Seq[AvoidForSimpleOverflow] = Seq.empty,
inInterpolation: InInterpolation = InInterpolation.allow,
ignoreInSyntax: Boolean = true,
avoidAfterYield: Boolean = true
) {
if (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1687,7 +1687,7 @@ class FormatOps(
nlSplitFunc: Int => Split,
isKeep: Boolean,
spaceIndents: Seq[Indent] = Seq.empty
): Seq[Split] = {
)(implicit style: ScalafmtConfig): Seq[Split] = {
def bheadFT = tokens.getHead(body)
val blastFT = tokens.getLastNonTrivial(body)
val blast = blastFT.left
Expand Down Expand Up @@ -1788,14 +1788,14 @@ class FormatOps(
nlSplitFunc: Int => Split,
isKeep: Boolean,
spaceIndents: Seq[Indent]
): Seq[Split] =
)(implicit style: ScalafmtConfig): Seq[Split] =
if (body.tokens.isEmpty) Seq(Split(Space, 0))
else foldedNonEmptyNonComment(body, nlSplitFunc, isKeep, spaceIndents)

private def unfoldedSpaceNonEmptyNonComment(
body: Tree,
slbOnly: Boolean
): Split = {
)(implicit style: ScalafmtConfig): Split = {
val expire = nextNonCommentSameLine(tokens.getLastNonTrivial(body)).left
def slbSplit(end: T)(implicit fileLine: FileLine) =
Split(Space, 0).withSingleLine(end, noSyntaxNL = true)
Expand All @@ -1816,7 +1816,7 @@ class FormatOps(
nlSplitFunc: Int => Split,
spaceIndents: Seq[Indent],
slbOnly: Boolean
): Seq[Split] =
)(implicit style: ScalafmtConfig): Seq[Split] =
if (body.tokens.isEmpty) Seq(Split(Space, 0).withIndents(spaceIndents))
else {
val spaceSplit = unfoldedSpaceNonEmptyNonComment(body, slbOnly)
Expand Down Expand Up @@ -1848,7 +1848,7 @@ class FormatOps(
body: Tree,
isKeep: Boolean,
spaceIndents: Seq[Indent] = Seq.empty
)(nlSplitFunc: Int => Split): Seq[Split] =
)(nlSplitFunc: Int => Split)(implicit style: ScalafmtConfig): Seq[Split] =
checkComment(ft, nlSplitFunc) { _ =>
foldedNonComment(body, nlSplitFunc, isKeep, spaceIndents)
}
Expand All @@ -1857,7 +1857,7 @@ class FormatOps(
ft: FormatToken,
body: Tree,
spaceIndents: Seq[Indent] = Seq.empty
)(nlSplitFunc: Int => Split): Seq[Split] =
)(nlSplitFunc: Int => Split)(implicit style: ScalafmtConfig): Seq[Split] =
checkComment(ft, nlSplitFunc) { _ =>
unfoldedNonComment(body, nlSplitFunc, spaceIndents, true)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.scalafmt.internal

import scala.meta.tokens.Token

import org.scalafmt.config.ScalafmtConfig
import org.scalafmt.internal.Policy.NoPolicy
import org.scalafmt.util.PolicyOps
import org.scalameta.FileLine
Expand Down Expand Up @@ -142,7 +143,7 @@ case class Split(
noSyntaxNL: Boolean = false,
killOnFail: Boolean = false,
rank: Int = 0
)(implicit fileLine: FileLine): Split =
)(implicit fileLine: FileLine, style: ScalafmtConfig): Split =
withSingleLineAndOptimal(
expire,
expire,
Expand All @@ -158,7 +159,7 @@ case class Split(
noSyntaxNL: Boolean = false,
killOnFail: Boolean = false,
rank: Int = 0
)(implicit fileLine: FileLine): Split =
)(implicit fileLine: FileLine, style: ScalafmtConfig): Split =
expire.fold(this)(
withSingleLine(_, exclude, noSyntaxNL, killOnFail, rank)
)
Expand All @@ -170,7 +171,7 @@ case class Split(
noSyntaxNL: Boolean = false,
killOnFail: Boolean = false,
rank: Int = 0
)(implicit fileLine: FileLine): Split =
)(implicit fileLine: FileLine, style: ScalafmtConfig): Split =
withOptimalToken(optimal, killOnFail)
.withSingleLineNoOptimal(expire, exclude, noSyntaxNL, rank)

Expand All @@ -179,7 +180,7 @@ case class Split(
exclude: => TokenRanges = TokenRanges.empty,
noSyntaxNL: Boolean = false,
rank: Int = 0
)(implicit fileLine: FileLine): Split =
)(implicit fileLine: FileLine, style: ScalafmtConfig): Split =
withPolicy(
SingleLineBlock(expire, exclude, noSyntaxNL = noSyntaxNL, rank = rank)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package org.scalafmt.util

import scala.meta.tokens.{Token => T}

import org.scalafmt.config.ScalafmtConfig
import org.scalafmt.internal._
import org.scalameta.FileLine

Expand All @@ -16,12 +17,13 @@ object PolicyOps {
penalizeLambdas: Boolean = true,
noSyntaxNL: Boolean = false,
val rank: Int = 0
)(implicit fileLine: FileLine)
)(implicit fileLine: FileLine, style: ScalafmtConfig)
extends Policy.Clause {
override val noDequeue: Boolean = false
private val checkSyntax = noSyntaxNL || !style.newlines.ignoreInSyntax
override val f: Policy.Pf = {
case Decision(ft, s) if penalizeLambdas || !ft.left.is[T.RightArrow] =>
if (noSyntaxNL && ft.leftHasNewline) s.map(_.withPenalty(penalty))
if (checkSyntax && ft.leftHasNewline) s.map(_.withPenalty(penalty))
else s.map(x => if (x.isNL) x.withPenalty(penalty) else x)
}
override def toString: String = s"PNL:${super.toString}+$penalty"
Expand All @@ -33,7 +35,7 @@ object PolicyOps {
penalty: Int,
penalizeLambdas: Boolean = true,
noSyntaxNL: Boolean = false
)(implicit fileLine: FileLine): Policy = {
)(implicit fileLine: FileLine, style: ScalafmtConfig): Policy = {
new PenalizeAllNewlines(
Policy.End.Before(expire),
penalty,
Expand All @@ -54,15 +56,16 @@ object PolicyOps {
okSLC: Boolean = false,
noSyntaxNL: Boolean = false,
val rank: Int = 0
)(implicit fileLine: FileLine)
)(implicit fileLine: FileLine, style: ScalafmtConfig)
extends Policy.Clause {
import TokenOps.isLeftCommentThenBreak
override val noDequeue: Boolean = true
override def toString: String = "SLB:" + super.toString
private val checkSyntax = noSyntaxNL || !style.newlines.ignoreInSyntax
override val f: Policy.Pf = {
case Decision(ft, s)
if !(ft.right.is[T.EOF] || okSLC && isLeftCommentThenBreak(ft)) =>
if (noSyntaxNL && ft.leftHasNewline) Seq.empty else s.filterNot(_.isNL)
if (checkSyntax && ft.leftHasNewline) Seq.empty else s.filterNot(_.isNL)
}
}

Expand All @@ -74,7 +77,7 @@ object PolicyOps {
okSLC: Boolean = false,
noSyntaxNL: Boolean = false,
rank: Int = 0
)(implicit fileLine: FileLine): Policy =
)(implicit fileLine: FileLine, style: ScalafmtConfig): Policy =
policyWithExclude(exclude, Policy.End.On, Policy.End.After)(
Policy.End.On(expire),
new SingleLineBlock(
Expand Down
31 changes: 31 additions & 0 deletions scalafmt-tests/src/test/resources/default/String.stat
Original file line number Diff line number Diff line change
Expand Up @@ -599,3 +599,34 @@ val s = raw"""${
else "else"
end if
}"""
<<< #3608 ignoreInSyntax
maxColumn = 30
assumeStandardLibraryStripMargin = true
newlines.ignoreInSyntax = true
===
println("""|
|A very long string
|with multiple lines
|""".stripMargin
)
>>>
println("""|
|A very long string
|with multiple lines
|""".stripMargin)
<<< #3608 !ignoreInSyntax
maxColumn = 30
assumeStandardLibraryStripMargin = true
newlines.ignoreInSyntax = false
===
println("""|
|A very long string
|with multiple lines
|""".stripMargin
)
>>>
println(
"""|
|A very long string
|with multiple lines
|""".stripMargin)
14 changes: 14 additions & 0 deletions scalafmt-tests/src/test/resources/test/StripMargin.stat
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,20 @@ final class MyClass {
println(s"""${1}
""".stripMargin)
}
<<< #2025 3 !ignoreInSyntax
newlines.ignoreInSyntax = false
===
final class MyClass {
println(s"""${1}
""".stripMargin)
}
>>>
final class MyClass {
println(
s"""${1}
""".stripMargin
)
}
<<< #3090 string without margin
maxColumn = 100
optIn.breaksInsideChains = true
Expand Down
Loading