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 2018/08/05 14:02:11 UTC
[1/2] groovy git commit: GROOVY-8731: add some additional wording in
the documentation for this case
Repository: groovy
Updated Branches:
refs/heads/GROOVY_2_5_X c874c367f -> b96f4bc66
GROOVY-8731: add some additional wording in the documentation for this case
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/b96f4bc6
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/b96f4bc6
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/b96f4bc6
Branch: refs/heads/GROOVY_2_5_X
Commit: b96f4bc66dc8af951b829576efedff9e1e7cb6de
Parents: 2756cc1
Author: Paul King <pa...@asert.com.au>
Authored: Sun Aug 5 23:59:26 2018 +1000
Committer: Paul King <pa...@asert.com.au>
Committed: Mon Aug 6 00:02:02 2018 +1000
----------------------------------------------------------------------
src/spec/doc/core-traits.adoc | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/b96f4bc6/src/spec/doc/core-traits.adoc
----------------------------------------------------------------------
diff --git a/src/spec/doc/core-traits.adoc b/src/spec/doc/core-traits.adoc
index 61f66b4..2f5b9a1 100644
--- a/src/spec/doc/core-traits.adoc
+++ b/src/spec/doc/core-traits.adoc
@@ -320,7 +320,7 @@ include::{projectdir}/src/spec/test/TraitsSpecificationTest.groovy[tags=multiple
=== User conflict resolution
In case this behavior is not the one you want, you can explicitly choose which method to call using the `Trait.super.foo` syntax.
-In the example above, we can force to choose the method from trait A, by writing this:
+In the example above, we can ensure the method from trait A is invoked by writing this:
[source,groovy]
----
@@ -669,9 +669,16 @@ information below is valid for {groovyVersion} only.
It is possible to define static methods in a trait, but it comes with numerous limitations:
-* traits with static methods cannot be compiled statically or type checked. All static methods/properties/field are accessed dynamically (it's a limitation from the JVM).
-* the trait is interpreted as a _template_ for the implementing class, which means that each implementing class will get its own static methods/properties/methods. So a
-static member declared on a trait doesn't belong to the `Trait`, but to it's implementing class.
+* Traits with static methods cannot be compiled statically or type checked. All static methods,
+properties and field are accessed dynamically (it's a limitation from the JVM).
+* The trait is interpreted as a _template_ for the implementing class, which means that each
+implementing class will get its own static methods, properties and fields. So a static member
+declared on a trait doesn't belong to the `Trait`, but to it's implementing class.
+* You should typically not mix static and instance methods of the same signature. The normal
+rules for applying traits apply (including multiple inheritance conflict resolution). If the
+method chosen is static but some implemented trait has an instance variant, a compilation error
+will occur. If the method chosen is the instance variant, the static variant will be ignored
+(the behavior is similar to static methods in Java interfaces for this case).
Let's start with a simple example:
[2/2] groovy git commit: GROOVY-8731: improve error message for the
case of static and instance methods with same signature when using traits
Posted by pa...@apache.org.
GROOVY-8731: improve error message for the case of static and instance methods with same signature when using traits
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/2756cc12
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/2756cc12
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/2756cc12
Branch: refs/heads/GROOVY_2_5_X
Commit: 2756cc120213d215dbb9eb7290dd470680139613
Parents: c874c36
Author: Paul King <pa...@asert.com.au>
Authored: Sun Aug 5 23:38:05 2018 +1000
Committer: Paul King <pa...@asert.com.au>
Committed: Mon Aug 6 00:02:02 2018 +1000
----------------------------------------------------------------------
.../groovy/transform/trait/TraitComposer.java | 59 ++++++++++++++++++--
.../traitx/TraitASTTransformationTest.groovy | 25 +++++++++
2 files changed, 78 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/2756cc12/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 304a35b..f7a57f8 100644
--- a/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
+++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java
@@ -19,6 +19,7 @@
package org.codehaus.groovy.transform.trait;
import groovy.transform.CompileStatic;
+import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.AnnotationNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
@@ -108,7 +109,7 @@ public abstract class TraitComposer {
List<ClassNode> traits = findTraits(cNode);
for (ClassNode trait : traits) {
TraitHelpersTuple helpers = Traits.findHelpers(trait);
- applyTrait(trait, cNode, helpers);
+ applyTrait(trait, cNode, helpers, unit);
superCallTransformer.visitClass(cNode);
if (unit!=null) {
ASTTransformationCollectorCodeVisitor collector = new ASTTransformationCollectorCodeVisitor(unit, cu.getTransformLoader());
@@ -138,7 +139,7 @@ public abstract class TraitComposer {
}
}
- private static void applyTrait(final ClassNode trait, final ClassNode cNode, final TraitHelpersTuple helpers) {
+ private static void applyTrait(final ClassNode trait, final ClassNode cNode, final TraitHelpersTuple helpers, SourceUnit unit) {
ClassNode helperClassNode = helpers.getHelper();
ClassNode fieldHelperClassNode = helpers.getFieldHelper();
ClassNode staticFieldHelperClassNode = helpers.getStaticFieldHelper();
@@ -174,7 +175,7 @@ public abstract class TraitComposer {
origParams[i-1] = parameter;
argList.addExpression(new VariableExpression(params[i - 1]));
}
- createForwarderMethod(trait, cNode, methodNode, originalMethod, helperClassNode, methodGenericsSpec, helperMethodParams, origParams, params, argList);
+ createForwarderMethod(trait, cNode, methodNode, originalMethod, helperClassNode, methodGenericsSpec, helperMethodParams, origParams, params, argList, unit);
}
}
cNode.addObjectInitializerStatements(new ExpressionStatement(
@@ -330,11 +331,11 @@ public abstract class TraitComposer {
MethodNode helperMethod,
MethodNode originalMethod,
ClassNode helperClassNode,
- Map<String,ClassNode> genericsSpec,
+ Map<String, ClassNode> genericsSpec,
Parameter[] helperMethodParams,
Parameter[] traitMethodParams,
Parameter[] forwarderParams,
- ArgumentListExpression helperMethodArgList) {
+ ArgumentListExpression helperMethodArgList, SourceUnit unit) {
MethodCallExpression mce = new MethodCallExpression(
new ClassExpression(helperClassNode),
helperMethod.getName(),
@@ -396,6 +397,15 @@ public abstract class TraitComposer {
bridgeAnnotation
);
+ MethodNode existingMethod = findExistingMethod(targetNode, forwarder);
+ if (existingMethod != null) {
+ if (!forwarder.isStatic() && existingMethod.isStatic()) {
+ // found an existing static method that is going to conflict with interface
+ unit.addError(createException(trait, targetNode, forwarder, existingMethod));
+ return;
+ }
+ }
+
if (!shouldSkipMethod(targetNode, forwarder.getName(), forwarderParams)) {
targetNode.addMethod(forwarder);
}
@@ -403,6 +413,35 @@ public abstract class TraitComposer {
createSuperForwarder(targetNode, forwarder, genericsSpec);
}
+ private static SyntaxException createException(ClassNode trait, ClassNode targetNode, MethodNode forwarder, MethodNode existingMethod) {
+ String middle;
+ ASTNode errorTarget;
+ if (existingMethod.getLineNumber() == -1) {
+ // came from a trait
+ errorTarget = targetNode;
+ List<AnnotationNode> allAnnos = existingMethod.getAnnotations(Traits.TRAITBRIDGE_CLASSNODE);
+ AnnotationNode bridgeAnno = allAnnos == null ? null : allAnnos.get(0);
+ String fromTrait = null;
+ if (bridgeAnno != null) {
+ Expression traitClass = bridgeAnno.getMember("traitClass");
+ if (traitClass instanceof ClassExpression) {
+ ClassExpression ce = (ClassExpression) traitClass;
+ fromTrait = ce.getType().getNameWithoutPackage();
+ }
+ }
+ middle = "in '" + targetNode.getNameWithoutPackage();
+ if (fromTrait != null) {
+ middle += "' from trait '" + fromTrait;
+ }
+ } else {
+ errorTarget = existingMethod;
+ middle = "declared in '" + targetNode.getNameWithoutPackage();
+ }
+ String message = "The static '" + forwarder.getName() + "' method " + middle +
+ "' conflicts with the instance method having the same signature from trait '" + trait.getNameWithoutPackage() + "'";
+ return new SyntaxException(message, errorTarget);
+ }
+
private static GenericsType[] removeNonPlaceHolders(GenericsType[] oldTypes) {
if (oldTypes==null || oldTypes.length==0) return oldTypes;
ArrayList<GenericsType> l = new ArrayList<GenericsType>(Arrays.asList(oldTypes));
@@ -542,8 +581,16 @@ public abstract class TraitComposer {
return exceptionNodes;
}
+ private static MethodNode findExistingMethod(final ClassNode cNode, final MethodNode forwarder) {
+ return findExistingMethod(cNode, forwarder.getName(), forwarder.getParameters());
+ }
+
+ private static MethodNode findExistingMethod(final ClassNode cNode, final String name, final Parameter[] params) {
+ return cNode.getDeclaredMethod(name, params);
+ }
+
private static boolean shouldSkipMethod(final ClassNode cNode, final String name, final Parameter[] params) {
- if (isExistingProperty(name, cNode, params) || cNode.getDeclaredMethod(name, params)!=null) {
+ if (isExistingProperty(name, cNode, params) || findExistingMethod(cNode, name, params) != null) {
// override exists in the weaved class itself
return true;
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/2756cc12/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
index e449198..ffddb08 100644
--- a/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
@@ -2511,4 +2511,29 @@ assert c.b() == 2
'''
}
+ //GROOVY-8731
+ void testStaticMethodsIgnoredWhenExistingInstanceMethodsFound() {
+ assertScript '''
+ trait StaticFooBarBaz {
+ static int foo() { 100 }
+ static int baz() { 200 }
+ static int bar() { 300 }
+ }
+
+ trait InstanceBar {
+ int bar() { -10 }
+ }
+
+ class FooBarBaz implements StaticFooBarBaz, InstanceBar {
+ int baz() { -20 }
+ }
+
+ assert FooBarBaz.foo() == 100
+ new FooBarBaz().with {
+ assert bar() == -10
+ assert baz() == -20
+ }
+ '''
+ }
+
}