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/03 15:21:01 UTC
[groovy] branch GROOVY_3_0_X 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 GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_3_0_X by this push:
new 8f393a3f64 GROOVY-10858: STC: method pointer/reference selection and reporting
8f393a3f64 is described below
commit 8f393a3f64701678c9a90e65fb6b241e679d7355
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Sat Dec 3 08:28:30 2022 -0600
GROOVY-10858: STC: method pointer/reference selection and reporting
3_0_X backport
---
...StaticTypesMethodReferenceExpressionWriter.java | 11 ++-
.../transform/stc/StaticTypeCheckingVisitor.java | 104 ++++++++++++++-------
.../transform/stc/MethodReferenceTest.groovy | 32 +++++--
3 files changed, 103 insertions(+), 44 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 6dfb345ae1..7d119fe5f1 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
@@ -162,8 +162,15 @@ 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());
+ 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() && !samReturnType.equals(ClassHelper.VOID_TYPE)) {
addFatalError("Invalid return type: void is not convertible to " + samReturnType.getText(), methodReference);
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 4747f30576..caf9a3480d 100644
--- a/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -137,6 +137,7 @@ import java.util.StringJoiner;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Function;
import java.util.function.Supplier;
+import java.util.stream.Collectors;
import static org.apache.groovy.util.BeanUtils.capitalize;
import static org.apache.groovy.util.BeanUtils.decapitalize;
@@ -1798,34 +1799,43 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
}
/**
- * This method is used to filter search results in which null means "no match",
- * to filter out illegal access to instance members from a static context.
+ * Filters search result to prevent access to instance members from a static
+ * context.
* <p>
- * Return null if the given member is not static, but we want to access in
- * a static way (staticOnly=true). If we want to access in a non-static way
- * we always return the member, since then access to static members and
+ * Return null if the given member is not static, but we want to access in a
+ * static way (staticOnly=true). If we want to access in a non-static way we
+ * always return the member, since then access to static members and
* non-static members is allowed.
+ *
+ * @return {@code member} or null
*/
- @SuppressWarnings("unchecked")
private <T> T allowStaticAccessToMember(final T member, final boolean staticOnly) {
- if (member == null) return null;
- if (!staticOnly) return member;
+ if (member == null || !staticOnly) return member;
+
+ if (member instanceof List) {
+ @SuppressWarnings("unchecked")
+ T list = (T) ((List<MethodNode>) member).stream()
+ .map(m -> allowStaticAccessToMember(m, true))
+ .filter(Objects::nonNull).collect(Collectors.toList());
+ return list;
+ }
+
boolean isStatic;
- if (member instanceof Variable) {
- Variable v = (Variable) member;
- isStatic = Modifier.isStatic(v.getModifiers());
- } else if (member instanceof List) {
- List<MethodNode> list = (List<MethodNode>) member;
- if (list.size() == 1) {
- return (T) Collections.singletonList(allowStaticAccessToMember(list.get(0), staticOnly));
- }
- return (T) Collections.emptyList();
- } else {
- MethodNode mn = (MethodNode) member;
- isStatic = mn.isStatic();
+ if (member instanceof FieldNode) {
+ isStatic = ((FieldNode) member).isStatic();
+ } else if (member instanceof PropertyNode) {
+ isStatic = ((PropertyNode) member).isStatic();
+ } else { // assume member instanceof MethodNode
+ isStatic = isStaticInContext((MethodNode) member);
}
- if (staticOnly && !isStatic) return null;
- return 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 void storeWithResolve(ClassNode type, final ClassNode receiver, final ClassNode declaringClass, final boolean isStatic, final Expression expressionToStoreOn) {
@@ -2162,7 +2172,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ClassNode resultType;
if (isDoubleCategory(getUnwrapper(typeRe))) {
resultType = type;
- } else if (typeRe == ArrayList_TYPE) {
+ } else if (typeRe.equals(ArrayList_TYPE)) {
resultType = ArrayList_TYPE;
} else {
MethodNode mn = findMethodOrFail(expression, type, name);
@@ -2469,9 +2479,10 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
String nameText = nameExpr.getText();
if ("new".equals(nameText)) {
- ClassNode receiverType = getType(expression.getExpression());
- if (isClassClassNodeWrappingConcreteType(receiverType)) {
- storeType(expression, wrapClosureType(receiverType));
+ ClassNode type = getType(expression.getExpression());
+ if (isClassClassNodeWrappingConcreteType(type)){
+ type = type.getGenericsTypes()[0].getType();
+ storeType(expression,wrapClosureType(type));
}
return;
}
@@ -2484,17 +2495,16 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
ClassNode receiverType = wrapTypeIfNecessary(currentReceiver.getType());
candidates = findMethodsWithGenerated(receiverType, nameText);
- if (isBeingCompiled(receiverType) && !receiverType.isInterface()) {
- // 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()))) {
- candidates.add(generated);
- }
- candidates.addAll(GROOVY_OBJECT_TYPE.getMethods(nameText));
+ // GROOVY-10741: check for reference to a property node's method
+ MethodNode generated = findPropertyMethod(receiverType, nameText);
+ 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;
}
@@ -2516,7 +2526,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
} else if (!(expression instanceof MethodReferenceExpression)) {
ClassNode type = wrapTypeIfNecessary(getType(expression.getExpression()));
if (isClassClassNodeWrappingConcreteType(type)) type = type.getGenericsTypes()[0].getType();
- addStaticTypeError("Cannot find matching method " + type.getText() + "#" + nameText + ". Please check if the declared type is correct and if the method exists.", nameExpr);
+ addStaticTypeError("Cannot find matching method " + prettyPrintTypeName(type) + "#" + nameText + ". Please check if the declared type is correct and if the method exists.", nameExpr);
}
}
}
@@ -2546,6 +2556,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);
}
@@ -2554,7 +2586,7 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
if (typesBeforeVisit != null) {
for (Map.Entry<VariableExpression, Map<StaticTypesMarker, Object>> entry : typesBeforeVisit.entrySet()) {
for (StaticTypesMarker marker : StaticTypesMarker.values()) {
- // GROOVY-9344, GROOVY-9516
+ // GROOVY-9344, GROOVY-9516: keep type
if (marker == INFERRED_TYPE) continue;
Object value = entry.getValue().get(marker);
diff --git a/src/test/groovy/transform/stc/MethodReferenceTest.groovy b/src/test/groovy/transform/stc/MethodReferenceTest.groovy
index bbca175e90..84b2e9655c 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)
)
}
@@ -608,7 +608,7 @@ final class MethodReferenceTest {
assertScript imports + '''
@CompileStatic
void test() {
- Function<String, Integer> f = Integer::new
+ Function<String, Integer> f = Integer::new // deprcated; parseInt/valueOf
def result = ["1", "2", "3"].stream().map(f).collect(Collectors.toList())
assert result == [1, 2, 3]
}
@@ -841,7 +841,7 @@ final class MethodReferenceTest {
@CompileStatic
void test() {
def result = ['a', 'ab', 'abc'].stream().map(String::size).collect(Collectors.toList())
- assert [1, 2, 3] == result
+ assert result == [1, 2, 3]
}
test()
@@ -867,7 +867,6 @@ final class MethodReferenceTest {
@CompileStatic
void test() {
def result = [[a:1], [b:2], [c:3]].stream().map(Object::toString).collect(Collectors.toList())
- assert result.size() == 3
assert result == ['[a:1]', '[b:2]', '[c:3]']
}
@@ -899,7 +898,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
@@ -910,7 +909,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")
}
@NotYetImplemented
@@ -979,6 +978,27 @@ final class MethodReferenceTest {
test()
'''
+ assertScript imports + '''
+ @CompileStatic
+ void test() {
+ Supplier<String> s = 'x'::toString
+ def result = s.get()
+ assert result == 'x'
+ }
+
+ test()
+ '''
+ }
+
+ @Test // GROOVY-10813, GROOVY-10858
+ void testMethodSelection3() {
+ def err = shouldFail imports + '''
+ @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