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.

Sign up for GitHub

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

Jump to bottom

Route: hide NL before dot for keep via a splittag #4351

Merged
merged 2 commits into from
Sep 27, 2024
Merged

Route: hide NL before dot for keep via a splittag #4351

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
53cedc0
FormatOps: refactor BinPackOneline policy method
kitbellew Sep 27, 2024
923c626
Route: hide NL before dot for keep via a splittag
kitbellew Sep 27, 2024
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
43 changes: 27 additions & 16 deletions scalafmt-core/shared/src/main/scala/org/scalafmt/internal/FormatOps.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2928,10 +2928,7 @@ class FormatOps(
iter(afterDelims)
}

def policyBefore(token: T, mayBreakBeforeToken: Boolean) = {
if (mayBreakBeforeToken) Some(decideNewlinesOnlyBeforeToken(token))
else closeBreakPolicy()
}.fold(Policy.noPolicy) { policy =>
def policyWithDelay(policy: Policy) = {
val beforeDelimsEnd = beforeDelims.right.end
// force break if multiline and if there's no other break
delayedBreakPolicy(Policy.End == beforeDelimsEnd, exclude)(
Expand All @@ -2941,30 +2938,44 @@ class FormatOps(
)
}

afterDelims.right match {
(afterDelims.right match {
case c: T.Dot => // check if Dot rule includes a break option
val ro = afterDelims.meta.rightOwner
val ok = GetSelectLike.onRightOpt(ro, afterDelims).exists { x =>
c -> GetSelectLike.onRightOpt(afterDelims).map { x =>
implicit val cfg = styleMap.at(afterDelims)
(cfg.newlines.getSelectChains ne Newlines.classic) || {
val (expireTree, nextSelect) =
findLastApplyAndNextSelect(ro, cfg.encloseSelectChains)
canStartSelectChain(x, nextSelect, expireTree)
cfg.newlines.getSelectChains match {
case Newlines.classic =>
val (expireTree, nextSelect) =
findLastApplyAndNextSelect(x.tree, cfg.encloseSelectChains)
Right(canStartSelectChain(x, nextSelect, expireTree))
case Newlines.keep => Left(Policy.on(c, "BP1L.NL") {
case Decision(`afterDelims`, ss) => Decision
.onlyNewlineSplits(ss)
.map(_.preActivateFor(SplitTag.SelectChainBinPackNL))
})
case _ => Right(true)
}
}
(Some(c), policyBefore(c, ok))
}.getOrElse(Right(false))
case x @ LeftParenOrBracket() =>
nextNonCommentSameLineAfter(afterDelims).right match {
case _: T.Comment => (None, NoPolicy)
case _: T.Comment => null
case c => // check if break would cause cfg style but not otherwise
val cfg = styleMap.at(x)
val ok = cfg.newlines.sourceIgnored || ! {
cfg.configStyleCallSite.prefer &&
cfg.danglingParentheses.atCallSite(afterDelims.meta.rightOwner)
} || next(afterDelims).hasBreak
(Some(c), policyBefore(c, ok))
c -> Right(ok)
}
case _ => null
}) match {
case null => (None, NoPolicy)
case (c, policyOrOkToBreak) =>
val policyOpt = policyOrOkToBreak match {
case Left(policy) => Some(policy)
case Right(okToBreak) if !okToBreak => closeBreakPolicy()
case _ => Some(decideNewlinesOnlyBeforeToken(c))
}
case _ => (None, NoPolicy)
(Some(c), policyOpt.fold(Policy.noPolicy)(policyWithDelay))
}
}

Expand Down
5 changes: 2 additions & 3 deletions scalafmt-core/shared/src/main/scala/org/scalafmt/internal/Router.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1780,9 +1780,8 @@ class Router(formatOps: FormatOps) {
ignore = next(ft).right.is[T.Comment],
))
else Seq(
Split(NoSplit, 0)
.withSingleLine(getSlbEnd(), killOnFail = shouldKillOnFail()),
Split(Newline, Constants.ExceedColumnPenalty * 3),
Split(NoSplit, 0),
Split(Newline, 1).onlyFor(SplitTag.SelectChainBinPackNL),
)

case Newlines.unfold =>
Expand Down
1 change: 1 addition & 0 deletions scalafmt-core/shared/src/main/scala/org/scalafmt/internal/SplitTag.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ object SplitTag {
case object OneArgPerLine extends SplitTag
case object SelectChainFirstNL extends SplitTag
case object SelectChainSecondNL extends SplitTag
case object SelectChainBinPackNL extends SplitTag
case object InfixChainNoNL extends SplitTag
case object OnelineWithChain extends SplitTag
case object VerticalMultilineSingleLine extends SplitTag
Expand Down
9 changes: 5 additions & 4 deletions scalafmt-tests/shared/src/test/resources/newlines/source_keep.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -945,8 +945,9 @@ object a {
}
>>>
object a {
(intercept[
java.lang.IllegalStateException] { in.readObject }).getMessage should ===(
(intercept[java.lang.IllegalStateException] {
in.readObject
}).getMessage should ===(
"Trying to deserialize a serialized ActorRef without an ActorSystem in scope." +
" Use 'akka.serialization.Serialization.currentSystem.withValue(system) { ... }'")
}
Expand Down Expand Up @@ -8957,8 +8958,8 @@ object a {
>>>
object a {
if (foo) bar match {
case nme.IMPORT | nme.OUTER | nme.ANON_CLASS_NAME | nme
.ANON_FUN_NAME | nme.CONSTRUCTOR => ()
case nme.IMPORT | nme.OUTER | nme.ANON_CLASS_NAME | nme.ANON_FUN_NAME | nme.CONSTRUCTOR =>
()
}
}
<<< #4133 control body with complex apply/select/try 1
Expand Down
74 changes: 31 additions & 43 deletions scalafmt-tests/shared/src/test/resources/scala3/OptionalBraces_keep.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6700,8 +6700,7 @@ object a:
errorButContinue(
em"""The start of this line does not match any of the previous indentation widths.
|Indentation width of current line : $nextWidth
|This falls between previous widths: ${ir
.width} and $lw"""
|This falls between previous widths: ${ir.width} and $lw"""
))
<<< scala.js tucked close paren after optional braces region, !dangling
binPack.preset = always
Expand Down Expand Up @@ -6742,8 +6741,7 @@ object a:
errorButContinue(
em"""The start of this line does not match any of the previous indentation widths.
|Indentation width of current line : $nextWidth
|This falls between previous widths: ${ir
.width} and $lw"""))
|This falls between previous widths: ${ir.width} and $lw"""))
<<< scala.js overflow within for-yield dangling
binPack.preset = always
danglingParentheses.preset = true
Expand All @@ -6757,10 +6755,8 @@ object a:
def foo =
for template <- templates do
val current_Map_String__Object =
template_templateFile_settings_getOrElse(
"page",
Map.empty
).asInstanceOf[Map_String_Object]
template_templateFile_settings_getOrElse("page",
Map.empty).asInstanceOf[Map_String_Object]
<<< scala.js overflow within for-yield !dangling
binPack.preset = always
danglingParentheses.preset = false
Expand Down Expand Up @@ -7047,8 +7043,9 @@ object a {
>>>
object a {
@threadUnsafe lazy val AnnotationRetentionSourceAttr: TermSymbol =
requiredClass("java.lang.annotation.RetentionPolicy").linkedClass
.requiredValue("SOURCE")
requiredClass(
"java.lang.annotation.RetentionPolicy").linkedClass.requiredValue(
"SOURCE")
}
<<< #4133 overflow apply with binpack, dangling
maxColumn = 70
Expand Down Expand Up @@ -7079,15 +7076,9 @@ object a:
object a:
if (args.isEmpty) {
run(insertClasspathInArgs(args1,
List(
dottyCompiler,
dottyInterfaces,
asm,
dottyStaging,
dottyTastyInspector,
tastyCore,
compilerInterface
).mkString(File.pathSeparator)))
List(dottyCompiler, dottyInterfaces, asm, dottyStaging,
dottyTastyInspector, tastyCore,
compilerInterface).mkString(File.pathSeparator)))
} else run(args)
<<< #4133 overflow apply/selects with binpack, dangling
maxColumn = 70
Expand All @@ -7103,15 +7094,11 @@ object a:
if (args.isEmpty) {
run(insertClasspathInArgs(
args1,
List(
dottyCompiler,
dottyInterfaces,
asm,
dottyStaging,
dottyTastyInspector,
tastyCore,
compilerInterface
).mkString(File.pathSeparator).mkString(File.pathSeparator)
List(dottyCompiler, dottyInterfaces, asm, dottyStaging,
dottyTastyInspector, tastyCore,
compilerInterface).mkString(File.pathSeparator).mkString(
File.pathSeparator
)
))
} else run(args)
<<< #4133 overflow apply with binpack, !dangling
Expand Down Expand Up @@ -7144,8 +7131,8 @@ object a:
if (args.isEmpty) {
run(insertClasspathInArgs(args1,
List(dottyCompiler, dottyInterfaces, asm, dottyStaging,
dottyTastyInspector, tastyCore, compilerInterface).mkString(
File.pathSeparator)))
dottyTastyInspector, tastyCore,
compilerInterface).mkString(File.pathSeparator)))
} else run(args)
<<< #4133 overflow apply/selects with binpack, !dangling
maxColumn = 70
Expand All @@ -7163,8 +7150,9 @@ object a:
insertClasspathInArgs(
args1,
List(dottyCompiler, dottyInterfaces, asm, dottyStaging,
dottyTastyInspector, tastyCore, compilerInterface)
.mkString(File.pathSeparator).mkString(File.pathSeparator)
dottyTastyInspector, tastyCore,
compilerInterface).mkString(File.pathSeparator).mkString(
File.pathSeparator)
))
} else run(args)
<<< #4133 overflow select with binpack, dangling
Expand All @@ -7178,8 +7166,9 @@ object a:
>>>
object a:
val foo = map { postFile =>
val destPath = ctx_docsPath_resolve_defaultDirectory_resolve(year)
.resolve(month).resolve(day).resolve(name)
val destPath = ctx_docsPath_resolve_defaultDirectory_resolve(
year
).resolve(month).resolve(day).resolve(name)
}
<<< #4133 overflow select with binpack, !dangling
binPack.preset = always
Expand All @@ -7192,8 +7181,8 @@ object a:
>>>
object a:
val foo = map { postFile =>
val destPath = ctx_docsPath_resolve_defaultDirectory_resolve(year)
.resolve(month).resolve(day).resolve(name)
val destPath = ctx_docsPath_resolve_defaultDirectory_resolve(
year).resolve(month).resolve(day).resolve(name)
}
<<< #4133 overflow selects/applies, nobreak-dot-break, binpack + !dangling
maxColumn = 70
Expand Down Expand Up @@ -7228,13 +7217,12 @@ object a:
)
>>>
object a:
lazy val `stdlib-bootstrapped` =
project.in(file("stdlib-bootstrapped")).withCommonSettings(
Bootstrapped).dependsOn(dottyCompiler(
Bootstrapped) % "provided; compile->runtime; test->test")
.settings(
moduleName := "scala-library"
)
lazy val `stdlib-bootstrapped` = project.in(
file("stdlib-bootstrapped")).withCommonSettings(
Bootstrapped).dependsOn(dottyCompiler(
Bootstrapped) % "provided; compile->runtime; test->test").settings(
moduleName := "scala-library"
)
<<< #4133 overflow selects/applies, break-dot-nobreak, binpack + !dangling
maxColumn = 70
binPack.preset = always
Expand Down
10 changes: 5 additions & 5 deletions scalafmt-tests/shared/src/test/resources/scalajs/Advanced.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ newlines.source = keep
===
debuglog("infer method inst " + fun + ", tparams = " + tparams + ", args = " + args1.map(_.tpe) + ", pt = " + pt + ", lobounds = " + tparams.map(_.tpe.lowerBound) + ", parambounds = " + tparams.map(_.info)) //debug
>>>
debuglog("infer method inst " + fun + ", tparams = " + tparams + ", args = " + args1
.map(_.tpe) + ", pt = " + pt + ", lobounds = " + tparams
.map(_.tpe.lowerBound) + ", parambounds = " + tparams.map(
debuglog("infer method inst " + fun + ", tparams = " + tparams + ", args = " + args1.map(
_.tpe) + ", pt = " + pt + ", lobounds = " + tparams.map(
_.tpe.lowerBound) + ", parambounds = " + tparams.map(
_.info)) //debug
<<< overflow infix with applies
maxColumn = 74
Expand All @@ -38,5 +38,5 @@ newlines.source = keep
realeasy && isJavaAtLeast(9) && !perFile.exists(releaseFlag.matches) && toolArgsFor(sources)(ToolName.javaVersion).isEmpty
>>>
realeasy && isJavaAtLeast(9) && !perFile.exists(
releaseFlag.matches) && toolArgsFor(sources)(ToolName.javaVersion)
.isEmpty
releaseFlag.matches) && toolArgsFor(sources)(
ToolName.javaVersion).isEmpty
31 changes: 15 additions & 16 deletions scalafmt-tests/shared/src/test/resources/scalajs/DefDef.stat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -820,9 +820,9 @@ val memberSel: Elems =
Ol(
elems = {
if (
!universe.settings.docGroups.value || tpl
.members.map(_.group).distinct.forall(
_ == ModelFactory.defaultGroup)
!universe.settings.docGroups.value || tpl.members.map(
_.group).distinct.forall(
_ == ModelFactory.defaultGroup)
)
NoElems
else
Expand All @@ -834,8 +834,7 @@ val memberSel: Elems =
elems = Span(
elems = Txt("Alphabetic"))) :: {
if (
tpl.linearizationTemplates
.isEmpty && tpl.conversions.isEmpty
tpl.linearizationTemplates.isEmpty && tpl.conversions.isEmpty
)
NoElems
else
Expand All @@ -847,8 +846,7 @@ val memberSel: Elems =
)
) ++ (
if (
tpl.linearizationTemplates.isEmpty && tpl
.conversions.isEmpty
tpl.linearizationTemplates.isEmpty && tpl.conversions.isEmpty
) NoElems
else {
(if (tpl.linearizationTemplates.isEmpty) NoElems
Expand Down Expand Up @@ -886,13 +884,15 @@ val memberSel: Elems =
tpl.conversions.map { conv =>
val name =
conv.conversionQualifiedName
val hide = universe.settings
.hiddenImplicits(name)
val hide =
universe.settings.hiddenImplicits(
name)
Li(`class` = "in", name = name,
`data-hidden` = hide.toString,
`data-hidden` =
hide.toString,
elems = Span(
elems = Txt("by " + conv
.conversionShortName)))
elems = Txt(
"by " + conv.conversionShortName)))
}
}
)
Expand Down Expand Up @@ -930,10 +930,9 @@ val memberSel: Elems =
elems = Txt("Protected")))
) ++ List(Li(`class` = "private out",
elems = Span(
elems = Txt("Private"))))
.filter(_ =>
universe.settings.visibilityPrivate
.value))
elems = Txt(
"Private")))).filter(_ =>
universe.settings.visibilityPrivate.value))
))
)
))
Expand Down
Loading