You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@groovy.apache.org by "paulk-asert (via GitHub)" <gi...@apache.org> on 2025/04/17 06:16:15 UTC
[PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
paulk-asert opened a new pull request, #2199:
URL: https://github.com/apache/groovy/pull/2199
…ified will cause a groovy compile error for switch expressions even with type checking enabled
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "daniellansun (via GitHub)" <gi...@apache.org>.
daniellansun commented on PR #2199:
URL: https://github.com/apache/groovy/pull/2199#issuecomment-2814267260
LGTM.
P.S. It's better to add some marker to limit visiting further.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #2199:
URL: https://github.com/apache/groovy/pull/2199#issuecomment-2811947062
## [Codecov](https://app.codecov.io/gh/apache/groovy/pull/2199?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 68.9408%. Comparing base [(`b7711d8`)](https://app.codecov.io/gh/apache/groovy/commit/b7711d8511da7d9eb55af7a4a9a66e736cc8b914?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to head [(`3b05487`)](https://app.codecov.io/gh/apache/groovy/commit/3b054877c219b3db7200a83e07e32e15f38a3315?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/2199?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## master #2199 +/- ##
==================================================
+ Coverage 68.9403% 68.9408% +0.0005%
+ Complexity 29552 29550 -2
==================================================
Files 1423 1423
Lines 113900 113905 +5
Branches 19752 19754 +2
==================================================
+ Hits 78523 78527 +4
Misses 28776 28776
- Partials 6601 6602 +1
```
| [Files with missing lines](https://app.codecov.io/gh/apache/groovy/pull/2199?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [...a/org/codehaus/groovy/control/CompilationUnit.java](https://app.codecov.io/gh/apache/groovy/pull/2199?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fcontrol%2FCompilationUnit.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NvbnRyb2wvQ29tcGlsYXRpb25Vbml0LmphdmE=) | `80.6794% <100.0000%> (+0.2073%)` | :arrow_up: |
... and [4 files with indirect coverage changes](https://app.codecov.io/gh/apache/groovy/pull/2199/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>
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "daniellansun (via GitHub)" <gi...@apache.org>.
daniellansun commented on PR #2199:
URL: https://github.com/apache/groovy/pull/2199#issuecomment-2819582901
Merged. Thanks.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "daniellansun (via GitHub)" <gi...@apache.org>.
daniellansun merged PR #2199:
URL: https://github.com/apache/groovy/pull/2199
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "daniellansun (via GitHub)" <gi...@apache.org>.
daniellansun commented on PR #2199:
URL: https://github.com/apache/groovy/pull/2199#issuecomment-2816547275
If switch expression closure delegates to the enum class, switch expression and switch statement will be different. As a result, it's hard for users to understand if they do not know the delegating.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "eric-milles (via GitHub)" <gi...@apache.org>.
eric-milles commented on PR #2199:
URL: https://github.com/apache/groovy/pull/2199#issuecomment-2815666933
Is it possible for the switch expression closure to delegate to the enum class? Would that be safe to do?
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "eric-milles (via GitHub)" <gi...@apache.org>.
eric-milles commented on PR #2199:
URL: https://github.com/apache/groovy/pull/2199#issuecomment-2812884480
Can you explain the design of this one? I don't quite follow. I think this plays into the reason we limited the previous fix to STC.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on PR #2199:
URL: https://github.com/apache/groovy/pull/2199#issuecomment-2812980365
> Can you explain the design of this one? I don't quite follow. I think this plays into the reason we limited the previous fix to STC.
This just applies the previous fix (so only STC) to Switch Expressions as well as Switch Statements. The ClassCodeExpressionTransformer for the previous fix doesn't normally traverse deeply. We "create" a Switch Expression by wrapping a Switch Statement into a Closure. The change here just causes it to traverse deeper into the wrapped code. I did try adding some additional marker metadata to make traversal even more selective, but it was transformed away before it gets to where we'd look it up. We could try to track down where that happens and make it preserve the metadata, but looking inside for the Closure expression should eliminate many traversal paths.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "daniellansun (via GitHub)" <gi...@apache.org>.
daniellansun commented on PR #2199:
URL: https://github.com/apache/groovy/pull/2199#issuecomment-2817255442
If delegation applied, method calls within switch expression can also be delegated to enum class. But switch statement treats the method calls as invalid.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GROOVY-11614: Enums in switch/case statements that are not fully qual… [groovy]
Posted by "eric-milles (via GitHub)" <gi...@apache.org>.
eric-milles commented on PR #2199:
URL: https://github.com/apache/groovy/pull/2199#issuecomment-2817232959
They are already different. Switch statement supports unqualified enum constants under STC. Switch expression does not. The expression case could be made to work with delegation, in my estimation. It could be limited to STC as well for the initial cut. Any other differences in behavior could be listed.
--
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: notifications-unsubscribe@groovy.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org