Skip to content

Commit

Permalink
Add support for static mocks on DoNotMockEnforcer
Browse files Browse the repository at this point in the history
Fixes mockito#3219
Fix mockStatic bypassing DoNotMockEnforcer
Add (optional) method on DoNotMockEnforcer for static mocks
  • Loading branch information
andrebrait committed Jan 2, 2024
1 parent 66792b6 commit 3b9bc54
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 17 deletions.
37 changes: 23 additions & 14 deletions src/main/java/org/mockito/internal/MockitoCore.java
Expand Up @@ -26,10 +26,10 @@
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.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;

import org.mockito.InOrder;
Expand Down Expand Up @@ -71,8 +71,9 @@
public class MockitoCore {

private static final DoNotMockEnforcer DO_NOT_MOCK_ENFORCER = Plugins.getDoNotMockEnforcer();
private static final Set<Class<?>> MOCKABLE_CLASSES =
Collections.synchronizedSet(new HashSet<>());
private static final Map<Boolean, Set<Class<?>>> MOCKABLE_CLASSES_CACHES = Map.of(
true, ConcurrentHashMap.newKeySet(),
false, ConcurrentHashMap.newKeySet());

public <T> T mock(Class<T> typeToMock, MockSettings settings) {
if (!(settings instanceof MockSettingsImpl)) {
Expand All @@ -84,41 +85,48 @@ public <T> T mock(Class<T> typeToMock, MockSettings settings) {
}
MockSettingsImpl impl = (MockSettingsImpl) settings;
MockCreationSettings<T> creationSettings = impl.build(typeToMock);
checkDoNotMockAnnotation(creationSettings.getTypeToMock(), creationSettings);
checkDoNotMockAnnotation(creationSettings.getTypeToMock(), creationSettings, false);
T mock = createMock(creationSettings);
mockingProgress().mockingStarted(mock, creationSettings);
return mock;
}

private void checkDoNotMockAnnotation(
Class<?> typeToMock, MockCreationSettings<?> creationSettings) {
checkDoNotMockAnnotationForType(typeToMock);
Class<?> typeToMock, MockCreationSettings<?> creationSettings, boolean isStatic) {
checkDoNotMockAnnotationForType(typeToMock, isStatic);
for (Class<?> aClass : creationSettings.getExtraInterfaces()) {
checkDoNotMockAnnotationForType(aClass);
checkDoNotMockAnnotationForType(aClass, isStatic);
}
}

private static void checkDoNotMockAnnotationForType(Class<?> type) {
private static void checkDoNotMockAnnotationForType(Class<?> type, boolean isStatic) {
// Object and interfaces do not have a super class
if (type == null) {
return;
}

if (MOCKABLE_CLASSES.contains(type)) {
final Set<Class<?>> mockableClassesCache = MOCKABLE_CLASSES_CACHES.get(isStatic);

if (mockableClassesCache.contains(type)) {
return;
}

String warning = DO_NOT_MOCK_ENFORCER.checkTypeForDoNotMockViolation(type);
final String warning;
if (isStatic) {
warning = DO_NOT_MOCK_ENFORCER.checkTypeForDoNotMockStaticViolation(type);
} else {
warning = DO_NOT_MOCK_ENFORCER.checkTypeForDoNotMockViolation(type);
}
if (warning != null) {
throw new DoNotMockException(warning);
}

checkDoNotMockAnnotationForType(type.getSuperclass());
checkDoNotMockAnnotationForType(type.getSuperclass(), isStatic);
for (Class<?> aClass : type.getInterfaces()) {
checkDoNotMockAnnotationForType(aClass);
checkDoNotMockAnnotationForType(aClass, isStatic);
}

MOCKABLE_CLASSES.add(type);
mockableClassesCache.add(type);
}

public <T> MockedStatic<T> mockStatic(Class<T> classToMock, MockSettings settings) {
Expand All @@ -131,6 +139,7 @@ public <T> MockedStatic<T> mockStatic(Class<T> classToMock, MockSettings setting
}
MockSettingsImpl impl = MockSettingsImpl.class.cast(settings);
MockCreationSettings<T> creationSettings = impl.buildStatic(classToMock);
checkDoNotMockAnnotation(creationSettings.getTypeToMock(), creationSettings, true);
MockMaker.StaticMockControl<T> control = createStaticMock(classToMock, creationSettings);
control.enable();
mockingProgress().mockingStarted(classToMock, creationSettings);
Expand Down
24 changes: 21 additions & 3 deletions src/main/java/org/mockito/plugins/DoNotMockEnforcer.java
Expand Up @@ -10,14 +10,32 @@
public interface DoNotMockEnforcer {

/**
* If this type is allowed to be mocked. Return an empty optional if the enforcer allows
* If this type is allowed to be mocked. Return {@code null} if the enforcer allows
* this type to be mocked. Return a message if there is a reason this type can not be mocked.
*
* <p>
* 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
* @return Optional message if this type can not be mocked, or {@code null} otherwise
*
* @see #checkTypeForDoNotMockStaticViolation(Class)
*/
String checkTypeForDoNotMockViolation(Class<?> type);

/**
* If this type is allowed to be <em>statically</em> mocked. Return {@code null} if the enforcer
* allows this type to be mocked. Return a message if there is a reason this type can not be mocked.
* <p>
* 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 <em>statically</em> mocked, or {@code null} otherwise
*
* @see #checkTypeForDoNotMockViolation(Class)
*/
default String checkTypeForDoNotMockStaticViolation(Class<?> type) {
return checkTypeForDoNotMockViolation(type);
}
}
74 changes: 74 additions & 0 deletions src/test/java/org/mockitousage/annotation/DoNotMockTest.java
Expand Up @@ -6,10 +6,12 @@

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;

import org.junit.Test;
import org.mockito.DoNotMock;
import org.mockito.Mock;
import org.mockito.MockedStatic;
import org.mockito.MockitoAnnotations;
import org.mockito.exceptions.misusing.DoNotMockException;

Expand All @@ -24,6 +26,16 @@ public void can_not_mock_class_annotated_with_donotmock() {
.isInstanceOf(DoNotMockException.class);
}

@Test
public void can_not_statically_mock_class_annotated_with_donotmock() {
assertThatThrownBy(
() -> {
MockedStatic<NotMockable> notMockable =
mockStatic(NotMockable.class);
})
.isInstanceOf(DoNotMockException.class);
}

@Test
public void can_not_mock_class_via_mock_annotation() {
assertThatThrownBy(
Expand All @@ -43,6 +55,16 @@ public void can_not_mock_class_with_interface_annotated_with_donotmock() {
.isInstanceOf(DoNotMockException.class);
}

@Test
public void can_not_statically_mock_class_with_interface_annotated_with_donotmock() {
assertThatThrownBy(
() -> {
MockedStatic<SubclassOfNotMockableInterface> notMockable =
mockStatic(SubclassOfNotMockableInterface.class);
})
.isInstanceOf(DoNotMockException.class);
}

@Test
public void can_not_mock_subclass_with_unmockable_interface() {
assertThatThrownBy(
Expand All @@ -53,6 +75,16 @@ public void can_not_mock_subclass_with_unmockable_interface() {
.isInstanceOf(DoNotMockException.class);
}

@Test
public void can_not_statically_mock_subclass_with_unmockable_interface() {
assertThatThrownBy(
() -> {
MockedStatic<DoubleSubclassOfInterface> notMockable =
mockStatic(DoubleSubclassOfInterface.class);
})
.isInstanceOf(DoNotMockException.class);
}

@Test
public void can_not_mock_subclass_with_unmockable_superclass() {
assertThatThrownBy(
Expand All @@ -63,6 +95,16 @@ public void can_not_mock_subclass_with_unmockable_superclass() {
.isInstanceOf(DoNotMockException.class);
}

@Test
public void can_not_statically_mock_subclass_with_unmockable_superclass() {
assertThatThrownBy(
() -> {
MockedStatic<SubclassOfNotMockableSuperclass> notMockable =
mockStatic(SubclassOfNotMockableSuperclass.class);
})
.isInstanceOf(DoNotMockException.class);
}

@Test
public void
can_not_mock_subclass_with_unmockable_interface_that_extends_non_mockable_interface() {
Expand All @@ -74,6 +116,17 @@ public void can_not_mock_subclass_with_unmockable_superclass() {
.isInstanceOf(DoNotMockException.class);
}

@Test
public void
can_not_statically_mock_subclass_with_unmockable_interface_that_extends_non_mockable_interface() {
assertThatThrownBy(
() -> {
MockedStatic<SubclassOfSubInterfaceOfNotMockableInterface> notMockable =
mockStatic(SubclassOfSubInterfaceOfNotMockableInterface.class);
})
.isInstanceOf(DoNotMockException.class);
}

@Test
public void thrown_exception_includes_non_mockable_reason() {
assertThatThrownBy(
Expand All @@ -94,6 +147,17 @@ public void thrown_exception_includes_special_non_mockable_reason() {
.hasMessageContaining("Special reason");
}

@Test
public void thrown_exception_includes_special_non_mockable_reason_static() {
assertThatThrownBy(
() -> {
MockedStatic<NotMockableWithReason> notMockable =
mockStatic(NotMockableWithReason.class);
})
.isInstanceOf(DoNotMockException.class)
.hasMessageContaining("Special reason");
}

@Test
public void can_not_mock_class_with_custom_donotmock_annotation() {
assertThatThrownBy(
Expand All @@ -104,6 +168,16 @@ public void can_not_mock_class_with_custom_donotmock_annotation() {
.isInstanceOf(DoNotMockException.class);
}

@Test
public void can_not_statically_mock_class_with_custom_donotmock_annotation() {
assertThatThrownBy(
() -> {
MockedStatic<NotMockableWithDifferentAnnotation> notMockable =
mockStatic(NotMockableWithDifferentAnnotation.class);
})
.isInstanceOf(DoNotMockException.class);
}

@DoNotMock
private static class NotMockable {}

Expand Down
Expand Up @@ -6,9 +6,12 @@

import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.mockStatic;

import org.junit.Test;
import org.mockito.DoNotMock;
import org.mockito.MockedStatic;
import org.mockito.exceptions.base.MockitoException;
import org.mockito.exceptions.misusing.DoNotMockException;

public class DoNotmockEnforcerTest {
Expand All @@ -27,6 +30,41 @@ public void uses_custom_enforcer_allows_special_cased_class() {
NotMockableButSpecialCased notMockable = mock(NotMockableButSpecialCased.class);
}

@Test
public void uses_custom_enforcer_disallows_static_mocks_of_type_with_specific_name() {
assertThatThrownBy(
() -> {
MockedStatic<StaticallyNotMockable> notMockable = mockStatic(StaticallyNotMockable.class);
})
.isInstanceOf(DoNotMockException.class)
.hasMessage("Cannot mockStatic!");
}

@Test
public void uses_custom_enforcer_disallows_static_mocks_of_type_that_inherits_from_non_statically_mockable() {
assertThatThrownBy(
() -> {
MockedStatic<StaticallyNotMockableChild> notMockable = mockStatic(StaticallyNotMockableChild.class);
})
.isInstanceOf(DoNotMockException.class)
.hasMessage("Cannot mockStatic!");
}

@Test
public void uses_custom_enforcer_allows_static_mocks_of_type_with_specific_name() {
/*
Current MockMaker does not support static mocks, so asserting we get its exception rather than
a DoNotMockException is enough to assert the DoNotMockEnforcer let it through.
*/
assertThatThrownBy(
() -> {
MockedStatic<StaticallyMockable> notMockable = mockStatic(StaticallyMockable.class);
})
.isInstanceOf(MockitoException.class)
.isNotInstanceOf(DoNotMockException.class)
.hasMessageContaining("does not support the creation of static mocks");
}

@Test
public void uses_custom_enforcer_has_custom_message() {
assertThatThrownBy(
Expand All @@ -41,4 +79,10 @@ static class NotMockable {}

@DoNotMock
static class NotMockableButSpecialCased {}

static class StaticallyNotMockable {}

static class StaticallyNotMockableChild extends StaticallyNotMockable {}

static class StaticallyMockable {}
}
Expand Up @@ -19,4 +19,15 @@ public String checkTypeForDoNotMockViolation(Class<?> type) {
}
return null;
}

@Override
public String checkTypeForDoNotMockStaticViolation(Class<?> type) {
if (type.getName().endsWith("StaticallyMockable")) {
return null;
}
if (type.getName().startsWith("org.mockitousage.plugins.donotmockenforcer")) {
return "Cannot mockStatic!";
}
return null;
}
}

0 comments on commit 3b9bc54

Please sign in to comment.