Skip to content

Discourage default arg for extension receiver #22492

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Feb 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case GivenSearchPriorityID // errorNumber: 205
case EnumMayNotBeValueClassesID // errorNumber: 206
case IllegalUnrollPlacementID // errorNumber: 207
case ExtensionHasDefaultID // errorNumber: 208

def errorNumber = ordinal - 1

Expand Down
11 changes: 11 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2514,6 +2514,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"
Expand Down
25 changes: 16 additions & 9 deletions compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1163,22 +1163,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
Expand All @@ -1190,7 +1189,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

Expand Down
24 changes: 24 additions & 0 deletions tests/warn/i12460.check
Original file line number Diff line number Diff line change
@@ -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.
---------------------------------------------------------------------------------------------------------------------
9 changes: 9 additions & 0 deletions tests/warn/i12460.scala
Original file line number Diff line number Diff line change
@@ -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
Loading