You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by GitBox <gi...@apache.org> on 2020/11/09 23:04:23 UTC
[GitHub] [groovy] eric-milles opened a new pull request #1414: GROOVY-9803: translate generics of method reference using SAM parameters
eric-milles opened a new pull request #1414:
URL: https://github.com/apache/groovy/pull/1414
Since this change resolves additional generics, the test case for GROOVY-9762 fails when comparing `List<? super Integer>` to `List<Integer>`. Wherever `Collections::singletonList` is matched up with `Function<? super T, ? extends U>` is getting `List<? super Integer>`. Java just shows `List<Integer>` for `Optional.of(123).map(Collections::singletonList)`.
https://issues.apache.org/jira/browse/GROOVY-9803
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on a change in pull request #1414: GROOVY-9803: translate generics of method reference using SAM parameters
Posted by GitBox <gi...@apache.org>.
eric-milles commented on a change in pull request #1414:
URL: https://github.com/apache/groovy/pull/1414#discussion_r523426095
##########
File path: src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
##########
@@ -5272,6 +5275,46 @@ private static void extractGenericsConnectionsForSuperClassAndInterfaces(final M
}
}
+ private static MethodNode chooseMethod(final MethodPointerExpression source, final Supplier<ClassNode[]> samSignature) {
+ List<MethodNode> options = source.getNodeMetaData(MethodNode.class);
+ if (options == null || options.isEmpty()) {
+ return null;
+ }
+
+ ClassNode[] paramTypes = samSignature.get();
+ return options.stream().filter((MethodNode option) -> {
+ ClassNode[] types = collateMethodReferenceParameterTypes(source, option);
+ if (types.length == paramTypes.length) {
+ for (int i = 0, n = types.length; i < n; i += 1) {
+ if (!types[i].isGenericsPlaceHolder() && !isAssignableTo(types[i], paramTypes[i])) {
+ return false;
+ }
+ }
+ return true;
+ }
+ return false;
+ }).findFirst().orElse(null); // TODO: order matches by param distance
Review comment:
I have no further planned changes. It should be workable as it is, unless there is another test case that can demonstrate an issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on a change in pull request #1414: GROOVY-9803: translate generics of method reference using SAM parameters
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on a change in pull request #1414:
URL: https://github.com/apache/groovy/pull/1414#discussion_r523466787
##########
File path: src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
##########
@@ -5272,6 +5275,46 @@ private static void extractGenericsConnectionsForSuperClassAndInterfaces(final M
}
}
+ private static MethodNode chooseMethod(final MethodPointerExpression source, final Supplier<ClassNode[]> samSignature) {
+ List<MethodNode> options = source.getNodeMetaData(MethodNode.class);
+ if (options == null || options.isEmpty()) {
+ return null;
+ }
+
+ ClassNode[] paramTypes = samSignature.get();
+ return options.stream().filter((MethodNode option) -> {
+ ClassNode[] types = collateMethodReferenceParameterTypes(source, option);
+ if (types.length == paramTypes.length) {
+ for (int i = 0, n = types.length; i < n; i += 1) {
+ if (!types[i].isGenericsPlaceHolder() && !isAssignableTo(types[i], paramTypes[i])) {
+ return false;
+ }
+ }
+ return true;
+ }
+ return false;
+ }).findFirst().orElse(null); // TODO: order matches by param distance
Review comment:
Gotcha
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on pull request #1414: GROOVY-9803: translate generics of method reference using SAM parameters
Posted by GitBox <gi...@apache.org>.
eric-milles commented on pull request #1414:
URL: https://github.com/apache/groovy/pull/1414#issuecomment-726306330
I don't know if the change in `applyGenericsContextToParameterClass` is the right thing to do. I have no Java specification section to quote for how return type generics should be handled. Java gives `Set<Integer>` for `Optional.of(123).map(Collections::singleton).get()`. I read over [this](https://docs.oracle.com/javase/tutorial/extra/generics/morefun.html) but didn't figure any specific guidance for how `map` should work in this case.
**Note:** `applyGenericsContextToParameterClass` is only used by `StaticTypeCheckingVisitor`. I would have moved it there if not for binary backwards compatibility.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on pull request #1414: GROOVY-9803: translate generics of method reference using SAM parameters
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on pull request #1414:
URL: https://github.com/apache/groovy/pull/1414#issuecomment-727269491
Merged. Thanks!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 merged pull request #1414: GROOVY-9803: translate generics of method reference using SAM parameters
Posted by GitBox <gi...@apache.org>.
danielsun1106 merged pull request #1414:
URL: https://github.com/apache/groovy/pull/1414
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] danielsun1106 commented on a change in pull request #1414: GROOVY-9803: translate generics of method reference using SAM parameters
Posted by GitBox <gi...@apache.org>.
danielsun1106 commented on a change in pull request #1414:
URL: https://github.com/apache/groovy/pull/1414#discussion_r523416325
##########
File path: src/main/java/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
##########
@@ -5272,6 +5275,46 @@ private static void extractGenericsConnectionsForSuperClassAndInterfaces(final M
}
}
+ private static MethodNode chooseMethod(final MethodPointerExpression source, final Supplier<ClassNode[]> samSignature) {
+ List<MethodNode> options = source.getNodeMetaData(MethodNode.class);
+ if (options == null || options.isEmpty()) {
+ return null;
+ }
+
+ ClassNode[] paramTypes = samSignature.get();
+ return options.stream().filter((MethodNode option) -> {
+ ClassNode[] types = collateMethodReferenceParameterTypes(source, option);
+ if (types.length == paramTypes.length) {
+ for (int i = 0, n = types.length; i < n; i += 1) {
+ if (!types[i].isGenericsPlaceHolder() && !isAssignableTo(types[i], paramTypes[i])) {
+ return false;
+ }
+ }
+ return true;
+ }
+ return false;
+ }).findFirst().orElse(null); // TODO: order matches by param distance
Review comment:
Are you going to tweak the PR further?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on pull request #1414: GROOVY-9803: translate generics of method reference using SAM parameters
Posted by GitBox <gi...@apache.org>.
eric-milles commented on pull request #1414:
URL: https://github.com/apache/groovy/pull/1414#issuecomment-729730386
@chrylis You can try out your specific case using the CI build of groovy or the latest snapshot build of groovy-eclipse.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] chrylis commented on pull request #1414: GROOVY-9803: translate generics of method reference using SAM parameters
Posted by GitBox <gi...@apache.org>.
chrylis commented on pull request #1414:
URL: https://github.com/apache/groovy/pull/1414#issuecomment-729259561
I'm not sure from reading the code change, but does this handle arbitrarily nested generics? The actual real-world type that the compiler is having trouble with is (combining Vavr, Spring MVC, and some of my own record types)
```
<E, A, R> Either<JsonApiErrorResponse<E>, HeadersAndBody<JsonApiDataItemResponse<A, R>>>
```
where invoking `Either#map` expects a `Function<? super HeadersAndBody<JsonApiDataItemResponse<A, R>>` (this part works) but the closure (or Parrot lambda) seems not to correctly preserve the nested type parameters into the closure.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org