Add !! as opt-out operator for assertions... (#1532)

Author: leonard84

docs/spock_primer.adoc

@@ -242,6 +242,62 @@ def setup() {
 
 If an explicit condition is violated, it will produce the same nice diagnostic message as an implicit condition.
 
+====== Opt-out of condition handling
+
+Sometimes, you may want to use a top-level expression that is not treated as a condition.
+
+For example, you may want to loop over a list with `each` and do assertions on its items.
+
+[source,groovy,indent=0]
+----
+include::{sourcedir}/primer/ConditionsSpec.groovy[tag=each-assert-normal]
+----
+
+However, this will fail if the list is empty, as `each` returns the list again and an empty list is falsy according to Groovy-truth.
+
+To avoid this, you can use the `!!` operator to opt-out of condition handling:
+[source,groovy,indent=0]
+----
+include::{sourcedir}/primer/ConditionsSpec.groovy[tag=each-assert-opt-out-operator]
+----
+
+The `!!` operator can also be used to hide the contents of an expression in an explicit assertion statement.
+This can be helpful, if the rendered expression is too long to be readable, or if you want to hide the data itself.
+
+
+[source,groovy,indent=0]
+----
+include::{sourcedir}/primer/ConditionsSpec.groovy[tag=explicit-with-opt-out-operator-and-message]
+----
+
+will be rendered as
+
+----
+include::{sourcedir}/primer/ConditionsSpec.groovy[tag=explicit-with-opt-out-operator-and-message-result]
+----
+
+as opposed to
+
+----
+include::{sourcedir}/primer/ConditionsSpec.groovy[tag=normal-condition-result]
+----
+
+and you can even leave out the message, although it is not recommended, to get this:
+
+[source,groovy,indent=0]
+----
+include::{sourcedir}/primer/ConditionsSpec.groovy[tag=explicit-with-opt-out-operator]
+----
+
+will be rendered as
+
+----
+include::{sourcedir}/primer/ConditionsSpec.groovy[tag=explicit-with-opt-out-operator-result]
+----
+
+IMPORTANT: The `!!` must be the outermost expression, see https://groovy-lang.org/operators.html#_operator_precedence[Groovy's docs on Operator Precedence].
+           If in doubt use parentheses, `!!(expression)`.
+
 ===== Exception Conditions
 
 Exception conditions are used to describe that a `when` block should throw an exception. They are defined using the

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

@@ -596,6 +596,13 @@ private Statement rewriteCondition(Statement conditionStat, Expression condition
   }
 
   private Statement rewriteCondition(Expression expr, Expression message, boolean explicit) {
+    boolean optOut = isOptOutExpression(expr);
+    if (optOut) {
+      expr = removeOptOutPrefix(expr);
+      if (!explicit) {
+        return new ExpressionStatement(expr);
+      }
+    }
     if (isSpecialCollectionCondition(expr)) { // todo check if we can better integrate it
       expr = transformSpecialCollectionCondition(expr);
     }
@@ -606,21 +613,37 @@ private Statement rewriteCondition(Expression expr, Expression message, boolean
       if ((Identifiers.WITH.equals(methodName) || Identifiers.VERIFY_ALL.equals(methodName))) {
         return surroundSpecialTryCatch(expr);
       }
-      return rewriteMethodCondition(methodCallExpression, message, explicit);
+      return rewriteMethodCondition(methodCallExpression, message, explicit, optOut);
     }
 
     if (expr instanceof StaticMethodCallExpression)
-      return rewriteStaticMethodCondition((StaticMethodCallExpression) expr, message, explicit);
+      return rewriteStaticMethodCondition((StaticMethodCallExpression) expr, message, explicit, optOut);
+
+    return rewriteOtherCondition(expr, message, optOut);
+  }
+
+  /**
+   * Treat the nonsensical `!!` prefix as opt-out from implicit conditions
+   */
+  private boolean isOptOutExpression(Expression expr) {
+    return expr instanceof NotExpression && ((NotExpression) expr).getExpression() instanceof NotExpression;
+  }
 
-    return rewriteOtherCondition(expr, message);
+  private Expression removeOptOutPrefix(Expression expr) {
+    return ((NotExpression)((NotExpression)expr).getExpression()).getExpression();
   }
 
-  private Statement rewriteMethodCondition(MethodCallExpression condition, Expression message, boolean explicit) {
+  private Statement rewriteMethodCondition(MethodCallExpression condition, Expression message, boolean explicit, boolean optOut) {
     MethodCallExpression rewritten;
     int lastVariableNum;
-    final Expression converted = convert(condition);
-    rewritten = (MethodCallExpression) unrecord(converted);
-    lastVariableNum = extractVariableNumber(converted);
+    if (explicit && optOut) {
+      rewritten = condition;
+      lastVariableNum = -1;
+    } else {
+      final Expression converted = convert(condition);
+      rewritten = (MethodCallExpression) unrecord(converted);
+      lastVariableNum = extractVariableNumber(converted);
+    }
 
     List<Expression> args = new ArrayList<>();
     args.add(rewritten.getObjectExpression());
@@ -638,16 +661,21 @@ private Statement rewriteMethodCondition(MethodCallExpression condition, Express
             resources.getAstNodeCache().SpockRuntime_VerifyMethodCondition,
             condition,
             message,
-            args));
+            args, optOut), optOut);
   }
 
   private Statement rewriteStaticMethodCondition(StaticMethodCallExpression condition, Expression message,
-      boolean explicit) {
+                                                 boolean explicit, boolean optOut) {
     StaticMethodCallExpression rewritten;
     int lastVariableNum;
-    final Expression converted = convert(condition);
-    rewritten = (StaticMethodCallExpression) unrecord(converted);
-    lastVariableNum = extractVariableNumber(converted);
+    if (explicit && optOut) {
+      rewritten = condition;
+      lastVariableNum = -1;
+    } else {
+      final Expression converted = convert(condition);
+      rewritten = (StaticMethodCallExpression) unrecord(converted);
+      lastVariableNum = extractVariableNumber(converted);
+    }
 
     List<Expression> args = new ArrayList<>();
     args.add(new ClassExpression(rewritten.getOwnerType()));
@@ -665,16 +693,18 @@ private Statement rewriteStaticMethodCondition(StaticMethodCallExpression condit
             resources.getAstNodeCache().SpockRuntime_VerifyMethodCondition,
             condition,
             message,
-            args));
+            args,
+            optOut),
+        optOut);
   }
 
