Support defining conditions in @Verify/@VerifyAll helper methods (#2112)

## Description

The PR adds two new annotations: `@spock.lang.Verify` and
`@spock.lang.VerifyAll` that can be applied to helper methods containing
condition blocks. Top-level implicit or explicit assertion statements in
these methods will be rewritten using the `ConditionRewriter`. `@Verify`
and `@VerifyAll` helpers can be defined in any class, not only in the
ones extending from `Specification`. The method is required to have a
`void` return type.

---------

Signed-off-by: Pavlo Shevchenko <pavel.shevchenko.95@gmail.com>
Co-authored-by: Leonard Brünings <lbruenings@gradle.com>

Author: pshevche

docs/release_notes.adoc

@@ -5,12 +5,23 @@ include::include.adoc[]
 
 == 2.4-M6 (tbd)
 
+=== Highlights
+
+* Add support for defining condition blocks with implicit assertions in helper methods annotated with `@Verify` or `@VerifyAll` spockPull:2112[]
+
+=== Breaking Changes
+
+* _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[]
+
 === Misc
 
 * Fix ExtensionException in OSGi environment for global extension spockIssue:2076[]
 ** This issue was introduced with spockPull:1995[]
 * Fix `@RestoreSystemProperties` not restoring state between iterations of a data-driven feature spockIssue:2104[]
 * Fix `VerifyError: Stack map does not match the one at exception handler` introduced in 2.4-M5 spockIssue:2080[]
+* Throw `SpockMultipleFailuresError` instead of a generic `MultipleFailuresError` in case of multiple failed assertions spockPull:2112[]
 
 == 2.4-M5 (2025-01-07)
 

docs/spock_primer.adoc

@@ -576,30 +576,19 @@ false                         ...
 
 Not very helpful. Fortunately, we can do better:
 
-[source,groovy]
+[source,groovy,indent=0]
 ----
-void matchesPreferredConfiguration(pc) {
-  assert pc.vendor == "Sunny"
-  assert pc.clockRate >= 2333
-  assert pc.ram >= 4096
-  assert pc.os == "Linux"
-}
+include::{sourcedir}/primer/VerifyMethodsDocSpec.groovy[tag=verify-helper-method]
 ----
 
-When factoring out conditions into a helper method, two points need to be considered: First, implicit conditions must
-be turned into explicit conditions with the `assert` keyword. Second, the helper method must have return type `void`.
+When factoring out conditions into a helper method, you need to ensure the method has return type `void`.
 Otherwise, Spock might interpret the return value as a failing condition, which is not what we want.
 
 As expected, the improved helper method tells us exactly what's wrong:
 
-[source,groovy]
+[source,groovy,indent=0]
 ----
-Condition not satisfied:
-
-assert pc.clockRate >= 2333
-       |  |         |
-       |  1666      false
-       ...
+include::{sourcedir}/primer/VerifyMethodsDocSpec.groovy[tag=verify-helper-method-result]
 ----
 
 A final advice: Although code reuse is generally a good thing, don't take it too far. Be aware that the use of fixture
@@ -651,7 +640,7 @@ with(service) {
 Sometimes an IDE has trouble to determine the type of the target, in that case you can help out by manually specifying the
 target type via `with(target, type, closure)`.
 
-== Using `verifyAll` to assert multiple expectations together
+== Using `verifyAll` or `@VerifyAll` helper methods to assert multiple expectations together
 
 Normal expectations fail the test on the first failed assertions. Sometimes it is helpful to collect these failures before
 failing the test to have more information, this behavior is also known as soft assertions.
@@ -676,7 +665,6 @@ def "offered PC matches preferred configuration"() {
 
 or it can be used without a target.
 
-
 [source,groovy]
 ----
   expect:
@@ -688,6 +676,13 @@ or it can be used without a target.
 
 Like `with` you can also optionally define a type hint for the IDE.
 
+Alternatively, `verifyAll` conditions can be extracted into a helper method annotated with `@VerifyAll`
+
+[source,groovy,indent=0]
+----
+include::{sourcedir}/primer/VerifyMethodsDocSpec.groovy[tag=verify-all-helper-method]
+----
+
 == Using `verifyEach` to assert on each element of an `Iterable`
 
 There are several ways to do assertions on `Iterable` or `Collection`.

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

@@ -80,13 +80,21 @@ private ConditionRewriter(IRewriteResources resources, String valueRecorderSuffi
     errorCollectorName = SpockNames.ERROR_COLLECTOR + errorCollectorSuffix;
   }
 
+  public static Statement rewriteExplicitCondition(AssertStatement stat, IRewriteResources resources) {
+    return rewriteExplicitCondition(stat, resources, "", "");
+  }
+
   public static Statement rewriteExplicitCondition(AssertStatement stat, IRewriteResources resources,
                                                    String valueRecorderSuffix, String errorCollectorSuffix) {
     ConditionRewriter rewriter = new ConditionRewriter(resources, valueRecorderSuffix, errorCollectorSuffix);
     Expression message = AstUtil.getAssertionMessage(stat);
     return rewriter.rewriteCondition(stat, stat.getBooleanExpression().getExpression(), message, true);
   }
 
+  public static Statement rewriteImplicitCondition(ExpressionStatement stat, IRewriteResources resources) {
+    return rewriteImplicitCondition(stat, resources, "", "");
+  }
+
   public static Statement rewriteImplicitCondition(ExpressionStatement stat, IRewriteResources resources,
                                                    String valueRecorderSuffix, String errorCollectorSuffix) {
     ConditionRewriter rewriter = new ConditionRewriter(resources, valueRecorderSuffix, errorCollectorSuffix);

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

@@ -28,6 +28,8 @@
 import java.util.List;
 
 import static org.codehaus.groovy.ast.expr.MethodCallExpression.NO_ARGUMENTS;
+import static org.spockframework.compiler.condition.ImplicitConditionsUtils.checkIsValidImplicitCondition;
+import static org.spockframework.compiler.condition.ImplicitConditionsUtils.isImplicitCondition;
 
 /**
  * Walks the statement and expression tree to:
@@ -41,12 +43,12 @@
  * @author Peter Niederwieser
  */
 public class DeepBlockRewriter extends AbstractDeepBlockRewriter {
-  private final IRewriteResources resources;
+  private final ISpecRewriteResources resources;
   private boolean insideInteraction = false;
   private int interactionClosureDepth = 0;
   private int closureDepth = 0;
 
-  public DeepBlockRewriter(IRewriteResources resources) {
+  public DeepBlockRewriter(ISpecRewriteResources resources) {
     super(resources.getCurrentBlock(), resources.getAstNodeCache());
     this.resources = resources;
   }
@@ -200,7 +202,7 @@ private boolean handleImplicitCondition(ExpressionStatement stat) {
     }
     if (!isImplicitCondition(stat)) return false;
 
-    checkIsValidImplicitCondition(stat);
+    checkIsValidImplicitCondition(stat, resources.getErrorReporter());
 
     String methodName = AstUtil.getMethodName(stat.getExpression());
     boolean isConditionMethodCall = Identifiers.CONDITION_METHODS.contains(methodName);
@@ -304,10 +306,10 @@ private boolean handleInteractionBlockCall(MethodCallExpression expr) {
 
   private void defineRecorders(ClosureExpression expr) {
     if (groupConditionFound) {
-      resources.defineErrorCollector(AstUtil.getStatements(expr), getErrorCollectorSuffix());
+      resources.getErrorRecorders().defineErrorCollector(AstUtil.getStatements(expr), getErrorCollectorSuffix());
     }
     if (conditionFound) {
-      resources.defineValueRecorder(AstUtil.getStatements(expr), getValueRecorderSuffix());
+      resources.getErrorRecorders().defineValueRecorder(AstUtil.getStatements(expr), getValueRecorderSuffix());
     }
   }
 
@@ -350,19 +352,4 @@ private boolean isInteractionExpression(InteractionRewriter rewriter, Expression
       return false;
     }
   }
-
-  // assumption: not already an interaction
-  public static boolean isImplicitCondition(Statement stat) {
-    return stat instanceof ExpressionStatement
-        && !(((ExpressionStatement) stat).getExpression() instanceof DeclarationExpression);
-  }
-
-  private void checkIsValidImplicitCondition(Statement stat) {
-    BinaryExpression binExpr = AstUtil.getExpression(stat, BinaryExpression.class);
-    if (binExpr == null) return;
-
-    if (Types.ofType(binExpr.getOperation().getType(), Types.ASSIGNMENT_OPERATOR)) {
-      resources.getErrorReporter().error(stat, "Expected a condition, but found an assignment. Did you intend to write '==' ?");
-    }
-  }
 }

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

@@ -16,34 +16,21 @@
 
 package org.spockframework.compiler;
 
-import java.util.List;
-
 import org.codehaus.groovy.ast.ASTNode;
-import org.codehaus.groovy.ast.expr.Expression;
-import org.codehaus.groovy.ast.expr.MethodCallExpression;
-import org.codehaus.groovy.ast.expr.VariableExpression;
-import org.codehaus.groovy.ast.stmt.Statement;
+import org.spockframework.compiler.condition.IConditionErrorRecorders;
 
-import org.spockframework.compiler.model.Block;
-import org.spockframework.compiler.model.Method;
-import org.spockframework.compiler.model.Spec;
+import java.util.List;
 
 /**
- *
  * @author Peter Niederwieser
  */
 public interface IRewriteResources {
-  Spec getCurrentSpec();
-  Method getCurrentMethod();
-  Block getCurrentBlock();
-
-  void defineValueRecorder(List<Statement> stats, String variableNameSuffix);
-  void defineErrorRethrower(List<Statement> stats);
-  void defineErrorCollector(List<Statement> stats, String variableNameSuffix);
-  VariableExpression captureOldValue(Expression oldValue);
-  MethodCallExpression getMockInvocationMatcher();
-
   AstNodeCache getAstNodeCache();
+
   String getSourceText(ASTNode node);
+
   ErrorReporter getErrorReporter();
+
+  IConditionErrorRecorders getErrorRecorders();
+
 }

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

@@ -0,0 +1,36 @@
+/*
+ * Copyright 2025 the original author or authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.spockframework.compiler;
+
+import org.codehaus.groovy.ast.expr.Expression;
+import org.codehaus.groovy.ast.expr.MethodCallExpression;
+import org.codehaus.groovy.ast.expr.VariableExpression;
+import org.spockframework.compiler.model.Block;
+import org.spockframework.compiler.model.Method;
+import org.spockframework.compiler.model.Spec;
+
+public interface ISpecRewriteResources extends IRewriteResources {
+  Spec getCurrentSpec();
+
+  Method getCurrentMethod();
+
+  Block getCurrentBlock();
+
+  VariableExpression captureOldValue(Expression oldValue);
+
+  MethodCallExpression getMockInvocationMatcher();
+}

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

@@ -36,7 +36,7 @@
  * @author Peter Niederwieser
  */
 public class InteractionRewriter {
-  private final IRewriteResources resources;
+  private final ISpecRewriteResources resources;
   private final ClosureExpression activeWithOrMockClosure;
 
   // information about the interaction; filled in by parse()
@@ -52,7 +52,7 @@ public class InteractionRewriter {
   // "new InteractionBuilder(..).setCount(..).setTarget(..).setMethod(..).addArg(..).addResult(..).build()"
   private Expression builderExpr;
 
-  public InteractionRewriter(IRewriteResources resources, @Nullable ClosureExpression activeWithOrMockClosure) {
+  public InteractionRewriter(ISpecRewriteResources resources, @Nullable ClosureExpression activeWithOrMockClosure) {
     this.resources = resources;
     this.activeWithOrMockClosure = activeWithOrMockClosure;
   }

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

@@ -20,7 +20,7 @@
 import org.codehaus.groovy.control.Janitor;
 import org.codehaus.groovy.control.SourceUnit;
 
-public class SourceLookup {
+public class SourceLookup implements AutoCloseable {
   private final SourceUnit sourceUnit;
   private final Janitor janitor = new Janitor();
 
@@ -48,6 +48,7 @@ public String lookup(ASTNode node) {
     return text.toString().trim();
   }
 
+  @Override
   public void close() {
     janitor.cleanup();
   }