Copilot commented on code in PR #2378:
URL: https://github.com/apache/groovy/pull/2378#discussion_r2779162176
##########
src/testFixtures/groovy/org/codehaus/groovy/ast/builder/AstAssert.groovy:
##########
@@ -237,7 +237,11 @@ class AstAssert {
assertSyntaxTree([expected.arguments], [actual.arguments])
},
ForStatement : { expected, actual ->
- assertSyntaxTree([expected.variable], [actual.variable])
+ assert expected.valueVariable.asBoolean() ==
actual.valueVariable.asBoolean()
+ if (expected.valueVariable) {
+ assertSyntaxTree([expected.valueVariable],
[actual.valueVariable])
+ }
+ assertSyntaxTree([expected.indexVariable],
[actual.indexVariable])
Review Comment:
`ForStatement` assertion calls `expected.valueVariable.asBoolean()` /
`actual.valueVariable.asBoolean()` and unconditionally passes
`expected.indexVariable` into `assertSyntaxTree`. If `valueVariable` or
`indexVariable` is null (common for C-style loops and for-in loops without an
index variable), this can throw at runtime (null receiver for `asBoolean()`
and/or `item.getClass()` inside `assertSyntaxTree` when the list contains
null). Consider comparing presence with a null-safe check (e.g.,
`expected.valueVariable != null`) and only calling `assertSyntaxTree` for
`indexVariable`/`valueVariable` when they are non-null; otherwise assert the
corresponding actual variable is also null.
```suggestion
boolean expectedHasValueVariable = (expected.valueVariable
!= null)
boolean actualHasValueVariable = (actual.valueVariable !=
null)
Assert.assertEquals("Wrong presence of value variable in
ForStatement", expectedHasValueVariable, actualHasValueVariable)
if (expectedHasValueVariable) {
assertSyntaxTree([expected.valueVariable],
[actual.valueVariable])
}
boolean expectedHasIndexVariable = (expected.indexVariable
!= null)
boolean actualHasIndexVariable = (actual.indexVariable !=
null)
Assert.assertEquals("Wrong presence of index variable in
ForStatement", expectedHasIndexVariable, actualHasIndexVariable)
if (expectedHasIndexVariable) {
assertSyntaxTree([expected.indexVariable],
[actual.indexVariable])
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
