Skip to content

Commit afb4806

Browse files
authored
Discourage default arg for extension receiver (#22492)
Fixes #12460 I shied away from making it an error, as that is a language change that violates the rule that extension methods are ordinary methods. There are other restrictions, but an extension always allows explicit invocation `m()(x)` that could leverage a default. The caret is wrong on the second test case (~todo~). Edit: span of default getter was union of first parameter and the param RHS, so that the synthetic position of the getter symbol was a point at the first parameter. Now the getter tree gets the span of the RHS. (That span is non-synthetic, but the definition is still synthetic. The name pos of a synthetic is the point.)
1 parent 4952e0a commit afb4806

File tree

6 files changed

+62
-9
lines changed

6 files changed

+62
-9
lines changed

Diff for: compiler/src/dotty/tools/dotc/ast/Desugar.scala

+1
Original file line numberDiff line numberDiff line change
@@ -418,6 +418,7 @@ object desugar {
418418
.withMods(Modifiers(
419419
meth.mods.flags & (AccessFlags | Synthetic) | (vparam.mods.flags & Inline),
420420
meth.mods.privateWithin))
421+
.withSpan(vparam.rhs.span)
421422
val rest = defaultGetters(vparams :: paramss1, n + 1)
422423
if vparam.rhs.isEmpty then rest else defaultGetter :: rest
423424
case _ :: paramss1 => // skip empty parameter lists and type parameters

Diff for: compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala

+1
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
221221
case GivenSearchPriorityID // errorNumber: 205
222222
case EnumMayNotBeValueClassesID // errorNumber: 206
223223
case IllegalUnrollPlacementID // errorNumber: 207
224+
case ExtensionHasDefaultID // errorNumber: 208
224225

225226
def errorNumber = ordinal - 1
226227

Diff for: compiler/src/dotty/tools/dotc/reporting/messages.scala

+11
Original file line numberDiff line numberDiff line change
@@ -2514,6 +2514,17 @@ class ExtensionNullifiedByMember(method: Symbol, target: Symbol)(using Context)
25142514
|
25152515
|The extension may be invoked as though selected from an arbitrary type if conversions are in play."""
25162516

2517+
class ExtensionHasDefault(method: Symbol)(using Context)
2518+
extends Message(ExtensionHasDefaultID):
2519+
def kind = MessageKind.PotentialIssue
2520+
def msg(using Context) =
2521+
i"""Extension method ${hl(method.name.toString)} should not have a default argument for its receiver."""
2522+
def explain(using Context) =
2523+
i"""The receiver cannot be omitted when an extension method is invoked as a selection.
2524+
|A default argument for that parameter would never be used in that case.
2525+
|An extension method can be invoked as a regular method, but if that is the intended usage,
2526+
|it should not be defined as an extension."""
2527+
25172528
class TraitCompanionWithMutableStatic()(using Context)
25182529
extends SyntaxMsg(TraitCompanionWithMutableStaticID) {
25192530
def msg(using Context) = i"Companion of traits cannot define mutable @static fields"

Diff for: compiler/src/dotty/tools/dotc/typer/RefChecks.scala

+16-9
Original file line numberDiff line numberDiff line change
@@ -1163,22 +1163,21 @@ object RefChecks {
11631163
* that are either both opaque types or both not.
11641164
*/
11651165
def checkExtensionMethods(sym: Symbol)(using Context): Unit =
1166-
if sym.is(Extension) && !sym.nextOverriddenSymbol.exists then
1166+
if sym.is(Extension) then
11671167
extension (tp: Type)
1168-
def strippedResultType = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true).resultType
1169-
def firstExplicitParamTypes = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true).firstParamTypes
1168+
def explicit = Applications.stripImplicit(tp.stripPoly, wildcardOnly = true)
11701169
def hasImplicitParams = tp.stripPoly match { case mt: MethodType => mt.isImplicitMethod case _ => false }
1171-
val target = sym.info.firstExplicitParamTypes.head // required for extension method, the putative receiver
1172-
val methTp = sym.info.strippedResultType // skip leading implicits and the "receiver" parameter
1170+
val explicitInfo = sym.info.explicit // consider explicit value params
1171+
val target = explicitInfo.firstParamTypes.head // required for extension method, the putative receiver
1172+
val methTp = explicitInfo.resultType // skip leading implicits and the "receiver" parameter
11731173
def hidden =
11741174
target.nonPrivateMember(sym.name)
1175-
.filterWithPredicate:
1176-
member =>
1175+
.filterWithPredicate: member =>
11771176
member.symbol.isPublic && {
11781177
val memberIsImplicit = member.info.hasImplicitParams
11791178
val paramTps =
11801179
if memberIsImplicit then methTp.stripPoly.firstParamTypes
1181-
else methTp.firstExplicitParamTypes
1180+
else methTp.explicit.firstParamTypes
11821181

11831182
paramTps.isEmpty || memberIsImplicit && !methTp.hasImplicitParams || {
11841183
val memberParamTps = member.info.stripPoly.firstParamTypes
@@ -1190,7 +1189,15 @@ object RefChecks {
11901189
}
11911190
}
11921191
.exists
1193-
if !target.typeSymbol.isOpaqueAlias && hidden
1192+
if sym.is(HasDefaultParams) then
1193+
val getterDenot =
1194+
val receiverName = explicitInfo.firstParamNames.head
1195+
val num = sym.info.paramNamess.flatten.indexWhere(_ == receiverName)
1196+
val getterName = DefaultGetterName(sym.name.toTermName, num = num)
1197+
sym.owner.info.member(getterName)
1198+
if getterDenot.exists
1199+
then report.warning(ExtensionHasDefault(sym), getterDenot.symbol.srcPos)
1200+
if !target.typeSymbol.isOpaqueAlias && !sym.nextOverriddenSymbol.exists && hidden
11941201
then report.warning(ExtensionNullifiedByMember(sym, target.typeSymbol), sym.srcPos)
11951202
end checkExtensionMethods
11961203

Diff for: tests/warn/i12460.check

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
-- [E208] Potential Issue Warning: tests/warn/i12460.scala:3:23 --------------------------------------------------------
2+
3 |extension (s: String = "hello, world") def invert = s.reverse.toUpperCase // warn
3+
| ^
4+
| Extension method invert should not have a default argument for its receiver.
5+
|---------------------------------------------------------------------------------------------------------------------
6+
| Explanation (enabled by `-explain`)
7+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
8+
| The receiver cannot be omitted when an extension method is invoked as a selection.
9+
| A default argument for that parameter would never be used in that case.
10+
| An extension method can be invoked as a regular method, but if that is the intended usage,
11+
| it should not be defined as an extension.
12+
---------------------------------------------------------------------------------------------------------------------
13+
-- [E208] Potential Issue Warning: tests/warn/i12460.scala:5:37 --------------------------------------------------------
14+
5 |extension (using String)(s: String = "hello, world") def revert = s.reverse.toUpperCase // warn
15+
| ^
16+
| Extension method revert should not have a default argument for its receiver.
17+
|---------------------------------------------------------------------------------------------------------------------
18+
| Explanation (enabled by `-explain`)
19+
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
20+
| The receiver cannot be omitted when an extension method is invoked as a selection.
21+
| A default argument for that parameter would never be used in that case.
22+
| An extension method can be invoked as a regular method, but if that is the intended usage,
23+
| it should not be defined as an extension.
24+
---------------------------------------------------------------------------------------------------------------------

Diff for: tests/warn/i12460.scala

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
//> using options -explain -Ystop-after:refchecks
2+
3+
extension (s: String = "hello, world") def invert = s.reverse.toUpperCase // warn
4+
5+
extension (using String)(s: String = "hello, world") def revert = s.reverse.toUpperCase // warn
6+
7+
extension (s: String)
8+
def divert(m: String = "hello, world") = (s+m).reverse.toUpperCase // ok
9+
def divertimento(using String)(m: String = "hello, world") = (s+m).reverse.toUpperCase // ok

0 commit comments

Comments
 (0)