Some improvements for the work from PR #1753 (#1809)

I had a half-finished review of PR #1753, then got a bad cold.
Now you merged it already, so I moved my review comments into this PR
here.

Most changes are just minor.

One notable change is, that I changed the order of cglib and mockito.
Even though cglib is deprecated, I think it should sill be selected
before mockito for consistency.
If you currently have cglib on the classpath but no byte buddy, it will
be used,
then with the new version suddenly mockito will be used which could
change the behavior.
It swapped the order, so that mockito is below, so that it is just used
as fallback for cases that couldn't be handled before.
What do you think?

Another notable change is, that I changed the toString of the mock maker
settings implementations that just returned the
mock maker id before and now return "mockito mock maker settings" or
"<ID> simple mock maker settings".
What do you think?

Author: Vampire

build.gradle

@@ -436,6 +436,7 @@ def configureJavadoc(TaskProvider javadoc, boolean root = false) {
       options.addStringOption('source', '1.8')
       links "https://docs.groovy-lang.org/docs/groovy-$groovyVersion/html/gapi/"
       links "https://junit.org/junit4/javadoc/latest/"
+      links "https://javadoc.io/doc/org.mockito/mockito-core/${libs.versions.mockito5.get()}/"
       // Use offline package list, as the JavaVersion is fixed to 8 for the time being
       // and Hamcrest has certificate issues for their domain.
       linksOffline("https://docs.oracle.com/javase/8/docs/api/", "${root ? '' : '../'}javadoc/java-8")

docs/extensions.adoc

@@ -1060,24 +1060,24 @@ The following mock makers are built-in, and are selected in this order:
 * `java-proxy`: Uses the `java.lang.reflect.Proxy` API to create mocks of interfaces.
 * `byte-buddy`: Uses https://bytebuddy.net/[Byte Buddy] to create mock objects.
 ** Requires `net.bytebuddy:byte-buddy` 1.9+ on the class path.
-* `mockito`: Uses https://site.mockito.org/[Mockito] to create mocks of classes.
-** Can be configured to use additional Mockito feature like mock `Serializable`
-** Requires `org.mockito:mockito-core` 4.11+ on the class path.
 * `cglib`: Deprecated: Uses https://github.com/cglib/cglib[CGLIB] to create mock objects.
 ** Requires `cglib:cglib-nodep` 3.2.0+ on the class path.
+* `mockito`: Uses https://site.mockito.org/[Mockito] to create mock objects.
+** Can be configured to use additional Mockito feature like mock `Serializable`
+** Requires `org.mockito:mockito-core` 4.11+ on the class path.
 
 .Capabilities of the different built-in mock makers
-[cols=".^~h,^.^15,^.^15,^.^15,^.^18"]
+[cols=".^~h,^.^15,^.^15,^.^15,^.^15"]
 |===
 ^|          Capability           | `java-proxy` | `byte-buddy` | `cglib` | `mockito`
 
-| Interface                      |      ✔       |      ✔       |    ✔   |    ✔
-| Class                          |      ✘       |      ✔       |    ✔   |    ✔
-| Additional Interfaces          |      ✔       |      ✔       |    ✔   |    ✔
-| Explicit Constructor Arguments |      ✘       |      ✔       |    ✔   |    ✔
-| Final Class                    |      ✘       |      ✘       |    ✘   |    ✔
-| Final Method                   |      ✘       |      ✘       |    ✘   |    ✔
-| Static Method                  |      ✘       |      ✘       |    ✘   |    ✘
+| Interface                      |      ✔       |      ✔       |    ✔    |     ✔
+| Class                          |      ✘       |      ✔       |    ✔    |     ✔
+| Additional Interfaces          |      ✔       |      ✔       |    ✔    |     ✔
+| Explicit Constructor Arguments |      ✘       |      ✔       |    ✔    |     ✔
+| Final Class                    |      ✘       |      ✘       |    ✘    |     ✔
+| Final Method                   |      ✘       |      ✘       |    ✘    |     ✔
+| Static Method                  |      ✘       |      ✘       |    ✘    |     ✘
 |===
 
 The class `spock.mock.MockMakers` provides constants and methods for the built-in mock makers.
@@ -1097,27 +1097,27 @@ include::{sourcedir}/extension/MockMakerConfigurationDocSpec.groovy[tag=mock-mak
 The `mockito` Mock Maker provides the ability to mock final types, enums and final methods.
 The mocking of final classes is automatically enabled, if `org.mockito:mockito-core` 4.11+ is on the class path.
 
-For mocking of final methods, you need to select `mockito` during mock construction,
+For mocking of final methods, you need to select the `mockito` mock maker during mock construction,
 like:
 [source,groovy,indent=0]
 ----
 include::{sourcedir}/interaction/MockMakerDocSpec.groovy[tag=mockito]
 ----
 
-If you want to make final method mockable by default, you can select this mock maker as the preferred mock maker.
-It can't mock `native` methods, see Mockito for details.
+If you want to make final methods mockable by default, you can select this mock maker as the preferred mock maker.
+It can't mock native methods, see https://javadoc.io/doc/org.mockito/mockito-core/5.6.0/org/mockito/Mockito.html#Mocking_Final[the Mockito documentation] for details.
 
-CAUTION: If you try to mock a final method without a Mock Maker supporting it.
-         It will silently fail, without honoring your specified interactions.
+CAUTION: If you try to mock a final method without a Mock Maker supporting it,
+         it will silently fail, without honoring your specified interactions.
 
-You can configure the created mocks with the `org.mockito.MockSettings` during construction to use features provided by Mockito:
+You can configure the created mock objects using the interface `org.mockito.MockSettings` during the construction to use features provided by Mockito:
 
 [source,groovy,indent=0]
 ----
 include::{sourcedir}/interaction/MockMakerDocSpec.groovy[tag=mock-serializable]
 ----
 
-The `mockito` uses the `org.mockito.MockMakers.INLINE` under the hood,
+The `mockito` mock maker uses `org.mockito.MockMakers.INLINE` under the hood,
 please see the Mockito manual "Mocking final types, enums and final methods" for all pros and cons,
 when using `org.mockito.MockMakers.INLINE`.
 

gradle/libs.versions.toml

@@ -4,6 +4,8 @@ groovy3 = '3.0.19'
 groovy4 = '4.0.15'
 jacoco = '0.8.11'
 asm = '9.6'
+mockito4 = '4.11.0'
+mockito5 = '5.6.0'
 
 [libraries]
 jetbrains-annotations = "org.jetbrains:annotations:24.0.1"
@@ -17,8 +19,8 @@ hamcrest = "org.hamcrest:hamcrest:2.2"
 jaxb = "javax.xml.bind:jaxb-api:2.3.1"
 junit4 = "junit:junit:4.13.2"
 log4j = "log4j:log4j:1.2.17"
-mockito4 = "org.mockito:mockito-core:4.11.0"
-mockito5 = "org.mockito:mockito-core:5.6.0"
+mockito4 = { module = "org.mockito:mockito-core", version.ref = "mockito4" }
+mockito5 = { module = "org.mockito:mockito-core", version.ref = "mockito5" }
 objenesis = "org.objenesis:objenesis:3.3"
 # This needs a classifier, but is has to be specified on the usage end https://melix.github.io/blog/2021/03/version-catalogs-faq.html#_why_can_t_i_use_excludes_or_classifiers
 jacoco-agent = { module = "org.jacoco:org.jacoco.agent", version.ref = "jacoco" }

spock-core/src/main/java/org/spockframework/mock/runtime/CglibMockMaker.java

@@ -48,7 +48,7 @@ public Set<MockMakerCapability> getCapabilities() {
 
   @Override
   public int getPriority() {
-    return 400;
+    return 300;
   }
 
   @Override

spock-core/src/main/java/org/spockframework/mock/runtime/IMockMaker.java

@@ -237,7 +237,7 @@ public MockMakerId getMockMakerId() {
 
         @Override
         public String toString() {
-          return id.toString();
+          return id + " simple mock maker settings";
         }
       };
     }

spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMaker.java

@@ -63,7 +63,7 @@ public Set<MockMakerCapability> getCapabilities() {
 
   @Override
   public int getPriority() {
-    return 300;
+    return 400;
   }
 
   @Override

spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerImpl.java

@@ -32,9 +32,7 @@
 import org.spockframework.mock.MockNature;
 import org.spockframework.mock.runtime.IMockMaker;
 import org.spockframework.mock.runtime.IProxyBasedMockInterceptor;
-import org.spockframework.runtime.GroovyRuntimeUtil;
 import org.spockframework.util.ExceptionUtil;
-import org.spockframework.util.ObjectUtil;
 import org.spockframework.util.ReflectionUtil;
 import org.spockframework.util.ThreadSafe;
 
@@ -71,7 +69,7 @@ Object makeMock(IMockMaker.IMockCreationSettings settings) throws CannotCreateMo
         mockitoSettings.extraInterfaces(settings.getAdditionalInterface().toArray(CLASS_ARRAY));
       }
       if (settings.getConstructorArgs() != null) {
-        mockitoSettings.useConstructor(settings.getConstructorArgs().toArray(GroovyRuntimeUtil.EMPTY_ARGUMENTS));
+        mockitoSettings.useConstructor(settings.getConstructorArgs().toArray());
       } else if (settings.getMockNature() == MockNature.SPY) {
         //We need to say Mockito it shall use the constructor otherwise it will not initialize fields of the spy, see org.mockito.Mockito.spy(java.lang.Class<T>), which does the same
         mockitoSettings.useConstructor();
@@ -82,7 +80,7 @@ Object makeMock(IMockMaker.IMockCreationSettings settings) throws CannotCreateMo
 
       applyMockMakerSettingsFromUser(settings, mockitoSettings);
 
-      MockCreationSettings<Object> mockitoCreationSettings = ObjectUtil.uncheckedCast(mockitoSettings.build(settings.getMockType()));
+      MockCreationSettings<?> mockitoCreationSettings = mockitoSettings.build(settings.getMockType());
       SpockMockHandler handler = new SpockMockHandler(settings.getMockInterceptor());
 
       return inlineMockMaker.createMock(mockitoCreationSettings, handler);

spock-core/src/main/java/org/spockframework/mock/runtime/mockito/MockitoMockMakerSettings.java

@@ -19,10 +19,12 @@
 import groovy.lang.Closure;
 import org.mockito.MockSettings;
 import org.spockframework.mock.runtime.IMockMaker;
+import org.spockframework.runtime.GroovyRuntimeUtil;
 import spock.mock.MockMakers;
 
 import static java.util.Objects.requireNonNull;
 import static org.spockframework.mock.runtime.IMockMaker.MockMakerId;
+import static org.spockframework.util.ObjectUtil.uncheckedCast;
 
 public final class MockitoMockMakerSettings implements IMockMaker.IMockMakerSettings {
   private final Closure<?> mockitoCode;
@@ -40,20 +42,16 @@ public MockMakerId getMockMakerId() {
     return MockMakers.mockito.getMockMakerId();
   }
 
-  void applySettings(MockSettings mockSettings) {
-    requireNonNull(mockSettings);
-    callClosure(mockitoCode, mockSettings);
-  }
-
-  private static void callClosure(Closure<?> closure, Object delegate) {
-    Closure<?> settingsClosure = (Closure<?>) closure.clone();
-    settingsClosure.setResolveStrategy(Closure.DELEGATE_FIRST);
-    settingsClosure.setDelegate(delegate);
-    settingsClosure.call(delegate);
+  void applySettings(MockSettings mockitoSettings) {
+    requireNonNull(mockitoSettings);
+    Closure<?> mockitoCode = uncheckedCast(this.mockitoCode.clone());
+    mockitoCode.setResolveStrategy(Closure.DELEGATE_FIRST);
+    mockitoCode.setDelegate(mockitoSettings);
+    GroovyRuntimeUtil.invokeClosure(mockitoCode, mockitoSettings);
   }
 
   @Override
   public String toString() {
-    return getMockMakerId().toString();
+    return getMockMakerId() + " mock maker settings";
   }
 }

spock-core/src/main/java/spock/mock/MockMakers.java

@@ -19,7 +19,8 @@
 import groovy.lang.Closure;
 import groovy.lang.DelegatesTo;
 import groovy.transform.stc.ClosureParams;
-import groovy.transform.stc.FromString;
+import groovy.transform.stc.SimpleType;
+import org.mockito.MockSettings;
 import org.spockframework.mock.runtime.ByteBuddyMockMaker;
 import org.spockframework.mock.runtime.CglibMockMaker;
 import org.spockframework.mock.runtime.IMockMaker;
@@ -79,8 +80,10 @@ private MockMakers() {
    * </ul>
    */
   public static final IMockMaker.IMockMakerSettings javaProxy = simple(JavaProxyMockMaker.ID);
+
   /**
-   * Uses <a href="https://site.mockito.org/">mockito</a> to create mocks of final classes, enums and final methods.
+   * Uses <a href="https://site.mockito.org/">Mockito</a> to create mocks,
+   * which also supports final classes, enums and final methods.
    *
    * <p>The supported mocking features are:
    * <ul>
@@ -91,13 +94,14 @@ private MockMakers() {
    * <li>{@code FINAL_CLASS}</li>
    * <li>{@code FINAL_METHOD}</li>
    *
-   * <p>It uses the mockito {@code org.mockito.MockMakers.INLINE} under the hood,
-   * please see the mockito manual for all pros and cons, when using {@code MockMakers.INLINE}.
+   * <p>It uses {@link org.mockito.MockMakers#INLINE} under the hood,
+   * please see the Mockito manual for all pros and cons, when using {@code MockMakers.INLINE}.
    */
   public static final IMockMaker.IMockMakerSettings mockito = simple(MockitoMockMaker.ID);
 
   /**
-   * Uses <a href="https://site.mockito.org/">mockito</a> to create mocks of final classes, enums and final methods.
+   * Uses <a href="https://site.mockito.org/">Mockito</a> to create mocks,
+   * which also supports final classes, enums and final methods.
    *
    * <p>The supported mocking features are:
    * <ul>
@@ -108,13 +112,13 @@ private MockMakers() {
    * <li>{@code FINAL_CLASS}</li>
    * <li>{@code FINAL_METHOD}</li>
    *
-   * <p>It uses the mockito {@code org.mockito.MockMakers.INLINE} under the hood,
-   * please see the mockito manual for all pros and cons, when using {@code MockMakers.INLINE}.
+   * <p>It uses {@link org.mockito.MockMakers#INLINE} under the hood,
+   * please see the Mockito manual for all pros and cons, when using {@code MockMakers.INLINE}.
    *
-   * @param settingsCode the code to execute to configure {@code org.mockito.MockSettings} for further configuration of the mock to create
+   * @param settingsCode the code to execute to configure {@link MockSettings} for further configuration of the mock to create
    */
   public static IMockMaker.IMockMakerSettings mockito(@DelegatesTo(type = "org.mockito.MockSettings", strategy = Closure.DELEGATE_FIRST)
-                                                      @ClosureParams(value = FromString.class, options = "org.mockito.MockSettings")
+                                                      @ClosureParams(value = SimpleType.class, options = "org.mockito.MockSettings")
                                                             Closure<?> settingsCode) {
     return MockitoMockMakerSettings.createSettings(settingsCode);
   }

spock-specs/mock-integration/mock-integration.gradle

@@ -30,7 +30,7 @@ def codeGenerationLibraries = [
   mockito4 : configurations.mockito4
 ]
 
-if (rootProject.ext.javaVersion >= 11) {
+if (rootProject.javaVersion >= 11) {
   //Mockito 5 requires at least Java 11 to run
   codeGenerationLibraries.put("mockito5", configurations.mockito5)
 }
@@ -41,17 +41,17 @@ codeGenerationLibraries.each { key, config ->
     classpath += config
 
     if (key == "cglib") {
-      if (rootProject.ext.javaVersion >= 17) {
+      if (rootProject.javaVersion >= 17) {
         jvmArgs(
           //cglib requires access to java.lang.ClassLoader.defineClass() from net.sf.cglib.core.ReflectUtils
           "--add-opens=java.base/java.lang=ALL-UNNAMED"
         )
       }
-      onlyIf { rootProject.ext.javaVersion < 21}
+      onlyIf { rootProject.javaVersion < 21}
     }
     if (key == "mockito4") {
       // mockito4 only supports up to Java 20
-      onlyIf { rootProject.ext.javaVersion < 21}
+      onlyIf { rootProject.javaVersion < 21}
     }
   }
   tasks.register("test${key.capitalize()}WithObjenesis", Test) {
@@ -60,17 +60,17 @@ codeGenerationLibraries.each { key, config ->
     classpath += configurations.objenesis
 
     if (key == "cglib") {
-      if (rootProject.ext.javaVersion >= 17) {
+      if (rootProject.javaVersion >= 17) {
         jvmArgs(
           //cglib requires access to java.lang.ClassLoader.defineClass() from net.sf.cglib.core.ReflectUtils
           "--add-opens=java.base/java.lang=ALL-UNNAMED"
         )
       }
-      onlyIf { rootProject.ext.javaVersion < 21}
+      onlyIf { rootProject.javaVersion < 21}
     }
     if (key == "mockito4") {
       // mockito4 only supports up to Java 20
-      onlyIf { rootProject.ext.javaVersion < 21}
+      onlyIf { rootProject.javaVersion < 21}
     }
   }
 }