From c521daeb737283783efb0b9fea9ce219ff1594ab Mon Sep 17 00:00:00 2001 From: Som Snytt Date: Fri, 19 May 2023 15:47:03 -0700 Subject: [PATCH] Warn if extension receiver has matching member --- .../tools/dotc/core/SymDenotations.scala | 1 + .../tools/dotc/reporting/ErrorMessageID.scala | 1 + .../dotty/tools/dotc/reporting/messages.scala | 9 +++++++ .../src/dotty/tools/dotc/typer/Typer.scala | 9 ++++++- .../tools/dotc/printing/PrintingTest.scala | 2 +- .../dotty/tools/scripting/ScriptTestEnv.scala | 1 + .../src/tests/implicitConversions.scala | 4 ++-- .../src/tests/inheritedMembers1.scala | 1 + .../fatal-warnings/i9241.scala | 1 + tests/neg/i16743.check | 11 +++++++++ tests/neg/i16743.scala | 24 +++++++++++++++++++ 11 files changed, 60 insertions(+), 4 deletions(-) create mode 100644 tests/neg/i16743.check create mode 100644 tests/neg/i16743.scala diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index aa97435d64bb..211ff4985b69 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -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) diff --git a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala index fc679210db17..f2bab9efd2ee 100644 --- a/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala +++ b/compiler/src/dotty/tools/dotc/reporting/ErrorMessageID.scala @@ -195,6 +195,7 @@ enum ErrorMessageID(val isActive: Boolean = true) extends java.lang.Enum[ErrorMe case MatchTypeScrutineeCannotBeHigherKindedID // errorNumber: 179 case AmbiguousExtensionMethodID // errorNumber 180 case UnqualifiedCallToAnyRefMethodID // errorNumber: 181 + case ExtensionNullifiedByMemberID // errorNumber: 182 def errorNumber = ordinal - 1 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index d205b816214c..d39a14b97419 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -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" diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala index 9a9e194c9e0b..71e449eea1e2 100644 --- a/compiler/src/dotty/tools/dotc/typer/Typer.scala +++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala @@ -2486,7 +2486,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)) diff --git a/compiler/test/dotty/tools/dotc/printing/PrintingTest.scala b/compiler/test/dotty/tools/dotc/printing/PrintingTest.scala index 2c970e93f573..63ee5c54fab7 100644 --- a/compiler/test/dotty/tools/dotc/printing/PrintingTest.scala +++ b/compiler/test/dotty/tools/dotc/printing/PrintingTest.scala @@ -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") diff --git a/compiler/test/dotty/tools/scripting/ScriptTestEnv.scala b/compiler/test/dotty/tools/scripting/ScriptTestEnv.scala index ebae5bfca6be..f4bd1df03183 100644 --- a/compiler/test/dotty/tools/scripting/ScriptTestEnv.scala +++ b/compiler/test/dotty/tools/scripting/ScriptTestEnv.scala @@ -218,6 +218,7 @@ object ScriptTestEnv { def toUrl: String = Paths.get(absPath).toUri.toURL.toString // Treat norm paths with a leading '/' as absolute (Windows java.io.File#isAbsolute treats them as relative) + @annotation.nowarn // hidden by Path#isAbsolute? def isAbsolute = p.norm.startsWith("/") || (isWin && p.norm.secondChar == ":") } diff --git a/scaladoc-testcases/src/tests/implicitConversions.scala b/scaladoc-testcases/src/tests/implicitConversions.scala index 720eab1ccb1a..f12b8ad11801 100644 --- a/scaladoc-testcases/src/tests/implicitConversions.scala +++ b/scaladoc-testcases/src/tests/implicitConversions.scala @@ -45,7 +45,7 @@ class B { class C { def extensionInCompanion: String = ??? } - +@annotation.nowarn // extensionInCompanion object C { implicit def companionConversion(c: C): B = ??? @@ -70,4 +70,4 @@ package nested { } class Z -} \ No newline at end of file +} diff --git a/scaladoc-testcases/src/tests/inheritedMembers1.scala b/scaladoc-testcases/src/tests/inheritedMembers1.scala index d8fa44607e5e..561e50ceaec2 100644 --- a/scaladoc-testcases/src/tests/inheritedMembers1.scala +++ b/scaladoc-testcases/src/tests/inheritedMembers1.scala @@ -2,6 +2,7 @@ package tests package inheritedMembers1 +/*<-*/@annotation.nowarn/*->*/ class A { def A: String diff --git a/tests/neg-custom-args/fatal-warnings/i9241.scala b/tests/neg-custom-args/fatal-warnings/i9241.scala index d3be9bc9278d..a1b29c2fe403 100644 --- a/tests/neg-custom-args/fatal-warnings/i9241.scala +++ b/tests/neg-custom-args/fatal-warnings/i9241.scala @@ -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 diff --git a/tests/neg/i16743.check b/tests/neg/i16743.check new file mode 100644 index 000000000000..b4e9c7c17eee --- /dev/null +++ b/tests/neg/i16743.check @@ -0,0 +1,11 @@ +-- [E182] 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. + --------------------------------------------------------------------------------------------------------------------- diff --git a/tests/neg/i16743.scala b/tests/neg/i16743.scala new file mode 100644 index 000000000000..b0008341e53e --- /dev/null +++ b/tests/neg/i16743.scala @@ -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