This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new ce37e287af GROOVY-11675: split property may declare final modifier for
accessors
ce37e287af is described below
commit ce37e287afcacbb33e8ff28ba0c0b43b5747f8bf
Author: Eric Milles <[email protected]>
AuthorDate: Wed Sep 17 08:44:56 2025 -0500
GROOVY-11675: split property may declare final modifier for accessors
3_0_X backport
---
.../groovy/ast/tools/PropertyNodeUtils.java | 20 +++-
.../org/codehaus/groovy/classgen/Verifier.java | 36 +++----
.../transform/trait/TraitASTTransformation.java | 34 +++---
src/test/groovy/PropertyTest.groovy | 116 +++++++++++++++------
4 files changed, 132 insertions(+), 74 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/ast/tools/PropertyNodeUtils.java
b/src/main/java/org/codehaus/groovy/ast/tools/PropertyNodeUtils.java
index 513144ad54..c2475c84dd 100644
--- a/src/main/java/org/codehaus/groovy/ast/tools/PropertyNodeUtils.java
+++ b/src/main/java/org/codehaus/groovy/ast/tools/PropertyNodeUtils.java
@@ -23,6 +23,7 @@ import org.codehaus.groovy.ast.PropertyNode;
import java.lang.reflect.Modifier;
public class PropertyNodeUtils {
+
/**
* Fields within the AST that have no explicit visibility are deemed to be
properties
* and represented by a PropertyNode. The Groovy compiler creates accessor
methods and
@@ -30,14 +31,23 @@ public class PropertyNodeUtils {
* from the property are carried over to the backing field (so a property
marked as
* {@code transient} will have a {@code transient} backing field) but when
creating
* the accessor methods we don't carry over modifier values which don't
make sense for
- * methods (this includes VOLATILE and TRANSIENT) but other modifiers are
carried over,
+ * methods (such as {@code volatile} and {@code transient}) but other
modifiers are carried over,
* for example {@code static}.
*
- * @param propNode the original property node
+ * @since 2.4.8
+ * @param node the original property node
* @return the modifiers which make sense for an accessor method
*/
- public static int adjustPropertyModifiersForMethod(PropertyNode propNode) {
- // GROOVY-3726: clear volatile, transient modifiers so that they don't
get applied to methods
- return ~(Modifier.TRANSIENT | Modifier.VOLATILE) &
propNode.getModifiers();
+ public static int adjustPropertyModifiersForMethod(final PropertyNode
node) {
+ // GROOVY-3726, GROOVY-7969: clear modifiers that do not apply to
methods
+ int mods = node.getModifiers() &
~(Modifier.TRANSIENT|Modifier.VOLATILE);
+ /* GROOVY-11675: split property case may declare final modifier
+ if (node.getField() == null || node.getField().isSynthetic()) {
+ mods &= ~Modifier.FINAL;
+ }*/
+ if (node.isStatic()) {
+ mods &= ~Modifier.FINAL;
+ }
+ return mods;
}
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index b5fcf86146..f002498ffc 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -714,23 +714,22 @@ public class Verifier implements GroovyClassVisitor,
Opcodes {
String name = node.getName();
FieldNode field = node.getField();
+ String isserName = "is" + capitalize(name);
String getterName = "get" + capitalize(name);
String setterName = "set" + capitalize(name);
- int accessorModifiers = adjustPropertyModifiersForMethod(node);
-
Statement getterBlock = node.getGetterBlock();
if (getterBlock == null && !node.isPrivate()) {
MethodNode getter = classNode.getGetterMethod(getterName,
!node.isStatic());
if (getter == null &&
node.getType().equals(ClassHelper.boolean_TYPE)) {
- getter = classNode.getGetterMethod("is" + capitalize(name));
+ getter = classNode.getGetterMethod(isserName,
!node.isStatic());
}
if (methodNeedsReplacement(getter)) {
getterBlock = createGetterBlock(node, field);
}
}
Statement setterBlock = node.getSetterBlock();
- if (setterBlock == null && !node.isPrivate() &&
!isFinal(accessorModifiers)) {
+ if (setterBlock == null && !node.isPrivate() &&
!isFinal(node.getModifiers())) {
boolean voidOnly = false; // accept setter with non-void return
type
MethodNode setter = classNode.getSetterMethod(setterName,
voidOnly);
if (methodNeedsReplacement(setter)) {
@@ -738,36 +737,29 @@ public class Verifier implements GroovyClassVisitor,
Opcodes {
}
}
- int getterModifiers = accessorModifiers;
- // don't make static accessors final
- if (node.isStatic()) {
- getterModifiers &= ~ACC_FINAL;
- }
+ int accessorModifiers = adjustPropertyModifiersForMethod(node);
+
if (getterBlock != null) {
- visitGetter(node, getterBlock, getterModifiers, getterName);
+ addAccessMethod(getterName, accessorModifiers, node.getType(),
Parameter.EMPTY_ARRAY, getterBlock);
if (node.getType().equals(ClassHelper.boolean_TYPE) ||
node.getType().equals(ClassHelper.Boolean_TYPE)) {
- String isserName = "is" + capitalize(name);
MethodNode isser = classNode.getGetterMethod(isserName,
!node.isStatic());
if (methodNeedsReplacement(isser)) {
- visitGetter(node, getterBlock, getterModifiers, isserName);
+ addAccessMethod(isserName, accessorModifiers,
node.getType(), Parameter.EMPTY_ARRAY, getterBlock);
}
}
}
if (setterBlock != null) {
- Parameter[] setterParameterTypes = {new Parameter(node.getType(),
"value")};
- MethodNode setter = new MethodNode(setterName, accessorModifiers,
ClassHelper.VOID_TYPE, setterParameterTypes, ClassNode.EMPTY_ARRAY,
setterBlock);
- setter.setSynthetic(true);
- addPropertyMethod(setter);
- visitMethod(setter);
+ Parameter[] setterParameters = {new Parameter(node.getType(),
"value")};
+ addAccessMethod(setterName, accessorModifiers,
ClassHelper.VOID_TYPE, setterParameters, setterBlock);
}
}
- private void visitGetter(final PropertyNode node, final Statement
getterBlock, final int getterModifiers, final String getterName) {
- MethodNode getter = new MethodNode(getterName, getterModifiers,
node.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, getterBlock);
- getter.setSynthetic(true);
- addPropertyMethod(getter);
- visitMethod(getter);
+ private void addAccessMethod(final String name, final int mods, final
ClassNode returnType, final Parameter[] parameters, final Statement
accessBlock) {
+ MethodNode accessor = new MethodNode(name, mods, returnType,
parameters, ClassNode.EMPTY_ARRAY, accessBlock);
+ accessor.setSynthetic(true);
+ addPropertyMethod(accessor);
+ visitMethod(accessor);
}
protected void addPropertyMethod(final MethodNode method) {
diff --git
a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
index 2eb9b0ea86..2184be1bfc 100644
---
a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
+++
b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
@@ -61,6 +61,7 @@ import java.util.LinkedList;
import java.util.List;
import java.util.Set;
+import static java.lang.reflect.Modifier.isFinal;
import static org.apache.groovy.ast.tools.AnnotatedNodeUtils.markAsGenerated;
import static org.apache.groovy.ast.tools.ClassNodeUtils.addGeneratedMethod;
import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
@@ -77,6 +78,7 @@ import static
org.codehaus.groovy.ast.tools.GeneralUtils.params;
import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS;
import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
+import static
org.codehaus.groovy.ast.tools.PropertyNodeUtils.adjustPropertyModifiersForMethod;
import static
org.codehaus.groovy.transform.trait.SuperCallTraitTransformer.UNRESOLVED_HELPER_CLASS;
/**
@@ -390,40 +392,40 @@ public class TraitASTTransformation extends
AbstractASTTransformation implements
private static void processProperty(final ClassNode cNode, final
PropertyNode node) {
String name = node.getName();
FieldNode field = node.getField();
- int propNodeModifiers = node.getModifiers() & 0x1F; // GROOVY-3726
- String getterName = GeneralUtils.getGetterName(node);
- String setterName = GeneralUtils.getSetterName(name);
+ String isserName = "is" + capitalize(name);
+ String getterName = "get" + capitalize(name);
+ String setterName = "set" + capitalize(name);
Statement getterBlock = node.getGetterBlock();
- if (getterBlock == null) {
+ if (getterBlock == null && !node.isPrivate()) {
MethodNode getter = cNode.getGetterMethod(getterName);
if (getter == null &&
node.getType().equals(ClassHelper.boolean_TYPE)) {
- getter = cNode.getGetterMethod("is" + capitalize(name));
+ getter = cNode.getGetterMethod(isserName);
}
- if (!node.isPrivate() && methodNeedsReplacement(cNode, getter)) {
+ if (methodNeedsReplacement(cNode, getter)) {
getterBlock = stmt(fieldX(field));
}
}
Statement setterBlock = node.getSetterBlock();
- if (setterBlock == null) {
- // 2nd arg false below: though not usual, allow setter with
non-void return type
- MethodNode setter = cNode.getSetterMethod(setterName, false);
- if (!node.isPrivate() && (propNodeModifiers & ACC_FINAL) == 0
- && methodNeedsReplacement(cNode, setter)) {
+ if (setterBlock == null && !node.isPrivate() &&
!isFinal(node.getModifiers())) {
+ MethodNode setter = cNode.getSetterMethod(setterName,
/*void-only:*/false);
+ if (methodNeedsReplacement(cNode, setter)) {
setterBlock = assignS(fieldX(field), varX(name));
}
}
+ int methodModifiers = adjustPropertyModifiersForMethod(node); //
GROOVY-3726
+
if (getterBlock != null) {
- MethodNode getter = new MethodNode(getterName, propNodeModifiers,
node.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, getterBlock);
+ MethodNode getter = new MethodNode(getterName, methodModifiers,
node.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, getterBlock);
getter.setSynthetic(true);
addGeneratedMethod(cNode, getter);
if (node.getType().equals(ClassHelper.boolean_TYPE) ||
node.getType().equals(ClassHelper.Boolean_TYPE)) {
- MethodNode secondGetter = new MethodNode("is" +
capitalize(name), propNodeModifiers, node.getType(), Parameter.EMPTY_ARRAY,
ClassNode.EMPTY_ARRAY, getterBlock);
- secondGetter.setSynthetic(true);
- addGeneratedMethod(cNode, secondGetter);
+ getter = new MethodNode(isserName, methodModifiers,
node.getType(), Parameter.EMPTY_ARRAY, ClassNode.EMPTY_ARRAY, getterBlock);
+ getter.setSynthetic(true);
+ addGeneratedMethod(cNode, getter);
}
}
if (setterBlock != null) {
@@ -431,7 +433,7 @@ public class TraitASTTransformation extends
AbstractASTTransformation implements
Parameter setterParameter = new Parameter(node.getType(), name);
var.setAccessedVariable(setterParameter);
- MethodNode setter = new MethodNode(setterName, propNodeModifiers,
ClassHelper.VOID_TYPE, params(setterParameter), ClassNode.EMPTY_ARRAY,
setterBlock);
+ MethodNode setter = new MethodNode(setterName, methodModifiers,
ClassHelper.VOID_TYPE, params(setterParameter), ClassNode.EMPTY_ARRAY,
setterBlock);
setter.setSynthetic(true);
addGeneratedMethod(cNode, setter);
}
diff --git a/src/test/groovy/PropertyTest.groovy
b/src/test/groovy/PropertyTest.groovy
index 09c072d99d..4ba34d3bdb 100644
--- a/src/test/groovy/PropertyTest.groovy
+++ b/src/test/groovy/PropertyTest.groovy
@@ -117,38 +117,92 @@ class PropertyTest extends GroovyTestCase {
assert foo.body == "James"
}
+ void testSplitProperty() {
+ assertScript '''
+ import java.lang.reflect.*
+ import groovy.transform.Generated
+
+ class C {
+ @Deprecated private final Integer one
+ final Integer one
+
+ protected synchronized Integer two
+ synchronized Integer two
+
+ public Integer three
+ @Deprecated Integer three
+ }
+
+ Member m = C.getDeclaredField('one')
+ assert m.isAnnotationPresent(Deprecated)
+ assert m.modifiers == Modifier.PRIVATE + Modifier.FINAL
+
+ m = C.getDeclaredMethod('getOne')
+ assert m.isAnnotationPresent(Generated)
+ assert !m.isAnnotationPresent(Deprecated)
+ assert m.modifiers == Modifier.PUBLIC + Modifier.FINAL
+
+ groovy.test.GroovyAssert.shouldFail(NoSuchMethodException) {
+ m = C.getDeclaredMethod('setOne', Integer)
+ }
+
+ m = C.getDeclaredField('two')
+ assert m.modifiers == Modifier.PRIVATE
+ // field cannot carry modifier SYNCHRONIZED
+
+ m = C.getDeclaredMethod('getTwo')
+ assert m.modifiers == Modifier.PUBLIC + Modifier.SYNCHRONIZED
+ assert m.isAnnotationPresent(Generated)
+
+ m = C.getDeclaredMethod('setTwo', Integer)
+ assert m.modifiers == Modifier.PUBLIC + Modifier.SYNCHRONIZED
+ assert m.isAnnotationPresent(Generated)
+
+ m = C.getDeclaredField('three')
+ assert m.modifiers == Modifier.PRIVATE
+ assert m.isAnnotationPresent(Deprecated)
+
+ m = C.getDeclaredMethod('getThree')
+ assert m.modifiers == Modifier.PUBLIC
+ assert m.isAnnotationPresent(Generated)
+ assert !m.isAnnotationPresent(Deprecated)
+
+ m = C.getDeclaredMethod('setThree', Integer)
+ assert m.modifiers == Modifier.PUBLIC
+ assert m.isAnnotationPresent(Generated)
+ assert !m.isAnnotationPresent(Deprecated)
+ '''
+ }
+
void testFinalProperty() {
- def shell = new GroovyShell();
- assertScript """
- class A {
- final foo = 1
- }
- A.class.declaredMethods.each {
- assert it.name!="setFoo"
-
- }
- assert new A().foo==1
- """
- shouldFail {
- shell.execute """
- class A {
- final foo = 1
- }
- new A().foo = 2
- """
- }
+ assertScript '''
+ class C {
+ final foo = 1
+ }
+ C.declaredMethods.each {
+ assert it.name != "setFoo"
+ }
+
+ assert new C().foo == 1
+ '''
+
+ shouldFail '''
+ class C {
+ final foo = 1
+ }
+
+ new C().foo = 2
+ '''
}
void testFinalField() {
- def shell = new GroovyShell();
- shouldFail {
- shell.execute """
- class A {
- public final foo = 1
- }
- new A().foo = 2
- """
- }
+ shouldFail '''
+ class C {
+ public final foo = 1
+ }
+
+ new C().foo = 2
+ '''
}
void testFinalPropertyWithInheritance() {
@@ -255,7 +309,7 @@ class PropertyTest extends GroovyTestCase {
}
class A extends Base {
private String name = 'AA'
-
+
@Override
String getName() {
this.name
@@ -437,7 +491,7 @@ class PropertyTest extends GroovyTestCase {
static String getAProp() { 'AProp' }
static String getpNAME() { 'pNAME' }
}
-
+
import static A.*
assert prop == 'Prop'
@@ -467,7 +521,7 @@ class PropertyTest extends GroovyTestCase {
static String getAProp() { 'AProp' }
static String getpNAME() { 'pNAME' }
}
-
+
import static A.*
class B {
