From 8221064a683188ace8ffc2d2830ebf83fe8d6ace Mon Sep 17 00:00:00 2001 From: Tom Bruyneel Date: Sat, 24 Feb 2024 16:58:11 +0100 Subject: [PATCH] Issue 1768: make behaviour of SetValue consistent when using interfaces --- .../Runtime/InteropInterfaceExtendTests.cs | 40 ++++++++++++++++++- .../Runtime/InteropTests.MemberAccess.cs | 2 +- Jint/Engine.cs | 2 +- Jint/Native/Object/ObjectInstance.cs | 2 +- .../Expressions/JintCallExpression.cs | 18 +++++++-- 5 files changed, 56 insertions(+), 8 deletions(-) diff --git a/Jint.Tests/Runtime/InteropInterfaceExtendTests.cs b/Jint.Tests/Runtime/InteropInterfaceExtendTests.cs index fc50064581..88d594f6f4 100644 --- a/Jint.Tests/Runtime/InteropInterfaceExtendTests.cs +++ b/Jint.Tests/Runtime/InteropInterfaceExtendTests.cs @@ -33,6 +33,11 @@ public class CI1 : Super, I1 string I1.SubPropertySuperMethod { get; } = "I1.SubPropertySuperMethod"; + public string CI1Method() + { + return "CI1.CI1Method"; + } + public string OverloadSuperMethod() { return "I0.OverloadSuperMethod()"; @@ -93,6 +98,7 @@ public InterfaceHolder() public Indexer IndexerCI1 { get; } public Indexer IndexerI1 { get; } public Indexer IndexerSuper { get; } + } private readonly Engine _engine; @@ -109,7 +115,9 @@ public InterfaceTests() .SetValue("assert", new Action(Assert.True)) .SetValue("equal", new Action(Assert.Equal)) .SetValue("holder", holder) - ; + .SetValue("i1FromHolder", (I1)holder.CI1) // written explicitely to make the intent clear + .SetValue("ci1FromHolder", holder.CI1); // written explicitely to make the intent clear + ; } [Fact] @@ -146,6 +154,34 @@ public void CallSubPropertySuperMethod_SuperMethod() holder.I1.SubPropertySuperMethod(), _engine.Evaluate("holder.I1.SubPropertySuperMethod()")); }); - Assert.Equal("Property 'SubPropertySuperMethod' of object is not a function", ex.Message); + Assert.Equal("'Jint.Tests.Runtime.InterfaceTests+I1' does not contain a definition for 'SubPropertySuperMethod'", ex.Message); + } + + [Fact] + public void CallClassMethodsFromInterfaceInstance() + { + Assert.Equal( + holder.CI1.CI1Method(), + _engine.Evaluate("ci1FromHolder.CI1Method()")); + + Assert.Equal( + holder.CI1.CI1Method(), + _engine.Evaluate("holder.CI1.CI1Method()")); + + var exFromSetValue = Assert.Throws(() => + { + Assert.Equal( + holder.CI1.CI1Method(), + _engine.Evaluate("i1FromHolder.CI1Method()")); + }); + Assert.Equal("'Jint.Tests.Runtime.InterfaceTests+I1' does not contain a definition for 'CI1Method'", exFromSetValue.Message); + + var exFromInvokeReturn = Assert.Throws(() => + { + Assert.Equal( + holder.CI1.CI1Method(), + _engine.Evaluate("holder.I1.CI1Method()")); + }); + Assert.Equal("'Jint.Tests.Runtime.InterfaceTests+I1' does not contain a definition for 'CI1Method'", exFromInvokeReturn.Message); } } diff --git a/Jint.Tests/Runtime/InteropTests.MemberAccess.cs b/Jint.Tests/Runtime/InteropTests.MemberAccess.cs index 404ca08f1e..590a541675 100644 --- a/Jint.Tests/Runtime/InteropTests.MemberAccess.cs +++ b/Jint.Tests/Runtime/InteropTests.MemberAccess.cs @@ -108,7 +108,7 @@ static string TestAllowGetTypeOption(bool allowGetType) Assert.Equal("System.Uri", TestAllowGetTypeOption(allowGetType: true)); var ex = Assert.Throws(() => TestAllowGetTypeOption(allowGetType: false)); - Assert.Equal("Property 'GetType' of object is not a function", ex.Message); + Assert.Equal("'System.Uri' does not contain a definition for 'GetType'", ex.Message); } [Fact] diff --git a/Jint/Engine.cs b/Jint/Engine.cs index 98b6c80b50..452354dcc1 100644 --- a/Jint/Engine.cs +++ b/Jint/Engine.cs @@ -302,7 +302,7 @@ public Engine SetValue(string name, [DynamicallyAccessedMembers(InteropHelper.De { return obj is Type t ? SetValue(name, t) - : SetValue(name, JsValue.FromObject(this, obj)); + : SetValue(name, JsValue.FromObjectWithType(this, obj, typeof(T))); } internal void LeaveExecutionContext() diff --git a/Jint/Native/Object/ObjectInstance.cs b/Jint/Native/Object/ObjectInstance.cs index 87511bc354..1b40cf51ba 100644 --- a/Jint/Native/Object/ObjectInstance.cs +++ b/Jint/Native/Object/ObjectInstance.cs @@ -1406,7 +1406,7 @@ internal static JsObject OrdinaryObjectCreate(Engine engine, ObjectInstance? pro var callable = jsValue as ICallable; if (callable is null) { - ExceptionHelper.ThrowTypeError(realm, "Value returned for property '" + p + "' of object is not a function"); + ExceptionHelper.ThrowTypeError(realm, $"Value returned for property '" + p + "' of object is not a function"); } return callable; } diff --git a/Jint/Runtime/Interpreter/Expressions/JintCallExpression.cs b/Jint/Runtime/Interpreter/Expressions/JintCallExpression.cs index b7d791f47d..95a0666d57 100644 --- a/Jint/Runtime/Interpreter/Expressions/JintCallExpression.cs +++ b/Jint/Runtime/Interpreter/Expressions/JintCallExpression.cs @@ -6,6 +6,7 @@ using Jint.Native.Object; using Jint.Runtime.CallStack; using Jint.Runtime.Environments; +using Jint.Runtime.Interop; using Environment = Jint.Runtime.Environments.Environment; namespace Jint.Runtime.Interpreter.Expressions @@ -227,9 +228,20 @@ private static void ThrowReferenceNotFunction(Reference? referenceRecord1, objec [MethodImpl(MethodImplOptions.NoInlining)] private static void ThrowMemberIsNotFunction(Reference? referenceRecord1, object reference, Engine engine) { - var message = referenceRecord1 == null - ? reference + " is not a function" - : $"Property '{referenceRecord1.ReferencedName}' of object is not a function"; + string message = reference + " is not a function"; + + if (referenceRecord1 != null) + { + if (referenceRecord1.ThisValue is ObjectWrapper clrObject) + { + message = $"'{clrObject.ClrType}' does not contain a definition for '{referenceRecord1.ReferencedName}'"; + } + else + { + message = $"Property '{referenceRecord1.ReferencedName}' of object is not a function"; + } + } + ExceptionHelper.ThrowTypeError(engine.Realm, message); }