You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2020/12/21 21:28:27 UTC

[GitHub] [commons-lang] mdbuck77 opened a new pull request #680: LANG-1544:

mdbuck77 opened a new pull request #680:
URL: https://github.com/apache/commons-lang/pull/680


   - Null guards in place to handle one or more nulls specified as one of the parameters of the method to invoke.
   - Check for an exact match of the actual parameter types against all of the methods on the class. This prevents picking an "upcasted" method (i.e. int specified but a method with a double is chosen).
   - Throw an IllegalStateException with a helpful message if multiple candidate methods were found. This happens when multiple Methods had the same "distance" from the desired parameter types. Before this change the algorithm would just chose the first one.
   - Tests for the above.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] mdbuck77 commented on a change in pull request #680: [LANG-1544] MethodUtils.invokeMethod NullPointerException in case of null in args list

Posted by GitBox <gi...@apache.org>.
mdbuck77 commented on a change in pull request #680:
URL: https://github.com/apache/commons-lang/pull/680#discussion_r547395975



##########
File path: src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java
##########
@@ -742,35 +745,51 @@ public static Method getMatchingMethod(final Class<?> cls, final String methodNa
         Validate.notNull(cls, "cls");
         Validate.notEmpty(methodName, "methodName");
 
-        // Address methods in superclasses
-        Method[] methodArray = cls.getDeclaredMethods();
-        final List<Class<?>> superclassList = ClassUtils.getAllSuperclasses(cls);
-        for (final Class<?> klass : superclassList) {
-            methodArray = ArrayUtils.addAll(methodArray, klass.getDeclaredMethods());
-        }
+        final List<Method> methods = Arrays.stream(cls.getDeclaredMethods())
+                .filter(method -> method.getName().equals(methodName))
+                .collect(toList());
+
+        ClassUtils.getAllSuperclasses(cls).stream()
+                .map(Class::getDeclaredMethods)
+                .flatMap(Arrays::stream)
+                .filter(method -> method.getName().equals(methodName))
+                .forEach(methods::add);
 
-        Method inexactMatch = null;
-        for (final Method method : methodArray) {
-            if (methodName.equals(method.getName()) &&
-                    Objects.deepEquals(parameterTypes, method.getParameterTypes())) {
+        for (Method method : methods) {
+            if (Arrays.deepEquals(method.getParameterTypes(), parameterTypes)) {
                 return method;
-            } else if (methodName.equals(method.getName()) &&
-                    ClassUtils.isAssignable(parameterTypes, method.getParameterTypes(), true)) {
-                if ((inexactMatch == null) || (distance(parameterTypes, method.getParameterTypes())
-                        < distance(parameterTypes, inexactMatch.getParameterTypes()))) {
-                    inexactMatch = method;
-                }
             }
+        }
+
+        final TreeMap<Double, List<Method>> candidates = new TreeMap<>();
 
+        methods.stream()
+                .filter(method -> ClassUtils.isAssignable(parameterTypes, method.getParameterTypes(), true))
+                .forEach(method -> {
+                    final double distance = distance(parameterTypes, method.getParameterTypes());
+                    List<Method> methods1 = candidates.computeIfAbsent(distance, k -> new ArrayList<>());
+                    methods1.add(method);
+                });
+
+        if (candidates.isEmpty()) {
+            return null;
         }
-        return inexactMatch;
+
+        final List<Method> bestCandidates = candidates.values().iterator().next();
+        if (bestCandidates.size() == 1) {
+            return bestCandidates.get(0);
+        }
+
+        final String target = methodName + Arrays.stream(parameterTypes).map(String::valueOf).collect(Collectors.joining(",", "(", ")"));
+        final String strCandidates = bestCandidates.stream().map(Method::toString).collect(Collectors.joining("\n  "));
+        throw new IllegalStateException("Found multiple candidates for method " + target + " on class " + cls + ":\n  " + strCandidates);

Review comment:
       I replaced the newlines with opening and closing brackets.
   
   Michael




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] garydgregory commented on pull request #680: LANG-1544:

Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #680:
URL: https://github.com/apache/commons-lang/pull/680#issuecomment-749585160


   https://issues.apache.org/jira/browse/LANG-1544


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls edited a comment on pull request #680: [LANG-1544] MethodUtils.invokeMethod NullPointerException in case of null in args list

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #680:
URL: https://github.com/apache/commons-lang/pull/680#issuecomment-749221904


   
   [![Coverage Status](https://coveralls.io/builds/35897811/badge)](https://coveralls.io/builds/35897811)
   
   Coverage increased (+0.003%) to 95.02% when pulling **dcc01d911c1f0433f49123ef15873e386b5d45a8 on mdbuck77:LANG-1544_MethodUtils_Throws_NPE** into **c9e825e823e30c5b1e3ddc9de5e8fd0094d52ee5 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] mdbuck77 commented on a change in pull request #680: [LANG-1544] MethodUtils.invokeMethod NullPointerException in case of null in args list

Posted by GitBox <gi...@apache.org>.
mdbuck77 commented on a change in pull request #680:
URL: https://github.com/apache/commons-lang/pull/680#discussion_r547396204



##########
File path: src/test/java/org/apache/commons/lang3/reflect/MethodUtilsTest.java
##########
@@ -1018,4 +1019,83 @@ public void testDistance() throws Exception {
 
         distanceMethod.setAccessible(false);
     }
+
+    @Test
+    public void testGetMatchingMethod() throws NoSuchMethodException {
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod"),
+                GetMatchingMethodClass.class.getMethod("testMethod"));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", Long.class),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.class));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", (Class<?>) null),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.class));
+
+        assertThrows(IllegalStateException.class,
+                () -> MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod2", (Class<?>) null));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.TYPE, Long.class),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.TYPE, Long.class));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.class, Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.class, Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", null, Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.class, Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.TYPE, null),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.TYPE, Long.class));
+
+        assertThrows(IllegalStateException.class,
+                () -> MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod4", null, null));
+    }
+
+    private static final class GetMatchingMethodClass {

Review comment:
       Done.
   
   Michael




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] garydgregory commented on a change in pull request #680: [LANG-1544] MethodUtils.invokeMethod NullPointerException in case of null in args list

Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #680:
URL: https://github.com/apache/commons-lang/pull/680#discussion_r547326472



