Skip to content

Commit

Permalink
Router: ignore line breaks for blocks
Browse files Browse the repository at this point in the history
While previously we were trying to keep blocks without line breaks if
that's how they were originally input, now we actively attempt to take a
multi-line representation and fold it into a single line (fold), or take
a single-line representation and unfold it into multiple lines (unfold).

Fixes #271 #1002 #1043
  • Loading branch information
kitbellew committed Mar 9, 2020
1 parent b4d4a70 commit c71a109
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 42 deletions.
Expand Up @@ -125,6 +125,15 @@ case class Newlines(
s"newlines: can't use source=${source} and afterCurlyLambda=$afterCurlyLambda"
)
}

@inline
def sourceIs(hint: Newlines.SourceHints): Boolean =
hint eq source

@inline
def sourceIn(hints: Newlines.SourceHints*): Boolean =
hints.contains(source)

}

object Newlines {
Expand Down
@@ -1,7 +1,7 @@
package org.scalafmt.internal

import org.scalafmt.Error.UnexpectedTree
import org.scalafmt.config.{ImportSelectors, NewlineCurlyLambda}
import org.scalafmt.config.{ImportSelectors, NewlineCurlyLambda, Newlines}
import org.scalafmt.internal.ExpiresOn.{Left, Right}
import org.scalafmt.internal.Length.{Num, StateColumn}
import org.scalafmt.internal.Policy.NoPolicy
Expand Down Expand Up @@ -250,14 +250,28 @@ class Router(formatOps: FormatOps) {
else
getSingleLineDecisionPre2019NovOpt

def getSingleLineLambdaDecisionOpt = {
val ok = !style.newlines.alwaysBeforeCurlyBraceLambdaParams &&
getSpaceAndNewlineAfterCurlyLambda(newlines)._1
if (ok) Some(getSingleLineDecisionFor2019Nov) else None
}

// null if skipping
val singleLineDecisionOpt =
if (lambdaPolicy == null) getClassicSingleLineDecisionOpt
else if (style.activeForEdition_2020_01 &&
!style.newlines.alwaysBeforeCurlyBraceLambdaParams &&
getSpaceAndNewlineAfterCurlyLambda(newlines)._1)
Some(getSingleLineDecisionFor2019Nov)
else None
val singleLineDecisionOpt = style.newlines.source match {
case Newlines.keep if newlines != 0 => None
case Newlines.unfold => None
case Newlines.fold =>
// do not fold top-level blocks
if (leftOwner.parent.exists(_.parent.isEmpty) ||
leftOwner.is[Template]) None
else if (lambdaPolicy != null) getSingleLineLambdaDecisionOpt
else Some(getSingleLineDecisionFor2019Nov)
// old behaviour
case _ =>
if (lambdaPolicy == null) getClassicSingleLineDecisionOpt
else if (!style.activeForEdition_2020_01) None
else getSingleLineLambdaDecisionOpt
}

val singleLineSplit =
singleLineDecisionOpt.fold(Split.ignored) { sld =>
Expand All @@ -275,7 +289,8 @@ class Router(formatOps: FormatOps) {
Split(Space, 0)
.notIf(
style.newlines.alwaysBeforeCurlyBraceLambdaParams ||
isSelfAnnotation || lambdaPolicy == null
isSelfAnnotation || lambdaPolicy == null ||
style.newlines.sourceIs(Newlines.keep) && newlines != 0
)
.withOptimalTokenOpt(lambdaArrow)
.withIndent(lambdaIndent, close, Right)
Expand Down
35 changes: 11 additions & 24 deletions scalafmt-tests/src/test/resources/newlines/source_fold.stat
Expand Up @@ -14,9 +14,8 @@ println("bbb") } else c
}
>>>
object a {
if (a) {
println("bbb")
} else c
if (a) { println("bbb") }
else c
}
<<< 1.3: block, if, non-egyptian curlies
object a {
Expand All @@ -30,11 +29,8 @@ c
}}
>>>
object a {
if (a) {
println("bbb")
} else {
c
}
if (a) { println("bbb") }
else { c }
}
<<< 1.4: block #1043
object a {
Expand All @@ -51,9 +47,7 @@ object a {
c - 'A' + 10
} else if ('a' <= c && c <= 'f') {
c - 'a' + 10
} else {
-1
}
} else { -1 }
}
<<< 1.5: block #1043 with lines joined
object a {
Expand All @@ -68,9 +62,7 @@ object a {
c - 'A' + 10
} else if ('a' <= c && c <= 'f') {
c - 'a' + 10
} else {
-1
}
} else { -1 }
}
<<< 1.6: block, try-catch, line too long
object a {
Expand Down Expand Up @@ -101,13 +93,9 @@ try {
}
>>>
object a {
try {
a
} catch {
case b => ; case c =>
} finally {
d
}
try { a }
catch { case b => ; case c => }
finally { d }
}
<<< 1.9: block, try-catch, with breaks, top-level
try {
Expand All @@ -117,9 +105,8 @@ try {
>>>
try {
a
} catch {
case b => ; case c =>
} finally {
} catch { case b => ; case c => }
finally {
d
}
<<< 1.a: block, class
Expand Down
28 changes: 19 additions & 9 deletions scalafmt-tests/src/test/resources/newlines/source_unfold.stat
Expand Up @@ -46,8 +46,9 @@ object a {
-1 }}
>>>
object a {
if ('0' <= c && c <= '9') { c - '0' }
else if ('A' <= c && c <= 'F') {
if ('0' <= c && c <= '9') {
c - '0'
} else if ('A' <= c && c <= 'F') {
c - 'A' + 10
} else if ('a' <= c && c <= 'f') {
c - 'a' + 10
Expand All @@ -63,8 +64,9 @@ object a {
}}
>>>
object a {
if ('0' <= c && c <= '9') { c - '0' }
else if ('A' <= c && c <= 'F') {
if ('0' <= c && c <= '9') {
c - '0'
} else if ('A' <= c && c <= 'F') {
c - 'A' + 10
} else if ('a' <= c && c <= 'f') {
c - 'a' + 10
Expand All @@ -80,17 +82,23 @@ try { aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa } catch { case b => }
object a {
try {
aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
} catch { case b => }
} catch {
case b =>
}
}
<<< 1.7: block, try-catch, no breaks
object a {
try { a } catch { case b => ; case c => } finally { d }
}
>>>
object a {
try { a }
catch { case b => ; case c => }
finally { d }
try {
a
} catch {
case b => ; case c =>
} finally {
d
}
}
<<< 1.8: block, try-catch, with breaks
object a {
Expand Down Expand Up @@ -149,7 +157,9 @@ object a {
type c = Int
}
object b {
def apply(c: Int) = { new d(c) }
def apply(c: Int) = {
new d(c)
}
}
}
<<< 1.c: block followed by an infix, overflowing if no break
Expand Down

0 comments on commit c71a109

Please sign in to comment.