Skip to content

Commit

Permalink
Improve error message for inaccessible types (#18406)
Browse files Browse the repository at this point in the history
* If accessed from an object:
```diff
- from module class B$
+ from object B
```
* If accessed from a package:
```diff
- from module class <filename>$package$
+ from top-level definition in package foo
```
  • Loading branch information
smarter authored Oct 13, 2023
2 parents 40cee39 + 1472b7a commit 231ca72
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 29 deletions.
12 changes: 11 additions & 1 deletion compiler/src/dotty/tools/dotc/core/NameOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ object NameOps {
false
}

/** is this the name of an object enclosing packagel-level definitions? */
/** is this the name of an object enclosing package-level definitions? */
def isPackageObjectName: Boolean = name match {
case name: TermName => name == nme.PACKAGE || name.endsWith(str.TOPLEVEL_SUFFIX)
case name: TypeName =>
Expand All @@ -119,6 +119,16 @@ object NameOps {
}
}

/** is this the name of an object enclosing top-level definitions? */
def isTopLevelPackageObjectName: Boolean = name match {
case name: TermName => name.endsWith(str.TOPLEVEL_SUFFIX)
case name: TypeName =>
name.toTermName match {
case ModuleClassName(original) => original.isTopLevelPackageObjectName
case _ => false
}
}

/** Convert this module name to corresponding module class name */
def moduleClassName: TypeName = name.derived(ModuleClassName).toTypeName

Expand Down
12 changes: 8 additions & 4 deletions compiler/src/dotty/tools/dotc/core/SymDenotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -672,6 +672,10 @@ object SymDenotations {
def isPackageObject(using Context): Boolean =
name.isPackageObjectName && owner.is(Package) && this.is(Module)

/** Is this symbol a package object containing top-level definitions? */
def isTopLevelDefinitionsObject(using Context): Boolean =
name.isTopLevelPackageObjectName && owner.is(Package) && this.is(Module)

/** Is this symbol a toplevel definition in a package object? */
def isWrappedToplevelDef(using Context): Boolean =
!isConstructor && owner.isPackageObject
Expand Down Expand Up @@ -911,17 +915,17 @@ object SymDenotations {
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"""
| Access to protected $this not permitted because enclosing ${encl.showLocated}
| is not a subclass of ${owner.showLocated} where target is defined""")
| Protected $this can only be accessed from $location.""")
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
true
else
val location = if cls.is(Final) then cls.showLocated else cls.showLocated + " or one of its subclasses"
fail(i"""
| Access to protected ${symbol.show} not permitted because prefix type ${pre.widen.show}
| does not conform to ${cls.showLocated} where the access takes place""")
| Protected $this can only be accessed from $location.""")
end isProtectedAccessOK

if pre eq NoPrefix then true
Expand Down
7 changes: 6 additions & 1 deletion compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1113,13 +1113,18 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) {
fullNameString(sym)
else if (sym.is(ModuleClass) && sym.isPackageObject && sym.name.stripModuleClassSuffix == tpnme.PACKAGE)
nameString(sym.owner.name)
else if (sym.is(ModuleClass) && sym.isTopLevelDefinitionsObject)
nameString(sym.owner.name)
else if (sym.is(ModuleClass))
nameString(sym.name.stripModuleClassSuffix) + idString(sym)
else if (hasMeaninglessName(sym))
simpleNameString(sym.owner) + idString(sym)
else
nameString(sym)
(keywordText(kindString(sym)) ~~ {

if sym.is(ModuleClass) && sym.isTopLevelDefinitionsObject then
"the top-level definitions in package " + nameString(sym.owner.name)
else (keywordText(kindString(sym)) ~~ {
if (sym.isAnonymousClass)
toTextParents(sym.info.parents) ~~ "{...}"
else
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/reporting/messages.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2984,7 +2984,7 @@ extends ReferenceMsg(CannotBeAccessedID):
i"${if (sym.owner == pre.typeSymbol) sym.show else sym.showLocated} cannot"
case _ =>
i"none of the overloaded alternatives named $name can"
val where = if (ctx.owner.exists) s" from ${ctx.owner.enclosingClass}" else ""
val where = if (ctx.owner.exists) i" from ${ctx.owner.enclosingClass}" else ""
val whyNot = new StringBuffer
alts.foreach(_.isAccessibleFrom(pre, superAccess, whyNot))
i"$whatCanNot be accessed as a member of $pre$where.$whyNot"
Expand Down
2 changes: 1 addition & 1 deletion tests/neg-macros/annot-result-owner.check
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
-- Error: tests/neg-macros/annot-result-owner/Test_2.scala:1:0 ---------------------------------------------------------
1 |@insertVal // error
|^^^^^^^^^^
|macro annotation @insertVal added value definitionWithWrongOwner$macro$1 with an inconsistent owner. Expected it to be owned by package object Test_2$package but was owned by method foo.
|macro annotation @insertVal added value definitionWithWrongOwner$macro$1 with an inconsistent owner. Expected it to be owned by the top-level definitions in package <empty> but was owned by method foo.
-- Error: tests/neg-macros/annot-result-owner/Test_2.scala:5:2 ---------------------------------------------------------
5 | @insertVal // error
| ^^^^^^^^^^
Expand Down
9 changes: 4 additions & 5 deletions tests/neg/i12573.check
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
-- [E008] Not Found Error: tests/neg/i12573.scala:23:38 ----------------------------------------------------------------
23 |val w: Value[8] = DFBits(Value[8](8)).getDFType.width // error
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| value getDFType is not a member of DFBits[(8 : Int)].
| Extension methods were tried, but the search failed with:
|value getDFType is not a member of DFBits[(8 : Int)].
|Extension methods were tried, but the search failed with:
|
| method getDFType cannot be accessed as a member of DFType.type from module class i12573$package$.
| Access to protected method getDFType not permitted because enclosing package object i12573$package
| is not a subclass of object DFType where target is defined
| method getDFType cannot be accessed as a member of DFType.type from the top-level definitions in package <empty>.
| Protected method getDFType can only be accessed from object DFType.
24 changes: 8 additions & 16 deletions tests/neg/i7709.check
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,39 @@
5 | class B extends X.Y // error
| ^^^
| class Y cannot be accessed as a member of X.type from class B.
| Access to protected class Y not permitted because enclosing object A
| is not a subclass of object X where target is defined
| 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.
| Access to protected class Y not permitted because enclosing object A
| is not a subclass of object X where target is defined
| 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.
| Access to protected class Y not permitted because enclosing object A
| is not a subclass of object X where target is defined
| 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.
| Access to protected class Y not permitted because enclosing object A
| is not a subclass of object X where target is defined
| 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.
| Access to protected class Y not permitted because enclosing trait T
| is not a subclass of object X where target is defined
| 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.
| Access to protected class Y not permitted because enclosing class C
| is not a subclass of class XX where target is defined
| 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.
| Access to protected class Y not permitted because enclosing class D
| is not a subclass of class XX where target is defined
| 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.
| Access to protected class Y not permitted because enclosing package p
| is not a subclass of object X in package p where target is defined
| Protected class Y can only be accessed from object X in package p.
20 changes: 20 additions & 0 deletions tests/neg/not-accessible.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
-- [E173] Reference Error: tests/neg/not-accessible.scala:8:23 ---------------------------------------------------------
8 | def test(a: A) = a.x // error
| ^^^
| value x cannot be accessed as a member of (a : foo.A) from class B.
-- [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.
-- [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.
-- [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.
-- [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 <empty>.
15 changes: 15 additions & 0 deletions tests/neg/not-accessible.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package foo:

class A(private[A] val x: Int)

def test(a: A) = a.x // error

class B:
def test(a: A) = a.x // error
object B:
def test(a: A) = a.x // error

package bar:
def test(a: A) = a.x // error

def test(a: foo.A) = a.x // error

0 comments on commit 231ca72

Please sign in to comment.