You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by su...@apache.org on 2021/05/30 13:41:04 UTC

[groovy] branch master updated: GROOVY-10112: IndexedProperty AST transform should only provide the getter for immutable fields

This is an automated email from the ASF dual-hosted git repository.

sunlan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git


The following commit(s) were added to refs/heads/master by this push:
     new d3d06f7  GROOVY-10112: IndexedProperty AST transform should only provide the getter for immutable fields
d3d06f7 is described below

commit d3d06f7c0d4e5d621345919b4020e35456d1ace6
Author: Paul King <pa...@asert.com.au>
AuthorDate: Thu May 27 15:48:40 2021 +1000

    GROOVY-10112: IndexedProperty AST transform should only provide the getter for immutable fields
---
 .../java/groovy/transform/IndexedProperty.java     |  3 ++-
 .../transform/ImmutableASTTransformation.java      |  2 ++
 .../IndexedPropertyASTTransformation.java          |  8 ++++--
 .../transform/IndexedPropertyTransformTest.groovy  | 30 +++++++++++++++++++---
 4 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/src/main/java/groovy/transform/IndexedProperty.java b/src/main/java/groovy/transform/IndexedProperty.java
index 9ec47b4..d395ce4 100644
--- a/src/main/java/groovy/transform/IndexedProperty.java
+++ b/src/main/java/groovy/transform/IndexedProperty.java
@@ -28,7 +28,8 @@ import java.lang.annotation.Target;
 /**
  * Field annotation used with properties to provide an indexed getter and setter for the property.
  * Groovy provides nice GPath syntax support for accessing indexed properties but Java tools
- * or frameworks may expect the JavaBean style setters and getters.
+ * or frameworks may expect the JavaBean style getters and setters.
+ * Only the getter is produced when an immutable field can be determined.
  * <p>
  * <em>Example usage:</em> suppose you have a class with the following properties:
  * <pre>
diff --git a/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java
index 4149753..b74e318 100644
--- a/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/ImmutableASTTransformation.java
@@ -95,6 +95,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation implem
     private static final Class<? extends Annotation> MY_CLASS = ImmutableBase.class;
     public static final ClassNode MY_TYPE = makeWithoutCaching(MY_CLASS, false);
     private static final String MY_TYPE_NAME = MY_TYPE.getNameWithoutPackage();
+    public static final String IMMUTABLE_BREADCRUMB = "_IMMUTABLE_BREADCRUMB";
 
     private CompilationUnit compilationUnit;
 
@@ -225,6 +226,7 @@ public class ImmutableASTTransformation extends AbstractASTTransformation implem
     private static void adjustPropertyForImmutability(final PropertyNode pNode, final List<PropertyNode> newNodes, final PropertyHandler handler) {
         final FieldNode fNode = pNode.getField();
         fNode.setModifiers((pNode.getModifiers() & (~ACC_PUBLIC)) | ACC_FINAL | ACC_PRIVATE);
+        fNode.setNodeMetaData(IMMUTABLE_BREADCRUMB, Boolean.TRUE);
         pNode.setSetterBlock(null);
         Statement getter = handler.createPropGetter(pNode);
         if (getter != null) {
diff --git a/src/main/java/org/codehaus/groovy/transform/IndexedPropertyASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/IndexedPropertyASTTransformation.java
index c7e51a6..e31affe 100644
--- a/src/main/java/org/codehaus/groovy/transform/IndexedPropertyASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/IndexedPropertyASTTransformation.java
@@ -42,6 +42,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.indexX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.params;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
+import static org.codehaus.groovy.transform.ImmutableASTTransformation.IMMUTABLE_BREADCRUMB;
 import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
 import static org.objectweb.asm.Opcodes.ACC_STATIC;
 
@@ -72,11 +73,14 @@ public class IndexedPropertyASTTransformation extends AbstractASTTransformation
                 return;
             }
             ClassNode fType = fNode.getType();
+            // TODO consider looking for an initial value expression that is a call to asUnmodifiable() or an
+            // explicit call to Collections.unmodifiableList(). But currently that is processed one stage too early.
+            boolean immutable = Boolean.TRUE.equals(fNode.getNodeMetaData(IMMUTABLE_BREADCRUMB));
             if (fType.isArray()) {
-                addArraySetter(fNode);
+                if (!immutable) addArraySetter(fNode);
                 addArrayGetter(fNode);
             } else if (fType.isDerivedFrom(LIST_TYPE)) {
-                addListSetter(fNode);
+                if (!immutable) addListSetter(fNode);
                 addListGetter(fNode);
             } else {
                 addError("Error during " + MY_TYPE_NAME + " processing. Non-Indexable property '" + fNode.getName() +
diff --git a/src/test/org/codehaus/groovy/transform/IndexedPropertyTransformTest.groovy b/src/test/org/codehaus/groovy/transform/IndexedPropertyTransformTest.groovy
index e56e2b2..7b20fbe 100644
--- a/src/test/org/codehaus/groovy/transform/IndexedPropertyTransformTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/IndexedPropertyTransformTest.groovy
@@ -26,7 +26,7 @@ import groovy.test.GroovyShellTestCase
 class IndexedPropertyTransformTest extends GroovyShellTestCase {
 
     void testStandardCase() {
-        assertScript """
+        assertScript '''
             import groovy.transform.IndexedProperty
             class Demo {
                 @IndexedProperty List<Integer> foo
@@ -46,11 +46,11 @@ class IndexedPropertyTransformTest extends GroovyShellTestCase {
             d.baz = ['cat']
             d.setBaz(0, 'dog')
             assert d.getBaz(0) == 'dog'
-        """
+        '''
     }
 
     void testWithCompileStatic() {
-        assertScript """
+        assertScript '''
             import groovy.transform.IndexedProperty
             @groovy.transform.CompileStatic
             class Demo {
@@ -71,7 +71,29 @@ class IndexedPropertyTransformTest extends GroovyShellTestCase {
             d.baz = ['cat']
             d.setBaz(0, 'dog')
             assert d.getBaz(0) == 'dog'
-        """
+        '''
+    }
+
+    void testReadOnlyProperty() {
+        assertScript '''
+            import groovy.transform.*
+            import static groovy.test.GroovyAssert.shouldFail
+
+            class Demo1 {
+                @IndexedProperty List<Integer> foo
+            }
+            @Immutable
+            class Demo2 {
+                @IndexedProperty List<Integer> foo
+            }
+
+            assert Demo1.getMethod('getFoo', int)
+            assert Demo1.getMethod('setFoo', int, Integer)
+            assert Demo2.getMethod('getFoo', int)
+            shouldFail(NoSuchMethodException) {
+                Demo2.getMethod('setFoo', int, Integer)
+            }
+        '''
     }
 
 }
\ No newline at end of file