From 0f5292d81128ef5666b06c57bf713b0a08bb1b5b Mon Sep 17 00:00:00 2001
From: odersky <odersky@gmail.com>
Date: Tue, 4 Jul 2023 22:10:00 +0200
Subject: [PATCH] Avoid shadowing by private definitions in more situations

There's a tricky case where when selecting a member we find a private definition
first, which is not accessible, but which shadows another definition. We handled
that case correctly on direct member selection, but not when a member was looked
up in an import, not when a member was searched in an implicit.
---
 .../src/dotty/tools/dotc/core/Types.scala     | 12 ++++++++-
 .../dotty/tools/dotc/typer/ImportInfo.scala   |  2 +-
 .../src/dotty/tools/dotc/typer/Typer.scala    | 11 +++++---
 tests/pos/i18135.scala                        | 26 +++++++++++++++++++
 4 files changed, 45 insertions(+), 6 deletions(-)
 create mode 100644 tests/pos/i18135.scala

diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala
index c9df6fc69727..e7bf09239bdf 100644
--- a/compiler/src/dotty/tools/dotc/core/Types.scala
+++ b/compiler/src/dotty/tools/dotc/core/Types.scala
@@ -703,6 +703,16 @@ object Types {
       }
       findMember(name, pre, required, excluded)
     }
+    
+    /** The implicit members with given name. If there are none and the denotation
+     *  contains private members, also look for shadowed non-private implicits.
+     */
+    def implicitMembersNamed(name: Name)(using Context): List[SingleDenotation] =
+      val d = member(name)
+      val alts = d.altsWith(_.isOneOf(GivenOrImplicitVal))
+      if alts.isEmpty && d.hasAltWith(_.symbol.is(Private)) then
+        nonPrivateMember(name).altsWith(_.isOneOf(GivenOrImplicitVal))
+      else alts
 
     /** Find member of this type with given `name`, all `required`
      *  flags and no `excluded` flag and produce a denotation that contains
@@ -1006,7 +1016,7 @@ object Types {
     final def implicitMembers(using Context): List[TermRef] = {
       record("implicitMembers")
       memberDenots(implicitFilter,
-          (name, buf) => buf ++= member(name).altsWith(_.isOneOf(GivenOrImplicitVal)))
+          (name, buf) => buf ++= implicitMembersNamed(name))
         .toList.map(d => TermRef(this, d.symbol.asTerm))
     }
 
diff --git a/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala b/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala
index 4d8d5ada46a1..ba05cba229ae 100644
--- a/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala
+++ b/compiler/src/dotty/tools/dotc/typer/ImportInfo.scala
@@ -148,7 +148,7 @@ class ImportInfo(symf: Context ?=> Symbol,
     else
       for
         renamed <- reverseMapping.keys
-        denot <- pre.member(reverseMapping(renamed).nn).altsWith(_.isOneOf(GivenOrImplicitVal))
+        denot <- pre.implicitMembersNamed(reverseMapping(renamed).nn)
       yield
         val original = reverseMapping(renamed).nn
         val ref = TermRef(pre, original, denot)
diff --git a/compiler/src/dotty/tools/dotc/typer/Typer.scala b/compiler/src/dotty/tools/dotc/typer/Typer.scala
index 51787cb5004a..f8392668b36c 100644
--- a/compiler/src/dotty/tools/dotc/typer/Typer.scala
+++ b/compiler/src/dotty/tools/dotc/typer/Typer.scala
@@ -288,16 +288,19 @@ class Typer(@constructorOnly nestingLevel: Int = 0) extends Namer
           case ImportType(expr) =>
             val pre = expr.tpe
             val denot0 = pre.memberBasedOnFlags(name, required, excluded)
-              .accessibleFrom(pre)(using refctx)
+            var accessibleDenot = denot0.accessibleFrom(pre)(using refctx)
+            if !accessibleDenot.exists && denot0.hasAltWith(_.symbol.is(Private)) then
+              accessibleDenot = pre.memberBasedOnFlags(name, required, excluded | Private)
+                .accessibleFrom(pre)(using refctx)
             // Pass refctx so that any errors are reported in the context of the
             // reference instead of the context of the import scope
-            if denot0.exists then
+            if accessibleDenot.exists then
               val denot =
                 if checkBounds then
-                  denot0.filterWithPredicate { mbr =>
+                  accessibleDenot.filterWithPredicate { mbr =>
                     mbr.matchesImportBound(if mbr.symbol.is(Given) then imp.givenBound else imp.wildcardBound)
                   }
-                else denot0
+                else accessibleDenot
               def isScalaJsPseudoUnion =
                 denot.name == tpnme.raw.BAR && ctx.settings.scalajs.value && denot.symbol == JSDefinitions.jsdefn.PseudoUnionClass
               // Just like Scala2Unpickler reinterprets Scala.js pseudo-unions
diff --git a/tests/pos/i18135.scala b/tests/pos/i18135.scala
new file mode 100644
index 000000000000..0be6321dc74d
--- /dev/null
+++ b/tests/pos/i18135.scala
@@ -0,0 +1,26 @@
+class Conf
+
+class Bar(_conf: Conf) {
+  implicit val conf: Conf = _conf
+}
+
+class Foo(conf: Conf) extends Bar(conf)
+//class Foo(_conf: Conf) extends Bar(_conf)
+// using a different name fixes it
+
+class Test {
+  def test(foo: Foo) = {
+    import foo.*
+    //implicit val conf: Conf = foo.conf
+    // manually redefining it also fixes it
+    assert(conf != null)
+    assert(implicitly[Conf] != null)
+  }
+  def test2(foo: Foo) = {
+    import foo.conf
+    //implicit val conf: Conf = foo.conf
+    // manually redefining it also fixes it
+    assert(conf != null)
+    assert(implicitly[Conf] != null)
+  }
+}
\ No newline at end of file