You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2022/12/02 17:06:27 UTC
[groovy] branch master updated: GROOVY-10858: STC: method pointer/reference selection and reporting
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 53ebf7ea41 GROOVY-10858: STC: method pointer/reference selection and reporting
53ebf7ea41 is described below
commit 53ebf7ea411efdca3790595020485d89020f8a00
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Fri Dec 2 09:55:21 2022 -0600
GROOVY-10858: STC: method pointer/reference selection and reporting
---
...StaticTypesMethodReferenceExpressionWriter.java | 22 +++++++++---
.../transform/stc/StaticTypeCheckingVisitor.java | 40 +++++++++++++++++++---
.../transform/stc/MethodReferenceTest.groovy | 18 +++++++---
3 files changed, 66 insertions(+), 14 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
index 19b8b030d0..3f1eb99bd5 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesMethodReferenceExpressionWriter.java
@@ -18,6 +18,7 @@
*/
package org.codehaus.groovy.classgen.asm.sc;
+import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.MethodNode;
@@ -141,7 +142,7 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
if (!isClassExpression) {
if (isConstructorReference) { // TODO: move this check to the parser
- controller.getSourceUnit().addFatalError("Constructor reference must be TypeName::new", methodReferenceExpression);
+ addFatalError("Constructor reference must be TypeName::new", methodReferenceExpression);
} else if (methodRefMethod.isStatic() && !targetIsArgument) {
// "string"::valueOf refers to static method, so instance is superfluous
typeOrTargetRef = makeClassTarget(typeOrTargetRefType, typeOrTargetRef);
@@ -188,11 +189,18 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
private void validate(final MethodReferenceExpression methodReference, final ClassNode targetType, final String methodName, final MethodNode methodNode, final Parameter[] samParameters, final ClassNode samReturnType) {
if (methodNode == null) {
- String error = String.format("Failed to find the expected method[%s(%s)] in the type[%s]",
- methodName, Arrays.stream(samParameters).map(e -> e.getType().getText()).collect(joining(",")), targetType.getText());
- controller.getSourceUnit().addFatalError(error, methodReference);
+ String error;
+ if (!(methodReference.getExpression() instanceof ClassExpression)) {
+ error = "Failed to find method '%s(%s)'";
+ } else {
+ error = "Failed to find class method '%s(%s)'";
+ if (samParameters.length > 0)
+ error += " or instance method '%1$s(" + Arrays.stream(samParameters).skip(1).map(e -> e.getType().toString(false)).collect(joining(",")) + ")'";
+ }
+ error = String.format(error + " for the type: %s", methodName, Arrays.stream(samParameters).map(e -> e.getType().toString(false)).collect(joining(",")), targetType.toString(false));
+ addFatalError(error, methodReference);
} else if (methodNode.isVoidMethod() && !ClassHelper.isPrimitiveVoid(samReturnType)) {
- controller.getSourceUnit().addFatalError("Invalid return type: void is not convertible to " + samReturnType.getText(), methodReference);
+ addFatalError("Invalid return type: void is not convertible to " + samReturnType.getText(), methodReference);
} else if (samParameters.length > 0 && isTypeReferringInstanceMethod(methodReference.getExpression(), methodNode) && !isAssignableTo(samParameters[0].getType(), targetType)) {
throw new RuntimeParserException("Invalid receiver type: " + samParameters[0].getType().getText() + " is not compatible with " + targetType.getText(), methodReference.getExpression());
}
@@ -382,6 +390,10 @@ public class StaticTypesMethodReferenceExpressionWriter extends MethodReferenceE
return filterMethodsByVisibility(methods, controller.getClassNode());
}
+ private void addFatalError(final String msg, final ASTNode node) {
+ controller.getSourceUnit().addFatalError(msg, node);
+ }
+
//--------------------------------------------------------------------------
private static boolean isConstructorReference(final String name) {
diff --git a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index 3ef7d60c4b..666841ebc2 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -1801,11 +1801,18 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
} else if (member instanceof PropertyNode) {
isStatic = ((PropertyNode) member).isStatic();
} else { // assume member instanceof MethodNode
- isStatic = member instanceof ExtensionMethodNode ? ((ExtensionMethodNode) member).isStaticExtension() : ((MethodNode) member).isStatic();
+ isStatic = isStaticInContext((MethodNode) member);
}
return (isStatic ? member : null);
}
+ /**
+ * Is the method called in a static or non-static manner?
+ */
+ private boolean isStaticInContext(final MethodNode method) {
+ return method instanceof ExtensionMethodNode ? ((ExtensionMethodNode) method).isStaticExtension() : method.isStatic();
+ }
+
private boolean storeField(final FieldNode field, final PropertyExpression expressionToStoreOn, final ClassNode receiver, final ClassCodeVisitorSupport visitor, final String delegationData, final boolean lhsOfAssignment) {
if (visitor != null) visitor.visitField(field);
checkOrMarkPrivateAccess(expressionToStoreOn, field, lhsOfAssignment);
@@ -2427,12 +2434,14 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
candidates = findMethodsWithGenerated(receiverType, nameText);
// GROOVY-10741: check for reference to a property node's method
MethodNode generated = findPropertyMethod(receiverType, nameText);
- if (generated != null && candidates.stream().noneMatch(mn -> mn.getName().equals(generated.getName()))){
+ if (generated != null && candidates.stream().noneMatch(m -> m.getName().equals(generated.getName()))) {
candidates.add(generated);
}
candidates.addAll(findDGMMethodsForClassNode(getSourceUnit().getClassLoader(), receiverType, nameText));
- candidates = filterMethodsByVisibility(candidates, typeCheckingContext.getEnclosingClassNode());
+ if (candidates.size() > 1) {
+ candidates = filterMethodCandidates(candidates, expression.getExpression(), expression.getNodeMetaData(CLOSURE_ARGUMENTS));
+ }
if (!candidates.isEmpty()) {
break;
}
@@ -2485,6 +2494,28 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
return null;
}
+ private List<MethodNode> filterMethodCandidates(final List<MethodNode> candidates, final Expression objectOrType, /*@Nullable*/ ClassNode[] signature) {
+ List<MethodNode> result = filterMethodsByVisibility(candidates, typeCheckingContext.getEnclosingClassNode());
+ // assignment or parameter target type may help reduce the list
+ if (result.size() > 1 && signature != null) {
+ ClassNode type = getType(objectOrType);
+ if (!isClassClassNodeWrappingConcreteType(type)) {
+ result = chooseBestMethod(type, result, signature);
+ } else {
+ type = type.getGenericsTypes()[0].getType(); // Class<Type> --> Type
+ Map<Boolean, List<MethodNode>> staticAndNonStatic = result.stream().collect(Collectors.partitioningBy(this::isStaticInContext));
+
+ result = new ArrayList<>(result.size());
+ result.addAll(chooseBestMethod(type, staticAndNonStatic.get(Boolean.TRUE), signature));
+ if (!staticAndNonStatic.get(Boolean.FALSE).isEmpty()) {
+ if (signature.length > 0) signature= Arrays.copyOfRange(signature, 1, signature.length);
+ result.addAll(chooseBestMethod(type, staticAndNonStatic.get(Boolean.FALSE), signature));
+ }
+ }
+ }
+ return result;
+ }
+
protected DelegationMetadata getDelegationMetadata(final ClosureExpression expression) {
return expression.getNodeMetaData(DELEGATION_METADATA);
}
@@ -3680,8 +3711,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (isClassClassNodeWrappingConcreteType(sourceExprType) && !"new".equals(methodReference.getMethodName().getText())) {
// GROOVY-10734: Type::instanceMethod has implied first param
List<MethodNode> candidates = methodReference.getNodeMetaData(MethodNode.class);
- if (candidates != null && !candidates.isEmpty() && candidates.stream().allMatch(mn ->
- !(mn instanceof ExtensionMethodNode ? ((ExtensionMethodNode) mn).isStaticExtension() : mn.isStatic()))) {
+ if (candidates != null && !candidates.isEmpty() && candidates.stream().allMatch(mn -> !isStaticInContext(mn))) {
firstParamType = sourceExprType.getGenericsTypes()[0].getType();
}
}
diff --git a/src/test/groovy/transform/stc/MethodReferenceTest.groovy b/src/test/groovy/transform/stc/MethodReferenceTest.groovy
index e155747d2a..3b0047e9df 100644
--- a/src/test/groovy/transform/stc/MethodReferenceTest.groovy
+++ b/src/test/groovy/transform/stc/MethodReferenceTest.groovy
@@ -229,7 +229,7 @@ final class MethodReferenceTest {
@CompileStatic
Map test(Collection<C> items) {
items.stream().collect(
- Collectors.groupingBy(C::getP) // Failed to find the expected method[getP(Object)] in the type[C]
+ Collectors.groupingBy(C::getP)
)
}
@@ -726,7 +726,6 @@ final class MethodReferenceTest {
'''
}
- @NotYetImplemented
@Test // class::staticMethod
void testFunctionCS4() {
assertScript shell, '''
@@ -894,7 +893,7 @@ final class MethodReferenceTest {
[1.0G, 2.0G, 3.0G].stream().reduce(0.0G, BigDecimal::addx)
}
'''
- assert err.message.contains('Failed to find the expected method[addx(java.math.BigDecimal,java.math.BigDecimal)] in the type[java.math.BigDecimal]')
+ assert err.message.contains("Failed to find class method 'addx(java.math.BigDecimal,java.math.BigDecimal)' or instance method 'addx(java.math.BigDecimal)' for the type: java.math.BigDecimal")
}
@Test // GROOVY-9463
@@ -905,7 +904,7 @@ final class MethodReferenceTest {
Function<String,String> reference = String::toLowerCaseX
}
'''
- assert err.message.contains('Failed to find the expected method[toLowerCaseX(java.lang.String)] in the type[java.lang.String]')
+ assert err.message.contains("Failed to find class method 'toLowerCaseX(java.lang.String)' or instance method 'toLowerCaseX()' for the type: java.lang.String")
}
@Test // GROOVY-10714
@@ -985,6 +984,17 @@ final class MethodReferenceTest {
'''
}
+ @Test // GROOVY-10813, GROOVY-10858
+ void testMethodSelection3() {
+ def err = shouldFail shell, '''
+ @CompileStatic
+ void test() {
+ Supplier<String> s = Object::toString // all options require an object
+ }
+ '''
+ assert err.message.contains("Failed to find class method 'toString()' for the type: java.lang.Object")
+ }
+
@Test // GROOVY-10742, GROOVY-10858
void testIncompatibleReturnType() {
def err = shouldFail shell, '''