From 8aec15b3e3072dac90b9b08ae7614929ef22586a Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 13 Oct 2023 15:16:00 +0200 Subject: [PATCH 1/2] Refactor CannotBeAccessed/isAccessibleFrom --- .../tools/dotc/core/SymDenotations.scala | 21 ++++--------------- .../dotty/tools/dotc/reporting/messages.scala | 10 +++++++-- tests/neg/i18686.check | 13 ++++++++++++ tests/neg/i18686.scala | 13 ++++++++++++ 4 files changed, 38 insertions(+), 19 deletions(-) create mode 100644 tests/neg/i18686.check create mode 100644 tests/neg/i18686.scala diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index b18c6993303f..f1f4be4e4999 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -877,7 +877,7 @@ object SymDenotations { * As a side effect, drop Local flags of members that are not accessed via the ThisType * of their owner. */ - final def isAccessibleFrom(pre: Type, superAccess: Boolean = false, whyNot: StringBuffer | Null = null)(using Context): Boolean = { + final def isAccessibleFrom(pre: Type, superAccess: Boolean = false)(using Context): Boolean = { /** Are we inside definition of `boundary`? * If this symbol is Java defined, package structure is interpreted to be flat. @@ -906,26 +906,13 @@ object SymDenotations { /** Is protected access to target symbol permitted? */ def isProtectedAccessOK: Boolean = - inline def fail(str: String): false = - if whyNot != null then whyNot.nn.append(str) - false val cls = owner.enclosingSubClass if !cls.exists then - 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 - val location = if owner.is(Final) then owner.showLocated else owner.showLocated + " or one of its subclasses" - fail(i""" - | Protected $this can only be accessed from $location.""") - else if isType || pre.derivesFrom(cls) || isConstructor || owner.is(ModuleClass) then + pre.termSymbol.isPackageObject && accessWithin(pre.termSymbol.owner) + else // allow accesses to types from arbitrary subclasses fixes #4737 // don't perform this check for static members - true - else - val location = if cls.is(Final) then cls.showLocated else cls.showLocated + " or one of its subclasses" - fail(i""" - | Protected $this can only be accessed from $location.""") + isType || pre.derivesFrom(cls) || isConstructor || owner.is(ModuleClass) end isProtectedAccessOK if pre eq NoPrefix then true diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 6d115c0e18a9..db7f00914ddc 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -876,7 +876,7 @@ extends Message(PatternMatchExhaustivityID) { val pathes = List( ActionPatch( - srcPos = endPos, + srcPos = endPos, replacement = uncoveredCases.map(c => indent(s"case $c => ???", startColumn)) .mkString("\n", "\n", "") ), @@ -2986,7 +2986,13 @@ extends ReferenceMsg(CannotBeAccessedID): i"none of the overloaded alternatives named $name can" val where = if (ctx.owner.exists) i" from ${ctx.owner.enclosingClass}" else "" val whyNot = new StringBuffer - alts.foreach(_.isAccessibleFrom(pre, superAccess, whyNot)) + for alt <- alts do + if alt.is(Protected) then + val cls = alt.owner.enclosingSubClass + val owner = if cls.exists then cls else alt.owner + val location = if owner.is(Final) then owner.showLocated else owner.showLocated + " or one of its subclasses" + whyNot.append(i""" + | Protected $alt can only be accessed from $location.""") i"$whatCanNot be accessed as a member of $pre$where.$whyNot" def explain(using Context) = "" diff --git a/tests/neg/i18686.check b/tests/neg/i18686.check new file mode 100644 index 000000000000..cfecb5522248 --- /dev/null +++ b/tests/neg/i18686.check @@ -0,0 +1,13 @@ +-- [E173] Reference Error: tests/neg/i18686.scala:9:16 ----------------------------------------------------------------- +9 | println(Foo.Bar1) // error + | ^^^^^^^^ + | value Bar1 cannot be accessed as a member of Foo.type from object Main. +-- [E173] Reference Error: tests/neg/i18686.scala:10:16 ---------------------------------------------------------------- +10 | println(Foo.Bar2) // error + | ^^^^^^^^ + | value Bar2 cannot be accessed as a member of Foo.type from object Main. +-- [E173] Reference Error: tests/neg/i18686.scala:11:16 ---------------------------------------------------------------- +11 | println(Foo.Bar3) // error + | ^^^^^^^^ + | value Bar3 cannot be accessed as a member of Foo.type from object Main. + | Protected value Bar3 can only be accessed from object Foo. diff --git a/tests/neg/i18686.scala b/tests/neg/i18686.scala new file mode 100644 index 000000000000..d6a45b171394 --- /dev/null +++ b/tests/neg/i18686.scala @@ -0,0 +1,13 @@ +object Foo: + private val Bar1: Int = 3 + private[Foo] val Bar2: Int = 3 + protected val Bar3: Int = 3 +end Foo + +object Main: + def main(args: Array[String]): Unit = + println(Foo.Bar1) // error + println(Foo.Bar2) // error + println(Foo.Bar3) // error + end main +end Main From 6c1fce06fcbf1c8742e49dda52f6be032830ef6a Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 13 Oct 2023 15:34:18 +0200 Subject: [PATCH 2/2] Explain accessible scope of private members in error message Fixes #18686 --- community-build/community-projects/stdLib213 | 2 +- .../dotty/tools/dotc/reporting/messages.scala | 23 ++++++++---- compiler/test-resources/repl/i1370 | 1 + tests/neg/i12573.check | 2 +- tests/neg/i14432c.check | 1 + tests/neg/i18686.check | 35 ++++++++++++++----- tests/neg/i18686.scala | 11 ++++-- tests/neg/i18686b.check | 28 +++++++++++++++ tests/neg/i18686b.scala | 22 ++++++++++++ tests/neg/i18686c.check | 8 +++++ tests/neg/i18686c.scala | 8 +++++ tests/neg/i7709.check | 16 ++++----- tests/neg/not-accessible.check | 5 +++ 13 files changed, 135 insertions(+), 27 deletions(-) create mode 100644 tests/neg/i18686b.check create mode 100644 tests/neg/i18686b.scala create mode 100644 tests/neg/i18686c.check create mode 100644 tests/neg/i18686c.scala diff --git a/community-build/community-projects/stdLib213 b/community-build/community-projects/stdLib213 index 88383e58f82c..6243e902928c 160000 --- a/community-build/community-projects/stdLib213 +++ b/community-build/community-projects/stdLib213 @@ -1 +1 @@ -Subproject commit 88383e58f82cd728afc9316081c2350489c39943 +Subproject commit 6243e902928c344fb0e82e21120bb257f08a2af2 diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index db7f00914ddc..ab896eabeb6d 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2987,12 +2987,23 @@ extends ReferenceMsg(CannotBeAccessedID): val where = if (ctx.owner.exists) i" from ${ctx.owner.enclosingClass}" else "" val whyNot = new StringBuffer for alt <- alts do - if alt.is(Protected) then - val cls = alt.owner.enclosingSubClass - val owner = if cls.exists then cls else alt.owner - val location = if owner.is(Final) then owner.showLocated else owner.showLocated + " or one of its subclasses" - whyNot.append(i""" - | Protected $alt can only be accessed from $location.""") + val cls = alt.owner.enclosingSubClass + val owner = if cls.exists then cls else alt.owner + val location: String = + if alt.is(Protected) then + if alt.privateWithin.exists && alt.privateWithin != owner then + if owner.is(Final) then alt.privateWithin.showLocated + else alt.privateWithin.showLocated + ", or " + owner.showLocated + " or one of its subclasses" + else + if owner.is(Final) then owner.showLocated + else owner.showLocated + " or one of its subclasses" + else + alt.privateWithin.orElse(owner).showLocated + val accessMod = if alt.is(Protected) then "protected" else "private" + val within = if alt.privateWithin.exists then i"[${alt.privateWithin.name}]" + else "" + whyNot.append(i""" + | $accessMod$within $alt can only be accessed from $location.""") i"$whatCanNot be accessed as a member of $pre$where.$whyNot" def explain(using Context) = "" diff --git a/compiler/test-resources/repl/i1370 b/compiler/test-resources/repl/i1370 index 4bd92b4d5f83..e10020bf4891 100644 --- a/compiler/test-resources/repl/i1370 +++ b/compiler/test-resources/repl/i1370 @@ -3,4 +3,5 @@ scala> object Lives { class Private { def foo1: Any = new Private.C1; def foo2: 1 | object Lives { class Private { def foo1: Any = new Private.C1; def foo2: Any = new Private.C2 }; object Private { class C1 private {}; private class C2 {} } } | ^^^^^^^^^^ |constructor C1 cannot be accessed as a member of Lives.Private.C1 from class Private. + | private constructor C1 can only be accessed from class C1 in object Private. 1 error found diff --git a/tests/neg/i12573.check b/tests/neg/i12573.check index 8c744fda685b..50fe36aa2aa9 100644 --- a/tests/neg/i12573.check +++ b/tests/neg/i12573.check @@ -5,4 +5,4 @@ |Extension methods were tried, but the search failed with: | | method getDFType cannot be accessed as a member of DFType.type from the top-level definitions in package . - | Protected method getDFType can only be accessed from object DFType. + | protected method getDFType can only be accessed from object DFType. diff --git a/tests/neg/i14432c.check b/tests/neg/i14432c.check index c0f69a1095d7..2710f0dfb3ed 100644 --- a/tests/neg/i14432c.check +++ b/tests/neg/i14432c.check @@ -2,6 +2,7 @@ 12 |class Bar extends example.Foo(23) { // error: cant access private[example] ctor | ^^^^^^^^^^^ | constructor Foo cannot be accessed as a member of example.Foo from class Bar. + | private[example] constructor Foo can only be accessed from package example. -- [E172] Type Error: tests/neg/i14432c.scala:16:43 -------------------------------------------------------------------- 16 | val mFoo = summon[Mirror.Of[example.Foo]] // error: no mirror | ^ diff --git a/tests/neg/i18686.check b/tests/neg/i18686.check index cfecb5522248..6ed69c515051 100644 --- a/tests/neg/i18686.check +++ b/tests/neg/i18686.check @@ -1,13 +1,30 @@ --- [E173] Reference Error: tests/neg/i18686.scala:9:16 ----------------------------------------------------------------- -9 | println(Foo.Bar1) // error - | ^^^^^^^^ - | value Bar1 cannot be accessed as a member of Foo.type from object Main. --- [E173] Reference Error: tests/neg/i18686.scala:10:16 ---------------------------------------------------------------- -10 | println(Foo.Bar2) // error +-- [E173] Reference Error: tests/neg/i18686.scala:13:16 ---------------------------------------------------------------- +13 | println(Foo.Bar1) // error + | ^^^^^^^^ + | value Bar1 cannot be accessed as a member of Foo.type from object Main. + | private value Bar1 can only be accessed from object Foo. +-- [E173] Reference Error: tests/neg/i18686.scala:14:16 ---------------------------------------------------------------- +14 | println(Foo.Bar2) // error | ^^^^^^^^ | value Bar2 cannot be accessed as a member of Foo.type from object Main. --- [E173] Reference Error: tests/neg/i18686.scala:11:16 ---------------------------------------------------------------- -11 | println(Foo.Bar3) // error + | private[Foo] value Bar2 can only be accessed from object Foo. +-- [E173] Reference Error: tests/neg/i18686.scala:15:16 ---------------------------------------------------------------- +15 | println(Foo.Bar3) // error | ^^^^^^^^ | value Bar3 cannot be accessed as a member of Foo.type from object Main. - | Protected value Bar3 can only be accessed from object Foo. + | protected value Bar3 can only be accessed from object Foo. +-- [E173] Reference Error: tests/neg/i18686.scala:16:16 ---------------------------------------------------------------- +16 | println(Foo.Bar4) // error + | ^^^^^^^^ + | value Bar4 cannot be accessed as a member of Foo.type from object Main. + | protected[Foo] value Bar4 can only be accessed from object Foo. +-- [E173] Reference Error: tests/neg/i18686.scala:17:20 ---------------------------------------------------------------- +17 | println(Foo.Baz.Bar5) // error + | ^^^^^^^^^^^^ + | value Bar5 cannot be accessed as a member of Foo.Baz.type from object Main. + | private[Foo] value Bar5 can only be accessed from object Foo. +-- [E173] Reference Error: tests/neg/i18686.scala:18:20 ---------------------------------------------------------------- +18 | println(Foo.Baz.Bar6) // error + | ^^^^^^^^^^^^ + | value Bar6 cannot be accessed as a member of Foo.Baz.type from object Main. + | protected[Foo] value Bar6 can only be accessed from object Foo. diff --git a/tests/neg/i18686.scala b/tests/neg/i18686.scala index d6a45b171394..88da7b9d802a 100644 --- a/tests/neg/i18686.scala +++ b/tests/neg/i18686.scala @@ -1,7 +1,11 @@ object Foo: - private val Bar1: Int = 3 - private[Foo] val Bar2: Int = 3 + private val Bar1: Int = 1 + private[Foo] val Bar2: Int = 2 protected val Bar3: Int = 3 + protected[Foo] val Bar4: Int = 5 + object Baz: + private[Foo] val Bar5: Int = 5 + protected[Foo] val Bar6: Int = 6 end Foo object Main: @@ -9,5 +13,8 @@ object Main: println(Foo.Bar1) // error println(Foo.Bar2) // error println(Foo.Bar3) // error + println(Foo.Bar4) // error + println(Foo.Baz.Bar5) // error + println(Foo.Baz.Bar6) // error end main end Main diff --git a/tests/neg/i18686b.check b/tests/neg/i18686b.check new file mode 100644 index 000000000000..6394aeedf35a --- /dev/null +++ b/tests/neg/i18686b.check @@ -0,0 +1,28 @@ +-- [E173] Reference Error: tests/neg/i18686b.scala:15:16 --------------------------------------------------------------- +15 | println(foo.Bar1) // error + | ^^^^^^^^ + | value Bar1 cannot be accessed as a member of Foo from object Main. + | private value Bar1 can only be accessed from class Foo. +-- [E173] Reference Error: tests/neg/i18686b.scala:16:16 --------------------------------------------------------------- +16 | println(foo.Bar2) // error + | ^^^^^^^^ + | value Bar2 cannot be accessed as a member of Foo from object Main. + | private[Foo] value Bar2 can only be accessed from class Foo. +-- [E173] Reference Error: tests/neg/i18686b.scala:17:16 --------------------------------------------------------------- +17 | println(foo.Bar3) // error + | ^^^^^^^^ + | value Bar3 cannot be accessed as a member of Foo from object Main. + | protected value Bar3 can only be accessed from class Foo or one of its subclasses. +-- [E173] Reference Error: tests/neg/i18686b.scala:18:16 --------------------------------------------------------------- +18 | println(foo.Bar4) // error + | ^^^^^^^^ + | value Bar4 cannot be accessed as a member of Foo from object Main. + | protected[Foo] value Bar4 can only be accessed from class Foo or one of its subclasses. +-- [E008] Not Found Error: tests/neg/i18686b.scala:19:20 --------------------------------------------------------------- +19 | println(foo.Baz.Bar5) // error + | ^^^^^^^^^^^^ + | value Bar5 is not a member of object Foo#Baz +-- [E008] Not Found Error: tests/neg/i18686b.scala:20:20 --------------------------------------------------------------- +20 | println(foo.Baz.Bar6) // error + | ^^^^^^^^^^^^ + | value Bar6 is not a member of object Foo#Baz diff --git a/tests/neg/i18686b.scala b/tests/neg/i18686b.scala new file mode 100644 index 000000000000..86f8066073c0 --- /dev/null +++ b/tests/neg/i18686b.scala @@ -0,0 +1,22 @@ +class Foo: + private val Bar1: Int = 1 + private[Foo] val Bar2: Int = 2 + protected val Bar3: Int = 3 + protected[Foo] val Bar4: Int = 5 + class Baz: + private[Foo] val Bar5: Int = 5 + protected[Foo] val Bar6: Int = 6 +end Foo + +def foo = new Foo + +object Main: + def main(args: Array[String]): Unit = + println(foo.Bar1) // error + println(foo.Bar2) // error + println(foo.Bar3) // error + println(foo.Bar4) // error + println(foo.Baz.Bar5) // error + println(foo.Baz.Bar6) // error + end main +end Main diff --git a/tests/neg/i18686c.check b/tests/neg/i18686c.check new file mode 100644 index 000000000000..328d9cfd42a6 --- /dev/null +++ b/tests/neg/i18686c.check @@ -0,0 +1,8 @@ +-- [E173] Reference Error: tests/neg/i18686c.scala:8:6 ----------------------------------------------------------------- +8 | foo.foo // error + | ^^^^^^^ + |method foo cannot be accessed as a member of (foo² : Bar.Foo) from the top-level definitions in package . + | protected[Bar] method foo can only be accessed from object Bar, or class Foo in object Bar or one of its subclasses. + | + |where: foo is a method in class Foo + | foo² is a parameter in method test diff --git a/tests/neg/i18686c.scala b/tests/neg/i18686c.scala new file mode 100644 index 000000000000..d120e416ed9f --- /dev/null +++ b/tests/neg/i18686c.scala @@ -0,0 +1,8 @@ +object Bar: + class Foo: + protected[Bar] def foo = 23 + class Qux extends Foo: + val qux = foo + +def test(foo: Bar.Foo) = + foo.foo // error diff --git a/tests/neg/i7709.check b/tests/neg/i7709.check index b3b4e21b9db9..14d2dbaf4cde 100644 --- a/tests/neg/i7709.check +++ b/tests/neg/i7709.check @@ -2,39 +2,39 @@ 5 | class B extends X.Y // error | ^^^ | class Y cannot be accessed as a member of X.type from class B. - | Protected class Y can only be accessed from object X. + | protected class Y can only be accessed from object X. -- [E173] Reference Error: tests/neg/i7709.scala:6:21 ------------------------------------------------------------------ 6 | class B2 extends X.Y: // error | ^^^ | class Y cannot be accessed as a member of X.type from class B2. - | Protected class Y can only be accessed from object X. + | protected class Y can only be accessed from object X. -- [E173] Reference Error: tests/neg/i7709.scala:9:28 ------------------------------------------------------------------ 9 | class B4 extends B3(new X.Y) // error | ^^^ | class Y cannot be accessed as a member of X.type from class B4. - | Protected class Y can only be accessed from object X. + | protected class Y can only be accessed from object X. -- [E173] Reference Error: tests/neg/i7709.scala:11:34 ----------------------------------------------------------------- 11 | def this(n: Int) = this(new X.Y().toString) // error | ^^^ | class Y cannot be accessed as a member of X.type from class B5. - | Protected class Y can only be accessed from object X. + | protected class Y can only be accessed from object X. -- [E173] Reference Error: tests/neg/i7709.scala:13:20 ----------------------------------------------------------------- 13 | class B extends X.Y // error | ^^^ | class Y cannot be accessed as a member of X.type from class B. - | Protected class Y can only be accessed from object X. + | protected class Y can only be accessed from object X. -- [E173] Reference Error: tests/neg/i7709.scala:18:18 ----------------------------------------------------------------- 18 | def y = new xx.Y // error | ^^^^ | class Y cannot be accessed as a member of XX from class C. - | Protected class Y can only be accessed from class XX or one of its subclasses. + | protected class Y can only be accessed from class XX or one of its subclasses. -- [E173] Reference Error: tests/neg/i7709.scala:23:20 ----------------------------------------------------------------- 23 | def y = new xx.Y // error | ^^^^ | class Y cannot be accessed as a member of XX from class D. - | Protected class Y can only be accessed from class XX or one of its subclasses. + | protected class Y can only be accessed from class XX or one of its subclasses. -- [E173] Reference Error: tests/neg/i7709.scala:31:20 ----------------------------------------------------------------- 31 | class Q extends X.Y // error | ^^^ | class Y cannot be accessed as a member of p.X.type from class Q. - | Protected class Y can only be accessed from object X in package p. + | protected class Y can only be accessed from object X in package p. diff --git a/tests/neg/not-accessible.check b/tests/neg/not-accessible.check index 00206d281016..54585460a1d8 100644 --- a/tests/neg/not-accessible.check +++ b/tests/neg/not-accessible.check @@ -2,19 +2,24 @@ 8 | def test(a: A) = a.x // error | ^^^ | value x cannot be accessed as a member of (a : foo.A) from class B. + | private[A] value x can only be accessed from class A in package foo. -- [E173] Reference Error: tests/neg/not-accessible.scala:10:23 -------------------------------------------------------- 10 | def test(a: A) = a.x // error | ^^^ | value x cannot be accessed as a member of (a : foo.A) from object B. + | private[A] value x can only be accessed from class A in package foo. -- [E173] Reference Error: tests/neg/not-accessible.scala:13:23 -------------------------------------------------------- 13 | def test(a: A) = a.x // error | ^^^ | value x cannot be accessed as a member of (a : foo.A) from the top-level definitions in package bar. + | private[A] value x can only be accessed from class A in package foo. -- [E173] Reference Error: tests/neg/not-accessible.scala:5:21 --------------------------------------------------------- 5 | def test(a: A) = a.x // error | ^^^ | value x cannot be accessed as a member of (a : foo.A) from the top-level definitions in package foo. + | private[A] value x can only be accessed from class A in package foo. -- [E173] Reference Error: tests/neg/not-accessible.scala:15:23 -------------------------------------------------------- 15 |def test(a: foo.A) = a.x // error | ^^^ | value x cannot be accessed as a member of (a : foo.A) from the top-level definitions in package . + | private[A] value x can only be accessed from class A in package foo.