Best-effort error reporting for interactions on final methods

Add error reporting code to the byte-buddy` mock maker to report interaction on final methods, which can't be handled. The IMockInteractionValidation interface allows IMockMakers to implement different kinds of validations on mock interactions. Fixes #2039
spockframework · Nov 7, 2024 · 2b54442 · 2b54442
1 parent 8dd331d
commit 2b54442
Show file tree

Hide file tree

Showing 24 changed files with 442 additions and 54 deletions.
diff --git a/docs/release_notes.adoc b/docs/release_notes.adoc
@@ -26,6 +26,7 @@ include::include.adoc[]
 * Size of data providers is no longer calculated multiple times but only once
 * Fix exception when using `@RepeatUntilFailure` with a data provider with unknown iteration amount. spockPull:2031[]
 * Clarified documentation about data providers and `size()` calls spockIssue:2022[]
+* Add best-effort error reporting for interactions on final methods when using the `byte-buddy` mock maker spockIssue:2039[]
 
 == 2.4-M4 (2024-03-21)
 

diff --git a/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java b/spock-core/src/main/java/org/spockframework/compiler/AstUtil.java
@@ -40,7 +40,6 @@
  */
 public abstract class AstUtil {
   private static final Pattern DATA_TABLE_SEPARATOR = Pattern.compile("_{2,}+");
-  private static final String GET_METHOD_NAME = "get";
   private static final String GET_AT_METHOD_NAME = new IntegerArrayGetAtMetaMethod().getName();
 
   /**

diff --git a/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java b/spock-core/src/main/java/org/spockframework/compiler/SpecRewriter.java
@@ -33,6 +33,9 @@
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
 import static org.spockframework.compiler.AstUtil.createDirectMethodCall;
+import static org.spockframework.runtime.GroovyRuntimeUtil.GET;
+import static org.spockframework.runtime.GroovyRuntimeUtil.propertyToMethodName;
+import static org.spockframework.runtime.GroovyRuntimeUtil.SET;
 
 /**
  * A Spec visitor responsible for most of the rewriting of a Spec's AST.
@@ -135,7 +138,7 @@ private void createSharedFieldGetter(Field field) {
   }
 
   private void createFinalFieldGetter(Field field) {
-    String getterName = "get" + MetaClassHelper.capitalize(field.getName());
+    String getterName = propertyToMethodName(GET, field.getName());
     MethodNode getter = spec.getAst().getMethod(getterName, Parameter.EMPTY_ARRAY);
     if (getter != null) {
       errorReporter.error(field.getAst(),
@@ -158,7 +161,7 @@ private void createFinalFieldGetter(Field field) {
   }
 
   private void createSharedFieldSetter(Field field) {
-    String setterName = "set" + MetaClassHelper.capitalize(field.getName());
+    String setterName = propertyToMethodName(SET, field.getName());
     Parameter[] params = new Parameter[] { new Parameter(field.getAst().getType(), "$spock_value") };
     MethodNode setter = spec.getAst().getMethod(setterName, params);
     if (setter != null) {

diff --git a/spock-core/src/main/java/org/spockframework/mock/IMockObject.java b/spock-core/src/main/java/org/spockframework/mock/IMockObject.java
@@ -15,6 +15,7 @@
 package org.spockframework.mock;
 
 import org.spockframework.mock.runtime.SpecificationAttachable;
+import org.spockframework.util.Beta;
 import org.spockframework.util.Nullable;
 import spock.lang.Specification;
 
@@ -81,4 +82,12 @@ public interface IMockObject extends SpecificationAttachable {
    * @return whether this mock object matches the target of the specified interaction
    */
   boolean matches(Object target, IMockInteraction interaction);
+
+  /**
+   * Returns the used mock configuration which created this mock.
+   *
+   * @return the mock configuration
+   */
+  @Beta
+  IMockConfiguration getConfiguration();
 }
diff --git a/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java b/spock-core/src/main/java/org/spockframework/mock/ISpockMockObject.java
@@ -14,10 +14,28 @@
 
 package org.spockframework.mock;
 
+import org.spockframework.mock.runtime.IMockInteractionValidation;
+import org.spockframework.util.Beta;
+import org.spockframework.util.Nullable;
+
 /**
- * Marker-like interface implemented by all mock objects that allows
- * {@link MockUtil} to detect mock objects. Not intended for direct use.
+ * MockObject interface implemented by some {@link spock.mock.MockMakers} that allows the {@link org.spockframework.mock.runtime.MockMakerRegistry}
+ * to detect mock objects.
+ *
+ * <p>Not intended for direct use.
  */
 public interface ISpockMockObject {
+
   IMockObject $spock_get();
+
+  /**
+   * @return the {@link IMockInteractionValidation} used to verify {@link IMockInteraction}
+   * @see IMockInteractionValidation
+   * @since 2.4
+   */
+  @Nullable
+  @Beta
+  default IMockInteractionValidation $spock_mockInteractionValidation() {
+    return null;
+  }
 }
diff --git a/spock-core/src/main/java/org/spockframework/mock/MockUtil.java b/spock-core/src/main/java/org/spockframework/mock/MockUtil.java
@@ -35,7 +35,7 @@ public class MockUtil {
    * @return whether the given object is a (Spock) mock object
    */
   public boolean isMock(Object object) {
-    return getMockMakerRegistry().asMockOrNull(object) != null;
+    return asMockOrNull(object) != null;
   }
 
   /**
@@ -69,6 +69,17 @@ public IMockObject asMock(Object object) {
     return mockOrNull;
   }
 
+  /**
+   * Returns information about a mock object or {@code null}, if the object is not a mock.
+   *
+   * @param object a mock object
+   * @return information about the mock object or {@code null}, if the object is not a mock.
+   */
+  @Nullable
+  public IMockObject asMockOrNull(Object object) {
+    return getMockMakerRegistry().asMockOrNull(object);
+  }
+
   /**
    * Attaches mock to a Specification.
    *

diff --git a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualMethodNameConstraint.java
@@ -31,6 +31,10 @@ public EqualMethodNameConstraint(String methodName) {
     this.methodName = methodName;
   }
 
+  public String getMethodName() {
+    return methodName;
+  }
+
   @Override
   public boolean isSatisfiedBy(IMockInvocation invocation) {
     return invocation.getMethod().getName().equals(methodName);

diff --git a/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java b/spock-core/src/main/java/org/spockframework/mock/constraint/EqualPropertyNameConstraint.java
@@ -29,6 +29,10 @@ public EqualPropertyNameConstraint(String propertyName) {
     this.propertyName = propertyName;
   }
 
+  public String getPropertyName() {
+    return propertyName;
+  }
+
   @Override
   public boolean isSatisfiedBy(IMockInvocation invocation) {
     return propertyName.equals(getPropertyName(invocation));

diff --git a/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java b/spock-core/src/main/java/org/spockframework/mock/constraint/TargetConstraint.java
@@ -17,6 +17,7 @@
 package org.spockframework.mock.constraint;
 
 import org.spockframework.mock.*;
+import org.spockframework.mock.runtime.IMockInteractionValidation;
 import org.spockframework.runtime.Condition;
 import org.spockframework.runtime.InvalidSpecException;
 import org.spockframework.util.CollectionUtil;
@@ -50,13 +51,29 @@ public String describeMismatch(IMockInvocation invocation) {
   @Override
   public void setInteraction(IMockInteraction interaction) {
     this.interaction = interaction;
-    if (interaction.isRequired() && MOCK_UTIL.isMock(target)) {
-      IMockObject mockObject = MOCK_UTIL.asMock(target);
+    IMockObject mockObject = MOCK_UTIL.asMockOrNull(target);
+    if (mockObject != null) {
+      checkRequiredInteraction(mockObject, interaction);
+      validateMockInteraction(mockObject, interaction);
+    }
+  }
+
+  private void checkRequiredInteraction(IMockObject mockObject, IMockInteraction interaction) {
+    if (interaction.isRequired()) {
       if (!mockObject.isVerified()) {
         String mockName = mockObject.getName() != null ? mockObject.getName() : "unnamed";
         throw new InvalidSpecException("Stub '%s' matches the following required interaction:" +
           "\n\n%s\n\nRemove the cardinality (e.g. '1 *'), or turn the stub into a mock.\n").withArgs(mockName, interaction);
       }
     }
   }
+
+  private void validateMockInteraction(IMockObject mockObject, IMockInteraction interaction) {
+    if (target instanceof ISpockMockObject) {
+      IMockInteractionValidation validation = ((ISpockMockObject) target).$spock_mockInteractionValidation();
+      if (validation != null) {
+        validation.validateMockInteraction(mockObject, interaction);
+      }
+    }
+  }
 }
diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java b/spock-core/src/main/java/org/spockframework/mock/runtime/BaseMockInterceptor.java
@@ -1,5 +1,7 @@
 package org.spockframework.mock.runtime;
 
+import org.spockframework.mock.IMockObject;
+import org.spockframework.mock.ISpockMockObject;
 import org.spockframework.runtime.GroovyRuntimeUtil;
 
 import java.lang.reflect.Method;
@@ -8,8 +10,13 @@
 
 import groovy.lang.*;
 import org.jetbrains.annotations.Nullable;
+import org.spockframework.util.ReflectionUtil;
+
+import static java.util.Objects.requireNonNull;
 
 public abstract class BaseMockInterceptor implements IProxyBasedMockInterceptor {
+  static final Method MOCK_INTERACTION_VALIDATION_METHOD = requireNonNull(ReflectionUtil.getMethodByName(ISpockMockObject.class, "$spock_mockInteractionValidation"));
+
   private MetaClass mockMetaClass;
 
   BaseMockInterceptor(MetaClass mockMetaClass) {
@@ -39,11 +46,11 @@ protected String handleGetProperty(GroovyObject target, Object[] args) {
       MetaClass metaClass = target.getMetaClass();
       //First try the isXXX before getXXX, because this is the expected behavior, if it is boolean property.
       MetaMethod booleanVariant = metaClass
-        .getMetaMethod(GroovyRuntimeUtil.propertyToMethodName("is", propertyName), GroovyRuntimeUtil.EMPTY_ARGUMENTS);
+        .getMetaMethod(GroovyRuntimeUtil.propertyToMethodName(GroovyRuntimeUtil.IS, propertyName), GroovyRuntimeUtil.EMPTY_ARGUMENTS);
       if (booleanVariant != null && booleanVariant.getReturnType() == boolean.class) {
         methodName = booleanVariant.getName();
       } else {
-        methodName = GroovyRuntimeUtil.propertyToMethodName("get", propertyName);
+        methodName = GroovyRuntimeUtil.propertyToMethodName(GroovyRuntimeUtil.GET, propertyName);
       }
     }
     return methodName;
@@ -52,4 +59,11 @@ protected String handleGetProperty(GroovyObject target, Object[] args) {
   protected boolean isMethod(Method method, String name, Class<?>... parameterTypes) {
     return method.getName().equals(name) && Arrays.equals(method.getParameterTypes(), parameterTypes);
   }
+
+  public static Object handleSpockMockInterface(Method method, IMockObject mockObject) {
+    if (method.getName().equals(MOCK_INTERACTION_VALIDATION_METHOD.getName())) {
+      return null; //This should be handled in the corresponding MockMakers.
+    }
+    return mockObject;
+  }
 }
diff --git a/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java b/spock-core/src/main/java/org/spockframework/mock/runtime/ByteBuddyMockFactory.java
@@ -22,6 +22,7 @@
 import net.bytebuddy.TypeCache;
 import net.bytebuddy.description.annotation.AnnotationDescription;
 import net.bytebuddy.description.method.MethodDescription;
+import net.bytebuddy.description.modifier.MethodManifestation;
 import net.bytebuddy.description.modifier.SynchronizationState;
 import net.bytebuddy.description.modifier.SyntheticState;
 import net.bytebuddy.description.modifier.Visibility;
@@ -31,6 +32,7 @@
 import net.bytebuddy.dynamic.loading.MultipleParentClassLoader;
 import net.bytebuddy.dynamic.scaffold.TypeValidation;
 import net.bytebuddy.implementation.FieldAccessor;
+import net.bytebuddy.implementation.FixedValue;
 import net.bytebuddy.implementation.MethodDelegation;
 import net.bytebuddy.implementation.bind.annotation.Morph;
 import org.codehaus.groovy.runtime.callsite.AbstractCallSite;
@@ -45,6 +47,7 @@
 import java.util.concurrent.ThreadLocalRandom;
 
 import static net.bytebuddy.matcher.ElementMatchers.none;
+import static org.spockframework.mock.runtime.BaseMockInterceptor.MOCK_INTERACTION_VALIDATION_METHOD;
 
 class ByteBuddyMockFactory {
   /**
@@ -136,6 +139,10 @@ Object createMock(IMockMaker.IMockCreationSettings settings) {
           .method(m -> !isGroovyMOPMethod(type, m))
           .intercept(mockInterceptor())
           .transform(mockTransformer())
+          .method(m -> m.represents(MOCK_INTERACTION_VALIDATION_METHOD))
+          // Implement the $spock_mockInteractionValidation() method which returns the static field below, so we have a validation instance for every mock class
+          .intercept(FixedValue.reference(new ByteBuddyMockInteractionValidation(), MOCK_INTERACTION_VALIDATION_METHOD.getName()))
+          .transform(validateMockInteractionTransformer())
           .implement(ByteBuddyInterceptorAdapter.InterceptorAccess.class)
           .intercept(FieldAccessor.ofField("$spock_interceptor"))
           .defineField("$spock_interceptor", IProxyBasedMockInterceptor.class, Visibility.PRIVATE, SyntheticState.SYNTHETIC)
@@ -149,6 +156,10 @@ Object createMock(IMockMaker.IMockCreationSettings settings) {
     return proxy;
   }
 
+  private static Transformer<MethodDescription> validateMockInteractionTransformer() {
+    return Transformer.ForMethod.withModifiers(SynchronizationState.PLAIN, Visibility.PUBLIC, MethodManifestation.FINAL);
+  }
+
   private static Transformer<MethodDescription> mockTransformer() {
     return Transformer.ForMethod.withModifiers(SynchronizationState.PLAIN, Visibility.PUBLIC); //Overridden methods should be public and non-synchronized.
   }