[
https://issues.apache.org/jira/browse/GROOVY-12537?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18134164#comment-18134164
]
ASF GitHub Bot commented on GROOVY-12537:
-----------------------------------------
paulk-asert commented on PR #3052:
URL: https://github.com/apache/groovy/pull/3052#issuecomment-6494751582
Thanks @daniellansun. Pushed f16a2f1, rebased on master now that #3044 is
merged.
Blocking: outer == null admits too much. Agreed, and it was worse than
lenient: the branch admitted a precompiled private member type, which doesn't
work at runtime. @CompileStatic code in another package reading
lib.Lib.Hidden.x compiled and then threw IllegalAccessError, where 6.0.0
rejected it at compile time. That branch now follows javac:
- private is never accessible. A nest lies within one source unit, so a
precompiled member type can't share the accessor's nest. This rejects the
same-package case too, consistent with how master (after #3044) treats a source
member type of another top-level class.
- package-private is accessible from the same package only.
- protected is still admitted without the subclass check. Without the
enclosing type, the method can't make it. Its class file marks it public, so
the access can't fail at runtime; the only difference from javac is that we
accept it. The Javadoc now gives the reasoning for each case, so nobody
tightens or loosens it without context. If your review text (it cuts off at
"The new branch:") was also about protected, I can follow up once #3077's
outer-class data is available for classes read from bytecode.
1. #3044. Rebased on it. Its tests in FieldsAndPropertiesSTCTest, the write,
attribute, closure, enum and other-top-level-class negative cases, are on
master and pass unchanged. The early return it added is subsumed by the private
branch here.
2. Same-unit test. testProtectedMemberTypeFromSubclassBodyInSameUnit now
compiles to a temp dir, loads the classes and asserts run() == 'ff'.
3. Negatives. Added:
- protected member type from a non-subclass in another package (same unit)
- package-private member type from another package, both in the same unit
and precompiled
- precompiled private member type, from the same package and from another
package, loaded both from a Class and from class files
The two private tests fail without the change.
Groovy12537 passes 9/9. groovy.transform.stc.*,
org.codehaus.groovy.classgen.asm.sc.*, bugs.* and gls.innerClass.* pass (6,400
tests, 0 failures).
> STC: field access through a protected or doubly-nested private member type is
> wrongly rejected as inaccessible
> --------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12537
> URL: https://issues.apache.org/jira/browse/GROOVY-12537
> Project: Groovy
> Issue Type: Bug
> Components: Static Type Checker
> Affects Versions: 6.0.0
> Reporter: Paul King
> Assignee: Paul King
> Priority: Major
>
> Description:
> Since GROOVY-12314, the type checker rejects a field access when the type
> that qualifies it is not accessible from the calling class. The accessibility
> check gets member types wrong in two cases, so valid code that compiled
> before now fails under @CompileStatic and @TypeChecked:
> 1. A protected member type of a class compiled in an earlier step, used from
> a subclass in another package:
> {code:groovy}
> // p/Base.groovy, compiled first
> package p
> class Base {
> protected static class Prot { public static String F = 'f' }
> }
> {code}
> {code:groovy}
> // q/Sub.groovy, compiled with p.Base on the classpath
> package q
> @groovy.transform.CompileStatic
> class Sub extends p.Base {
> static String run() { p.Base.Prot.F }
> }
> {code}
> {noformat}
> [Static type checking] - Cannot access field: F of class: p.Base$Prot from
> class: q.Sub
> {noformat}
> 2. A private member type nested inside another member type, used from the
> top-level class:
> {code:groovy}
> package q
> @groovy.transform.CompileStatic
> class Outer {
> private static class A {
> private static class B { public static String F = 'f' }
> }
> static String run() { A.B.F }
> }
> {code}
> {noformat}
> [Static type checking] - Cannot access field: F of class: q.Outer$A$B from
> class: q.Outer
> {noformat}
> Java accepts both (JLS 6.6.1, 6.6.2), and both compiled and ran before
> GROOVY-12314.
> Cause: StaticTypeCheckingVisitor#hasAccessToClass checks a member type's
> modifiers with the rule used for members, using the enclosing type as the
> receiver:
> * For a type compiled in an earlier step, ClassNode#getOuterClass() is null,
> so the enclosing type is unknown. A protected member type is then checked
> against itself, and no subclass derives from it.
> * Protected access is checked against the calling class alone. JLS 6.6.2
> allows it anywhere in the body of the subclass, including its nested classes,
> closures and anonymous classes.
> * Private access only allows the immediately enclosing type and classes
> nested inside the private type. It should allow anything in the same nest, as
> the existing inSameNest check does.
> Proposed fix: when the enclosing type is unknown, enforce only
> package-private access, so the check rejects only what it can actually
> decide. Otherwise, check the enclosing type's accessibility first, then:
> public => accessible; private => same nest; same package as the enclosing
> type => accessible; protected => the calling class or one of its enclosing
> classes derives from the enclosing type. Every change here makes the rule
> more permissive, so nothing the check accepts today will be rejected. The
> GROOVY-12310 case (a package-private nested enum in a Java class) is still
> rejected.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
