You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2019/07/31 08:57:08 UTC

[commons-jexl] branch master updated: JEXL-308: improve overload method selection, simplify most-applicable method selection, add tests Task #JEXL-308 - Improve overloaded method selection

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 <hb...@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);
+    }
 }