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 2021/04/01 04:30:37 UTC

[groovy] branch master updated (a93b782 -> 2381666)

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

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


    from a93b782  GROOVY-10004: @Lazy transform should check for explicit getters/setters (closes #1537)
     new 33d033a  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (package scope should know about split properties)
     new 60b6ccd  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (formatting only)
     new 34d12df  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (test cases for properties with init values)
     new cd8730b  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (tweak existing test)
     new 6a78b75  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (documentation and property with annotation example)
     new be41cee  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (delayed checking of duplicate fields and properties)
     new dede595  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (ignore @CompileStatic on properties - never used, just transferred to accessor methods for a split property)
     new 5291c10  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (ignore AST transforms on property nodes - never relevant just transferred to accessor methods for split properties)
     new e93cb4c  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (adjust verifier to know about copying annotations for split properties)
     new 2381666  GROOVY-9993: Field and a property with the same name: clarification of boundary cases (delay duplicate checking, instead check for mismatch types and expand split properties up front) (closes #1538)

The 10 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../apache/groovy/parser/antlr4/AstBuilder.java    | 77 +++++++++++------
 .../groovy/classgen/ClassCompletionVerifier.java   |  7 ++
 .../org/codehaus/groovy/classgen/Verifier.java     | 21 ++++-
 .../groovy/transform/ASTTransformationVisitor.java |  5 ++
 .../transform/PackageScopeASTTransformation.java   | 25 +++++-
 .../transform/StaticTypesTransformation.java       |  3 +-
 .../transform/sc/StaticCompileTransformation.java  |  3 +-
 src/spec/doc/core-object-orientation.adoc          | 76 ++++++++++++++++-
 src/spec/test/ClassTest.groovy                     | 27 ++++--
 .../MultipleDefinitionOfSameVariableTest.groovy    | 98 ++++++++++++++--------
 .../org/codehaus/groovy/ast/LineColumnCheck.txt    |  2 +-
 11 files changed, 268 insertions(+), 76 deletions(-)

[groovy] 10/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (delay duplicate checking, instead check for mismatch types and expand split properties up front) (closes #1538)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 2381666cf136b247574e00c1db2bb40648ab562a
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 21:10:17 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (delay duplicate checking, instead check for mismatch types and expand split properties up front) (closes #1538)
---
 .../apache/groovy/parser/antlr4/AstBuilder.java    | 77 +++++++++++++++-------
 .../org/codehaus/groovy/ast/LineColumnCheck.txt    |  2 +-
 2 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
index 244b420..526d541 100644
--- a/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
+++ b/src/main/java/org/apache/groovy/parser/antlr4/AstBuilder.java
@@ -20,6 +20,7 @@ package org.apache.groovy.parser.antlr4;
 
 import groovy.lang.Tuple2;
 import groovy.lang.Tuple3;
+import groovy.transform.CompileStatic;
 import groovy.transform.Trait;
 import org.antlr.v4.runtime.ANTLRErrorListener;
 import org.antlr.v4.runtime.CharStream;
@@ -115,6 +116,7 @@ import org.codehaus.groovy.ast.stmt.ThrowStatement;
 import org.codehaus.groovy.ast.stmt.TryCatchStatement;
 import org.codehaus.groovy.ast.stmt.WhileStatement;
 import org.codehaus.groovy.ast.tools.ClosureUtils;
+import org.codehaus.groovy.classgen.Verifier;
 import org.codehaus.groovy.control.CompilationFailedException;
 import org.codehaus.groovy.control.CompilePhase;
 import org.codehaus.groovy.control.SourceUnit;
@@ -164,6 +166,9 @@ import static org.apache.groovy.parser.antlr4.GroovyParser.STATIC;
 import static org.apache.groovy.parser.antlr4.GroovyParser.SUB;
 import static org.apache.groovy.parser.antlr4.GroovyParser.VAR;
 import static org.apache.groovy.parser.antlr4.util.PositionConfigureUtils.configureAST;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.assignX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX;
 import static org.codehaus.groovy.classgen.asm.util.TypeUtil.isPrimitiveType;
 import static org.codehaus.groovy.runtime.DefaultGroovyMethods.asBoolean;
 import static org.codehaus.groovy.runtime.DefaultGroovyMethods.last;
@@ -1666,67 +1671,89 @@ public class AstBuilder extends GroovyParserBaseVisitor<Object> {
         return null;
     }
 
-    private void declareProperty(final VariableDeclarationContext ctx, final ModifierManager modifierManager, final ClassNode variableType, final ClassNode classNode, final int i, final VariableExpression variableExpression, final String fieldName, final int modifiers, final Expression initialValue) {
-        if (classNode.hasProperty(fieldName)) {
-            throw createParsingFailedException("The property '" + fieldName + "' is declared multiple times", ctx);
+    private static class PropertyExpander extends Verifier {
+        private PropertyExpander(final ClassNode cNode) {
+            setClassNode(cNode);
+        }
+
+        @Override
+        protected Statement createSetterBlock(PropertyNode propertyNode, FieldNode field) {
+            return stmt(assignX(varX(field), varX(VALUE_STR, field.getType())));
         }
 
+        @Override
+        protected Statement createGetterBlock(PropertyNode propertyNode, FieldNode field) {
+            return stmt(varX(field));
+        }
+    }
+
+    private void declareProperty(final VariableDeclarationContext ctx, final ModifierManager modifierManager, final ClassNode variableType, final ClassNode classNode, final int i, final VariableExpression variableExpression, final String fieldName, final int modifiers, final Expression initialValue) {
         PropertyNode propertyNode;
         FieldNode fieldNode = classNode.getDeclaredField(fieldName);
 
         if (fieldNode != null && !classNode.hasProperty(fieldName)) {
             if (fieldNode.hasInitialExpression() && initialValue != null) {
-                throw createParsingFailedException("A field and a property have the same name '" + fieldName + "' and both have initial values", ctx);
+                throw createParsingFailedException("The split property definition named '" + fieldName + "' must not have an initial value for both the field and the property", ctx);
+            }
+            if (!fieldNode.getType().equals(variableType)) {
+                throw createParsingFailedException("The split property definition named '" + fieldName + "' must not have different types for the field and the property", ctx);
             }
             classNode.getFields().remove(fieldNode);
             propertyNode = new PropertyNode(fieldNode, modifiers | Opcodes.ACC_PUBLIC, null, null);
             classNode.addProperty(propertyNode);
+            if (initialValue != null) {
+                fieldNode.setInitialValueExpression(initialValue);
+            }
+            modifierManager.attachAnnotations(propertyNode);
+            propertyNode.addAnnotation(new AnnotationNode(ClassHelper.make(CompileStatic.class)));
+            // expand properties early so AST transforms will be handled correctly
+            PropertyExpander expander = new PropertyExpander(classNode);
+            expander.visitProperty(propertyNode);
         } else {
-            propertyNode =
-                    classNode.addProperty(
-                            fieldName,
-                            modifiers | Opcodes.ACC_PUBLIC,
-                            variableType,
-                            initialValue,
-                            null,
-                            null);
+            propertyNode = new PropertyNode(fieldName, modifiers | Opcodes.ACC_PUBLIC, variableType, classNode, initialValue, null, null);
+            classNode.addProperty(propertyNode);
 
             fieldNode = propertyNode.getField();
+            fieldNode.setModifiers(modifiers & ~Opcodes.ACC_PUBLIC | Opcodes.ACC_PRIVATE);
+            fieldNode.setSynthetic(!classNode.isInterface());
+            modifierManager.attachAnnotations(fieldNode);
+            modifierManager.attachAnnotations(propertyNode);
+            if (i == 0) {
+                configureAST(fieldNode, ctx, initialValue);
+            } else {
+                configureAST(fieldNode, variableExpression, initialValue);
+            }
         }
 
-        fieldNode.setModifiers(modifiers & ~Opcodes.ACC_PUBLIC | Opcodes.ACC_PRIVATE);
-        fieldNode.setSynthetic(!classNode.isInterface());
-        modifierManager.attachAnnotations(fieldNode);
-
         groovydocManager.handle(fieldNode, ctx);
         groovydocManager.handle(propertyNode, ctx);
 
         if (i == 0) {
-            configureAST(fieldNode, ctx, initialValue);
             configureAST(propertyNode, ctx, initialValue);
         } else {
-            configureAST(fieldNode, variableExpression, initialValue);
             configureAST(propertyNode, variableExpression, initialValue);
         }
     }
 
     private void declareField(final VariableDeclarationContext ctx, final ModifierManager modifierManager, final ClassNode variableType, final ClassNode classNode, final int i, final VariableExpression variableExpression, final String fieldName, final int modifiers, final Expression initialValue) {
-        FieldNode existingFieldNode = classNode.getDeclaredField(fieldName);
-        if (null != existingFieldNode && !existingFieldNode.isSynthetic()) {
-            throw createParsingFailedException("The field '" + fieldName + "' is declared multiple times", ctx);
-        }
-
         FieldNode fieldNode;
         PropertyNode propertyNode = classNode.getProperty(fieldName);
 
         if (null != propertyNode && propertyNode.getField().isSynthetic()) {
             if (propertyNode.hasInitialExpression() && initialValue != null) {
-                throw createParsingFailedException("A field and a property have the same name '" + fieldName + "' and both have initial values", ctx);
+                throw createParsingFailedException("The split property definition named '" + fieldName + "' must not have an initial value for both the field and the property", ctx);
+            }
+            if (!propertyNode.getType().equals(variableType)) {
+                throw createParsingFailedException("The split property definition named '" + fieldName + "' must not have different types for the field and the property", ctx);
             }
             classNode.getFields().remove(propertyNode.getField());
-            fieldNode = new FieldNode(fieldName, modifiers, variableType, classNode.redirect(), initialValue);
+            fieldNode = new FieldNode(fieldName, modifiers, variableType, classNode.redirect(), propertyNode.hasInitialExpression() ? propertyNode.getInitialExpression() : initialValue);
             propertyNode.setField(fieldNode);
+            propertyNode.addAnnotation(new AnnotationNode(ClassHelper.make(CompileStatic.class)));
             classNode.addField(fieldNode);
+            // expand properties early so AST transforms will be handled correctly
+            PropertyExpander expander = new PropertyExpander(classNode);
+            expander.visitProperty(propertyNode);
         } else {
             fieldNode =
                     classNode.addField(
diff --git a/src/test/org/codehaus/groovy/ast/LineColumnCheck.txt b/src/test/org/codehaus/groovy/ast/LineColumnCheck.txt
index 8ce27ff..4148608 100644
--- a/src/test/org/codehaus/groovy/ast/LineColumnCheck.txt
+++ b/src/test/org/codehaus/groovy/ast/LineColumnCheck.txt
@@ -39,7 +39,7 @@ class Test {
 :::[FieldNode,(2:2),(2:24)][ConstantExpression,(2:17),(2:24)];
 [FieldNode,(3:2),(3:15)];
 [FieldNode,(4:2),(4:26)];
-[FieldNode,(6:2),(6:15)][ConstantExpression,(5:26),(5:40)];
+[FieldNode,(5:2),(5:40)][ConstantExpression,(5:26),(5:40)];
 [FieldNode,(8:2),(8:38)][ConstantExpression,(8:25),(8:38)]
 
 ###wholeAnnotationExpressionSelection:::

[groovy] 03/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (test cases for properties with init values)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 34d12dfc980377f2cfbdd589298faad03469d87e
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 21:00:26 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (test cases for properties with init values)
---
 .../MultipleDefinitionOfSameVariableTest.groovy    | 30 ++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/src/test/gls/scope/MultipleDefinitionOfSameVariableTest.groovy b/src/test/gls/scope/MultipleDefinitionOfSameVariableTest.groovy
index 82b7f5a..f0ebb42 100644
--- a/src/test/gls/scope/MultipleDefinitionOfSameVariableTest.groovy
+++ b/src/test/gls/scope/MultipleDefinitionOfSameVariableTest.groovy
@@ -163,4 +163,34 @@ class MultipleDefinitionOfSameVariableTest extends CompilableTestSupport {
         '''
     }
 
+    void testFieldAndPropertyWithInit() {
+        assertScript '''
+            class X {
+                def foo = 3
+                public foo
+                public bar
+                def bar = 4
+            }
+
+            def x = new X()
+            def result = [x.foo, x.bar]
+            assert result == [3, 4]
+        '''
+    }
+
+    void testPropertyAndFieldWithInit() {
+        assertScript '''
+            class Y {
+                def foo
+                public foo = 5
+                public bar = 6
+                def bar
+            }
+
+            def y = new Y()
+            result = [y.foo, y.bar]
+            assert result == [5, 6]
+        '''
+    }
+
 }
\ No newline at end of file

[groovy] 08/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (ignore AST transforms on property nodes - never relevant just transferred to accessor methods for split properties)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 5291c10a46a1393c9feb90ba29be58240b820030
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 21:05:59 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (ignore AST transforms on property nodes - never relevant just transferred to accessor methods for split properties)
---
 .../java/org/codehaus/groovy/transform/ASTTransformationVisitor.java | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java b/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java
index 140858c..1483d08 100644
--- a/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/ASTTransformationVisitor.java
@@ -28,6 +28,7 @@ import org.codehaus.groovy.ast.AnnotationNode;
 import org.codehaus.groovy.ast.ClassCodeVisitorSupport;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.GroovyClassVisitor;
+import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.classgen.GeneratorContext;
 import org.codehaus.groovy.control.ASTTransformationsContext;
@@ -199,6 +200,10 @@ public final class ASTTransformationVisitor extends ClassCodeVisitorSupport {
         return result;
     }
 
+    @Override
+    public void visitProperty(PropertyNode node) {
+        // ignore: we'll already have allocated to field or accessor method by now
+    }
 
     public static void addPhaseOperations(final CompilationUnit compilationUnit) {
         ASTTransformationsContext context = compilationUnit.getASTTransformationsContext();

[groovy] 06/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (delayed checking of duplicate fields and properties)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit be41cee8ce94996dd3385e559e7936271337a414
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 21:03:06 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (delayed checking of duplicate fields and properties)
---
 .../java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index a199fae..fb26380 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -293,6 +293,10 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport {
         return "field '" + node.getName() + "'";
     }
 
+    private static String getDescription(PropertyNode node) {
+        return "property '" + node.getName() + "'";
+    }
+
     private static String getDescription(Parameter node) {
         return "parameter '" + node.getName() + "'";
     }
@@ -537,6 +541,9 @@ public class ClassCompletionVerifier extends ClassCodeVisitorSupport {
 
     @Override
     public void visitProperty(PropertyNode node) {
+        if (currentClass.getProperty(node.getName()) != node) {
+            addError("The " + getDescription(node) + " is declared multiple times.", node);
+        }
         checkDuplicateProperties(node);
         checkGenericsUsage(node, node.getType());
         super.visitProperty(node);

[groovy] 02/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (formatting only)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 60b6ccd6fc92eaa14f1561f8b2f3a4a2a7e6cd07
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 20:59:40 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (formatting only)
---
 .../MultipleDefinitionOfSameVariableTest.groovy    | 68 +++++++++++-----------
 1 file changed, 34 insertions(+), 34 deletions(-)

diff --git a/src/test/gls/scope/MultipleDefinitionOfSameVariableTest.groovy b/src/test/gls/scope/MultipleDefinitionOfSameVariableTest.groovy
index 146cd49..82b7f5a 100644
--- a/src/test/gls/scope/MultipleDefinitionOfSameVariableTest.groovy
+++ b/src/test/gls/scope/MultipleDefinitionOfSameVariableTest.groovy
@@ -23,68 +23,68 @@ import gls.CompilableTestSupport
 class MultipleDefinitionOfSameVariableTest extends CompilableTestSupport {
 
     void testInSameBlock() {
-        shouldNotCompile """
+        shouldNotCompile '''
             def foo = 1
             def foo = 2
-        """
+        '''
 
-        shouldNotCompile """
+        shouldNotCompile '''
             class Foo {
                 def foo() {
                     def bar=1
                     def bar=2
                 }
             }
-        """
+        '''
     }
 
     void testInSubBlocks() {
-        shouldNotCompile """
+        shouldNotCompile '''
              def foo = 1
              5.times { def foo=2 }
-        """
+        '''
 
-        shouldNotCompile """
+        shouldNotCompile '''
             def foo = 1
             label1: { def foo=2 }
-        """
+        '''
 
-        shouldNotCompile """
+        shouldNotCompile '''
             def foo = 1
             for (i in []) { def foo=2 }
-        """
+        '''
 
-        shouldNotCompile """
+        shouldNotCompile '''
             def foo = 1
             while (true) { def foo=2 }
-        """
+        '''
     }
 
     void testInNestedClosure() {
-        shouldNotCompile """
+        shouldNotCompile '''
             def foo = 1
             5.times { 6.times {def foo=2 }
-        """
+        '''
 
-        assertScript """
+        assertScript '''
             def foo = 1
             5.times { 6.times {foo=2 } }
             assert foo == 2
-        """
+        '''
     }
 
     void testBindingHiding() {
-        assertScript """
+        assertScript '''
             foo = 1
             def foo = 3
             assert foo==3
             assert this.foo == 1
             assert binding.foo == 1
-        """
+        '''
     }
 
     void testBindingAccessInMethod() {
-        assertScript """
+        assertScript '''
             def methodUsingBinding() {
                 try {
                     s = "  bbb  ";
@@ -95,72 +95,72 @@ class MultipleDefinitionOfSameVariableTest extends CompilableTestSupport {
             }
             methodUsingBinding()
             assert s == "bbb"
-        """
+        '''
     }
 
     void testMultipleOfSameName() {
-        shouldNotCompile """
+        shouldNotCompile '''
             class DoubleField {
                 def zero = 0
                 def zero = 0
             }
-        """
+        '''
     }
 
     void testPropertyField() {
-        shouldCompile """
+        shouldCompile '''
             class A {
                 def foo
                 private foo
             }
-        """
+        '''
     }
 
     void testPropertyFieldBothInit() {
-        shouldNotCompile """
+        shouldNotCompile '''
             class A {
                 def foo = 3
                 private foo = 4
             }
-        """
+        '''
     }
 
     void testFieldProperty() {
-        shouldCompile """
+        shouldCompile '''
             class A {
                 private foo
                 def foo
             }
-        """
+        '''
     }
 
     void testFieldPropertyBothInit() {
-        shouldNotCompile """
+        shouldNotCompile '''
             class A {
                 private foo = 'a'
                 def foo = 'b'
             }
-        """
+        '''
     }
 
     void testFieldPropertyProperty() {
-        shouldNotCompile """
+        shouldNotCompile '''
             class A {
                 private foo
                 def foo
                 def foo
             }
-        """
+        '''
     }
 
     void testPropertyFieldField() {
-        shouldNotCompile """
+        shouldNotCompile '''
             class A {
                 def foo
                 private foo
                 private foo
             }
-        """
+        '''
     }
 
 }
\ No newline at end of file

[groovy] 09/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (adjust verifier to know about copying annotations for split properties)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit e93cb4ce8c22b4f2ab2fe96447e72fa7cc749d04
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 21:08:54 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (adjust verifier to know about copying annotations for split properties)
---
 .../java/org/codehaus/groovy/classgen/Verifier.java | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 4db0de2..9bdd9a6 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -97,6 +97,7 @@ import static org.apache.groovy.ast.tools.ExpressionUtils.transformInlineConstan
 import static org.apache.groovy.ast.tools.MethodNodeUtils.getCodeAsBlock;
 import static org.apache.groovy.ast.tools.MethodNodeUtils.getPropertyName;
 import static org.apache.groovy.ast.tools.MethodNodeUtils.methodDescriptorWithoutReturnType;
+import static org.codehaus.groovy.ast.AnnotationNode.METHOD_TARGET;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.binX;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.block;
 import static org.codehaus.groovy.ast.tools.GeneralUtils.bytecodeX;
@@ -712,13 +713,13 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             getterModifiers &= ~ACC_FINAL;
         }
         if (getterBlock != null) {
-            visitGetter(node, getterBlock, getterModifiers, getterName);
+            visitGetter(node, field, getterBlock, getterModifiers, getterName);
 
             if (node.getGetterName() == null && getterName.startsWith("get") && (node.getType().equals(ClassHelper.boolean_TYPE) || node.getType().equals(ClassHelper.Boolean_TYPE))) {
                 String altGetterName = "is" + capitalize(name);
                 MethodNode altGetter = classNode.getGetterMethod(altGetterName, !node.isStatic());
                 if (methodNeedsReplacement(altGetter)) {
-                    visitGetter(node, getterBlock, getterModifiers, altGetterName);
+                    visitGetter(node, field, getterBlock, getterModifiers, altGetterName);
                 }
             }
         }
@@ -727,17 +728,31 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
             MethodNode setter = new MethodNode(setterName, accessorModifiers, ClassHelper.VOID_TYPE, setterParameterTypes, ClassNode.EMPTY_ARRAY, setterBlock);
             setter.setSynthetic(true);
             addPropertyMethod(setter);
+            if (!field.isSynthetic()) {
+                copyMethodAnnotations(node, setter);
+            }
             visitMethod(setter);
         }
     }
 
-    private void visitGetter(final PropertyNode node, final Statement getterBlock, final int getterModifiers, final String getterName) {
+    private void visitGetter(final PropertyNode node, final FieldNode field, 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);
+        if (!field.isSynthetic()) {
+            copyMethodAnnotations(node, getter);
+        }
         visitMethod(getter);
     }
 
+    private void copyMethodAnnotations(PropertyNode node, MethodNode accessor) {
+        for (AnnotationNode annotationNode : node.getAnnotations()) {
+            if (annotationNode.isTargetAllowed(METHOD_TARGET)) {
+                accessor.addAnnotation(annotationNode);
+            }
+        }
+    }
+
     protected void addPropertyMethod(final MethodNode method) {
         classNode.addMethod(method);
         markAsGenerated(classNode, method);

[groovy] 05/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (documentation and property with annotation example)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 6a78b756d972088ef9f3963ddf71f99f99f59a9a
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 21:02:10 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (documentation and property with annotation example)
---
 src/spec/doc/core-object-orientation.adoc | 76 +++++++++++++++++++++++++++++--
 src/spec/test/ClassTest.groovy            | 15 ++++++
 2 files changed, 88 insertions(+), 3 deletions(-)

diff --git a/src/spec/doc/core-object-orientation.adoc b/src/spec/doc/core-object-orientation.adoc
index b23b90b..e822b5c 100644
--- a/src/spec/doc/core-object-orientation.adoc
+++ b/src/spec/doc/core-object-orientation.adoc
@@ -701,7 +701,7 @@ include::../test/ClassTest.groovy[tags=property_access,indent=0]
 <3> write access to the property is done outside of the `Person` class so it will implicitly call `setName`
 <4> read access to the property is done outside of the `Person` class so it will implicitly call `getName`
 <5> this will call the `name` method on `Person` which performs a direct access to the field
-<6> this will call the `wonder` method on `Person` which performs a direct read access to the field
+<6> this will call the `title` method on `Person` which performs a direct read access to the field
 
 It is worth noting that this behavior of accessing the backing field directly is done in order to prevent a stack
 overflow when using the property access syntax within a class that defines the property.
@@ -729,8 +729,9 @@ This syntactic sugar is at the core of many DSLs written in Groovy.
 
 ===== Property naming conventions
 
-It is generally recommended that the first two letters of a property name are lowercase and for multiword properties
-that camel case is used. In those cases, generated getters and setters will have a name formed by capitalizing the
+It is generally recommended that the first two letters of a property name are
+lowercase and for multi-word properties that camel case is used.
+In those cases, generated getters and setters will have a name formed by capitalizing the
 property name and adding a `get` or `set` prefix (or optionally "is" for a boolean getter).
 So, `getLength` would be a getter  for a `length` property and `setFirstName` a setter for a `firstName` property.
 `isEmpty` might be the getter method name for a property named `empty`.
@@ -762,6 +763,75 @@ e.g. you can have `aProp` and `AProp`, or `pNAME` and `PNAME`. The getters would
 and `getpNAME` and `getPNAME` respectively.
 ====
 
+===== Annotations on a property
+
+Annotations, including those associated with AST transforms,
+are copied on to the backing field for the property.
+This allows AST transforms which are applicable to fields to
+be applied to properties, e.g.:
+
+[source,groovy]
+----
+include::../test/ClassTest.groovy[tags=annotated_properties,indent=0]
+----
+<1> Confirms no eager initialization
+<2> Normal property access
+<3> Confirms initialization upon property access
+
+===== Split property definition with an explicit backing field
+
+Groovy's property syntax is a convenient shorthand when your class design
+follows certain conventions which align with common JavaBean practice.
+If your class doesn't exactly fit these conventions,
+you can certainly write the getter, setter and backing field long hand like you would in Java.
+However, Groovy does provide a split definition capability which still provides
+a shortened syntax while allowing slight adjustments to the conventions.
+For a split definition, you write a field and a property with the same name and type.
+Only one of the field or property may have an initial value.
+
+For split properties, annotations on the field remain on the backing field for the property.
+Annotations on the property part of the definition are copied onto the getter and setter methods.
+
+This mechanism allows a number of common variations that property users may wish
+to use if the standard property definition doesn't exactly fit their needs.
+For example, if the backing field should be `protected` rather than `private`:
+
+[source,groovy]
+----
+class HasPropertyWithProtectedField {
+    protected String name  // <1>
+    String name            // <2>
+}
+----
+<1> Protected backing field for name property instead of normal private one
+<2> Declare name property
+
+Or, the same example but with a package-private backing field:
+
+[source,groovy]
+----
+class HasPropertyWithPackagePrivateField {
+    String name                // <1>
+    @PackageScope String name  // <2>
+}
+----
+<1> Declare name property
+<2> Package-private backing field for name property instead of normal private one
+
+As a final example, we may wish to apply method-related AST transforms,
+or in general, any annotation to the setters/getters,
+e.g. to have the accessors be synchronized:
+
+[source,groovy]
+----
+class HasPropertyWithSynchronizedAccessorMethods {
+    private String name        // <1>
+    @Synchronized String name  // <2>
+}
+----
+<1> Backing field for name property
+<2> Declare name property with annotation for setter/getter
+
 === Annotation
 
 [[ann-definition]]
diff --git a/src/spec/test/ClassTest.groovy b/src/spec/test/ClassTest.groovy
index 80e9e32..427a760 100644
--- a/src/spec/test/ClassTest.groovy
+++ b/src/spec/test/ClassTest.groovy
@@ -378,6 +378,21 @@ class ClassTest extends GroovyTestCase {
             p.groovy = true                     // <3>
             // end::pseudo_properties[]
         '''
+
+        assertScript '''
+            // tag::annotated_properties[]
+            class Animal {
+                int lowerCount = 0
+                @Lazy String name = { lower().toUpperCase() }()
+                String lower() { lowerCount++; 'sloth' }
+            }
+
+            def a = new Animal()
+            assert a.lowerCount == 0  // <1>
+            assert a.name == 'SLOTH'  // <2>
+            assert a.lowerCount == 1  // <3>
+            // end::annotated_properties[]
+        '''
     }
 
 

[groovy] 01/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (package scope should know about split properties)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit 33d033a39cf91c39359c1afab92d7e4ea19fbec4
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 20:57:03 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (package scope should know about split properties)
---
 .../transform/PackageScopeASTTransformation.java   | 25 ++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/PackageScopeASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/PackageScopeASTTransformation.java
index 9a9bc9f..22ac1a3 100644
--- a/src/main/java/org/codehaus/groovy/transform/PackageScopeASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/PackageScopeASTTransformation.java
@@ -136,12 +136,13 @@ public class PackageScopeASTTransformation extends AbstractASTTransformation {
         }
     }
 
-    private static void visitFieldNode(FieldNode fNode) {
+    private void visitFieldNode(FieldNode fNode) {
         final ClassNode cNode = fNode.getDeclaringClass();
         final List<PropertyNode> pList = cNode.getProperties();
         PropertyNode foundProp = null;
+        String fName = fNode.getName();
         for (PropertyNode pNode : pList) {
-            if (pNode.getName().equals(fNode.getName())) {
+            if (pNode.getName().equals(fName) && pNode.getAnnotations(MY_TYPE) != null) {
                 foundProp = pNode;
                 break;
             }
@@ -149,6 +150,26 @@ public class PackageScopeASTTransformation extends AbstractASTTransformation {
         if (foundProp != null) {
             revertVisibility(fNode);
             pList.remove(foundProp);
+            // now check for split property
+            foundProp = null;
+            for (PropertyNode pNode : pList) {
+                if (pNode.getName().equals(fName)) {
+                    foundProp = pNode;
+                    break;
+                }
+            }
+            if (foundProp != null) {
+                FieldNode oldField = foundProp.getField();
+                cNode.getFields().remove(oldField);
+                cNode.getFieldIndex().put(fName, fNode);
+                if (foundProp.hasInitialExpression()) {
+                    if (fNode.hasInitialExpression()) {
+                        addError("The split property definition named '" + fName + "' must not have an initial value for both the field and the property", fNode);
+                    }
+                    fNode.setInitialValueExpression(foundProp.getInitialExpression());
+                }
+                foundProp.setField(fNode);
+            }
         }
     }
 

[groovy] 07/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (ignore @CompileStatic on properties - never used, just transferred to accessor methods for a split property)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit dede59548c359db4b1bfdf6ee1c22e74c01da1bd
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 21:04:54 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (ignore @CompileStatic on properties - never used, just transferred to accessor methods for a split property)
---
 .../java/org/codehaus/groovy/transform/StaticTypesTransformation.java  | 3 ++-
 .../org/codehaus/groovy/transform/sc/StaticCompileTransformation.java  | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/main/java/org/codehaus/groovy/transform/StaticTypesTransformation.java b/src/main/java/org/codehaus/groovy/transform/StaticTypesTransformation.java
index 74e958a..4a722d1 100644
--- a/src/main/java/org/codehaus/groovy/transform/StaticTypesTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/StaticTypesTransformation.java
@@ -24,6 +24,7 @@ import org.codehaus.groovy.ast.AnnotatedNode;
 import org.codehaus.groovy.ast.AnnotationNode;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.expr.ConstantExpression;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.ast.expr.ListExpression;
@@ -69,7 +70,7 @@ public class StaticTypesTransformation implements ASTTransformation, Compilation
             visitor.setMethodsToBeVisited(Collections.singleton(methodNode));
             visitor.initialize();
             visitor.visitMethod(methodNode);
-        } else {
+        } else if (!(node instanceof PropertyNode)) {
             source.addError(new SyntaxException(STATIC_ERROR_PREFIX + "Unimplemented node type",
                     node.getLineNumber(), node.getColumnNumber(), node.getLastLineNumber(), node.getLastColumnNumber()));
         }
diff --git a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompileTransformation.java b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompileTransformation.java
index b2857f3..3488739 100644
--- a/src/main/java/org/codehaus/groovy/transform/sc/StaticCompileTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/sc/StaticCompileTransformation.java
@@ -23,6 +23,7 @@ import org.codehaus.groovy.ast.AnnotatedNode;
 import org.codehaus.groovy.ast.AnnotationNode;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.MethodNode;
+import org.codehaus.groovy.ast.PropertyNode;
 import org.codehaus.groovy.ast.expr.Expression;
 import org.codehaus.groovy.classgen.asm.WriterControllerFactory;
 import org.codehaus.groovy.classgen.asm.sc.StaticTypesWriterControllerFactoryImpl;
@@ -78,7 +79,7 @@ public class StaticCompileTransformation extends StaticTypesTransformation {
             visitor.setMethodsToBeVisited(Collections.singleton(methodNode));
             visitor.initialize();
             visitor.visitMethod(methodNode);
-        } else {
+        } else if (!(target instanceof PropertyNode)) {
             source.addError(new SyntaxException(STATIC_ERROR_PREFIX + "Unimplemented node type", target));
         }
         if (visitor != null) {

[groovy] 04/10: GROOVY-9993: Field and a property with the same name: clarification of boundary cases (tweak existing test)

Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

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

commit cd8730b5fdda2267264ede35aed5aa7c482ff6d0
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Mar 30 21:01:28 2021 +1000

    GROOVY-9993: Field and a property with the same name: clarification of boundary cases (tweak existing test)
---
 src/spec/test/ClassTest.groovy | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/spec/test/ClassTest.groovy b/src/spec/test/ClassTest.groovy
index b6a4bd9..80e9e32 100644
--- a/src/spec/test/ClassTest.groovy
+++ b/src/spec/test/ClassTest.groovy
@@ -333,17 +333,17 @@ class ClassTest extends GroovyTestCase {
             class Person {
                 String name
                 void name(String name) {
-                    this.name = "Wonder$name"       // <1>
+                    this.name = "Wonder $name"      // <1>
                 }
-                String wonder() {
+                String title() {
                     this.name                       // <2>
                 }
             }
             def p = new Person()
-            p.name = 'Marge'                        // <3>
-            assert p.name == 'Marge'                // <4>
-            p.name('Marge')                         // <5>
-            assert p.wonder() == 'WonderMarge'      // <6>
+            p.name = 'Diana'                        // <3>
+            assert p.name == 'Diana'                // <4>
+            p.name('Woman')                         // <5>
+            assert p.title() == 'Wonder Woman'      // <6>
             // end::property_access[]
         '''