Fixes mockito#457 ClassCastExceptions can't occur any more cause the …

…parameter type is dynamically checked.
ChristianSchwarz · Jun 24, 2016 · 1f15555 · 1f15555
1 parent 3fe0fd7
commit 1f15555
Show file tree

Hide file tree

Showing 10 changed files with 161 additions and 34 deletions.
diff --git a/src/main/java/org/mockito/AdditionalMatchers.java b/src/main/java/org/mockito/AdditionalMatchers.java
@@ -51,7 +51,7 @@ public class AdditionalMatchers {
      *            the given value.
      * @return <code>null</code>.
      */
-    public static <T extends Comparable<T>> T geq(Comparable<T> value) {
+    public static <T extends Comparable<T>> T geq(T value) {
         reportMatcher(new GreaterOrEqual<T>(value));
         return null;
     }
@@ -149,7 +149,7 @@ public static short geq(short value) {
      *            the given value.
      * @return <code>null</code>.
      */
-    public static <T extends Comparable<T>> T leq(Comparable<T> value) {
+    public static <T extends Comparable<T>> T leq(T value) {
         reportMatcher(new LessOrEqual<T>(value));
         return null;
     }
@@ -247,7 +247,7 @@ public static short leq(short value) {
      *            the given value.
      * @return <code>null</code>.
      */
-    public static <T extends Comparable<T>> T gt(Comparable<T> value) {
+    public static <T extends Comparable<T>> T gt(T value) {
         reportMatcher(new GreaterThan<T>(value));
         return null;
     }
@@ -345,7 +345,7 @@ public static short gt(short value) {
      *            the given value.
      * @return <code>null</code>.
      */
-    public static <T extends Comparable<T>> T lt(Comparable<T> value) {
+    public static <T extends Comparable<T>> T lt(T value) {
         reportMatcher(new LessThan<T>(value));
         return null;
     }
@@ -444,7 +444,7 @@ public static short lt(short value) {
      *            the given value.
      * @return <code>null</code>.
      */
-    public static <T extends Comparable<T>> T cmpEq(Comparable<T> value) {
+    public static <T extends Comparable<T>> T cmpEq(T value) {
         reportMatcher(new CompareEqual<T>(value));
         return null;
     }

diff --git a/src/main/java/org/mockito/internal/invocation/ArgumentsComparator.java b/src/main/java/org/mockito/internal/invocation/ArgumentsComparator.java
@@ -4,12 +4,14 @@
  */
 package org.mockito.internal.invocation;
 
+import static java.lang.reflect.Modifier.isPublic;
+
+import java.lang.reflect.Method;
+import java.util.List;
 import org.mockito.ArgumentMatcher;
 import org.mockito.internal.matchers.VarargMatcher;
 import org.mockito.invocation.Invocation;
 
-import java.util.List;
-
 @SuppressWarnings("unchecked")
 public class ArgumentsComparator {
     public boolean argumentsMatch(InvocationMatcher invocationMatcher, Invocation actual) {
@@ -22,21 +24,25 @@ public boolean argumentsMatch(InvocationMatcher invocationMatcher, Object[] actu
             return false;
         }
         for (int i = 0; i < actualArgs.length; i++) {
-            if (!invocationMatcher.getMatchers().get(i).matches(actualArgs[i])) {
+            ArgumentMatcher<Object> argumentMatcher = invocationMatcher.getMatchers().get(i);
+            Object argument = actualArgs[i];
+
+            if (!matches(argumentMatcher, argument)) {
                 return false;
             }
         }
         return true;
     }
 
-    //ok, this method is a little bit messy but the vararg business unfortunately is messy...      
-    private boolean varArgsMatch(InvocationMatcher invocationMatcher, Invocation actual) {
+    // ok, this method is a little bit messy but the vararg business
+    // unfortunately is messy...
+    private static boolean varArgsMatch(InvocationMatcher invocationMatcher, Invocation actual) {
         if (!actual.getMethod().isVarArgs()) {
-            //if the method is not vararg forget about it
+            // if the method is not vararg forget about it
             return false;
         }
 
-        //we must use raw arguments, not arguments...
+        // we must use raw arguments, not arguments...
         Object[] rawArgs = actual.getRawArguments();
         List<ArgumentMatcher> matchers = invocationMatcher.getMatchers();
 
@@ -46,18 +52,71 @@ private boolean varArgsMatch(InvocationMatcher invocationMatcher, Invocation act
 
         for (int i = 0; i < rawArgs.length; i++) {
             ArgumentMatcher m = matchers.get(i);
-            //it's a vararg because it's the last array in the arg list
-            if (rawArgs[i] != null && rawArgs[i].getClass().isArray() && i == rawArgs.length-1) {
-                //this is very important to only allow VarargMatchers here. If you're not sure why remove it and run all tests.
+            // it's a vararg because it's the last array in the arg list
+            if (rawArgs[i] != null && rawArgs[i].getClass().isArray() && i == rawArgs.length - 1) {
+                // this is very important to only allow VarargMatchers here. If
+                // you're not sure why remove it and run all tests.
                 if (!(m instanceof VarargMatcher) || !m.matches(rawArgs[i])) {
                     return false;
                 }
-            //it's not a vararg (i.e. some ordinary argument before varargs), just do the ordinary check
-            } else if (!m.matches(rawArgs[i])){
+                // it's not a vararg (i.e. some ordinary argument before
+                // varargs), just do the ordinary check
+            } else if (!m.matches(rawArgs[i])) {
                 return false;
             }
         }
 
         return true;
     }
+
+    private static boolean matches(ArgumentMatcher<Object> argumentMatcher, Object argument) {
+        if (isCompatible(argumentMatcher, argument)) {
+            return argumentMatcher.matches(argument);
+        }
+
+        return false;
+    }
+
+    /**
+     * Returns <code>true</code> if the given <b>argument</b> can be passed to the given <code>argumentMatcher</code> 
+     * without causing a {@link ClassCastException}.
+     */
+    private static boolean isCompatible(ArgumentMatcher<?> argumentMatcher, Object argument) {
+        if (argument == null)
+            return true;
+
+        Class<?> expectedArgumentType = getArgumentType(argumentMatcher);
+        Class<?> actualArgumentType = argument.getClass();
+
+        return expectedArgumentType.isAssignableFrom(actualArgumentType);
+    }
+
+    /**
+     * Returns the type of {@link ArgumentMatcher#matches(T)} of the given
+     * {@link ArgumentMatcher} implementation.
+     */
+    private static Class<?> getArgumentType(ArgumentMatcher<?> argumentMatcher) {
+        Method[] methods = argumentMatcher.getClass().getMethods();
+        for (Method method : methods) {
+            if (isMatchesMethod(method)) {
+                return method.getParameterTypes()[0];
+            }
+
+        }
+        throw new NoSuchMethodError("Method 'matches(T)' not found in ArgumentMatcher: " + argumentMatcher + " !\r\n Please file a bug with this stack trace at: https://github.com/mockito/mockito/issues/new ");
+    }
+
+    /**
+     * Returns <code>true</code> if the given method is
+     * {@link ArgumentMatcher#matches(T)}
+     */
+    private static boolean isMatchesMethod(Method method) {
+        if (!isPublic(method.getModifiers()))
+            return false;
+
+        if (method.getParameterCount() != 1)
+            return false;
+
+        return method.getName().equals("matches");
+    }
 }
diff --git a/src/main/java/org/mockito/internal/matchers/CompareEqual.java b/src/main/java/org/mockito/internal/matchers/CompareEqual.java
@@ -9,7 +9,7 @@
 
 public class CompareEqual<T extends Comparable<T>> extends CompareTo<T> implements Serializable {
 
-    public CompareEqual(Comparable<T> value) {
+    public CompareEqual(T value) {
         super(value);
     }
 

diff --git a/src/main/java/org/mockito/internal/matchers/CompareTo.java b/src/main/java/org/mockito/internal/matchers/CompareTo.java
@@ -9,24 +9,27 @@
 
 import java.io.Serializable;
 
-
 public abstract class CompareTo<T extends Comparable<T>> implements ArgumentMatcher<T>, Serializable {
-    private final Comparable<T> wanted;
+    private final T wanted;
 
-    public CompareTo(Comparable<T> value) {
+    public CompareTo(T value) {
         this.wanted = value;
     }
 
-    @SuppressWarnings("unchecked")
     public boolean matches(T actual) {
-        return matchResult(((Comparable)actual).compareTo(wanted));
+        if (actual == null) {
+            return false;
+        }
+
+        int result = actual.compareTo(wanted);
+        return matchResult(result);
     }
 
     public String toString() {
         return getName() + "(" + wanted + ")";
     }
-    
+
     protected abstract String getName();
-    
+
     protected abstract boolean matchResult(int result);
 }
diff --git a/src/main/java/org/mockito/internal/matchers/GreaterOrEqual.java b/src/main/java/org/mockito/internal/matchers/GreaterOrEqual.java
@@ -9,7 +9,7 @@
 
 public class GreaterOrEqual<T extends Comparable<T>> extends CompareTo<T> implements Serializable {
 
-    public GreaterOrEqual(Comparable<T> value) {
+    public GreaterOrEqual(T value) {
         super(value);
     }
 

diff --git a/src/main/java/org/mockito/internal/matchers/GreaterThan.java b/src/main/java/org/mockito/internal/matchers/GreaterThan.java
@@ -9,7 +9,7 @@
 
 public class GreaterThan<T extends Comparable<T>> extends CompareTo<T> implements Serializable {
 
-    public GreaterThan(Comparable<T> value) {
+    public GreaterThan(T value) {
         super(value);
     }
 

diff --git a/src/main/java/org/mockito/internal/matchers/LessOrEqual.java b/src/main/java/org/mockito/internal/matchers/LessOrEqual.java
@@ -9,7 +9,7 @@
 
 public class LessOrEqual<T extends Comparable<T>> extends CompareTo<T> implements Serializable {
 
-    public LessOrEqual(Comparable<T> value) {
+    public LessOrEqual(T value) {
         super(value);
     }
 

diff --git a/src/main/java/org/mockito/internal/matchers/LessThan.java b/src/main/java/org/mockito/internal/matchers/LessThan.java
@@ -9,7 +9,7 @@
 
 public class LessThan<T extends Comparable<T>> extends CompareTo<T> implements Serializable {
 
-    public LessThan(Comparable<T> value) {
+    public LessThan(T value) {
         super(value);
     }
 

diff --git a/src/test/java/org/mockito/internal/matchers/ComparableMatchersTest.java b/src/test/java/org/mockito/internal/matchers/ComparableMatchersTest.java
@@ -5,14 +5,13 @@
 
 package org.mockito.internal.matchers;
 
-import org.junit.Test;
-import org.mockitoutil.TestBase;
-
-import java.math.BigDecimal;
-
 import static junit.framework.TestCase.assertEquals;
 import static junit.framework.TestCase.assertTrue;
 
+import java.math.BigDecimal;
+import org.junit.Test;
+import org.mockitoutil.TestBase;
+
 public class ComparableMatchersTest extends TestBase {
 
     @Test
@@ -43,6 +42,8 @@ public void testCompareEqual() {
         CompareEqual<BigDecimal> cmpEq = new CompareEqual<BigDecimal>(new BigDecimal("5.00"));
         assertTrue(cmpEq.matches(new BigDecimal("5")));
     }
+
+
 
     private void test(CompareTo<String> compareTo, boolean lower, boolean higher,
             boolean equals, String name) {

diff --git a/src/test/java/org/mockitousage/bugs/CompareMatcherTest.java b/src/test/java/org/mockitousage/bugs/CompareMatcherTest.java
@@ -0,0 +1,64 @@
+package org.mockitousage.bugs;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.AdditionalMatchers.leq;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.mockitousage.IMethods;
+
+public class CompareMatcherTest {
+    private static final Object NOT_COMPARABLE = new Object();
+
+    /**
+     * Should not throw an {@link NullPointerException}
+     * @see Bug-ID https://github.com/mockito/mockito/issues/457
+     */
+    @Test
+    public void compareNullArgument() {
+        final IMethods mock = mock(IMethods.class);
+
+        when(mock.forInteger(leq(5))).thenReturn("");
+
+        assertThat(mock.forInteger(null)).isNull();//a default value must be returned
+    }
+
+    /**
+     * Should not throw an {@link ClassCastException}
+     */
+    @Test
+    public void compareToNonCompareable() {
+        final IMethods mock = mock(IMethods.class);
+
+        when(mock.forObject(leq(5))).thenReturn("");
+
+        assertThat(mock.forObject(NOT_COMPARABLE)).isNull();//a default value must be returned
+    }
+
+    /**
+     * Should not throw an {@link ClassCastException}
+     */
+    @Test
+    public void compareToNull() {
+        final IMethods mock = mock(IMethods.class);
+
+        when(mock.forInteger(leq((Integer)null))).thenReturn("");
+
+        assertThat(mock.forInteger(null)).isNull();//a default value must be returned
+    }
+
+    /**
+     * Should not throw an {@link ClassCastException}
+     */
+    @Test
+    public void compareToStringVsInt() {
+        final IMethods mock = mock(IMethods.class);
+
+        when(mock.forObject(Mockito.startsWith("Hello"))).thenReturn("x");
+
+        assertThat(mock.forObject(123)).isNull();//a default value must be returned
+    }
+
+}