[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17745172#comment-17745172
]
Eric Milles commented on GROOVY-10099:
--
When the method target is known, {{InvocationWriter}} adds an {{instanceof}}
check to sort out if the single argument for the variadic parameter is an array
or a scalar (see GROOVY-10722). The cached and uncached paths have no fore
knowledge of the method target. So all I can think of is to write a typecast
for every trailing argument. I don't think that would be reasonable to do.
See InvokeDynamicWriter#makeIndyCall args loop.
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
>Reporter: Rachel Greenham
>Assignee: Eric Milles
>Priority: Major
> Labels: varargs
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 3h 20m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the static name 3 with blah=null
> params is null? false
> params length is 1
> [null]
> Arrays.asList(blah)? [null]
> name: the dynamic name 1
> params is null? false
> params length is 1
> [blah]
> name: the dynamic name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the dynamic name 3 with blah=null
> params is null? true
> Exception thrown
> java.lang.NullPointerException
> ...{code}
> (etc. stack trace not shown for formatting reasons.)
> VarArgsTest.jsh - a jshell script demonstrating Java's behaviour,
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17743464#comment-17743464
]
Eric Milles commented on GROOVY-10099:
--
The {{Wrapper}} type is what Groovy writes to pass type(cast) information to
the runtime. GROOVY-6146 supports casting the last (nullable) argument and
calling the varargs method appropriately. There may be a path to supporting
the non-array expression as only argument to variadic parameter.
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
>Reporter: Rachel Greenham
>Assignee: Eric Milles
>Priority: Major
> Labels: varargs
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 3h 20m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the static name 3 with blah=null
> params is null? false
> params length is 1
> [null]
> Arrays.asList(blah)? [null]
> name: the dynamic name 1
> params is null? false
> params length is 1
> [blah]
> name: the dynamic name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the dynamic name 3 with blah=null
> params is null? true
> Exception thrown
> java.lang.NullPointerException
> ...{code}
> (etc. stack trace not shown for formatting reasons.)
> VarArgsTest.jsh - a jshell script demonstrating Java's behaviour, very
> similar to the groovy test, but omitting the dynamic portion of the test for
> obvious reasons. (The statements in the Groovy script
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17583790#comment-17583790
]
Eric Milles commented on GROOVY-10099:
--
https://github.com/apache/groovy/commit/7824bd62fc4cc21e1a6b6b376f4835f6f6049d1a
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
>Reporter: Rachel Greenham
>Assignee: Eric Milles
>Priority: Major
> Labels: varargs
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 3h 20m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the static name 3 with blah=null
> params is null? false
> params length is 1
> [null]
> Arrays.asList(blah)? [null]
> name: the dynamic name 1
> params is null? false
> params length is 1
> [blah]
> name: the dynamic name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the dynamic name 3 with blah=null
> params is null? true
> Exception thrown
> java.lang.NullPointerException
> ...{code}
> (etc. stack trace not shown for formatting reasons.)
> VarArgsTest.jsh - a jshell script demonstrating Java's behaviour, very
> similar to the groovy test, but omitting the dynamic portion of the test for
> obvious reasons. (The statements in the Groovy script ending in semicolons
> are left that way precisely to mark that they're identical to the Java test.)
> Runnable with
>
> {code:java}
> jshell PRINTING VarArgsTest.jsh
> {code}
> Output seen:
> {code:java}
> name:
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17583735#comment-17583735
]
Eric Milles commented on GROOVY-10099:
--
It is consistent with Java calling conventions that a {{null}} literal in the
last position is for the array and not a single-element array. Yes, this can
be influenced with a typecast in Java and static Groovy. And an expression can
muddy the waters as well. Long story short, the runtime lacks any info about
the static nature of a {{null}} value. I don't think it would be good to have
3 sets of behaviors -- dynamic/legacy, type-checked and static compilation.
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
>Reporter: Rachel Greenham
>Priority: Major
> Labels: breaking, varargs
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 3h 20m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the static name 3 with blah=null
> params is null? false
> params length is 1
> [null]
> Arrays.asList(blah)? [null]
> name: the dynamic name 1
> params is null? false
> params length is 1
> [blah]
> name: the dynamic name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the dynamic name 3 with blah=null
> params is null? true
> Exception thrown
> java.lang.NullPointerException
> ...{code}
> (etc. stack trace not shown for formatting reasons.)
> VarArgsTest.jsh - a jshell script demonstrating Java's
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17583728#comment-17583728
]
ASF GitHub Bot commented on GROOVY-10099:
-
eric-milles commented on PR #1601:
URL: https://github.com/apache/groovy/pull/1601#issuecomment-1224298737
-1 for me
I will look over your tests and see about adding them to the build at least
to demonstrate GROOVY-10099.
`InvocationWriter#loadArguments` already has significant checking/handling
for variadic situations. And `null` is supposed to represent the array, not an
array with one `null` element. I think the safe advice is to use
`@CompileStatic` if you have a scenario that passes a single nullable argument
for the array parameter and you don't want to wrap in your code.
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Labels: varargs
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 3h 20m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the static name 3 with blah=null
> params is null? false
> params length is 1
> [null]
> Arrays.asList(blah)? [null]
> name: the dynamic name 1
> params is null? false
> params length is 1
> [blah]
> name: the dynamic name 2
>
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17583729#comment-17583729
]
ASF GitHub Bot commented on GROOVY-10099:
-
eric-milles closed pull request #1601: GROOVY-10099: Resolve ambiguous varargs
behaviour in dynamic mode by recourse to compiler-stage information
URL: https://github.com/apache/groovy/pull/1601
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Labels: varargs
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 3h 20m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the static name 3 with blah=null
> params is null? false
> params length is 1
> [null]
> Arrays.asList(blah)? [null]
> name: the dynamic name 1
> params is null? false
> params length is 1
> [blah]
> name: the dynamic name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the dynamic name 3 with blah=null
> params is null? true
> Exception thrown
> java.lang.NullPointerException
> ...{code}
> (etc. stack trace not shown for formatting reasons.)
> VarArgsTest.jsh - a jshell script demonstrating Java's behaviour, very
> similar to the groovy test, but omitting the dynamic portion of the test for
> obvious reasons.
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17446770#comment-17446770
]
Jochen Theodorou commented on GROOVY-10099:
---
Since this popped up for me...
We actually do have some kind of compile time type information for the dynamic
calls, just that they are not very extensive and maybe some are incorrect atm.
But if you do
{code:Java}
String x = null
foo(x)
{code}
then we know even in dynamic groovy at compile time, that foo is called with
something of static type String. For invokedynamic we would here then produce a
callsite that takes a String. And this is actually part of why indy code has
problems at times - it needs to do a lot of type lookups and "zero
transformations" just to satisfy the lambda logic for the JVM optimizations.
But what we then do, because of how the MOP works, there is no place for this
information to be carried along. And at the time of method selection we do not
know this anymore. If indy is the only way to call a method (and that is the
case now afaik), we could now do small changes to for example transport the
MethodType or something similar of the call site. And then we could do a more
educated guess for selecting something for null.
This is of course totally ignoring the fact, that if we are not making calls
with static typed variables, but with complex expressions no, scratch
that.. a simple foo(x.bar(y)) is actually already enough. In dynamic Groovy we
*cannot* know the return type of x.bar(y). Any educated guess for a null is
then over.
Dynamic Groovy is dynamic typed and that has many implication that deviate it
from static Java. We try to be near Java, but we cannot always do that.
So to come back to [~Rachel Greenham]: It is no faux-pas in principle to be
using type data known at compile time. In fact the "primitive optimizations" I
added back in I think 1.6 where exactly doing that, use and promote types to
bypass the Meta Object Protocol (MOP) and have thus fast code that works on
primitives and calculations using them. They are working under the assumption,
that runtime meta class transformations are active and have a guard for that.
That means I actually let the compiler produce 2 paths in the bytecode, of
which one is considering types and the other is not, depending on if somebody
hooks into the MOP or not. But looking at semantics the program was still
supposed to produce the same result. Sometimes considering the type for a null
call and sometimes not, because we cannot statically be sure what it is... well
that is really something we should not do - unless we have really really good
reasons for it.
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Labels: varargs
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 3h 20m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
>
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17360755#comment-17360755
]
Paul King commented on GROOVY-10099:
bq. Am I making a category error here?
I wouldn't call it an error but you have correctly identified one of the
complexities. In dynamic mode, method dispatch is determined by runtime type,
so looking too much at types during compilation is not routine. And if you need
to look at types, @CompileStatic/@TypeChecked is our way to look at those types
"properly". In this case, when dealing with {{null}}, we don't have a runtime
type, yet we have, at least for some of the cases (and to be honest, the only
ones I think we should consider addressing), an explicit cast to give us more
typing information. If we could make that information available in some fashion
at runtime, we could achieve the outcome you are after.
With that background, the current PR could be considered a bit of a "hack" that
fights against the natural approach that should be in place. Having said that,
it certainly covers the ground of what things we'd need to check against if we
can think of a better approach. And, a conceptually nicer approach might incur
other costs we deem unacceptable for the majority of non-vararg calls, so maybe
a hack is the best we can hope for. I'll think about it some more when I have
more time. A few other deadlines might get in the way of a rapid response but
you or other folks might have ideas in the meantime.
As a minimum, we should expand our current documentation with more examples
like what is in the test cases, to elaborate on the current workarounds - to
make those developer conversations you alluded too less awkward in the meantime.
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17360668#comment-17360668
]
Rachel Greenham commented on GROOVY-10099:
--
Thoughts that keep me awake at night:
Am I making a category error here? In dynamic compiling mode is it actually a
big faux-pas in principle to be using type data known at compile time to affect
the code being compiled? Looking through the docs last night I was noticing how
- outside of @CompileStatic or @TypeChecked, the use of types seemed to be just
for
* Self-documentation
* Invoking *from* a type-checked setting
* Allowing IDEs to offer autocomplete suggestions
Nothing about making the slightest difference to the code that runs.
I had just thought, well, the information's *available* while compiling, and
lost at runtime, we should be using it to decide what to do, but is that kind
of opportunism and pragmatism breaking some actual rule and principle of
dynamic programming in force here? And that's why one answer to the actual
problem might be: use @CompileStatic.
Someone would've said something, right? :)
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17360099#comment-17360099
]
Rachel Greenham commented on GROOVY-10099:
--
Seems to be an argument for ignoring issues because it's easier than fixing
them, which seems to have been the fate of all those issues quoted: No debate,
just... ignored and left to wither. It feels like this one only got attention
(for which thank you, by the way) because it was followed up by an actual PR
which had to be responded to. :)
(Seriously, there's nothing wrong with being a voice for caution, but it feels
like it invites a robust defence, so here goes: :) )
So the advice is: "If you need to call a varargs method, do so via a
statically-compiled intermediate method, so it doesn't blow up in certain
situations." It's a bit... rubbish, isn't it? If you're in @CompileStatic
anyway for other reasons, like "this bit needs to run faster", fine, but having
to do it, and lose the _benefits_ of a dynamic language, just to get
predictable behaviour out of varargs calls, is extremely retrograde.
I'm trying to get Groovy integrated into our platform. We already have a way of
invoking scripts that basically goes through a method on a context object that
looks like this (names have been changed):
{code:java}
public Object perform(String scriptName, Object... args) {...}{code}
Which will now preferentially find and execute a groovy script, and fall back
to scripts. And it may be called trivially both from scripts in the
legacy scripting language (obscure, older-than-groovy, barrier-to-hiring), and
from Java code, like:
{code:java}
perform("/misc/sanitize-basket", session.getBasket());{code}
Which could be used verbatim in Groovy, though more idiomatically like:
{code:java}
perform '/misc/sanitize-basket', session.basket{code}
Only, when you do this - either - in the rare-but-valid case of that
Session::getBasket method returning null, the target script blows up, because
its generated code looks like:
{code:java}
public Object run(Object[] args) {
Basket basket = (Basket)args[0];
if (basket != null) { ...
{code}
But only when called from Groovy. Not from anywhere else, because normal
varargs behaviour is, if basket is null you still get the array with a null
basket, because the argument _type_... is Basket. It's known by the compiler so
it wraps it up.
If I _wanted_ to call it with a null constant, or obtain it untyped, I'd
_expect_ to have to cast it, in case it was null:
{code:java}
perform '/misc/sanitize-basket', (Basket)null
perform '/misc/sanitize-basket', session.attrs["basket-${name}"] as Basket
{code}
... but at the moment that wouldn't even work.
It's an acceptance issue. Imagine the conversation:
{quote}"So you changed perform so it catches nulls and makes an array which...
hopefully doesn't break anything else..."
"The scripts have to be given an args array the length of the number
of declared parameters, but if they're not declaring any they don't even look
at it. So that should be safe if it's not null. I don't think they ever let you
actually declare an array as a parameter. It's one of the limitations."
"Hmm. But otherwise the workaround is, to call varargs methods via another
intermediate method compiled statically?"
"Yeah. Or you can make it an array here, like: {{}}
{code:java}
[session.basket] as Basket[]{code}
"So now you're making an ArrayList and toArraying it. Every time."
"Uh, yeah. Or you can make the array directly, like we did in Java before
varargs."
__
"
{code:java}
session.basket ?: Basket.ARRAY_WITH_NULL
{code}
? :D"
"Did we look at JRuby yet?"{color:#172b4d} {color}
{quote}
:)
I can't stress enough that the PR is attempting to intervene only where the
expression type is known, and known not to be dynamically typed or an array.
Maybe there's bugs in that identification, so we fix those if they arise. But
as for the principle of it: if the type is known, when is it ever right to be
knowingly sending a non-array argument to an array parameter? Such that it
could only escape an immediate ClassCastException if it's null, or if it's
intercepted and turned into an identical array anyway at runtime.
And I don't care about the no-cast null constant case. I thought it made sense
to make it consistent with @CompileStatic behaviour, but that's not a hill I
want to die on. It can go on instead being treated as a dynamically-typed
expression. This is only fixing what happens when we _have_ the type. The
reporter of GROOVY-6146 appeared to think the same way: Marking in bold the
important bit: that even when you cast it explicitly to String it ignores you
and still treats it like it's an array. This PR would fix that bug too.
Does it break the mitigations people might have put in to get around this?
* If they ensure the argument can never be null? Then no: It's just turning
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17359584#comment-17359584
]
Eric Milles commented on GROOVY-10099:
--
GROOVY-6146 raises this concern as well. Is there any sense of how common this
might be? And is @CompileStatic an acceptable enough workaround. There are a
number of legacy choices that are hard to get away from and static compilation
offers an opportunity to make more compile-time decisions.
GROOVY-2433/GROOVY-5438 comes to mind.
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
> Time Spent: 1h 10m
> Remaining Estimate: 0h
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the static name 3 with blah=null
> params is null? false
> params length is 1
> [null]
> Arrays.asList(blah)? [null]
> name: the dynamic name 1
> params is null? false
> params length is 1
> [blah]
> name: the dynamic name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the dynamic name 3 with blah=null
> params is null? true
> Exception thrown
> java.lang.NullPointerException
> ...{code}
> (etc. stack trace not shown for formatting reasons.)
> VarArgsTest.jsh - a jshell script demonstrating Java's behaviour,
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17349833#comment-17349833
]
Rachel Greenham commented on GROOVY-10099:
--
That fix won't work. It breaks too much. I'm sure you're as unsurprised as I
am. I had fun though. :D
Fundamentally you can't fix it at runtime because you can't tell if a single
null value is a null object reference or a null array reference. And there's
much that depends on the implicit assumption that it's an array reference. For
instance, if you change it, you can't set or initialise a POGO's array-typed
field to null. Which doesn't break immediately, it breaks later, when something
tries to use it.
If this can be fixed anywhere, it's in the compiler. Maybe the AstBuilder? It
has more information. After all, it works as expected in {{@CompileStatic}}.
Even in {{@CompileDynamic}} mode, it presumably has (or could have) a better
idea of what a given _expression type_ is; at least quite a lot of the time: if
it's a call to a real method, or a typed object reference, or a cast or
type-coercion in the expression itself. It would be able to tell if the
expression's _type_ is an array type or not. And if it's not an array type, and
the parameter it fits to is a varargs parameter, whatever value it evaluates to
at runtime (including null) should be wrapped in an array of one, and it can
write that array-wrapping operation into the compiled code. Like it does in
{{@CompileStatic}}, like javac does... As with {{@TypeChecked}} mode, it would
still be writing an invokeMethod call, but it would wrap that parameter in an
array. Then all the runtime invoke code sees is that array and it just passes
it through.
And if it _hasn't_ got a definitive idea of the expression type, because it's
an untyped variable, or an expression that _must_ be resolved to a dynamic call
(ie: something that {{@CompileStatic}} would balk at), it would just pass it in
as it is, the same behaviour as now, so it breaks nothing. And if it doesn't
behave how the coder expects, they'd be able to resolve it by disambiguating
the type of that expression in their own code (eg: with a cast, a type
coercion, or declaring a variable or method return type properly).
So it wouldn't solve it everywhere, 100%, as it couldn't solve it for dynamic
expressions of unknowable type at compiletime. But solving it for known
expression types would reduce the footprint of the problem a _lot_, I expect,
and put the rest in the hands of the coder to eg: put a cast in if necessary.
This would have solved my original problem, as it was a situation where the
expression type was known, or at least knowable. But it was just one line in a
script that didn't otherwise need to be {{@CompileStatic}}.
I'm not averse to trying that approach, but I don't know where to start...
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time -
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17348592#comment-17348592
]
Rachel Greenham commented on GROOVY-10099:
--
I'll go at it at the weekend.
I was wondering if part of the original confusion/debate was because in groovy
Object[] and Object... are treated interchangeably, based simply on whether the
last parameter's type is an array type, whereas that's not the case in Java.
Therefore, in the immediate aftermath of Java getting varargs, GROOVY_1026
maybe was an attempt at keeping things behaving as they did before, for
Object[] methods.
I don't think we can tell if a Groovy method was declared with Object..., as
its Method::isVarArgs just returns true either way, but Java does let one
distinguish. If we need to reduce the footprint of the change it might be to
restrict it to calling Java classes' (does not implement GroovyObject) methods
where isVarArgs() returns true, cache that in CachedMethod because it's those
java methods that tend to assume they'll never receive null. But I expect the
first preference would be to have a simpler change that is consistent
everywhere. If it doesn't tear the world down around our ears!
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17348559#comment-17348559
]
Paul King commented on GROOVY-10099:
If you have a fix and the test suite completes successfully bar one or more of
those specific tests in VarargsMethodTest, then I think we would be in good
shape and a PR would be very welcome including the test changes. If numerous
other things break, then I think we would have some more thinking to do before
deciding on the best fix. Up to you in that scenario, whether you want to
submit a PR at this stage.
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the static name 3 with blah=null
> params is null? false
> params length is 1
> [null]
> Arrays.asList(blah)? [null]
> name: the dynamic name 1
> params is null? false
> params length is 1
> [blah]
> name: the dynamic name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the dynamic name 3 with blah=null
> params is null? true
> Exception thrown
> java.lang.NullPointerException
> ...{code}
> (etc. stack trace not shown for formatting reasons.)
> VarArgsTest.jsh - a jshell script demonstrating
[jira] [Commented] (GROOVY-10099) A single null argument to a varargs parameter is received as null
[
https://issues.apache.org/jira/browse/GROOVY-10099?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17348314#comment-17348314
]
Rachel Greenham commented on GROOVY-10099:
--
Noting that the current behaviour is deliberate and specifically tested for in
VarargsMethodTest.groovy, so that too would need to be changed, so implying
some kind of discussion on _whether_ to do so...
Should I just submit the patch, including the test change, and if so, as a PR
on GitHub?
> A single null argument to a varargs parameter is received as null
> -
>
> Key: GROOVY-10099
> URL: https://issues.apache.org/jira/browse/GROOVY-10099
> Project: Groovy
> Issue Type: Bug
> Environment: Observed on Groovy 3.0.8 on macOS Big Sur (Intel), but I
> don't think that's relevant; it'll be everywhere.
>Reporter: Rachel Greenham
>Priority: Major
> Fix For: 4.x
>
> Attachments: VarArgsTest.groovy, VarArgsTest.jsh
>
>
> (NB: I would set the priority to P2 default to be triaged, but I seem not to
> have that option, so I left it at the default I was presented with.)
> When calling a method with a varargs parameter with a single value, that
> value is wrapped into an array of length 1. This is the behaviour in Java,
> and is the expected behaviour, and _most_ of the time is the behaviour in
> Groovy too.
> But when that single value is null, Groovy will instead just pass the null
> into the method. Java will not do this: You'll get an array with a single
> null in it. Because Groovy's behaviour is unexpected, especially when
> interfacing with Java code, NullPointerExceptions can often ensue.
> Adding to the inconsistencies, if the Groovy code calling the method is in a
> {{@CompileStatic}} context, it behaves just like Java, and the method
> (whether or not _it_ is statically compiled or a dynamic Groovy method)
> receives an array with a null in it.
> So the behaviour in Groovy is inconsistent, both with itself in a
> {{@CompileStatic}} situation, and with Java, and is requiring workarounds in
> Java code to handle this normally-impossible eventuality. (Even if no varargs
> parameter is given you get an empty array, as in fact you do in Groovy too.)
> This may be an "early instalment weirdness": There's an ancient ticket, from
> not long after varargs were introduced into Java, which appears to have
> argued - successfully at the time - that the normal behaviour is a bug:
> https://issues.apache.org/jira/browse/GROOVY-1026
> Further adding to the confusion may be that Groovy usually elides the
> difference between an {{Object[]}} parameter and an {{Object...}} parameter:
> They both behave the same.
> The offending code appears to be in
> org.codehaus.groovy.reflection.ParameterTypes.java in method fitToVars, lines
> 200-215 in master at the time of writing, which even includes a comment that
> "if the last argument is null, then we don't have to do anything", with which
> I respectfully disagree. :) That behaviour should be to return an array with
> a single null in it (Handily, MetaClassHelper.ARRAY_WITH_NULL saves having to
> make a new one.)
> In principle it's an easy fix (although I've left tagging to others as this
> is my first issue here), but there'd be an obvious nervousness about changing
> behaviour like this when there might be a lot of old code out there depending
> on it behaving the way it does now. OTOH the way it behaves now is breaking
> the expectations of those of us coming to Groovy from a lifetime of Java...
> Attachments:
> VarArgsTest.groovy - a script saved from, and runnable in, groovyConsole,
> demonstrating the behaviour. The behaviour is the same regardless of whether
> the console is launched with the -indy option. (The issue was initially
> observed in indy.) The dynamic portion of the test, when run, ends in a
> NullPointerException as Arrays.asList is not expecting a null varargs
> parameter. Output seen (-indy or not):
>
> {code:java}
> name: the static name 1
> params is null? false
> params length is 1
> [blah]
> name: the static name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the static name 3 with blah=null
> params is null? false
> params length is 1
> [null]
> Arrays.asList(blah)? [null]
> name: the dynamic name 1
> params is null? false
> params length is 1
> [blah]
> name: the dynamic name 2
> params is null? false
> params length is 2
> [blah, blue]
> name: the dynamic name 3 with blah=null
> params is null? true
> Exception thrown
> java.lang.NullPointerException
> ...{code}
> (etc. stack trace not shown for formatting reasons.)
> VarArgsTest.jsh - a jshell script demonstrating Java's behaviour, very
> similar to the groovy test, but omitting the dynamic portion of the test for
> obvious
