Repository navigation
GROOVY-12509: STC: allow members of a private nested type from another class of the same nest - #3044
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3044 +/- ##
==================================================
+ Coverage 73.6107% 73.6116% +0.0009%
Complexity 42321 42321
==================================================
Files 1657 1657
Lines 145631 145632 +1
Branches 27470 27470
==================================================
+ Hits 107200 107202 +2
Misses 28466 28466
+ Partials 9965 9964 -1
🚀 New features to boost your workflow:
|
5252718 to
ca201ca
Compare
This comment has been minimized.
This comment has been minimized.
|
The failed Could someone with the rights rerun the failed job? |
|
Is it possible to step this in with smaller increments? It is hard to consider all the implications of such changes when they come all at once. Nest mates and private access went through a lot of careful consideration leading up to Groovy 5 release. |
|
hi @eric-milles, yeah, I can break this up |
…r class of the same nest Since GROOVY-12310 (6.0.0), the type checker rejects a field whose qualifying type is out of reach, through hasAccessToClass. That check did not know about nests, so a public field or enum constant of a private nested type, such as A.Hidden.x or A.E.ONE, failed with "Cannot access field" when used from a sibling of A or even from the outer class. Groovy 5.0.8 compiles and runs the same code, and javac accepts it (JLS 6.6.1). Methods and constructors of the same private type were never affected. hasAccessToClass now treats every type of a nest as accessible from every other type of the same nest. A private nested type of another top-level class is still rejected, and so is the package-private nested type of another package that GROOVY-12310 is about.
ca201ca to
8fbea12
Compare
|
I've cut this down to the first step: the 6.0.0 regression for fields of a private nested type (GROOVY-12310). The rest follows one pull request at a time, each opened after the previous one is merged:
Each description lists the code that stops compiling and anything that behaves differently under |
daniellansun
left a comment
There was a problem hiding this comment.
Looks good to me.
hasAccessToClass was applying the member-access rule with the enclosing type as receiver, so a private nested type was treated as unreachable from another type of the same nest even when the field itself was public (or an enum constant). The early inSameNest check matches JLS 6.6.1 / nestmates, and the existing isFieldAccessible nestmate path still covers the private-field case in the test.
Thanks for keeping this to the GROOVY-12310 regression and for the negative test on another top-level nest. The matrix (sibling, outer, write, attribute, closure, double nesting, enum) is exactly what I want to see here.
One process note: #3052 rewrites the same method more broadly (protected + precompiled outer). Whichever lands second will need a careful rebase; the tests in this PR should stay.
+1 for master, and I agree it is a candidate for GROOVY_6_0_X.
Since GROOVY-12310 (6.0.0), the type checker rejects a field whose qualifying type is out of reach. That check does not know about nests, so a public field or an enum constant of a private nested type fails with "Cannot access field" when it is used from another class of the same top-level class, even from the outer class itself. 5.0.8 compiles the same code, and javac accepts it (JLS 6.6.1).
Every type of a nest is now accessible from every other type of the same nest. A private nested type of another top-level class is still rejected, and so is the package-private nested type of another package that GROOVY-12310 is about. Methods and constructors of such a type were never affected.
This is the first part of GROOVY-12509. Private methods, constructors and accessors of another class of the same nest follow in separate pull requests.
Tests
FieldsAndPropertiesSTCTest, also run statically compiled byFieldsAndPropertiesStaticCompileTest: reads, a write, an attribute, a closure, a doubly nested private type and an enum constant, from a sibling and from the outer class, and a private nested type of another top-level class, which stays rejected. The first test fails without the change. 14,940 tests in thegroovy,gls,bugs,org.codehaus.groovy.classgen,transform,runtime,reflection,vmplugin,controlandastpackages and the stub generator tests pass, apart frombugs.Groovy12363, which fails on master too when it runs afterbugs.Groovy12142in the same JVM (#3025 fixes it).groovy-contracts,groovy-typecheckers,groovy-ginq,groovy-macro-library,groovy-json,groovy-xmlandgroovy-templatespass their tests, and every subproject compiles its main and test sources. The Checkstyle and CodeNarc gates pass.Backport
This is a regression in 6.0.0, so the change is a candidate for
GROOVY_6_0_X.