Merge pull request #10395 from som-snytt/backport/refcheck-predef

Avoid printing Predef module as type

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

@@ -5567,6 +5567,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
             }
             else {
               val pre1  = if (sym.isTopLevel) sym.owner.thisType else if (qual == EmptyTree) NoPrefix else qual.tpe
+              if (settings.lintUniversalMethods && !pre1.isInstanceOf[ThisType] && isUniversalMember(sym))
+                context.warning(tree.pos, s"${sym.nameString} not selected from this instance", WarningCategory.LintUniversalMethods)
               val tree1 = if (qual == EmptyTree) tree else {
                 val pos = tree.pos
                 Select(atPos(pos.focusStart)(qual), name).setPos(pos)

src/reflect/scala/reflect/internal/Definitions.scala

@@ -251,7 +251,9 @@ trait Definitions extends api.StandardDefinitions {
       scope
     }
     /** Is this symbol a member of Object or Any? */
-    def isUniversalMember(sym: Symbol) = ObjectClass isSubClass sym.owner
+    def isUniversalMember(sym: Symbol) =
+      if (sym.isOverloaded) sym.alternatives.exists(alt => ObjectClass.isSubClass(alt.owner))
+      else ObjectClass.isSubClass(sym.owner)
 
     /** Is this symbol unimportable? Unimportable symbols include:
      *  - constructors, because <init> is not a real name
@@ -1269,6 +1271,7 @@ trait Definitions extends api.StandardDefinitions {
     def Object_finalize  = getMemberMethod(ObjectClass, nme.finalize_)
     def Object_notify    = getMemberMethod(ObjectClass, nme.notify_)
     def Object_notifyAll = getMemberMethod(ObjectClass, nme.notifyAll_)
+    def Object_wait      = getMemberMethod(ObjectClass, nme.wait_)
     def Object_equals    = getMemberMethod(ObjectClass, nme.equals_)
     def Object_hashCode  = getMemberMethod(ObjectClass, nme.hashCode_)
     def Object_toString  = getMemberMethod(ObjectClass, nme.toString_)

src/reflect/scala/reflect/internal/Types.scala

@@ -1102,13 +1102,13 @@ trait Types
 // Spec: "The base types of a singleton type `$p$.type` are the base types of the type of $p$."
 //    override def baseTypeSeq: BaseTypeSeq = underlying.baseTypeSeq
     override def isHigherKinded = false // singleton type classifies objects, thus must be kind *
-    override def safeToString: String = {
-      // Avoiding printing Predef.type and scala.package.type as "type",
-      // since in all other cases we omit those prefixes.
-      val sym = termSymbol.skipPackageObject
-      if (sym.isOmittablePrefix) sym.fullName + ".type"
-      else prefixString + "type"
-    }
+    // Avoid printing Predef.type and scala.package.type as "type",
+    // since in all other cases we omit those prefixes. Do not skipPackageObject.
+    override def safeToString: String =
+      termSymbol match {
+        case s if s.isOmittablePrefix => s"${if (s.isPackageObjectOrClass || s.isJavaDefined) s.fullNameString else s.nameString}.type"
+        case _ => s"${prefixString}type"
+      }
   }
 
   /** An object representing an erroneous type */
@@ -2135,7 +2135,9 @@ trait Types
     override protected def finishPrefix(rest: String) = objectPrefix + rest
     override def directObjectString = super.safeToString
     override def toLongString = toString
-    override def safeToString = prefixString + "type"
+    override def safeToString =
+      if (sym.isOmittablePrefix) s"${if (sym.isPackageObjectOrClass || sym.isJavaDefined) sym.fullNameString else sym.nameString}.type"
+      else s"${prefixString}type"
     override def prefixString = if (sym.isOmittablePrefix) "" else prefix.prefixString + sym.nameString + "."
   }
   class PackageTypeRef(pre0: Type, sym0: Symbol) extends ModuleTypeRef(pre0, sym0) {

src/reflect/scala/reflect/runtime/JavaMirrors.scala

@@ -352,9 +352,10 @@ private[scala] trait JavaMirrors extends internal.SymbolTable with api.JavaUnive
     // because both AnyVal and its primitive descendants define their own getClass methods
     private def isGetClass(meth: MethodSymbol) = (meth.name string_== "getClass") && meth.paramss.flatten.isEmpty
     private def isStringConcat(meth: MethodSymbol) = meth == String_+ || (meth.owner.isPrimitiveValueClass && meth.returnType =:= StringClass.toType)
-    lazy val bytecodelessMethodOwners = Set[Symbol](AnyClass, AnyValClass, AnyRefClass, ObjectClass, ArrayClass) ++ ScalaPrimitiveValueClasses
-    lazy val bytecodefulObjectMethods = Set[Symbol](Object_clone, Object_equals, Object_finalize, Object_hashCode, Object_toString,
-                                        Object_notify, Object_notifyAll) ++ ObjectClass.info.member(nme.wait_).asTerm.alternatives.map(_.asMethod)
+    lazy val bytecodelessMethodOwners =
+      Set[Symbol](AnyClass, AnyValClass, AnyRefClass, ObjectClass, ArrayClass) ++ ScalaPrimitiveValueClasses
+    lazy val bytecodefulObjectMethods =
+      Set[Symbol](Object_clone, Object_equals, Object_finalize, Object_hashCode, Object_toString, Object_notify, Object_notifyAll) ++ Object_wait.alternatives
     private def isBytecodelessMethod(meth: MethodSymbol): Boolean = {
       if (isGetClass(meth) || isStringConcat(meth) || meth.owner.isPrimitiveValueClass || meth == runDefinitions.Predef_classOf || meth.isMacro) return true
       bytecodelessMethodOwners(meth.owner) && !bytecodefulObjectMethods(meth)

test/files/neg/i17266.check

@@ -0,0 +1,15 @@
+i17266.scala:13: warning: synchronized not selected from this instance
+    synchronized { // error
+    ^
+i17266.scala:26: warning: wait not selected from this instance
+    wait() // error
+    ^
+i17266.scala:32: warning: notify not selected from this instance
+  def `maybe notify`(): Unit = notify()
+                               ^
+i17266.scala:33: warning: notifyAll not selected from this instance
+  def `maybe notifyAll`(): Unit = notifyAll()
+                                  ^
+error: No warnings can be incurred under -Werror.
+4 warnings
+1 error

test/files/neg/i17266.scala

@@ -0,0 +1,200 @@
+
+// scalac: -Werror -Xsource:3 -Xlint:universal-methods
+
+// Dotty has top-level defs, so the reference is linted based on context.
+// For Scala 2, check result of looking up the identifier.
+// Universal members are not imported from root contexts (in particular, Predef).
+// Use an explicit import to exercise the warning.
+
+class Test(val x: Any) extends AnyVal {
+  import Predef.*
+
+  def test1 =
+    synchronized { // error
+      println("hello")
+    }
+
+  /* correctly errors in Scala 2
+  def test2 =
+    this.synchronized { // not an error (should be?)
+      println("hello")
+    }
+  */
+
+  // surprise, ~not~ a universal member
+  def test16 =
+    wait() // error
+
+  // OK because Any, so this is kosher
+  def `maybe hashcode` = hashCode
+
+  // it does know about notify
+  def `maybe notify`(): Unit = notify()
+  def `maybe notifyAll`(): Unit = notifyAll()
+
+}
+
+// Can't work these tests inside value class.
+//
+class ObjectHolder {
+
+  object MyLib
+
+  /* ambiguous
+  def test3 = {
+    import MyLib.*
+    synchronized { // error
+      println("hello")
+    }
+  }
+  */
+
+  def test4 =
+    1.synchronized { // not an error (should be?)
+      println("hello")
+    }
+
+  object Test4 {
+    synchronized { // not an error
+      println("hello")
+    }
+  }
+
+  object Test5 {
+    def test5 =
+      synchronized { // not an error
+        println("hello")
+      }
+  }
+
+  object Test6 {
+    import MyLib.*
+    synchronized { // not an error
+      println("hello")
+    }
+  }
+
+  object Test7 {
+    import MyLib.*
+    def test7 =
+      synchronized { // not an error
+        println("hello")
+      }
+  }
+
+  /*
+  object Test7b {
+    def test8 =
+      import MyLib.*
+      synchronized { // already an error: Reference to synchronized is ambiguous.
+        println("hello")
+      }
+  }
+  */
+
+  class Test8 {
+    synchronized { // not an error
+      println("hello")
+    }
+  }
+
+  class Test9 {
+    def test5 =
+      synchronized { // not an error
+        println("hello")
+      }
+  }
+
+  class Test10 {
+    import MyLib.*
+    synchronized { // not an error
+      println("hello")
+    }
+  }
+
+  class Test11 {
+    import MyLib.*
+    def test7 =
+      synchronized { // not an error
+        println("hello")
+      }
+  }
+
+  trait Test12 {
+    synchronized { // not an error
+      println("hello")
+    }
+  }
+
+  trait Test13 {
+    def test5 =
+      synchronized { // not an error
+        println("hello")
+      }
+  }
+
+  trait Test14 {
+    import MyLib.*
+    synchronized { // not an error
+      println("hello")
+    }
+  }
+
+  trait Test15 {
+    import MyLib.*
+    def test7 =
+      synchronized { // not an error
+        println("hello")
+      }
+  }
+
+  def test16 =
+    wait() // error
+
+  def test17 =
+    this.wait() // not an error (should be?)
+
+  /* ambiguous
+  def test18 = {
+    import MyLib.*
+    wait() // error
+  }
+  */
+
+  def test19 =
+    1.wait() // not an error (should be?)
+
+  /* ambiguous
+  def test20 =
+    wait(10) // error
+  */
+
+  def test21 =
+    this.wait(10) // not an error (should be?)
+
+  /* ambiguous
+  def test22 = {
+    import MyLib.*
+    wait(10) // error
+  }
+  */
+
+  def test23 =
+    1.wait(10) // not an error (should be?)
+
+  def test24 =
+    hashCode() // error
+
+  def test25 =
+    this.hashCode() // not an error (should be?)
+
+  /* ambiguous
+  def test26 = {
+    import MyLib.*
+    hashCode() // error
+  }
+  */
+
+  def test27 =
+    1.hashCode()// not an error (should be? probably not)
+}

test/files/neg/i17266c.check

@@ -0,0 +1,36 @@
+i17266c.scala:7: warning: eq not selected from this instance
+  def f = eq("hello, world")
+          ^
+i17266c.scala:8: warning: synchronized not selected from this instance
+  def g = synchronized { println("hello, world") }
+          ^
+i17266c.scala:12: warning: eq not selected from this instance
+  def f = eq(s)
+          ^
+i17266c.scala:13: warning: synchronized not selected from this instance
+  def g = synchronized { println(s) }
+          ^
+i17266c.scala:18: warning: eq not selected from this instance
+  def f = eq(s)
+          ^
+i17266c.scala:19: warning: synchronized not selected from this instance
+  def g = synchronized { println(s) }
+          ^
+i17266c.scala:7: warning: comparing values of types X.type and String using `eq` will always yield false
+  def f = eq("hello, world")
+            ^
+i17266c.scala:12: warning: comparing values of types Predef.type and String using `eq` will always yield false
+  def f = eq(s)
+            ^
+i17266c.scala:18: warning: comparing values of types p.package.type and String using `eq` will always yield false
+  def f = eq(s)
+            ^
+i17266c.scala:22: warning: comparing values of types X.type and String using `eq` will always yield false
+  def f = X.eq("hello, world")
+              ^
+i17266c.scala:27: warning: Z and String are unrelated: they will most likely never compare equal
+  def f = eq("hello, world")
+            ^
+error: No warnings can be incurred under -Werror.
+11 warnings
+1 error

test/files/neg/i17266c.scala

@@ -0,0 +1,29 @@
+// scalac: -Werror -Xlint:universal-methods
+
+object X
+
+class A(val s: String) extends AnyVal {
+  import X._
+  def f = eq("hello, world")
+  def g = synchronized { println("hello, world") }
+}
+class B(val s: String) extends AnyVal {
+  import Predef._
+  def f = eq(s)
+  def g = synchronized { println(s) }
+}
+package object p
+class C(val s: String) extends AnyVal {
+  import p.`package`._
+  def f = eq(s)
+  def g = synchronized { println(s) }
+}
+class Y(val s: String) {
+  def f = X.eq("hello, world")
+  def g = X.synchronized { println("hello, world") }
+}
+class Z(val s: String) {
+  import X._
+  def f = eq("hello, world")
+  def g = synchronized { println("hello, world") }
+}

test/files/neg/t12785.check

@@ -0,0 +1,9 @@
+t12785.scala:6: warning: comparing values of types Predef.type and Array[B] using `eq` will always yield false
+  def startsWith[B >: A](that: Array[B]): Boolean = eq(that)
+                                                      ^
+t12785.scala:10: warning: comparing values of types scala.package.type and Array[B] using `eq` will always yield false
+    eq(that)
+      ^
+error: No warnings can be incurred under -Werror.
+2 warnings
+1 error

test/files/neg/t12785.scala

@@ -0,0 +1,14 @@
+// scalac: -Werror
+
+import scala.Predef._
+
+final class ArrayOps[A](private val xs: Array[A]) extends AnyVal {
+  def startsWith[B >: A](that: Array[B]): Boolean = eq(that)
+
+  def endsWith[B >: A](that: Array[B]): Boolean = {
+    import scala.`package`._
+    eq(that)
+  }
+}
+
+//warning: comparing values of types type and Array[B] using `eq` will always yield false