[ 
https://issues.apache.org/jira/browse/GROOVY-12399?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18114178#comment-18114178
 ] 

ASF GitHub Bot commented on GROOVY-12399:
-----------------------------------------

codecov-commenter commented on PR #2922:
URL: https://github.com/apache/groovy/pull/2922#issuecomment-5632892485

   ## 
[Codecov](https://app.codecov.io/gh/apache/groovy/pull/2922?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 `70.00000%` with `21 lines` in your changes missing 
coverage. Please review.
   :white_check_mark: Project coverage is 71.2112%. Comparing base 
([`3eca1c9`](https://app.codecov.io/gh/apache/groovy/commit/3eca1c94b999d74d955407862a53d21bfbd9f7dd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
 to head 
([`f58f6f8`](https://app.codecov.io/gh/apache/groovy/commit/f58f6f8b5fd8a4e707accb681923afad6adcb2c2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2922?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/2922?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==)
 | 64.8148% | [8 Missing and 11 partials :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2922?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   | 
[...us/groovy/classgen/asm/SwitchExpressionWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2922?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FSwitchExpressionWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9Td2l0Y2hFeHByZXNzaW9uV3JpdGVyLmphdmE=)
 | 91.6667% | [1 Missing :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2922?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 |
   | 
[...sgen/asm/sc/StaticTypesSwitchExpressionWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2922?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2Fsc%2FStaticTypesSwitchExpressionWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9zYy9TdGF0aWNUeXBlc1N3aXRjaEV4cHJlc3Npb25Xcml0ZXIuamF2YQ==)
 | 50.0000% | [0 Missing and 1 partial :warning: 
](https://app.codecov.io/gh/apache/groovy/pull/2922?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>
   
   
   
   [![Impacted file tree 
graph](https://app.codecov.io/gh/apache/groovy/pull/2922/graphs/tree.svg?width=650&height=150&src=pr&token=1r45138NfQ&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)](https://app.codecov.io/gh/apache/groovy/pull/2922?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
   
   ```diff
   @@                Coverage Diff                 @@
   ##               master      #2922        +/-   ##
   ==================================================
   + Coverage     71.2065%   71.2112%   +0.0047%     
   - Complexity      37721      37758        +37     
   ==================================================
     Files            1584       1584                
     Lines          136489     136553        +64     
     Branches        25358      25383        +25     
   ==================================================
   + Hits            97189      97241        +52     
   - Misses          30538      30545         +7     
   - Partials         8762       8767         +5     
   ```
   
   | [Files with missing 
lines](https://app.codecov.io/gh/apache/groovy/pull/2922?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
 | Coverage Δ | |
   |---|---|---|
   | 
[...apache/groovy/ast/tools/SwitchExpressionUtils.java](https://app.codecov.io/gh/apache/groovy/pull/2922?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fapache%2Fgroovy%2Fast%2Ftools%2FSwitchExpressionUtils.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2dyb292eS9hc3QvdG9vbHMvU3dpdGNoRXhwcmVzc2lvblV0aWxzLmphdmE=)
 | `62.6866% <ø> (ø)` | |
   | 
[...roovy/transform/stc/StaticTypeCheckingVisitor.java](https://app.codecov.io/gh/apache/groovy/pull/2922?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Ftransform%2Fstc%2FStaticTypeCheckingVisitor.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L3RyYW5zZm9ybS9zdGMvU3RhdGljVHlwZUNoZWNraW5nVmlzaXRvci5qYXZh)
 | `87.2230% <100.0000%> (+0.0060%)` | :arrow_up: |
   | 
[...us/groovy/classgen/asm/SwitchExpressionWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2922?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2FSwitchExpressionWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9Td2l0Y2hFeHByZXNzaW9uV3JpdGVyLmphdmE=)
 | `94.3548% <91.6667%> (+0.6048%)` | :arrow_up: |
   | 
[...sgen/asm/sc/StaticTypesSwitchExpressionWriter.java](https://app.codecov.io/gh/apache/groovy/pull/2922?src=pr&el=tree&filepath=src%2Fmain%2Fjava%2Forg%2Fcodehaus%2Fgroovy%2Fclassgen%2Fasm%2Fsc%2FStaticTypesSwitchExpressionWriter.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3JjL21haW4vamF2YS9vcmcvY29kZWhhdXMvZ3Jvb3Z5L2NsYXNzZ2VuL2FzbS9zYy9TdGF0aWNUeXBlc1N3aXRjaEV4cHJlc3Npb25Xcml0ZXIuamF2YQ==)
 | `93.9698% <50.0000%> (+0.0305%)` | :arrow_up: |
   | 
[...va/org/apache/groovy/parser/antlr4/AstBuilder.java](https://app.codecov.io/gh/apache/groovy/pull/2922?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==)
 | `86.2056% <64.8148%> (-0.4281%)` | :arrow_down: |
   
   ... and [9 files with indirect coverage 
changes](https://app.codecov.io/gh/apache/groovy/pull/2922/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>




> Arrow switch statement without default throws IllegalStateException when no 
> arm matches
> ---------------------------------------------------------------------------------------
>
>                 Key: GROOVY-12399
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12399
>             Project: Groovy
>          Issue Type: Bug
>    Affects Versions: 6.0.0-RC-2
>            Reporter: Paul King
>            Priority: Major
>
> Since GROOVY-12255 (first-class switch expressions), a {{switch}} used as a 
> *statement* is only rewritten to a {{SwitchStatement}} when an arrow arm is a 
> block that does not {{yield}}. A single expression arm, or a block containing 
> exactly one expression statement, becomes an implicit {{yield}} and the 
> switch stays a {{SwitchExpression}} even though its value is discarded. Such 
> a switch now inherits the expression rules: an unmatched selector throws 
> {{IllegalStateException}} in dynamic code and a missing {{default}} is a 
> compile error under {{@TypeChecked}} / {{@CompileStatic}}.
> {code:groovy}
> def f(int i) {
>     def r = 'none'
>     switch (i) {
>         case 1 -> r = 'one'
>     }
>     r
> }
> f(42)
> {code}
> Groovy 5.1.2: returns {{'none'}}. Groovy 6.0.0-RC-1:
> {noformat}
> java.lang.IllegalStateException: the switch expression does not cover the 
> value 42
> {noformat}
> With {{@TypeChecked}} on {{f}}:
> {noformat}
> the switch expression does not cover all possible input values
> {noformat}
> The equivalent Java compiles and runs, returning {{"none"}}: an arrow-form 
> switch *statement* needs no {{default}} and does nothing when unmatched.
> {code:java}
> static String f(int i) {
>     String r = "none";
>     switch (i) {
>         case 1 -> r = "one";
>     }
>     return r;
> }
> f(42); // "none"
> {code}
> The boundary in Groovy depends on arm shape rather than position, which shows 
> it is not intended:
> {code:groovy}
> switch (i) { case 1 -> r = 'one' }               // throws when unmatched
> switch (i) { case 1 -> { r = 'one' } }           // throws when unmatched
> switch (i) { case 1 -> { r = 'one'; r += '!' } } // fine
> switch (i) { case 1 -> { if (i) r = 'one' } }    // fine
> switch (i) { case 1: r = 'one' }                 // fine
> {code}
> h3. Background
> In Java (JEP 361) the arrow is not what makes a switch an expression: both 
> switch statements and switch expressions may use either the arrow or the 
> colon form. The arrow controls fall-through; the position controls whether a 
> value is produced. A Java switch statement with arrow arms needs no 
> {{default}} and does nothing when unmatched (exhaustiveness is only required 
> for statements that use patterns or {{case null}}, JEP 441).
> Groovy's implementation grew up with the other reading. GROOVY-9272 (4.0) 
> added the arrow form _as_ the switch expression: the grammar rule is 
> {{switchExpression}}, it accepts only arrows, and a switch in statement 
> position was simply a switch expression whose value nobody read. That was 
> harmless while the lowering was a closure call that evaluated to {{null}} 
> when nothing matched. GROOVY-12255 kept "arrow means expression", gave 
> expressions real semantics (exhaustiveness, the unmatched throw) and added a 
> statement rewrite only for the one shape that visibly broke, blocks without 
> {{yield}}. The GROOVY-12255 compatibility notes describe the new behaviour 
> for switch _expressions_ only and state that a switch used as a statement is 
> a {{SwitchStatement}}; the statement-position tests in {{Groovy12255.groovy}} 
> all carry a {{default}} or always match, so the unmatched case was never 
> exercised.
> h3. Fix
> Classify by position rather than syntax, as Java does: a switch whose value 
> is not used always builds a {{SwitchStatement}}, whatever the arm shape. This 
> applies to dynamic and statically compiled code alike since the rewrite 
> happens in the parser. The exhaustiveness error and the unmatched-selector 
> throw stay reserved for switches whose value is used ({{def x = switch ...}}, 
> {{return switch ...}}, an argument, an arm of an enclosing switch expression).
> Implicit-return position, which Java has no equivalent for, is the one 
> Groovy-specific case: the last statement of a method, closure or script body 
> (followed through nested blocks, if/else branches, try/catch blocks and 
> colon-switch arms, as the compiler's {{ReturnAdder}} does). Both idioms below 
> were valid in 4.x/5.x and pull in opposite directions, so neither "statement" 
> nor "strict expression" fits:
> {code:groovy}
> list.collect { switch (it) { case 1 -> 'a'; default -> 'b' } }  // the 
> closure's value is the switch
> switch (args[0]) { case 'run' -> run() }                         // last 
> statement of a script; no match must not throw
> {code}
> There the switch keeps its value, but an unmatched selector yields {{null}} 
> rather than throwing and the type checker does not require exhaustiveness, 
> exactly as 4.x/5.x behaved. Code that wants the strict expression rules in 
> that position writes "return switch (...) \{...}".



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to