diff --git a/rhino/src/main/java/org/mozilla/javascript/JavaMembers.java b/rhino/src/main/java/org/mozilla/javascript/JavaMembers.java index 8f02fb63d6..2d50c0e220 100644 --- a/rhino/src/main/java/org/mozilla/javascript/JavaMembers.java +++ b/rhino/src/main/java/org/mozilla/javascript/JavaMembers.java @@ -727,28 +727,23 @@ private static MemberBox extractSetMethod( // instance of the target arg to determine that. // - // Make two passes: one to find a method with direct type assignment, - // and one to find a widening conversion. - for (int pass = 1; pass <= 2; ++pass) { - for (MemberBox method : methods) { - if (!isStatic || method.isStatic()) { - Class[] params = method.argTypes; - if (params.length == 1) { - if (pass == 1) { - if (params[0] == type) { - return method; - } - } else { - if (pass != 2) Kit.codeBug(); - if (params[0].isAssignableFrom(type)) { - return method; - } - } + MemberBox acceptableMatch = null; + for (MemberBox method : methods) { + if (!isStatic || method.isStatic()) { + Class[] params = method.argTypes; + if (params.length == 1) { + if (params[0] == type) { + // perfect match, no need to continue scanning + return method; + } + if (acceptableMatch == null && params[0].isAssignableFrom(type)) { + // do not return at this point, there can still be perfect match + acceptableMatch = method; } } } } - return null; + return acceptableMatch; } private static MemberBox extractSetMethod(MemberBox[] methods, boolean isStatic) { diff --git a/rhino/src/main/java/org/mozilla/javascript/NativeJavaMethod.java b/rhino/src/main/java/org/mozilla/javascript/NativeJavaMethod.java index 4c9112e9d8..d23343b588 100644 --- a/rhino/src/main/java/org/mozilla/javascript/NativeJavaMethod.java +++ b/rhino/src/main/java/org/mozilla/javascript/NativeJavaMethod.java @@ -265,144 +265,115 @@ int findCachedFunction(Context cx, Object[] args) { static int findFunction(Context cx, MemberBox[] methodsOrCtors, Object[] args) { if (methodsOrCtors.length == 0) { return -1; - } else if (methodsOrCtors.length == 1) { - MemberBox member = methodsOrCtors[0]; - Class[] argTypes = member.argTypes; - int alength = argTypes.length; - - if (member.vararg) { - alength--; - if (alength > args.length) { - return -1; - } - } else { - if (alength != args.length) { - return -1; - } - } - for (int j = 0; j != alength; ++j) { - if (!NativeJavaObject.canConvert(args[j], argTypes[j])) { - if (debug) printDebug("Rejecting (args can't convert) ", member, args); - return -1; - } + } + if (methodsOrCtors.length == 1) { + if (failFastConversionWeights(args, methodsOrCtors[0]) == null) { + return -1; } - if (debug) printDebug("Found ", member, args); + if (debug) printDebug("Found ", methodsOrCtors[0], args); return 0; } int firstBestFit = -1; + int[] firstBestFitWeights = null; + int[] extraBestFits = null; + int[][] extraBestFitWeights = null; int extraBestFitsCount = 0; search: for (int i = 0; i < methodsOrCtors.length; i++) { MemberBox member = methodsOrCtors[i]; - Class[] argTypes = member.argTypes; - int alength = argTypes.length; - if (member.vararg) { - alength--; - if (alength > args.length) { - continue search; - } - } else { - if (alength != args.length) { - continue search; - } - } - for (int j = 0; j < alength; j++) { - if (!NativeJavaObject.canConvert(args[j], argTypes[j])) { - if (debug) printDebug("Rejecting (args can't convert) ", member, args); - continue search; - } + + final var weights = failFastConversionWeights(args, member); + if (weights == null) { + continue search; } + if (firstBestFit < 0) { if (debug) printDebug("Found first applicable ", member, args); firstBestFit = i; - } else { - // Compare with all currently fit methods. - // The loop starts from -1 denoting firstBestFit and proceed - // until extraBestFitsCount to avoid extraBestFits allocation - // in the most common case of no ambiguity - int betterCount = 0; // number of times member was preferred over - // best fits - int worseCount = 0; // number of times best fits were preferred - // over member - for (int j = -1; j != extraBestFitsCount; ++j) { - int bestFitIndex; - if (j == -1) { - bestFitIndex = firstBestFit; - } else { - bestFitIndex = extraBestFits[j]; - } - MemberBox bestFit = methodsOrCtors[bestFitIndex]; - if (cx.hasFeature(Context.FEATURE_ENHANCED_JAVA_ACCESS) - && bestFit.isPublic() != member.isPublic()) { - // When FEATURE_ENHANCED_JAVA_ACCESS gives us access - // to non-public members, continue to prefer public - // methods in overloading - if (!bestFit.isPublic()) ++betterCount; - else ++worseCount; + firstBestFitWeights = weights; + continue search; + } + + // Compare with all currently fit methods. + // The loop starts from -1 denoting firstBestFit and proceed + // until extraBestFitsCount to avoid extraBestFits allocation + // in the most common case of no ambiguity + int betterCount = 0; // number of times member was preferred over + // best fits + int worseCount = 0; // number of times best fits were preferred + // over member + for (int j = -1; j != extraBestFitsCount; ++j) { + int bestFitIndex = j < 0 ? firstBestFit : extraBestFits[j]; + MemberBox bestFit = methodsOrCtors[bestFitIndex]; + int[] bestFitWeights = j < 0 ? firstBestFitWeights : extraBestFitWeights[j]; + if (cx.hasFeature(Context.FEATURE_ENHANCED_JAVA_ACCESS) + && bestFit.isPublic() != member.isPublic()) { + // When FEATURE_ENHANCED_JAVA_ACCESS gives us access + // to non-public members, continue to prefer public + // methods in overloading + if (!bestFit.isPublic()) ++betterCount; + else ++worseCount; + } else { + int preference = + preferSignature(args, member, weights, bestFit, bestFitWeights); + if (preference == PREFERENCE_AMBIGUOUS) { + break; + } else if (preference == PREFERENCE_FIRST_ARG) { + ++betterCount; + } else if (preference == PREFERENCE_SECOND_ARG) { + ++worseCount; } else { - int preference = - preferSignature( - args, - argTypes, - member.vararg, - bestFit.argTypes, - bestFit.vararg); - if (preference == PREFERENCE_AMBIGUOUS) { - break; - } else if (preference == PREFERENCE_FIRST_ARG) { - ++betterCount; - } else if (preference == PREFERENCE_SECOND_ARG) { - ++worseCount; - } else { - if (preference != PREFERENCE_EQUAL) Kit.codeBug(); - // This should not happen in theory - // but on some JVMs, Class.getMethods will return all + if (preference != PREFERENCE_EQUAL) Kit.codeBug(); + // This should not happen in theory + // but on some JVMs, Class.getMethods will return all + // static methods of the class hierarchy, even if + // a derived class's parameters match exactly. + // We want to call the derived class's method. + if (bestFit.isStatic() + && bestFit.getDeclaringClass() + .isAssignableFrom(member.getDeclaringClass())) { + // On some JVMs, Class.getMethods will return all // static methods of the class hierarchy, even if // a derived class's parameters match exactly. // We want to call the derived class's method. - if (bestFit.isStatic() - && bestFit.getDeclaringClass() - .isAssignableFrom(member.getDeclaringClass())) { - // On some JVMs, Class.getMethods will return all - // static methods of the class hierarchy, even if - // a derived class's parameters match exactly. - // We want to call the derived class's method. - if (debug) - printDebug("Substituting (overridden static)", member, args); - if (j == -1) { - firstBestFit = i; - } else { - extraBestFits[j] = i; - } + if (debug) printDebug("Substituting (overridden static)", member, args); + if (j == -1) { + firstBestFit = i; + firstBestFitWeights = weights; } else { - if (debug) - printDebug("Ignoring same signature member ", member, args); + extraBestFits[j] = i; + extraBestFitWeights[j] = weights; } - continue search; + } else { + if (debug) printDebug("Ignoring same signature member ", member, args); } + continue search; } } - if (betterCount == 1 + extraBestFitsCount) { - // member was preferred over all best fits - if (debug) printDebug("New first applicable ", member, args); - firstBestFit = i; - extraBestFitsCount = 0; - } else if (worseCount == 1 + extraBestFitsCount) { - // all best fits were preferred over member, ignore it - if (debug) printDebug("Rejecting (all current bests better) ", member, args); - } else { - // some ambiguity was present, add member to best fit set - if (debug) printDebug("Added to best fit set ", member, args); - if (extraBestFits == null) { - // Allocate maximum possible array - extraBestFits = new int[methodsOrCtors.length - 1]; - } - extraBestFits[extraBestFitsCount] = i; - ++extraBestFitsCount; + } + if (betterCount == 1 + extraBestFitsCount) { + // member was preferred over all best fits + if (debug) printDebug("New first applicable ", member, args); + firstBestFit = i; + firstBestFitWeights = weights; + extraBestFitsCount = 0; + } else if (worseCount == 1 + extraBestFitsCount) { + // all best fits were preferred over member, ignore it + if (debug) printDebug("Rejecting (all current bests better) ", member, args); + } else { + // some ambiguity was present, add member to best fit set + if (debug) printDebug("Added to best fit set ", member, args); + if (extraBestFits == null) { + // Allocate maximum possible array + extraBestFits = new int[methodsOrCtors.length - 1]; + extraBestFitWeights = new int[methodsOrCtors.length - 1][]; } + extraBestFits[extraBestFitsCount] = i; + extraBestFitWeights[extraBestFitsCount] = weights; + ++extraBestFitsCount; } } @@ -453,24 +424,40 @@ static int findFunction(Context cx, MemberBox[] methodsOrCtors, Object[] args) { private static final int PREFERENCE_AMBIGUOUS = 3; /** - * Determine which of two signatures is the closer fit. Returns one of PREFERENCE_EQUAL, - * PREFERENCE_FIRST_ARG, PREFERENCE_SECOND_ARG, or PREFERENCE_AMBIGUOUS. + * Determine which of two signatures is the closer fit. Returns one of {@link + * #PREFERENCE_EQUAL}, {@link #PREFERENCE_FIRST_ARG}, {@link #PREFERENCE_SECOND_ARG}, or {@link + * #PREFERENCE_AMBIGUOUS}. */ private static int preferSignature( - Object[] args, Class[] sig1, boolean vararg1, Class[] sig2, boolean vararg2) { + Object[] args, + MemberBox member1, + int[] computedWeights1, + MemberBox member2, + int[] computedWeights2) { + final var types1 = member1.argTypes; + final var types2 = member2.argTypes; + int totalPreference = 0; for (int j = 0; j < args.length; j++) { - Class type1 = vararg1 && j >= sig1.length ? sig1[sig1.length - 1] : sig1[j]; - Class type2 = vararg2 && j >= sig2.length ? sig2[sig2.length - 1] : sig2[j]; + final var type1 = + member1.vararg && j >= types1.length ? types1[types1.length - 1] : types1[j]; + final var type2 = + member2.vararg && j >= types2.length ? types2[types2.length - 1] : types2[j]; if (type1 == type2) { continue; } - Object arg = args[j]; + final var arg = args[j]; // Determine which of type1, type2 is easier to convert from arg. - int rank1 = NativeJavaObject.getConversionWeight(arg, type1); - int rank2 = NativeJavaObject.getConversionWeight(arg, type2); + final var rank1 = + j < computedWeights1.length + ? computedWeights1[j] + : NativeJavaObject.getConversionWeight(arg, type1); + final var rank2 = + j < computedWeights2.length + ? computedWeights2[j] + : NativeJavaObject.getConversionWeight(arg, type2); int preference; if (rank1 < rank2) { @@ -501,6 +488,44 @@ private static int preferSignature( return totalPreference; } + /** + * 1. {@code args} is too short for {@code member} calling -> return {@code null} + * + *

2. at least one arg cannot be converted -> return {@code null} + * + *

3. otherwise -> return an int array holding all computed conversion weights, whose length + * will be {@code args.length} for non-vararg member or {@code args.length-1} for vararg member + * + * @see NativeJavaObject#getConversionWeight(Object, Class) + * @see NativeJavaObject#canConvert(Object, Class) + */ + static int[] failFastConversionWeights(Object[] args, MemberBox member) { + final var argTypes = member.argTypes; + var typeLen = argTypes.length; + if (member.vararg) { + typeLen--; + if (typeLen > args.length) { + return null; + } + } else { + if (typeLen != args.length) { + return null; + } + } + final var weights = new int[typeLen]; + for (int i = 0; i < typeLen; i++) { + final var weight = NativeJavaObject.getConversionWeight(args[i], argTypes[i]); + if (weight >= NativeJavaObject.CONVERSION_NONE) { + if (debug) { + printDebug("Rejecting (args can't convert) ", member, args); + } + return null; + } + weights[i] = weight; + } + return weights; + } + private static final boolean debug = false; private static void printDebug(String msg, MemberBox member, Object[] args) { diff --git a/tests/src/test/java/org/mozilla/javascript/tests/NativeJavaMethodTest.java b/tests/src/test/java/org/mozilla/javascript/tests/NativeJavaMethodTest.java new file mode 100644 index 0000000000..6fa26a38ae --- /dev/null +++ b/tests/src/test/java/org/mozilla/javascript/tests/NativeJavaMethodTest.java @@ -0,0 +1,145 @@ +package org.mozilla.javascript.tests; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; +import org.mozilla.javascript.Context; +import org.mozilla.javascript.EvaluatorException; +import org.mozilla.javascript.ScriptableObject; + +/** + * @author ZZZank + */ +public class NativeJavaMethodTest { + + public static class MethodDummy { + public final List captured = new ArrayList<>(); + + public void f1(String s) { + captured.add("1"); + } + + public void f1(String s, int forOverload) { + captured.add("1.1"); + } + + public void f1(String s, String forOverload) { + captured.add("1.2"); + } + + public void f2(String s1, String s2) { + captured.add("2"); + } + + public void f2(String s1, String s2, String... sN) { + captured.add("2.N"); + } + + public void fN(String s1, String s2, String... sN) { + captured.add("N." + sN.length); + } + } + + private static void expect(List expected, String... lines) { + Utils.runWithAllModes( + cx -> { + final var methodDummy = new MethodDummy(); + final var scope = initContext(cx, methodDummy); + + cx.evaluateString( + scope, String.join("\n", lines), "NativeJavaMethodTest.js", 0, null); + + Assertions.assertEquals(expected, methodDummy.captured); + return null; + }); + } + + private static ScriptableObject initContext(Context cx, MethodDummy methodDummy) { + cx.setLanguageVersion(Context.VERSION_ES6); + final var scope = cx.initStandardObjects(); + ScriptableObject.putProperty(scope, "d", Context.javaToJS(methodDummy, scope)); + return scope; + } + + private static void expectException( + Class exception, String msgPrefix, String... lines) { + Utils.runWithAllModes( + cx -> { + final var methodDummy = new MethodDummy(); + final var scope = initContext(cx, methodDummy); + + final var ex = + Assertions.assertThrows( + exception, + () -> + cx.evaluateString( + scope, + String.join("\n", lines), + "NativeJavaMethodTest.js", + 0, + null)); + + Assertions.assertTrue( + ex.getMessage().startsWith(msgPrefix), + String.format( + "'%s' does not start with '%s'", ex.getMessage(), msgPrefix)); + + return null; + }); + } + + @Test + void fixedArg() { + expect(Arrays.asList("1", "2"), "d.f1('xxx');", "d.f2('x', 'y');"); + } + + @Test + void overload() { + expect( + Arrays.asList("1", "1.2", "1.1", "1.2"), + "d.f1('xxx');", + "d.f1('x', 'y');", + "d.f1('x', 3);", + "d.f1('x', '3');"); + } + + @Test + void varArg() { + expect( + Arrays.asList("N.0", "N.3", "N.2"), + "d.fN('x', 'y');", + "d.fN('x', 'y', 'extra1', 'extra2', 'extra3');", + "d.fN('x', 'y', 'extra1', 'extra2');"); + } + + @Test + void argsTooShortForFixedArg() { + expectException(EvaluatorException.class, "Can't find method", "d.f2('x');"); + } + + @Test + void argsTooShortForVarArg() { + expectException(EvaluatorException.class, "Can't find method", "d.fN('x');"); + } + + @Test + void varArgAndFixedArgPreference() { + expect( + Arrays.asList("2", "2.N", "2"), + "d.f2('x', 'y');", + "d.f2('x', 'y', 'overflow');", + "d.f2('y', 'x');"); + } + + @Test + void cursed() { + expect( + Arrays.asList("2.N", "2"), + "const dum = new Packages.org.mozilla.javascript.tests.NativeJavaMethodTest$MethodDummy();\n", + "dum.f2('x', 'y', 'overflow');", + "dum.f2('x', 'y');", + "d.captured.addAll(dum.captured)"); + } +}