groovy git commit: GROOVY-8233: Java stub generation incorrect for static properties of traits

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

groovy git commit: GROOVY-8233: Java stub generation incorrect for static properties of traits

paulk
Repository: groovy
Updated Branches:
  refs/heads/master 8cd5a444f -> ed467c5f8


GROOVY-8233: Java stub generation incorrect for static properties of traits


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

Branch: refs/heads/master
Commit: ed467c5f8fd7cc230435a82c9964ea8dd9f6eb19
Parents: 8cd5a44
Author: Paul King <[hidden email]>
Authored: Thu Jul 5 23:26:23 2018 +1000
Committer: Paul King <[hidden email]>
Committed: Mon Jul 9 20:30:50 2018 +1000

----------------------------------------------------------------------
 .../groovy/tools/javac/JavaStubGenerator.java   | 10 +++++-
 .../transform/trait/TraitASTTransformation.java | 33 +++++++++++++++---
 .../groovy/transform/trait/TraitComposer.java   | 12 +++++++
 .../transform/trait/TraitHelpersTuple.java      | 14 +++++++-
 .../codehaus/groovy/transform/trait/Traits.java | 17 +++++++--
 .../GroovyXImpl.groovy                          | 21 ++++++++++++
 .../GroovyXTrait.groovy                         | 25 ++++++++++++++
 .../traitStaticPropertiesStub/JavaXImpl.java    | 25 ++++++++++++++
 .../TraitStaticPropertiesStubTest.groovy        | 36 ++++++++++++++++++++
 9 files changed, 184 insertions(+), 9 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/ed467c5f/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java b/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
index 9814ee4..581e723 100644
--- a/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
+++ b/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
@@ -162,7 +162,7 @@ public class JavaStubGenerator {
                         if (Traits.isTrait(inode)) {
                             List<PropertyNode> traitProps = inode.getProperties();
                             for (PropertyNode pn : traitProps) {
-                                visitProperty(pn);
+                                super.visitProperty(pn);
                             }
                         }
                     }
@@ -173,6 +173,14 @@ public class JavaStubGenerator {
                     return null;
                 }
 
+                @Override
+                public void visitProperty(PropertyNode node) {
+                    // GROOVY-8233 skip static properties for traits since they don't make the interface
+                    if (!node.isStatic() || !Traits.isTrait(node.getDeclaringClass())) {
+                        super.visitProperty(node);
+                    }
+                }
+
                 public void addCovariantMethods(ClassNode cn) {}
                 protected void addInitialization(ClassNode node) {}
                 protected void addPropertyMethod(MethodNode method) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/ed467c5f/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
----------------------------------------------------------------------
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 9d3ea0c..f52ee6c 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
@@ -184,13 +184,18 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
         // prepare fields
         List<FieldNode> fields = new ArrayList<FieldNode>();
         Set<String> fieldNames = new HashSet<String>();
+        boolean hasStatic = false;
         for (FieldNode field : cNode.getFields()) {
             if (!"metaClass".equals(field.getName()) && (!field.isSynthetic() || field.getName().indexOf('$') < 0)) {
                 fields.add(field);
                 fieldNames.add(field.getName());
+                if (field.isStatic()) {
+                    hasStatic = true;
+                }
             }
         }
         ClassNode fieldHelper = null;
+        ClassNode staticFieldHelper = null;
         if (!fields.isEmpty()) {
             fieldHelper = new InnerClassNode(
                     cNode,
@@ -198,6 +203,14 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
                     ACC_STATIC | ACC_PUBLIC | ACC_INTERFACE | ACC_ABSTRACT,
                     ClassHelper.OBJECT_TYPE
             );
+            if (hasStatic) {
+                staticFieldHelper = new InnerClassNode(
+                        cNode,
+                        Traits.staticFieldHelperClassName(cNode),
+                        ACC_STATIC | ACC_PUBLIC | ACC_INTERFACE | ACC_ABSTRACT,
+                        ClassHelper.OBJECT_TYPE
+                );
+            }
         }
 
         // add methods
@@ -225,7 +238,7 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
 
         // add fields
         for (FieldNode field : fields) {
-            processField(field, initializer, staticInitializer, fieldHelper, helper, cNode, fieldNames);
+            processField(field, initializer, staticInitializer, fieldHelper, helper, staticFieldHelper, cNode, fieldNames);
         }
 
         // clear properties to avoid generation of methods
