From d74b018576e3317cca3563bdf943dbab595ab389 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Mon, 7 Oct 2024 07:45:44 -0700 Subject: [PATCH] FormatOps: simplify logic of defnSiteLastToken --- .../org/scalafmt/internal/FormatOps.scala | 33 +++++++------------ .../scala/org/scalafmt/internal/Router.scala | 14 ++++---- .../src/test/resources/default/Advanced.stat | 2 +- .../src/test/resources/default/Lambda.stat | 2 +- .../test/scala/org/scalafmt/FormatTests.scala | 2 +- 5 files changed, 22 insertions(+), 31 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala index d80b37a097..8390c89fd2 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala @@ -209,28 +209,17 @@ class FormatOps( // invoked on closing paren, part of ParamClause @tailrec - final def defnSiteLastToken(close: FormatToken, t: Tree): FormatToken = { - def alt = getLast(t) - t match { - case _: Term.ParamClause | _: Type.FuncParamClause | - _: Member.ParamClauseGroup => t.parent match { - case Some(p) => defnSiteLastToken(close, p) - case _ => alt - } - case t: Defn.Def => getHeadOpt(t.body).fold(alt) { ft => - if (ft.left.is[T.LeftBrace] && t.body.is[Term.Block]) ft - else prevNonCommentBefore(ft) - } - case _: Ctor.Primary => close match { - // This is a terrible terrible hack. Please consider removing this. - // The RightParen() LeftBrace() pair is presumably a ") {" combination - // at a class definition - case FormatToken(_: T.RightParen, _: T.LeftBrace, _) => next(close) - case _ => close - } - case t => t.tokens.find(x => x.is[T.Equals] && owners(x) == t) - .fold(alt)(before) - } + final def defnSiteLastToken(t: Tree): Option[FormatToken] = t match { + case _: Term.ParamClause | _: Type.FuncParamClause | _: Type.FunctionType | + _: Member.ParamClauseGroup => t.parent match { + case Some(p) => defnSiteLastToken(p) + case _ => None + } + case t: Defn.Macro => tokenBeforeOpt(t.body).map(prevNonCommentBefore) + case t: Tree.WithBody => tokenBeforeOpt(t.body) + case t: Stat.WithTemplate => tokenBeforeOpt(t.templ) + case t: Decl => getLastOpt(t) + case _ => None } @inline 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 d414ecbcb1..c54d4c017f 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 @@ -415,7 +415,8 @@ class Router(formatOps: FormatOps) { } } - val nlSplit = Split(nl, 1).withPolicy(newlineBeforeClosingCurly) + val nlSplit = Split(nl, if (nl.isNL) 1 else 0) + .withPolicy(newlineBeforeClosingCurly) .withIndent(style.indent.main, close, Before) // must be after nlSplit @@ -953,10 +954,11 @@ class Router(formatOps: FormatOps) { val isBeforeOpenParen = if (defnSite) style.newlines.isBeforeOpenParenDefnSite else style.newlines.isBeforeOpenParenCallSite - val optimalFt: FormatToken = getSlbEndOnLeft( - if (isBeforeOpenParen || !defnSite || isBracket) afterClose - else defnSiteLastToken(afterClose, leftOwner), - ) + val optimalFtOpt = + if (isBeforeOpenParen || !defnSite || isBracket) None + else defnSiteLastToken(leftOwner) + val optimalFt: FormatToken = + getSlbEndOnLeft(optimalFtOpt.getOrElse(afterClose)) val optimal = optimalFt.left val wouldDangle = onlyConfigStyle || mustDangleForTrailingCommas || @@ -2676,7 +2678,7 @@ class Router(formatOps: FormatOps) { def newlineSplit(cost: Int)(implicit fileLine: FileLine) = CtrlBodySplits .withIndent(Split(Newline2x(ft), cost), body, endFt) - def getClassicSplits = + def getClassicSplits(implicit fileLine: FileLine) = if (ft.hasBreak) Seq(newlineSplit(0)) else Seq(baseSplit, newlineSplit(1)) style.newlines.beforeMultilineDef.fold { diff --git a/scalafmt-tests/shared/src/test/resources/default/Advanced.stat b/scalafmt-tests/shared/src/test/resources/default/Advanced.stat index 3118cd9ef5..e7cecd5746 100644 --- a/scalafmt-tests/shared/src/test/resources/default/Advanced.stat +++ b/scalafmt-tests/shared/src/test/resources/default/Advanced.stat @@ -206,7 +206,7 @@ options.testFilter match { }, { // else callStatement })(jstpe.AnyType) ->>> { stateVisits = 1739, stateVisits2 = 1727 } +>>> { stateVisits = 1740, stateVisits2 = 1728 } callStatement = js.If( genIsInstanceOf(callTrg, rtClass.tpe), { if (implMethodSym.owner == ObjectClass) { diff --git a/scalafmt-tests/shared/src/test/resources/default/Lambda.stat b/scalafmt-tests/shared/src/test/resources/default/Lambda.stat index 1fa6c65bb0..48c9e78481 100644 --- a/scalafmt-tests/shared/src/test/resources/default/Lambda.stat +++ b/scalafmt-tests/shared/src/test/resources/default/Lambda.stat @@ -167,7 +167,7 @@ opt[File]("migrate2hocon") Some(AbsoluteFile.fromFile(file, c.common.workingDirectory)))) .text("""migrate .scalafmt CLI style configuration to hocon style configuration in .scalafmt.conf""") }} ->>> { stateVisits = 890, stateVisits2 = 889 } +>>> { stateVisits = 895, stateVisits2 = 894 } { { opt[File]("migrate2hocon") diff --git a/scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala b/scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala index 977bf54b80..822b9fc980 100644 --- a/scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala +++ b/scalafmt-tests/shared/src/test/scala/org/scalafmt/FormatTests.scala @@ -137,7 +137,7 @@ class FormatTests extends FunSuite with CanRunTests with FormatAssertions { override def afterAll(): Unit = { logger.debug(s"Total explored: ${Debug.explored}") if (!onlyUnit && !onlyManual) - assertEquals(Debug.explored, 1776701, "total explored") + assertEquals(Debug.explored, 1775524, "total explored") val results = debugResults.result() // TODO(olafur) don't block printing out test results. // I don't want to deal with scalaz's Tasks :'(