You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2026/09/02 11:21:47 UTC
(groovy) 01/03: GROOVY-12314: MOP: treat a field whose access cannot be established as absent
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 3e11ee970e461d77f5f09c338d9fac2a5230988c
Author: Paul King <pa...@asert.com.au>
AuthorDate: Fri Aug 28 12:32:03 2026 +1000
GROOVY-12314: MOP: treat a field whose access cannot be established as absent
A non-public field of a strongly encapsulated class (e.g. a JDK class
without --add-opens) can be selected as a meta property but only fails
when read or written: the indy property path escalated the refusal to
GroovyBugError and attribute access leaked a raw IllegalAccessException.
Skip such fields during selection so the normal missing-member handling
applies (MissingPropertyException / MissingFieldException, as in 4.x),
and make the indy selectors degrade to the generic MetaProperty or
adapter path instead of GroovyBugError. Forceable access (open modules,
class-path classes, --add-opens) is unaffected.
---
src/main/java/groovy/lang/MetaClassImpl.java | 34 ++++-
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 26 ++--
.../codehaus/groovy/reflection/CachedField.java | 15 +-
.../transform/stc/StaticTypeCheckingVisitor.java | 46 +++++-
.../org/codehaus/groovy/vmplugin/v8/Selector.java | 24 +--
.../stc/ParameterizedReceiverTestExtension.groovy | 33 +++++
src/test/groovy/bugs/Groovy12310.groovy | 118 +++++++++++++++
src/test/groovy/bugs/Groovy12314.groovy | 164 +++++++++++++++++++++
src/test/groovy/bugs/Groovy12314Support.java | 31 ++++
src/test/groovy/groovy/MapTest.groovy | 33 +++++
.../stc/FieldsAndPropertiesSTCTest.groovy | 85 +++++++++++
.../packageScope/DifferentPackageTest.groovy | 6 +-
12 files changed, 579 insertions(+), 36 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index 170760a531..053358e6a7 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -1451,7 +1451,13 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
Object value = null;
if (!(metaProperty instanceof ReadOnlyMetaProperty)) {
- value = metaProperty.getProperty(object);
+ try {
+ value = metaProperty.getProperty(object);
+ } catch (GroovyRuntimeException gre) {
+ // GROOVY-12314: a field the reflective path cannot force access to
+ // (strongly encapsulated declaring class) is treated as absent
+ if (!(gre.getCause() instanceof IllegalAccessException)) throw gre;
+ }
} else if (object instanceof Map<?, ?> map) {
value = map.get(methodName);
} else if (object instanceof Script script) {
@@ -3071,8 +3077,14 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
&& (!isMap || isStatic // GROOVY-8065
|| field.isPublic())) { // GROOVY-11367
if (!field.isFinal()) {
- field.setProperty(object, newValue);
- return;
+ try {
+ field.setProperty(object, newValue);
+ return;
+ } catch (GroovyRuntimeException gre) {
+ // GROOVY-12314: the reflective path cannot force access (strongly
+ // encapsulated declaring class): continue as if the field were absent
+ if (!(gre.getCause() instanceof IllegalAccessException)) throw gre;
+ }
} else {
throw new ReadOnlyPropertyException(name, theClass); // GROOVY-5985
}
@@ -3229,6 +3241,12 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
try {
// delegate the get operation to the metaproperty
if (mp != null) return mp.getProperty(object);
+ } catch (GroovyRuntimeException gre) {
+ // GROOVY-12314: a field the reflective path cannot force access to
+ // (strongly encapsulated declaring class) reports missing instead
+ if (!(gre.getCause() instanceof IllegalAccessException)) {
+ throw new GroovyRuntimeException("Cannot read field: " + attribute, gre);
+ }
} catch (Exception e) {
throw new GroovyRuntimeException("Cannot read field: " + attribute, e);
}
@@ -3268,8 +3286,14 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
mp = mbp.getField();
}
if (mp != null) {
- mp.setProperty(object, newValue);
- return;
+ try {
+ mp.setProperty(object, newValue);
+ return;
+ } catch (GroovyRuntimeException gre) {
+ // GROOVY-12314: a field the reflective path cannot force access to
+ // (strongly encapsulated declaring class) reports missing instead
+ if (!(gre.getCause() instanceof IllegalAccessException)) throw gre;
+ }
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
index 577c09d23f..421ed1f5de 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java
@@ -61,7 +61,6 @@ import static org.apache.groovy.ast.tools.ExpressionUtils.isThisExpression;
import static org.apache.groovy.util.BeanUtils.capitalize;
import static org.codehaus.groovy.ast.ClassHelper.CLASS_Type;
import static org.codehaus.groovy.ast.ClassHelper.CLOSURE_TYPE;
-import static org.codehaus.groovy.ast.ClassHelper.COLLECTION_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.GROOVY_OBJECT_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.Iterator_TYPE;
import static org.codehaus.groovy.ast.ClassHelper.LIST_TYPE;
@@ -123,7 +122,6 @@ import static org.objectweb.asm.Opcodes.PUTSTATIC;
public class StaticTypesCallSiteWriter extends CallSiteWriter {
private static final ClassNode INVOKERHELPER_TYPE = ClassHelper.make(InvokerHelper.class);
- private static final MethodNode COLLECTION_SIZE_METHOD = COLLECTION_TYPE.getMethod("size", Parameter.EMPTY_ARRAY);
private static final MethodNode MAP_GET_METHOD = MAP_TYPE.getMethod("get", new Parameter[]{new Parameter(OBJECT_TYPE, "key")});
private static final MethodNode GROOVYOBJECT_GETPROPERTY_METHOD = GROOVY_OBJECT_TYPE.getMethod("getProperty", new Parameter[]{new Parameter(STRING_TYPE, "propertyName")});
private static final MethodNode INVOKERHELPER_GETPROPERTY_METHOD = INVOKERHELPER_TYPE.getMethod("getProperty", new Parameter[]{new Parameter(OBJECT_TYPE, "object"), new Parameter(STRING_TYPE, "propertyName")});
@@ -175,14 +173,10 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
controller.getMethodVisitor().visitInsn(ARRAYLENGTH);
controller.getOperandStack().replace(int_TYPE);
return;
- } else if (isOrImplements(receiverType, COLLECTION_TYPE) && ("size".equals(propertyName) || "length".equals(propertyName))) {
- MethodCallExpression expr = callX(receiver, "size");
- expr.setMethodTarget(COLLECTION_SIZE_METHOD);
- expr.setImplicitThis(implicitThis);
- expr.setSafe(safe);
- expr.visit(controller.getAcg());
- return;
}
+ // GROOVY-12314: no special case for "size"/"length" on Collection receivers: the
+ // type checker no longer admits either name unless a genuine member backs it, and
+ // rewriting an admitted member to size() produced silently wrong values
if (makeGetPropertyWithGetter(receiver, receiverType, propertyName, safe, implicitThis)) return;
@@ -883,8 +877,16 @@ public class StaticTypesCallSiteWriter extends CallSiteWriter {
}*/
private void addPropertyAccessError(final Expression receiver, final String propertyName, final ClassNode receiverType) {
- String receiverName = (receiver instanceof ClassExpression ? receiver.getType() : receiverType).toString(false);
- String message = "Access to " + receiverName + "#" + propertyName + " is forbidden";
- controller.getSourceUnit().addError(new SyntaxException(message, receiver));
+ ClassNode receiverNode = (receiver instanceof ClassExpression ? receiver.getType() : receiverType);
+ // GROOVY-12314: report a placeholder's erasure, not "E"; an unconditional redirect would
+ // instead degrade a parameterized receiver to its declaration's type parameters ("List<E>")
+ if (receiverNode.isGenericsPlaceHolder()) receiverNode = receiverNode.redirect();
+ String message = "Access to " + receiverNode.toString(false) + "#" + propertyName + " is forbidden";
+ int line = receiver.getLineNumber(), column = receiver.getColumnNumber();
+ if (line < 1) { // GROOVY-12314: synthetic receiver: fall back to the current statement position
+ line = controller.getLineNumber();
+ column = 1;
+ }
+ controller.getSourceUnit().addError(new SyntaxException(message, line, column));
}
}
diff --git a/src/main/java/org/codehaus/groovy/reflection/CachedField.java b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
index 77206de2de..97ef6bba43 100644
--- a/src/main/java/org/codehaus/groovy/reflection/CachedField.java
+++ b/src/main/java/org/codehaus/groovy/reflection/CachedField.java
@@ -46,10 +46,19 @@ public class CachedField extends MetaProperty {
}
private final Field field;
- private boolean madeAccessible;
+ private volatile boolean madeAccessible;
+ private boolean accessAttempted; // guarded by synchronization on this
private void makeAccessible() {
- ReflectionUtils.makeAccessibleInPrivilegedAction(field);
- madeAccessible = true;
+ // at most one attempt, remembering either outcome: a failed attempt (strongly
+ // encapsulated declaring class) cannot succeed later. The attempt is recorded
+ // only once it has completed, under synchronization, so a concurrent caller
+ // waits instead of reading the field before setAccessible has taken effect.
+ synchronized (this) {
+ if (!accessAttempted) {
+ madeAccessible = ReflectionUtils.makeAccessibleInPrivilegedAction(field).isPresent();
+ accessAttempted = true;
+ }
+ }
}
/**
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index f2442ad3e9..bd4de48501 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -892,6 +892,13 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
private void recordMissingProperty(final PropertyExpression expression) {
+ FieldNode inaccessibleField = expression.getNodeMetaData("inaccessible field");
+ if (inaccessibleField != null) { // GROOVY-12314: the property exists but Java access rules reject it
+ addStaticTypeError("Cannot access field: " + inaccessibleField.getName() + " of class: " + prettyPrintTypeName(inaccessibleField.getDeclaringClass())
+ + " from class: " + prettyPrintTypeName(typeCheckingContext.getEnclosingClassNode()), expression.getLineNumber() > 0 ? expression : expression.getProperty());
+ return;
+ }
+
var objectExpression = expression.getObjectExpression();
var objectExpressionType = findCurrentInstanceOfClass(objectExpression, getType(objectExpression));
@@ -2482,6 +2489,21 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
return false;
}
+ /**
+ * Determines if the given type itself may be referenced from the accessor
+ * (JLS 6.6.1): a member of an inaccessible type is out of reach no matter
+ * what the member's own modifiers say, because resolving any reference to
+ * the type fails first. A nested type is accessible when its enclosing
+ * type is and its own modifiers admit the accessor.
+ */
+ private static boolean hasAccessToClass(final ClassNode accessor, final ClassNode type) {
+ ClassNode outer = type.getOuterClass();
+ if (outer != null && !hasAccessToClass(accessor, outer)) {
+ return false;
+ }
+ return hasAccessToMember(accessor, outer != null ? outer : type, type.getModifiers());
+ }
+
private ClassNode getTypeForMultiValueExpression(final ClassNode compositeType, final Expression prop) {
GenericsType[] gts = compositeType.getGenericsTypes();
ClassNode itemType = (gts != null && gts.length == 1 ? getCombinedBoundType(gts[0]) : OBJECT_TYPE);
@@ -2594,19 +2616,27 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
* 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
+ * permissive private access. Since GROOVY-12290 (private) and GROOVY-12314
+ * (package-private, protected) that leniency no longer admits plain property
+ * syntax to a field of a foreign nest that Java rules would reject — 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) {
+ // GROOVY-12310: when the qualifying type itself is out of reach (e.g. a public
+ // enum constant of a package-private nested enum), no static emission can name
+ // it -- the JVM rejects the class resolution before it looks at the member
+ if (!hasAccessToClass(typeCheckingContext.getEnclosingClassNode(), receiver)) {
+ return false;
+ }
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
+ if (exactReceiver && delegationData == null
&& typeCheckingContext.getEnclosingClosure() == null
&& !(expression instanceof AttributeExpression)
+ && !hasAccessToMember(typeCheckingContext.getEnclosingClassNode(), field.getDeclaringClass(), field.getModifiers()) // GROOVY-12314
&& !inSameNest(field.getDeclaringClass(), typeCheckingContext.getEnclosingClassNode())) {
exactReceiver = false; // GROOVY-12290
}
@@ -2619,7 +2649,15 @@ out: if ((samParameterTypes.length == 1 && isOrImplements(samParameterTypes[0
if (!accessible) {
if (expressionToStoreOn instanceof AttributeExpression) {
addStaticTypeError("Cannot access field: " + field.getName() + " of class: " + prettyPrintTypeName(field.getDeclaringClass()), expressionToStoreOn.getProperty());
- } else if (!field.isProtected()) { // private or package-private
+ } else { // GROOVY-12314: fall through to accessor, extension or map/list resolution
+ // remember a rejected package-private or protected field declared by the
+ // receiver's own class: if nothing else resolves the property, the error
+ // reports the access violation rather than a missing property. A private
+ // field stays hidden (GROOVY-12290), and an inaccessible inherited field is
+ // not a member of the subclass (JLS 8.2; GROOVY-9093, GROOVY-9293).
+ if (!field.isPrivate() && receiver.equals(field.getDeclaringClass())) {
+ expressionToStoreOn.putNodeMetaData("inaccessible field", field);
+ }
return false;
}
}
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
index 47f8e426c4..198a9a73e3 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
@@ -438,12 +438,16 @@ public abstract class Selector {
insertName = true; // pass "name" field as argument
} else if (mp instanceof CachedField && !mp.isStatic()) {
try {
- // GROOVY-9144, GROOVY-9596: get lookup for sender and unreflect before forcing access
- @SuppressWarnings("removal")
- MethodHandles.Lookup lookup = ((Java8) VMPluginFactory.getPlugin()).newLookup(sender);
- handle = ((CachedField) mp).asAccessMethod(lookup);
+ // GROOVY-9144, GROOVY-9596: unreflect against the call-site lookup before
+ // forcing access; the lookup carries the caller's own access rights, exactly
+ // as the bytecode a Java compiler would emit, so it may still reach e.g. an
+ // inherited protected field (FilterReader#in from a subclass)
+ handle = ((CachedField) mp).asAccessMethod(callSite.getLookup());
} catch (IllegalAccessException e) {
- throw new GroovyBugError(e);
+ // GROOVY-12314: refusal is an access-control outcome, not an internal
+ // error; leave the handle unset so the full MOP path applies -- it
+ // retries the reflective route and converts a reflective failure
+ // into the normal missing-member handling
}
} else {
handle = META_PROPERTY_GETTER.bindTo(mp);
@@ -604,13 +608,13 @@ public abstract class Selector {
: TypeHelper.getWrapperClass(fieldType).isInstance(value);
if (!accepts) return; // needs coercion: adapter path
try {
- // like the property-get field path: lookup for the sender, then unreflect
- @SuppressWarnings("removal")
- MethodHandles.Lookup lookup = ((Java8) VMPluginFactory.getPlugin()).newLookup(sender);
- handle = field.asWriteAccessMethod(lookup);
+ // like the property-get field path: unreflect against the call-site lookup
+ handle = field.asWriteAccessMethod(callSite.getLookup());
if (LOG_ENABLED) LOG.info("direct field write handle set for property write");
} catch (IllegalAccessException e) {
- throw new GroovyBugError(e);
+ // GROOVY-12314: refusal is an access-control outcome, not an internal
+ // error; leave handle unset so the sender-aware adapter path applies
+ if (LOG_ENABLED) LOG.info("direct field write refused, using adapter path");
}
}
diff --git a/src/test-resources/groovy/transform/stc/ParameterizedReceiverTestExtension.groovy b/src/test-resources/groovy/transform/stc/ParameterizedReceiverTestExtension.groovy
new file mode 100644
index 0000000000..25d98de1d5
--- /dev/null
+++ b/src/test-resources/groovy/transform/stc/ParameterizedReceiverTestExtension.groovy
@@ -0,0 +1,33 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import org.codehaus.groovy.ast.GenericsType
+
+import static org.codehaus.groovy.ast.tools.GenericsUtils.makeClassSafeWithGenerics
+
+// resolves every dynamic variable dynamically with type Optional<String> and
+// silences all property missing errors
+unresolvedVariable { var ->
+ if (isDynamic(var)) {
+ makeDynamic(var, makeClassSafeWithGenerics(make(Optional), new GenericsType(STRING_TYPE)))
+ handled = true
+ }
+}
+unresolvedProperty { pexp ->
+ handled = true
+}
diff --git a/src/test/groovy/bugs/Groovy12310.groovy b/src/test/groovy/bugs/Groovy12310.groovy
new file mode 100644
index 0000000000..659ba56f25
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy12310.groovy
@@ -0,0 +1,118 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package bugs
+
+import org.codehaus.groovy.control.MultipleCompilationErrorsException
+import org.junit.jupiter.api.BeforeEach
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.shouldFail
+
+/**
+ * GROOVY-12310: a member of an inaccessible declaring type -- here a public
+ * enum constant of a package-private nested enum, the {@code jakarta.faces}
+ * {@code UIInput.PropertyKeys} shape -- must be rejected by the type checker
+ * instead of compiling to a direct reference that fails at runtime with
+ * {@code IllegalAccessError}.
+ */
+final class Groovy12310 {
+
+ private GroovyShell shell
+
+ @BeforeEach
+ void setUp() {
+ def loader = new GroovyClassLoader()
+ loader.parseClass '''
+ package p
+ class Outer {
+ @groovy.transform.PackageScope
+ enum PropertyKeys { localValueSet, other }
+ }
+ '''
+ shell = new GroovyShell(loader)
+ }
+
+ @Test
+ void testDynamicAccessUnaffected() {
+ assert shell.evaluate('p.Outer.PropertyKeys.localValueSet').name() == 'localValueSet'
+ assert shell.evaluate('p.Outer.PropertyKeys.@localValueSet').name() == 'localValueSet'
+ assert shell.evaluate("p.Outer.PropertyKeys['localValueSet']").name() == 'localValueSet'
+ }
+
+ @Test
+ void testStaticClassReferenceAndSubscriptUnaffected() {
+ assert shell.evaluate('''
+ @groovy.transform.CompileStatic
+ def f() { p.Outer.PropertyKeys }
+ f()
+ ''').simpleName == 'PropertyKeys'
+ assert shell.evaluate('''
+ @groovy.transform.CompileStatic
+ def f() { p.Outer.PropertyKeys['localValueSet'] }
+ f()
+ ''').name() == 'localValueSet'
+ }
+
+ @Test
+ void testStaticPropertyAccessRejected() {
+ // was: compiled to a direct field reference, then at runtime
+ // "IllegalAccessError: failed to access class p.Outer$PropertyKeys"
+ def err = shouldFail MultipleCompilationErrorsException, {
+ shell.evaluate '''
+ @groovy.transform.CompileStatic
+ def f() { p.Outer.PropertyKeys.localValueSet }
+ '''
+ }
+ assert err.message.contains('Cannot access field: localValueSet of class: p.Outer$PropertyKeys')
+ }
+
+ @Test
+ void testStaticAttributeAccessRejected() {
+ def err = shouldFail MultipleCompilationErrorsException, {
+ shell.evaluate '''
+ @groovy.transform.CompileStatic
+ def f() { p.Outer.PropertyKeys.@localValueSet }
+ '''
+ }
+ assert err.message.contains('Cannot access field: localValueSet of class: p.Outer$PropertyKeys')
+ }
+
+ @Test
+ void testMakeDynamicExtensionEscapeHatch() {
+ // an extension may resolve the reference dynamically; the accessibility
+ // rejection must leave that route open (the identifier stays in source,
+ // the emission goes through the dynamic runtime)
+ def extDir = File.createTempDir()
+ new File(extDir, 'Groovy12310Extension.groovy').text = '''
+ unresolvedProperty { pexp ->
+ if (pexp.propertyAsString == 'localValueSet') {
+ makeDynamic(pexp)
+ handled = true
+ }
+ }
+ '''
+ shell.classLoader.addClasspath(extDir.absolutePath)
+ def result = shell.evaluate '''
+ @groovy.transform.CompileStatic(extensions='Groovy12310Extension.groovy')
+ def f() { p.Outer.PropertyKeys.localValueSet }
+ f()
+ '''
+ assert result.name() == 'localValueSet'
+ }
+}
diff --git a/src/test/groovy/bugs/Groovy12314.groovy b/src/test/groovy/bugs/Groovy12314.groovy
new file mode 100644
index 0000000000..7ee5799dc9
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy12314.groovy
@@ -0,0 +1,164 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package bugs
+
+import groovy.test.GroovyAssert
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
+import static org.junit.jupiter.api.Assumptions.assumeFalse
+
+/**
+ * GROOVY-12314: property/attribute access to a field whose reflective access
+ * cannot be established (e.g. a non-public field of a strongly encapsulated
+ * JDK class, absent {@code --add-opens}) must degrade to the normal missing
+ * member handling instead of GroovyBugError or a raw IllegalAccessException.
+ * The JDK field tests assume the JVM does not open {@code java.base/java.util}
+ * and skip themselves when it does.
+ */
+final class Groovy12314 {
+
+ private static void assumeJavaUtilIsEncapsulated() {
+ assumeFalse(ArrayList.class.getModule().isOpen('java.util', Groovy12314.class.getModule()),
+ 'the JVM opens java.base/java.util, so JDK fields are reflectively reachable')
+ }
+
+ @Test
+ void testProtectedJdkFieldPropertyRead() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ // was: GroovyBugError "BUG! UNCAUGHT EXCEPTION: member is protected"
+ shouldFail(MissingPropertyException) {
+ list.modCount
+ }
+ }
+
+ @Test
+ void testPackagePrivateJdkFieldPropertyRead() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ shouldFail(MissingPropertyException) {
+ list.elementData
+ }
+ }
+
+ @Test
+ void testProtectedJdkFieldAttributeRead() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ // was: raw IllegalAccessException escaping from CachedField#getProperty
+ shouldFail(MissingFieldException) {
+ list.@modCount
+ }
+ }
+
+ @Test
+ void testProtectedJdkFieldPropertyWrite() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ // the field exists but cannot be written via property syntax
+ shouldFail(ReadOnlyPropertyException) {
+ list.modCount = 7
+ }
+ }
+
+ @Test
+ void testProtectedJdkFieldAttributeWrite() {
+ assumeJavaUtilIsEncapsulated()
+ def list = [1, 2]
+ shouldFail(MissingFieldException) {
+ list.@modCount = 7
+ }
+ }
+
+ @Test
+ void testPermissiveAccessToOpenClassesUnaffected() {
+ // classes on the class path live in an unnamed (open) module: dynamic
+ // Groovy's permissive field access there must keep working
+ assertScript '''
+ class Holder {
+ private int secret = 42
+ protected int shielded = 43
+ }
+ def h = new Holder()
+ assert h.secret == 42
+ assert h.@secret == 42
+ assert h.shielded == 43
+ assert h.@shielded == 43
+ '''
+ }
+
+ @Test
+ void testPermissiveWriteToOpenClassesUnaffected() {
+ // a non-public field is never given a direct write handle (only public
+ // fields are sender-independent), so the write must actually succeed
+ // through the sender-aware adapter path
+ assertScript '''
+ class Holder {
+ private int secret = 42
+ protected int shielded = 43
+ }
+ def h = new Holder()
+ h.secret = 52
+ assert h.@secret == 52
+ h.shielded = 53
+ assert h.@shielded == 53
+ '''
+ }
+
+ @Test
+ void testPublicFieldOfPackagePrivateClass() {
+ // the call-site lookup cannot see the declaring class, so unreflecting the
+ // public field is refused at first; forcing accessibility and retrying
+ // (GROOVY-9144, GROOVY-9596) must still yield working read/write handles
+ def loader = new GroovyClassLoader()
+ loader.parseClass '''
+ package p
+ @groovy.transform.PackageScope
+ class Holder12314 {
+ public int value = 42
+ }
+ class Factory12314 {
+ static Object make() { new Holder12314() }
+ }
+ '''
+ new GroovyShell(loader).evaluate '''
+ def obj = p.Factory12314.make()
+ assert obj.value == 42
+ obj.value = 52
+ assert obj.value == 52
+ '''
+ }
+
+ @Test
+ void testForbiddenAccessErrorReportsTypeArguments() {
+ // the classgen safety-net error must name the receiver's type arguments
+ // (Optional<String>), not the declaration's type parameters (Optional<T>)
+ def err = shouldFail '''
+ @groovy.transform.CompileStatic(extensions='groovy/transform/stc/ParameterizedReceiverTestExtension.groovy')
+ class C {
+ def foo() {
+ bar.baz
+ }
+ }
+ '''
+ assert err.message =~ /Access to java\.util\.Optional<java\.lang\.String>#baz is forbidden/
+ }
+}
diff --git a/src/test/groovy/bugs/Groovy12314Support.java b/src/test/groovy/bugs/Groovy12314Support.java
new file mode 100644
index 0000000000..da2414717e
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy12314Support.java
@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package bugs;
+
+import java.util.ArrayList;
+
+/**
+ * GROOVY-12314: a Java collection type with public fields named for the
+ * {@code size}/{@code length} property conventions; property access must
+ * resolve to the fields, not be rewritten to {@code Collection#size()}.
+ */
+public class Groovy12314Support extends ArrayList<Object> {
+ public int size = 42;
+ public int length = 99;
+}
diff --git a/src/test/groovy/groovy/MapTest.groovy b/src/test/groovy/groovy/MapTest.groovy
index 3729ec739d..783e161102 100644
--- a/src/test/groovy/groovy/MapTest.groovy
+++ b/src/test/groovy/groovy/MapTest.groovy
@@ -308,6 +308,39 @@ import static groovy.test.GroovyAssert.shouldFail
"""
}
+ // GROOVY-5001, GROOVY-5491, GROOVY-11367, GROOVY-12314
+ @ParameterizedTest @ValueSource(strings=['','static'])
+ void testMapSubclassPropertyAccess(String mode) {
+ assertScript """import groovy.transform.*
+ class M extends HashMap { // just like GROOVY-662, GROOVY-8065, GROOVY-8074
+ def $mode v = 'v'
+ public $mode w = 'w'
+ protected $mode x = 'x'
+ @PackageScope $mode y = 'y'
+ private $mode z = 'z'
+ }
+
+ def map = new M()
+ assert map.v == 'v' // property accessor before map entry
+ assert map.w == 'w' // public field before map entry
+ assert map.x == null // non-public field: map entry, even though absent
+ assert map.y == null // and even though the caller is in the same package
+ assert map.z == null
+
+ ['v','w','x','y','z'].each { map.put(it, 'entry') }
+
+ assert map.v == 'v'
+ assert map.w == 'w'
+ assert map.x == 'entry'
+ assert map.y == 'entry'
+ assert map.z == 'entry'
+
+ assert map.@x == 'x' // attribute access still reaches the field
+ assert map.@y == 'y'
+ assert map.@z == 'z'
+ """
+ }
+
@Test
void testMapLeftShift() {
def map = [a:1, b:2]
diff --git a/src/test/groovy/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy b/src/test/groovy/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
index 555bab9206..4e1ab160f3 100644
--- a/src/test/groovy/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
+++ b/src/test/groovy/groovy/transform/stc/FieldsAndPropertiesSTCTest.groovy
@@ -1992,6 +1992,91 @@ class FieldsAndPropertiesSTCTest extends StaticTypeCheckingTestCase {
'''
}
+ // GROOVY-12314: package-private and protected fields that Java access rules
+ // reject are not admitted via property syntax either; resolution falls
+ // through to accessors, extensions and map/list handling. When nothing else
+ // resolves the property, a field declared by the receiver's own class is
+ // reported as an access violation (as javac does), while an inaccessible
+ // inherited field is not a member of the subclass (JLS 8.2) and stays missing
+ @Test
+ void testNonPublicForeignFieldViaPropertySyntax() {
+ shouldFailWithMessages '''
+ def m(ArrayList list) { list.modCount } // protected, inherited from java.util.AbstractList
+ ''',
+ 'No such property: modCount for class: java.util.ArrayList'
+
+ shouldFailWithMessages '''
+ def m(ArrayList list) { list.elementData } // package-private, declared by java.util.ArrayList
+ ''',
+ 'Cannot access field: elementData of class: java.util.ArrayList from class: '
+
+ shouldFailWithMessages '''
+ def m(ArrayList list) { list.modCount = 1 }
+ ''',
+ 'No such property: modCount for class: java.util.ArrayList'
+
+ shouldFailWithMessages '''
+ def m(ArrayList list) { list.elementData = null }
+ ''',
+ 'Cannot access field: elementData of class: java.util.ArrayList from class: '
+ }
+
+ // GROOVY-12314: "size" and "length" are methods, not properties, of collections
+ @Test
+ void testCollectionSizeIsNotAProperty() {
+ shouldFailWithMessages '''
+ List list = [1, 2]
+ list.size
+ ''',
+ 'No such property: size for class: java.util.ArrayList'
+
+ shouldFailWithMessages '''
+ List list = [1, 2]
+ list.length
+ ''',
+ 'No such property: length for class: java.util.ArrayList'
+ }
+
+ // GROOVY-12314: a public field named "size" or "length" on a Java collection
+ // type is honoured; it is not rewritten to Collection#size()
+ @Test
+ void testPublicSizeOrLengthFieldOfCollectionType() {
+ assertScript '''
+ def j = new bugs.Groovy12314Support()
+ j.add(1)
+ assert j.size == 42
+ assert j.length == 99
+ '''
+ assertScript '''
+ class SizedGroovy extends ArrayList<Object> {
+ public int size = 42
+ }
+ def g = new SizedGroovy()
+ g.add(1)
+ assert g.size == 42
+ '''
+ }
+
+ // GROOVY-12314: Java access rules still admit these
+ @Test
+ void testAccessibleNonPublicFieldViaPropertySyntax() {
+ assertScript '''
+ class A { protected int x = 1 }
+ class B extends A { def m() { x + this.x } }
+ assert new B().m() == 2
+ '''
+ assertScript '''
+ class A { protected int x = 1 }
+ class B { def m(A a) { a.x } } // same package
+ assert new B().m(new A()) == 1
+ '''
+ assertScript '''
+ class A { @groovy.transform.PackageScope int x = 1 }
+ class B { def m(A a) { a.x } } // same package
+ assert new B().m(new A()) == 1
+ '''
+ }
+
// GROOVY-5737
@Test
void testGeneratedFieldAccessInClosure() {
diff --git a/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy b/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
index c03cc964a4..28dc21f0a3 100644
--- a/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
+++ b/src/test/groovy/org/codehaus/groovy/transform/packageScope/DifferentPackageTest.groovy
@@ -367,7 +367,8 @@ final class DifferentPackageTest {
'''
)
}
- assert err.message =~ /Access to p.One#value is forbidden/
+ // GROOVY-12314: rejected by the type checker as an access violation, no longer during class generation
+ assert err.message =~ /Cannot access field: value of class: p.One from class: q.Two/
}
// GROOVY-9093
@@ -431,7 +432,8 @@ final class DifferentPackageTest {
'''
)
}
- assert err.message =~ /Access to p.One#CONST is forbidden/
+ // GROOVY-12314: rejected by the type checker as an access violation, no longer during class generation
+ assert err.message =~ /Cannot access field: CONST of class: p.One from class: q.Other/
}
@Test