[
https://issues.apache.org/jira/browse/GROOVY-12254?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18104232#comment-18104232
]
ASF GitHub Bot commented on GROOVY-12254:
-----------------------------------------
codecov-commenter commented on PR #2783:
URL: https://github.com/apache/groovy/pull/2783#issuecomment-5274814298
##
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2783?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:x: Patch coverage is `92.30769%` with `1 line` in your changes missing
coverage. Please review.
:white_check_mark: Project coverage is 70.1108%. Comparing base
([`fbf16d5`](https://app.codecov.io/gh/apache/groovy/commit/fbf16d5f1995bfd2ba541e537f743454e8e9cd2f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`acd1b0e`](https://app.codecov.io/gh/apache/groovy/commit/acd1b0e707bdaa0f3775e385c5665ece641c28d2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on master.
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2783?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Patch % | Lines |
|---|---|---|
|
[...va/org/apache/groovy/parser/antlr4/AstBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2783?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fparser%2Fantlr4%2FAstBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9wYXJzZXIvYW50bHI0L0FzdEJ1aWxkZXIuamF2YQ==)
| 92.3077% | [0 Missing and 1 partial :warning:
](https://app.codecov.io/gh/apache/groovy/pull/2783?src=pr&el=tree&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/2783?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2783 +/- ##
==================================================
- Coverage 70.1169% 70.1108% -0.0061%
+ Complexity 35771 35769 -2
==================================================
Files 1561 1561
Lines 132349 132362 +13
Branches 24330 24331 +1
==================================================
+ Hits 92799 92800 +1
- Misses 31155 31161 +6
- Partials 8395 8401 +6
```
| [Files with missing
lines](https://app.codecov.io/gh/apache/groovy/pull/2783?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
| Coverage Δ | |
|---|---|---|
|
[...va/org/apache/groovy/parser/antlr4/AstBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2783?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fparser%2Fantlr4%2FAstBuilder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9wYXJzZXIvYW50bHI0L0FzdEJ1aWxkZXIuamF2YQ==)
| `87.5214% <92.3077%> (+0.0267%)` | :arrow_up: |
... and [7 files with indirect coverage
changes](https://app.codecov.io/gh/apache/groovy/pull/2783/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>
> Make defer fully contextual: parse as a keyword only inside async closures
> --------------------------------------------------------------------------
>
> Key: GROOVY-12254
> URL: https://issues.apache.org/jira/browse/GROOVY-12254
> Project: Groovy
> Issue Type: Improvement
> Affects Versions: 6.0.0-beta-1
> Reporter: Paul King
> Priority: Major
>
> h3. Problem
> Groovy 6.0.0-beta-1 introduced {{defer}} as a contextual keyword as part of
> native async/await support (GROOVY-9381). While {{defer}} remains usable as
> an identifier in most positions (variable names, assignments, map keys,
> property access, explicit-receiver calls, expression-position calls), the
> {{deferStmtAlt}} grammar alternative is unguarded, so any statement matching
> {{defer <expression>}} is claimed by the keyword in _all_ contexts, not just
> inside {{async}} closures.
> This silently breaks pre-Groovy-6 code that calls a user-defined {{defer}}
> method in statement position:
> {code:groovy}
> def defer(Closure c) { c() }
> defer { println 'hi' } // Groovy 5: method call; 6.0.0-beta-1: parsed as a
> defer statement
> defer 'command style' // likewise
> defer('parens') // likewise — ('parens') is itself a valid expression
> {code}
> In beta-1 such code compiles but fails at runtime with an obscure error that
> leaks an implementation detail:
> {code}
> groovy.lang.MissingPropertyException: No such property: $__deferScope__ for
> class: Foo
> {code}
> A genuinely misplaced {{defer}} (i.e. outside any {{async}} closure, with no
> user method intended) produces the same unfriendly error, including under
> {{@CompileStatic}} — there is currently no compile-time diagnostic on any
> path.
> h3. Proposed fix
> Gate the statement alternative with a semantic predicate, following the
> existing {{yieldStatement}} / {{inSwitchExpressionLevel}} pattern in the
> grammar:
> * a new parser field {{inAsyncClosureLevel}}, incremented/decremented around
> the closure/lambda of {{asyncClosureExprAlt}}
> * {{deferStmtAlt}} guarded so it is only viable when {{inAsyncClosureLevel >
> 0}}
> Outside an {{async}} closure the {{defer}} token then falls through to the
> identifier path, so statement-position calls parse as ordinary method calls
> again, restoring Groovy 5 source compatibility. A misplaced {{defer}} with no
> such method in scope fails with Groovy's normal {{MissingMethodException}}
> rather than the internal-variable error.
> In addition, an AstBuilder check reports a clear compile-time error for
> {{defer}} used inside a _nested_ (non-async) closure within an {{async}}
> closure. The runtime semantics there were never defined — the scope wrapping
> deliberately does not descend into nested closures
> ({{AsyncTransformHelper.containsDefer}}) — so this now fails fast:
> {code}
> defer must be used directly in the body of an async closure, not in a nested
> closure
> {code}
> h3. Behaviour comparison
> || code || Groovy 5 || 6.0.0-beta-1 || with this fix ||
> | defer { ... } / {{defer x}} / {{defer\(x)}} as a statement, user {{defer}}
> method in scope | method call | runtime {{MissingPropertyException}}
> (internal variable) | method call (compatible with Groovy 5) |
> | same, no {{defer}} method in scope | {{MissingMethodException}} | runtime
> {{MissingPropertyException}} (internal variable) | {{MissingMethodException}}
> |
> | {{defer}} directly inside async { } (closure or lambda) | n/a | works |
> works (unchanged) |
> | {{defer}} in a nested non-async closure inside async { } | n/a | undefined
> behaviour / obscure runtime error | clear compile-time error |
> h3. Known limitation
> A statement-position call to a user-defined {{defer}} method _inside_ an
> {{async}} closure (at any nesting depth) is still claimed by the keyword.
> This is the irreducible cost of the contextual-keyword design; the fix
> confines it to async closure bodies instead of applying everywhere.
> h3. Alternative considered
> An AstBuilder-only check (no grammar change) turning every out-of-async
> {{defer}} statement into a compile-time error with a dedicated message. This
> gives the best possible diagnostics but keeps beta-1's source incompatibility
> for DSLs with a {{defer}} method, so the predicate approach is preferred.
> (Proposed pending team review.)
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
