From d718ccd36c4294893ca1a718b8ef0eef49f9d2bc Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Fri, 14 Feb 2020 16:51:47 +0100
Subject: [PATCH 01/11] Docs for extension instances

---
 .../reference/contextual/extension-methods.md | 114 ++++++++++++++----
 1 file changed, 91 insertions(+), 23 deletions(-)

diff --git a/docs/docs/reference/contextual/extension-methods.md b/docs/docs/reference/contextual/extension-methods.md
index a2035f111fe7..28f4ab0a8b02 100644
--- a/docs/docs/reference/contextual/extension-methods.md
+++ b/docs/docs/reference/contextual/extension-methods.md
@@ -124,59 +124,127 @@ If an extension method has type parameters, they come immediately after the `def
 ```scala
 List(1, 2, 3).second[Int]
 ```
-### Collective Extensions
 
-A collective extension defines one or more concrete methods that have the same type parameters
-and prefix parameter. Examples:
+### Extension Instances
 
+It is quite common to wrap one or more extension methods in a given instance,
+in order to make them available as methods without needing to be imported explicitly.
+This pattern is supported by a special `extension` syntax. Example:
 ```scala
-extension stringOps on (xs: Seq[String]) {
-  def longestStrings: Seq[String] = {
+extension ops {
+  def (xs: Seq[String]).longestStrings: Seq[String] = {
     val maxLength = xs.map(_.length).max
     xs.filter(_.length == maxLength)
   }
+  def (xs: Seq[String]).longestString: String = xs.longestStrings.head
+  def [T](xs: List[T]).second: T = xs.tail.head
+}
+```
+An extension instance can only contain extension methods. Other definitions are not allowed. The name `ops`
+of the extension is optional. It can be left out:
+```scala
+extension {
+  def (xs: Seq[String]).longestStrings: Seq[String] = ...
+  def [T](xs: List[T]).second: T = ...
+}
+```
+If the name of an extension is not explicitly given, it is synthesized from the name and type of the first implemented extension method.
+
+Extension instances map directly to given instances. The `ops` extension above
+would expand to
+```scala
+given ops as AnyRef {
+  def (xs: Seq[String]).longestStrings: Seq[String] = ...
+  def (xs: Seq[String]).longestString: String = ...
+  def [T](xs: List[T]).second: T = ...
+}
+```
+The type "implemented" by this given instance is `AnyRef`, which
+is not a type one can summon by itself. This means that the instance can
+only be used for its extension methods.
+
+### Collective Extensions
+
+Sometimes, one wants to define several extension methods that share the same
+left-hand parameter type. In this case one can "pull out" the common parameters
+into the extension instance itself. Examples:
+```scala
+extension stringOps on (ss: Seq[String]) {
+  def longestStrings: Seq[String] = {
+    val maxLength = ss.map(_.length).max
+    ss.filter(_.length == maxLength)
+  }
+  def longestString: String = longestStrings.head
 }
 
 extension listOps on [T](xs: List[T]) {
-  def second = xs.tail.head
-  def third: T = xs.tail.tail.head
+  def second: T = xs.tail.head
+  def third: T = xs.tail.second
 }
 
 extension on [T](xs: List[T])(using Ordering[T]) {
   def largest(n: Int) = xs.sorted.takeRight(n)
 }
 ```
-If an extension is anonymous (as in the last clause), its name is synthesized from the name of the first defined extension method.
-
-The extensions above are equivalent to the following regular given instances where the implemented parent is `AnyRef` and the leading parameters are repeated in each extension method definition:
+Collective extensions like these are a shorthand for extension instances where
+the parameters following the `on` are repeated for each implemented method.
+Furthermore, each method's body starts with a synthesized import that
+imports all other names of methods defined in the same extension. This lets
+one use co-defined extension methods without the repeated prefix parameter,
+as is shown in the body of the `longestString` method above.
+
+For instance, the collective extensions above are equivalent to the following extension instances:
 ```scala
-given stringOps as AnyRef {
-  def (xs: Seq[String]).longestStrings: Seq[String] = {
-    val maxLength = xs.map(_.length).max
-    xs.filter(_.length == maxLength)
+extension stringOps {
+  def (ss: Seq[String]).longestStrings: Seq[String] = {
+    import ss.{longestStrings, longestString}
+    val maxLength = ss.map(_.length).max
+    ss.filter(_.length == maxLength)
+  }
+  def (ss: Seq[String]).longestString: String = {
+    import ss.{longestStrings, longestString}
+    longestStrings.head
   }
 }
-given listOps as AnyRef {
-  def [T](xs: List[T]).second = xs.tail.head
-  def [T](xs: List[T]).third: T = xs.tail.tail.head
+extension listOps {
+  def [T](xs: List[T]).second: T = {
+    import xs.{second, third}
+    xs.tail.head
+  }
+  def [T](xs: List[T]).third: T = {
+    import xs.{second, third}
+    xs.tail.second
+  }
 }
-given extension_largest_List_T as AnyRef {
-  def [T](xs: List[T]).largest(using Ordering[T])(n: Int) =
+extension {
+  def [T](xs: List[T]).largest(using Ordering[T])(n: Int) = {
+    import xs.largest
     xs.sorted.takeRight(n)
+  }
 }
 ```
 
 ### Syntax
 
 Here are the syntax changes for extension methods and collective extensions relative
-to the [current syntax](../../internals/syntax.md). `extension` is a soft keyword, recognized only in tandem with `on`. It can be used as an identifier everywhere else.
-
+to the [current syntax](../../internals/syntax.md).
 ```
 DefSig            ::=  ...
                     |  ExtParamClause [nl] [‘.’] id DefParamClauses
 ExtParamClause    ::=  [DefTypeParamClause] ‘(’ DefParam ‘)’
 TmplDef           ::=  ...
                     |  ‘extension’ ExtensionDef
-ExtensionDef      ::=  [id] ‘on’ ExtParamClause {GivenParamClause} ExtMethods
-ExtMethods        ::=  ‘{’ ‘def’ DefDef {semi ‘def’ DefDef} ‘}’
+ExtensionDef      ::=  [id] [‘on’ ExtParamClause {GivenParamClause}] TemplateBody
 ```
+The template body of an extension must consist only of extension method definitions for a regular
+extension instance, and only of normal method definitions for a collective extension instance.
+It must not be empty.
+
+`extension` and `on` are soft keywords, recognized only when they appear at the start of a
+statement in one of the patterns
+```scala
+extension on ...
+extension <ident> on ...
+extension { ...
+extension <ident> { ...
+```
\ No newline at end of file

From 6c461ff7d00027cee47c0be4ea7923ef5843012f Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Fri, 14 Feb 2020 18:35:01 +0100
Subject: [PATCH 02/11] Replace import by direct substitution

Don't postulate a synthetic import when expanding collective extensions.
Such an import would not typecheck. Postulate a direct substitution of
co-defined method identifiers instead.
---
 .../reference/contextual/extension-methods.md | 38 ++++++++-----------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/docs/docs/reference/contextual/extension-methods.md b/docs/docs/reference/contextual/extension-methods.md
index 28f4ab0a8b02..ea75387106a5 100644
--- a/docs/docs/reference/contextual/extension-methods.md
+++ b/docs/docs/reference/contextual/extension-methods.md
@@ -188,41 +188,35 @@ extension on [T](xs: List[T])(using Ordering[T]) {
 ```
 Collective extensions like these are a shorthand for extension instances where
 the parameters following the `on` are repeated for each implemented method.
-Furthermore, each method's body starts with a synthesized import that
-imports all other names of methods defined in the same extension. This lets
-one use co-defined extension methods without the repeated prefix parameter,
-as is shown in the body of the `longestString` method above.
-
-For instance, the collective extensions above are equivalent to the following extension instances:
+For instance, the collective extensions above expand to the following extension instances:
 ```scala
 extension stringOps {
   def (ss: Seq[String]).longestStrings: Seq[String] = {
-    import ss.{longestStrings, longestString}
     val maxLength = ss.map(_.length).max
     ss.filter(_.length == maxLength)
   }
-  def (ss: Seq[String]).longestString: String = {
-    import ss.{longestStrings, longestString}
-    longestStrings.head
-  }
+  def (ss: Seq[String]).longestString: String =
+    ss.longestStrings.head
 }
 extension listOps {
-  def [T](xs: List[T]).second: T = {
-    import xs.{second, third}
-    xs.tail.head
-  }
-  def [T](xs: List[T]).third: T = {
-    import xs.{second, third}
-    xs.tail.second
-  }
+  def [T](xs: List[T]).second: T = xs.tail.head
+  def [T](xs: List[T]).third: T = xs.tail.second
 }
 extension {
-  def [T](xs: List[T]).largest(using Ordering[T])(n: Int) = {
-    import xs.largest
+  def [T](xs: List[T]).largest(using Ordering[T])(n: Int) =
     xs.sorted.takeRight(n)
-  }
 }
 ```
+One special tweak is shown in the `longestString` method of the `stringOps` extension. It's original definition was
+```scala
+def longestString: String = longestStrings.head
+```
+This uses `longestStrings` as an implicit extension method call on the joint
+parameter `ss`. The usage is made explicit when translating the method:
+```scala
+def (ss: Seq[String]).longestString: String =
+  ss.longestStrings.head
+```
 
 ### Syntax
 

From 69ff7c789229a5826ff30cfdc5a185b8d23a593b Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Fri, 14 Feb 2020 18:58:36 +0100
Subject: [PATCH 03/11] Change syntax and Parser to allow extension instances

---
 .../dotty/tools/dotc/parsing/Parsers.scala    | 37 +++++++++++++++----
 docs/docs/internals/syntax.md                 |  3 +-
 tests/pos/reference/extension-methods.scala   | 15 +++++++-
 tests/run/extmethod-overload.scala            |  4 +-
 tests/run/instances.scala                     |  4 +-
 5 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
index 84eca75e84c2..4bc362f321da 100644
--- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
+++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
@@ -931,7 +931,11 @@ object Parsers {
       lookahead.nextToken()
       if lookahead.isIdent && !lookahead.isIdent(nme.on) then
         lookahead.nextToken()
+      if lookahead.isNewLine then
+        lookahead.nextToken()
       lookahead.isIdent(nme.on)
+      || lookahead.token == LBRACE
+      || lookahead.token == COLON
 
 /* --------- OPERAND/OPERATOR STACK --------------------------------------- */
 
@@ -3470,6 +3474,20 @@ object Parsers {
       Template(constr, parents, Nil, EmptyValDef, Nil)
     }
 
+    def checkExtensionMethod(tparams: List[Tree],
+        vparamss: List[List[Tree]], stat: Tree): Unit = stat match {
+      case stat: DefDef =>
+        if stat.mods.is(Extension) && vparamss.nonEmpty then
+          syntaxError(i"no extension method allowed here since leading parameter was already given", stat.span)
+        else if !stat.mods.is(Extension) && vparamss.isEmpty then
+          syntaxError(i"an extension method is required here", stat.span)
+        else if tparams.nonEmpty && stat.tparams.nonEmpty then
+          syntaxError(i"extension method cannot have type parameters since some were already given previously",
+            stat.tparams.head.span)
+      case stat =>
+        syntaxError(i"extension clause can only define methods", stat.span)
+    }
+
     /** GivenDef          ::=  [GivenSig] [‘_’ ‘<:’] Type ‘=’ Expr
      *                      |  [GivenSig] ConstrApps [TemplateBody]
      *  GivenSig          ::=  [id] [DefTypeParamClause] {UsingParamClauses} ‘as’
@@ -3516,20 +3534,25 @@ object Parsers {
       finalizeDef(gdef, mods1, start)
     }
 
-    /** ExtensionDef  ::=  [id] ‘on’ ExtParamClause {UsingParamClause} ExtMethods
+    /** ExtensionDef  ::=  [id] [‘on’ ExtParamClause {UsingParamClause}] TemplateBody
      */
     def extensionDef(start: Offset, mods: Modifiers): ModuleDef =
       in.nextToken()
       val name = if isIdent && !isIdent(nme.on) then ident() else EmptyTermName
       in.endMarkerScope(if name.isEmpty then nme.extension else name) {
-        if !isIdent(nme.on) then syntaxErrorOrIncomplete("`on` expected")
-        if isIdent(nme.on) then in.nextToken()
-        val tparams = typeParamClauseOpt(ParamOwner.Def)
-        val extParams = paramClause(0, prefix = true)
-        val givenParamss = paramClauses(givenOnly = true)
+        val (tparams, vparamss) =
+          if isIdent(nme.on) then
+            in.nextToken()
+            val tparams = typeParamClauseOpt(ParamOwner.Def)
+            val extParams = paramClause(0, prefix = true)
+            val givenParamss = paramClauses(givenOnly = true)
+            (tparams, extParams :: givenParamss)
+          else
+            (Nil, Nil)
         possibleTemplateStart()
         if !in.isNestedStart then syntaxError("Extension without extension methods")
-        val templ = templateBodyOpt(makeConstructor(tparams, extParams :: givenParamss), Nil, Nil)
+        val templ = templateBodyOpt(makeConstructor(tparams, vparamss), Nil, Nil)
+        templ.body.foreach(checkExtensionMethod(tparams, vparamss, _))
         val edef = ModuleDef(name, templ)
         finalizeDef(edef, addFlag(mods, Given), start)
       }
diff --git a/docs/docs/internals/syntax.md b/docs/docs/internals/syntax.md
index 6a1a169eece9..ee59cab8d50f 100644
--- a/docs/docs/internals/syntax.md
+++ b/docs/docs/internals/syntax.md
@@ -386,7 +386,8 @@ EnumDef           ::=  id ClassConstr InheritClauses EnumBody
 GivenDef          ::=  [GivenSig] [‘_’ ‘<:’] Type ‘=’ Expr
                     |  [GivenSig] ConstrApps [TemplateBody]
 GivenSig          ::=  [id] [DefTypeParamClause] {UsingParamClause} ‘as’
-ExtensionDef      ::=  [id] ‘on’ ExtParamClause {WithParamsOrTypes} ExtMethods
+ExtensionDef      ::=  [id] [‘on’ ExtParamClause {UsingParamClause}]
+                       TemplateBody
 ExtMethods        ::=  [nl] ‘{’ ‘def’ DefDef {semi ‘def’ DefDef} ‘}’
 ExtParamClause    ::=  [DefTypeParamClause] ‘(’ DefParam ‘)’
 Template          ::=  InheritClauses [TemplateBody]                            Template(constr, parents, self, stats)
diff --git a/tests/pos/reference/extension-methods.scala b/tests/pos/reference/extension-methods.scala
index 8fe07d523e7a..78ba2233464f 100644
--- a/tests/pos/reference/extension-methods.scala
+++ b/tests/pos/reference/extension-methods.scala
@@ -56,14 +56,25 @@ object ExtMethods:
   extension on [T](xs: List[T])(using Ordering[T]):
     def largest(n: Int) = xs.sorted.takeRight(n)
 
-  given stringOps1 as AnyRef {
+  extension ops:
+    def (xs: Seq[String]).longestStrings: Seq[String] =
+      val maxLength = xs.map(_.length).max
+      xs.filter(_.length == maxLength)
+    def (xs: Seq[String]).longestString: String = xs.longestStrings.head
+    def [T](xs: List[T]).second: T = xs.tail.head
+
+  extension:
+    def [T](xs: List[T]) longest (using Ordering[T])(n: Int) =
+      xs.sorted.takeRight(n)
+
+  given stringOps2 as AnyRef {
     def (xs: Seq[String]).longestStrings: Seq[String] = {
       val maxLength = xs.map(_.length).max
       xs.filter(_.length == maxLength)
     }
   }
 
-  given listOps1 as AnyRef {
+  given listOps2 as AnyRef {
     def [T](xs: List[T]) second = xs.tail.head
     def [T](xs: List[T]) third: T = xs.tail.tail.head
   }
diff --git a/tests/run/extmethod-overload.scala b/tests/run/extmethod-overload.scala
index edb88dd90990..7ae67268957c 100644
--- a/tests/run/extmethod-overload.scala
+++ b/tests/run/extmethod-overload.scala
@@ -22,13 +22,13 @@ object Test extends App {
   // Test with extension methods in given object
   object test1 {
 
-    given Foo as AnyRef {
+    extension Foo:
       def (x: Int) |+| (y: Int) = x + y
       def (x: Int) |+| (y: String) = x + y.length
 
       def [T](xs: List[T]) +++ (ys: List[T]): List[T] = xs ++ ys ++ ys
       def [T](xs: List[T]) +++ (ys: Iterator[T]): List[T] = xs ++ ys ++ ys
-    }
+    end Foo
 
     assert((1 |+| 2) == 3)
     assert((1 |+| "2") == 2)
diff --git a/tests/run/instances.scala b/tests/run/instances.scala
index 471b603d3ce9..c8533fad9225 100644
--- a/tests/run/instances.scala
+++ b/tests/run/instances.scala
@@ -31,10 +31,8 @@ object Test extends App {
   extension listListOps on [T](xs: List[List[T]]):
     def flattened = xs.foldLeft[List[T]](Nil)(_ ++ _)
 
-  // A right associative op. Note: can't use given extension for this!
-  given prepend as AnyRef {
+  extension prepend:
     def [T](x: T) :: (xs: Seq[T]) = x +: xs
-  }
 
   val ss: Seq[Int] = List(1, 2, 3)
   val ss1 = 0 :: ss

From c20893bf25db26be9b4d23848df2435a4e122094 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Sat, 15 Feb 2020 18:39:11 +0100
Subject: [PATCH 04/11] Disallow abstract methods in extensions

---
 compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 2 ++
 compiler/src/dotty/tools/dotc/typer/Nullables.scala | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
index 4bc362f321da..9772a7c30c4e 100644
--- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
+++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
@@ -3484,6 +3484,8 @@ object Parsers {
         else if tparams.nonEmpty && stat.tparams.nonEmpty then
           syntaxError(i"extension method cannot have type parameters since some were already given previously",
             stat.tparams.head.span)
+        else if stat.rhs.isEmpty then
+          syntaxError(i"extension method cannot be abstract", stat.span)
       case stat =>
         syntaxError(i"extension clause can only define methods", stat.span)
     }
diff --git a/compiler/src/dotty/tools/dotc/typer/Nullables.scala b/compiler/src/dotty/tools/dotc/typer/Nullables.scala
index 04795bc28a3d..7b593e529aec 100644
--- a/compiler/src/dotty/tools/dotc/typer/Nullables.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Nullables.scala
@@ -170,7 +170,7 @@ object Nullables:
       case info :: infos1 =>
         if info.asserted.contains(ref) then true
         else if info.retracted.contains(ref) then false
-        else impliesNotNull(infos1)(ref)
+        else infos1.impliesNotNull(ref)
       case _ =>
         false
 

From e8ed677d980fc709e8645204641a5c0ca71434a4 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Sat, 15 Feb 2020 18:40:26 +0100
Subject: [PATCH 05/11] Refactor extension desugaring

---
 .../src/dotty/tools/dotc/ast/Desugar.scala    | 66 +++++++++----------
 1 file changed, 32 insertions(+), 34 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/ast/Desugar.scala b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
index c2c29f60ed92..d1cfbddfb5ed 100644
--- a/compiler/src/dotty/tools/dotc/ast/Desugar.scala
+++ b/compiler/src/dotty/tools/dotc/ast/Desugar.scala
@@ -859,21 +859,22 @@ object desugar {
    *     given object name extends parents { self => body' }
    *
    *  where every definition in `body` is expanded to an extension method
-   *  taking type parameters `tparams` and a leading parameter `(x: T)`.
-   *  See: makeExtensionDef
+   *  taking type parameters `tparams` and a leading paramter `(x: T)`.
+   *  See: collectiveExtensionBody
    */
   def moduleDef(mdef: ModuleDef)(implicit ctx: Context): Tree = {
     val impl = mdef.impl
     val mods = mdef.mods
     impl.constr match {
-      case DefDef(_, tparams, (vparams @ (vparam :: Nil)) :: givenParamss, _, _) =>
+      case DefDef(_, tparams, vparamss @ (vparam :: Nil) :: givenParamss, _, _) =>
+        // Transform collective extension
         assert(mods.is(Given))
         return moduleDef(
           cpy.ModuleDef(mdef)(
             mdef.name,
             cpy.Template(impl)(
               constr = emptyConstructor,
-              body = impl.body.map(makeExtensionDef(_, tparams, vparams, givenParamss)))))
+              body = collectiveExtensionBody(impl.body, tparams, vparamss))))
       case _ =>
     }
 
@@ -916,43 +917,40 @@ object desugar {
     }
   }
 
-  /** Given tpe parameters `Ts` (possibly empty) and a leading value parameter `(x: T)`,
-   *  map a method definition
+  /** Transform the statements of a collective extension
+   *   @param stats    the original statements as they were parsed
+   *   @param tparams  the collective type parameters
+   *   @param vparamss the collective value parameters, consisting
+   *                   of a single leading value parameter, followed by
+   *                   zero or more context parameter clauses
    *
-   *     def foo [Us] paramss ...
+   *  Note: It is already assured by Parser.checkExtensionMethod that all
+   *  statements conform to requirements.
    *
-   *  to
+   *  Each method in stats is transformed into an extension method. Example:
+   *
+   *    extension on [Ts](x: T)(using C):
+   *      def f(y: T) = ???
+   *      def g(z: T) = f(z)
    *
-   *     <extension> def foo[Ts ++ Us](x: T) parammss ...
+   *  is turned into
    *
-   *  If the given member `mdef` is not of this form, flag it as an error.
+   *    extension:
+   *      <extension> def f[Ts](x: T)(using C)(y: T) = ???
+   *      <extension> def g[Ts](x: T)(using C)(z: T) = f(z)
    */
-
-  def makeExtensionDef(mdef: Tree, tparams: List[TypeDef], leadingParams: List[ValDef],
-                       givenParamss: List[List[ValDef]])(using ctx: Context): Tree = {
-    mdef match {
-      case mdef: DefDef =>
-        if (mdef.mods.is(Extension)) {
-          ctx.error(NoExtensionMethodAllowed(mdef), mdef.sourcePos)
-          mdef
-        } else {
-          if (tparams.nonEmpty && mdef.tparams.nonEmpty) then
-            ctx.error(ExtensionMethodCannotHaveTypeParams(mdef), mdef.tparams.head.sourcePos)
-            mdef
-          else cpy.DefDef(mdef)(
+  def collectiveExtensionBody(stats: List[Tree],
+      tparams: List[TypeDef], vparamss: List[List[ValDef]])(using ctx: Context): List[Tree] =
+    for stat <- stats yield
+      stat match
+        case mdef: DefDef =>
+          cpy.DefDef(mdef)(
             tparams = tparams ++ mdef.tparams,
-            vparamss = leadingParams :: givenParamss ::: mdef.vparamss
+            vparamss = vparamss ::: mdef.vparamss,
           ).withMods(mdef.mods | Extension)
-        }
-      case mdef: Import =>
-        mdef
-      case mdef if !mdef.isEmpty => {
-        ctx.error(ExtensionCanOnlyHaveDefs(mdef), mdef.sourcePos)
-        mdef
-      }
-      case mdef => mdef
-    }
-  }
+        case mdef =>
+          mdef
+  end collectiveExtensionBody
 
   /** Transforms
    *

From 3e1a1f2b589ad99c019e9ca9f9ad47f9cb273a14 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Sat, 15 Feb 2020 18:44:12 +0100
Subject: [PATCH 06/11] Set Extension flag for collective extension instances

---
 compiler/src/dotty/tools/dotc/core/Flags.scala            | 4 ++--
 compiler/src/dotty/tools/dotc/parsing/Parsers.scala       | 8 ++++----
 .../src/dotty/tools/dotc/typer/ImportSuggestions.scala    | 2 +-
 compiler/src/dotty/tools/dotc/typer/Namer.scala           | 4 ++--
 compiler/src/dotty/tools/dotc/typer/RefChecks.scala       | 4 ++--
 5 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala
index f21abcb53d1b..f49e8f4463fd 100644
--- a/compiler/src/dotty/tools/dotc/core/Flags.scala
+++ b/compiler/src/dotty/tools/dotc/core/Flags.scala
@@ -301,7 +301,7 @@ object Flags {
   /** A method that has default params */
   val (_, DefaultParameterized @ _, _) = newFlags(27, "<defaultparam>")
 
-  /** An extension method */
+  /** An extension method, or a collective extension instance */
   val (_, Extension @ _, _) = newFlags(28, "<extension>")
 
   /** An inferable (`given`) parameter */
@@ -506,7 +506,7 @@ object Flags {
   val RetainedModuleClassFlags: FlagSet = RetainedModuleValAndClassFlags | Enum
 
   /** Flags retained in export forwarders */
-  val RetainedExportFlags = Given | Implicit | Extension | Inline
+  val RetainedExportFlags = Given | Implicit | Inline
 
   /** Flags that apply only to classes */
   val ClassOnlyFlags = Sealed | Open | Abstract.toTypeFlags
diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
index 9772a7c30c4e..67913723cdab 100644
--- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
+++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
@@ -3542,21 +3542,21 @@ object Parsers {
       in.nextToken()
       val name = if isIdent && !isIdent(nme.on) then ident() else EmptyTermName
       in.endMarkerScope(if name.isEmpty then nme.extension else name) {
-        val (tparams, vparamss) =
+        val (tparams, vparamss, extensionFlag) =
           if isIdent(nme.on) then
             in.nextToken()
             val tparams = typeParamClauseOpt(ParamOwner.Def)
             val extParams = paramClause(0, prefix = true)
             val givenParamss = paramClauses(givenOnly = true)
-            (tparams, extParams :: givenParamss)
+            (tparams, extParams :: givenParamss, Extension)
           else
-            (Nil, Nil)
+            (Nil, Nil, EmptyFlags)
         possibleTemplateStart()
         if !in.isNestedStart then syntaxError("Extension without extension methods")
         val templ = templateBodyOpt(makeConstructor(tparams, vparamss), Nil, Nil)
         templ.body.foreach(checkExtensionMethod(tparams, vparamss, _))
         val edef = ModuleDef(name, templ)
-        finalizeDef(edef, addFlag(mods, Given), start)
+        finalizeDef(edef, addFlag(mods, Given | extensionFlag), start)
       }
 
 /* -------- TEMPLATES ------------------------------------------- */
diff --git a/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala b/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
index 47749a28265c..978c8466c75e 100644
--- a/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
+++ b/compiler/src/dotty/tools/dotc/typer/ImportSuggestions.scala
@@ -205,7 +205,7 @@ trait ImportSuggestions:
         .alternatives
         .map(mbr => TermRef(site, mbr.symbol))
         .filter(ref =>
-          ref.symbol.is(Extension)
+          ref.symbol.isAllOf(ExtensionMethod)
           && isApplicableMethodRef(ref, argType :: Nil, WildcardType))
         .headOption
 
diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala
index c2e81b18c348..335e9a4bbd5f 100644
--- a/compiler/src/dotty/tools/dotc/typer/Namer.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala
@@ -1096,8 +1096,8 @@ class Namer { typer: Typer =>
                     (StableRealizable, ExprType(path.tpe.select(sym)))
                   else
                     (EmptyFlags, mbr.info.ensureMethodic)
-                val mbrFlags = Exported | Method | Final | maybeStable | sym.flags & RetainedExportFlags
-                val forwarderName = checkNoConflict(alias, isPrivate = false, span)
+                var mbrFlags = Exported | Method | Final | maybeStable | sym.flags & RetainedExportFlags
+                if sym.isAllOf(ExtensionMethod) then mbrFlags |= Extension
                 ctx.newSymbol(cls, forwarderName, mbrFlags, mbrInfo, coord = span)
               }
             forwarder.info = avoidPrivateLeaks(forwarder)
diff --git a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala
index 681ef893e58b..7eebc6aebfe4 100644
--- a/compiler/src/dotty/tools/dotc/typer/RefChecks.scala
+++ b/compiler/src/dotty/tools/dotc/typer/RefChecks.scala
@@ -394,9 +394,9 @@ object RefChecks {
         overrideError("is erased, cannot override non-erased member")
       else if (other.is(Erased) && !member.isOneOf(Erased | Inline)) // (1.9.1)
         overrideError("is not erased, cannot override erased member")
-      else if (member.is(Extension) && !other.is(Extension)) // (1.9.2)
+      else if (member.isAllOf(ExtensionMethod) && !other.isAllOf(ExtensionMethod)) // (1.9.2)
         overrideError("is an extension method, cannot override a normal method")
-      else if (other.is(Extension) && !member.is(Extension)) // (1.9.2)
+      else if (other.isAllOf(ExtensionMethod) && !member.isAllOf(ExtensionMethod)) // (1.9.2)
         overrideError("is a normal method, cannot override an extension method")
       else if ((member.isInlineMethod || member.isScala2Macro) && other.is(Deferred) &&
                  member.extendedOverriddenSymbols.forall(_.is(Deferred))) // (1.10)

From 7acd0ea65cb50cf793124e8e0896dd638f0067d1 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Sat, 15 Feb 2020 23:52:58 +0100
Subject: [PATCH 07/11] Allow cross references in collective extensions

---
 .../src/dotty/tools/dotc/core/Flags.scala     |  2 +-
 .../dotty/tools/dotc/transform/SymUtils.scala |  3 +++
 .../src/dotty/tools/dotc/typer/Typer.scala    | 23 +++++++++++++++++--
 tests/run/collective-extensions.scala         | 23 +++++++++++++++++++
 4 files changed, 48 insertions(+), 3 deletions(-)
 create mode 100644 tests/run/collective-extensions.scala

diff --git a/compiler/src/dotty/tools/dotc/core/Flags.scala b/compiler/src/dotty/tools/dotc/core/Flags.scala
index f49e8f4463fd..c5b033342460 100644
--- a/compiler/src/dotty/tools/dotc/core/Flags.scala
+++ b/compiler/src/dotty/tools/dotc/core/Flags.scala
@@ -499,7 +499,7 @@ object Flags {
 
   /** Flags that can apply to a module val */
   val RetainedModuleValFlags: FlagSet = RetainedModuleValAndClassFlags |
-    Override | Final | Method | Implicit | Given | Lazy |
+    Override | Final | Method | Implicit | Given | Lazy | Extension |
     Accessor | AbsOverride | StableRealizable | Captured | Synchronized | Erased
 
   /** Flags that can apply to a module class */
diff --git a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala
index cee9866c8a02..ab57c3a3bc93 100644
--- a/compiler/src/dotty/tools/dotc/transform/SymUtils.scala
+++ b/compiler/src/dotty/tools/dotc/transform/SymUtils.scala
@@ -220,4 +220,7 @@ class SymUtils(val self: Symbol) extends AnyVal {
   /** Is symbol a splice operation? */
   def isSplice(implicit ctx: Context): Boolean =
     self == defn.InternalQuoted_exprSplice || self == defn.QuotedType_splice
+
+  def isCollectiveExtensionClass(using Context): Boolean =
+    self.is(ModuleClass) && self.sourceModule.is(Extension, butNot = Method)
 }
diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
index 2e5afd3cde0f..0bdf6e2c75e0 100644
--- a/compiler/src/dotty/tools/dotc/typer/Typer.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -401,6 +401,19 @@ class Typer extends Namer
     if (name == nme.ROOTPKG)
       return tree.withType(defn.RootPackage.termRef)
 
+    /** Convert a reference `f` to an extension method in a collective extension
+     *  on parameter `x` to `x.f`
+     */
+    def extensionMethodSelect(xmethod: Symbol): untpd.Tree =
+      val leadParamName = xmethod.info.paramNamess.head.head
+      def isLeadParam(sym: Symbol) =
+        sym.is(Param) && sym.owner.owner == xmethod.owner && sym.name == leadParamName
+      def leadParam(ctx: Context): Symbol =
+        ctx.scope.lookupAll(leadParamName).find(isLeadParam) match
+          case Some(param) => param
+          case None => leadParam(ctx.outersIterator.dropWhile(_.scope eq ctx.scope).next)
+      untpd.cpy.Select(tree)(untpd.ref(leadParam(ctx).termRef), name)
+
     val rawType = {
       val saved1 = unimported
       val saved2 = foundUnderScala2
@@ -441,8 +454,14 @@ class Typer extends Namer
         errorType(new MissingIdent(tree, kind, name.show), tree.sourcePos)
 
     val tree1 = ownType match {
-      case ownType: NamedType if !prefixIsElidable(ownType) =>
-        ref(ownType).withSpan(tree.span)
+      case ownType: NamedType =>
+        val sym = ownType.symbol
+        if sym.isAllOf(ExtensionMethod)
+            && sym.owner.isCollectiveExtensionClass
+            && ctx.owner.isContainedIn(sym.owner)
+        then typed(extensionMethodSelect(sym), pt)
+        else if prefixIsElidable(ownType) then tree.withType(ownType)
+        else ref(ownType).withSpan(tree.span)
       case _ =>
         tree.withType(ownType)
     }
diff --git a/tests/run/collective-extensions.scala b/tests/run/collective-extensions.scala
new file mode 100644
index 000000000000..9c8c2d4de04d
--- /dev/null
+++ b/tests/run/collective-extensions.scala
@@ -0,0 +1,23 @@
+extension on (x: String):
+  def foo(y: String): String = x ++ y
+  def bar(y: String): String = foo(y)
+  def baz(y: String): String =
+    val x = y
+    bar(x)
+  def bam(y: String): String = this.baz(x)(y)
+  def ban(foo: String): String = x + foo
+  def bao(y: String): String =
+    val bam = "ABC"
+    x ++ y ++ bam
+
+  def app(n: Int, suffix: String): String =
+    if n == 0 then x ++ suffix
+    else app(n - 1, suffix ++ suffix)
+
+@main def Test =
+  assert("abc".bar("def") == "abcdef")
+  assert("abc".baz("def") == "abcdef")
+  assert("abc".bam("def") == "abcdef")
+  assert("abc".ban("def") == "abcdef")
+  assert("abc".bao("def") == "abcdefABC")
+  assert("abc".app(3, "!") == "abc!!!!!!!!")

From e4cb221c33f7775efab0c73f1869bf80c56967be Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Sat, 29 Feb 2020 18:17:18 +0100
Subject: [PATCH 08/11] Fix rebase breakage

---
 compiler/src/dotty/tools/dotc/typer/Namer.scala | 1 +
 1 file changed, 1 insertion(+)

diff --git a/compiler/src/dotty/tools/dotc/typer/Namer.scala b/compiler/src/dotty/tools/dotc/typer/Namer.scala
index 335e9a4bbd5f..0f74dbc456bc 100644
--- a/compiler/src/dotty/tools/dotc/typer/Namer.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Namer.scala
@@ -1098,6 +1098,7 @@ class Namer { typer: Typer =>
                     (EmptyFlags, mbr.info.ensureMethodic)
                 var mbrFlags = Exported | Method | Final | maybeStable | sym.flags & RetainedExportFlags
                 if sym.isAllOf(ExtensionMethod) then mbrFlags |= Extension
+                val forwarderName = checkNoConflict(alias, isPrivate = false, span)
                 ctx.newSymbol(cls, forwarderName, mbrFlags, mbrInfo, coord = span)
               }
             forwarder.info = avoidPrivateLeaks(forwarder)

From 46bb161ddb223630f73dd54435f596b0034c87ab Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Mon, 2 Mar 2020 09:11:13 +0100
Subject: [PATCH 09/11] Allow EmptyTrees in extensions

---
 compiler/src/dotty/tools/dotc/parsing/Parsers.scala | 1 +
 1 file changed, 1 insertion(+)

diff --git a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
index 67913723cdab..d01868ab5cb2 100644
--- a/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
+++ b/compiler/src/dotty/tools/dotc/parsing/Parsers.scala
@@ -3486,6 +3486,7 @@ object Parsers {
             stat.tparams.head.span)
         else if stat.rhs.isEmpty then
           syntaxError(i"extension method cannot be abstract", stat.span)
+      case EmptyTree =>
       case stat =>
         syntaxError(i"extension clause can only define methods", stat.span)
     }

From 2c88bb5fff070e5047d95f1b28381e24409b7d20 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Mon, 2 Mar 2020 09:11:34 +0100
Subject: [PATCH 10/11] Adapt check files

---
 tests/neg/extension-cannot-have-type.check    | 6 ++----
 tests/neg/extension-method-not-allowed.check  | 6 ++----
 tests/neg/extensions-can-only-have-defs.check | 6 ++----
 tests/neg/i6900.scala                         | 8 ++++----
 4 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/tests/neg/extension-cannot-have-type.check b/tests/neg/extension-cannot-have-type.check
index b98355683c05..b7bfc18ad778 100644
--- a/tests/neg/extension-cannot-have-type.check
+++ b/tests/neg/extension-cannot-have-type.check
@@ -1,6 +1,4 @@
--- [E151] Syntax Error: tests/neg/extension-cannot-have-type.scala:3:10 ------------------------------------------------
+-- Error: tests/neg/extension-cannot-have-type.scala:3:10 --------------------------------------------------------------
 3 |    def f[U](u: U): T = ??? // error : extension method cannot have type params
   |          ^
-  |          Extension method cannot have type parameters since some were already given previously
-
-longer explanation available when compiling with `-explain`
+  |          extension method cannot have type parameters since some were already given previously
diff --git a/tests/neg/extension-method-not-allowed.check b/tests/neg/extension-method-not-allowed.check
index fd109a3e921d..8886f484b869 100644
--- a/tests/neg/extension-method-not-allowed.check
+++ b/tests/neg/extension-method-not-allowed.check
@@ -1,6 +1,4 @@
--- [E150] Syntax Error: tests/neg/extension-method-not-allowed.scala:3:8 -----------------------------------------------
+-- Error: tests/neg/extension-method-not-allowed.scala:3:8 -------------------------------------------------------------
 3 |    def (c: T).f: T = ??? // error : No extension method allowed here
   |    ^^^^^^^^^^^^^^^^^^^^^
-  |    No extension method allowed here, since collective parameters are given
-
-longer explanation available when compiling with `-explain`
+  |    no extension method allowed here since leading parameter was already given
diff --git a/tests/neg/extensions-can-only-have-defs.check b/tests/neg/extensions-can-only-have-defs.check
index 7fd29b92aba8..c155678f6fed 100644
--- a/tests/neg/extensions-can-only-have-defs.check
+++ b/tests/neg/extensions-can-only-have-defs.check
@@ -1,6 +1,4 @@
--- [E152] Syntax Error: tests/neg/extensions-can-only-have-defs.scala:3:8 ----------------------------------------------
+-- Error: tests/neg/extensions-can-only-have-defs.scala:3:8 ------------------------------------------------------------
 3 |    val v: T = ??? // error : extensions can only have defs
   |    ^^^^^^^^^^^^^^
-  |    Only methods allowed here, since collective parameters are given
-
-longer explanation available when compiling with `-explain`
+  |    extension clause can only define methods
diff --git a/tests/neg/i6900.scala b/tests/neg/i6900.scala
index a81b4ba58350..c9ead6a99051 100644
--- a/tests/neg/i6900.scala
+++ b/tests/neg/i6900.scala
@@ -4,12 +4,12 @@ object Test2 {
   extension on [A](a: A):
     def foo[C]: C => A = _ => a   // error: extension method cannot have type parameters
 
-  1.foo.foo                      // error: foo is undefined
+  1.foo.foo
 
   // ... but have to pass 2 parameters
-  1.foo.foo[Any => Int, String]                  // error: foo is undefined
-  1.foo[Int, String].foo                         // error: foo is undefined
-  1.foo[Int, String].foo[String => Int, String]  // error: foo is undefined
+  1.foo.foo[Any => Int, String]
+  1.foo[Int, String].foo
+  1.foo[Int, String].foo[String => Int, String]
 
 }
 

From e732ee55b5b3b7d3ddbb7f97d602e23036145a91 Mon Sep 17 00:00:00 2001
From: Martin Odersky <odersky@gmail.com>
Date: Mon, 2 Mar 2020 10:58:30 +0100
Subject: [PATCH 11/11] Drop error messages tests

I believe error messages tests should be dropped, unless they test something really specific.
E.g. a way information is represented that is easy to break.

For normal tests we have check files, or (most of the time even better) `// error` annotations.

The problem with over-engineered solutions like error messages tests is that they cause a huge
friction when things change. I lost an inordinate amount of time getting the extensions PR
over the line since in the meantime there landed an elaborate error messages commit. Fixing rebase
breakages over this PR is not a good use of my time! Two things could improve that situation:

 - we go slower on inessential PRs like error messages, _in particular_ if there is a pending
   conflict with a pending PR
 - we go faster merging essential PRs.
---
 .../dotc/reporting/ErrorMessagesTests.scala   | 68 -------------------
 1 file changed, 68 deletions(-)

diff --git a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
index 9dee80d55271..e1c017ad161c 100644
--- a/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
+++ b/compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala
@@ -1840,74 +1840,6 @@ class ErrorMessagesTests extends ErrorMessagesTest {
         assertEquals("given x @ String", x.show)
       }
 
-  @Test def extensionMethodsNotAllowed =
-    checkMessagesAfter(RefChecks.name) {
-      """object Test {
-        |  extension on[T] (t: T) {
-        |    def (c: T).f: T = ???
-        |  }
-        |}
-      """.stripMargin
-    }
-      .expect { (ictx, messages) ⇒
-        implicit val ctx: Context = ictx
-        assertMessageCount(1, messages)
-        val errorMsg = messages.head.msg
-        val NoExtensionMethodAllowed(x) :: Nil = messages
-        assertEquals("No extension method allowed here, since collective parameters are given", errorMsg)
-        assertEquals("def (c: T) f: T = ???", x.show)
-      }
-
-  @Test def extensionMethodTypeParamsNotAllowed =
-    checkMessagesAfter(RefChecks.name) {
-      """object Test {
-        |  extension on[T] (t: T) {
-        |    def f[U](u: U): T = ???
-        |  }
-        |}
-      """.stripMargin
-    }
-      .expect { (ictx, messages) ⇒
-        implicit val ctx: Context = ictx
-        assertMessageCount(1, messages)
-        val errorMsg = messages.head.msg
-        val ExtensionMethodCannotHaveTypeParams(x) :: Nil = messages
-        assertEquals("Extension method cannot have type parameters since some were already given previously", errorMsg)
-        assertEquals("def f[U](u: U): T = ???", x.show)
-      }
-
-  @Test def extensionMethodCanOnlyHaveDefs =
-    checkMessagesAfter(RefChecks.name) {
-      """object Test {
-        |  extension on[T] (t: T) {
-        |    val v: T = t
-        |  }
-        |}
-      """.stripMargin
-    }
-      .expect { (ictx, messages) ⇒
-        implicit val ctx: Context = ictx
-        assertMessageCount(1, messages)
-        val errorMsg = messages.head.msg
-        val ExtensionCanOnlyHaveDefs(x) :: Nil = messages
-        assertEquals("Only methods allowed here, since collective parameters are given", errorMsg)
-        assertEquals("val v: T = t", x.show)
-      }
-
-  @Test def anonymousInstanceMustImplementAType =
-    checkMessagesAfter(RefChecks.name) {
-      """object Test {
-        |  extension on[T] (t: T) { }
-        |}
-      """.stripMargin
-    }
-      .expect { (ictx, messages) ⇒
-        implicit val ctx: Context = ictx
-        assertMessageCount(1, messages)
-        val errorMsg = messages.head.msg
-        assertEquals("anonymous instance must implement a type or have at least one extension method", errorMsg)
-      }
-
   @Test def typeSplicesInValPatterns =
     checkMessagesAfter(RefChecks.name) {
       s"""import scala.quoted._