[
https://issues.apache.org/jira/browse/GROOVY-12463?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18128591#comment-18128591
]
ASF GitHub Bot commented on GROOVY-12463:
-----------------------------------------
codecov-commenter commented on PR #2988:
URL: https://github.com/apache/groovy/pull/2988#issuecomment-6259604675
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2988?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 72.4100%. Comparing base
([`d77e6df`](https://app.codecov.io/gh/apache/groovy/commit/d77e6df56a8d59ca30afbfa4c66afb9c685a22db?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`ddf552a`](https://app.codecov.io/gh/apache/groovy/commit/ddf552af5c2fd3154e04c4999f79e6857bb93fa6?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/groovy/pull/2988?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2988 +/- ##
==================================================
- Coverage 72.4166% 72.4100% -0.0066%
+ Complexity 40606 40603 -3
==================================================
Files 1643 1643
Lines 142633 142628 -5
Branches 26788 26787 -1
==================================================
- Hits 103290 103277 -13
- Misses 29850 29857 +7
- Partials 9493 9494 +1
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2988?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...cts/generation/AssertStatementCreationUtility.java](https://app.codecov.io/gh/apache/groovy/pull/2988?src=pr&el=tree&filepath=subprojects%2Fgroovy-contracts%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fcontracts%2Fgeneration%2FAssertStatementCreationUtility.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VicHJvamVjdHMvZ3Jvb3Z5LWNvbnRyYWN0cy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZ3Jvb3Z5L2NvbnRyYWN0cy9nZW5lcmF0aW9uL0Fzc2VydFN0YXRlbWVudENyZWF0aW9uVXRpbGl0eS5qYXZh)
| `94.2529% <100.0000%> (-0.3123%)` | :arrow_down: |
... and [10 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2988/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> 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
> Priority: Major
>
> 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)
