From aed47fd78aec49aebf7d2d97b167e58c9c165cc2 Mon Sep 17 00:00:00 2001
From: odersky <odersky@gmail.com>
Date: Wed, 5 Jul 2023 00:14:35 +0200
Subject: [PATCH] Add clause for protected visibility from package objects

We usually have an access rule that the access to a protected member `foo` in class `C`
must be from somewhere nested in a subclass of `C`. But that fails if the member is
accessed from a package object `p.package`. In that case, the access does not need to be in
the same object, it just has to be in package `p`. This clause was previously missing
and is now added.

Why was this only recently discovered?

#18057 fixed an issue where toplevel protected members were always accessible
because explicit package object prefixes were added after the accessibility check was done,
and would re-establish the previous members without doing an accessibility check. The fix was
done by adding package objects first, then doing he rest of the checks. But that also means
that protected toplevel objects now get checked as members of their synthetic package object
instead of as members of their package.

The change here also makes specs2 compile again.
---
 .../dotty/communitybuild/CommunityBuildTest.scala |  6 +-----
 .../dotty/tools/dotc/core/SymDenotations.scala    | 11 +++++++----
 tests/pos/i18124/definition.scala                 | 15 +++++++++++++++
 tests/pos/i18124/usage.scala                      |  8 ++++++++
 4 files changed, 31 insertions(+), 9 deletions(-)
 create mode 100644 tests/pos/i18124/definition.scala
 create mode 100644 tests/pos/i18124/usage.scala

diff --git a/community-build/test/scala/dotty/communitybuild/CommunityBuildTest.scala b/community-build/test/scala/dotty/communitybuild/CommunityBuildTest.scala
index 8837f7319117..bf6b6d431509 100644
--- a/community-build/test/scala/dotty/communitybuild/CommunityBuildTest.scala
+++ b/community-build/test/scala/dotty/communitybuild/CommunityBuildTest.scala
@@ -93,11 +93,7 @@ class CommunityBuildTestC:
   @Test def sconfig = projects.sconfig.run()
   @Test def shapeless = projects.shapeless.run()
   @Test def sourcecode = projects.sourcecode.run()
-
-  // Disabled. Currently fails in FutureMatchers.scala. The call to
-  // `checkResultFailure` goes to a protected method which is not accessible.
-  // I tried to fix it, but get test failures.
-  // @Test def specs2 = projects.specs2.run()
+  @Test def specs2 = projects.specs2.run()
 
   @Test def stdLib213 = projects.stdLib213.run()
   @Test def ujson = projects.ujson.run()
diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
index aa97435d64bb..988a37be4388 100644
--- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
+++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala
@@ -907,10 +907,13 @@ object SymDenotations {
           false
         val cls = owner.enclosingSubClass
         if !cls.exists then
-          val encl = if ctx.owner.isConstructor then ctx.owner.enclosingClass.owner.enclosingClass else ctx.owner.enclosingClass
-          fail(i"""
-               | Access to protected $this not permitted because enclosing ${encl.showLocated}
-               | is not a subclass of ${owner.showLocated} where target is defined""")
+          if pre.termSymbol.isPackageObject && accessWithin(pre.termSymbol.owner) then
+            true
+          else
+            val encl = if ctx.owner.isConstructor then ctx.owner.enclosingClass.owner.enclosingClass else ctx.owner.enclosingClass
+            fail(i"""
+                 | Access to protected $this not permitted because enclosing ${encl.showLocated}
+                 | is not a subclass of ${owner.showLocated} where target is defined""")
         else if isType || pre.derivesFrom(cls) || isConstructor || owner.is(ModuleClass) then
           // allow accesses to types from arbitrary subclasses fixes #4737
           // don't perform this check for static members
diff --git a/tests/pos/i18124/definition.scala b/tests/pos/i18124/definition.scala
new file mode 100644
index 000000000000..1377c94fe7cd
--- /dev/null
+++ b/tests/pos/i18124/definition.scala
@@ -0,0 +1,15 @@
+// definition.scala
+package oolong.bson:
+
+  trait BsonValue
+  protected def merge(
+      base: BsonValue,
+      patch: BsonValue,
+      arraySubvalues: Boolean = false
+  ): BsonValue = ???
+
+  private def foo: Int = 1
+
+  package inner:
+    protected[bson] def bar = 2
+
diff --git a/tests/pos/i18124/usage.scala b/tests/pos/i18124/usage.scala
new file mode 100644
index 000000000000..0bc0417c01ad
--- /dev/null
+++ b/tests/pos/i18124/usage.scala
@@ -0,0 +1,8 @@
+// usage.scala
+package oolong.bson
+
+extension (bv: BsonValue)
+  def :+(other: BsonValue): BsonValue = merge(other, bv, false)
+
+val x = foo
+val y = inner.bar