Skip to content

Commit

Permalink
Fix for #953: redirect trait helper methods to original trait methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@eric-milles
eric-milles committed Sep 22, 2019
1 parent c6a508d commit dace0f4
Show file tree
Hide file tree
Showing 9 changed files with 33 additions and 23 deletions.
8 changes: 4 additions & 4 deletions ...ore.tests.builder/src/org/eclipse/jdt/core/groovy/tests/search/TraitInferencingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void testPublicMethod3() {
" }\n" +
"}";

assertDeclType(contents, "method", "A$Trait$Helper");
assertDeclType(contents, "method", "A");
assertExprType(contents, "method", "java.lang.Void");
}

Expand All @@ -102,7 +102,7 @@ public void testPublicMethod4() {
" }\n" +
"}";

assertDeclType(contents, "method", "B$Trait$Helper");
assertDeclType(contents, "method", "B");
assertExprType(contents, "method", "java.lang.Void");
}

Expand Down Expand Up @@ -258,7 +258,7 @@ public void testPrivateMethod3() {
" }\n" +
"}";

assertDeclType(contents, "method", "A$Trait$Helper");
assertDeclType(contents, "method", "A");
assertExprType(contents, "method", "java.lang.Void");
}

Expand All @@ -277,7 +277,7 @@ public void testPrivateMethod4() {
" }\n" +
"}";

assertDeclType(contents, "method", "B$Trait$Helper");
assertDeclType(contents, "method", "B");
assertExprType(contents, "method", "java.lang.Void");
}

