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 17:22:01 UTC
[groovy] branch GROOVY_3_0_X updated (8f393a3f64 -> 73aaa4521e)
This is an automated email from the ASF dual-hosted git repository.
emilles pushed a change to branch GROOVY_3_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
discard 8f393a3f64 GROOVY-10858: STC: method pointer/reference selection and reporting
new 73aaa4521e GROOVY-10858: STC: method pointer/reference selection and reporting
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (8f393a3f64)
\
N -- N -- N refs/heads/GROOVY_3_0_X (73aaa4521e)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../groovy/transform/stc/StaticTypeCheckingVisitor.java | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
[groovy] 01/01: GROOVY-10858: STC: method pointer/reference selection and reporting
Posted by em...@apache.org.
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
commit 73aaa4521eee8d0ce0a0476e819a7062ca91224a
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 | 95 +++++++++++++++-------
.../transform/stc/MethodReferenceTest.groovy | 32 ++++++--
3 files changed, 100 insertions(+), 38 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..81f5d68f9f 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;
}
@@ -2487,14 +2498,16 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
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()))) {
+ if (generated != null && candidates.stream().noneMatch(m -> m.getName().equals(generated.getName()))) {
candidates.add(generated);
}
candidates.addAll(GROOVY_OBJECT_TYPE.getMethods(nameText));
}
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 +2529,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 +2559,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 +2589,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