Fix right associativity of extension methods

nicolasstucki · nicolasstucki · commit e598bef1b8d8 · 2023-12-06T09:28:16.000+01:00
This patch removes the special treatment of extension methods while desugaring. Instead of swapping the arguments of the extension method at definition site, we let the usual desugaring take care of it. ```scala extension (xs: List[Int]) def -:(x: Int) = xs.remove(x) 1 -: List(2, 3) // desugars into List(2, 3).-:(1) 1 :: List(2, 3) // desugars into List(2, 3).::(1) ``` This implies that the following makes sense now: ```scala List(2, 3).-:(1) // equivalent to the one bellow -:(List(2, 3))(1) ``` Note that the parameters are passed in order as in the signature, as they should. This used to break intuition in the previous implementation. Futhermore, this fixes parameter scoping and dependencies, as it trivially respects the original signature. An example of this is in the following code example, where `T` is bounded by `tuple.type`, which would have been out of scope in the previous implementation due to the swapping of parameters. ```scala extension (tuple: Tuple) def *:[T >: tuple.type <: Tuple, H](x: H): H *: T = ... ``` The downside is that this is a breaking change as we change the semantics of the extension method definitions. We need to find a way to migrate from one to the other without breaking code. One option might be to have a different syntax for the new semantics, and deprecate the old one. One possibility would be to use `infix def` extensions to mark to mark these methods. One issue encountered in the library is with the `IArray.{++:, +:}` operations. Luckily, it seems that by rewriting them into the new semantics we get a binary and TASTy compatible signature. Fixes #19197
diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
@@ -996,7 +996,7 @@ object desugar {
 
               def badRightAssoc(problem: String) =
                 report.error(em"right-associative extension method $problem", mdef.srcPos)
-                extParamss ++ mdef.paramss
+                () // extParamss ++ mdef.paramss
 
               rightParam match
                 case ValDefs(vparam :: Nil) =>
@@ -1010,15 +1010,16 @@ object desugar {
                     //   def %:[A](using B)[E](f: F)(c: C)(using D)(g: G)(using H): Res = ???
                     //
                     // If you change the names of the clauses below, also change them in right-associative-extension-methods.md
-                    val (leftTyParamsAndLeadingUsing, leftParamAndTrailingUsing) = extParamss.span(isUsingOrTypeParamClause)
-                    leftTyParamsAndLeadingUsing ::: rightTyParams ::: rightParam :: leftParamAndTrailingUsing ::: paramss1
+                    // val (leftTyParamsAndLeadingUsing, leftParamAndTrailingUsing) = extParamss.span(isUsingOrTypeParamClause)
+                    () // leftTyParamsAndLeadingUsing ::: rightTyParams ::: rightParam :: leftParamAndTrailingUsing ::: paramss1
                   else
                     badRightAssoc("cannot start with using clause")
                 case _ =>
                   badRightAssoc("must start with a single parameter")
             case _ =>
               // no value parameters, so not an infix operator.
-              extParamss ++ mdef.paramss
+              () // extParamss ++ mdef.paramss
+          extParamss ++ mdef.paramss
         else
           extParamss ++ mdef.paramss
     ).withMods(mdef.mods | ExtensionMethod)
diff --git a/compiler/src/dotty/tools/dotc/ast/Positioned.scala b/compiler/src/dotty/tools/dotc/ast/Positioned.scala
@@ -215,8 +215,8 @@ abstract class Positioned(implicit @constructorOnly src: SourceFile) extends Src
         check(tree.trailingParamss)
       case tree: DefDef if tree.mods.is(ExtensionMethod) =>
         tree.paramss match
-          case vparams1 :: vparams2 :: rest if tree.name.isRightAssocOperatorName =>
-            // omit check for right-associatiove extension methods; their parameters were swapped
+          // case vparams1 :: vparams2 :: rest if tree.name.isRightAssocOperatorName =>
+          //   // omit check for right-associatiove extension methods; their parameters were swapped
           case _ =>
             check(tree.paramss)
         check(tree.tpt)
diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
@@ -933,35 +933,35 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
         val coreSig =
           if isExtension then
             val paramss =
-              if tree.name.isRightAssocOperatorName then
-                // If you change the names of the clauses below, also change them in right-associative-extension-methods.md
-                // we have the following encoding of tree.paramss:
-                //   (leftTyParams ++ leadingUsing
-                //      ++ rightTyParams ++ rightParam
-                //      ++ leftParam ++ trailingUsing ++ rest)
-                //   e.g.
-                //     extension [A](using B)(c: C)(using D)
-                //       def %:[E](f: F)(g: G)(using H): Res = ???
-                //   will have the following values:
-                //   - leftTyParams = List(`[A]`)
-                //   - leadingUsing = List(`(using B)`)
-                //   - rightTyParams = List(`[E]`)
-                //   - rightParam = List(`(f: F)`)
-                //   - leftParam = List(`(c: C)`)
-                //   - trailingUsing = List(`(using D)`)
-                //   - rest = List(`(g: G)`, `(using H)`)
-                // we need to swap (rightTyParams ++ rightParam) with (leftParam ++ trailingUsing)
-                val (leftTyParams, rest1) = tree.paramss.span(isTypeParamClause)
-                val (leadingUsing, rest2) = rest1.span(isUsingClause)
-                val (rightTyParams, rest3) = rest2.span(isTypeParamClause)
-                val (rightParam, rest4) = rest3.splitAt(1)
-                val (leftParam, rest5) = rest4.splitAt(1)
-                val (trailingUsing, rest6) = rest5.span(isUsingClause)
-                if leftParam.nonEmpty then
-                  leftTyParams ::: leadingUsing ::: leftParam ::: trailingUsing ::: rightTyParams ::: rightParam ::: rest6
-                else
-                  tree.paramss // it wasn't a binary operator, after all.
-              else
+              // if tree.name.isRightAssocOperatorName then
+              //   // If you change the names of the clauses below, also change them in right-associative-extension-methods.md
+              //   // we have the following encoding of tree.paramss:
+              //   //   (leftTyParams ++ leadingUsing
+              //   //      ++ rightTyParams ++ rightParam
+              //   //      ++ leftParam ++ trailingUsing ++ rest)
+              //   //   e.g.
+              //   //     extension [A](using B)(c: C)(using D)
+              //   //       def %:[E](f: F)(g: G)(using H): Res = ???
+              //   //   will have the following values:
+              //   //   - leftTyParams = List(`[A]`)
+              //   //   - leadingUsing = List(`(using B)`)
+              //   //   - rightTyParams = List(`[E]`)
+              //   //   - rightParam = List(`(f: F)`)
+              //   //   - leftParam = List(`(c: C)`)
+              //   //   - trailingUsing = List(`(using D)`)
+              //   //   - rest = List(`(g: G)`, `(using H)`)
+              //   // we need to swap (rightTyParams ++ rightParam) with (leftParam ++ trailingUsing)
+              //   val (leftTyParams, rest1) = tree.paramss.span(isTypeParamClause)
+              //   val (leadingUsing, rest2) = rest1.span(isUsingClause)
+              //   val (rightTyParams, rest3) = rest2.span(isTypeParamClause)
+              //   val (rightParam, rest4) = rest3.splitAt(1)
+              //   val (leftParam, rest5) = rest4.splitAt(1)
+              //   val (trailingUsing, rest6) = rest5.span(isUsingClause)
+              //   if leftParam.nonEmpty then
+              //     leftTyParams ::: leadingUsing ::: leftParam ::: trailingUsing ::: rightTyParams ::: rightParam ::: rest6
+              //   else
+              //     tree.paramss // it wasn't a binary operator, after all.
+              // else
                 tree.paramss
             val trailingParamss = paramss
               .dropWhile(isUsingOrTypeParamClause)
diff --git a/library/src/scala/IArray.scala b/library/src/scala/IArray.scala
@@ -319,14 +319,10 @@ object IArray:
     def zipAll[T1 >: T, U](that: Iterable[U], thisElem: T1, thatElem: U): IArray[(T1, U)] = genericArrayOps(arr).zipAll(that, thisElem, thatElem)
     def zipWithIndex: IArray[(T, Int)] = genericArrayOps(arr).zipWithIndex
 
-  extension [T, U >: T: ClassTag](prefix: IterableOnce[T])
-    def ++:(arr: IArray[U]): IArray[U] = genericArrayOps(arr).prependedAll(prefix)
-
-  extension [T, U >: T: ClassTag](prefix: IArray[T])
-    def ++:(arr: IArray[U]): IArray[U] = genericArrayOps(arr).prependedAll(prefix)
-
-  extension [T, U >: T: ClassTag](x: T)
-    def +:(arr: IArray[U]): IArray[U] = genericArrayOps(arr).prepended(x)
+  extension [T, U >: T: ClassTag](arr: IArray[U])
+    def ++:(prefix: IterableOnce[T]): IArray[U] = genericArrayOps(arr).prependedAll(prefix)
+    def ++:(prefix: IArray[T]): IArray[U] = genericArrayOps(arr).prependedAll(prefix)
+    def +:(x: T): IArray[U] = genericArrayOps(arr).prepended(x)
 
   // For backwards compatibility with code compiled without -Yexplicit-nulls
   private inline def mapNull[A, B](a: A, inline f: B): B =
diff --git a/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverTypeSuite.scala b/presentation-compiler/test/dotty/tools/pc/tests/hover/HoverTypeSuite.scala
@@ -152,8 +152,8 @@ class HoverTypeSuite extends BaseHoverSuite:
          |class C
          |
          |object Foo:
-         |    extension [T](using A)(main: T)(using B)
-         |      def %:[R](res: R)(using C): R = ???
+         |    extension [R](using A)(res: R)(using B)
+         |      def %:[T](main: T)(using C): R = ???
          |    given A with {}
          |    given B with {}
          |    given C with {}
@@ -162,7 +162,7 @@ class HoverTypeSuite extends BaseHoverSuite:
          |end Foo
          |""".stripMargin,
       """|Int
-         |extension [T](using A)(main: T) def %:[R](res: R)(using B)(using C): R""".stripMargin.hover
+         |extension [R](using A)(using B)(res: R) def %:[T](main: T)(using C): R""".stripMargin.hover
     )
 
   @Test def `using` =
