Remove redundant check for extension methods from Parsers and port errors of the Desugar check to the new scheme

Author: ausmarton

compiler/src/dotty/tools/dotc/ast/Desugar.scala

@@ -930,21 +930,24 @@ object desugar {
 
   def makeExtensionDef(mdef: Tree, tparams: List[TypeDef], leadingParams: List[ValDef],
                        givenParamss: List[List[ValDef]])(using ctx: Context): Tree = {
-    val allowed = "allowed here, since collective parameters are given"
     mdef match {
       case mdef: DefDef =>
         if (mdef.mods.is(Extension)) {
-          ctx.error(em"No extension method $allowed", mdef.sourcePos)
+          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)(
+            tparams = tparams ++ mdef.tparams,
+            vparamss = leadingParams :: givenParamss ::: mdef.vparamss
+          ).withMods(mdef.mods | Extension)
         }
-        else cpy.DefDef(mdef)(
-          tparams = tparams ++ mdef.tparams,
-          vparamss = leadingParams :: givenParamss ::: mdef.vparamss
-        ).withMods(mdef.mods | Extension)
       case mdef: Import =>
         mdef
       case mdef =>
-        ctx.error(em"Only methods $allowed", mdef.sourcePos)
+        ctx.error(ExtensionCanOnlyHaveDefs(mdef), mdef.sourcePos)
         mdef
     }
   }

compiler/src/dotty/tools/dotc/parsing/Parsers.scala

@@ -3470,17 +3470,6 @@ object Parsers {
       Template(constr, parents, Nil, EmptyValDef, Nil)
     }
 
