From 6a2e515383a91a012cdfc5df6d262144ccb7ecb5 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 Earlier, we would only 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. --- .../scala/org/scalafmt/util/TreeOps.scala | 7 ++-- .../src/test/resources/default/Class.stat | 15 ++++++--- .../test/resources/scala3/OptionalBraces.stat | 33 ++++++++++++------- .../resources/scala3/OptionalBraces_fold.stat | 33 ++++++++++++------- .../resources/scala3/OptionalBraces_keep.stat | 33 ++++++++++++------- .../scala3/OptionalBraces_unfold.stat | 33 ++++++++++++------- .../vertical-multiline/verticalMultiline.stat | 3 +- 7 files changed, 103 insertions(+), 54 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 aea43de542..53b5a03d83 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 @@ -638,10 +638,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 a2a87aa961..cb68616f46 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 ) {} @@ -3995,7 +3996,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 ) {} @@ -4024,14 +4026,16 @@ object a: c: C ): B = ??? 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 ) {} @@ -4098,14 +4102,16 @@ object a: c: C ): B = ??? 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 ) {} @@ -4148,7 +4154,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 @@ -4234,7 +4241,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] @@ -4423,7 +4431,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit implicit c: C + implicit + implicit c: C ) {} class F( a: A, @@ -4436,7 +4445,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4487,6 +4497,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 60d746a4de..f078773c90 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 ) {} @@ -3806,7 +3807,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 ) {} @@ -3835,14 +3837,16 @@ object a: c: C ): B = ??? 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 ) {} @@ -3909,14 +3913,16 @@ object a: c: C ): B = ??? 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 ) {} @@ -3959,7 +3965,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 @@ -4045,7 +4052,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] @@ -4234,7 +4242,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit implicit c: C + implicit + implicit c: C ) {} class F( a: A, @@ -4247,7 +4256,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4298,6 +4308,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 823f39c737..920a792f47 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 ) {} @@ -4034,7 +4035,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 ) {} @@ -4063,14 +4065,16 @@ object a: c: C ): B = ??? 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 ) {} @@ -4137,14 +4141,16 @@ object a: c: C ): B = ??? 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 ) {} @@ -4187,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, implicitParamListModifierPrefer = before @@ -4273,7 +4280,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] @@ -4465,7 +4473,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit implicit c: C + implicit + implicit c: C ) {} class F( a: A, @@ -4478,7 +4487,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4529,6 +4539,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 b26aa3d177..0df55f0c78 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 ) {} @@ -4126,7 +4127,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 ) {} @@ -4155,14 +4157,16 @@ object a: c: C ): B = ??? 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 ) {} @@ -4229,14 +4233,16 @@ object a: c: C ): B = ??? 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 ) {} @@ -4279,7 +4285,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 @@ -4365,7 +4372,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] @@ -4554,7 +4562,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit implicit c: C + implicit + implicit c: C ) {} class F( a: A, @@ -4567,7 +4576,8 @@ object a: a: A, as: A* )(b: B, bs: B*)( - implicit c: C, + implicit + c: C, d: D, implicit e: E ) {} @@ -4618,6 +4628,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) {} }