Merge pull request scala#11120 from som-snytt/issue/13111-dubious-overload-followup

Warn confusing member when contributing it

Author: lrytz

src/compiler/scala/tools/nsc/typechecker/RefChecks.scala

@@ -160,6 +160,8 @@ abstract class RefChecks extends Transform {
         })
       }
     }
+    // warn if clazz defines nullary member `m` where `def m(implicit t: T)` is also defined, or conversely.
+    // That is confusing because the expression `m` might invoke either member.
     private def checkDubiousOverloads(clazz: Symbol): Unit = if (settings.warnDubiousOverload) {
       // nullary members or methods with leading implicit params
       def ofInterest(tp: Type): Boolean = tp match {
@@ -173,6 +175,12 @@ abstract class RefChecks extends Transform {
         case PolyType(_, rt) => isNullary(rt)
         case _ => true // includes NullaryMethodType
       }
+      // if all the competing symbols are members of a base class of clazz, don't warn in clazz, since the base warns
+      def warnable(syms: List[Symbol]): Boolean =
+        !clazz.baseClasses.drop(1).exists { base =>
+          val members = base.info.members
+          syms.forall(sym => members.lookupSymbolEntry(sym) != null)
+        }
       def warnDubious(sym: Symbol, alts: List[Symbol]): Unit = {
         val usage = if (sym.isMethod && !sym.isGetter) "Calls to parameterless" else "Usages of"
         val simpl = "a single implicit parameter list"
@@ -205,27 +213,28 @@ abstract class RefChecks extends Transform {
             else isCompetitive(syms, sawNlly, sawNonNlly)
           case _ => false
         })
-      for ((_, syms) <- byName if syms.lengthCompare(1) > 0 && isCompetitive(syms, sawNlly=false, sawNonNlly=false)) {
-        val (nullaries, alts) = syms.partition(sym => isNullary(sym.info))
-        //assert(!alts.isEmpty)
-        nullaries match {
-          case nullary :: Nil => warnDubious(nullary, syms)
-          case nullaries =>
-            //assert(!nullaries.isEmpty)
-            val dealiased =
-              nullaries.find(_.isPrivateLocal) match {
-                case Some(local) =>
-                  nullaries.find(sym => sym.isAccessor && sym.accessed == local) match {
-                    case Some(accessor) => nullaries.filter(_ != local) // drop local if it has an accessor
-                    case _ => nullaries
-                  }
-                case _ => nullaries
-              }
-            // there are multiple exactly for a private local and an inherited member
-            for (nullary <- dealiased)
-              warnDubious(nullary, nullary :: alts)
+      for ((_, syms) <- byName)
+        if (syms.lengthCompare(1) > 0 && isCompetitive(syms, sawNlly = false, sawNonNlly = false) && warnable(syms)) {
+          val (nullaries, alts) = syms.partition(sym => isNullary(sym.info))
+          //assert(!alts.isEmpty)
+          nullaries match {
+            case nullary :: Nil => warnDubious(nullary, alts)
+            case nullaries =>
+              //assert(!nullaries.isEmpty)
+              val dealiased =
+                nullaries.find(_.isPrivateLocal) match {
+                  case Some(local) =>
+                    nullaries.find(sym => sym.isAccessor && sym.accessed == local) match {
+                      case Some(accessor) => nullaries.filter(_ != local) // drop local if it has an accessor
+                      case _ => nullaries
+                    }
+                  case _ => nullaries
+                }
+              // there are multiple exactly for a private local and an inherited member
+              for (nullary <- dealiased)
+                warnDubious(nullary, nullary :: alts) // add (other) nullary to list of alts to show as overloads
+          }
         }
-      }
     }
 
 // Override checking ------------------------------------------------------------

test/files/neg/t7415.check

@@ -7,32 +7,35 @@ t7415.scala:14: warning: Usages of value foo will be easy to mistake for calls t
 t7415.scala:18: warning: Usages of value foo will be easy to mistake for calls to def foo(implicit a: T): Int, which has a single implicit parameter list.
   private[this] val foo = 42 // warn
                     ^
-t7415.scala:31: warning: Calls to parameterless method foo will be easy to mistake for calls to def foo(implicit a: T): Int, which has a single implicit parameter list.
+t7415.scala:33: warning: Calls to parameterless method foo will be easy to mistake for calls to def foo(implicit a: T): Int, which has a single implicit parameter list.
 class Mixed extends Base with T1 // warn here
       ^
-t7415.scala:41: warning: Usages of value foo will be easy to mistake for calls to overloads which have a single implicit parameter list:
+t7415.scala:36: warning: Calls to parameterless method foo will be easy to mistake for calls to def foo(implicit a: T): Int, which has a single implicit parameter list.
+  def foo(implicit a: T) = 0 // warn although x.foo picks this one
+      ^
+t7415.scala:47: warning: Usages of value foo will be easy to mistake for calls to overloads which have a single implicit parameter list:
   def foo(implicit e: String): Int
   def foo(implicit e: Int): Int
   val foo = 0 // warn
       ^
-t7415.scala:54: warning: Usages of value x will be easy to mistake for calls to def x(implicit t: T): Int, which has a single implicit parameter list.
+t7415.scala:60: warning: Usages of value x will be easy to mistake for calls to def x(implicit t: T): Int, which has a single implicit parameter list.
   def x(implicit t: T) = 27 // warn
       ^
-t7415.scala:65: warning: Usages of value i will be easy to mistake for calls to def i(implicit t: T): Int, which has a single implicit parameter list.
+t7415.scala:71: warning: Usages of value i will be easy to mistake for calls to def i(implicit t: T): Int, which has a single implicit parameter list.
 class R(val i: Int) extends Q // warn
             ^
-t7415.scala:66: warning: Usages of value i will be easy to mistake for calls to def i(implicit t: T): Int, which has a single implicit parameter list.
+t7415.scala:72: warning: Usages of value i will be easy to mistake for calls to def i(implicit t: T): Int, which has a single implicit parameter list.
 class S(i: Int) extends R(i) { // warn
       ^
-t7415.scala:66: warning: Usages of value i will be easy to mistake for calls to def i(implicit t: T): Int, which has a single implicit parameter list.
+t7415.scala:72: warning: Usages of value i will be easy to mistake for calls to def i(implicit t: T): Int, which has a single implicit parameter list.
 class S(i: Int) extends R(i) { // warn
         ^
-t7415.scala:76: warning: Calls to parameterless method f will be easy to mistake for calls to def f[A](implicit t: T): Int, which has a single implicit parameter list.
+t7415.scala:82: warning: Calls to parameterless method f will be easy to mistake for calls to def f[A](implicit t: T): Int, which has a single implicit parameter list.
   def f[A] = 27 // warn
       ^
-t7415.scala:82: warning: Calls to parameterless method foo will be easy to mistake for calls to def foo(implicit a: T): Int, which has a single implicit parameter list.
-  val d1 = new Derived1 {} // warn
-               ^
+t7415.scala:88: warning: Usages of value an will be easy to mistake for calls to def an[A](implicit evidence$1: scala.reflect.ClassTag[A]): Unit, which has a single implicit parameter list.
+  val an = new Object // warn
+      ^
 error: No warnings can be incurred under -Werror.
-11 warnings
+12 warnings
 1 error

test/files/neg/t7415.scala

@@ -24,12 +24,18 @@ class C2 extends Derived2 {
 }
 */
 
+class Derived extends Derived2 // no warn, all foo are already members of Derived2
+
 trait T1 {
   def foo = 0
 }
 
 class Mixed extends Base with T1 // warn here
 
+class Inverted extends T1 {
+  def foo(implicit a: T) = 0 // warn although x.foo picks this one
+}
+
 class D {
   def foo(a: List[Int])(implicit d: DummyImplicit) = 0
   def foo(a: List[String]) = 1
@@ -77,9 +83,15 @@ trait PDerived extends PBase {
   def g[A] = f[A] // no warn
 }
 
+trait Matchers {
+  def an[A: reflect.ClassTag] = ()
+  val an = new Object // warn
+}
+object InnocentTest extends Matchers
+
 object Test extends App {
   implicit val t: T = new T {}
-  val d1 = new Derived1 {} // warn
+  val d1 = new Derived1 {} // no warn innocent client, already warned in evil parent
   println(d1.foo) // !
   val more = new MoreInspiration
   println(more.foo) // ?