Inherited members can induce ambiguity

Spec binding precedence of inherited definition.
Use -Ylegacy-binding for old precedence.

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/reflect/quasiquotes/Parsers.scala

@@ -92,7 +92,7 @@ trait Parsers { self: Quasiquotes =>
               case _ => gen.mkBlock(stats, doFlatten = true)
             }
           case nme.unapply => gen.mkBlock(stats, doFlatten = false)
-          case other       => global.abort("unreachable")
+          case other       => this.global.abort("unreachable")
         }
 
         // tq"$a => $b"

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/settings/ScalaSettings.scala

@@ -267,6 +267,8 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
   val exposeEmptyPackage = BooleanSetting ("-Yexpose-empty-package", "Internal only: expose the empty package.").internalOnly()
   val Ydelambdafy        = ChoiceSetting  ("-Ydelambdafy", "strategy", "Strategy used for translating lambdas into JVM code.", List("inline", "method"), "method")
 
+  val legacyBinding = BooleanSetting("-Ylegacy-binding", "Observe legacy name binding preference for inherited member competing with local definition.")
+
   // Allows a specialised jar to be written. For instance one that provides stable hashing of content, or customisation of the file storage
   val YjarFactory = StringSetting   ("-YjarFactory", "classname", "factory for jar files", classOf[DefaultJarFactory].getName)
   val YaddBackendThreads = IntSetting   ("-Ybackend-parallelism", "maximum worker threads for backend", 1, Some((1,16)), (x: String) => None )

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,15 @@ 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(owner: Symbol, other: Symbol, addendum: String) =
+      LookupAmbiguous(s"it is both defined in $owner and available as ${other.fullLocationString}$addendum")
 
     def apply(thisContext: Context, name: Name)(qualifies: Symbol => Boolean): NameLookup = {
       lookupError  = null
@@ -1370,6 +1372,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 +1390,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,15 +1455,20 @@ trait Contexts { self: Analyzer =>
       }
 
       // cx.scope eq null arises during FixInvalidSyms in Duplicators
-      def nextDefinition(): Unit =
+      def nextDefinition(): 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
         }
+        foundInPrefix = inPrefix && defSym.exists
+        foundInSuper  = foundInPrefix && defSym.owner != cx.owner
+      }
       nextDefinition()
 
       if (symbolDepth < 0)
@@ -1490,8 +1498,11 @@ 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) Local definitions and declarations 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.
+       *      (Same precedence as 1 under -Ylegacy-binding.)
        *  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
@@ -1552,22 +1563,38 @@ 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 && !settings.legacyBinding.value ||
+        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
+          done = (cx eq NoContext) || defSym.exists && foundCompetingSymbol()
           if (!done && (cx ne NoContext)) cx = cx.outer
         }
         if (defSym.exists && (defSym ne defSym0)) {
+          val addendum =
+            if (wasFoundInSuper)
+            s"""|
+                |Since 2.13.11, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+                |If shadowing was intended, write `this.${defSym0.name}`.
+                |Or use `-Ylegacy-binding` to enable the previous behavior everywhere.""".stripMargin
+            else ""
           val ambiguity =
-            if (preferDef) ambiguousDefinitions(owner = defSym.owner, defSym0)
+            if (preferDef) ambiguousDefinitions(owner = defSym.owner, defSym0, addendum)
             else ambiguousDefnAndImport(owner = defSym.owner, imp1)
           return ambiguity
         }

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/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/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.check

@@ -0,0 +1,8 @@
+t11921.scala:6: error: reference to coll is ambiguous;
+it is both defined in method lazyMap and available as method coll in trait Iterable
+Since 2.13.11, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+If shadowing was intended, write `this.coll`.
+Or use `-Ylegacy-binding` to enable the previous behavior everywhere.
+      def iterator = coll.iterator.map(f)    // coll is ambiguous
+                     ^
+1 error

test/files/neg/t11921.scala

@@ -0,0 +1,16 @@
+
+
+class C {
+  def lazyMap[A, B](coll: Iterable[A], f: A => B) =
+    new Iterable[B] {
+      def iterator = coll.iterator.map(f)    // coll is ambiguous
+    }
+}
+
+/* was:
+t11921.scala:5: error: type mismatch;
+ found   : A => B
+ required: B => B
+      def iterator = coll.iterator.map(f)
+                                       ^
+*/

test/files/neg/t11921b.check

@@ -0,0 +1,29 @@
+t11921b.scala:11: error: reference to x is ambiguous;
+it is both defined in object Test and available as value x in class C
+Since 2.13.11, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+If shadowing was intended, write `this.x`.
+Or use `-Ylegacy-binding` to enable the previous behavior everywhere.
+      println(x)  // error
+              ^
+t11921b.scala:15: error: reference to x is ambiguous;
+it is both defined in object Test and available as value x in class C
+Since 2.13.11, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+If shadowing was intended, write `this.x`.
+Or use `-Ylegacy-binding` to enable the previous behavior everywhere.
+        println(x)  // error
+                ^
+t11921b.scala:26: error: reference to y is ambiguous;
+it is both defined in method c and available as value y in class D
+Since 2.13.11, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+If shadowing was intended, write `this.y`.
+Or use `-Ylegacy-binding` to enable the previous behavior everywhere.
+      println(y)  // error
+              ^
+t11921b.scala:38: error: reference to y is ambiguous;
+it is both defined in method c and available as value y in class D
+Since 2.13.11, symbols inherited from a superclass no longer shadow symbols defined in an outer scope.
+If shadowing was intended, write `this.y`.
+Or use `-Ylegacy-binding` to enable the previous behavior everywhere.
+        println(y)  // error
+                ^
+4 errors