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:47 UTC

[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)

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:::