From b5e60f877ea2d919b4a1282be30f2819d7993e31 Mon Sep 17 00:00:00 2001 From: Roland Praml Date: Wed, 21 Aug 2024 14:57:15 +0200 Subject: [PATCH] Add support for covariant return types --- .../org/mozilla/javascript/JavaMembers.java | 20 ++-- .../tests/CovariantReturnTypeTest.java | 112 ++++++++++++++++++ 2 files changed, 121 insertions(+), 11 deletions(-) create mode 100644 tests/src/test/java/org/mozilla/javascript/tests/CovariantReturnTypeTest.java diff --git a/rhino/src/main/java/org/mozilla/javascript/JavaMembers.java b/rhino/src/main/java/org/mozilla/javascript/JavaMembers.java index cefb97100f..4df4892add 100644 --- a/rhino/src/main/java/org/mozilla/javascript/JavaMembers.java +++ b/rhino/src/main/java/org/mozilla/javascript/JavaMembers.java @@ -326,11 +326,8 @@ private void discoverAccessibleMethods( int mods = method.getModifiers(); if (isPublic(mods) || isProtected(mods) || includePrivate) { - MethodSignature sig = new MethodSignature(method); - if (!map.containsKey(sig)) { - if (includePrivate) method.trySetAccessible(); - map.put(sig, method); - } + if (includePrivate) method.trySetAccessible(); + registerMethod(map, method); } } Class[] interfaces = clazz.getInterfaces(); @@ -343,11 +340,7 @@ private void discoverAccessibleMethods( // Some security settings (i.e., applets) disallow // access to Class.getDeclaredMethods. Fall back to // Class.getMethods. - Method[] methods = clazz.getMethods(); - for (Method method : methods) { - MethodSignature sig = new MethodSignature(method); - if (!map.containsKey(sig)) map.put(sig, method); - } + discoverPublicMethods(clazz, map); break; // getMethods gets superclass methods, no // need to loop any more } @@ -387,7 +380,12 @@ void discoverPublicMethods(Class clazz, Map map) { static void registerMethod(Map map, Method method) { MethodSignature sig = new MethodSignature(method); // Array may contain methods with same signature but different return value! - if (!map.containsKey(sig)) { + Method existing = map.get(sig); + if (existing == null) { + map.put(sig, method); + } else if (existing.getReturnType().isAssignableFrom(method.getReturnType())) { + // if there are multiple methods with same name (which is allowed in bytecode, but not + // in JLS) we will take the best method map.put(sig, method); } } diff --git a/tests/src/test/java/org/mozilla/javascript/tests/CovariantReturnTypeTest.java b/tests/src/test/java/org/mozilla/javascript/tests/CovariantReturnTypeTest.java new file mode 100644 index 0000000000..34e1972733 --- /dev/null +++ b/tests/src/test/java/org/mozilla/javascript/tests/CovariantReturnTypeTest.java @@ -0,0 +1,112 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +package org.mozilla.javascript.tests; + +import org.junit.Test; +import org.mozilla.javascript.tools.shell.Global; + +/** + * If you inherit from an abstract class and narrow the return type, you will get both methods on + * Class.getMethods() and in the bytecode. + * + *

See explanation of {@link Class#getMethods()} There may be more than one method with a + * particular name and parameter types in a class because while the Java language forbids a class to + * declare multiple methods with the same signature but different return types, the Java virtual + * machine does not. + * + *

Note: It totally depends on the JVM, in which order the methods are returend, so we have a + * class here, that tries to simulate that 5 times to increase the probability that rhino will not + * detect at least one case as bean property + * + * @author Roland Praml + */ +public class CovariantReturnTypeTest { + + public abstract static class A { + public abstract Object getValue1(); + + public abstract Object getValue2(); + + public abstract Object getValue3(); + + public abstract Object getValue4(); + + public abstract Object getValue5(); + } + + public static class B extends A { + private Integer value1; + private Integer value2; + private Integer value3; + private Integer value4; + private Integer value5; + + // Note: Class B will inherit + @Override + public Integer getValue1() { + return value1; + } + + public void setValue1(Integer value1) { + this.value1 = value1; + } + + @Override + public Integer getValue2() { + return value2; + } + + public void setValue2(Integer value2) { + this.value2 = value2; + } + + @Override + public Integer getValue3() { + return value3; + } + + public void setValue3(Integer value3) { + this.value3 = value3; + } + + @Override + public Integer getValue4() { + return value4; + } + + public void setValue4(Integer value4) { + this.value4 = value4; + } + + @Override + public Integer getValue5() { + return value5; + } + + public void setValue5(Integer value5) { + this.value5 = value5; + } + } + + static final String LS = System.getProperty("line.separator"); + + @Test + public void checkIt() { + Utils.runWithAllOptimizationLevels( + cx -> { + final Global scope = new Global(); + scope.init(cx); + scope.put("obj", scope, new B()); + Object ret = + cx.evaluateString( + scope, + "obj.value1 = 1; obj.value2 = 2; obj.value3 = 3; obj.value4 = 4; obj.value5 = 5", + "myScript.js", + 1, + null); + return null; + }); + } +}