Fixes mockito#2311

Made changes to Equals.toStringWithType by reusing same method to achieve both cases by sending a boolean as an input, based on which either simple name or fully qualified name will be used for describing the type. Also in the method ArgumentMatchingTool.getNotMatchingArgsWithSameName return Set<String> which return the simple names of classes having more than one classes with different FQCN.
saurabh7248 · Jun 13, 2021 · 446c5fe · 446c5fe
1 parent d544cfc
commit 446c5fe
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 61 deletions.
diff --git a/src/main/java/org/mockito/internal/matchers/ContainsExtraTypeInfo.java b/src/main/java/org/mockito/internal/matchers/ContainsExtraTypeInfo.java
@@ -9,29 +9,25 @@
  * When ArgumentMatcher fails, chance is that the actual object has the same output of toString() than
  * the wanted object. This looks weird when failures are reported.
  * Therefore when matcher fails but toString() yields the same outputs,
- * we will try to use the {@link #toStringWithType()} method.
+ * we will try to use the {@link #toStringWithType(boolean)} method.
  */
 public interface ContainsExtraTypeInfo {
 
     /**
+     * @param useFullyQualifiedClassName - uses fully qualified class name if true else simple class name
      * Returns more verbose description of the object which include type information
      */
-    String toStringWithType();
-
-    /**
-     * Returns more verbose description of the object which include type information and fully qualified class name
-     */
-    String toStringWithFullName();
+    String toStringWithType(boolean useFullyQualifiedClassName);
 
     /**
      * Checks if target target has matching type.
-     * If the type matches, there is no point in rendering result from {@link #toStringWithType()}
+     * If the type matches, there is no point in rendering result from {@link #toStringWithType(boolean)}
      */
     boolean typeMatches(Object target);
 
     /**
-     * Checks if target target's class has same simple name.
-     * If the simple names matches, we need to use {@link #toStringWithFullName()}
+     *
+     * @return Returns the Class of the argument
      */
-    boolean sameName(Object target);
+    Class getWantedClass();
 }
diff --git a/src/main/java/org/mockito/internal/matchers/Equals.java b/src/main/java/org/mockito/internal/matchers/Equals.java
@@ -51,13 +51,8 @@ public int hashCode() {
     }
 
     @Override
-    public String toStringWithType() {
-        return "(" + wanted.getClass().getSimpleName() + ") " + describe(wanted);
-    }
-
-    @Override
-    public String toStringWithFullName() {
-        return "(" + wanted.getClass().getCanonicalName() + ") " + describe(wanted);
+    public String toStringWithType(boolean useFullyQualifiedClassName) {
+        return "(" + (useFullyQualifiedClassName ? wanted.getClass().getCanonicalName() : wanted.getClass().getSimpleName()) + ") " + describe(wanted);
     }
 
     @Override
@@ -66,7 +61,8 @@ public boolean typeMatches(Object target) {
     }
 
     @Override
-    public boolean sameName(Object target) {
-        return wanted != null && target != null && target.getClass().getSimpleName().equals(wanted.getClass().getSimpleName());
+    public Class getWantedClass() {
+        return wanted.getClass();
     }
+
 }
diff --git a/src/main/java/org/mockito/internal/matchers/text/MatchersPrinter.java b/src/main/java/org/mockito/internal/matchers/text/MatchersPrinter.java
@@ -31,9 +31,10 @@ private Iterator<FormattedText> applyPrintSettings(
         int i = 0;
         for (final ArgumentMatcher matcher : matchers) {
             if (matcher instanceof ContainsExtraTypeInfo && printSettings.extraTypeInfoFor(i)) {
-                out.add(new FormattedText(((ContainsExtraTypeInfo) matcher).toStringWithType()));
-            } else if(matcher instanceof ContainsExtraTypeInfo && printSettings.fullyQualifiedNameFor(i)){
-                out.add(new FormattedText(((ContainsExtraTypeInfo) matcher).toStringWithFullName()));
+                out.add(new FormattedText(((ContainsExtraTypeInfo) matcher).toStringWithType(false)));
+            } else if(matcher instanceof ContainsExtraTypeInfo
+                      && printSettings.fullyQualifiedNameFor(((ContainsExtraTypeInfo) matcher).getWantedClass().getSimpleName())){
+                out.add(new FormattedText(((ContainsExtraTypeInfo) matcher).toStringWithType(true)));
             } else {
                 out.add(new FormattedText(MatcherToString.toString(matcher)));
             }

diff --git a/src/main/java/org/mockito/internal/reporting/PrintSettings.java b/src/main/java/org/mockito/internal/reporting/PrintSettings.java
@@ -5,8 +5,10 @@
 package org.mockito.internal.reporting;
 
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Set;
 
 import org.mockito.ArgumentMatcher;
 import org.mockito.internal.matchers.text.MatchersPrinter;
@@ -19,7 +21,7 @@ public class PrintSettings {
     public static final int MAX_LINE_LENGTH = 45;
     private boolean multiline;
     private List<Integer> withTypeInfo = new LinkedList<>();
-    private List<Integer> withFullyQualifiedName = new LinkedList<>();
+    private Set<String> withFullyQualifiedName = Collections.emptySet();
 
     public void setMultiline(boolean multiline) {
         this.multiline = multiline;
@@ -39,16 +41,16 @@ public boolean extraTypeInfoFor(int argumentIndex) {
         return withTypeInfo.contains(argumentIndex);
     }
 
-    public boolean fullyQualifiedNameFor(int argumentIndex) {
-        return withFullyQualifiedName.contains(argumentIndex);
+    public boolean fullyQualifiedNameFor(String simpleClassName) {
+        return withFullyQualifiedName.contains(simpleClassName);
     }
 
     public void setMatchersToBeDescribedWithExtraTypeInfo(Integer[] indexesOfMatchers) {
         this.withTypeInfo = Arrays.asList(indexesOfMatchers);
     }
 
-    public void setMatchersToBeDescribedWithFullName(Integer[] indexesOfMatchers) {
-        this.withFullyQualifiedName= Arrays.asList(indexesOfMatchers);
+    public void setMatchersToBeDescribedWithFullName(Set<String> indexesOfMatchers) {
+        this.withFullyQualifiedName= indexesOfMatchers;
     }
 
     public String print(List<ArgumentMatcher> matchers, Invocation invocation) {

diff --git a/src/main/java/org/mockito/internal/reporting/SmartPrinter.java b/src/main/java/org/mockito/internal/reporting/SmartPrinter.java
@@ -7,6 +7,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import java.util.Set;
 
 import org.mockito.invocation.Invocation;
 import org.mockito.invocation.MatchableInvocation;
@@ -29,20 +30,20 @@ public SmartPrinter(
                 wanted,
                 Collections.singletonList(actual),
                 indexesOfMatchersToBeDescribedWithExtraTypeInfo,
-                new Integer[0]);
+                Collections.emptySet());
     }
 
     public SmartPrinter(
             MatchableInvocation wanted,
             List<Invocation> allActualInvocations,
             Integer[] indexesOfMatchersToBeDescribedWithExtraTypeInfo,
-            Integer[] indexesOfMatchersToBeDescribedWithFullName) {
+            Set<String> classNamesToBeDescribedWithFullName) {
         PrintSettings printSettings = new PrintSettings();
         printSettings.setMultiline(isMultiLine(wanted, allActualInvocations));
         printSettings.setMatchersToBeDescribedWithExtraTypeInfo(
                 indexesOfMatchersToBeDescribedWithExtraTypeInfo);
         printSettings.setMatchersToBeDescribedWithFullName(
-                indexesOfMatchersToBeDescribedWithFullName);
+                classNamesToBeDescribedWithFullName);
 
         this.wanted = printSettings.print(wanted);
 

diff --git a/src/main/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingTool.java b/src/main/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingTool.java
@@ -4,8 +4,13 @@
  */
 package org.mockito.internal.verification.argumentmatching;
 
+import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedList;
 import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.mockito.ArgumentMatcher;
 import org.mockito.internal.matchers.ContainsExtraTypeInfo;
@@ -52,24 +57,26 @@ private static boolean toStringEquals(ArgumentMatcher m, Object arg) {
     /**
      * Suspiciously not matching arguments are those that don't match, and the classes have same simple name.
      */
-    public static Integer[] getNotMatchingArgsWithSameNameIndexes(
-        List<ArgumentMatcher> matchers, Object[] arguments) {
-        if (matchers.size() != arguments.length) {
-            return new Integer[0];
-        }
-
-        List<Integer> suspicious = new LinkedList<>();
-        int i = 0;
+    public static Set<String> getNotMatchingArgsWithSameName(
+            List<ArgumentMatcher> matchers, Object[] arguments) {
+        Set<String> repeatedClassNames = new HashSet<>();
+        Map<String, Set<String>> classesHavingSameName = new HashMap<>();
         for (ArgumentMatcher m : matchers) {
-            if (m instanceof ContainsExtraTypeInfo
-                && !safelyMatches(m, arguments[i])
-                && !((ContainsExtraTypeInfo) m).typeMatches(arguments[i])
-                && ((ContainsExtraTypeInfo) m).sameName(arguments[i])) {
-                suspicious.add(i);
+            if (m instanceof ContainsExtraTypeInfo) {
+                Class wantedClass = ((ContainsExtraTypeInfo) m).getWantedClass();
+                classesHavingSameName.computeIfAbsent(wantedClass.getSimpleName(), className -> new HashSet<>())
+                                     .add(wantedClass.getCanonicalName());
             }
-            i++;
         }
-        return suspicious.toArray(new Integer[0]);
+        for (Object argument : arguments) {
+            Class wantedClass = argument.getClass();
+            classesHavingSameName.computeIfAbsent(wantedClass.getSimpleName(), className -> new HashSet<>())
+                                 .add(wantedClass.getCanonicalName());
+        }
+        return classesHavingSameName.entrySet().stream()
+                                    .filter(classEntry -> classEntry.getValue().size() > 1)
+                                    .map(classEntry -> classEntry.getKey())
+                                    .collect(Collectors.toSet());
     }
 
 }
diff --git a/src/main/java/org/mockito/internal/verification/checkers/MissingInvocationChecker.java b/src/main/java/org/mockito/internal/verification/checkers/MissingInvocationChecker.java
@@ -11,10 +11,11 @@
 import static org.mockito.internal.invocation.InvocationsFinder.findInvocations;
 import static org.mockito.internal.invocation.InvocationsFinder.findPreviousVerifiedInOrder;
 import static org.mockito.internal.invocation.InvocationsFinder.findSimilarInvocation;
-import static org.mockito.internal.verification.argumentmatching.ArgumentMatchingTool.getNotMatchingArgsWithSameNameIndexes;
+import static org.mockito.internal.verification.argumentmatching.ArgumentMatchingTool.getNotMatchingArgsWithSameName;
 import static org.mockito.internal.verification.argumentmatching.ArgumentMatchingTool.getSuspiciouslyNotMatchingArgsIndexes;
 
 import java.util.List;
+import java.util.Set;
 
 import org.mockito.internal.reporting.SmartPrinter;
 import org.mockito.internal.util.collections.ListUtil;
@@ -42,9 +43,9 @@ public static void checkMissingInvocation(
 
         Integer[] indexesOfSuspiciousArgs =
                 getSuspiciouslyNotMatchingArgsIndexes(wanted.getMatchers(), similar.getArguments());
-        Integer[] indexesOfArgsWithSameSimpleName =
-            getNotMatchingArgsWithSameNameIndexes(wanted.getMatchers(), similar.getArguments());
-        SmartPrinter smartPrinter = new SmartPrinter(wanted, invocations, indexesOfSuspiciousArgs,indexesOfArgsWithSameSimpleName);
+        Set<String> classesWithSameSimpleName =
+            getNotMatchingArgsWithSameName(wanted.getMatchers(), similar.getArguments());
+        SmartPrinter smartPrinter = new SmartPrinter(wanted, invocations, indexesOfSuspiciousArgs,classesWithSameSimpleName);
         List<Location> actualLocations =
                 ListUtil.convert(
                         invocations,

diff --git a/src/test/java/org/mockito/internal/matchers/EqualsTest.java b/src/test/java/org/mockito/internal/matchers/EqualsTest.java
@@ -28,21 +28,21 @@ public void shouldArraysBeEqual() {
 
     @Test
     public void shouldDescribeWithExtraTypeInfo() throws Exception {
-        String descStr = new Equals(100).toStringWithType();
+        String descStr = new Equals(100).toStringWithType(false);
 
         assertEquals("(Integer) 100", descStr);
     }
 
     @Test
     public void shouldDescribeWithExtraTypeInfoOfLong() throws Exception {
-        String descStr = new Equals(100L).toStringWithType();
+        String descStr = new Equals(100L).toStringWithType(false);
 
         assertEquals("(Long) 100L", descStr);
     }
 
     @Test
     public void shouldDescribeWithTypeOfString() throws Exception {
-        String descStr = new Equals("x").toStringWithType();
+        String descStr = new Equals("x").toStringWithType(false);
 
         assertEquals("(String) \"x\"", descStr);
     }

diff --git a/...est/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingToolTest.java b/...est/java/org/mockito/internal/verification/argumentmatching/ArgumentMatchingToolTest.java
@@ -111,12 +111,7 @@ public boolean matches(String item) {
             }
 
             @Override
-            public String toStringWithType() {
-                return "";
-            }
-
-            @Override
-            public String toStringWithFullName() {
+            public String toStringWithType(boolean useFullyQualifiedClassName) {
                 return "";
             }
 
@@ -126,8 +121,8 @@ public boolean typeMatches(Object target) {
             }
 
             @Override
-            public boolean sameName(Object target) {
-                return true;
+            public Class getWantedClass() {
+                return String.class;
             }
 
         }