[
https://issues.apache.org/jira/browse/GROOVY-8045?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17542251#comment-17542251
]
ASF GitHub Bot commented on GROOVY-8045:
----------------------------------------
sonatype-lift[bot] commented on code in PR #1722:
URL: https://github.com/apache/groovy/pull/1722#discussion_r882143891
##########
src/main/java/org/codehaus/groovy/reflection/ParameterTypes.java:
##########
@@ -292,77 +290,72 @@ public boolean isValidExactMethod(Class[] args) {
return true;
}
- private static boolean testComponentAssignable(Class toTestAgainst, Class
toTest) {
- Class component = toTest.getComponentType();
- if (component == null) return false;
- return MetaClassHelper.isAssignableFrom(toTestAgainst, component);
- }
-
- private static boolean isValidVarargsMethod(Class[] arguments, int size,
CachedClass[] pt, int paramMinus1) {
- // first check normal number of parameters
- for (int i = 0; i < paramMinus1; i++) {
- if (pt[i].isAssignableFrom(arguments[i])) continue;
- return false;
+ private static boolean isValidVargsMethod(Class[] argumentTypes,
CachedClass[] parameterTypes, int nthParameter) {
+ for (int i = 0; i < nthParameter; i += 1) {
+ if (!parameterTypes[i].isAssignableFrom(argumentTypes[i])) {
+ return false;
+ }
}
+ CachedClass arrayType = parameterTypes[nthParameter];
+ CachedClass componentType =
ReflectionCache.getCachedClass(arrayType.getTheClass().getComponentType());
+
// check direct match
- CachedClass varg = pt[paramMinus1];
- Class clazz = varg.getTheClass().getComponentType();
- if (size == pt.length &&
- (varg.isAssignableFrom(arguments[paramMinus1]) ||
- testComponentAssignable(clazz,
arguments[paramMinus1]))) {
- return true;
+ if (argumentTypes.length == parameterTypes.length) {
+ Class argumentType = argumentTypes[nthParameter];
+ if (arrayType.isAssignableFrom(argumentType) ||
(argumentType.isArray()
+ &&
componentType.isAssignableFrom(argumentType.getComponentType()))) {
Review Comment:
*NULL_DEREFERENCE:* object `componentType` last assigned on line 301 could
be null and is dereferenced at line 307.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with
`help` or `ignore`)
---
Was this a good recommendation?
[ [🙁 Not
relevant](https://www.sonatype.com/lift-comment-rating?comment=243425071&lift_comment_rating=1)
] - [ [😕 Won't
fix](https://www.sonatype.com/lift-comment-rating?comment=243425071&lift_comment_rating=2)
] - [ [😑 Not critical, will
fix](https://www.sonatype.com/lift-comment-rating?comment=243425071&lift_comment_rating=3)
] - [ [🙂 Critical, will
fix](https://www.sonatype.com/lift-comment-rating?comment=243425071&lift_comment_rating=4)
] - [ [😊 Critical, fixing
now](https://www.sonatype.com/lift-comment-rating?comment=243425071&lift_comment_rating=5)
]
##########
src/main/java/org/codehaus/groovy/reflection/ParameterTypes.java:
##########
@@ -292,77 +290,72 @@ public boolean isValidExactMethod(Class[] args) {
return true;
}
- private static boolean testComponentAssignable(Class toTestAgainst, Class
toTest) {
- Class component = toTest.getComponentType();
- if (component == null) return false;
- return MetaClassHelper.isAssignableFrom(toTestAgainst, component);
- }
-
- private static boolean isValidVarargsMethod(Class[] arguments, int size,
CachedClass[] pt, int paramMinus1) {
- // first check normal number of parameters
- for (int i = 0; i < paramMinus1; i++) {
- if (pt[i].isAssignableFrom(arguments[i])) continue;
- return false;
+ private static boolean isValidVargsMethod(Class[] argumentTypes,
CachedClass[] parameterTypes, int nthParameter) {
+ for (int i = 0; i < nthParameter; i += 1) {
+ if (!parameterTypes[i].isAssignableFrom(argumentTypes[i])) {
+ return false;
+ }
}
+ CachedClass arrayType = parameterTypes[nthParameter];
+ CachedClass componentType =
ReflectionCache.getCachedClass(arrayType.getTheClass().getComponentType());
+
// check direct match
- CachedClass varg = pt[paramMinus1];
- Class clazz = varg.getTheClass().getComponentType();
- if (size == pt.length &&
- (varg.isAssignableFrom(arguments[paramMinus1]) ||
- testComponentAssignable(clazz,
arguments[paramMinus1]))) {
- return true;
+ if (argumentTypes.length == parameterTypes.length) {
+ Class argumentType = argumentTypes[nthParameter];
+ if (arrayType.isAssignableFrom(argumentType) ||
(argumentType.isArray()
+ &&
componentType.isAssignableFrom(argumentType.getComponentType()))) {
+ return true;
+ }
}
- // check varged
- for (int i = paramMinus1; i < size; i++) {
- if (MetaClassHelper.isAssignableFrom(clazz, arguments[i]))
continue;
- return false;
+ // check vararg match
+ for (int i = nthParameter; i < argumentTypes.length; i += 1) {
+ if (!componentType.isAssignableFrom(argumentTypes[i])) {
Review Comment:
*NULL_DEREFERENCE:* object `componentType` last assigned on line 301 could
be null and is dereferenced at line 314.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with
`help` or `ignore`)
---
Was this a good recommendation?
[ [🙁 Not
relevant](https://www.sonatype.com/lift-comment-rating?comment=243425264&lift_comment_rating=1)
] - [ [😕 Won't
fix](https://www.sonatype.com/lift-comment-rating?comment=243425264&lift_comment_rating=2)
] - [ [😑 Not critical, will
fix](https://www.sonatype.com/lift-comment-rating?comment=243425264&lift_comment_rating=3)
] - [ [🙂 Critical, will
fix](https://www.sonatype.com/lift-comment-rating?comment=243425264&lift_comment_rating=4)
] - [ [😊 Critical, fixing
now](https://www.sonatype.com/lift-comment-rating?comment=243425264&lift_comment_rating=5)
]
##########
src/main/java/org/codehaus/groovy/reflection/ParameterTypes.java:
##########
@@ -292,77 +290,72 @@ public boolean isValidExactMethod(Class[] args) {
return true;
}
- private static boolean testComponentAssignable(Class toTestAgainst, Class
toTest) {
- Class component = toTest.getComponentType();
- if (component == null) return false;
- return MetaClassHelper.isAssignableFrom(toTestAgainst, component);
- }
-
- private static boolean isValidVarargsMethod(Class[] arguments, int size,
CachedClass[] pt, int paramMinus1) {
- // first check normal number of parameters
- for (int i = 0; i < paramMinus1; i++) {
- if (pt[i].isAssignableFrom(arguments[i])) continue;
- return false;
+ private static boolean isValidVargsMethod(Class[] argumentTypes,
CachedClass[] parameterTypes, int nthParameter) {
+ for (int i = 0; i < nthParameter; i += 1) {
+ if (!parameterTypes[i].isAssignableFrom(argumentTypes[i])) {
+ return false;
+ }
}
+ CachedClass arrayType = parameterTypes[nthParameter];
+ CachedClass componentType =
ReflectionCache.getCachedClass(arrayType.getTheClass().getComponentType());
+
// check direct match
- CachedClass varg = pt[paramMinus1];
- Class clazz = varg.getTheClass().getComponentType();
- if (size == pt.length &&
- (varg.isAssignableFrom(arguments[paramMinus1]) ||
- testComponentAssignable(clazz,
arguments[paramMinus1]))) {
- return true;
+ if (argumentTypes.length == parameterTypes.length) {
+ Class argumentType = argumentTypes[nthParameter];
+ if (arrayType.isAssignableFrom(argumentType) ||
(argumentType.isArray()
+ &&
componentType.isAssignableFrom(argumentType.getComponentType()))) {
+ return true;
+ }
}
- // check varged
- for (int i = paramMinus1; i < size; i++) {
- if (MetaClassHelper.isAssignableFrom(clazz, arguments[i]))
continue;
- return false;
+ // check vararg match
+ for (int i = nthParameter; i < argumentTypes.length; i += 1) {
+ if (!componentType.isAssignableFrom(argumentTypes[i])) {
+ return false;
+ }
}
+
return true;
}
public boolean isValidMethod(Object[] arguments) {
if (arguments == null) return true;
-
- final int size = arguments.length;
- CachedClass[] paramTypes = getParameterTypes();
- final int paramMinus1 = paramTypes.length - 1;
-
- if (size >= paramMinus1 && paramTypes.length > 0 &&
- paramTypes[(paramMinus1)].isArray) {
- // first check normal number of parameters
- for (int i = 0; i < paramMinus1; i++) {
- if (paramTypes[i].isAssignableFrom(getArgClass(arguments[i])))
continue;
- return false;
+ final CachedClass[] parameterTypes = getParameterTypes();
+ final int nArguments = arguments.length, nParameters =
parameterTypes.length, nthParameter = nParameters - 1;
Review Comment:
*NULL_DEREFERENCE:* object `parameterTypes` last assigned on line 324 could
be null and is dereferenced at line 325.
(at-me [in a reply](https://help.sonatype.com/lift/talking-to-lift) with
`help` or `ignore`)
---
Was this a good recommendation?
[ [🙁 Not
relevant](https://www.sonatype.com/lift-comment-rating?comment=243425289&lift_comment_rating=1)
] - [ [😕 Won't
fix](https://www.sonatype.com/lift-comment-rating?comment=243425289&lift_comment_rating=2)
] - [ [😑 Not critical, will
fix](https://www.sonatype.com/lift-comment-rating?comment=243425289&lift_comment_rating=3)
] - [ [🙂 Critical, will
fix](https://www.sonatype.com/lift-comment-rating?comment=243425289&lift_comment_rating=4)
] - [ [😊 Critical, fixing
now](https://www.sonatype.com/lift-comment-rating?comment=243425289&lift_comment_rating=5)
]
> Implicit closure coercion doesn't work for elements of array of functional
> objects
> ----------------------------------------------------------------------------------
>
> Key: GROOVY-8045
> URL: https://issues.apache.org/jira/browse/GROOVY-8045
> Project: Groovy
> Issue Type: Bug
> Components: Compiler
> Affects Versions: 2.4.7
> Reporter: Dimitar Dimitrov
> Assignee: Eric Milles
> Priority: Major
> Labels: varargs
>
> Implicit closure coercion is described
> [here|http://groovy-lang.org/releasenotes/groovy-2.2.html] - it assumes that
> the closures don't need to be casted to functional types and the generic
> types will be inferred by the compiler.
> Here is one contrived case that works from Java (this is not production code
> and is writen explicitly for illustration purposes):
> {code}
> public class GroovyAccDemo {
> @SafeVarargs
> public static <T, R> Function<T, R> ensemble(Function<T, R>...
> hypotheses) {
> return t -> Arrays.stream(hypotheses)
> .map(v -> v.apply(t))
> .collect(Collectors.groupingBy(e -> e,
> Collectors.counting()))
> .entrySet()
> .stream()
> .max(Comparator.comparingLong(Map.Entry::getValue))
> .map(Map.Entry::getKey).orElseGet(() -> null);
> }
> public static void main(String[] args) {
> Function<Integer, Integer> foo = ensemble(
> i -> i*i,
> i -> i+i,
> i -> i*i - (i+i)
> );
> }
> }
> {code}
> Here the {{ensemble()}} method accepts a number of compatible functions and
> returns a single function that calls all hypotheses and returns the most
> popular result.
> The main method illustrates that we can use the {{ensemble()}} function with
> Java Lambdas without any explicit casts.
> If we try to do the same in Groovy, we'll get runtime error (or compile error
> if static compilation is enabled):
> {code}
> foo = GroovyAccDemo.<Integer, Integer> ensemble(
> { i -> i * i },
> { i -> i + i },
> { i -> i * i - (i + i) }
> );
> {code}
> We can make it work by explicitly coercing the closures like this:
> {code}
> foo = GroovyAccDemo.ensemble(
> { i -> i*i } as Function,
> { i -> i+i } as Function,
> { i -> i*i - (i+i ) } as Function
> );
> {code}
> This may seem as contrived use case, but it makes the use of certain API's
> more tedious from Groovy than from Java, which just feels wrong ;-)
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