-    def checkExtensionMethod(tparams: List[Tree], stat: Tree): Unit = stat match {
-      case stat: DefDef =>
-        if stat.mods.is(Extension) then
-          syntaxError(i"no extension method allowed here since leading parameter was already given", 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’
@@ -3541,7 +3530,6 @@ object Parsers {
         possibleTemplateStart()
         if !in.isNestedStart then syntaxError("Extension without extension methods")
         val templ = templateBodyOpt(makeConstructor(tparams, extParams :: givenParamss), Nil, Nil)
-        templ.body.foreach(checkExtensionMethod(tparams, _))
         val edef = ModuleDef(name, templ)
         finalizeDef(edef, addFlag(mods, Given), start)
       }

compiler/src/dotty/tools/dotc/reporting/diagnostic/ErrorMessageID.scala

@@ -158,6 +158,9 @@ enum ErrorMessageID extends java.lang.Enum[ErrorMessageID] {
     ModifierRedundantForObjectsID,
     TypedCaseDoesNotExplicitlyExtendTypedEnumID,
     IllegalRedefinitionOfStandardKindID,
+    NoExtensionMethodAllowedID,
+    ExtensionMethodCannotHaveTypeParamsID,
+    ExtensionCanOnlyHaveDefsID,
     UnexpectedPatternForSummonFromID
 
   def errorNumber = ordinal - 2

compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala

@@ -2470,6 +2470,42 @@ object messages {
            |""".stripMargin
   }
 
+ case class NoExtensionMethodAllowed(mdef: untpd.DefDef)(implicit ctx: Context)
+    extends Message(NoExtensionMethodAllowedID) {
+    val kind: String = "Syntax"
+    val msg: String = em"No extension method allowed here, since collective parameters are given"
+
+    val explanation: String =
+      em"""|Extension method:
+           |  `${mdef}`
+           |is defined inside an extension clause which has collective parameters.
+           |""".stripMargin
+  }
+
+ case class ExtensionMethodCannotHaveTypeParams(mdef: untpd.DefDef)(implicit ctx: Context)
+    extends Message(ExtensionMethodCannotHaveTypeParamsID) {
+    val kind: String = "Syntax"
+    val msg: String = i"Extension method cannot have type parameters since some were already given previously"
+
+    val explanation: String =
+      em"""|Extension method:
+           |  `${mdef}`
+           |has type parameters `[${mdef.tparams.map(_.show).mkString(",")}]`, while the extension clause has
+           |it's own type parameters. Please consider moving these to the extension clause's type parameter list.
+           |""".stripMargin
+  }
+
+ case class ExtensionCanOnlyHaveDefs(mdef: untpd.Tree)(implicit ctx: Context)
+    extends Message(ExtensionCanOnlyHaveDefsID) {
+    val kind: String = "Syntax"
+    val msg: String = em"Only methods allowed here, since collective parameters are given"
+
+    val explanation: String =
+      em"""Extension clauses can only have `def`s
+          | `${mdef.show}` is not a valid expression here.
+          |""".stripMargin
+  }
+
   case class UnexpectedPatternForSummonFrom(tree: Tree[_])(implicit ctx: Context)
     extends Message(UnexpectedPatternForSummonFromID) {
     val kind: String = "Syntax"

compiler/src/dotty/tools/dotc/typer/Applications.scala

@@ -1076,7 +1076,7 @@ trait Applications extends Compatibility {
       msgs match
         case msg :: Nil =>
           msg.contained match
-            case messages.NotAMember(_, nme.unapply, _, _) => return notAnExtractor(tree)
+            case NotAMember(_, nme.unapply, _, _) => return notAnExtractor(tree)
             case _ =>
         case _ =>
       msgs.foreach(ctx.reporter.report)

compiler/test/dotty/tools/dotc/reporting/ErrorMessagesTests.scala

@@ -1839,4 +1839,58 @@ class ErrorMessagesTests extends ErrorMessagesTest {
         assertEquals("Unexpected pattern for summonFrom. Expected `x: T` or `_`", errorMsg)
         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)
+      }
 }

tests/neg/SummonFrom.check

@@ -1,10 +1,10 @@
--- [E150] Syntax Error: tests/neg/SummonFrom.scala:4:7 -----------------------------------------------------------------
+-- [E153] Syntax Error: tests/neg/SummonFrom.scala:4:7 -----------------------------------------------------------------
 4 |  case x => ??? // error
   |       ^
   |       Unexpected pattern for summonFrom. Expected `x: T` or `_`
 
 longer explanation available when compiling with `-explain`
--- [E150] Syntax Error: tests/neg/SummonFrom.scala:8:7 -----------------------------------------------------------------
+-- [E153] Syntax Error: tests/neg/SummonFrom.scala:8:7 -----------------------------------------------------------------
 8 |  case x@String => ??? // error
   |       ^^^^^^^^
   |       Unexpected pattern for summonFrom. Expected `x: T` or `_`

tests/neg/extension-cannot-have-type.check

@@ -0,0 +1,6 @@
+-- [E151] Syntax 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`

tests/neg/extension-cannot-have-type.scala

@@ -0,0 +1,5 @@
+object Test {
+  extension on[T] (t: T) {
+    def f[U](u: U): T = ??? // error : extension method cannot have type params
+  }
+}

tests/neg/extension-method-not-allowed.check

@@ -0,0 +1,6 @@
+-- [E150] Syntax 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`

tests/neg/extension-method-not-allowed.scala

@@ -0,0 +1,5 @@
+object Test {
+  extension on[T] (t: T) {
+    def (c: T).f: T = ??? // error : No extension method allowed here
+  }
+}

tests/neg/extensions-can-only-have-defs.check

@@ -0,0 +1,6 @@
+-- [E152] Syntax 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`

tests/neg/extensions-can-only-have-defs.scala

@@ -0,0 +1,5 @@
+object Test {
+  extension on[T] (t: T) {
+    val v: T = ??? // error : extensions can only have defs
+  }
+}

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
+  1.foo.foo                      // error: foo is undefined
 
   // ... but have to pass 2 parameters
-  1.foo.foo[Any => Int, String]
-  1.foo[Int, String].foo
-  1.foo[Int, String].foo[String => Int, String]
+  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
 
 }