Skip to content

Commit

Permalink
Tweak ExtensionNullifiedByMember (#22268)
Browse files Browse the repository at this point in the history
The warning accounted for an opaque receiver but not for opaque
parameter types.

This commit warns only if corresponding parameters are both opaque (or
both transparent).

The warning message about extensions that will _never_ be selected has
limited generality.

This commit addresses conversions to the receiver type.

Maybe the correct fix is "never say never".

Enhances the message noticed at #22267
Fixes #22279
  • Loading branch information
sjrd authored Jan 6, 2025
2 parents 1448123 + aa44a3c commit d16453e
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 17 deletions.
11 changes: 7 additions & 4 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2504,12 +2504,15 @@ class ExtensionNullifiedByMember(method: Symbol, target: Symbol)(using Context)
extends Message(ExtensionNullifiedByMemberID):
def kind = MessageKind.PotentialIssue
def msg(using Context) =
i"""Extension method ${hl(method.name.toString)} will never be selected
|because ${hl(target.name.toString)} already has a member with the same name and compatible parameter types."""
val targetName = hl(target.name.toString)
i"""Extension method ${hl(method.name.toString)} will never be selected from type $targetName
|because $targetName already has a member with the same name and compatible parameter types."""
def explain(using Context) =
i"""An extension method can be invoked as a regular method, but if that is intended,
i"""Although extensions can be overloaded, they do not overload existing member methods.
|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.
|Although extensions can be overloaded, they do not overload existing member methods."""
|
|The extension may be invoked as though selected from an arbitrary type if conversions are in play."""

class TraitCompanionWithMutableStatic()(using Context)
extends SyntaxMsg(TraitCompanionWithMutableStaticID) {
Expand Down
9 changes: 8 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1156,6 +1156,11 @@ object RefChecks {
*
* If the extension method is nullary, it is always hidden by a member of the same name.
* (Either the member is nullary, or the reference is taken as the eta-expansion of the member.)
*
* This check is in lieu of a more expensive use-site check that an application failed to use an extension.
* That check would account for accessibility and opacity. As a limitation, this check considers
* only public members, a target receiver that is not an alias, and corresponding method parameters
* that are either both opaque types or both not.
*/
def checkExtensionMethods(sym: Symbol)(using Context): Unit =
if sym.is(Extension) && !sym.nextOverriddenSymbol.exists then
Expand All @@ -1179,7 +1184,9 @@ object RefChecks {
val memberParamTps = member.info.stripPoly.firstParamTypes
!memberParamTps.isEmpty
&& memberParamTps.lengthCompare(paramTps) == 0
&& memberParamTps.lazyZip(paramTps).forall((m, x) => x frozen_<:< m)
&& memberParamTps.lazyZip(paramTps).forall: (m, x) =>
m.typeSymbol.denot.isOpaqueAlias == x.typeSymbol.denot.isOpaqueAlias
&& (x frozen_<:< m)
}
}
.exists
Expand Down
24 changes: 12 additions & 12 deletions tests/warn/i16743.check
Original file line number Diff line number Diff line change
@@ -1,84 +1,84 @@
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:30:6 --------------------------------------------------------
30 | def t = 27 // warn
| ^
| Extension method t will never be selected
| Extension method t will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:32:6 --------------------------------------------------------
32 | def g(x: String)(i: Int): String = x*i // warn
| ^
| Extension method g will never be selected
| Extension method g will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:33:6 --------------------------------------------------------
33 | def h(x: String): String = x // warn
| ^
| Extension method h will never be selected
| Extension method h will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:35:6 --------------------------------------------------------
35 | def j(x: Any, y: Int): String = (x.toString)*y // warn
| ^
| Extension method j will never be selected
| Extension method j will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:36:6 --------------------------------------------------------
36 | def k(x: String): String = x // warn
| ^
| Extension method k will never be selected
| Extension method k will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:38:6 --------------------------------------------------------
38 | def m(using String): String = "m" + summon[String] // warn
| ^
| Extension method m will never be selected
| Extension method m will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:39:6 --------------------------------------------------------
39 | def n(using String): String = "n" + summon[String] // warn
| ^
| Extension method n will never be selected
| Extension method n will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:40:6 --------------------------------------------------------
40 | def o: String = "42" // warn
| ^
| Extension method o will never be selected
| Extension method o will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:41:6 --------------------------------------------------------
41 | def u: Int = 27 // warn
| ^
| Extension method u will never be selected
| Extension method u will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:44:6 --------------------------------------------------------
44 | def at: Int = 42 // warn
| ^
| Extension method at will never be selected
| Extension method at will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:46:6 --------------------------------------------------------
46 | def x(using String)(n: Int): Int = summon[String].toInt + n // warn
| ^
| Extension method x will never be selected
| Extension method x will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
-- [E194] Potential Issue Warning: tests/warn/i16743.scala:47:6 --------------------------------------------------------
47 | def y(using String)(s: String): String = s + summon[String] // warn
| ^
| Extension method y will never be selected
| Extension method y will never be selected from type T
| because T already has a member with the same name and compatible parameter types.
|
| longer explanation available when compiling with `-explain`
14 changes: 14 additions & 0 deletions tests/warn/i22267.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
-- [E194] Potential Issue Warning: tests/warn/i22267.scala:13:26 -------------------------------------------------------
13 | extension (self: C) def m(n: Double): Unit = println(2->n) // warn
| ^
| Extension method m will never be selected from type C
| because C already has a member with the same name and compatible parameter types.
|--------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Although extensions can be overloaded, they do not overload existing member methods.
| 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.
|
| The extension may be invoked as though selected from an arbitrary type if conversions are in play.
--------------------------------------------------------------------------------------------------------------------
28 changes: 28 additions & 0 deletions tests/warn/i22267.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
//> using options -explain

import language.implicitConversions

case class M[T](value: T)
given [T]: Conversion[M[T], T] = _.value
class C:
def m(n: Double): Unit = println(0->n)
object C:
given Ops = Ops()
class Ops:
extension (self: C) def m(n: Int): Unit = println(1->n)
extension (self: C) def m(n: Double): Unit = println(2->n) // warn
extension (self: C) def m(s: String): Unit = println(3->s)

@main def test() =
val c = M(C())
def i = 42
def pi = 3.14
c.value.m(i)
c.value.m(pi)
c.m(i) // conversion
c.m(pi) // conversion
c.m("hello, world") // extension
//m(c)(pi)
val c0 = C()
c0.m(pi)
c0.m("hello, world")
24 changes: 24 additions & 0 deletions tests/warn/i22279.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@

import scala.io.Source

object Lengths:
opaque type Length = Int
object Length:
def apply(i: Int): Length = i
extension (source: Source)
def take(length: Length): IndexedSeq[Char] = // no warn
source.take(length).to(IndexedSeq)
end Lengths

trait Taken:
def take(n: Lengths.Length): Taken = ???

object Lengthy:
import Lengths.*
extension (taken: Taken) def take(n: Length): Taken = ??? // warn

@main def test() = println:
import Lengths.*
val src = Source.fromString("hello, world")
val len = Length("hello".length)
src.take(len)

0 comments on commit d16453e

Please sign in to comment.