fix logic for obtaining TypeInfoFactory

Fix TypeInfoFactory used in FunctionObject to properly initialize using the scope instead of "this".

Author: ZZZank

rhino/build.gradle

@@ -89,6 +89,8 @@ decycle {
     ignoring from: "org.mozilla.javascript.ScriptableObject", to: "org.mozilla.javascript.lc.type.TypeInfoFactory"
     ignoring from: "org.mozilla.javascript.FunctionObject", to: "org.mozilla.javascript.lc.type.TypeInfo*"
     ignoring from: "org.mozilla.javascript.Context", to: "org.mozilla.javascript.lc.type.TypeInfo*"
+    ignoring from: "org.mozilla.javascript.ScriptRuntime", to: "org.mozilla.javascript.lc.type.TypeInfoFactory"
+    ignoring from: "org.mozilla.javascript.ScriptRuntime", to: "org.mozilla.javascript.lc.type.impl.factory.ConcurrentFactory"
 
     // currently accepted cycles (may be resolved, when it is clear, how to separate the liveconnect stuff)
     ignoring from: "org.mozilla.javascript.lc.type.TypeInfoFactory", to: "org.mozilla.javascript.lc.type.impl.factory.*"

rhino/src/main/java/org/mozilla/javascript/FunctionObject.java

@@ -80,7 +80,9 @@ public class FunctionObject extends BaseFunction {
      * @see org.mozilla.javascript.Scriptable
      */
     public FunctionObject(String name, Member methodOrConstructor, Scriptable scope) {
-        var typeInfoFactory = TypeInfoFactory.get(this);
+        // fallback to global factory for compatibility with old behaviour, where the `scope` can be
+        // an object not yet initialized via `initStandardObject(...)`
+        var typeInfoFactory = TypeInfoFactory.getOrElse(scope, TypeInfoFactory.GLOBAL);
 
         if (methodOrConstructor instanceof Constructor) {
             member = new MemberBox((Constructor<?>) methodOrConstructor, typeInfoFactory);

rhino/src/main/java/org/mozilla/javascript/ScriptRuntime.java

@@ -22,6 +22,7 @@
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
 import org.mozilla.javascript.ast.FunctionNode;
+import org.mozilla.javascript.lc.type.impl.factory.ConcurrentFactory;
 import org.mozilla.javascript.typedarrays.NativeArrayBuffer;
 import org.mozilla.javascript.typedarrays.NativeBigInt64Array;
 import org.mozilla.javascript.typedarrays.NativeBigUint64Array;
@@ -167,6 +168,7 @@ public static ScriptableObject initSafeStandardObjects(
 
         scope.associateValue(LIBRARY_SCOPE_KEY, scope);
         new ClassCache().associate(scope);
+        new ConcurrentFactory().associate(scope);
 
         LambdaConstructor function = BaseFunction.init(cx, scope, sealed);
         LambdaConstructor obj = NativeObject.init(cx, scope, sealed);

rhino/src/main/java/org/mozilla/javascript/ScriptableObject.java

@@ -1499,9 +1499,11 @@ public void defineProperty(String propertyName, Class<?> clazz, int attributes)
      */
     public void defineProperty(
             String propertyName, Object delegateTo, Method getter, Method setter, int attributes) {
+        var typeFactory = TypeInfoFactory.getOrElse(this, TypeInfoFactory.GLOBAL);
+
         MemberBox getterBox = null;
         if (getter != null) {
-            getterBox = new MemberBox(getter, TypeInfoFactory.get(this));
+            getterBox = new MemberBox(getter, typeFactory);
 
             boolean delegatedForm;
             if (!Modifier.isStatic(getter.getModifiers())) {
@@ -1542,7 +1544,7 @@ public void defineProperty(
             if (setter.getReturnType() != Void.TYPE)
                 throw Context.reportRuntimeErrorById("msg.setter.return", setter.toString());
 
-            setterBox = new MemberBox(setter, TypeInfoFactory.get(this));
+            setterBox = new MemberBox(setter, typeFactory);
 
             boolean delegatedForm;
             if (!Modifier.isStatic(setter.getModifiers())) {

rhino/src/main/java/org/mozilla/javascript/lc/type/TypeInfoFactory.java

@@ -21,7 +21,6 @@
 import java.util.function.Supplier;
 import org.mozilla.javascript.Scriptable;
 import org.mozilla.javascript.ScriptableObject;
-import org.mozilla.javascript.lc.type.impl.factory.ConcurrentFactory;
 import org.mozilla.javascript.lc.type.impl.factory.NoCacheFactory;
 import org.mozilla.javascript.lc.type.impl.factory.WeakReferenceFactory;
 
@@ -259,45 +258,52 @@ static TypeInfo matchPredefined(Class<?> clazz) {
     /**
      * Associate this TypeInfoFactory object with the given top-level scope.
      *
+     * <p>NOTE: If you're about associate a custom TypeInfoFactory to a scope, call this method
+     * before {@code initStandardObjects(...)} or {@code initSafeStandardObjects(...)}
+     *
      * @param topScope scope to associate this TypeInfoFactory object with.
-     * @return {@code true} if no previous TypeInfoFactory object were associated with the scope and
-     *     this TypeInfoFactory were successfully associated, false otherwise.
+     * @return {@code this} if no previous TypeInfoFactory object was associated with the scope and
+     *     this TypeInfoFactory is successfully associated, or the old associated factory otherwise.
      * @throws IllegalArgumentException if provided scope is not top scope
      * @see #get(Scriptable scope)
      */
-    default boolean associate(ScriptableObject topScope) {
+    default TypeInfoFactory associate(ScriptableObject topScope) {
         if (topScope.getParentScope() != null) {
             throw new IllegalArgumentException("provided scope not top scope");
         }
-        return this == topScope.associateValue("TypeInfoFactory", this);
+        return (TypeInfoFactory) topScope.associateValue("TypeInfoFactory", this);
     }
 
     /**
-     * Search for TypeInfoFactory in the given scope.If none was found, it will try to associate a
-     * new ClassCache object to the top scope.
+     * Search for TypeInfoFactory in the given scope.
      *
      * @param scope scope to search for TypeInfoFactory object.
-     * @return previously associated TypeInfoFactory object, or a new instance of TypeInfoFactory if
-     *     none was found
+     * @return previously associated TypeInfoFactory object.
      * @throws IllegalArgumentException if the top scope of provided scope have no associated
-     *     TypeInfoFactory, and cannot have TypeInfoFactory associated due to the top scope not
-     *     being a {@link ScriptableObject}
+     *     TypeInfoFactory.
      * @see #associate(ScriptableObject topScope)
      */
     static TypeInfoFactory get(Scriptable scope) {
-        TypeInfoFactory got =
-                (TypeInfoFactory) ScriptableObject.getTopScopeValue(scope, "TypeInfoFactory");
+        var got = getOrElse(scope, null);
         if (got == null) {
-            // we expect this to not happen frequently, so computing top scope twice is acceptable
-            var topScope = ScriptableObject.getTopLevelScope(scope);
-            if (!(topScope instanceof ScriptableObject)) {
-                // Note: it's originally a RuntimeException, the super class of
-                // IllegalArgumentException, so this will not break error catching
-                throw new IllegalArgumentException(
-                        "top scope have no associated TypeInfoFactory and cannot have TypeInfoFactory associated due to not being a ScriptableObject");
-            }
-            got = new ConcurrentFactory();
-            got.associate(((ScriptableObject) topScope));
+            throw new IllegalArgumentException("top scope have no associated TypeInfoFactory");
+        }
+        return got;
+    }
+
+    /**
+     * Search for TypeInfoFactory in the given scope. When none was found, {@code fallback} is
+     * returned instead
+     *
+     * @param scope scope to search for TypeInfoFactory object.
+     * @return previously associated TypeInfoFactory object, or {@code fallback} if none was found
+     * @see #get(Scriptable)
+     * @see #associate(ScriptableObject topScope)
+     */
+    static TypeInfoFactory getOrElse(Scriptable scope, TypeInfoFactory fallback) {
+        var got = (TypeInfoFactory) ScriptableObject.getTopScopeValue(scope, "TypeInfoFactory");
+        if (got == null) {
+            return fallback;
         }
         return got;
     }

tests/src/test/java/org/mozilla/javascript/tests/type_info/AlwaysFailFactory.java

@@ -0,0 +1,53 @@
+package org.mozilla.javascript.tests.type_info;
+
+import java.lang.reflect.GenericArrayType;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.TypeVariable;
+import java.lang.reflect.WildcardType;
+import java.util.List;
+import org.mozilla.javascript.lc.type.TypeInfo;
+import org.mozilla.javascript.lc.type.TypeInfoFactory;
+
+/**
+ * @author ZZZank
+ */
+enum AlwaysFailFactory implements TypeInfoFactory {
+    INSTANCE;
+
+    public static final String MESSAGE = "attempting to create TypeInfo on AlwaysFailFactory";
+
+    @Override
+    public TypeInfo create(Class<?> clazz) {
+        throw new AssertionError(MESSAGE);
+    }
+
+    @Override
+    public TypeInfo create(GenericArrayType genericArrayType) {
+        throw new AssertionError(MESSAGE);
+    }
+
+    @Override
+    public TypeInfo create(TypeVariable<?> typeVariable) {
+        throw new AssertionError(MESSAGE);
+    }
+
+    @Override
+    public TypeInfo create(ParameterizedType parameterizedType) {
+        throw new AssertionError(MESSAGE);
+    }
+
+    @Override
+    public TypeInfo create(WildcardType wildcardType) {
+        throw new AssertionError(MESSAGE);
+    }
+
+    @Override
+    public TypeInfo toArray(TypeInfo component) {
+        throw new AssertionError(MESSAGE);
+    }
+
+    @Override
+    public TypeInfo attachParam(TypeInfo base, List<TypeInfo> params) {
+        throw new AssertionError(MESSAGE);
+    }
+}

tests/src/test/java/org/mozilla/javascript/tests/type_info/CustomTypeInfoFactoryTest.java

@@ -1,10 +1,11 @@
 package org.mozilla.javascript.tests.type_info;
 
 import java.io.*;
+import java.lang.reflect.Modifier;
+import java.util.Arrays;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
-import org.mozilla.javascript.ContextFactory;
-import org.mozilla.javascript.ScriptableObject;
+import org.mozilla.javascript.*;
 import org.mozilla.javascript.lc.type.TypeInfoFactory;
 import org.mozilla.javascript.lc.type.impl.factory.ConcurrentFactory;
 
@@ -13,13 +14,63 @@
  */
 public class CustomTypeInfoFactoryTest {
 
+    /**
+     * @see #exampleFunctionObjectMethod(Context, Scriptable, Object[], Function)
+     * @see AlwaysFailFactory
+     */
     @Test
-    public void associate() throws Exception {
+    public void functionObject() {
+        var contextFactory = new ContextFactory();
+
+        try (var cx = contextFactory.enterContext()) {
+            var scope = new NativeObject();
+            AlwaysFailFactory.INSTANCE.associate(scope);
+            cx.initStandardObjects(scope);
+
+            var method =
+                    Arrays.stream(CustomTypeInfoFactoryTest.class.getDeclaredMethods())
+                            .filter(
+                                    m -> {
+                                        var mod = m.getModifiers();
+                                        return Modifier.isPublic(mod) && Modifier.isStatic(mod);
+                                    })
+                            .filter(m -> m.getName().equals("exampleFunctionObjectMethod"))
+                            .findFirst()
+                            .orElseThrow();
+            Assertions.assertThrowsExactly(
+                    AssertionError.class,
+                    () -> new FunctionObject("test", method, scope),
+                    AlwaysFailFactory.MESSAGE);
+        }
+    }
+
+    public static void exampleFunctionObjectMethod(
+            Context cx, Scriptable scope, Object[] args, Function fn) {
+        throw new AssertionError("method for test purpose only, do not invoke");
+    }
+
+    @Test
+    public void associateAfterInit() throws Exception {
         var contextFactory = new ContextFactory();
 
         try (var cx = contextFactory.enterContext()) {
             var scope = cx.initStandardObjects();
+
+            var toAssociate = AlwaysFailFactory.INSTANCE;
+            var associated = toAssociate.associate(scope);
+
+            Assertions.assertNotSame(toAssociate, associated);
+        }
+    }
+
+    @Test
+    public void associate() throws Exception {
+        var contextFactory = new ContextFactory();
+
+        try (var cx = contextFactory.enterContext()) {
+            var scope = new NativeObject();
             new NoGenericNoCacheFactory().associate(scope);
+            cx.initStandardObjects(scope);
 
             var typeFactory = TypeInfoFactory.get(scope);
 
@@ -45,8 +96,9 @@ public void serdeCustom() throws Exception {
         var contextFactory = new ContextFactory();
         byte[] data;
         try (var cx = contextFactory.enterContext()) {
-            var scope = cx.initStandardObjects();
+            var scope = new NativeObject();
             new NoGenericNoCacheFactory().associate(scope);
+            cx.initStandardObjects(scope);
 
             data = simulateSer(scope);
         }
@@ -64,8 +116,9 @@ public void serdeGlobal() throws Exception {
         var contextFactory = new ContextFactory();
         byte[] data;
         try (var cx = contextFactory.enterContext()) {
-            var scope = cx.initStandardObjects();
+            var scope = new NativeObject();
             TypeInfoFactory.GLOBAL.associate(scope);
+            cx.initStandardObjects(scope);
 
             data = simulateSer(scope);
         }

tests/src/test/java/org/mozilla/javascript/tests/type_info/NoGenericNoCacheFactory.java

@@ -3,7 +3,7 @@
 import java.lang.reflect.TypeVariable;
 import java.util.List;
 import java.util.Map;
-import java.util.function.Function;
+import org.mozilla.javascript.ScriptableObject;
 import org.mozilla.javascript.lc.type.TypeInfo;
 import org.mozilla.javascript.lc.type.TypeInfoFactory;
 import org.mozilla.javascript.lc.type.VariableTypeInfo;
@@ -13,9 +13,8 @@
 import org.mozilla.javascript.lc.type.impl.factory.FactoryBase;
 
 /**
- * {@link org.mozilla.javascript.lc.type.TypeInfoFactory} implementation with no cache, and no
- * generic support, as an example usage of custom type factory via {@link
- * org.mozilla.javascript.ContextFactory#setTypeFactoryProvider(Function)}
+ * {@link TypeInfoFactory} implementation with no cache, and no generic support, as an example usage
+ * of custom type factory via {@link TypeInfoFactory#associate(ScriptableObject)}
  *
  * @author ZZZank
  */