-  private Statement rewriteOtherCondition(Expression condition, Expression message) {
-    Expression rewritten = convert(condition);
+  private Statement rewriteOtherCondition(Expression condition, Expression message, boolean optOut) {
+    Expression rewritten = optOut ? condition : convert(condition);
 
     final Expression executeAndVerify = rewriteToSpockRuntimeCall(resources.getAstNodeCache().SpockRuntime_VerifyCondition,
-        condition, message, singletonList(rewritten));
+        condition, message, singletonList(rewritten), optOut);
 
-    return surroundWithTryCatch(condition, message, executeAndVerify);
+    return surroundWithTryCatch(condition, message, executeAndVerify, optOut);
   }
 
   private Expression transformSpecialCollectionCondition(Expression condition) {
@@ -725,7 +755,7 @@ private boolean isStringLikeExpression(Expression expression) {
     return false;
   }
 
-  private TryCatchStatement surroundWithTryCatch(Expression condition, Expression message, Expression executeAndVerify) {
+  private TryCatchStatement surroundWithTryCatch(Expression condition, Expression message, Expression executeAndVerify, boolean optOut) {
     final TryCatchStatement tryCatchStatement = new TryCatchStatement(
         new ExpressionStatement(executeAndVerify),
         EmptyStatement.INSTANCE
@@ -740,7 +770,7 @@ private TryCatchStatement surroundWithTryCatch(Expression condition, Expression
                     resources.getAstNodeCache().SpockRuntime_ConditionFailedWithException,
                     new ArgumentListExpression(asList(
                         new VariableExpression(errorCollectorName),
-                      new VariableExpression(valueRecorderName),                      // recorder
+                        optOut ? ConstantExpression.NULL : new VariableExpression(valueRecorderName),  // recorder
                         new ConstantExpression(resources.getSourceText(condition)),   // text
                         new ConstantExpression(condition.getLineNumber()),            // line
                         new ConstantExpression(condition.getColumnNumber()),          // column
@@ -779,15 +809,15 @@ private TryCatchStatement surroundSpecialTryCatch(Expression executeAndVerify) {
   }
 
   private Expression rewriteToSpockRuntimeCall(MethodNode method, Expression condition, Expression message,
-      List<Expression> additionalArgs) {
+      List<Expression> additionalArgs, boolean optOut) {
     List<Expression> args = new ArrayList<>();
 
     MethodCallExpression result = createDirectMethodCall(
         new ClassExpression(resources.getAstNodeCache().SpockRuntime), method,
         new ArgumentListExpression(args));
 
     args.add(new VariableExpression(errorCollectorName, resources.getAstNodeCache().ErrorCollector));
-    args.add(createDirectMethodCall(
+    args.add(createDirectMethodCall(optOut ? ConstantExpression.NULL :
             new VariableExpression(valueRecorderName),
             resources.getAstNodeCache().ValueRecorder_Reset,
             ArgumentListExpression.EMPTY_ARGUMENTS));

spock-specs/src/test/groovy/org/spockframework/docs/primer/ConditionsSpec.groovy

@@ -0,0 +1,82 @@
+package org.spockframework.docs.primer
+
+import org.spockframework.runtime.ConditionNotSatisfiedError
+import org.spockframework.smoke.condition.ConditionRenderingSpec
+import spock.lang.FailsWith
+
+class ConditionsSpec extends ConditionRenderingSpec {
+
+  def "normal condition rendering"() {
+    expect:
+    isRendered(/* tag::normal-condition-result[] */"""
+foo == bar
+|   |  |
+|   |  very long bar
+|   false
+|   3 differences (76% similarity)
+|   very long (foo)
+|   very long (bar)
+very long foo
+
+foobar
+""", // end::normal-condition-result[]
+      {
+        // tag::normal-condition[]
+        def foo = 'very long foo'
+        def bar = 'very long bar'
+        assert foo == bar : "foobar"
+        // end::normal-condition[]
+      }
+    )
+  }
+
+  def "condition rendering can be disabled via opt-out operator"() {
+    expect:
+    isRendered(/* tag::explicit-with-opt-out-operator-and-message-result[] */"""
+(foo == bar)
+
+foobar
+""", // end::explicit-with-opt-out-operator-and-message-result[]
+      {
+        // tag::explicit-with-opt-out-operator-and-message[]
+        def foo = 'very long foo'
+        def bar = 'very long bar'
+        assert !!(foo == bar) : "foobar"
+        // end::explicit-with-opt-out-operator-and-message[]
+      }
+    )
+  }
+
+  def "condition rendering can be disabled via opt-out operator without message"() {
+    expect:
+    isRendered(/* tag::explicit-with-opt-out-operator-result[] */"""
+(foo == bar)
+""",// end::explicit-with-opt-out-operator-result[]
+      {
+        def foo = 'very long foo'
+        def bar = 'very long bar'
+        // tag::explicit-with-opt-out-operator[]
+        assert !!(foo == bar)
+        // end::explicit-with-opt-out-operator[]
+    })
+  }
+
+  @FailsWith(ConditionNotSatisfiedError)
+  def "using each-assert fails normally"(){
+    given:
+    def aList = []
+    // tag::each-assert-normal[]
+    expect:
+    aList.each { assert it > 0 }
+    // end::each-assert-normal[]
+  }
+
+  def "using each-assert with opt-out passes"(){
+    given:
+    def aList = []
+    // tag::each-assert-opt-out-operator[]
+    expect:
+    !!aList.each { assert it > 0 }
+    // end::each-assert-opt-out-operator[]
+  }
+}

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

@@ -16,14 +16,13 @@
 
 package org.spockframework.smoke.condition
 
+import org.opentest4j.AssertionFailedError
 import org.spockframework.EmbeddedSpecification
+import org.spockframework.runtime.ConditionFailedWithExceptionError
 import org.spockframework.runtime.ConditionNotSatisfiedError
 
 import spock.lang.Issue
 import spock.lang.FailsWith
-import spock.lang.Specification
-
-import java.util.concurrent.Callable
 
 import static java.lang.Math.max
 import static java.lang.Math.min
@@ -293,7 +292,6 @@ class ConditionEvaluation extends EmbeddedSpecification {
   def "NotExpression"() {
     expect:
     !false
-    !!true
     !(true && false)
   }
 
@@ -391,6 +389,45 @@ class ConditionEvaluation extends EmbeddedSpecification {
     [aType, bType] << ['int[]', 'Integer[]', 'List', 'Queue', 'Deque'].with { [it, it] }.combinations()
   }
 
+  @FailsWith(ConditionFailedWithExceptionError)
+  def "regular implicit condition"() {
+    expect:
+    customContains("foo", "bar")
+  }
+
+  @FailsWith(AssertionFailedError)
+  def "opt-out of implicit-condition handling does not wrap exceptions"() {
+    expect:
+    !!customContains("foo", "bar")
+  }
+
+  def "opt-out of implicit-condition handling won't fail for falsy values"() {
+    expect:
+    !!aList.each { assert it > 0 }
+
+    where:
+    aList << [[1], []]
+  }
+
+  def "opt-out of implicit-condition handling won't fail static method conditions returning falsy value"() {
+    expect:
+    !!min(0, 0)
+  }
+
+  @FailsWith(ConditionNotSatisfiedError)
+  def "opt-out only works if it is the outermost expression"() {
+    expect:
+    !!false && false
+  }
+
+
+  def "opt-out only works for the outermost expression"() {
+    expect:
+    !!(true && false)
+    !!true
+    !!false
+  }
+
   /*
   def "MapEntryExpression"() {
       // tested as part of testMapExpression
@@ -413,6 +450,12 @@ class ConditionEvaluation extends EmbeddedSpecification {
   }
   */
 
+  void customContains(String a, String b) {
+    if(!a.contains(b)) {
+      throw new AssertionFailedError("'$a' does not contain '$b'", a, b)
+    }
+  }
+
   static class Properties {
     def getNext() { this }