Paul King created GROOVY-12463:
----------------------------------
Summary: groovy-contracts: postcondition and class invariant not
checked on a trailing implicit return when the method also has an explicit
early return
Key: GROOVY-12463
URL: https://issues.apache.org/jira/browse/GROOVY-12463
Project: Groovy
Issue Type: Bug
Components: groovy-contracts
Affects Versions: 6.0.0
Reporter: Paul King
h2. Problem
When a non-void method has an explicit {{return}} on one path and produces its
value through an implicit return on another, the implicit path skips the
contract checks. Neither the {{@Ensures}} postcondition nor the class
{{@Invariant}} is checked there, and the method returns an unchecked value.
{code:groovy}
import groovy.contracts.*
class H {
@Ensures({ result > 0 })
int implicitAfterEarly(boolean b) { if (b) return 1; -1 }
@Ensures({ result > 0 })
int explicitBoth(boolean b) { if (b) return 1; return -1 }
@Ensures({ result > 0 })
int ifElseImplicit(boolean b) { if (b) { 1 } else { -1 } }
}
def h = new H()
h.implicitAfterEarly(false) // returns -1, no PostconditionViolation
h.explicitBoth(false) // PostconditionViolation
h.ifElseImplicit(false) // PostconditionViolation
{code}
The same happens with {{@CompileStatic}} and with class invariants:
{code:groovy}
@Invariant({ count >= 0 })
class K {
int count = 0
int dec(boolean b) { if (b) return count; count -= 5; count }
}
new K().dec(false) // returns -5, no ClassInvariantViolation
{code}
This is easy to hit with guard clauses, a common style: return early for bad
input, then let the last expression be the result. Found while writing a
contract for a date parser of that shape, where a 31 February result was not
caught.
h2. Cause
{{AssertStatementCreationUtility.getReturnStatements}} collects the method's
explicit {{ReturnStatement}} nodes. It runs {{ReturnAdder}}, which turns
trailing expressions and value-producing if/else, switch and try statements
into returns, only when that list is empty. GROOVY-12129 introduced the
{{ReturnAdder}} delegation, but its tests do not cover a method that mixes the
two. With any explicit return present, the trailing expression stays an
expression statement, no assertion is woven before it, and the compiler adds
the return later, unchecked.
All three callers are affected:
* {{PostconditionGenerator}}: {{@Ensures}}.
* {{ClassInvariantGenerator}}: invariant checks on non-void methods.
* {{AnnotationClosureVisitor.recordNonNullReturnViolations}}: the {{return
null}} paths handed to NullChecker.
h2. Fix
Always run {{ReturnAdder}} before collecting the returns. {{ReturnAdder}}
performs the conversion the compiler applies later anyway and leaves existing
return statements unchanged. Applying it unconditionally does not change what
the method computes; it only makes every exit path visible to the weaving.
h2. Tests
Added to {{ImplicitReturnPostconditionTests}} and {{SimpleClassInvariantTests}}:
* trailing implicit return after an early return is checked (violation
detected);
* the returned values are unchanged by weaving (vacuous postcondition);
* trailing implicit if/else after an early return is checked;
* the same under {{@CompileStatic}}, with an {{Optional}} result;
* the class invariant is checked on the trailing implicit path.
Four of the five fail before the fix. The values test passes either way,
because the bug skipped the check without changing the value. With the fix, the
full groovy-contracts suite (364 tests) and groovy-typecheckers suite (356
tests) pass.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
