From a6246e0aac644c22288c4663578b2fd791bc31b7 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Tue, 3 Sep 2024 09:51:01 -0700 Subject: [PATCH 1/3] Add tests with overflow `if`-cond before `then` --- .../src/test/resources/scala3/OptionalBraces.stat | 15 ++++++++++++++- .../resources/scala3/OptionalBraces_fold.stat | 15 ++++++++++++++- .../resources/scala3/OptionalBraces_keep.stat | 15 ++++++++++++++- .../resources/scala3/OptionalBraces_unfold.stat | 15 ++++++++++++++- 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat index 94b31d27fb..77dab60caa 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat @@ -95,7 +95,20 @@ extension [A]( def add2(b: A) = a + b def add3(b: A) = a + b -<<< if(cond) indentation +<<< #4133 if cond overflow before then +object a: + if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using gadtCtx) then { + // c1 + } +>>> +object a: + if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using + gadtCtx + ) + then { + // c1 + } +<<< if(cond) indentation trait A: val cond = if (true) diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat index 641f2977c9..085dd3ed59 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat @@ -96,7 +96,20 @@ extension [A]( def add2(b: A) = a + b def add3(b: A) = a + b -<<< if(cond) indentation +<<< #4133 if cond overflow before then +object a: + if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using gadtCtx) then { + // c1 + } +>>> +object a: + if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using + gadtCtx + ) + then { + // c1 + } +<<< if(cond) indentation trait A: val cond = if (true) diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat index e995cbbbd3..e3f4987cff 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat @@ -100,7 +100,20 @@ extension [A](a: Map[ def add2(b: A) = a + b def add3(b: A) = a + b -<<< if(cond) indentation +<<< #4133 if cond overflow before then +object a: + if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using gadtCtx) then { + // c1 + } +>>> +Idempotency violated +=> Diff (- obtained, + expected) + if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using +- gadtCtx +- ) ++ gadtCtx) + then { +<<< if(cond) indentation trait A: val cond = if (true) diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat index 63f816f135..71b511b629 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat @@ -96,7 +96,20 @@ extension [A]( def add2(b: A) = a + b def add3(b: A) = a + b -<<< if(cond) indentation +<<< #4133 if cond overflow before then +object a: + if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using gadtCtx) then { + // c1 + } +>>> +object a: + if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using + gadtCtx + ) + then { + // c1 + } +<<< if(cond) indentation trait A: val cond = if (true) From bff727b7ae2ce5f514905a016f004c07ea8e3440 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Mon, 2 Sep 2024 08:12:32 -0700 Subject: [PATCH 2/3] Router: reuse indent computation in apply parens --- .../scala/org/scalafmt/internal/Router.scala | 23 +++++++++---------- .../scala/org/scalafmt/internal/Split.scala | 2 ++ .../resources/scala3/OptionalBraces_keep.stat | 2 +- 3 files changed, 14 insertions(+), 13 deletions(-) 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 4c12c33725..d5f8ccf27b 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 @@ -899,9 +899,10 @@ class Router(formatOps: FormatOps) { val nestedPenalty = 1 + nestedApplies(leftOwner) + lhsPenalty - val indent = - if (anyDefnSite) Num(style.indent.getDefnSite(leftOwner)) - else Num(style.indent.callSite) + val indentLen = + if (anyDefnSite) style.indent.getDefnSite(leftOwner) + else style.indent.callSite + val indent = Indent(Num(indentLen), close, ExpiresOn.Before) val isBeforeOpenParen = if (defnSite) style.newlines.isBeforeOpenParenDefnSite @@ -935,7 +936,6 @@ class Router(formatOps: FormatOps) { else getNoSplitAfterOpening(ft, commentNL = null, spaceOk = !isBracket) val rightIsComment = right.is[T.Comment] - val noSplitIndent = if (rightIsComment) indent else Num(0) val align = !rightIsComment && clauseSiteFlags.alignOpenDelim && (!handleImplicit || style.newlines.forceAfterImplicitParamListModifier) @@ -955,7 +955,7 @@ class Router(formatOps: FormatOps) { decideNewlinesOnlyAfterToken(breakToken) Seq( Split(Newline, nestedPenalty + Constants.ExceedColumnPenalty) - .withPolicy(newlinePolicy).withIndent(indent, close, Before), + .withPolicy(newlinePolicy).withIndent(indent), Split(NoSplit, nestedPenalty).withSingleLineNoOptimal(breakToken) .andPolicy(newlinePolicy & newlineAfterAssignDecision), ) @@ -992,7 +992,7 @@ class Router(formatOps: FormatOps) { else insideBracesBlock(ft, close) def singleLine(newlinePenalty: Int)(implicit fileLine: FileLine): Policy = - if (multipleArgs && (isBracket || excludeBlocks.ranges.isEmpty)) + if (multipleArgs && (isBracket || excludeBlocks.isEmpty)) SingleLineBlock(close, noSyntaxNL = true) else if (isBracket) PenalizeAllNewlines(close, newlinePenalty, penalizeLambdas = false) @@ -1030,8 +1030,7 @@ class Router(formatOps: FormatOps) { else if (onlyConfigStyle) Seq( Split(noSplitMod, 0).withPolicy(oneArgOneLine & implicitPolicy) .withOptimalToken(right, killOnFail = true) - .withIndent(extraOneArgPerLineIndent) - .withIndent(indent, close, Before), + .withIndents(extraOneArgPerLineIndent, indent), ) else { val noSplitPolicy = @@ -1046,18 +1045,18 @@ class Router(formatOps: FormatOps) { ) else if (splitsForAssign.isDefined) singleLine(3) else singleLine(10) + val okIndent = rightIsComment || handleImplicit Seq( Split(noSplitMod, 0, policy = noSplitPolicy) .notIf(mustDangleForTrailingCommas).withOptimalToken(optimal) - .withIndent(noSplitIndent, close, Before), + .withIndent(indent, ignore = !okIndent), Split(noSplitMod, (implicitPenalty + lhsPenalty) * bracketCoef) .withPolicy(oneArgOneLine & implicitPolicy).onlyIf( (notTooManyArgs && align) || (handleImplicit && style.newlines.notBeforeImplicitParamListModifier), ).withIndents( - if (align) getOpenParenAlignIndents(close) - else Seq(Indent(indent, close, ExpiresOn.Before)), + if (align) getOpenParenAlignIndents(close) else Seq(indent), ), ) } @@ -1085,7 +1084,7 @@ class Router(formatOps: FormatOps) { .map(InfixSplits(_, ft).nlPolicy), ) } - Seq(split.withIndent(indent, close, Before)) + Seq(split.withIndent(indent)) } splitsNoNL ++ splitsNL ++ splitsForAssign.getOrElse(Seq.empty) 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 4d50ef1788..07e6d44d05 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 @@ -263,6 +263,8 @@ case class Split( def withIndentOpt(indent: => Option[Indent]): Split = withMod(modExt.withIndentOpt(indent)) + def withIndents(indents: Indent*): Split = withMod(modExt.withIndents(indents)) + def withIndents(indents: Seq[Indent], ignore: Boolean = false): Split = withMod(modExt.withIndents(indents), ignore) diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat index e3f4987cff..531741f585 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat @@ -111,7 +111,7 @@ Idempotency violated if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using - gadtCtx - ) -+ gadtCtx) ++ gadtCtx) then { <<< if(cond) indentation trait A: From c5ecf30aea4e89aeac142a077508c3e3b965d0c2 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Tue, 3 Sep 2024 09:52:59 -0700 Subject: [PATCH 3/3] Router: keep `then` NL cost at 1 even if no space --- .../main/scala/org/scalafmt/internal/Router.scala | 6 +++--- .../test/resources/scala3/OptionalBraces_keep.stat | 13 ++++++------- 2 files changed, 9 insertions(+), 10 deletions(-) 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 d5f8ccf27b..81a195c542 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 @@ -2008,12 +2008,12 @@ class Router(formatOps: FormatOps) { Split(Newline, 1), ) case FormatToken(_, _: T.KwThen | _: T.KwDo, _) => - if (style.newlines.sourceIgnored || noBreak()) Seq( - Split(Space, 0) + val okSpace = style.newlines.sourceIgnored || noBreak() + Seq( + Split(!okSpace, 0)(Space) .withOptimalToken(nextNonCommentSameLineAfter(ft).left), Split(Newline, 1), ) - else Seq(Split(Newline, 0)) // Last else branch case FormatToken(_: T.KwElse, _, _) if (leftOwner match { case t: Term.If => t.elsep match { diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat index 531741f585..c236d38a7e 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat @@ -106,13 +106,12 @@ object a: // c1 } >>> -Idempotency violated -=> Diff (- obtained, + expected) - if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using -- gadtCtx -- ) -+ gadtCtx) - then { +object a: + if reducePattern(caseBindingMap, scrutineeSym.termRef, cdef.pat)(using + gadtCtx) + then { + // c1 + } <<< if(cond) indentation trait A: val cond =