##########
File path: src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java
##########
@@ -742,35 +745,51 @@ public static Method getMatchingMethod(final Class<?> cls, final String methodNa
         Validate.notNull(cls, "cls");
         Validate.notEmpty(methodName, "methodName");
 
-        // Address methods in superclasses
-        Method[] methodArray = cls.getDeclaredMethods();
-        final List<Class<?>> superclassList = ClassUtils.getAllSuperclasses(cls);
-        for (final Class<?> klass : superclassList) {
-            methodArray = ArrayUtils.addAll(methodArray, klass.getDeclaredMethods());
-        }
+        final List<Method> methods = Arrays.stream(cls.getDeclaredMethods())
+                .filter(method -> method.getName().equals(methodName))
+                .collect(toList());
+
+        ClassUtils.getAllSuperclasses(cls).stream()
+                .map(Class::getDeclaredMethods)
+                .flatMap(Arrays::stream)
+                .filter(method -> method.getName().equals(methodName))
+                .forEach(methods::add);
 
-        Method inexactMatch = null;
-        for (final Method method : methodArray) {
-            if (methodName.equals(method.getName()) &&
-                    Objects.deepEquals(parameterTypes, method.getParameterTypes())) {
+        for (Method method : methods) {
+            if (Arrays.deepEquals(method.getParameterTypes(), parameterTypes)) {
                 return method;
-            } else if (methodName.equals(method.getName()) &&
-                    ClassUtils.isAssignable(parameterTypes, method.getParameterTypes(), true)) {
-                if ((inexactMatch == null) || (distance(parameterTypes, method.getParameterTypes())
-                        < distance(parameterTypes, inexactMatch.getParameterTypes()))) {
-                    inexactMatch = method;
-                }
             }
+        }
+
+        final TreeMap<Double, List<Method>> candidates = new TreeMap<>();
 
+        methods.stream()
+                .filter(method -> ClassUtils.isAssignable(parameterTypes, method.getParameterTypes(), true))
+                .forEach(method -> {
+                    final double distance = distance(parameterTypes, method.getParameterTypes());
+                    List<Method> methods1 = candidates.computeIfAbsent(distance, k -> new ArrayList<>());
+                    methods1.add(method);
+                });
+
+        if (candidates.isEmpty()) {
+            return null;
         }
-        return inexactMatch;
+
+        final List<Method> bestCandidates = candidates.values().iterator().next();
+        if (bestCandidates.size() == 1) {
+            return bestCandidates.get(0);
+        }
+
+        final String target = methodName + Arrays.stream(parameterTypes).map(String::valueOf).collect(Collectors.joining(",", "(", ")"));
+        final String strCandidates = bestCandidates.stream().map(Method::toString).collect(Collectors.joining("\n  "));
+        throw new IllegalStateException("Found multiple candidates for method " + target + " on class " + cls + ":\n  " + strCandidates);

Review comment:
       Let's not hard-code Linux-specific new-line chars in error messages, but, if you really want new-lines, use `%n` with `String.format()`.

##########
File path: src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java
##########
@@ -742,35 +745,51 @@ public static Method getMatchingMethod(final Class<?> cls, final String methodNa
         Validate.notNull(cls, "cls");
         Validate.notEmpty(methodName, "methodName");
 
-        // Address methods in superclasses
-        Method[] methodArray = cls.getDeclaredMethods();
-        final List<Class<?>> superclassList = ClassUtils.getAllSuperclasses(cls);
-        for (final Class<?> klass : superclassList) {
-            methodArray = ArrayUtils.addAll(methodArray, klass.getDeclaredMethods());
-        }
+        final List<Method> methods = Arrays.stream(cls.getDeclaredMethods())
+                .filter(method -> method.getName().equals(methodName))
+                .collect(toList());
+
+        ClassUtils.getAllSuperclasses(cls).stream()
+                .map(Class::getDeclaredMethods)
+                .flatMap(Arrays::stream)
+                .filter(method -> method.getName().equals(methodName))
+                .forEach(methods::add);
 
-        Method inexactMatch = null;
-        for (final Method method : methodArray) {
-            if (methodName.equals(method.getName()) &&
-                    Objects.deepEquals(parameterTypes, method.getParameterTypes())) {
+        for (Method method : methods) {
+            if (Arrays.deepEquals(method.getParameterTypes(), parameterTypes)) {
                 return method;
-            } else if (methodName.equals(method.getName()) &&
-                    ClassUtils.isAssignable(parameterTypes, method.getParameterTypes(), true)) {
-                if ((inexactMatch == null) || (distance(parameterTypes, method.getParameterTypes())
-                        < distance(parameterTypes, inexactMatch.getParameterTypes()))) {
-                    inexactMatch = method;
-                }
             }
+        }
+
+        final TreeMap<Double, List<Method>> candidates = new TreeMap<>();
 
+        methods.stream()
+                .filter(method -> ClassUtils.isAssignable(parameterTypes, method.getParameterTypes(), true))
+                .forEach(method -> {
+                    final double distance = distance(parameterTypes, method.getParameterTypes());
+                    List<Method> methods1 = candidates.computeIfAbsent(distance, k -> new ArrayList<>());
+                    methods1.add(method);
+                });
+
+        if (candidates.isEmpty()) {
+            return null;
         }
-        return inexactMatch;
+
+        final List<Method> bestCandidates = candidates.values().iterator().next();
+        if (bestCandidates.size() == 1) {
+            return bestCandidates.get(0);
+        }
+
+        final String target = methodName + Arrays.stream(parameterTypes).map(String::valueOf).collect(Collectors.joining(",", "(", ")"));
+        final String strCandidates = bestCandidates.stream().map(Method::toString).collect(Collectors.joining("\n  "));
+        throw new IllegalStateException("Found multiple candidates for method " + target + " on class " + cls + ":\n  " + strCandidates);
     }
 
     /**
      * <p>Returns the aggregate number of inheritance hops between assignable argument class types.  Returns -1
      * if the arguments aren't assignable.  Fills a specific purpose for getMatchingMethod and is not generalized.</p>
-     * @param classArray
-     * @param toClassArray
+     * @param classArray the Class array to calculate the distance from.

Review comment:
       Maybe rename to `fromClassArray` to match `toClassArray`?

##########
File path: src/test/java/org/apache/commons/lang3/reflect/MethodUtilsTest.java
##########
@@ -1018,4 +1019,83 @@ public void testDistance() throws Exception {
 
         distanceMethod.setAccessible(false);
     }
+
+    @Test
+    public void testGetMatchingMethod() throws NoSuchMethodException {
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod"),
+                GetMatchingMethodClass.class.getMethod("testMethod"));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", Long.class),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.class));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", (Class<?>) null),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.class));
+
+        assertThrows(IllegalStateException.class,
+                () -> MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod2", (Class<?>) null));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.TYPE, Long.class),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.TYPE, Long.class));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.class, Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.class, Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", null, Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.class, Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.TYPE, null),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.TYPE, Long.class));
+
+        assertThrows(IllegalStateException.class,
+                () -> MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod4", null, null));
+    }
+
+    private static final class GetMatchingMethodClass {
+        public String testMethod() {
+            return "testMethod";
+        }
+
+        public String testMethod(Long aLong) {

Review comment:
       Use final where you can, like here for parms.
   

##########
File path: src/test/java/org/apache/commons/lang3/reflect/MethodUtilsTest.java
##########
@@ -1018,4 +1019,83 @@ public void testDistance() throws Exception {
 
         distanceMethod.setAccessible(false);
     }
+
+    @Test
+    public void testGetMatchingMethod() throws NoSuchMethodException {
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod"),
+                GetMatchingMethodClass.class.getMethod("testMethod"));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", Long.class),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.class));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", (Class<?>) null),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.class));
+
+        assertThrows(IllegalStateException.class,
+                () -> MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod2", (Class<?>) null));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.TYPE, Long.class),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.TYPE, Long.class));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.class, Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.class, Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", null, Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.class, Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.TYPE, null),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.TYPE, Long.class));
+
+        assertThrows(IllegalStateException.class,
+                () -> MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod4", null, null));
+    }
+
+    private static final class GetMatchingMethodClass {

Review comment:
       Please make the test fixture as simple as possible: All of these methods can just return `void`.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] coveralls commented on pull request #680: LANG-1544:

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #680:
URL: https://github.com/apache/commons-lang/pull/680#issuecomment-749221904


   
   [![Coverage Status](https://coveralls.io/builds/35875621/badge)](https://coveralls.io/builds/35875621)
   
   Coverage increased (+0.003%) to 95.019% when pulling **8bb3703f828c61b891e5cb79c34f572051f958ff on mdbuck77:LANG-1544_MethodUtils_Throws_NPE** into **c9e825e823e30c5b1e3ddc9de5e8fd0094d52ee5 on apache:master**.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] garydgregory merged pull request #680: [LANG-1544] MethodUtils.invokeMethod NullPointerException in case of null in args list

Posted by GitBox <gi...@apache.org>.
garydgregory merged pull request #680:
URL: https://github.com/apache/commons-lang/pull/680


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] mdbuck77 commented on a change in pull request #680: [LANG-1544] MethodUtils.invokeMethod NullPointerException in case of null in args list

Posted by GitBox <gi...@apache.org>.
mdbuck77 commented on a change in pull request #680:
URL: https://github.com/apache/commons-lang/pull/680#discussion_r547396391



##########
File path: src/main/java/org/apache/commons/lang3/reflect/MethodUtils.java
##########
@@ -742,35 +745,51 @@ public static Method getMatchingMethod(final Class<?> cls, final String methodNa
         Validate.notNull(cls, "cls");
         Validate.notEmpty(methodName, "methodName");
 
-        // Address methods in superclasses
-        Method[] methodArray = cls.getDeclaredMethods();
-        final List<Class<?>> superclassList = ClassUtils.getAllSuperclasses(cls);
-        for (final Class<?> klass : superclassList) {
-            methodArray = ArrayUtils.addAll(methodArray, klass.getDeclaredMethods());
-        }
+        final List<Method> methods = Arrays.stream(cls.getDeclaredMethods())
+                .filter(method -> method.getName().equals(methodName))
+                .collect(toList());
+
+        ClassUtils.getAllSuperclasses(cls).stream()
+                .map(Class::getDeclaredMethods)
+                .flatMap(Arrays::stream)
+                .filter(method -> method.getName().equals(methodName))
+                .forEach(methods::add);
 
-        Method inexactMatch = null;
-        for (final Method method : methodArray) {
-            if (methodName.equals(method.getName()) &&
-                    Objects.deepEquals(parameterTypes, method.getParameterTypes())) {
+        for (Method method : methods) {
+            if (Arrays.deepEquals(method.getParameterTypes(), parameterTypes)) {
                 return method;
-            } else if (methodName.equals(method.getName()) &&
-                    ClassUtils.isAssignable(parameterTypes, method.getParameterTypes(), true)) {
-                if ((inexactMatch == null) || (distance(parameterTypes, method.getParameterTypes())
-                        < distance(parameterTypes, inexactMatch.getParameterTypes()))) {
-                    inexactMatch = method;
-                }
             }
+        }
+
+        final TreeMap<Double, List<Method>> candidates = new TreeMap<>();
 
+        methods.stream()
+                .filter(method -> ClassUtils.isAssignable(parameterTypes, method.getParameterTypes(), true))
+                .forEach(method -> {
+                    final double distance = distance(parameterTypes, method.getParameterTypes());
+                    List<Method> methods1 = candidates.computeIfAbsent(distance, k -> new ArrayList<>());
+                    methods1.add(method);
+                });
+
+        if (candidates.isEmpty()) {
+            return null;
         }
-        return inexactMatch;
+
+        final List<Method> bestCandidates = candidates.values().iterator().next();
+        if (bestCandidates.size() == 1) {
+            return bestCandidates.get(0);
+        }
+
+        final String target = methodName + Arrays.stream(parameterTypes).map(String::valueOf).collect(Collectors.joining(",", "(", ")"));
+        final String strCandidates = bestCandidates.stream().map(Method::toString).collect(Collectors.joining("\n  "));
+        throw new IllegalStateException("Found multiple candidates for method " + target + " on class " + cls + ":\n  " + strCandidates);
     }
 
     /**
      * <p>Returns the aggregate number of inheritance hops between assignable argument class types.  Returns -1
      * if the arguments aren't assignable.  Fills a specific purpose for getMatchingMethod and is not generalized.</p>
-     * @param classArray
-     * @param toClassArray
+     * @param classArray the Class array to calculate the distance from.

Review comment:
       Good suggestion. Done.
   
   Michael




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [commons-lang] mdbuck77 commented on a change in pull request #680: [LANG-1544] MethodUtils.invokeMethod NullPointerException in case of null in args list

Posted by GitBox <gi...@apache.org>.
mdbuck77 commented on a change in pull request #680:
URL: https://github.com/apache/commons-lang/pull/680#discussion_r547396104



##########
File path: src/test/java/org/apache/commons/lang3/reflect/MethodUtilsTest.java
##########
@@ -1018,4 +1019,83 @@ public void testDistance() throws Exception {
 
         distanceMethod.setAccessible(false);
     }
+
+    @Test
+    public void testGetMatchingMethod() throws NoSuchMethodException {
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod"),
+                GetMatchingMethodClass.class.getMethod("testMethod"));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", Long.class),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.class));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod", (Class<?>) null),
+                GetMatchingMethodClass.class.getMethod("testMethod", Long.class));
+
+        assertThrows(IllegalStateException.class,
+                () -> MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod2", (Class<?>) null));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.TYPE, Long.class),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.TYPE, Long.class));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.class, Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.class, Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", null, Long.TYPE),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.class, Long.TYPE));
+
+        assertEquals(MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod3", Long.TYPE, null),
+                GetMatchingMethodClass.class.getMethod("testMethod3", Long.TYPE, Long.class));
+
+        assertThrows(IllegalStateException.class,
+                () -> MethodUtils.getMatchingMethod(GetMatchingMethodClass.class, "testMethod4", null, null));
+    }
+
+    private static final class GetMatchingMethodClass {
+        public String testMethod() {
+            return "testMethod";
+        }
+
+        public String testMethod(Long aLong) {

Review comment:
       Done.
   
   Michael




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org