Merge pull request #256 from scala/backport-lts-3.3-22492

Backport "Discourage default arg for extension receiver" to 3.3 LTS

Author: tgodzik

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

@@ -338,6 +338,7 @@ object desugar {
           .withMods(Modifiers(
             meth.mods.flags & (AccessFlags | Synthetic) | (vparam.mods.flags & Inline),
             meth.mods.privateWithin))
+          .withSpan(vparam.rhs.span)
         val rest = defaultGetters(vparams :: paramss1, n + 1)
         if vparam.rhs.isEmpty then rest else defaultGetter :: rest
       case _ :: paramss1 =>  // skip empty parameter lists and type parameters

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

@@ -220,6 +220,8 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
   case DeprecatedInfixNamedArgumentSyntaxID // errorNumber: 204
   case GivenSearchPriorityID // errorNumber: 205
   case EnumMayNotBeValueClassesID // errorNumber: 206
+  case IllegalUnrollPlacementID // errorNumber: 207  - unused in LTS
+  case ExtensionHasDefaultID // errorNumber: 208
 
   def errorNumber = ordinal - 1
 

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

@@ -2474,6 +2474,17 @@ class ExtensionNullifiedByMember(method: Symbol, target: Symbol)(using Context)
        |
        |The extension may be invoked as though selected from an arbitrary type if conversions are in play."""
 
+class ExtensionHasDefault(method: Symbol)(using Context)
+  extends Message(ExtensionHasDefaultID):
+  def kind = MessageKind.PotentialIssue
+  def msg(using Context) =
+    i"""Extension method ${hl(method.name.toString)} should not have a default argument for its receiver."""
+  def explain(using Context) =
+    i"""The receiver cannot be omitted when an extension method is invoked as a selection.
+       |A default argument for that parameter would never be used in that case.
+       |An extension method can be invoked as a regular method, but if that is the intended usage,
+       |it should not be defined as an extension."""
+
 class TraitCompanionWithMutableStatic()(using Context)
   extends SyntaxMsg(TraitCompanionWithMutableStaticID) {
   def msg(using Context) = i"Companion of traits cannot define mutable @static fields"

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

@@ -1062,22 +1062,21 @@ object RefChecks {
    *  that are either both opaque types or both not.
    */
   def checkExtensionMethods(sym: Symbol)(using Context): Unit =
-    if sym.is(Extension) && !sym.nextOverriddenSymbol.exists then
+    if sym.is(Extension) then
       extension (tp: Type)
-        def strippedResultType = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true).resultType
-        def firstExplicitParamTypes = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true).firstParamTypes
+        def explicit = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true)
         def hasImplicitParams = tp.stripPoly match { case mt: MethodType => mt.isImplicitMethod case _ => false }
-      val target = sym.info.firstExplicitParamTypes.head // required for extension method, the putative receiver
-      val methTp = sym.info.strippedResultType // skip leading implicits and the "receiver" parameter
+      val explicitInfo = sym.info.explicit // consider explicit value params
+      val target = explicitInfo.firstParamTypes.head // required for extension method, the putative receiver
+      val methTp = explicitInfo.resultType // skip leading implicits and the "receiver" parameter
       def hidden =
         target.nonPrivateMember(sym.name)
-        .filterWithPredicate:
-          member =>
+        .filterWithPredicate: member =>
           member.symbol.isPublic && {
             val memberIsImplicit = member.info.hasImplicitParams
             val paramTps =
               if memberIsImplicit then methTp.stripPoly.firstParamTypes
-              else methTp.firstExplicitParamTypes
+              else methTp.explicit.firstParamTypes
 
             paramTps.isEmpty || memberIsImplicit && !methTp.hasImplicitParams || {
               val memberParamTps = member.info.stripPoly.firstParamTypes
@@ -1089,7 +1088,15 @@ object RefChecks {
             }
           }
         .exists
-      if !target.typeSymbol.isOpaqueAlias && hidden
+      if sym.is(HasDefaultParams) then
+        val getterDenot =
+          val receiverName = explicitInfo.firstParamNames.head
+          val num = sym.info.paramNamess.flatten.indexWhere(_ == receiverName)
+          val getterName = DefaultGetterName(sym.name.toTermName, num = num)
+          sym.owner.info.member(getterName)
+        if getterDenot.exists
+        then report.warning(ExtensionHasDefault(sym), getterDenot.symbol.srcPos)
+      if !target.typeSymbol.isOpaqueAlias && !sym.nextOverriddenSymbol.exists && hidden
       then report.warning(ExtensionNullifiedByMember(sym, target.typeSymbol), sym.srcPos)
   end checkExtensionMethods
 

tests/warn/i12460.check

@@ -0,0 +1,24 @@
+-- [E208] Potential Issue Warning: tests/warn/i12460.scala:3:23 --------------------------------------------------------
+3 |extension (s: String = "hello, world") def invert = s.reverse.toUpperCase // warn
+  |                       ^
+  |                       Extension method invert should not have a default argument for its receiver.
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | The receiver cannot be omitted when an extension method is invoked as a selection.
+  | A default argument for that parameter would never be used in that case.
+  | An extension method can be invoked as a regular method, but if that is the intended usage,
+  | it should not be defined as an extension.
+   ---------------------------------------------------------------------------------------------------------------------
+-- [E208] Potential Issue Warning: tests/warn/i12460.scala:5:37 --------------------------------------------------------
+5 |extension (using String)(s: String = "hello, world") def revert = s.reverse.toUpperCase // warn
+  |                                     ^
+  |                                     Extension method revert should not have a default argument for its receiver.
+  |---------------------------------------------------------------------------------------------------------------------
+  | Explanation (enabled by `-explain`)
+  |- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+  | The receiver cannot be omitted when an extension method is invoked as a selection.
+  | A default argument for that parameter would never be used in that case.
+  | An extension method can be invoked as a regular method, but if that is the intended usage,
+  | it should not be defined as an extension.
+   ---------------------------------------------------------------------------------------------------------------------

tests/warn/i12460.scala

@@ -0,0 +1,9 @@
+//> using options -explain -Ystop-after:refchecks
+
+extension (s: String = "hello, world") def invert = s.reverse.toUpperCase // warn
+
+extension (using String)(s: String = "hello, world") def revert = s.reverse.toUpperCase // warn
+
+extension (s: String)
+  def divert(m: String = "hello, world") = (s+m).reverse.toUpperCase // ok
+  def divertimento(using String)(m: String = "hello, world") = (s+m).reverse.toUpperCase // ok