From 5c0ec3696ead380b34a1063583f902950fadd9ab Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Wed, 16 Aug 2023 13:41:12 +0200 Subject: [PATCH 1/3] Improve error message for inaccessible types [Cherry-picked f4137f2bb664de32c97e07912eda31501ffd8f8c] --- .../dotty/tools/dotc/reporting/messages.scala | 2 +- tests/neg/i12573.check | 4 ++-- tests/neg/not-accessible.check | 20 +++++++++++++++++++ tests/neg/not-accessible.scala | 15 ++++++++++++++ 4 files changed, 38 insertions(+), 3 deletions(-) create mode 100644 tests/neg/not-accessible.check create mode 100644 tests/neg/not-accessible.scala diff --git a/compiler/src/dotty/tools/dotc/reporting/messages.scala b/compiler/src/dotty/tools/dotc/reporting/messages.scala index 007f0a56f8d8..c6ad4648b69a 100644 --- a/compiler/src/dotty/tools/dotc/reporting/messages.scala +++ b/compiler/src/dotty/tools/dotc/reporting/messages.scala @@ -2961,7 +2961,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" diff --git a/tests/neg/i12573.check b/tests/neg/i12573.check index d250f4beabbe..a1774ed00bd7 100644 --- a/tests/neg/i12573.check +++ b/tests/neg/i12573.check @@ -4,6 +4,6 @@ | 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$. + | method getDFType cannot be accessed as a member of DFType.type from package object 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 \ No newline at end of file + | is not a subclass of object DFType where target is defined diff --git a/tests/neg/not-accessible.check b/tests/neg/not-accessible.check new file mode 100644 index 000000000000..e97468aade3b --- /dev/null +++ b/tests/neg/not-accessible.check @@ -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 package object not-accessible$package. +-- [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 package object not-accessible$package. +-- [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 package object not-accessible$package. diff --git a/tests/neg/not-accessible.scala b/tests/neg/not-accessible.scala new file mode 100644 index 000000000000..a0ff791e966f --- /dev/null +++ b/tests/neg/not-accessible.scala @@ -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 From 82eed5fdcc7ca060ec3abedac38593f26ad9af3a Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 17 Aug 2023 10:09:14 +0200 Subject: [PATCH 2/3] Pretty print top-level definitions object symbol [Cherry-picked 1920336f78152958d13cbdb476cc73aff63cdd59] --- compiler/src/dotty/tools/dotc/core/NameOps.scala | 12 +++++++++++- .../src/dotty/tools/dotc/core/SymDenotations.scala | 4 ++++ .../dotty/tools/dotc/printing/RefinedPrinter.scala | 7 ++++++- tests/neg-macros/annot-result-owner.check | 2 +- tests/neg/i12573.check | 10 +++++----- tests/neg/not-accessible.check | 6 +++--- 6 files changed, 30 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/NameOps.scala b/compiler/src/dotty/tools/dotc/core/NameOps.scala index 04440c9e9b39..644cbfbdb5a8 100644 --- a/compiler/src/dotty/tools/dotc/core/NameOps.scala +++ b/compiler/src/dotty/tools/dotc/core/NameOps.scala @@ -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 => @@ -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 diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 6507695b95df..75bdf6132c30 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -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 diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 5685ca1ca671..472ab4a9aac6 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -1099,13 +1099,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 + "top-level definition in package " + nameString(sym.owner.name) + else (keywordText(kindString(sym)) ~~ { if (sym.isAnonymousClass) toTextParents(sym.info.parents) ~~ "{...}" else diff --git a/tests/neg-macros/annot-result-owner.check b/tests/neg-macros/annot-result-owner.check index 5d67be058fdf..7064f62d8970 100644 --- a/tests/neg-macros/annot-result-owner.check +++ b/tests/neg-macros/annot-result-owner.check @@ -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 top-level definition in package but was owned by method foo. -- Error: tests/neg-macros/annot-result-owner/Test_2.scala:5:2 --------------------------------------------------------- 5 | @insertVal // error | ^^^^^^^^^^ diff --git a/tests/neg/i12573.check b/tests/neg/i12573.check index a1774ed00bd7..fa402273cada 100644 --- a/tests/neg/i12573.check +++ b/tests/neg/i12573.check @@ -1,9 +1,9 @@ -- [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 package object 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 top-level definition in package . + | Access to protected method getDFType not permitted because enclosing top-level definition in package + | is not a subclass of object DFType where target is defined diff --git a/tests/neg/not-accessible.check b/tests/neg/not-accessible.check index e97468aade3b..849c255aa68e 100644 --- a/tests/neg/not-accessible.check +++ b/tests/neg/not-accessible.check @@ -9,12 +9,12 @@ -- [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 package object not-accessible$package. + | value x cannot be accessed as a member of (a : foo.A) from top-level definition 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 package object not-accessible$package. + | value x cannot be accessed as a member of (a : foo.A) from top-level definition 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 package object not-accessible$package. + | value x cannot be accessed as a member of (a : foo.A) from top-level definition in package . From b79bccf7c1aaa35645aeb7b0fd2d236599cd004a Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 13 Oct 2023 10:18:13 +0200 Subject: [PATCH 3/3] Improve error message for inaccessible protected members [Cherry-picked 1472b7a4975106b224489e854cbedcb09db7ecb7] --- .../tools/dotc/core/SymDenotations.scala | 8 +++---- .../tools/dotc/printing/RefinedPrinter.scala | 2 +- tests/neg-macros/annot-result-owner.check | 2 +- tests/neg/i12573.check | 5 ++-- tests/neg/i7709.check | 24 +++++++------------ tests/neg/not-accessible.check | 6 ++--- 6 files changed, 19 insertions(+), 28 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala index 75bdf6132c30..cad15c96dcae 100644 --- a/compiler/src/dotty/tools/dotc/core/SymDenotations.scala +++ b/compiler/src/dotty/tools/dotc/core/SymDenotations.scala @@ -915,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 diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 472ab4a9aac6..2bacbe7f9036 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -1109,7 +1109,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { nameString(sym) if sym.is(ModuleClass) && sym.isTopLevelDefinitionsObject then - "top-level definition in package " + nameString(sym.owner.name) + "the top-level definitions in package " + nameString(sym.owner.name) else (keywordText(kindString(sym)) ~~ { if (sym.isAnonymousClass) toTextParents(sym.info.parents) ~~ "{...}" diff --git a/tests/neg-macros/annot-result-owner.check b/tests/neg-macros/annot-result-owner.check index 7064f62d8970..e2209998579c 100644 --- a/tests/neg-macros/annot-result-owner.check +++ b/tests/neg-macros/annot-result-owner.check @@ -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 top-level definition in 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 but was owned by method foo. -- Error: tests/neg-macros/annot-result-owner/Test_2.scala:5:2 --------------------------------------------------------- 5 | @insertVal // error | ^^^^^^^^^^ diff --git a/tests/neg/i12573.check b/tests/neg/i12573.check index fa402273cada..8c744fda685b 100644 --- a/tests/neg/i12573.check +++ b/tests/neg/i12573.check @@ -4,6 +4,5 @@ |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 top-level definition in package . - | Access to protected method getDFType not permitted because enclosing top-level definition in 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 . + | Protected method getDFType can only be accessed from object DFType. diff --git a/tests/neg/i7709.check b/tests/neg/i7709.check index 180cf1939d16..b3b4e21b9db9 100644 --- a/tests/neg/i7709.check +++ b/tests/neg/i7709.check @@ -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. diff --git a/tests/neg/not-accessible.check b/tests/neg/not-accessible.check index 849c255aa68e..00206d281016 100644 --- a/tests/neg/not-accessible.check +++ b/tests/neg/not-accessible.check @@ -9,12 +9,12 @@ -- [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 top-level definition in package bar. + | 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 top-level definition in package foo. + | 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 top-level definition in package . + | value x cannot be accessed as a member of (a : foo.A) from the top-level definitions in package .