Skip to content

Commit

Permalink
Warn if extension receiver has matching member
Browse files Browse the repository at this point in the history
  • Loading branch information
som-snytt committed Jun 2, 2023
1 parent 8814760 commit 35f27ec
Show file tree
Hide file tree
Showing 11 changed files with 62 additions and 5 deletions.
1 change: 1 addition & 0 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1348,6 +1348,7 @@ object SymDenotations {
* inClass <-- find denot.symbol class C { <-- symbol is here
*
* site: Subtype of both inClass and C
* } <-- balance the brace
*/
final def matchingDecl(inClass: Symbol, site: Type)(using Context): Symbol = {
var denot = inClass.info.nonPrivateDecl(name)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe
case AmbiguousExtensionMethodID // errorNumber 180
case UnqualifiedCallToAnyRefMethodID // errorNumber: 181
case NotConstantID // errorNumber: 182
case ExtensionNullifiedByMemberID // errorNumber: 183

def errorNumber = ordinal - 1

Expand Down
9 changes: 9 additions & 0 deletions compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2298,6 +2298,15 @@ class UnqualifiedCallToAnyRefMethod(stat: untpd.Tree, method: Symbol)(using Cont
|you intended."""
}

class ExtensionNullifiedByMember(method: Symbol, target: Symbol)(using Context)
extends Message(ExtensionNullifiedByMemberID):
def kind = MessageKind.PotentialIssue
def msg(using Context) = i"Suspicious extension ${hl(method.name.toString)} is already a member of ${hl(target.name.toString)}"
def explain(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.
|It can be called as a regular method, but should probably be defined that way."""

class TraitCompanionWithMutableStatic()(using Context)
extends SyntaxMsg(TraitCompanionWithMutableStaticID) {
def msg(using Context) = i"Companion of traits cannot define mutable @static fields"
Expand Down
9 changes: 8 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2492,7 +2492,14 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
linkConstructorParams(sym, tparamSyms, rhsCtx)

if sym.isInlineMethod then rhsCtx.addMode(Mode.InlineableBody)
if sym.is(ExtensionMethod) then rhsCtx.addMode(Mode.InExtensionMethod)
if sym.is(ExtensionMethod) then
if ctx.phase.isTyper then
val ValDef(_, paramTpt, _) = termParamssIn(paramss1).head.head
if !paramTpt.symbol.denot.isAliasType && !paramTpt.symbol.denot.isOpaqueAlias then
val other = paramTpt.tpe.nonPrivateMember(name)
if other.exists && sym.denot.info.resultType.matches(other.info) then
report.warning(ExtensionNullifiedByMember(sym, paramTpt.symbol), ddef.srcPos)
rhsCtx.addMode(Mode.InExtensionMethod)
val rhs1 = PrepareInlineable.dropInlineIfError(sym,
if sym.isScala2Macro then typedScala2MacroBody(ddef.rhs)(using rhsCtx)
else typedExpr(ddef.rhs, tpt1.tpe.widenExpr)(using rhsCtx))
Expand Down
2 changes: 1 addition & 1 deletion compiler/test/dotty/tools/dotc/printing/PrintingTest.scala
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import java.io.File
class PrintingTest {

def options(phase: String, flags: List[String]) =
List(s"-Xprint:$phase", "-color:never", "-classpath", TestConfiguration.basicClasspath) ::: flags
List(s"-Xprint:$phase", "-color:never", "-nowarn", "-classpath", TestConfiguration.basicClasspath) ::: flags

private def compileFile(path: JPath, phase: String): Boolean = {
val baseFilePath = path.toString.stripSuffix(".scala")
Expand Down
4 changes: 3 additions & 1 deletion compiler/test/dotty/tools/scripting/ScriptTestEnv.scala
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,10 @@ object ScriptTestEnv {

def toUrl: String = Paths.get(absPath).toUri.toURL.toString

// Used to be an extension on String
// Treat norm paths with a leading '/' as absolute (Windows java.io.File#isAbsolute treats them as relative)
def isAbsolute = p.norm.startsWith("/") || (isWin && p.norm.secondChar == ":")
//@annotation.nowarn // hidden by Path#isAbsolute
//def isAbsolute = p.norm.startsWith("/") || (isWin && p.norm.secondChar == ":")
}

extension(f: File) {
Expand Down
4 changes: 2 additions & 2 deletions scaladoc-testcases/src/tests/implicitConversions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class B {
class C {
def extensionInCompanion: String = ???
}

@annotation.nowarn // extensionInCompanion
object C {
implicit def companionConversion(c: C): B = ???

Expand All @@ -70,4 +70,4 @@ package nested {
}

class Z
}
}
1 change: 1 addition & 0 deletions scaladoc-testcases/src/tests/inheritedMembers1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package tests
package inheritedMembers1


/*<-*/@annotation.nowarn/*->*/
class A
{
def A: String
Expand Down
1 change: 1 addition & 0 deletions tests/neg-custom-args/fatal-warnings/i9241.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ final class Baz private (val x: Int) extends AnyVal {
}

extension (x: Int)
@annotation.nowarn
def unary_- : Int = ???
def unary_+[T] : Int = ???
def unary_!() : Int = ??? // error
Expand Down
11 changes: 11 additions & 0 deletions tests/neg/i16743.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
-- [E183] Potential Issue Error: tests/neg/i16743.scala:7:21 -----------------------------------------------------------
7 |extension (x: T) def t = 27 // error
| ^^^^^^^^^^
| Suspicious extension t is already a member of T
|---------------------------------------------------------------------------------------------------------------------
| Explanation (enabled by `-explain`)
|- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
| Extension method t will never be selected
| because T already has a member with the same name.
| It can be called as a regular method, but should probably be defined that way.
---------------------------------------------------------------------------------------------------------------------
24 changes: 24 additions & 0 deletions tests/neg/i16743.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@

// scalac: -Werror -explain

trait T:
def t = 42

extension (x: T) def t = 27 // error

@main def test() =
val x = new T {}
println:
x.t

trait Foo:
type X
extension (x: X) def t: Int

trait Bar extends Foo:
type X = T
extension (x: X) def t = x.t

opaque type IArray[+T] = Array[_ <: T]
object IArray:
extension (arr: IArray[Byte]) def length: Int = arr.asInstanceOf[Array[Byte]].length

0 comments on commit 35f27ec

Please sign in to comment.