Expand Down
10 changes: 8 additions & 2 deletions ...org.codehaus.groovy24/src/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,12 +204,18 @@ private ClassNode createHelperClass(final ClassNode cNode) {
for (final MethodNode methodNode : methods) {
boolean declared = methodNode.getDeclaringClass() == cNode;
if (declared) {
if (!methodNode.isSynthetic() && (methodNode.isProtected() || methodNode.getModifiers()==0)) {
unit.addError(new SyntaxException("Cannot have protected/package private method in a trait (" + cNode.getName() + "#" + methodNode.getTypeDescriptor() + ")",
if (!methodNode.isSynthetic() && (methodNode.isProtected() || (!methodNode.isPrivate() && !methodNode.isPublic()))) {
unit.addError(new SyntaxException("Cannot have protected/package-private method in a trait (" + cNode.getName() + "#" + methodNode.getTypeDescriptor() + ")",
methodNode.getLineNumber(), methodNode.getColumnNumber()));
return null;
}
/* GRECLIPSE edit
helper.addMethod(processMethod(cNode, helper, methodNode, fieldHelper, fieldNames));
*/
MethodNode newMethod = processMethod(cNode, helper, methodNode, fieldHelper, fieldNames);
newMethod.setOriginal(methodNode);
helper.addMethod(newMethod);
// GRECLIPSE end
if (methodNode.isPrivate() || methodNode.isStatic()) {
nonPublicAPIMethods.add(methodNode);
}
Expand Down
3 changes: 0 additions & 3 deletions base/org.codehaus.groovy24/src/org/codehaus/groovy/transform/trait/TraitComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,9 +381,6 @@ private static void createForwarderMethod(
bridgeAnnotation.addMember("traitClass", new ClassExpression(trait));
bridgeAnnotation.addMember("desc", new ConstantExpression(BytecodeHelper.getMethodDescriptor(helperMethod.getReturnType(), traitMethodParams)));
forwarder.addAnnotation(bridgeAnnotation);
// GRECLIPSE add
forwarder.setOriginal(originalMethod);
// GRECLIPSE end

if (!shouldSkipMethod(targetNode, forwarder.getName(), forwarderParams)) {
targetNode.addMethod(forwarder);
Expand Down
7 changes: 5 additions & 2 deletions ...org.codehaus.groovy25/src/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ private ClassNode createHelperClass(final ClassNode cNode) {
for (final MethodNode methodNode : methods) {
boolean declared = methodNode.getDeclaringClass() == cNode;
if (declared) {
if (!methodNode.isSynthetic() && (methodNode.isProtected() || methodNode.getModifiers()==0)) {
unit.addError(new SyntaxException("Cannot have protected/package private method in a trait (" + cNode.getName() + "#" + methodNode.getTypeDescriptor() + ")",
if (!methodNode.isSynthetic() && (methodNode.isProtected() || (!methodNode.isPrivate() && !methodNode.isPublic()))) {
unit.addError(new SyntaxException("Cannot have protected/package-private method in a trait (" + cNode.getName() + "#" + methodNode.getTypeDescriptor() + ")",
methodNode.getLineNumber(), methodNode.getColumnNumber()));
return null;
}
Expand All @@ -231,6 +231,9 @@ private ClassNode createHelperClass(final ClassNode cNode) {
} else {
// add non-abstract methods; abstract methods covered from trait interface
helper.addMethod(newMethod);
// GRECLIPSE add
newMethod.setOriginal(methodNode);
// GRECLIPSE end
}
}
if (methodNode.isPrivate() || methodNode.isStatic()) {
Expand Down
3 changes: 0 additions & 3 deletions base/org.codehaus.groovy25/src/org/codehaus/groovy/transform/trait/TraitComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,9 +384,6 @@ private static void createForwarderMethod(
bridgeAnnotation.addMember("traitClass", new ClassExpression(trait));
bridgeAnnotation.addMember("desc", new ConstantExpression(BytecodeHelper.getMethodDescriptor(helperMethod.getReturnType(), traitMethodParams)));
forwarder.addAnnotation(bridgeAnnotation);
// GRECLIPSE add
forwarder.setOriginal(originalMethod);
// GRECLIPSE end

MethodNode existingMethod = findExistingMethod(targetNode, forwarder);
if (existingMethod != null) {
Expand Down
7 changes: 5 additions & 2 deletions ...org.codehaus.groovy30/src/org/codehaus/groovy/transform/trait/TraitASTTransformation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -219,8 +219,8 @@ private ClassNode createHelperClass(final ClassNode cNode) {
for (final MethodNode methodNode : methods) {
boolean declared = methodNode.getDeclaringClass() == cNode;
if (declared) {
if (!methodNode.isSynthetic() && (methodNode.isProtected() || methodNode.getModifiers()==0)) {
unit.addError(new SyntaxException("Cannot have protected/package private method in a trait (" + cNode.getName() + "#" + methodNode.getTypeDescriptor() + ")",
if (!methodNode.isSynthetic() && (methodNode.isProtected() || (!methodNode.isPrivate() && !methodNode.isPublic()))) {
unit.addError(new SyntaxException("Cannot have protected/package-private method in a trait (" + cNode.getName() + "#" + methodNode.getTypeDescriptor() + ")",
methodNode.getLineNumber(), methodNode.getColumnNumber()));
return null;
}
Expand All @@ -231,6 +231,9 @@ private ClassNode createHelperClass(final ClassNode cNode) {
} else {
// add non-abstract methods; abstract methods covered from trait interface
helper.addMethod(newMethod);
// GRECLIPSE add
newMethod.setOriginal(methodNode);
// GRECLIPSE end
}
}
if (methodNode.isPrivate() || methodNode.isStatic()) {
Expand Down
3 changes: 0 additions & 3 deletions base/org.codehaus.groovy30/src/org/codehaus/groovy/transform/trait/TraitComposer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -384,9 +384,6 @@ private static void createForwarderMethod(
bridgeAnnotation.addMember("traitClass", new ClassExpression(trait));
bridgeAnnotation.addMember("desc", new ConstantExpression(BytecodeHelper.getMethodDescriptor(helperMethod.getReturnType(), traitMethodParams)));
forwarder.addAnnotation(bridgeAnnotation);
// GRECLIPSE add
forwarder.setOriginal(originalMethod);
// GRECLIPSE end

MethodNode existingMethod = findExistingMethod(targetNode, forwarder);
if (existingMethod != null) {
Expand Down
13 changes: 10 additions & 3 deletions base/org.eclipse.jdt.groovy.core/src/org/eclipse/jdt/groovy/search/SimpleTypeLookup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,7 @@ public TypeLookupResult lookupType(final ClassNode node, final VariableScope sco
if (!node.isRedirectNode() && GroovyUtils.isAnonymous(node)) {
// return extended/implemented type for anonymous inner class
type = node.getUnresolvedSuperClass(false); if (type == VariableScope.OBJECT_CLASS_NODE) type = node.getInterfaces()[0];

} else if (Flags.isSynthetic(node.getModifiers()) && node.getName().endsWith("Helper") && node.getName().contains("$Trait$")) {
} else if (isTraitHelper(node)) {
// return trait type for trait helper
type = node.getOuterClass();
}
Expand Down Expand Up @@ -407,13 +406,17 @@ protected TypeLookupResult findTypeForNameWithKnownObjectExpression(final String
MethodNode method = (MethodNode) declaration;
if (isStaticObjectExpression && !method.isStatic() && !isStaticReferenceToInstanceMethod(scope)) {
confidence = TypeConfidence.UNKNOWN;
} else if (Flags.isPrivate(method.getModifiers()) && isThisObjectExpression(scope) && isNotThisOrOuterClass(declaringType, resolvedDeclaringType)) {
} else if (method.isPrivate() && isThisObjectExpression(scope) && isNotThisOrOuterClass(declaringType, resolvedDeclaringType)) {
// "this.method()" reference to private method of super class yields MissingMethodException; "super.method()" is okay
confidence = TypeConfidence.UNKNOWN;
} else if (isLooseMatch(scope.getMethodCallArgumentTypes(), method.getParameters())) {
// if arguments and parameters are mismatched, a category method may make a better match
confidence = TypeConfidence.LOOSELY_INFERRED;
}
if (isTraitHelper(resolvedDeclaringType) && method.getOriginal() != method) {
resolvedDeclaringType = method.getOriginal().getDeclaringClass();
declaration = method.getOriginal(); // the trait method
}
}
} else if (VariableScope.CLASS_CLASS_NODE.equals(resolvedDeclaringType) && declaration instanceof MethodNode) {
// beware of matching Class methods too aggressively; Arrays.toString(Object[]) vs. Class.toString()
Expand Down Expand Up @@ -975,6 +978,10 @@ protected static boolean isTraitBridge(final MethodNode method) {
return method.getAnnotations().stream().map(AnnotationNode::getClassNode).anyMatch(Traits.TRAITBRIDGE_CLASSNODE::equals);
}

protected static boolean isTraitHelper(final ClassNode candidate) {
return Flags.isSynthetic(candidate.getModifiers()) && candidate.getName().endsWith("Helper") && candidate.getName().contains("$Trait$") && Traits.isTrait(candidate.getOuterClass());
}

/**
* Supplements {@link #isTypeCompatible} by supporting unequal lengths and
* tagging Closure -> SAM type as an inexact match.
Expand Down
2 changes: 1 addition & 1 deletion ...vy.eclipse.tests/src/org/codehaus/groovy/eclipse/test/ui/SemanticHighlightingTests.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3282,7 +3282,7 @@ final class SemanticHighlightingTests extends GroovyEclipseTestSuite {
new HighlightedTypedPosition(contents.lastIndexOf('T {'), 1, TRAIT),
new HighlightedTypedPosition(contents.indexOf('meth'), 4, METHOD),
new HighlightedTypedPosition(contents.lastIndexOf('T'), 1, TRAIT),
new HighlightedTypedPosition(contents.lastIndexOf('whatever'), 8, STATIC_CALL))
new HighlightedTypedPosition(contents.lastIndexOf('whatever'), 8, METHOD_CALL))
}

@Test
Expand Down

0 comments on commit dace0f4

Please sign in to comment.