[
https://issues.apache.org/jira/browse/GROOVY-12740?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18140809#comment-18140809
]
ASF GitHub Bot commented on GROOVY-12740:
-----------------------------------------
codeconsole opened a new pull request, #3275:
URL: https://github.com/apache/groovy/pull/3275
Fixes [GROOVY-12740](https://issues.apache.org/jira/browse/GROOVY-12740).
A `@Requires`, `@Ensures` or `@Invariant` closure (and a loop `@Invariant`)
whose body holds a `return`, `assert` or `if` statement crashed the compiler
with a `NullPointerException` from `AssertStatementCreationUtility`:
`ExpressionUtils` added a null condition for every statement that is not an
expression statement, and `ClosureExpressionValidator` never checked statement
kinds. A closure that declares an empty parameter list with an explicit arrow,
`{ -> amount > 0 }`, crashed too, because `ClosureExpression.getParameters()`
is null for it and `AnnotationClosureVisitor` passed that to `Arrays.asList`.
Changes in `groovy-contracts`:
- `ClosureExpressionValidator` reports a compile error for a statement that
is not an expression statement, naming the kind: `Annotation closure may only
contain boolean expressions; return statements are not supported (e.g. use
'@Requires({ argument1 > 0 })').` `LoopInvariantASTTransformation` reports the
same for a loop invariant.
- `ExpressionUtils.getBooleanExpressions` skips statements that are not
expression statements instead of adding null, and gains `describeStatementKind`.
- `AnnotationClosureVisitor` reads the parameters through
`ClosureUtils.getParametersSafe`, so `{ -> ... }` compiles and behaves like the
arrow-less closure.
Tests: nine new tests in `ClosureExpressionValidationTests` and one in
`LoopInvariantTests` cover return, assert and if statements in preconditions,
postconditions and class and loop invariants, a statement beside an expression,
and the explicit empty parameter list for all three annotations. All ten fail
on master with the crashes above; the `groovy-contracts` module passes with the
fix (406 tests).
> groovy-contracts: a contract closure holding a return, assert or if
> statement, or declaring an empty parameter list, crashes the compiler with a
> NullPointerException
> ---------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: GROOVY-12740
> URL: https://issues.apache.org/jira/browse/GROOVY-12740
> Project: Groovy
> Issue Type: Bug
> Components: groovy-contracts
> Affects Versions: 6.0.0
> Reporter: Scott Murphy Heiberg
> Priority: Major
>
> A {{@Requires}}, {{@Ensures}} or {{@Invariant}} closure whose body is not a
> plain expression crashes the compiler with an internal error instead of a
> compile error. A closure that declares an empty parameter list with an
> explicit arrow, which is a valid closure, crashes it too.
> {code:groovy}
> import groovy.contracts.*
> class Account {
> @Requires({ return amount > 0 }) // 1
> // @Requires({ assert amount > 0 }) // 2
> // @Requires({ if (amount <= 0) throw new
> IllegalArgumentException('amount') }) // 3
> // @Requires({ -> amount > 0 }) // 4
> void deposit(BigDecimal amount) {}
> }
> {code}
> Forms 1 to 3 fail with:
> {noformat}
> BUG! exception in phase 'semantic analysis' in source unit 'Account.groovy'
> Cannot invoke
> "org.codehaus.groovy.ast.expr.BooleanExpression.getNodeMetaData(Object)"
> because "booleanExpression" is null
> at
> org.apache.groovy.contracts.generation.AssertStatementCreationUtility.getAssertionStatement(AssertStatementCreationUtility.java:85)
> at
> org.apache.groovy.contracts.generation.AssertStatementCreationUtility.getAssertionStatements(AssertStatementCreationUtility.java:67)
> at
> org.apache.groovy.contracts.ast.visitor.AnnotationClosureVisitor.replaceWithClosureClassReference(AnnotationClosureVisitor.java:250)
> {noformat}
> Form 4 fails with:
> {noformat}
> BUG! exception in phase 'semantic analysis' in source unit 'Account.groovy'
> null
> at
> org.apache.groovy.contracts.ast.visitor.AnnotationClosureVisitor.replaceWithClosureClassReference(AnnotationClosureVisitor.java:236)
> {noformat}
> {{@Ensures}} and class-level {{@Invariant}} fail the same way with the same
> bodies, and so does {{@Invariant}} on a loop (reported as "unexpected
> NullPointerException").
> h3. Cause
> {{ExpressionUtils.getBooleanExpression}} builds the condition list from the
> closure's statements and adds a null entry for every statement that is not an
> expression statement, and {{ClosureExpressionValidator}} never checks
> statement kinds, so the null reaches {{AssertStatementCreationUtility}}. For
> form 4, {{ClosureExpression.getParameters()}} returns null (its documented
> value for a closure with no parameters at all, as opposed to the empty array
> for an implicit {{it}}) and {{AnnotationClosureVisitor}} passes it to
> {{Arrays.asList}}.
> h3. Expected
> Forms 1 to 3 report a compile error that names the statement kind, for example
> {noformat}
> [groovy-contracts] Annotation closure may only contain boolean expressions;
> return statements are not supported (e.g. use '@Requires({ argument1 > 0 })').
> {noformat}
> and form 4 compiles and behaves exactly like the closure without the arrow (a
> {{PreconditionViolation}} for {{deposit(-1)}}). A {{return}} is rejected
> rather than taken as the condition: each statement of the closure is its own
> assertion, so a return has no meaning in that list.
> Reproduced on Groovy 6.0.0 and on master (df1fe66ed1). Pull request to follow.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
