Fix SpyStatic() with an interaction closure throws NullPointerException (#2254)

The usage of 
```groovy
SpyStatic(Type){
}
```` 
had thrown an `NullPointerException` , because the closure got
automatically converted into IMockMakerSettings.

This adds a meaningful error message for the user to use
`SpyStatic(Type)` without a `Closure` instead of `SpyStatic(Type){}`.

---------

Co-authored-by: Leonard Brünings <lbruenings@gradle.com>

Author: AndreasTu

docs/release_notes.adoc

@@ -113,6 +113,7 @@ spockPull:2112[]
 * Fix filter blocks with shared fields and derived data variables spockPull:2088[]
 * Fix combined labels with comments being ignored spockPull:2121[]
 * Fix boxed Boolean `is` getter methods not properly mocked in Groovy <= 3 spockIssue:2131[]
+* Fix `SpyStatic()` with an interaction closure throws NullPointerException spockPull:2254[]
 
 Thanks to all the contributors to this release: Andreas Turban, Björn Kautler, Christoph Loy, Marcin Zajączkowski, Pavlo Shevchenko
 

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

@@ -16,18 +16,21 @@
 
 package org.spockframework.mock.runtime;
 
+import groovy.lang.Closure;
 import org.spockframework.mock.CannotCreateMockException;
 import org.spockframework.mock.IMockObject;
 import org.spockframework.mock.ISpockMockObject;
 import org.spockframework.mock.runtime.IMockMaker.IStaticMock;
 import org.spockframework.mock.runtime.IMockMaker.MockMakerCapability;
+import org.spockframework.runtime.GroovyRuntimeUtil;
 import org.spockframework.util.InternalSpockError;
 import org.spockframework.util.Nullable;
 import org.spockframework.util.ThreadSafe;
 import spock.mock.IMockMakerSettings;
 import spock.mock.MockMakerId;
 
 import java.lang.reflect.Modifier;
+import java.lang.reflect.Proxy;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -77,7 +80,7 @@ public static MockMakerRegistry createFromServiceLoader(MockMakerConfiguration c
     MockMakerId preferredMockMakerId;
     if (preferredMockMakerParam != null) {
       preferredMockMakerId = preferredMockMakerParam.getMockMakerId();
-      if (makerMap.get(preferredMockMakerParam.getMockMakerId()) == null) {
+      if (makerMap.get(preferredMockMakerId) == null) {
         throw new IllegalStateException("No IMockMaker with ID '" + preferredMockMakerId + "' exists, but was request via mockMaker.preferredMockMaker configuration. Is a runtime dependency missing?");
       }
     } else {
@@ -142,9 +145,10 @@ public IStaticMock makeStaticMock(IMockCreationSettings settings) throws CannotC
   public <T> T makeMockInternal(IMockCreationSettings settings, BiFunction<IMockMaker, IMockCreationSettings, T> code) throws CannotCreateMockException {
     IMockMakerSettings mockMakerSettings = settings.getMockMakerSettings();
     if (mockMakerSettings != null) {
-      MockMakerId mockMakerId = mockMakerSettings.getMockMakerId();
+      MockMakerId mockMakerId = getMockMakerId(settings, mockMakerSettings);
       IMockMaker mockMaker = makerMap.get(mockMakerId);
       if (mockMaker == null) {
+        checkForStaticMockUsageWithClosure(settings, mockMakerSettings, mockMakerId, null);
         throw new CannotCreateMockException(settings.getMockType(), " because MockMaker with ID '" + mockMakerId + "' does not exist.");
       }
       verifyIsMockable(mockMaker, settings);
@@ -153,6 +157,25 @@ public <T> T makeMockInternal(IMockCreationSettings settings, BiFunction<IMockMa
     return createWithAppropriateMockMaker(settings, code);
   }
 
+  private static MockMakerId getMockMakerId(IMockCreationSettings settings, IMockMakerSettings mockMakerSettings) {
+    try {
+      return mockMakerSettings.getMockMakerId();
+    } catch (ClassCastException ex) {
+      checkForStaticMockUsageWithClosure(settings, mockMakerSettings, null, ex);
+      throw ex;
+    }
+  }
+
+  private static void checkForStaticMockUsageWithClosure(IMockCreationSettings settings, IMockMakerSettings mockMakerSettings, @Nullable MockMakerId mockMakerId, @Nullable Throwable cause) {
+    if (settings.isStaticMock() && (mockMakerSettings instanceof Proxy || mockMakerSettings instanceof Closure)) {
+      String nature = settings.getMockNature().toString();
+      throw new CannotCreateMockException(settings.getMockType(), " because the MockMakerSettings returned the invalid ID '" + mockMakerId + "'."
+        + "\nThe syntax " + nature + "Static(" + settings.getMockType().getSimpleName() + "){} is not supported, please use " + nature + "Static(" + settings.getMockType().getSimpleName() + ") without a Closure instead."
+        , cause
+      );
+    }
+  }
+
   /**
    * Returns information about a mock object, or {@code null} if the object is no mock.
    *

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

@@ -16,6 +16,7 @@
  * <p>If you provide a method, you can define your settings in a typesafe manner for the user,
  * e.g. with a {@link Closure} parameter to configure the mock.
  *
+ * @author Andreas Turban
  * @since 2.4
  */
 @Beta
@@ -41,5 +42,8 @@ public String toString() {
     };
   }
 
+  /**
+   * @return the {@link MockMakerId} to use, must not be {@code null}.
+   */
   MockMakerId getMockMakerId();
 }

spock-specs/src/test/groovy/org/spockframework/mock/runtime/mockito/MockitoStaticMocksSpec.groovy

@@ -22,9 +22,12 @@ import org.mockito.exceptions.base.MockitoException
 import org.spockframework.mock.CannotCreateMockException
 import org.spockframework.mock.MockUtil
 import org.spockframework.runtime.InvalidSpecException
+import org.spockframework.runtime.SpecInternals
+import org.spockframework.runtime.model.SpecInfo
 import spock.lang.Issue
 import spock.lang.Shared
 import spock.lang.Specification
+import spock.mock.IMockMakerSettings
 import spock.mock.MockMakers
 
 import java.util.concurrent.Callable
@@ -491,6 +494,43 @@ class MockitoStaticMocksSpec extends Specification {
     !StaticClass.staticVarargsMethod("test2")
   }
 
+  def "SpyStatic with closure as IMockMakerSettings shall produce nice error message"() {
+    when:
+    SpyStatic(StaticClass) {
+
+    }
+
+    then:
+    def ex = thrown(CannotCreateMockException)
+    ex.message.startsWith(
+      """Cannot create mock for class $StaticClass.name because the MockMakerSettings returned the invalid ID 'null'.
+The syntax SpyStatic(StaticClass){} is not supported, please use SpyStatic(StaticClass) without a Closure instead.""")
+  }
+
+  def "SpyStatic with closure returning something shall produce nice error message"() {
+    when:
+    SpyStatic(StaticClass) {
+      "Dummy"
+    }
+
+    then:
+    def ex = thrown(CannotCreateMockException)
+    ex.message.startsWith(
+      """Cannot create mock for class $StaticClass.name because the MockMakerSettings returned the invalid ID 'null'.
+The syntax SpyStatic(StaticClass){} is not supported, please use SpyStatic(StaticClass) without a Closure instead.""")
+    ex.cause instanceof ClassCastException
+  }
+
+  def "ClassCastException inside the getMockMakerId() from a non-static Spy is thrown as-is and is not processed by the SpyStatic(){} closure check"() {
+    when:
+    Spy(Runnable, mockMaker: {
+      throw new ClassCastException()
+    } as IMockMakerSettings)
+
+    then:
+    thrown(ClassCastException)
+  }
+
   static class StaticClass {
 
     String instanceMethod() {