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

Issue 1768: make behaviour of SetValue consistent when using interfaces #1790

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions Jint.Tests/Runtime/InteropInterfaceExtendTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()";
Expand Down Expand Up @@ -93,6 +98,7 @@ public InterfaceHolder()
public Indexer<CI1> IndexerCI1 { get; }
public Indexer<I1> IndexerI1 { get; }
public Indexer<Super> IndexerSuper { get; }

}

private readonly Engine _engine;
Expand All @@ -109,7 +115,9 @@ public InterfaceTests()
.SetValue("assert", new Action<bool>(Assert.True))
.SetValue("equal", new Action<object, object>(Assert.Equal))
.SetValue("holder", holder)
;
.SetValue<I1>("i1FromHolder", (I1)holder.CI1) // written explicitely to make the intent clear
.SetValue<CI1>("ci1FromHolder", holder.CI1); // written explicitely to make the intent clear
;
}

[Fact]
Expand Down Expand Up @@ -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<JavaScriptException>(() =>
{
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<JavaScriptException>(() =>
{
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);
}
}
2 changes: 1 addition & 1 deletion Jint.Tests/Runtime/InteropTests.MemberAccess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ static string TestAllowGetTypeOption(bool allowGetType)
Assert.Equal("System.Uri", TestAllowGetTypeOption(allowGetType: true));

var ex = Assert.Throws<JavaScriptException>(() => 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]
Expand Down
2 changes: 1 addition & 1 deletion Jint/Engine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion Jint/Native/Object/ObjectInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
18 changes: 15 additions & 3 deletions Jint/Runtime/Interpreter/Expressions/JintCallExpression.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
}

Expand Down
Loading