asfgit closed pull request #851: GROOVY-8951: Traits defining getter conflicts
with generated getter (…
URL: https://github.com/apache/groovy/pull/851
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
index 899d5cb47f..3287481ffc 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -151,10 +151,10 @@ protected boolean writeDirectMethodCall(MethodNode
target, boolean implicitThis,
int opcode = INVOKEVIRTUAL;
if (target.isStatic()) {
opcode = INVOKESTATIC;
- } else if (target.isPrivate() || ((receiver instanceof
VariableExpression && ((VariableExpression) receiver).isSuperExpression()))) {
- opcode = INVOKESPECIAL;
} else if (declaringClass.isInterface()) {
opcode = INVOKEINTERFACE;
+ } else if (target.isPrivate() || ((receiver instanceof
VariableExpression && ((VariableExpression) receiver).isSuperExpression()))) {
+ opcode = INVOKESPECIAL;
}
// handle receiver
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 4b9efbb1e7..64b3ee94ea 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.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 void visitClass(final ClassNode node) {
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 @@ private void printMethods(PrintWriter out, ClassNode
classNode, boolean isEnum)
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 5b800615fe..e85fe4df7d 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 static void doExtendTraits(final ClassNode cNode,
final SourceUnit unit,
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 static void doExtendTraits(final ClassNode cNode,
final SourceUnit unit,
}
}
- 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 c83a71d13b..2703de49fa 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.util.Arrays;
import java.util.Iterator;
import java.util.LinkedHashSet;
+import java.util.LinkedList;
import java.util.List;
/**
@@ -362,6 +363,24 @@ static String getSuperTraitMethodName(ClassNode trait,
String method) {
return
trait.getName().replace("_","__").replace('.','_')+SUPER_TRAIT_METHOD_PREFIX+method;
}
+ /**
+ * 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/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
index 27ecd8d0c4..d4c27b3203 100644
---
a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
+++
b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
@@ -2668,4 +2668,24 @@ assert c.b() == 2
assert Counter.count == 11
'''
}
+
+ //GROOVY-8954
+ void testTraitWithPropertyAlsoFromInterfaceSC() {
+ assertScript '''
+ interface DomainProp {
+ boolean isNullable()
+ }
+
+ abstract class OrderedProp implements DomainProp { }
+
+ trait Nullable {
+ boolean nullable = true
+ }
+
+ @groovy.transform.CompileStatic
+ abstract class CustomProp extends OrderedProp implements Nullable
{ }
+
+ assert new CustomProp() {}
+ '''
+ }
}
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 0000000000..e4a5229ccf
--- /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()
+ }
+ }
+ }
+}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
