Quantcast

groovy git commit: GROOVY-8048: final fields for pre-compiled traits aren't processed correctly (closes #492)

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

groovy git commit: GROOVY-8048: final fields for pre-compiled traits aren't processed correctly (closes #492)

paulk
Repository: groovy
Updated Branches:
  refs/heads/master 01fdb705c -> 75ceda4b2


GROOVY-8048: final fields for pre-compiled traits aren't processed correctly (closes #492)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/75ceda4b
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/75ceda4b
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/75ceda4b

Branch: refs/heads/master
Commit: 75ceda4b2b9a6fb6025c24972ad53007b17ec916
Parents: 01fdb70
Author: paulk <[hidden email]>
Authored: Wed Feb 8 08:41:11 2017 +1000
Committer: paulk <[hidden email]>
Committed: Thu Feb 9 12:16:39 2017 +1000

----------------------------------------------------------------------
 .../transform/trait/TraitASTTransformation.java | 66 ++++++++++++--------
 .../groovy/transform/trait/TraitComposer.java   | 15 +++++
 src/test/groovy/bugs/Groovy8048Bug.groovy       | 46 ++++++++++++++
 3 files changed, 102 insertions(+), 25 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/75ceda4b/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java b/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
index f91ab4e..f287284 100644
--- a/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
+++ b/src/main/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
@@ -68,6 +68,8 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Set;
 
