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 2022/11/10 21:21:44 UTC
[GitHub] [groovy] Shadow-Devil opened a new pull request, #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Shadow-Devil opened a new pull request, #1824:
URL: https://github.com/apache/groovy/pull/1824
I've added a test case and a fix for the bug reported in GROOVY-10813.
I'm not sure if there are any other edge cases that will now break because of this change, but one [test](https://github.com/apache/groovy/blob/9210cfab6aac681a865c57dd97e96aac73f38fd4/src/test/groovy/transform/stc/GenericsSTCTest.groovy#L740), that failed with another solution that I tried, passes with this solution.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] Shadow-Devil commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
Shadow-Devil commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1020470376
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -855,10 +855,16 @@ private static Map<GenericsType, GenericsType> doMakeDeclaringAndActualGenericsT
GenericsType[] targetGenericsTypes = parameterizedType.redirect().getGenericsTypes();
if (targetGenericsTypes != null) {
GenericsType[] sourceGenericsTypes = parameterizedType.getGenericsTypes();
- if (sourceGenericsTypes == null) sourceGenericsTypes = EMPTY_GENERICS_ARRAY;
map = new LinkedHashMap<>();
- for (int i = 0, m = sourceGenericsTypes.length, n = targetGenericsTypes.length; i < n; i += 1) {
- map.put(targetGenericsTypes[i], i < m ? sourceGenericsTypes[i] : targetGenericsTypes[i]);
+ if (sourceGenericsTypes == null) {
+ //We have no sourceGenerics, so we need to return the erasures for the raw types (Groovy-10813)
+ for (GenericsType targetGenericsType : targetGenericsTypes) {
+ map.put(targetGenericsType, ClassHelper.OBJECT_TYPE.asGenericsType());
Review Comment:
Uh, now I got an example, that has not Object as its type:
```java
interface A<T extends String> {
T apply(T input);
}
class B implements A{
@Override
String apply(String input) {
return null
}
}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] Shadow-Devil commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
Shadow-Devil commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1020046859
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -950,7 +956,7 @@ public static boolean hasUnresolvedGenerics(final ClassNode type) {
public static Tuple2<ClassNode[], ClassNode> parameterizeSAM(final ClassNode samType) {
MethodNode abstractMethod = ClassHelper.findSAM(samType);
- Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMapOfExactType(abstractMethod.getDeclaringClass(), samType);
+ Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMap(abstractMethod.getDeclaringClass(), samType);
Review Comment:
I'm sorry, this change was not intended to be commited, it was just for testing.
I will revert it.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
eric-milles commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1019717220
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -950,7 +956,7 @@ public static boolean hasUnresolvedGenerics(final ClassNode type) {
public static Tuple2<ClassNode[], ClassNode> parameterizeSAM(final ClassNode samType) {
MethodNode abstractMethod = ClassHelper.findSAM(samType);
- Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMapOfExactType(abstractMethod.getDeclaringClass(), samType);
+ Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMap(abstractMethod.getDeclaringClass(), samType);
Review Comment:
We have been moving away from `makeDeclaringAndActualGenericsTypeMap` towards `makeDeclaringAndActualGenericsTypeMapOfExactType`. Can you explain this change?
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -950,7 +956,7 @@ public static boolean hasUnresolvedGenerics(final ClassNode type) {
public static Tuple2<ClassNode[], ClassNode> parameterizeSAM(final ClassNode samType) {
MethodNode abstractMethod = ClassHelper.findSAM(samType);
- Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMapOfExactType(abstractMethod.getDeclaringClass(), samType);
+ Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMap(abstractMethod.getDeclaringClass(), samType);
Review Comment:
Is this the stuff that sits under parametwrizeSAM? That was the key method for fixing lambda param types.
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -855,10 +855,16 @@ private static Map<GenericsType, GenericsType> doMakeDeclaringAndActualGenericsT
GenericsType[] targetGenericsTypes = parameterizedType.redirect().getGenericsTypes();
if (targetGenericsTypes != null) {
GenericsType[] sourceGenericsTypes = parameterizedType.getGenericsTypes();
- if (sourceGenericsTypes == null) sourceGenericsTypes = EMPTY_GENERICS_ARRAY;
map = new LinkedHashMap<>();
- for (int i = 0, m = sourceGenericsTypes.length, n = targetGenericsTypes.length; i < n; i += 1) {
- map.put(targetGenericsTypes[i], i < m ? sourceGenericsTypes[i] : targetGenericsTypes[i]);
+ if (sourceGenericsTypes == null) {
+ //We have no sourceGenerics, so we need to return the erasures for the raw types (Groovy-10813)
+ for (GenericsType targetGenericsType : targetGenericsTypes) {
+ map.put(targetGenericsType, ClassHelper.OBJECT_TYPE.asGenericsType());
Review Comment:
The erasure type is not always Object. Can you create an example where it is another type?
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] Shadow-Devil commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
Shadow-Devil commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1020468865
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -855,10 +855,16 @@ private static Map<GenericsType, GenericsType> doMakeDeclaringAndActualGenericsT
GenericsType[] targetGenericsTypes = parameterizedType.redirect().getGenericsTypes();
if (targetGenericsTypes != null) {
GenericsType[] sourceGenericsTypes = parameterizedType.getGenericsTypes();
- if (sourceGenericsTypes == null) sourceGenericsTypes = EMPTY_GENERICS_ARRAY;
map = new LinkedHashMap<>();
- for (int i = 0, m = sourceGenericsTypes.length, n = targetGenericsTypes.length; i < n; i += 1) {
- map.put(targetGenericsTypes[i], i < m ? sourceGenericsTypes[i] : targetGenericsTypes[i]);
+ if (sourceGenericsTypes == null) {
+ //We have no sourceGenerics, so we need to return the erasures for the raw types (Groovy-10813)
+ for (GenericsType targetGenericsType : targetGenericsTypes) {
+ map.put(targetGenericsType, ClassHelper.OBJECT_TYPE.asGenericsType());
Review Comment:
Another example where the type parameter has an upper bound:
´´´java
interface A<T extends String> extends Function<T, T> {}
class B implements A{
@Override
Object apply(Object input) {
return null
}
}
´´´
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles closed pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
eric-milles closed pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
URL: https://github.com/apache/groovy/pull/1824
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] Shadow-Devil commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
Shadow-Devil commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1020265603
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -950,7 +956,7 @@ public static boolean hasUnresolvedGenerics(final ClassNode type) {
public static Tuple2<ClassNode[], ClassNode> parameterizeSAM(final ClassNode samType) {
MethodNode abstractMethod = ClassHelper.findSAM(samType);
- Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMapOfExactType(abstractMethod.getDeclaringClass(), samType);
+ Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMap(abstractMethod.getDeclaringClass(), samType);
Review Comment:
> Is this the stuff that sits under parametwrizeSAM? That was the key method for fixing lambda param types.
parameterizeSAM uses this makeDeclaringAndActualGenericsTypeMapOfExactType method to map declaring generics to actual generics. I thought that I needed to adjust this mapping so for unresolved generics it will output Object, but I'm still unsure if that's the right solution (as you mentioned in the other comment).
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] Shadow-Devil commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
Shadow-Devil commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1020417769
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -855,10 +855,16 @@ private static Map<GenericsType, GenericsType> doMakeDeclaringAndActualGenericsT
GenericsType[] targetGenericsTypes = parameterizedType.redirect().getGenericsTypes();
if (targetGenericsTypes != null) {
GenericsType[] sourceGenericsTypes = parameterizedType.getGenericsTypes();
- if (sourceGenericsTypes == null) sourceGenericsTypes = EMPTY_GENERICS_ARRAY;
map = new LinkedHashMap<>();
- for (int i = 0, m = sourceGenericsTypes.length, n = targetGenericsTypes.length; i < n; i += 1) {
- map.put(targetGenericsTypes[i], i < m ? sourceGenericsTypes[i] : targetGenericsTypes[i]);
+ if (sourceGenericsTypes == null) {
+ //We have no sourceGenerics, so we need to return the erasures for the raw types (Groovy-10813)
+ for (GenericsType targetGenericsType : targetGenericsTypes) {
+ map.put(targetGenericsType, ClassHelper.OBJECT_TYPE.asGenericsType());
Review Comment:
I've played around with generics in java, but couldn't find an example where the erasure isn't of type Object.
I've also tried something like this:
```java
import java.util.function.*
interface T<E> extends Function<String, E>{}
class TT implements T{
@Override
Object apply(Object o) {
return null
}
}
```
and it still loses all type information. Maybe I'm just not advanced enough into generics and how they work. Could you perhaps help me here or give me a resource where to learn more about this? I've found some information about the topic [here](https://docs.oracle.com/javase/tutorial/java/generics/rawTypes.html) and [here](https://docs.oracle.com/javase/tutorial/java/generics/erasure.html)
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] Shadow-Devil commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
Shadow-Devil commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1020468865
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -855,10 +855,16 @@ private static Map<GenericsType, GenericsType> doMakeDeclaringAndActualGenericsT
GenericsType[] targetGenericsTypes = parameterizedType.redirect().getGenericsTypes();
if (targetGenericsTypes != null) {
GenericsType[] sourceGenericsTypes = parameterizedType.getGenericsTypes();
- if (sourceGenericsTypes == null) sourceGenericsTypes = EMPTY_GENERICS_ARRAY;
map = new LinkedHashMap<>();
- for (int i = 0, m = sourceGenericsTypes.length, n = targetGenericsTypes.length; i < n; i += 1) {
- map.put(targetGenericsTypes[i], i < m ? sourceGenericsTypes[i] : targetGenericsTypes[i]);
+ if (sourceGenericsTypes == null) {
+ //We have no sourceGenerics, so we need to return the erasures for the raw types (Groovy-10813)
+ for (GenericsType targetGenericsType : targetGenericsTypes) {
+ map.put(targetGenericsType, ClassHelper.OBJECT_TYPE.asGenericsType());
Review Comment:
Another example where the type parameter has an upper bound:
```java
interface A<T extends String> extends Function<T, T> {}
class B implements A{
@Override
Object apply(Object input) {
return null
}
}
```
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] Shadow-Devil commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
Shadow-Devil commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1020266114
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -855,10 +855,16 @@ private static Map<GenericsType, GenericsType> doMakeDeclaringAndActualGenericsT
GenericsType[] targetGenericsTypes = parameterizedType.redirect().getGenericsTypes();
if (targetGenericsTypes != null) {
GenericsType[] sourceGenericsTypes = parameterizedType.getGenericsTypes();
- if (sourceGenericsTypes == null) sourceGenericsTypes = EMPTY_GENERICS_ARRAY;
map = new LinkedHashMap<>();
- for (int i = 0, m = sourceGenericsTypes.length, n = targetGenericsTypes.length; i < n; i += 1) {
- map.put(targetGenericsTypes[i], i < m ? sourceGenericsTypes[i] : targetGenericsTypes[i]);
+ if (sourceGenericsTypes == null) {
+ //We have no sourceGenerics, so we need to return the erasures for the raw types (Groovy-10813)
+ for (GenericsType targetGenericsType : targetGenericsTypes) {
+ map.put(targetGenericsType, ClassHelper.OBJECT_TYPE.asGenericsType());
Review Comment:
I will try to find an example.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] Shadow-Devil commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
Shadow-Devil commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1020890479
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -855,10 +855,16 @@ private static Map<GenericsType, GenericsType> doMakeDeclaringAndActualGenericsT
GenericsType[] targetGenericsTypes = parameterizedType.redirect().getGenericsTypes();
if (targetGenericsTypes != null) {
GenericsType[] sourceGenericsTypes = parameterizedType.getGenericsTypes();
- if (sourceGenericsTypes == null) sourceGenericsTypes = EMPTY_GENERICS_ARRAY;
map = new LinkedHashMap<>();
- for (int i = 0, m = sourceGenericsTypes.length, n = targetGenericsTypes.length; i < n; i += 1) {
- map.put(targetGenericsTypes[i], i < m ? sourceGenericsTypes[i] : targetGenericsTypes[i]);
+ if (sourceGenericsTypes == null) {
+ //We have no sourceGenerics, so we need to return the erasures for the raw types (Groovy-10813)
+ for (GenericsType targetGenericsType : targetGenericsTypes) {
+ map.put(targetGenericsType, ClassHelper.OBJECT_TYPE.asGenericsType());
Review Comment:
I've added some tests that have an upper bound. I was a bit suprised that `T extends String & Object` just selects the first one when you use raw types, so the result is different from `T extends Object & String`
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] FelixSchnabel commented on a diff in pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
FelixSchnabel commented on code in PR #1824:
URL: https://github.com/apache/groovy/pull/1824#discussion_r1020045088
##########
src/main/java/org/codehaus/groovy/ast/tools/GenericsUtils.java:
##########
@@ -950,7 +956,7 @@ public static boolean hasUnresolvedGenerics(final ClassNode type) {
public static Tuple2<ClassNode[], ClassNode> parameterizeSAM(final ClassNode samType) {
MethodNode abstractMethod = ClassHelper.findSAM(samType);
- Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMapOfExactType(abstractMethod.getDeclaringClass(), samType);
+ Map<GenericsType, GenericsType> generics = makeDeclaringAndActualGenericsTypeMap(abstractMethod.getDeclaringClass(), samType);
Review Comment:
I'm sorry, this was a mistake.
I will revert this change.
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [groovy] eric-milles commented on pull request #1824: GROOVY-10813: Fix NullPointerException for lambdas with raw types
Posted by GitBox <gi...@apache.org>.
eric-milles commented on PR #1824:
URL: https://github.com/apache/groovy/pull/1824#issuecomment-1320201876
I relocated the lambda tests and reworked the generics tests to ensure the placeholders are not returned.
https://github.com/apache/groovy/commit/ef9b58463e83f842b56f24dff0ac4df0e02f112f
--
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.
To unsubscribe, e-mail: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org