diff --git a/tests/neg-custom-args/captures/lazylists-exceptions.scala b/tests/neg-custom-args/captures/lazylists-exceptions.scala
@@ -20,8 +20,8 @@ final class LazyCons[+T](val x: T, val xs: () => LazyList[T]^) extends LazyList[
   def tail: LazyList[T]^{this} = xs()
 end LazyCons
 
-extension [A](x: A)
-  def #:(xs1: => LazyList[A]^): LazyList[A]^{xs1} =
+extension [A](xs1: => LazyList[A]^)
+  def #:(x: A): LazyList[A]^{xs1} =
     LazyCons(x, () => xs1)
 
 def tabulate[A](n: Int)(gen: Int => A): LazyList[A]^{gen} =
diff --git a/tests/neg/i13075.scala b/tests/neg/i13075.scala
@@ -8,8 +8,8 @@ object Implementing_Tuples:
   type *:[H, T <: Tup] = ConsTup[H, T] // for type matching
   type EmptyTup = EmptyTup.type        // for type matching
 
-  extension [H](head: H)
-    def *:[T <: Tup](tail: T) = ConsTup(head, tail)
+  extension [T <: Tup](tail: T)
+    def *:[H](head: H) = ConsTup(head, tail)
 
   type Fold[T <: Tup, Seed, F[_,_]] = T match
     case EmptyTup => Seed
diff --git a/tests/neg/i9562.scala b/tests/neg/i9562.scala
@@ -9,4 +9,5 @@ object Unrelated:
     def h1: Int = foo     // error
     def h2: Int = h1 + 1  // OK
     def h3: Int = g       // error
-    def ++: (x: Int): Int = h1 + x  // OK
+  extension (x: Int)
+    def ++:(f: Foo): Int = f.h1 + x  // OK
diff --git a/tests/pos-custom-args/captures/lazylists-exceptions.scala b/tests/pos-custom-args/captures/lazylists-exceptions.scala
@@ -42,8 +42,8 @@ extension [A](xs: LzyList[A]^)
     if n == 0 then xs else xs.tail.drop(n - 1)
 end extension
 
-extension [A](x: A)
-  def #:(xs1: => LzyList[A]^): LzyList[A]^{xs1} =
+extension [A](xs1: => LzyList[A]^)
+  def #:(x: A): LzyList[A]^{xs1} =
     LzyCons(x, () => xs1)
 
 def lazyCons[A](x: A, xs1: => LzyList[A]^): LzyList[A]^{xs1} =
diff --git a/tests/pos-custom-args/captures/logger.scala b/tests/pos-custom-args/captures/logger.scala
@@ -32,8 +32,8 @@ final class LazyCons[+T](val x: T, val xs: () => LazyList[T]^) extends LazyList[
   def tail: LazyList[T]^{this} = xs()
 end LazyCons
 
-extension [A](x: A)
-  def #::(xs1: => LazyList[A]^): LazyList[A]^{xs1} =
+extension [A](xs1: => LazyList[A]^)
+  def #::(x: A): LazyList[A]^{xs1} =
     LazyCons(x, () => xs1)
 
 extension [A](xs: LazyList[A]^)
diff --git a/tests/pos-custom-args/captures/strictlists.scala b/tests/pos-custom-args/captures/strictlists.scala
@@ -28,11 +28,10 @@ extension [A](xs: StrictList[A])
   def concat(ys: StrictList[A]): StrictList[A] =
     if xs.isEmpty then ys
     else xs.head #: xs.tail.concat(ys)
-end extension
 
-extension [A](x: A)
-  def #:(xs1: StrictList[A]): StrictList[A] =
-    StrictCons(x, xs1)
+  def #:(x: A): StrictList[A] =
+    StrictCons(x, xs)
+end extension
 
 def tabulate[A](n: Int)(gen: Int => A) =
   def recur(i: Int): StrictList[A] =
diff --git a/tests/pos/IArrayToCons.scala b/tests/pos/IArrayToCons.scala
@@ -0,0 +1,13 @@
+import IArray.{+:, ++:}
+def test(arr: IArray[Int]): Unit =
+  1 +: arr;
+  arr.+:(1);
+  +:(arr)(1);
+
+  arr ++: arr;
+  arr.++:(arr);
+  ++:(arr)(arr);
+
+  Nil ++: arr;
+  arr.++:(Nil);
+  ++:(arr)(Nil);
diff --git a/tests/pos/i19197.scala b/tests/pos/i19197.scala
@@ -0,0 +1,6 @@
+extension (tuple: Tuple)
+  def **:[T >: tuple.type <: Tuple, H](x: H): H *: T = ???
+
+def test1: (Int, String, Char) = 1 **: ("a", 'b')
+def test2: (Int, String, Char) = ("a", 'b').**:(1)
+def test3: (Int, String, Char) = **:("a", 'b')(1)
diff --git a/tests/pos/i9562.scala b/tests/pos/i9562.scala
@@ -8,4 +8,6 @@ object Unrelated:
   extension (f: Foo)
     def h1: Int = 0
     def h2: Int = h1 + 1  // OK
-    def ++: (x: Int): Int = h2 + x  // OK
+
+  extension (x: Int)
+    def ++: (f: Foo): Int = f.h2 + x  // OK
diff --git a/tests/pos/reference/extension-methods.scala b/tests/pos/reference/extension-methods.scala
@@ -10,7 +10,7 @@ object ExtMethods:
   assert(circle.circumference == circumference(circle))
 
   extension (x: String) def < (y: String) = x.compareTo(y) < 0
-  extension [Elem](x: Elem) def #: (xs: Seq[Elem]) = x +: xs
+  extension [Elem](xs: Seq[Elem]) def #: (x: Elem) = x +: xs
   extension (x: Number) infix def min (y: Number) = x
 
   assert("a" < "bb")
diff --git a/tests/run/errorhandling/Result.scala b/tests/run/errorhandling/Result.scala
@@ -35,12 +35,15 @@ object Result:
       case (Err(e), Ok(_))    => Err(e :: Nil)
       case (Err(e1), Err(e2)) => Err(e1 :: e2 :: Nil)
 
+  end extension
+
+  extension [U <: Tuple, E](other: Result[U, List[E]])
     /** Validate both `r` and `other`; return a tuple of successes or a list of failures.
      *  Unlike with `zip`, the right hand side `other` must be a `Result` returning a `Tuple`,
      *  and the left hand side is added to it. See `Result.empty` for a convenient
      *  right unit of chains of `*:`s.
      */
-    def *: [U <: Tuple](other: Result[U, List[E]]): Result[T *: U, List[E]] = (r, other) match
+    def *: [T](r: Result[T, E]): Result[T *: U, List[E]] = (r, other) match
       case (Ok(x), Ok(ys))     => Ok(x *: ys)
       case (Ok(_), es: Err[?]) => es
       case (Err(e), Ok(_))     => Err(e :: Nil)
diff --git a/tests/run/export-in-extension.scala b/tests/run/export-in-extension.scala
@@ -11,7 +11,7 @@ object O:
     export cm.*
     def succ: Int = x + 1
     def succ2: Int = succ + 1
-    def ::: (y: Int) = x - y
+    def ::: (y: Int) = y - x
 
 object O2:
   import O.C
@@ -20,7 +20,7 @@ object O2:
     export cm.{bar, baz, bam, ::}
     def succ: Int = x + 1
     def succ2: Int = succ + 1
-    def ::: (y: Int) = x - y
+    def ::: (y: Int) = y - x
 
 @main def Test =
   import O.*
diff --git a/tests/run/i11583.scala b/tests/run/i11583.scala
@@ -12,11 +12,11 @@ class Env:
 //     */
 //   def &&:[T <: ctx.Term](trm: T)(ext: env.Extra): (ctx.Type, T, env.Extra) = (tpe, trm, ext)
 
-extension [Ctx <: Context](using ctx: Ctx)(tpe: String)(using env: Env)
-  def :#:[T <: Boolean](trm: T)(ext: env.Extra): (String, T, env.Extra) = (tpe, trm, ext)
+extension [Ctx <: Context, T <: Boolean](using ctx: Ctx)(trm: T)(using env: Env)
+  def :#:(tpe: String)(ext: env.Extra): (String, T, env.Extra) = (tpe, trm, ext)
 
-extension [A](a: A)
-  def :*:[T <: Tuple](t: T): A *: T = a *: t
+extension [T <: Tuple](t: T)
+  def :*:[A](a: A): A *: T = a *: t
 
 @main def Test =
 
diff --git a/tests/run/i9530.scala b/tests/run/i9530.scala
@@ -8,7 +8,7 @@ trait Scope:
 extension (using s: Scope)(expr: s.Expr)
    def show = expr.toString
    def eval = s.value(expr)
-   def *: (other: s.Expr) = s.combine(expr, other)
+   def *: (other: s.Expr) = s.combine(other, expr)
 
 def f(using s: Scope)(x: s.Expr): (String, s.Value) =
    (x.show, x.eval)
diff --git a/tests/run/instances.scala b/tests/run/instances.scala
@@ -31,7 +31,7 @@ object Test extends App {
   extension [T](xs: List[List[T]])
     def flattened = xs.foldLeft[List[T]](Nil)(_ ++ _)
 
-  extension [T](x: T) def :: (xs: Seq[T]) = x +: xs
+  extension [T](xs: Seq[T]) def :: (x: T) = x +: xs
 
   val ss: Seq[Int] = List(1, 2, 3)
   val ss1 = 0 :: ss
diff --git a/tests/run/interleaving.scala b/tests/run/interleaving.scala
diff --git a/tests/semanticdb/expect/RightAssociativeExtension.scala b/tests/semanticdb/expect/RightAssociativeExtension.scala
diff --git a/tests/semanticdb/metac.expect b/tests/semanticdb/metac.expect