@@ -245,12 +258,18 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
         unit.getAST().addClass(helper);
         if (fieldHelper != null) {
             unit.getAST().addClass(fieldHelper);
+            if (staticFieldHelper != null) {
+                unit.getAST().addClass(staticFieldHelper);
+            }
         }
 
         // resolve scope (for closures)
         resolveScope(helper);
-        if (fieldHelper!=null) {
+        if (fieldHelper != null) {
             resolveScope(fieldHelper);
+            if (staticFieldHelper != null) {
+                resolveScope(staticFieldHelper);
+            }
         }
         return helper;
     }
@@ -407,7 +426,7 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
     }
 
     private void processField(final FieldNode field, final MethodNode initializer, final MethodNode staticInitializer,
-                              final ClassNode fieldHelper, final ClassNode helper, final ClassNode trait,
+                              final ClassNode fieldHelper, final ClassNode helper, final ClassNode staticFieldHelper, 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() + ")",
@@ -417,6 +436,7 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
 
         Expression initialExpression = field.getInitialExpression();
         MethodNode selectedMethod = field.isStatic()?staticInitializer:initializer;
+        ClassNode target = fieldHelper;
         if (initialExpression != null) {
             VariableExpression thisObject = new VariableExpression(selectedMethod.getParameters()[0]);
             ExpressionStatement initCode = new ExpressionStatement(initialExpression);
@@ -436,6 +456,9 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
             BlockStatement code = (BlockStatement) selectedMethod.getCode();
             MethodCallExpression mce;
             if (field.isStatic()) {
+                if (staticFieldHelper != null) {
+                    target = staticFieldHelper;
+                }
                 mce = new MethodCallExpression(
                         new ClassExpression(INVOKERHELPER_CLASSNODE),
                         "invokeStaticMethod",
@@ -457,7 +480,7 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
             code.addStatement(new ExpressionStatement(mce));
         }
         // define setter/getter helper methods (setter added even for final fields for legacy compatibility)
-        fieldHelper.addMethod(
+        target.addMethod(
                 Traits.helperSetterName(field),
                 ACC_PUBLIC | ACC_ABSTRACT,
                 field.getOriginType(),
@@ -465,7 +488,7 @@ public class TraitASTTransformation extends AbstractASTTransformation implements
                 ClassNode.EMPTY_ARRAY,
                 null
         );
-        fieldHelper.addMethod(
+        target.addMethod(
                 Traits.helperGetterName(field),
                 ACC_PUBLIC | ACC_ABSTRACT,
                 field.getOriginType(),

http://git-wip-us.apache.org/repos/asf/groovy/blob/ed467c5f/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java b/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
index 34fe248..304a35b 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
@@ -141,6 +141,7 @@ public abstract class TraitComposer {
     private static void applyTrait(final ClassNode trait, final ClassNode cNode, final TraitHelpersTuple helpers) {
         ClassNode helperClassNode = helpers.getHelper();
         ClassNode fieldHelperClassNode = helpers.getFieldHelper();
+        ClassNode staticFieldHelperClassNode = helpers.getStaticFieldHelper();
         Map<String,ClassNode> genericsSpec = GenericsUtils.createGenericsSpec(cNode);
         genericsSpec = GenericsUtils.createGenericsSpec(trait, genericsSpec);
 
@@ -206,6 +207,17 @@ public abstract class TraitComposer {
                     declaredMethods.add(declaredMethod);
                 }
             }
+
+            if (staticFieldHelperClassNode != null) {
+                for (MethodNode declaredMethod : staticFieldHelperClassNode.getAllDeclaredMethods()) {
+                    if (declaredMethod.getName().endsWith(Traits.DIRECT_GETTER_SUFFIX)) {
+                        declaredMethods.add(0, declaredMethod);
+                    } else {
+                        declaredMethods.add(declaredMethod);
+                    }
+                }
+            }
+
             for (MethodNode methodNode : declaredMethods) {
                 String fieldName = methodNode.getName();
                 if (fieldName.endsWith(Traits.DIRECT_GETTER_SUFFIX) || fieldName.endsWith(Traits.DIRECT_SETTER_SUFFIX)) {

http://git-wip-us.apache.org/repos/asf/groovy/blob/ed467c5f/src/main/java/org/codehaus/groovy/transform/trait/TraitHelpersTuple.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/trait/TraitHelpersTuple.java b/src/main/java/org/codehaus/groovy/transform/trait/TraitHelpersTuple.java
index 9e807b9..5e3f626 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/TraitHelpersTuple.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitHelpersTuple.java
@@ -23,16 +23,21 @@ import org.codehaus.groovy.ast.ClassNode;
 /**
  * A class meant to hold reference to the helper and field helper of a trait.
  *
- * @author Cédric Champeau
  * @since 2.3.0
  */
 class TraitHelpersTuple {
     private final ClassNode helper;
     private final ClassNode fieldHelper;
+    private final ClassNode staticFieldHelper;
 
     public TraitHelpersTuple(final ClassNode helper, final ClassNode fieldHelper) {
+        this(helper, fieldHelper, null);
+    }
+
+    public TraitHelpersTuple(final ClassNode helper, final ClassNode fieldHelper, final ClassNode staticFieldHelper) {
         this.helper = helper;
         this.fieldHelper = fieldHelper;
+        this.staticFieldHelper = staticFieldHelper;
     }
 
     public ClassNode getHelper() {
@@ -42,4 +47,11 @@ class TraitHelpersTuple {
     public ClassNode getFieldHelper() {
         return fieldHelper;
     }
+
+    /**
+     * @since 2.5.1
+     */
+    public ClassNode getStaticFieldHelper() {
+        return staticFieldHelper;
+    }
 }

http://git-wip-us.apache.org/repos/asf/groovy/blob/ed467c5f/src/main/java/org/codehaus/groovy/transform/trait/Traits.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/codehaus/groovy/transform/trait/Traits.java b/src/main/java/org/codehaus/groovy/transform/trait/Traits.java
index 2143ff1..c83a71d 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/Traits.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/Traits.java
@@ -63,6 +63,7 @@ public abstract class Traits {
     static final String TRAIT_TYPE_NAME = "@" + TRAIT_CLASSNODE.getNameWithoutPackage();
     static final String TRAIT_HELPER = "$Trait$Helper";
     static final String FIELD_HELPER = "$Trait$FieldHelper";
+    static final String STATIC_FIELD_HELPER = "$Trait$StaticFieldHelper";
     static final String DIRECT_SETTER_SUFFIX = "$set";
     static final String DIRECT_GETTER_SUFFIX = "$get";
     static final String INIT_METHOD = "$init$";
@@ -86,6 +87,10 @@ public abstract class Traits {
         return traitNode.getName() + FIELD_HELPER;
     }
 
+    static String staticFieldHelperClassName(final ClassNode traitNode) {
+        return traitNode.getName() + STATIC_FIELD_HELPER;
+    }
+
     static String helperGetterName(final FieldNode field) {
         return remappedFieldName(unwrapOwner(field.getOwner()), field.getName()) + DIRECT_GETTER_SUFFIX;
     }
@@ -117,9 +122,14 @@ public abstract class Traits {
         return findHelpers(trait).getFieldHelper();
     }
 
+    public static ClassNode findStaticFieldHelper(final ClassNode trait) {
+        return findHelpers(trait).getStaticFieldHelper();
+    }
+
     static TraitHelpersTuple findHelpers(final ClassNode trait) {
         ClassNode helperClassNode = null;
         ClassNode fieldHelperClassNode = null;
+        ClassNode staticFieldHelperClassNode = null;
         Iterator<InnerClassNode> innerClasses = trait.redirect().getInnerClasses();
         if (innerClasses != null && innerClasses.hasNext()) {
             // trait defined in same source unit
@@ -127,6 +137,8 @@ public abstract class Traits {
                 ClassNode icn = innerClasses.next();
                 if (icn.getName().endsWith(Traits.FIELD_HELPER)) {
                     fieldHelperClassNode = icn;
+                } else if (icn.getName().endsWith(Traits.STATIC_FIELD_HELPER)) {
+                    staticFieldHelperClassNode = icn;
                 } else if (icn.getName().endsWith(Traits.TRAIT_HELPER)) {
                     helperClassNode = icn;
                 }
@@ -139,14 +151,15 @@ public abstract class Traits {
                 helperClassNode = ClassHelper.make(Class.forName(helperClassName, false, classLoader));
                 try {
                     fieldHelperClassNode = ClassHelper.make(classLoader.loadClass(Traits.fieldHelperClassName(trait)));
+                    staticFieldHelperClassNode = ClassHelper.make(classLoader.loadClass(Traits.staticFieldHelperClassName(trait)));
                 } catch (ClassNotFoundException e) {
-                    // not a problem, the field helper may be absent
+                    // not a problem, the field helpers may be absent
                 }
             } catch (ClassNotFoundException e) {
                 throw new GroovyBugError("Couldn't find trait helper classes on compile classpath!",e);
             }
         }
-        return new TraitHelpersTuple(helperClassNode,  fieldHelperClassNode);
+        return new TraitHelpersTuple(helperClassNode,  fieldHelperClassNode, staticFieldHelperClassNode);
     }
 
     /**

http://git-wip-us.apache.org/repos/asf/groovy/blob/ed467c5f/src/test-resources/stubgenerator/traitStaticPropertiesStub/GroovyXImpl.groovy
----------------------------------------------------------------------
diff --git a/src/test-resources/stubgenerator/traitStaticPropertiesStub/GroovyXImpl.groovy b/src/test-resources/stubgenerator/traitStaticPropertiesStub/GroovyXImpl.groovy
new file mode 100644
index 0000000..2299e47
--- /dev/null
+++ b/src/test-resources/stubgenerator/traitStaticPropertiesStub/GroovyXImpl.groovy
@@ -0,0 +1,21 @@
+/*
+ *  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 stubgenerator.traitStaticPropertiesStub
+
+class GroovyXImpl implements GroovyXTrait { }

http://git-wip-us.apache.org/repos/asf/groovy/blob/ed467c5f/src/test-resources/stubgenerator/traitStaticPropertiesStub/GroovyXTrait.groovy
----------------------------------------------------------------------
diff --git a/src/test-resources/stubgenerator/traitStaticPropertiesStub/GroovyXTrait.groovy b/src/test-resources/stubgenerator/traitStaticPropertiesStub/GroovyXTrait.groovy
new file mode 100644
index 0000000..5947c6a
--- /dev/null
+++ b/src/test-resources/stubgenerator/traitStaticPropertiesStub/GroovyXTrait.groovy
@@ -0,0 +1,25 @@
+/*
+ *  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 stubgenerator.traitStaticPropertiesStub
+
+trait GroovyXTrait {
+    static int bar
+    boolean foo
+    String baz() { 'baz' }
+}

http://git-wip-us.apache.org/repos/asf/groovy/blob/ed467c5f/src/test-resources/stubgenerator/traitStaticPropertiesStub/JavaXImpl.java
----------------------------------------------------------------------
diff --git a/src/test-resources/stubgenerator/traitStaticPropertiesStub/JavaXImpl.java b/src/test-resources/stubgenerator/traitStaticPropertiesStub/JavaXImpl.java
new file mode 100644
index 0000000..5862146
--- /dev/null
+++ b/src/test-resources/stubgenerator/traitStaticPropertiesStub/JavaXImpl.java
@@ -0,0 +1,25 @@
+/*
+ *  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 stubgenerator.traitStaticPropertiesStub;
+
+public class JavaXImpl extends GroovyXImpl {
+    public static void main(String[] args) {
+        new JavaXImpl();
+    }
+}

http://git-wip-us.apache.org/repos/asf/groovy/blob/ed467c5f/src/test/org/codehaus/groovy/tools/stubgenerator/TraitStaticPropertiesStubTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/tools/stubgenerator/TraitStaticPropertiesStubTest.groovy b/src/test/org/codehaus/groovy/tools/stubgenerator/TraitStaticPropertiesStubTest.groovy
new file mode 100644
index 0000000..82d5339
--- /dev/null
+++ b/src/test/org/codehaus/groovy/tools/stubgenerator/TraitStaticPropertiesStubTest.groovy
@@ -0,0 +1,36 @@
+/*
+ *  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 org.codehaus.groovy.tools.stubgenerator
+
+/**
+ * GROOVY-8233: Checks that static trait properties are handled correctly within stubs
+ */
+class TraitStaticPropertiesStubTest extends StubTestCase {
+    @Override
+    void verifyStubs() {
+        classes['stubgenerator.traitStaticPropertiesStub.GroovyXTrait'].with {
+            assert methods['getFoo'].signature == "boolean getFoo()"
+            assert !methods['getBar']
+        }
+        classes['stubgenerator.traitStaticPropertiesStub.GroovyXImpl'].with {
+            assert methods['getBar'].signature == "public static int getBar()"
+            assert methods['getFoo'].signature == "public boolean getFoo()"
+        }
+    }
+}