Add annotation to mark a type as DoNotMock (#1833)

Mocking types that users not own [1] or are severely complicating test
logic [2] leads to brittle or wrong tests. In particular, the
StackOverflow answer is wrong, as the contract of java.util.Map is
violated. When a new key is added to the Map, the stubbed return would be wrong.
In Google we have used the DoNotMock annotation via ErrorProne [3]
to annotate these types, as well as an internal list of types that can't
be mocked (this includes several java.util types). We are using a custom
Mockmaker to enforce this on run-time.

Based on our successful experience with DoNotMock (we have seen a large
reduction in bad/broken tests for types involved), we are proposing to
open source this into Mockito itself.

The DoNotMock annotation can be added to any type, e.g. classes and
interfaces. If, in the type hierarchy of the class-to-be-mocked, there
is a type that is annotated with DoNotMock, Mockito will throw a
DoNotMockException.

This would help preventing issues such as #1827 and #1734 which is
in-line with the guidance on our wiki [1]. A follow-up change would
allow us to define external types (like the java.util types) that can't
be mocked. (We can't add the annotation to the types, as they live in the
JDK instead.)

This PR also introduces the DoNotMockEnforcer interface which users can override
to implement their special handling of types annotated with DoNotMock.

[1]: https://github.com/mockito/mockito/wiki/How-to-write-good-tests#dont-mock-a-type-you-dont-own
[2]: https://stackoverflow.com/a/15820143
[3]: https://errorprone.info/api/latest/com/google/errorprone/annotations/DoNotMock.html

src/main/java/org/mockito/DoNotMock.java

@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2019 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockito;
+
+import static java.lang.annotation.ElementType.TYPE;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+/**
+ * Annotation representing a type that should not be mocked.
+ * <p>When marking a type {@code @DoNotMock}, you should always point to alternative testing
+ * solutions such as standard fakes or other testing utilities.
+ *
+ * Mockito enforces {@code @DoNotMock} with the {@link org.mockito.plugins.DoNotMockEnforcer}.
+ *
+ * If you want to use a custom {@code @DoNotMock} annotation, the {@link org.mockito.plugins.DoNotMockEnforcer}
+ * will match on annotations with a type ending in "org.mockito.DoNotMock". You can thus place
+ * your custom annotation in {@code com.my.package.org.mockito.DoNotMock} and Mockito will enforce
+ * that types annotated by {@code @com.my.package.org.mockito.DoNotMock} can not be mocked.
+ *
+ * <pre class="code"><code class="java">
+ * &#064;DoNotMock(reason = "Use a real instance instead")
+ * class DoNotMockMe {}
+ * </code></pre>
+ */
+@Target({TYPE})
+@Retention(RUNTIME)
+@Documented
+public @interface DoNotMock {
+    /**
+     * The reason why the annotated type should not be mocked.
+     *
+     * <p>This should suggest alternative APIs to use for testing objects of this type.
+     */
+    String reason() default "Create a real instance instead.";
+}

src/main/java/org/mockito/exceptions/misusing/DoNotMockException.java

@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2019 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockito.exceptions.misusing;
+
+import org.mockito.exceptions.base.MockitoException;
+
+/**
+ * Thrown when attempting to mock a class that is annotated with {@link org.mockito.DoNotMock}.
+ */
+public class DoNotMockException extends MockitoException {
+    public DoNotMockException(String message) {
+        super(message);
+    }
+}

src/main/java/org/mockito/internal/MockitoCore.java

@@ -27,13 +27,19 @@
 import static org.mockito.internal.verification.VerificationModeFactory.noMoreInteractions;
 
 import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
+
 import org.mockito.InOrder;
 import org.mockito.MockSettings;
 import org.mockito.MockedConstruction;
 import org.mockito.MockedStatic;
 import org.mockito.MockingDetails;
+import org.mockito.exceptions.misusing.DoNotMockException;
 import org.mockito.exceptions.misusing.NotAMockException;
+import org.mockito.internal.configuration.plugins.Plugins;
 import org.mockito.internal.creation.MockSettingsImpl;
 import org.mockito.internal.invocation.finder.VerifiableInvocationsFinder;
 import org.mockito.internal.listeners.VerificationStartedNotifier;
@@ -53,6 +59,7 @@
 import org.mockito.invocation.Invocation;
 import org.mockito.invocation.MockHandler;
 import org.mockito.mock.MockCreationSettings;
+import org.mockito.plugins.DoNotMockEnforcer;
 import org.mockito.plugins.MockMaker;
 import org.mockito.quality.Strictness;
 import org.mockito.stubbing.LenientStubber;
@@ -67,6 +74,10 @@
 @SuppressWarnings("unchecked")
 public class MockitoCore {
 
+    private static final DoNotMockEnforcer DO_NOT_MOCK_ENFORCER = Plugins.getDoNotMockEnforcer();
+    private static final Set<Class<?>> MOCKABLE_CLASSES =
+            Collections.synchronizedSet(new HashSet<>());
+
     public boolean isTypeMockable(Class<?> typeToMock) {
         return typeMockabilityOf(typeToMock).mockable();
     }
@@ -81,11 +92,43 @@ public <T> T mock(Class<T> typeToMock, MockSettings settings) {
         }
         MockSettingsImpl impl = (MockSettingsImpl) settings;
         MockCreationSettings<T> creationSettings = impl.build(typeToMock);
+        checkDoNotMockAnnotation(creationSettings.getTypeToMock(), creationSettings);
         T mock = createMock(creationSettings);
         mockingProgress().mockingStarted(mock, creationSettings);
         return mock;
     }
 
+    private void checkDoNotMockAnnotation(
+            Class<?> typeToMock, MockCreationSettings<?> creationSettings) {
+        checkDoNotMockAnnotationForType(typeToMock);
+        for (Class<?> aClass : creationSettings.getExtraInterfaces()) {
+            checkDoNotMockAnnotationForType(aClass);
+        }
+    }
+
+    private static void checkDoNotMockAnnotationForType(Class<?> type) {
+        // Object and interfaces do not have a super class
+        if (type == null) {
+            return;
+        }
+
+        if (MOCKABLE_CLASSES.contains(type)) {
+            return;
+        }
+
+        String warning = DO_NOT_MOCK_ENFORCER.checkTypeForDoNotMockViolation(type);
+        if (warning != null) {
+            throw new DoNotMockException(warning);
+        }
+
+        checkDoNotMockAnnotationForType(type.getSuperclass());
+        for (Class<?> aClass : type.getInterfaces()) {
+            checkDoNotMockAnnotationForType(aClass);
+        }
+
+        MOCKABLE_CLASSES.add(type);
+    }
+
     public <T> MockedStatic<T> mockStatic(Class<T> classToMock, MockSettings settings) {
         if (!MockSettingsImpl.class.isInstance(settings)) {
             throw new IllegalArgumentException(

src/main/java/org/mockito/internal/configuration/DefaultDoNotMockEnforcer.java

@@ -0,0 +1,30 @@
+/*
+ * Copyright (c) 2019 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockito.internal.configuration;
+
+import java.lang.annotation.Annotation;
+
+import org.mockito.DoNotMock;
+import org.mockito.plugins.DoNotMockEnforcer;
+
+public class DefaultDoNotMockEnforcer implements DoNotMockEnforcer {
+
+    @Override
+    public String checkTypeForDoNotMockViolation(Class<?> type) {
+        for (Annotation annotation : type.getAnnotations()) {
+            if (annotation.annotationType().getName().endsWith("org.mockito.DoNotMock")) {
+                String exceptionMessage =
+                        type + " is annotated with @org.mockito.DoNoMock and can't be mocked.";
+                if (DoNotMock.class.equals(annotation.annotationType())) {
+                    exceptionMessage += " " + type.getAnnotation(DoNotMock.class).reason();
+                }
+
+                return exceptionMessage;
+            }
+        }
+
+        return null;
+    }
+}

src/main/java/org/mockito/internal/configuration/plugins/DefaultMockitoPlugins.java

@@ -7,6 +7,7 @@
 import java.util.HashMap;
 import java.util.Map;
 import org.mockito.plugins.AnnotationEngine;
+import org.mockito.plugins.DoNotMockEnforcer;
 import org.mockito.plugins.InstantiatorProvider2;
 import org.mockito.plugins.MemberAccessor;
 import org.mockito.plugins.MockMaker;
@@ -47,6 +48,9 @@ class DefaultMockitoPlugins implements MockitoPlugins {
                 "org.mockito.internal.util.reflection.ReflectionMemberAccessor");
         DEFAULT_PLUGINS.put(
                 MODULE_ALIAS, "org.mockito.internal.util.reflection.ModuleMemberAccessor");
+        DEFAULT_PLUGINS.put(
+                DoNotMockEnforcer.class.getName(),
+                "org.mockito.internal.configuration.DefaultDoNotMockEnforcer");
     }
 
     @Override

src/main/java/org/mockito/internal/configuration/plugins/PluginRegistry.java

@@ -6,6 +6,7 @@
 
 import java.util.List;
 import org.mockito.plugins.AnnotationEngine;
+import org.mockito.plugins.DoNotMockEnforcer;
 import org.mockito.plugins.InstantiatorProvider2;
 import org.mockito.plugins.MemberAccessor;
 import org.mockito.plugins.MockMaker;
@@ -44,6 +45,9 @@ class PluginRegistry {
     private final List<MockResolver> mockResolvers =
             new PluginLoader(pluginSwitch).loadPlugins(MockResolver.class);
 
+    private final DoNotMockEnforcer doNotMockEnforcer =
+            new PluginLoader(pluginSwitch).loadPlugin(DoNotMockEnforcer.class);
+
     PluginRegistry() {
         instantiatorProvider =
                 new PluginLoader(pluginSwitch).loadPlugin(InstantiatorProvider2.class);
@@ -108,6 +112,16 @@ MockitoLogger getMockitoLogger() {
         return mockitoLogger;
     }
 
+    /**
+     * Returns the DoNotMock enforce for the current runtime.
+     *
+     * <p> Returns {@link org.mockito.internal.configuration.DefaultDoNotMockEnforcer} if no
+     * {@link DoNotMockEnforcer} extension exists or is visible in the current classpath.</p>
+     */
+    DoNotMockEnforcer getDoNotMockEnforcer() {
+        return doNotMockEnforcer;
+    }
+
     /**
      * Returns a list of available mock resolvers if any.
      *

src/main/java/org/mockito/internal/configuration/plugins/Plugins.java

@@ -4,8 +4,10 @@
  */
 package org.mockito.internal.configuration.plugins;
 
+import org.mockito.DoNotMock;
 import java.util.List;
 import org.mockito.plugins.AnnotationEngine;
+import org.mockito.plugins.DoNotMockEnforcer;
 import org.mockito.plugins.InstantiatorProvider2;
 import org.mockito.plugins.MemberAccessor;
 import org.mockito.plugins.MockMaker;
@@ -93,5 +95,15 @@ public static MockitoPlugins getPlugins() {
         return new DefaultMockitoPlugins();
     }
 
+    /**
+     * Returns the {@link DoNotMock} enforcer available for the current runtime.
+     *
+     * <p> Returns {@link org.mockito.internal.configuration.DefaultDoNotMockEnforcer} if no
+     * {@link DoNotMockEnforcer} extension exists or is visible in the current classpath.</p>
+     */
+    public static DoNotMockEnforcer getDoNotMockEnforcer() {
+        return registry.getDoNotMockEnforcer();
+    }
+
     private Plugins() {}
 }

src/main/java/org/mockito/plugins/DoNotMockEnforcer.java

@@ -0,0 +1,23 @@
+/*
+ * Copyright (c) 2019 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
+package org.mockito.plugins;
+
+/**
+ * Enforcer that is applied to every type in the type hierarchy of the class-to-be-mocked.
+ */
+public interface DoNotMockEnforcer {
+
+    /**
+     * If this type is allowed to be mocked. Return an empty optional if the enforcer allows
+     * this type to be mocked. Return a message if there is a reason this type can not be mocked.
+     *
+     * Note that Mockito performs traversal of the type hierarchy. Implementations of this class
+     * should therefore not perform type traversal themselves but rely on Mockito.
+     *
+     * @param type The type to check
+     * @return Optional message if this type can not be mocked, or an empty optional if type can be mocked
+     */
+    String checkTypeForDoNotMockViolation(Class<?> type);
+}