From 4c28424d5ab91f4e202e80f4c8baeb3c554e6046 Mon Sep 17 00:00:00 2001 From: Albert Meltzer <7529386+kitbellew@users.noreply.github.com> Date: Fri, 17 Feb 2023 13:07:22 -0800 Subject: [PATCH] TreeOps: fix handling implicit group keyword Previously, we would handle the first `implicit` differently if none of the params have an explicit `implicit` keyword. In reality, we should relax the check and do this if not all params have an explicit keyword. The idea being, if they all do, then the initial one belongs to the first parameter rather than the group. --- .../main/scala/org/scalafmt/util/TreeOps.scala | 7 +++---- .../src/test/resources/default/Class.stat | 15 ++++++++++----- .../test/resources/scala3/OptionalBraces.stat | 18 ++++++++++++------ .../resources/scala3/OptionalBraces_fold.stat | 18 ++++++++++++------ .../resources/scala3/OptionalBraces_keep.stat | 18 ++++++++++++------ .../scala3/OptionalBraces_unfold.stat | 18 ++++++++++++------ .../vertical-multiline/verticalMultiline.stat | 3 ++- 7 files changed, 63 insertions(+), 34 deletions(-) diff --git a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala index 8da8143210..69ea3bc9fe 100644 --- a/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala +++ b/scalafmt-core/shared/src/main/scala/org/scalafmt/util/TreeOps.scala @@ -639,10 +639,9 @@ object TreeOps { def getImplicitParamList(kwOwner: Tree): Option[Seq[Tree]] = kwOwner.parent match { case Some(Term.ArgClause(v, Some(`kwOwner`))) => Some(v) - case Some(Term.ParamClause(v, Some(`kwOwner`))) if (kwOwner match { - case _: Mod.Implicit => v.forall(noExplicitImplicit) - case _ => true - }) => + case Some(Term.ParamClause(v @ head :: rest, Some(`kwOwner`))) + if !kwOwner.is[Mod.Implicit] || rest.isEmpty || + !noExplicitImplicit(head) || rest.exists(noExplicitImplicit) => Some(v) case _ => None } diff --git a/scalafmt-tests/src/test/resources/default/Class.stat b/scalafmt-tests/src/test/resources/default/Class.stat index 86c67abee8..11c49e0aad 100644 --- a/scalafmt-tests/src/test/resources/default/Class.stat +++ b/scalafmt-tests/src/test/resources/default/Class.stat @@ -605,7 +605,8 @@ object a { implicit c: C, implicit d: D) {} class F(a: A, as: A*)(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E) {} } @@ -621,12 +622,14 @@ object a { >>> object a { class F(a: A, as: A*)(b: B, bs: B*)( - implicit implicit c: C) {} + implicit + implicit c: C) {} class F(a: A, as: A*)(b: B, bs: B*)( implicit c: C, implicit d: D) {} class F(a: A, as: A*)(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E) {} } @@ -663,12 +666,14 @@ object a { >>> object a { class F(a: A, as: A*)(b: B, bs: B*)( - implicit implicit c: C) {} + implicit + implicit c: C) {} class F(a: A, as: A*)(b: B, bs: B*)( implicit c: C, implicit d: D) {} class F(a: A, as: A*)(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E) {} } diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat index e02265c4ab..2b73be1dbc 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces.stat @@ -3931,7 +3931,8 @@ object a: implicit d: D ) {} class F(a: A, as: A*)(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -3974,7 +3975,8 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} <<< interleaved, vertical multiline, implicitParamListModifierPrefer = before @@ -4060,7 +4062,8 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} <<< interleaved, vertical multiline, implicitParamListModifierForce = [after] @@ -4249,7 +4252,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit implicit c: C + implicit + implicit c: C ) {} class F( a: A, @@ -4262,7 +4266,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4313,6 +4318,7 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat index db838cb630..2f21dd6a3a 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_fold.stat @@ -3742,7 +3742,8 @@ object a: implicit d: D ) {} class F(a: A, as: A*)(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -3785,7 +3786,8 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} <<< interleaved, vertical multiline, implicitParamListModifierPrefer = before @@ -3871,7 +3873,8 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} <<< interleaved, vertical multiline, implicitParamListModifierForce = [after] @@ -4060,7 +4063,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit implicit c: C + implicit + implicit c: C ) {} class F( a: A, @@ -4073,7 +4077,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4124,6 +4129,7 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat index 01314c9135..411de62854 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_keep.stat @@ -3970,7 +3970,8 @@ object a: implicit d: D ) {} class F(a: A, as: A*)(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4013,7 +4014,8 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} <<< interleaved, vertical multiline, implicitParamListModifierPrefer = before @@ -4099,7 +4101,8 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} <<< interleaved, vertical multiline, implicitParamListModifierForce = [after] @@ -4291,7 +4294,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit implicit c: C + implicit + implicit c: C ) {} class F( a: A, @@ -4304,7 +4308,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4355,6 +4360,7 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} diff --git a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat index 88c6ab299f..f9c78ca762 100644 --- a/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat +++ b/scalafmt-tests/src/test/resources/scala3/OptionalBraces_unfold.stat @@ -4062,7 +4062,8 @@ object a: implicit d: D ) {} class F(a: A, as: A*)(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4105,7 +4106,8 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} <<< interleaved, vertical multiline, implicitParamListModifierPrefer = before @@ -4191,7 +4193,8 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} <<< interleaved, vertical multiline, implicitParamListModifierForce = [after] @@ -4380,7 +4383,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit implicit c: C + implicit + implicit c: C ) {} class F( a: A, @@ -4393,7 +4397,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4444,6 +4449,7 @@ object a: as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} diff --git a/scalafmt-tests/src/test/resources/vertical-multiline/verticalMultiline.stat b/scalafmt-tests/src/test/resources/vertical-multiline/verticalMultiline.stat index a1f388ab97..3b70b5a720 100644 --- a/scalafmt-tests/src/test/resources/vertical-multiline/verticalMultiline.stat +++ b/scalafmt-tests/src/test/resources/vertical-multiline/verticalMultiline.stat @@ -363,7 +363,8 @@ object a { as: A* )(b: B, bs: B* - )(implicit c: C, + )(implicit + c: C, d: D, implicit e: E) {} }