sebastienros / jint

Javascript Interpreter for .NET
BSD 2-Clause "Simplified" License
4.04k stars 558 forks source link

reference.ReferencedName type is number but was string in older versions #1936

Closed garayx closed 1 month ago

garayx commented 1 month ago

Version used

Describe the bug

The type of reference.ReferencedName is a number in TryUnresolvableReference(Engine engine, Reference reference, out JsValue value) method in custom implementation of IReferenceResolver. Previously it was a string.

To Reproduce

    [Fact]
    public void ShouldWork123()
    {
        var engine = new Engine(options =>
        {
#if !NET8_0_OR_GREATER
            // Jint doesn't know about the types statically as they are not part of the out-of-the-box experience
            options.AddObjectConverter(SystemTextJsonValueConverter.Instance);
#endif

            options.ReferenceResolver = new MyReferenceResolver();
        });

        engine
            .Execute("""
                          function output(doc) {
                         var rows123 = [{}];
                     var test = null;
                         return { 
                             Rows : [{}].map(row=>({row:row, myRows:test.filter(x=>x)
                             })).map(__rvn4=>({
                                 Custom:__rvn4.myRows[0].Custom // this will throw
                                 // Custom:__rvn4.myRows // this will be null
                                 }))
                                 };
                     }
                     """);

        var result = engine.Evaluate("output()");

        var rows = result.AsObject()["Rows"];
        var custom = rows.AsArray()[0].AsObject()["Custom"];
        Assert.Equal(JsValue.Null, custom);
    }

    public  class MyReferenceResolver : IReferenceResolver
    {
        public bool TryUnresolvableReference(Engine engine, Reference reference, out JsValue value)
        {
            JsValue referencedName = reference.ReferencedName;

            // referencedName.IsString() was true prior to version 3.1.1
            // become false in 3.1.1+
            if (referencedName.IsString())
            {
                value = reference.IsPropertyReference ? JsValue.Undefined : JsValue.Null;
                return true;
            }

            //Assert.True(referencedName.IsString(), "Should not reach here");

            // referencedName.IsNumber() was false in version 3.1.0 and lower
            // referencedName.IsNumber() is true for version 3.1.1+
            //if (referencedName.IsNumber())
            //{
            //    value = reference.IsPropertyReference ? JsValue.Undefined : JsValue.Null;
            //    return true;
            //}

            value = JsValue.Null;
            return false;
        }

        public virtual bool TryPropertyReference(Engine engine, Reference reference, ref JsValue value)
        {
            return value.IsNull() || value.IsUndefined();
        }

        public bool TryGetCallable(Engine engine, object callee, out JsValue value)
        {
            value = new ClrFunction(engine, "function", static (_, _) => JsValue.Undefined);
            return true;
        }

        public bool CheckCoercible(JsValue value)
        {
            return true;
        }
    }

Expected behavior

Additional context

Exception:


Jint.Runtime.JavaScriptException
0 is not defined
   at Jint.Engine.ScriptEvaluation(ScriptRecord scriptRecord, ParserOptions parserOptions) in C:\Work\Projects\jint3x\Jint\Engine.cs:line 457
   at Jint.Engine.<>c__DisplayClass96_0.<Execute>b__0() in C:\Work\Projects\jint3x\Jint\Engine.cs:line 411
   at Jint.Engine.ExecuteWithConstraints[T](Boolean strict, Func`1 callback) in C:\Work\Projects\jint3x\Jint\Engine.cs:line 822
   at Jint.Engine.Execute(Prepared`1& preparedScript) in C:\Work\Projects\jint3x\Jint\Engine.cs:line 411
   at Jint.Engine.Evaluate(Prepared`1& preparedScript) in C:\Work\Projects\jint3x\Jint\Engine.cs:line 371
   at Jint.Engine.Evaluate(String code, String source) in C:\Work\Projects\jint3x\Jint\Engine.cs:line 348
   at Jint.Tests.PublicInterface.InteropTests.ShouldWork123() in C:\Work\Projects\jint3x\Jint.Tests.PublicInterface\InteropTests.SystemTextJson.cs:line 40
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Void** arguments, Signature sig, Boolean isConstructor)
   at System.Reflection.MethodBaseInvoker.InvokeWithNoArgs(Object obj, BindingFlags invokeAttr)

Jint.Runtime.JavaScriptException+JavaScriptErrorWrapperException
0 is not defined
   at map <anonymous>:7:20
   at output (doc) <anonymous>:6:16
   at <anonymous>:1:1
lahma commented 1 month ago

Without testing and digging deeper, isn't it beneficial to know that it's a number which allows to optimize expected behavior if targeting indexers for example? Would it be enough for your to check both number and string?