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 50b77b2f60baf8e3456c2cc19c944339b28f1d27
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Jan 8 13:13:08 2019 +1000

    GROOVY-8951: Traits defining getter conflicts with generated getter 
(improvements for pre-compiled case) (closes #851)
---
 .../groovy/tools/javac/JavaStubGenerator.java      | 71 ++++++++++++---------
 .../groovy/transform/trait/TraitComposer.java      | 14 +----
 .../codehaus/groovy/transform/trait/Traits.java    | 19 ++++++
 .../test/groovy/groovy/ant/Groovy8951Test.groovy   | 73 ++++++++++++++++++++++
 4 files changed, 135 insertions(+), 42 deletions(-)

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 4b9efbb..64b3ee9 100644
--- a/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
+++ b/src/main/java/org/codehaus/groovy/tools/javac/JavaStubGenerator.java
@@ -32,6 +32,7 @@ import org.codehaus.groovy.ast.MethodNode;
 import org.codehaus.groovy.ast.ModuleNode;
 import org.codehaus.groovy.ast.Parameter;
 import org.codehaus.groovy.ast.PropertyNode;
+import org.codehaus.groovy.ast.decompiled.DecompiledClassNode;
 import org.codehaus.groovy.ast.expr.ArgumentListExpression;
 import org.codehaus.groovy.ast.expr.ClassExpression;
 import org.codehaus.groovy.ast.expr.ClosureExpression;
@@ -150,12 +151,9 @@ public class JavaStubGenerator {
                     List<Statement> savedStatements = new 
ArrayList<Statement>(node.getObjectInitializerStatements());
                     super.visitClass(node);
                     
node.getObjectInitializerStatements().addAll(savedStatements);
-                    for (ClassNode inode : node.getAllInterfaces()) {
-                        if (Traits.isTrait(inode)) {
-                            List<PropertyNode> traitProps = 
inode.getProperties();
-                            for (PropertyNode pn : traitProps) {
-                                super.visitProperty(pn);
-                            }
+                    for (ClassNode trait : Traits.findTraits(node)) {
+                        for (PropertyNode traitProperty : 
trait.getProperties()) {
+                            super.visitProperty(traitProperty);
                         }
                     }
                 }
@@ -322,38 +320,53 @@ public class JavaStubGenerator {
             printMethod(out, classNode, method);
         }
 
-        for (ClassNode node : classNode.getAllInterfaces()) {
-            if (Traits.isTrait(node)) {
-                List<MethodNode> traitMethods = node.getMethods();
-                for (MethodNode traitMethod : traitMethods) {
-                    MethodNode method = 
classNode.getMethod(traitMethod.getName(), traitMethod.getParameters());
-                    if (method == null) {
-                        for (MethodNode methodNode : propertyMethods) {
-                            if 
(methodNode.getName().equals(traitMethod.getName())) {
-                                boolean sameParams = 
sameParameterTypes(methodNode);
-                                if (sameParams) {
-                                    method = methodNode;
-                                    break;
-                                }
-                            }
-                        }
-                        if (method==null && !traitMethod.isAbstract()) {
-                            printMethod(out, classNode, traitMethod);
+        // print the methods from traits
+        for (ClassNode trait : Traits.findTraits(classNode)) {
+            List<MethodNode> traitMethods = trait.getMethods();
+            for (MethodNode traitMethod : traitMethods) {
+                MethodNode existingMethod = 
classNode.getMethod(traitMethod.getName(), traitMethod.getParameters());
+                if (existingMethod != null) continue;
+                for (MethodNode propertyMethod : propertyMethods) {
+                    if 
(propertyMethod.getName().equals(traitMethod.getName())) {
+                        boolean sameParams = 
sameParameterTypes(propertyMethod, traitMethod);
+                        if (sameParams) {
+                            existingMethod = propertyMethod;
+                            break;
                         }
                     }
                 }
+                if (existingMethod != null) continue;
+                boolean isCandidate = isCandidateTraitMethod(trait, 
traitMethod);
+                if (!isCandidate) continue;
+                printMethod(out, classNode, traitMethod);
             }
         }
+    }
+
+    private boolean isCandidateTraitMethod(ClassNode trait, MethodNode 
traitMethod) {
+        boolean precompiled = trait.redirect() instanceof DecompiledClassNode;
+        if (!precompiled) return !traitMethod.isAbstract();
+        List<MethodNode> helperMethods = Traits.findHelper(trait).getMethods();
+        for (MethodNode helperMethod : helperMethods) {
+            boolean isSynthetic = (traitMethod.getModifiers() & 
Opcodes.ACC_SYNTHETIC) != 0;
+            if (helperMethod.getName().equals(traitMethod.getName()) && 
!isSynthetic && !traitMethod.getName().contains("$")) {
+                Parameter[] origParams = helperMethod.getParameters();
+                Parameter[] newParams = Arrays.copyOfRange(origParams, 1, 
origParams.length);
+                if (sameParameterTypes(newParams, 
traitMethod.getParameters())) return true;
+            }
+        }
+        return false;
+    }
 
+    private static boolean sameParameterTypes(final MethodNode firstMethod, 
final MethodNode secondMethod) {
+        return sameParameterTypes(firstMethod.getParameters(), 
secondMethod.getParameters());
     }
 
-    private static boolean sameParameterTypes(final MethodNode methodNode) {
-        Parameter[] a = methodNode.getParameters();
-        Parameter[] b = methodNode.getParameters();
-        boolean sameParams = a.length == b.length;
+    private static boolean sameParameterTypes(final Parameter[] firstParams, 
final Parameter[] secondParams) {
+        boolean sameParams = firstParams.length == secondParams.length;
         if (sameParams) {
-            for (int i = 0; i < a.length; i++) {
-                if (!a[i].getType().equals(b[i].getType())) {
+            for (int i = 0; i < firstParams.length; i++) {
+                if 
(!firstParams[i].getType().equals(secondParams[i].getType())) {
                     sameParams = false;
                     break;
                 }
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 5b80061..e85fe4d 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
@@ -106,7 +106,7 @@ public abstract class TraitComposer {
             return;
         }
         if (!cNode.getNameWithoutPackage().endsWith(Traits.TRAIT_HELPER)) {
-            List<ClassNode> traits = findTraits(cNode);
+            List<ClassNode> traits = Traits.findTraits(cNode);
             for (ClassNode trait : traits) {
                 TraitHelpersTuple helpers = Traits.findHelpers(trait);
                 applyTrait(trait, cNode, helpers, unit);
@@ -119,18 +119,6 @@ public abstract class TraitComposer {
         }
     }
 
-    private static List<ClassNode> findTraits(ClassNode cNode) {
-        LinkedHashSet<ClassNode> interfaces = new LinkedHashSet<ClassNode>();
-        Traits.collectAllInterfacesReverseOrder(cNode, interfaces);
-        List<ClassNode> traits = new LinkedList<ClassNode>();
-        for (ClassNode candidate : interfaces) {
-            if (Traits.isAnnotatedWithTrait(candidate)) {
-                traits.add(candidate);
-            }
-        }
-        return traits;
-    }
-
     private static void checkTraitAllowed(final ClassNode bottomTrait, final 
SourceUnit unit) {
         ClassNode superClass = bottomTrait.getSuperClass();
         if (superClass==null || ClassHelper.OBJECT_TYPE.equals(superClass)) 
return;
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 c83a71d..2703de4 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/Traits.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/Traits.java
@@ -44,6 +44,7 @@ import java.lang.reflect.Method;
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.LinkedHashSet;
+import java.util.LinkedList;
 import java.util.List;
 
 /**
@@ -363,6 +364,24 @@ public abstract class Traits {
     }
 
     /**
+     * Find all traits associated with the given classnode
+     *
+     * @param cNode the given classnode
+     * @return the list of ordered trait classnodes
+     */
+    public static List<ClassNode> findTraits(ClassNode cNode) {
+        LinkedHashSet<ClassNode> interfaces = new LinkedHashSet<ClassNode>();
+        collectAllInterfacesReverseOrder(cNode, interfaces);
+        List<ClassNode> traits = new LinkedList<ClassNode>();
+        for (ClassNode candidate : interfaces) {
+            if (isAnnotatedWithTrait(candidate)) {
+                traits.add(candidate);
+            }
+        }
+        return traits;
+    }
+
+    /**
      * Internal annotation used to indicate which methods in a trait interface 
have a
      * default implementation.
      */
diff --git 
a/subprojects/groovy-ant/src/test/groovy/groovy/ant/Groovy8951Test.groovy 
b/subprojects/groovy-ant/src/test/groovy/groovy/ant/Groovy8951Test.groovy
new file mode 100644
index 0000000..e4a5229
--- /dev/null
+++ b/subprojects/groovy-ant/src/test/groovy/groovy/ant/Groovy8951Test.groovy
@@ -0,0 +1,73 @@
+/*
+ *  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.ant
+
+class Groovy8951Test extends AntTestCase {
+    void testSetterAppearsInTraitClass() {
+//        def debugLogger = new org.apache.tools.ant.DefaultLogger()
+//        debugLogger.setMessageOutputLevel(4)
+//        debugLogger.setOutputPrintStream(System.out)
+//        debugLogger.setErrorPrintStream(System.err)
+
+        doInTmpDir { ant, baseDir ->
+            baseDir.src {
+                p1 {
+                    'Validateable.groovy'('''
+                        package p1
+                        @groovy.transform.CompileStatic
+                        trait Validateable {
+                            List errors
+                            List getErrors() {
+                                errors
+                            }
+                            abstract void foo()
+                        }
+                    ''')
+                }
+                p2 {
+                    'Main.java'('''
+                        package p2;
+                        public class Main {
+                            MyValidateable temp = new MyValidateable();
+                        }
+                    ''')
+                    'OtherTrait.groovy'('''
+                        package p2
+                        trait OtherTrait {
+                            void otherMethod() { println new Date() }
+                        }
+                    ''')
+                    'MyValidateable.groovy'('''
+                        package p2
+                        class MyValidateable implements OtherTrait, 
p1.Validateable {
+                            void foo() {}
+                        }
+                    ''')
+                }
+            }
+//            ant.project.addBuildListener(debugLogger)
+            ant.mkdir(dir: 'build')
+            ant.taskdef(name: 'groovyc', classname: 
'org.codehaus.groovy.ant.Groovyc')
+            ant.groovyc(srcdir: 'src', destdir: 'build', includes: 'p1/*')
+            ant.groovyc(srcdir: 'src', destdir: 'build', includes: 'p2/*' /*, 
keepStubs: true*/) {
+                javac()
+            }
+        }
+    }
+}

Reply via email to