+import static org.codehaus.groovy.ast.tools.GeneralUtils.returnS;
+
 /**
  * Handles generation of code for the @Trait annotation. A class annotated with @Trait will generate, instead: <ul>
  * <li>an <i>interface</i> with the same name</li> <li>an utility inner class that will be used by the compiler to
@@ -204,7 +206,7 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
 
         // add fields
         for (FieldNode field : fields) {
-            processField(field, initializer, staticInitializer, fieldHelper, cNode, fieldNames);
+            processField(field, initializer, staticInitializer, fieldHelper, helper, cNode, fieldNames);
         }
 
         // clear properties to avoid generation of methods
@@ -385,7 +387,8 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
     }
 
 
-    private void processField(final FieldNode field, final MethodNode initializer, final MethodNode staticInitializer, final ClassNode fieldHelper, final ClassNode trait, final Set<String> knownFields) {
+    private void processField(final FieldNode field, final MethodNode initializer, final MethodNode staticInitializer,
+                              final ClassNode fieldHelper, final ClassNode helper, final ClassNode trait, final Set<String> knownFields) {
         if (field.isProtected()) {
             unit.addError(new SyntaxException("Cannot have protected field in a trait (" + trait.getName() + "#" + field.getName() + ")",
                     field.getLineNumber(), field.getColumnNumber()));
@@ -394,32 +397,45 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
 
         Expression initialExpression = field.getInitialExpression();
         MethodNode selectedMethod = field.isStatic()?staticInitializer:initializer;
-        if (initialExpression != null && !field.isFinal()) {
-            VariableExpression thisObject = new VariableExpression(selectedMethod.getParameters()[0]);
-            ExpressionStatement initCode = new ExpressionStatement(initialExpression);
-            processBody(thisObject, selectedMethod, initCode, trait, fieldHelper, knownFields);
-            BlockStatement code = (BlockStatement) selectedMethod.getCode();
-            MethodCallExpression mce;
-            if (field.isStatic()) {
-                mce = new MethodCallExpression(
-                        new ClassExpression(INVOKERHELPER_CLASSNODE),
-                        "invokeStaticMethod",
-                        new ArgumentListExpression(
-                                thisObject,
-                                new ConstantExpression(Traits.helperSetterName(field)),
-                                initCode.getExpression()
-                        )
+        if (initialExpression != null) {
+            if (field.isFinal()) {
+                String baseName = field.isStatic() ? Traits.STATIC_INIT_METHOD : Traits.INIT_METHOD;
+                MethodNode fieldInitializer = new MethodNode(
+                        baseName + Traits.remappedFieldName(trait, field.getName()),
+                        ACC_STATIC | ACC_PUBLIC | ACC_SYNTHETIC,
+                        field.getOriginType(),
+                        Parameter.EMPTY_ARRAY,
+                        ClassNode.EMPTY_ARRAY,
+                        returnS(initialExpression)
                 );
+                helper.addMethod(fieldInitializer);
             } else {
-                mce = new MethodCallExpression(
-                        new CastExpression(createReceiverType(field.isStatic(), fieldHelper), thisObject),
-                        Traits.helperSetterName(field),
-                        new CastExpression(field.getOriginType(),initCode.getExpression())
-                );
+                VariableExpression thisObject = new VariableExpression(selectedMethod.getParameters()[0]);
+                ExpressionStatement initCode = new ExpressionStatement(initialExpression);
+                processBody(thisObject, selectedMethod, initCode, trait, fieldHelper, knownFields);
+                BlockStatement code = (BlockStatement) selectedMethod.getCode();
+                MethodCallExpression mce;
+                if (field.isStatic()) {
+                    mce = new MethodCallExpression(
+                            new ClassExpression(INVOKERHELPER_CLASSNODE),
+                            "invokeStaticMethod",
+                            new ArgumentListExpression(
+                                    thisObject,
+                                    new ConstantExpression(Traits.helperSetterName(field)),
+                                    initCode.getExpression()
+                            )
+                    );
+                } else {
+                    mce = new MethodCallExpression(
+                            new CastExpression(createReceiverType(field.isStatic(), fieldHelper), thisObject),
+                            Traits.helperSetterName(field),
+                            new CastExpression(field.getOriginType(),initCode.getExpression())
+                    );
+                }
+                mce.setImplicitThis(false);
+                mce.setSourcePosition(initialExpression);
+                code.addStatement(new ExpressionStatement(mce));
             }
-            mce.setImplicitThis(false);
-            mce.setSourcePosition(initialExpression);
-            code.addStatement(new ExpressionStatement(mce));
         }
         // define setter/getter helper methods
         if (!Modifier.isFinal(field.getModifiers())) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/75ceda4b/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java b/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
index fc4e0e5..704162d 100644
--- a/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
+++ b/src/main/org/codehaus/groovy/transform/trait/TraitComposer.java
@@ -24,6 +24,7 @@ import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.FieldNode;
 import org.codehaus.groovy.ast.GenericsType;
+import org.codehaus.groovy.ast.InnerClassNode;
 import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.PropertyNode;
@@ -70,6 +71,8 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
+import static org.codehaus.groovy.ast.tools.GeneralUtils.assignX;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.callX;
 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;
@@ -252,6 +255,18 @@ public abstract class TraitComposer {
                             GeneralUtils.copyAnnotatedNodeAnnotations(helperField, copied, notCopied);
                             FieldNode fieldNode = cNode.addField(fieldName, fieldMods, returnType, (fieldMods & Opcodes.ACC_FINAL) == 0 ? null : helperField.getInitialExpression());
                             fieldNode.addAnnotations(copied);
+                            // getInitialExpression above will be null if not in same source unit
+                            // so instead set within (static) initializer
+                            if (fieldNode.isFinal() && !(helperClassNode instanceof InnerClassNode)) {
+                                String baseName = fieldNode.isStatic() ? Traits.STATIC_INIT_METHOD : Traits.INIT_METHOD;
+                                Expression mce = callX(helperClassNode, baseName + fieldNode.getName());
+                                Statement stmt = stmt(assignX(varX(fieldNode.getName(), fieldNode.getType()), mce));
+                                if (isStatic == 0) {
+                                    cNode.addObjectInitializerStatements(stmt);
+                                } else {
+                                    cNode.addStaticInitializerStatements(Collections.<Statement>singletonList(stmt), false);
+                                }
+                            }
                         }
                     }
                     Parameter[] newParams;

http://git-wip-us.apache.org/repos/asf/groovy/blob/75ceda4b/src/test/groovy/bugs/Groovy8048Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy8048Bug.groovy b/src/test/groovy/bugs/Groovy8048Bug.groovy
new file mode 100644
index 0000000..e541f2f
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy8048Bug.groovy
@@ -0,0 +1,46 @@
+/*
+ *  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 groovy.bugs
+
+class Groovy8048Bug extends GroovyTestCase {
+    void testFinalFieldInPreCompiledTrait() {
+        def shell = new GroovyShell(getClass().classLoader)
+        shell.evaluate('''
+            class Bar implements groovy.bugs.Groovy8048Bug.Groovy8048Trait, BarTrait {
+                static void main(args) {
+                    assert Bar.otherItems1 && Bar.otherItems1[0] == 'bar1'
+                    assert Bar.items1 && Bar.items1[0] == 'foo1'
+                    new Bar().with {
+                        assert otherItems2 && otherItems2[0] == 'bar2'
+                        assert items2 && items2[0] == 'foo2'
+                    }
+                }
+                trait BarTrait {
+                    final static List otherItems1 = ['bar1']
+                    final List otherItems2 = ['bar2']
+                }
+            }
+        ''', 'testScript')
+    }
+
+    trait Groovy8048Trait {
+        final static List items1 = ['foo1']
+        final List items2 = ['foo2']
+    }
+}

Loading...