Assertions in Switch Expressions lead to compiler error

[joshuabrandes]

Describe the bug

Version: 2.4-M1-groovy-4.0

I have a test case where I need to get content form a map and assert its value. I first tried it with a switch expression and build a example I can share:

class MathTest extends Specification {

    def "test square"() {
        setup: "two numbers"
        var b = 2
        Map<Integer, Integer> map = new HashMap<>()
        map.put(b, Main.square(b))

        expect: "squaring the number"
        switch (b) {
            case 2 -> map.get(b) == 4
            default -> throw new Exception("Not implemented")
        }
    }
}

When I ant to run this test I get the following build output:

Groovyc: While compiling [tests of spock-testing]: Unexpected error during compilation of spec 'MathTest'. Maybe you have used invalid Spock syntax? Anyway, please file a bug report at https://issues.spockframework.org.

java.lang.ClassCastException: class org.codehaus.groovy.ast.stmt.AssertStatement cannot be cast to class org.codehaus.groovy.ast.stmt.BlockStatement (org.codehaus.groovy.ast.stmt.AssertStatement and org.codehaus.groovy.ast.stmt.BlockStatement are in unnamed module of loader org.jetbrains.jps.incremental.groovy.JointCompilationClassLoader @43b8b9ec)
	at org.spockframework.compiler.AstUtil.getStatements(AstUtil.java:88)
	at org.spockframework.compiler.DeepBlockRewriter.defineRecorders(DeepBlockRewriter.java:305)
	at org.spockframework.compiler.DeepBlockRewriter.doVisitClosureExpression(DeepBlockRewriter.java:136)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.visitClosureExpression(AbstractDeepBlockRewriter.java:152)
	at org.codehaus.groovy.ast.expr.ClosureExpression.visit(ClosureExpression.java:110)
	at org.codehaus.groovy.ast.CodeVisitorSupport.visitMethodCallExpression(CodeVisitorSupport.java:184)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.doVisitMethodCallExpression(AbstractDeepBlockRewriter.java:170)
	at org.spockframework.compiler.DeepBlockRewriter.doVisitMethodCallExpression(DeepBlockRewriter.java:148)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.visitMethodCallExpression(AbstractDeepBlockRewriter.java:132)
	at org.codehaus.groovy.ast.expr.MethodCallExpression.visit(MethodCallExpression.java:77)
	at org.codehaus.groovy.ast.CodeVisitorSupport.visitReturnStatement(CodeVisitorSupport.java:122)
	at org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitReturnStatement(ClassCodeVisitorSupport.java:222)
	at org.codehaus.groovy.ast.stmt.ReturnStatement.visit(ReturnStatement.java:73)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.replace(StatementReplacingVisitorSupport.java:44)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.replaceAll(StatementReplacingVisitorSupport.java:59)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.visitBlockStatement(StatementReplacingVisitorSupport.java:72)
	at org.codehaus.groovy.ast.stmt.BlockStatement.visit(BlockStatement.java:70)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.replace(StatementReplacingVisitorSupport.java:44)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.visitCaseStatement(StatementReplacingVisitorSupport.java:119)
	at org.codehaus.groovy.ast.stmt.CaseStatement.visit(CaseStatement.java:56)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.replace(StatementReplacingVisitorSupport.java:44)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.replaceAll(StatementReplacingVisitorSupport.java:59)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.visitSwitch(StatementReplacingVisitorSupport.java:112)
	at org.codehaus.groovy.ast.stmt.SwitchStatement.visit(SwitchStatement.java:54)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.replace(StatementReplacingVisitorSupport.java:44)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.replaceAll(StatementReplacingVisitorSupport.java:59)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.visitBlockStatement(StatementReplacingVisitorSupport.java:72)
	at org.codehaus.groovy.ast.stmt.BlockStatement.visit(BlockStatement.java:70)
	at org.codehaus.groovy.ast.CodeVisitorSupport.visitClosureExpression(CodeVisitorSupport.java:239)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.doVisitClosureExpression(AbstractDeepBlockRewriter.java:174)
	at org.spockframework.compiler.DeepBlockRewriter.doVisitClosureExpression(DeepBlockRewriter.java:135)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.visitClosureExpression(AbstractDeepBlockRewriter.java:152)
	at org.codehaus.groovy.ast.expr.ClosureExpression.visit(ClosureExpression.java:110)
	at org.codehaus.groovy.ast.CodeVisitorSupport.visitMethodCallExpression(CodeVisitorSupport.java:184)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.doVisitMethodCallExpression(AbstractDeepBlockRewriter.java:170)
	at org.spockframework.compiler.DeepBlockRewriter.doVisitMethodCallExpression(DeepBlockRewriter.java:148)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.visitMethodCallExpression(AbstractDeepBlockRewriter.java:132)
	at org.codehaus.groovy.ast.expr.MethodCallExpression.visit(MethodCallExpression.java:77)
	at org.codehaus.groovy.ast.CodeVisitorSupport.visitExpressionStatement(CodeVisitorSupport.java:117)
	at org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitExpressionStatement(ClassCodeVisitorSupport.java:204)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.doVisitExpressionStatement(AbstractDeepBlockRewriter.java:162)
	at org.spockframework.compiler.DeepBlockRewriter.visitInteractionAwareExpressionStatement(DeepBlockRewriter.java:96)
	at org.spockframework.compiler.DeepBlockRewriter.doVisitExpressionStatement(DeepBlockRewriter.java:78)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.visitExpressionStatement(AbstractDeepBlockRewriter.java:100)
	at org.codehaus.groovy.ast.stmt.ExpressionStatement.visit(ExpressionStatement.java:41)
	at org.spockframework.compiler.StatementReplacingVisitorSupport.replace(StatementReplacingVisitorSupport.java:44)
	at org.spockframework.compiler.AbstractDeepBlockRewriter.visit(AbstractDeepBlockRewriter.java:84)
	at org.spockframework.compiler.DeepBlockRewriter.visit(DeepBlockRewriter.java:56)
	at org.spockframework.compiler.SpecRewriter.visitAnyBlock(SpecRewriter.java:405)
	at org.spockframework.compiler.model.ExpectBlock.accept(ExpectBlock.java:31)
	at org.spockframework.compiler.model.Method.accept(Method.java:70)
	at org.spockframework.compiler.model.Spec.accept(Spec.java:112)
	at org.spockframework.compiler.SpockTransform$Impl.processSpec(SpockTransform.java:76)
	at org.spockframework.compiler.SpockTransform$Impl.visit(SpockTransform.java:63)
	at org.spockframework.compiler.SpockTransform.visit(SpockTransform.java:48)
	at org.codehaus.groovy.transform.ASTTransformationVisitor.lambda$addPhaseOperationsForGlobalTransforms$5(ASTTransformationVisitor.java:377)
	at org.codehaus.groovy.control.CompilationUnit$ISourceUnitOperation.doPhaseOperation(CompilationUnit.java:896)
	at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:692)
	at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:666)
	at org.jetbrains.groovy.compiler.rt.GroovyCompilerWrapper.compile(GroovyCompilerWrapper.java:48)
	at org.jetbrains.groovy.compiler.rt.DependentGroovycRunner.runGroovyc(DependentGroovycRunner.java:122)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
	at org.jetbrains.groovy.compiler.rt.GroovycRunner.intMain2(GroovycRunner.java:80)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
	at java.base/java.lang.reflect.Method.invoke(Method.java:578)
	at org.jetbrains.jps.incremental.groovy.InProcessGroovyc.runGroovycInThisProcess(InProcessGroovyc.java:167)
	at org.jetbrains.jps.incremental.groovy.InProcessGroovyc.lambda$runGroovyc$0(InProcessGroovyc.java:77)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:317)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642)
	at java.base/java.lang.Thread.run(Thread.java:1589)

To Reproduce

def "assert test"() {
    expect:
    def b = 3
    switch (b) {
        default -> assert 1 == 1
    }
}

Expected behavior

assertion is executed in the matching case

Actual behavior

compiler error

Java version

java version "21" 2023-09-19 LTS

Buildtool version

Apache Maven 3.8.5

What operating system are you using

Windows

Dependencies

--- maven-dependency-plugin:2.8:tree (default-cli) @ spock-testing ---
[INFO] org.example:spock-testing:jar:1.0-SNAPSHOT
[INFO] +- org.junit.jupiter:junit-jupiter-api:jar:5.8.2:test
[INFO] |  +- org.opentest4j:opentest4j:jar:1.2.0:compile
[INFO] |  +- org.junit.platform:junit-platform-commons:jar:1.8.2:compile
[INFO] |  \- org.apiguardian:apiguardian-api:jar:1.1.2:compile
[INFO] \- org.spockframework:spock-core:jar:2.4-M1-groovy-4.0:compile
[INFO]    +- org.apache.groovy:groovy:jar:4.0.6:compile
[INFO]    +- org.junit.platform:junit-platform-engine:jar:1.9.0:compile
[INFO]    \- org.hamcrest:hamcrest:jar:2.2:compile

Additional context

No response

[leonard84]

FWIW implicit assertions in if-blocks are ignored, so even if we fix the compile error, I'm not sure that we'd treat the branches of the switch-expression as implicit conditions.

In general I'd consider conditional logic in tests a code smell.
I know that practically might sometimes still make them necessary, but they should be avoided if possible.

[Vampire]

It is the opposite.
The first example works "just fine", just without power assertion output.
It is a switch expression, so that expression is the implicit assertion and as long as the switch evaluates to true the condition holds, if it evaluates to false, the condition fails.

So as long as there is only one condition in each of the branches, it works as might be expected.
If you have multiple conditions, then not, as only the branch result is considered.
In such a case you would need to use explicit assert like case 1 -> { assert 1 == 1; assert 2 == 2 } which also works fine and gives power assertion output.

The default in the first example could actually just be false and it would work the same.

The problem is the second example that does not compile properly.
If you have only one condition and make that an explicit assertion.
Because then it lands in the AST directly as one AssertStatement as body of the ClosureExpression that is the branch body even if no curly braces are used and we expect this always to be a BlockStatement.
No idea whether it is a bug in Groovy that in the assert case it is not a BlockStatement, but it should be easily fixable and should then work as expected.

If the compilation is fixed, then you could use explicit assertion even with only one condition in the branch and then get the power assertion output properly.

A simple work-around is not to use a switch expression, but a switch statement. (colon instead of arrow)
This works just fine right now.