Merge pull request #10220 from lrytz/pr10177

Inherited members no longer take precedence over outer definitions

spec/02-identifiers-names-and-scopes.md

@@ -7,7 +7,7 @@ chapter: 2
 # Identifiers, Names and Scopes
 
 Names in Scala identify types, values, methods, and classes which are
-collectively called _entities_. Names are introduced by local
+collectively called _entities_. Names are introduced by
 [definitions and declarations](04-basic-declarations-and-definitions.html#basic-declarations-and-definitions),
 [inheritance](05-classes-and-objects.html#class-members),
 [import clauses](04-basic-declarations-and-definitions.html#import-clauses), or
@@ -17,14 +17,16 @@ which are collectively called _bindings_.
 Bindings of each kind are assigned a precedence which determines
 whether one binding can shadow another:
 
-1. Definitions and declarations that are local, inherited, or made
-   available by a package clause and also defined in the same compilation unit
-   as the reference to them, have the highest precedence.
+1. Definitions and declarations in lexical scope that are not [top-level](09-top-level-definitions.html)
+   have the highest precedence.
+1. Definitions and declarations that are either inherited,
+   or made available by a package clause and also defined in the same compilation unit as the reference to them,
+   have the next highest precedence.
 1. Explicit imports have the next highest precedence.
 1. Wildcard imports have the next highest precedence.
-1. Bindings made available by a package clause, but not also defined in the
-   same compilation unit as the reference to them, as well as bindings
-   supplied by the compiler but not explicitly written in source code,
+1. Bindings made available by a package clause,
+   but not also defined in the same compilation unit as the reference to them,
+   as well as bindings supplied by the compiler but not explicitly written in source code,
    have the lowest precedence.
 
 There are two different name spaces, one for [types](03-types.html#types)

src/compiler/scala/tools/nsc/ast/parser/Parsers.scala

@@ -2098,7 +2098,7 @@ self =>
             if (in.token == SUBTYPE || in.token == SUPERTYPE) wildcardType(start, scala3Wildcard)
             else atPos(start) { Bind(tpnme.WILDCARD, EmptyTree) }
         } else {
-          typ() match {
+          this.typ() match {
             case Ident(name: TypeName) if nme.isVariableName(name) =>
               atPos(start) { Bind(name, EmptyTree) }
             case t => t
@@ -2289,7 +2289,7 @@ self =>
     }
     /** The implementation of the context sensitive methods for parsing outside of patterns. */
     final val outPattern = new PatternContextSensitive {
-      def argType(): Tree = typ()
+      def argType(): Tree = this.typ()
       def functionArgType(): Tree = paramType(repeatedParameterOK = false, useStartAsPosition = true)
     }
     /** The implementation for parsing inside of patterns at points where sequences are allowed. */

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

@@ -51,13 +51,6 @@ trait Contexts { self: Analyzer =>
   }
   private lazy val NoJavaMemberFound = (NoType, NoSymbol)
 
-  def ambiguousImports(imp1: ImportInfo, imp2: ImportInfo) =
-    LookupAmbiguous(s"it is imported twice in the same scope by\n$imp1\nand $imp2")
-  def ambiguousDefnAndImport(owner: Symbol, imp: ImportInfo) =
-    LookupAmbiguous(s"it is both defined in $owner and imported subsequently by \n$imp")
-  def ambiguousDefinitions(owner: Symbol, other: Symbol) =
-    LookupAmbiguous(s"it is both defined in $owner and available as ${other.fullLocationString}")
-
   private lazy val startContext = NoContext.make(
     Template(List(), noSelfType, List()) setSymbol global.NoSymbol setType global.NoType,
     rootMirror.RootClass,
@@ -1362,6 +1355,35 @@ trait Contexts { self: Analyzer =>
     private[this] var pre: Type                = _ // the prefix type of defSym, if a class member
     private[this] var cx: Context              = _ // the context under consideration
     private[this] var symbolDepth: Int         = _ // the depth of the directly found symbol
+    private[this] var foundInPrefix: Boolean   = _ // the symbol was found in pre
+    private[this] var foundInSuper: Boolean    = _ // the symbol was found super of context class (inherited)
+
+    def ambiguousImports(imp1: ImportInfo, imp2: ImportInfo) =
+      LookupAmbiguous(s"it is imported twice in the same scope by\n$imp1\nand $imp2")
+    def ambiguousDefnAndImport(owner: Symbol, imp: ImportInfo) =
+      LookupAmbiguous(s"it is both defined in $owner and imported subsequently by \n$imp")
+    def ambiguousDefinitions(sym: Symbol, other: Symbol, otherFoundInSuper: Boolean, otherEnclClass: Symbol) = {
+      if (otherFoundInSuper) {
+        val enclDesc = if (otherEnclClass.isAnonymousClass) "anonymous class" else otherEnclClass.toString
+        val parent = otherEnclClass.parentSymbols.find(_.isNonBottomSubClass(other.owner)).getOrElse(NoSymbol)
+        val inherit = if (parent.exists && parent != other.owner) s", inherited through parent $parent" else ""
+        val message =
+          s"""it is both defined in the enclosing ${sym.owner} and available in the enclosing $enclDesc as $other (defined in ${other.ownsString}$inherit)
+             |Since 3, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+             |To continue using the symbol from the superclass, write `this.${sym.name}`.""".stripMargin
+        if (currentRun.isScala3)
+          Some(LookupAmbiguous(message))
+        else {
+          // passing the message to `typedIdent` as attachment, we don't have the position here to report the warning
+          other.updateAttachment(LookupAmbiguityWarning(
+            s"""reference to ${sym.name} is ambiguous;
+               |$message
+               |Or use `-Wconf:msg=legacy-binding:s` to silence this warning.""".stripMargin))
+          None
+        }
+      } else
+        Some(LookupAmbiguous(s"it is both defined in ${sym.owner} and available as ${other.fullLocationString}"))
+    }
 
     def apply(thisContext: Context, name: Name)(qualifies: Symbol => Boolean): NameLookup = {
       lookupError  = null
@@ -1370,6 +1392,8 @@ trait Contexts { self: Analyzer =>
       pre          = NoPrefix
       cx           = thisContext
       symbolDepth  = -1
+      foundInPrefix = false
+      foundInSuper = false
 
       def finish(qual: Tree, sym: Symbol): NameLookup = (
         if (lookupError ne null) lookupError
@@ -1386,26 +1410,25 @@ trait Contexts { self: Analyzer =>
         finish(qual, sym)
       }
 
-      def lookupInPrefix(name: Name): Symbol = {
-        if (thisContext.unit.isJava) {
+      def lookupInPrefix(name: Name): Symbol =
+        if (thisContext.unit.isJava)
           thisContext.javaFindMember(pre, name, qualifies) match {
             case (_, NoSymbol) =>
               NoSymbol
             case (pre1, sym) =>
               pre = pre1
               sym
           }
-        } else {
+        else
           pre.member(name).filter(qualifies)
-        }
-      }
+
       def accessibleInPrefix(s: Symbol) =
         thisContext.isAccessible(s, pre, superAccess = false)
 
       def searchPrefix = {
         cx = cx.enclClass
         val found0 = lookupInPrefix(name)
-        val found1 = found0 filter accessibleInPrefix
+        val found1 = found0.filter(accessibleInPrefix)
         if (found0.exists && !found1.exists && inaccessible == null)
           inaccessible = LookupInaccessible(found0, analyzer.lastAccessCheckDetails)
 
@@ -1452,16 +1475,26 @@ trait Contexts { self: Analyzer =>
       }
 
       // cx.scope eq null arises during FixInvalidSyms in Duplicators
-      def nextDefinition(): Unit =
+      def nextDefinition(lastDef: Symbol, lastPre: Type): Unit = {
+        var inPrefix = false
+        defSym = NoSymbol
         while (defSym == NoSymbol && (cx ne NoContext) && (cx.scope ne null)) {
           pre    = cx.enclClass.prefix
-          defSym = lookupInScope(cx.owner, cx.enclClass.prefix, cx.scope) match {
-            case NoSymbol => searchPrefix
-            case found    => found
+          defSym = lookupInScope(cx.owner, pre, cx.scope) match {
+            case NoSymbol => inPrefix = true; searchPrefix
+            case found    => inPrefix = false; found
           }
           if (!defSym.exists) cx = cx.outer // push further outward
         }
-      nextDefinition()
+        if ((defSym.isAliasType || lastDef.isAliasType) && pre.memberType(defSym) =:= lastPre.memberType(lastDef))
+          defSym = NoSymbol
+        if (defSym.isStable && lastDef.isStable &&
+          (lastPre.memberType(lastDef).termSymbol == defSym || pre.memberType(defSym).termSymbol == lastDef))
+          defSym = NoSymbol
+        foundInPrefix = inPrefix && defSym.exists
+        foundInSuper  = foundInPrefix && defSym.owner != cx.owner
+      }
+      nextDefinition(NoSymbol, NoPrefix)
 
       if (symbolDepth < 0)
         symbolDepth = cx.depth
@@ -1490,12 +1523,17 @@ trait Contexts { self: Analyzer =>
        *
        * Scala: Bindings of different kinds have a defined precedence order:
        *
-       *  1) Definitions and declarations that are local, inherited, or made available by
-       *     a package clause and also defined in the same compilation unit as the reference, have highest precedence.
+       *  1) Definitions and declarations in lexical scope that are not top-level have the highest precedence.
+       *  1b) Definitions and declarations that are either inherited, or made
+       *      available by a package clause and also defined in the same compilation unit
+       *      as the reference to them, have the next highest precedence.
+       *      (Only in -Xsource:3, same precedence as 1 with a warning in Scala 2.)
        *  2) Explicit imports have next highest precedence.
        *  3) Wildcard imports have next highest precedence.
-       *  4) Definitions made available by a package clause, but not also defined in the same compilation unit
-       *     as the reference, have lowest precedence. Also "root" imports added implicitly.
+       *  4) Bindings made available by a package clause,
+       *     but not also defined in the same compilation unit as the reference to them,
+       *     as well as bindings supplied by the compiler but not explicitly written in source code,
+       *     have the lowest precedence.
        */
       def foreignDefined = defSym.exists && thisContext.isPackageOwnedInDifferentUnit(defSym)  // SI-2458
 
@@ -1552,24 +1590,33 @@ trait Contexts { self: Analyzer =>
           return ambiguousDefnAndImport(defSym.alternatives.head.owner, imp1)
       })
 
-      // If the defSym is at 4, and there is a def at 1 in scope due to packaging, then the reference is ambiguous.
-      if (foreignDefined && !defSym.hasPackageFlag && !thisContext.unit.isJava) {
+      // If the defSym is at 4, and there is a def at 1b in scope due to packaging, then the reference is ambiguous.
+      // Also if defSym is at 1b inherited, the reference can be rendered ambiguous by a def at 1a in scope.
+      val possiblyAmbiguousDefinition =
+        foundInSuper && cx.owner.isClass ||
+        foreignDefined && !defSym.hasPackageFlag
+      if (possiblyAmbiguousDefinition && !thisContext.unit.isJava) {
         val defSym0 = defSym
         val pre0    = pre
         val cx0     = cx
+        val wasFoundInSuper = foundInSuper
+        val foundCompetingSymbol: () => Boolean =
+          if (foreignDefined) () => !foreignDefined
+          else () => !defSym.isTopLevel && !defSym.owner.isPackageObjectOrClass && !foundInSuper && !foreignDefined
         while ((cx ne NoContext) && cx.depth >= symbolDepth) cx = cx.outer
+        if (wasFoundInSuper)
+          while ((cx ne NoContext) && (cx.owner eq cx0.owner)) cx = cx.outer
         var done = false
         while (!done) {
-          defSym = NoSymbol
-          nextDefinition()
-          done = (cx eq NoContext) || defSym.exists && !foreignDefined
+          nextDefinition(defSym0, pre0)
+          done = (cx eq NoContext) || defSym.exists && foundCompetingSymbol()
           if (!done && (cx ne NoContext)) cx = cx.outer
         }
         if (defSym.exists && (defSym ne defSym0)) {
           val ambiguity =
-            if (preferDef) ambiguousDefinitions(owner = defSym.owner, defSym0)
-            else ambiguousDefnAndImport(owner = defSym.owner, imp1)
-          return ambiguity
+            if (preferDef) ambiguousDefinitions(defSym, defSym0, wasFoundInSuper, cx0.enclClass.owner)
+            else Some(ambiguousDefnAndImport(owner = defSym.owner, imp1))
+          if (ambiguity.nonEmpty) return ambiguity.get
         }
         defSym = defSym0
         pre    = pre0

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

@@ -386,7 +386,7 @@ trait MacroAnnotationNamers { self: Analyzer =>
       sym.setInfo(new MaybeExpandeeCompleter(tree) {
         override def kind = s"maybeExpandeeCompleter for ${sym.accurateKindString} ${sym.rawname}#${sym.id}"
         override def maybeExpand(): Unit = {
-          val companion = if (tree.isInstanceOf[ClassDef]) patchedCompanionSymbolOf(sym, context) else NoSymbol
+          val companion = if (this.tree.isInstanceOf[ClassDef]) patchedCompanionSymbolOf(sym, context) else NoSymbol
 
           def maybeExpand(annotation: Tree, annottee: Tree, maybeExpandee: Tree): Option[List[Tree]] =
             if (context.macrosEnabled) { // TODO: when is this bit flipped -- can we pull this check out farther?
@@ -395,7 +395,7 @@ trait MacroAnnotationNamers { self: Analyzer =>
               if (mann.isClass && mann.hasFlag(MACRO)) {
                 assert(!currentRun.compiles(mann), mann)
                 val annm = prepareAnnotationMacro(annotation, mann, sym, annottee, maybeExpandee)
-                expandAnnotationMacro(tree, annm)
+                expandAnnotationMacro(this.tree, annm)
                 // if we encounter an error, we just return None, so that other macro annotations can proceed
                 // this is unlike macroExpand1 when any error in an expandee blocks expansions
                 // there it's necessary in order not to exacerbate typer errors
@@ -424,7 +424,7 @@ trait MacroAnnotationNamers { self: Analyzer =>
               enterSyms(expanded) // TODO: we can't reliably expand into imports, because they won't be accounted by definitions below us
             case None =>
               markNotExpandable(sym)
-              finishSymbolNotExpandee(tree)
+              finishSymbolNotExpandee(this.tree)
           }
 
           // take care of the companion if it's no longer needed

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

@@ -5501,6 +5501,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
               case sym      => typed1(tree setSymbol sym, mode, pt)
             }
           case LookupSucceeded(qual, sym)   =>
+            sym.getAndRemoveAttachment[LookupAmbiguityWarning].foreach(w =>
+              runReporting.warning(tree.pos, w.msg, WarningCategory.Other, context.owner))
             (// this -> Foo.this
             if (sym.isThisSym)
               typed1(This(sym.owner) setPos tree.pos, mode, pt)

src/reflect/scala/reflect/internal/StdAttachments.scala

@@ -152,4 +152,6 @@ trait StdAttachments {
 
   /** For `val i = 42`, marks field as inferred so accessor (getter) can warn if implicit. */
   case object FieldTypeInferred extends PlainAttachment
+
+  case class LookupAmbiguityWarning(msg: String) extends PlainAttachment
 }

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

@@ -772,8 +772,8 @@ trait Types
     def withFilter(p: Type => Boolean) = new FilterMapForeach(p)
 
     class FilterMapForeach(p: Type => Boolean) extends FilterTypeCollector(p){
-      def foreach[U](f: Type => U): Unit = collect(Type.this) foreach f
-      def map[T](f: Type => T): List[T]  = collect(Type.this) map f
+      def foreach[U](f: Type => U): Unit = this.collect(Type.this).foreach(f)
+      def map[T](f: Type => T): List[T]  = this.collect(Type.this).map(f)
     }
 
     @inline final def orElse(alt: => Type): Type = if (this ne NoType) this else alt

src/reflect/scala/reflect/io/ZipArchive.scala

@@ -392,7 +392,7 @@ final class ManifestResources(val url: URL) extends ZipArchive(null) {
       if (!zipEntry.isDirectory) {
         class FileEntry() extends Entry(zipEntry.getName) {
           override def lastModified = zipEntry.getTime()
-          override def input        = resourceInputStream(path)
+          override def input        = resourceInputStream(this.path)
           override def sizeOption   = None
         }
         val f = new FileEntry()

src/reflect/scala/reflect/runtime/JavaUniverseForce.scala

@@ -79,6 +79,7 @@ trait JavaUniverseForce { self: runtime.JavaUniverse  =>
     this.RootSelection
     this.TypedExpectingUnitAttachment
     this.FieldTypeInferred
+    this.LookupAmbiguityWarning
     this.noPrint
     this.typeDebug
     // inaccessible: this.posAssigner

src/scaladoc/scala/tools/nsc/doc/model/ModelFactory.scala

@@ -927,7 +927,7 @@ class ModelFactory(val global: Global, val settings: doc.Settings) {
         }
         else None
       def resultType =
-        makeTypeInTemplateContext(aSym.tpe, inTpl, aSym)
+        makeTypeInTemplateContext(aSym.tpe, this.inTpl, aSym)
       def isImplicit = aSym.isImplicit
     }
 

test/files/neg/t11921-alias.check

@@ -0,0 +1,31 @@
+t11921-alias.scala:18: warning: reference to TT is ambiguous;
+it is both defined in the enclosing object O and available in the enclosing class D as type TT (defined in class C)
+Since 3, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+To continue using the symbol from the superclass, write `this.TT`.
+Or use `-Wconf:msg=legacy-binding:s` to silence this warning.
+      def n(x: TT) = x // ambiguous
+               ^
+t11921-alias.scala:38: warning: reference to c is ambiguous;
+it is both defined in the enclosing class B and available in the enclosing anonymous class as value c (defined in class A)
+Since 3, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+To continue using the symbol from the superclass, write `this.c`.
+Or use `-Wconf:msg=legacy-binding:s` to silence this warning.
+      def n = c // ambiguous
+              ^
+t11921-alias.scala:57: warning: reference to name is ambiguous;
+it is both defined in the enclosing method m and available in the enclosing anonymous class as value name (defined in class C)
+Since 3, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+To continue using the symbol from the superclass, write `this.name`.
+Or use `-Wconf:msg=legacy-binding:s` to silence this warning.
+      println(name)
+              ^
+t11921-alias.scala:67: warning: reference to name is ambiguous;
+it is both defined in the enclosing method m and available in the enclosing anonymous class as value name (defined in class A, inherited through parent class C)
+Since 3, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+To continue using the symbol from the superclass, write `this.name`.
+Or use `-Wconf:msg=legacy-binding:s` to silence this warning.
+      println(name)
+              ^
+error: No warnings can be incurred under -Werror.
+4 warnings
+1 error