Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
garayx opened this issue Aug 8, 2024 · 1 comment · Fixed by #1938
Closed

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

garayx opened this issue Aug 8, 2024 · 1 comment · Fixed by #1938

Comments

@garayx
Copy link

garayx commented Aug 8, 2024

Version used

  • Started failing from version 3.1.1+
  • Worked on version 3.1.0

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

  • The test passes on version v3.1.0 but fails on version 3.1.1 and newer.
  • reference.ReferencedName is a number but was string previously.

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
Copy link
Collaborator

lahma commented Aug 8, 2024

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants