Enable implicit non-top-level conditions (#1160)

Author: Vampire

docs/release_notes.adoc

@@ -37,6 +37,25 @@ _This is a summary of the highlights of the milestone releases_
 
 === Breaking Changes
 
+* It is no longer true that only top-level expression statements are treated as implicit conditions.
+Just like within `with { ... }`, `verifyAll { ... }`, `@ConditionBlock` methods, and other places,
+each expression statement is now considered an implicit condition, except for statements in unknown
+closures.
++
+This enables the usage of `if` and similar while also being able to still safely use `.every { ... }`,
+but without the regular confusion why a test is not failing if the condition is inside an `if`.
+You can even have interactions in those nested places now.
++
+As now expression statements are considered implicit conditions that previously were not,
+this can lead to compilation errors in existing code-bases, because such nested statements were
+previously simply ignored by the Spock compiler and now also must be valid conditions.
++
+If those compile errors are not accidentally written assignments that should have been conditions
+or similar, you can for example use the <<spock_primer.adoc#opt-out-of-condition-handling,`!!` prefix operator>>
+to declare an expression statement explicitly as not being an implicit condition.
++
+spockPull:2250[]
+
 * _This affects users of the `@Snapshot` extension, only if you were using the snapshotter in parent specification classes._ +
 `@Snapshot` used to look up snapshots in directories named after the class containing feature methods. Now, the snapshots will be loaded from directories named after the bottom class in the specification hierarchy. The motivation of the change is to allow users to define features in base specification classes, but overwrite expected snapshots per child specification.
 spockPull:2112[]
@@ -69,6 +88,27 @@ will now require that the spec is annotated with <<parallel_execution.adoc#isola
 * Spock supports Groovy 5.0 spockIssue:2196[]
 * Spock is also tested to run correctly on Java 25 LTS spockPull:2212[]
 
+=== Breaking Changes
+
+* It is no longer true that only top-level expression statements are treated as implicit conditions.
+Just like within `with { ... }`, `verifyAll { ... }`, `@ConditionBlock` methods, and other places,
+each expression statement is now considered an implicit condition, except for statements in unknown
+closures.
++
+This enables the usage of `if` and similar while also being able to still safely use `.every { ... }`,
+but without the regular confusion why a test is not failing if the condition is inside an `if`.
+You can even have interactions in those nested places now.
++
+As now expression statements are considered implicit conditions that previously were not,
+this can lead to compilation errors in existing code-bases, because such nested statements were
+previously simply ignored by the Spock compiler and now also must be valid conditions.
++
+If those compile errors are not accidentally written assignments that should have been conditions
+or similar, you can for example use the <<spock_primer.adoc#opt-out-of-condition-handling,`!!` prefix operator>>
+to declare an expression statement explicitly as not being an implicit condition.
++
+spockPull:2250[]
+
 === Misc
 
 * Fix handling of condition method calls spockPull:2162[]

docs/spock_primer.adoc

@@ -230,8 +230,9 @@ digestible form. Nice, isn't it?
 ===== Implicit and explicit conditions
 
 Conditions are an essential ingredient of `then` blocks and `expect` blocks. Except for calls to `void` methods and
-expressions classified as interactions, all top-level expressions in these blocks are implicitly treated as conditions.
-To use conditions in other places, you need to designate them with Groovy's assert keyword:
+expressions classified as interactions, all expression statements in these blocks are implicitly treated as conditions
+except if they happen inside some unknown closure.
+To use conditions in other places, you need to designate them with Groovy's `assert` keyword:
 
 [source,groovy]
 ----

spock-core/src/main/java/org/spockframework/compiler/AbstractDeepBlockRewriter.java

@@ -146,7 +146,7 @@ public final void visitClosureExpression(ClosureExpression expr) {
     groupConditionFound = false;
     ISpecialMethodCall oldSpecialMethodCall = currSpecialMethodCall;
     if (!currSpecialMethodCall.isMatch(expr)) {
-      currSpecialMethodCall = NoSpecialMethodCall.INSTANCE; // unrelated closure terminates currSpecialMethodCall scope
+      currSpecialMethodCall = NoSpecialMethodCall.CLOSURE_INSTANCE; // unrelated closure terminates currSpecialMethodCall scope
     }
     try {
       Statement code = expr.getCode();

spock-core/src/main/java/org/spockframework/compiler/DeepBlockRewriter.java

@@ -193,7 +193,7 @@ private boolean handleInteraction(InteractionRewriter rewriter, ExpressionStatem
   }
 
   private boolean handleImplicitCondition(ExpressionStatement stat) {
-    if (!(stat == currTopLevelStat && isThenOrExpectOrFilterBlock()
+    if (!(((currSpecialMethodCall == NoSpecialMethodCall.INSTANCE) && isThenOrExpectOrFilterBlock())
         || currSpecialMethodCall.isConditionMethodCall()
         || currSpecialMethodCall.isConditionBlock()
         || currSpecialMethodCall.isGroupConditionBlock()

spock-core/src/main/java/org/spockframework/compiler/NoSpecialMethodCall.java

@@ -23,6 +23,7 @@
 
 public class NoSpecialMethodCall implements ISpecialMethodCall {
   public static final ISpecialMethodCall INSTANCE = new NoSpecialMethodCall();
+  public static final ISpecialMethodCall CLOSURE_INSTANCE = new NoSpecialMethodCall();
 
   @Override
   public boolean isMethodName(String name) {

spock-junit4/src/test/groovy/org/spockframework/junit4/junit/JUnitFixtureMethods.groovy

@@ -151,7 +151,7 @@ class JUnitFixtureMethods extends JUnitBaseSpec {
     then:
     def e = result.failures[exceptionPos].exception
     if (suppressed) {
-      e = e.suppressed[0]
+      !!(e = e.suppressed[0])
     }
     e instanceof RuntimeException
     e.message == name

spock-specs/src/test/groovy/org/spockframework/smoke/condition/ConditionEvaluation.groovy

@@ -56,6 +56,19 @@ class ConditionEvaluation extends EmbeddedSpecification {
         7
   }
 
+  @FailsWith(ConditionNotSatisfiedError)
+  def "failing non-top-level condition"() {
+    expect:
+    if (true) {
+      2 * 3 == 7
+    }
+  }
+
+  def "failing non-top-level non-condition"() {
+    expect:
+    [1, 2, 3].any { it == 2 }
+  }
+
   def "MethodCallExpression"() {
     expect:
     [1, 2, 3].size() == 3

spock-specs/src/test/groovy/org/spockframework/smoke/extension/TempDirExtensionSpec.groovy

@@ -88,12 +88,12 @@ class TempDirExtensionSpec extends EmbeddedSpecification {
       def tempFile = aaabbb.resolve("tmp.txt")
       Files.createDirectories(aaabbb)
       Files.write(tempFile, "ewfwf".bytes)
-      aaabbb.toFile().writable = false
-      aaa.toFile().writable = false
-      tempFile.toFile().writable = false
-      previousIteration = iterationDir
+      !!(aaabbb.toFile().writable = false)
+      !!(aaa.toFile().writable = false)
+      !!(tempFile.toFile().writable = false)
+      !!(previousIteration = iterationDir)
     } else if (i == 1) {
-      assert !previousIteration.exists()
+      !previousIteration.exists()
     }
 
     where: