This is an automated email from the ASF dual-hosted git repository. henrib pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
The following commit(s) were added to refs/heads/master by this push: new 09c8ca8 JEXL-308: improve overload method selection, simplify most-applicable method selection, add tests Task #JEXL-308 - Improve overloaded method selection new 545ce96 Merge origin/master 09c8ca8 is described below commit 09c8ca8f63f5aba63262c9ad3047105172f6695b Author: Henri Biestro <hbies...@gmail.com> AuthorDate: Wed Jul 31 10:48:17 2019 +0200 JEXL-308: improve overload method selection, simplify most-applicable method selection, add tests Task #JEXL-308 - Improve overloaded method selection --- .../jexl3/internal/introspection/MethodKey.java | 89 ++++++++------------ .../internal/introspection/DiscoveryTest.java | 95 ++++++++++++++++++++++ 2 files changed, 130 insertions(+), 54 deletions(-) diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java index 1977d14..8719af2 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java +++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/MethodKey.java @@ -598,12 +598,15 @@ public final class MethodKey { * c1 is less specific than c2, INCOMPARABLE if they are incomparable. */ private int moreSpecific(final Class<?>[] a, final Class<?>[] c1, final Class<?>[] c2) { - boolean c1MoreSpecific = false; - boolean c2MoreSpecific = false; - // compare lengths to handle comparisons where the size of the arrays // doesn't match, but the methods are both applicable due to the fact // that one is a varargs method + if (c1.length > a.length) { + return LESS_SPECIFIC; + } + if (c2.length > a.length) { + return MORE_SPECIFIC; + } if (c1.length > c2.length) { return MORE_SPECIFIC; } @@ -614,57 +617,35 @@ public final class MethodKey { final int length = c1.length; final int ultimate = c1.length - 1; - // ok, move on and compare those of equal lengths - for (int i = 0; i < length; ++i) { - if (c1[i] != c2[i]) { - boolean last = (i == ultimate); - if (a[i] == Void.class) { - if (c1[i] == Object.class && c2[i] != Object.class) { - c1MoreSpecific = true; - continue; - } - if (c1[i] != Object.class && c2[i] == Object.class) { - c2MoreSpecific = true; - continue; - } - } - c1MoreSpecific = c1MoreSpecific || isStrictConvertible(c2[i], c1[i], last); - c2MoreSpecific = c2MoreSpecific || isStrictConvertible(c1[i], c2[i], last); - } - } - - if (c1MoreSpecific) { - if (c2MoreSpecific) { - // Incomparable due to cross-assignable arguments (i.e. foo(String, Object) vs. foo(Object, String)) - return INCOMPARABLE; - } - return MORE_SPECIFIC; - } - if (c2MoreSpecific) { - return LESS_SPECIFIC; - } - - // attempt to choose by picking the one with the greater number of primitives or latest primitive parameter - int primDiff = 0; - for (int c = 0; c < length; ++c) { - boolean last = (c == ultimate); - if (isPrimitive(c1[c], last)) { - primDiff += 1 << c; - } - if (isPrimitive(c2[c], last)) { - primDiff -= 1 << c; - } - } - if (primDiff > 0) { - return MORE_SPECIFIC; - } - if (primDiff < 0) { - return LESS_SPECIFIC; - } - /* - * Incomparable due to non-related arguments (i.e. - * foo(Runnable) vs. foo(Serializable)) - */ + // ok, move on and compare those of equal lengths + for (int i = 0; i < length; ++i) { + if (c1[i] != c2[i]) { + boolean last = (i == ultimate); + // argument is null, prefer an Object param + if (a[i] == Void.class) { + if (c1[i] == Object.class && c2[i] != Object.class) { + return MORE_SPECIFIC; + } + if (c1[i] != Object.class && c2[i] == Object.class) { + return LESS_SPECIFIC; + } + } + // prefer primitive on non null arg, non primitive otherwise + boolean c1s = isPrimitive(c1[i], last); + boolean c2s = isPrimitive(c2[i], last); + if (c1s != c2s) { + return (c1s == (a[i] != Void.class))? MORE_SPECIFIC : LESS_SPECIFIC; + } + // if c2 can be converted to c1 but not the opposite, + // c1 is more specific than c2 + c1s = isStrictConvertible(c2[i], c1[i], last); + c2s = isStrictConvertible(c1[i], c2[i], last); + if (c1s != c2s) { + return c1s ? MORE_SPECIFIC : LESS_SPECIFIC; + } + } + } + // Incomparable due to non-related arguments (i.e.foo(Runnable) vs. foo(Serializable)) return INCOMPARABLE; } diff --git a/src/test/java/org/apache/commons/jexl3/internal/introspection/DiscoveryTest.java b/src/test/java/org/apache/commons/jexl3/internal/introspection/DiscoveryTest.java index 1b26a4b..5e32f09 100644 --- a/src/test/java/org/apache/commons/jexl3/internal/introspection/DiscoveryTest.java +++ b/src/test/java/org/apache/commons/jexl3/internal/introspection/DiscoveryTest.java @@ -16,6 +16,7 @@ */ package org.apache.commons.jexl3.internal.introspection; +import java.io.Serializable; import org.apache.commons.jexl3.JexlTestCase; import org.apache.commons.jexl3.internal.Engine; import org.apache.commons.jexl3.introspection.JexlPropertyGet; @@ -25,6 +26,9 @@ import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.commons.jexl3.introspection.JexlMethod; +import org.apache.commons.logging.Log; +import org.apache.commons.logging.LogFactory; import org.junit.Assert; import org.junit.Test; @@ -221,4 +225,95 @@ public class DiscoveryTest extends JexlTestCase { Assert.assertEquals(AbstractExecutor.TRY_FAILED, set.tryInvoke(map, Integer.valueOf(1), "nope")); } + public static class Bulgroz { + public Object list(int x) { + return 0; + } + public Object list(String x) { + return 1; + } + public Object list(Object x) { + return 2; + } + public Object list(int x, Object...y) { + return 3; + } + public Object list(int x, int y) { + return 4; + } + public Object list(String x, Object...y) { + return 5; + } + public Object list(String x, String y) { + return 6; + } + public Object list(Object x, Object...y) { + return 7; + } + public Object list(Object x, Object y) { + return 8; + } + public Object amb(Serializable x) { + return -1; + } + public Object amb(Number x) { + return -2; + } + } + + @Test + public void testMethodIntrospection() throws Exception { + Uberspect uber = new Uberspect(null, null); + Bulgroz bulgroz = new Bulgroz(); + JexlMethod jmethod; + Object result; + jmethod = uber.getMethod(bulgroz, "list", 0); + result = jmethod.invoke(bulgroz, 0); + Assert.assertEquals(0, result); + jmethod = uber.getMethod(bulgroz, "list", "1"); + result = jmethod.invoke(bulgroz, "1"); + Assert.assertEquals(1, result); + jmethod = uber.getMethod(bulgroz, "list", bulgroz); + result = jmethod.invoke(bulgroz, bulgroz); + Assert.assertEquals(2, result); + jmethod = uber.getMethod(bulgroz, "list", 1, bulgroz); + result = jmethod.invoke(bulgroz, 1, bulgroz); + Assert.assertEquals(3, result); + jmethod = uber.getMethod(bulgroz, "list", 1, bulgroz, bulgroz); + result = jmethod.invoke(bulgroz, 1, bulgroz, bulgroz); + Assert.assertEquals(3, result); + jmethod = uber.getMethod(bulgroz, "list", 1, 2); + result = jmethod.invoke(bulgroz, 1, 2); + Assert.assertEquals(4, result); + jmethod = uber.getMethod(bulgroz, "list", "1", bulgroz); + result = jmethod.invoke(bulgroz, "1", bulgroz); + Assert.assertEquals(5, result); + jmethod = uber.getMethod(bulgroz, "list", "1", "2"); + result = jmethod.invoke(bulgroz, "1", "2"); + Assert.assertEquals(6, result); + jmethod = uber.getMethod(bulgroz, "list", bulgroz, bulgroz); + result = jmethod.invoke(bulgroz, bulgroz, bulgroz); + Assert.assertEquals(8, result); + jmethod = uber.getMethod(bulgroz, "list", bulgroz, 1, bulgroz); + result = jmethod.invoke(bulgroz, bulgroz, 1, bulgroz); + Assert.assertEquals(7, result); + jmethod = uber.getMethod(bulgroz, "list", bulgroz, 1, "1"); + result = jmethod.invoke(bulgroz, bulgroz, 1, "1"); + Assert.assertEquals(7, result); + jmethod = uber.getMethod(bulgroz, "list", (Object) null); + result = jmethod.invoke(bulgroz, (Object) null); + Assert.assertEquals(2, result); + jmethod = uber.getMethod(bulgroz, "list", bulgroz, (Object) null); + result = jmethod.invoke(bulgroz, bulgroz, (Object) null); + Assert.assertEquals(8, result); + jmethod = uber.getMethod(bulgroz, "list", null, "1"); + result = jmethod.invoke(bulgroz, null, "1"); + Assert.assertEquals(8, result); + jmethod = uber.getMethod(bulgroz, "list", bulgroz, null, null); + result = jmethod.invoke(bulgroz, bulgroz, null, null); + Assert.assertEquals(7, result); + + jmethod = uber.getMethod(bulgroz, "amb", Double.valueOf(3)); + Assert.assertNotNull(null, jmethod); + } }