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

Author: Vampire

docs/release_notes.adoc

@@ -6,6 +6,27 @@ include::include.adoc[]
 
 == 2.5 (tbd)
 
+=== 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[]
+
 === Enhancements
 
 * Improve `TooManyInvocationsError` now reports unsatisfied interactions with argument mismatch details, making it easier to diagnose why invocations didn't match expected interactions spockPull:2315[]

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

@@ -151,7 +151,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

@@ -201,7 +201,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

@@ -59,6 +59,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:

spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyAllMethodsAstSpec/transforms_conditions_in_private_methods.groovy

@@ -84,9 +84,15 @@ public void $spock_feature_0_0() {
     finally {
     }
     if (true) {
-        [true, false].any({ ->
-            it
-        })
+        try {
+            org.spockframework.runtime.SpockRuntime.verifyMethodCondition($spock_errorCollector, $spock_valueRecorder.reset(), '[true, false].any { it }', 5, 9, null, $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(2), [$spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(0), true), $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(1), false)]), $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(3), 'any'), new java.lang.Object[]{$spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(4), { ->
+                it
+            })}, $spock_valueRecorder.realizeNas(7, false), false, 6)
+        }
+        catch (java.lang.Throwable $spock_condition_throwable) {
+            org.spockframework.runtime.SpockRuntime.conditionFailedWithException($spock_errorCollector, $spock_valueRecorder, '[true, false].any { it }', 5, 9, null, $spock_condition_throwable)}
+        finally {
+        }
     }
     this.with({ ->
         org.spockframework.runtime.ValueRecorder $spock_valueRecorder1 = new org.spockframework.runtime.ValueRecorder()

spock-specs/src/test/resources/snapshots/org/spockframework/smoke/ast/condition/VerifyMethodsAstSpec/transforms_conditions_in_private_methods.groovy

@@ -80,9 +80,15 @@ public void $spock_feature_0_0() {
     finally {
     }
     if (true) {
-        [true, false].any({ ->
-            it
-        })
+        try {
+            org.spockframework.runtime.SpockRuntime.verifyMethodCondition($spock_errorCollector, $spock_valueRecorder.reset(), '[true, false].any { it }', 5, 9, null, $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(2), [$spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(0), true), $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(1), false)]), $spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(3), 'any'), new java.lang.Object[]{$spock_valueRecorder.record($spock_valueRecorder.startRecordingValue(4), { ->
+                it
+            })}, $spock_valueRecorder.realizeNas(7, false), false, 6)
+        }
+        catch (java.lang.Throwable $spock_condition_throwable) {
+            org.spockframework.runtime.SpockRuntime.conditionFailedWithException($spock_errorCollector, $spock_valueRecorder, '[true, false].any { it }', 5, 9, null, $spock_condition_throwable)}
+        finally {
+        }
     }
     this.with({ ->
         org.spockframework.runtime.ValueRecorder $spock_valueRecorder1 = new org.spockframework.runtime.ValueRecorder()