[
https://issues.apache.org/jira/browse/GROOVY-12290?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18107049#comment-18107049
]
ASF GitHub Bot commented on GROOVY-12290:
-----------------------------------------
Copilot commented on code in PR #2828:
URL: https://github.com/apache/groovy/pull/2828#discussion_r3837894972
##########
src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java:
##########
@@ -2522,10 +2534,32 @@ private boolean isStaticInContext(final MethodNode
method) {
return method instanceof ExtensionMethodNode ? ((ExtensionMethodNode)
method).isStaticExtension() : method.isStatic();
}
- private boolean storeField(final FieldNode field, final PropertyExpression
expressionToStoreOn, final ClassNode receiver, final ClassCodeVisitorSupport
visitor, final String delegationData, final boolean lhsOfAssignment) {
- boolean superField =
isSuperExpression(expressionToStoreOn.getObjectExpression());
- boolean accessible = (!superField &&
receiver.equals(field.getDeclaringClass()) &&
!field.getDeclaringClass().isAbstract()) // GROOVY-7300, GROOVY-11358
+ /**
+ * Determines if the field can back the given property (or attribute)
+ * expression: accessible by Java rules, or admitted by the receiver-type
+ * leniency (GROOVY-7300, GROOVY-11358) that models dynamic Groovy's
+ * permissive private access. Since GROOVY-12290 that leniency no longer
+ * admits plain property syntax to a private field of a foreign nest — a
+ * direct field access that static compilation cannot honour (no access
+ * bridge exists). The deliberate dynamic escape hatches remain: attribute
+ * access (.@), and closure bodies (GROOVY-9195) — including
delegate-resolved
+ * access — whose property dispatch stays dynamic-capable under static
compilation.
+ */
+ private boolean isFieldAccessible(final FieldNode field, final ClassNode
receiver, final PropertyExpression expression, final String delegationData) {
+ boolean superField =
isSuperExpression(expression.getObjectExpression());
+ boolean exactReceiver = (!superField &&
receiver.equals(field.getDeclaringClass()) &&
!field.getDeclaringClass().isAbstract()); // GROOVY-7300, GROOVY-11358
+ if (exactReceiver && field.isPrivate() && delegationData == null
+ && typeCheckingContext.getEnclosingClosure() == null
+ && !(expression instanceof AttributeExpression)
+ && getNestHost(field.getDeclaringClass()) !=
getNestHost(typeCheckingContext.getEnclosingClassNode())) {
Review Comment:
The foreign-nest check uses reference inequality (`!=`) on the `ClassNode`
returned by `getNestHost(...)`. Elsewhere in this class nest-host comparisons
use `.equals(...)`, and `ClassNode` instances representing the same type are
not guaranteed to be the same object. Using `!=` can incorrectly treat
same-nest accesses as “foreign nest”, changing accessibility/readonly behavior.
> Private field access from another class compiles under @TypeChecked but fails
> under @CompileStatic
> --------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12290
> URL: https://issues.apache.org/jira/browse/GROOVY-12290
> Project: Groovy
> Issue Type: Bug
> Reporter: Paul King
> Priority: Major
>
> Accessing a private field of another class via property syntax compiles under
> {{@TypeChecked}} (and runs, thanks to dynamic dispatch's historical leniency
> about {{private}}) but fails to compile under {{@CompileStatic}}:
> {code:groovy}
> class P { private static final Map M = [a: 1] }
> class C {
> @groovy.transform.TypeChecked
> def tc() { P.M } // compiles, runs
> @groovy.transform.CompileStatic
> def cs() { P.M } // "Access to P#M is forbidden"
> }
> {code}
> The equivalent *method* case is already consistent: STC's method selection
> filters candidates by visibility, so a private static method of another class
> is rejected with the same {{[Static type checking]}} error in both modes.
> Field/property resolution performed no such check — the receiver-type
> leniency (GROOVY-7300, GROOVY-11358, which models dynamic Groovy's permissive
> private access) admitted the field, and the access was only refused later, at
> bytecode generation, by {{StaticTypesCallSiteWriter}} ({{"Access to X#Y is
> forbidden"}}) when it must emit a direct field access and no private-access
> bridge exists. Compilation modes should not disagree: whatever passes type
> checking should statically compile (same principle as GROOVY-12289).
> *Fix:* the receiver-type leniency (extracted to
> {{StaticTypeCheckingVisitor#isFieldAccessible}}) no longer admits plain
> property syntax to a private field of a foreign nest, so both modes report
> the standard {{No such property}} error at type-checking time (see also
> GROOVY-7165, whose expected error moves from the late writer message to the
> STC one). The deliberate dynamic escape hatches — which work at runtime in
> *all three* modes today — are preserved:
> * attribute access ({{new P().@f}}), Groovy's explicit encapsulation-piercing
> operator
> * closure bodies (GROOVY-9195), including delegate-resolved access ({{new
> P().with \{ f \}}} and DSL/builder patterns generally) — closure property
> dispatch stays dynamic-capable under static compilation
> * nest-mate access via private bridges: closures reading the owner's private
> fields, inner classes reading outer private statics, trait-private fields
> * property syntax that resolves to an accessible accessor over a private field
> Related adjustments:
> * An inaccessible field no longer suppresses the read-only-property detection
> (GROOVY-9127), so a class with a public getter, an out-of-scope setter and a
> private field now reports {{Cannot set read-only property}} — identically in
> both modes. This supersedes the GROOVY-11956 expectation that the map-style
> constructor case compiles under {{@TypeChecked}} and fails only under
> {{@CompileStatic}} (late, in the writer): it now fails consistently, arguably
> completing that ticket's read-only story.
> * Record component accessors ({{x()}} on precompiled records) now resolve in
> STC's getter lookup, relocating the GROOVY-12225 writer-side fix: {{p.x}}
> types via the accessor instead of leaning on the removed field leniency.
> *Not changed:* the GROOVY-9195 closure exemption preserves a pre-existing
> asymmetry — foreign-private field access is rejected in method bodies but
> permitted in closure bodies (where it compiles to dynamic dispatch and
> works). Unifying that would be a further behavior change with real DSL
> fallout, so it is documented rather than removed.
> *Migration note:* code that today compiles and runs under {{@TypeChecked}}
> only (e.g. tests or frameworks reading another class's internals via property
> syntax from a method body) becomes a compile error. Workarounds are
> straightforward ({{@PackageScope}}, an accessor, explicit {{.@}}, or a
> type-checking extension — the error goes through the unresolved-property
> path, so {{handleUnresolvedProperty}}/{{makeDynamic}} work as escape
> hatches). This tightening should be called out in release notes and targeted
> at Groovy 6.
> Dynamic Groovy is unaffected.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
