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

Tweak object wrapper reported members logic #1956

Merged
merged 1 commit into from
Aug 26, 2024
Merged
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
105 changes: 64 additions & 41 deletions Jint.Tests/Runtime/InteropTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3297,9 +3297,13 @@ private class Container
public BaseClass Get() => _child;
}

private class BaseClass { }
private class BaseClass
{
}

private class Child : BaseClass { }
private class Child : BaseClass
{
}

[Fact]
public void AccessingBaseTypeShouldBeEqualToAccessingDerivedType()
Expand Down Expand Up @@ -3328,6 +3332,7 @@ public interface IStringCollection : IIndexer<string>, ICountable<string>
public class Strings : IStringCollection
{
private readonly string[] _strings;

public Strings(string[] strings)
{
_strings = strings;
Expand All @@ -3340,7 +3345,7 @@ public Strings(string[] strings)

public class Utils
{
public IStringCollection GetStrings() => new Strings(new [] { "a", "b", "c" });
public IStringCollection GetStrings() => new Strings(new[] { "a", "b", "c" });
}

[Fact]
Expand Down Expand Up @@ -3384,6 +3389,7 @@ private class MetadataWrapper : IDictionary<string, object>
public bool ContainsKey(string key) => throw new NotImplementedException();
public void Add(string key, object value) => throw new NotImplementedException();
public bool Remove(string key) => throw new NotImplementedException();

public bool TryGetValue(string key, out object value)
{
value = "from-wrapper";
Expand Down Expand Up @@ -3467,6 +3473,7 @@ public void ShouldRespectConcreteGenericReturnTypes()
});

var result = new List<string>();

void Debug(object o)
{
result.Add($"{o?.GetType().Name ?? "null"}: {o ?? "null"}");
Expand All @@ -3487,102 +3494,126 @@ void Debug(object o)

private class ClrMembersVisibilityTestClass
{
public int A { get; set; } = 10;
public string Field = "field";

public int F()
public int Property { get; set; } = 10;

public int Method()
{
return 4;
}

public string Extras { get; set; }
}

[Fact]
public void ShouldNotSeeClrMethods()
public void PropertiesShouldNotSeeReportMethodsWhenMemberTypesActive()
{
var engine = new Engine(opt =>
{
opt.Interop.ObjectWrapperReportedMemberTypes = MemberTypes.Field | MemberTypes.Property;
});

engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass());

var val = engine.GetValue("clrInstance");

var obj = val.AsObject();
var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();

props.Should().BeEquivalentTo(["A"]);
var val = engine.GetValue("clrInstance");

var obj = val.AsObject();
var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();

props.Should().BeEquivalentTo("Property", "Extras", "Field");
}

[Fact]
public void ShouldSeeClrMethods()
public void PropertyKeysShouldReportMethods()
{
var engine = new Engine();


engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass());

var val = engine.GetValue("clrInstance");
var obj = val.AsObject();
var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();

props.Should().BeEquivalentTo("Property", "Extras", "Field", "Method");
}

[Fact]
public void PropertyKeysShouldObeyMemberFilter()
{
var engine = new Engine(options =>
{
options.SetTypeResolver(new TypeResolver
{
MemberFilter = member => member.Name == "Extras"
});
});

engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass());

var val = engine.GetValue("clrInstance");
var obj = val.AsObject();
var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();

props.Should().BeEquivalentTo(["A", "F"]);
props.Should().BeEquivalentTo("Extras");
}

private class ClrMembersVisibilityTestClass2
{
public int Get_A { get; set; } = 5;
}

[Fact]
public void ShouldSeeClrMethods2()
{
var engine = new Engine();

engine.SetValue("clrInstance", new ClrMembersVisibilityTestClass2());

var val = engine.GetValue("clrInstance");

var obj = val.AsObject();
var props = obj.GetOwnProperties().Select(x => x.Key.ToString()).ToList();
props.Should().BeEquivalentTo(["Get_A"]);

props.Should().BeEquivalentTo("Get_A");
}

[Fact]
public void ShouldNotThrowOnInspectingClrFunction()
{
var engine = new Engine();

engine.SetValue("clrDelegate", () => 4);

var val = engine.GetValue("clrDelegate");

var fn = val as Function;
var decl = fn!.FunctionDeclaration;

decl.Should().BeNull();
}

private class ShouldNotThrowOnInspectingClrFunctionTestClass
{
public int MyInt()
{
return 4;
}
}

[Fact]
public void ShouldNotThrowOnInspectingClrClassFunction()
{
var engine = new Engine();

engine.SetValue("clrCls", new ShouldNotThrowOnInspectingClrFunctionTestClass());

var val = engine.GetValue("clrCls");
var clrFn = val.Get("MyInt");

var fn = clrFn as Function;
var decl = fn!.FunctionDeclaration;

decl.Should().BeNull();
}

Expand All @@ -3592,13 +3623,5 @@ public void StringifyShouldIncludeInheritedFieldsAndProperties()
var engine = new Engine();
engine.SetValue("c", new Circle(12.34));
engine.Evaluate("JSON.stringify(c)").ToString().Should().Be("{\"Radius\":12.34,\"Color\":0,\"Id\":123}");


engine = new Engine(options =>
{
options.Interop.ObjectWrapperReportOnlyDeclaredMembers = true;
});
engine.SetValue("c", new Circle(12.34));
engine.Evaluate("JSON.stringify(c)").ToString().Should().Be("{\"Radius\":12.34}");
}
}
1 change: 0 additions & 1 deletion Jint/Native/Atomics/AtomicsInstance.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System.Numerics;
using System.Threading;
using Jint.Collections;
using Jint.Native.Object;
Expand Down
7 changes: 0 additions & 7 deletions Jint/Options.cs
Original file line number Diff line number Diff line change
Expand Up @@ -371,13 +371,6 @@ public class InteropOptions
/// All other values are ignored.
/// </summary>
public MemberTypes ObjectWrapperReportedMemberTypes { get; set; } = MemberTypes.Field | MemberTypes.Property | MemberTypes.Method;

/// <summary>
/// Whether object wrapper should only report members that are declared on the object type itself, not inherited members. Defaults to false.
/// This is different from JS logic where only object's own members are reported and not prototypes.
/// </summary>
/// <remarks>This configuration does not affect methods, only methods declared in type itself will be reported.</remarks>
public bool ObjectWrapperReportOnlyDeclaredMembers { get; set; }
}

public class ConstraintOptions
Expand Down
19 changes: 13 additions & 6 deletions Jint/Runtime/Interop/ObjectWrapper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -257,15 +257,16 @@ private IEnumerable<JsValue> EnumerateOwnPropertyKeys(Types types)

// we take public properties, fields and methods
var bindingFlags = BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public;
if (interopOptions.ObjectWrapperReportOnlyDeclaredMembers)
{
bindingFlags |= BindingFlags.DeclaredOnly;
}

if ((interopOptions.ObjectWrapperReportedMemberTypes & MemberTypes.Property) == MemberTypes.Property)
{
foreach (var p in ClrType.GetProperties(bindingFlags))
{
if (!interopOptions.TypeResolver.Filter(_engine, ClrType, p))
{
continue;
}

var indexParameters = p.GetIndexParameters();
if (indexParameters.Length == 0)
{
Expand All @@ -278,15 +279,21 @@ private IEnumerable<JsValue> EnumerateOwnPropertyKeys(Types types)
{
foreach (var f in ClrType.GetFields(bindingFlags))
{
if (!interopOptions.TypeResolver.Filter(_engine, ClrType, f))
{
continue;
}

yield return JsString.Create(f.Name);
}
}

if ((interopOptions.ObjectWrapperReportedMemberTypes & MemberTypes.Method) == MemberTypes.Method)
{
foreach (var m in ClrType.GetMethods(bindingFlags | BindingFlags.DeclaredOnly))
foreach (var m in ClrType.GetMethods(bindingFlags))
{
if (m.IsSpecialName)
// we won't report anything from base object as it would usually not be something to expect from JS perspective
if (m.DeclaringType == typeof(object) || m.IsSpecialName || !interopOptions.TypeResolver.Filter(_engine, ClrType, m))
{
continue;
}
Expand Down
1 change: 0 additions & 1 deletion Jint/Runtime/Interop/Reflection/DynamicObjectAccessor.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
using System.Dynamic;
using System.Reflection;
using Jint.Native;

#pragma warning disable IL2092
Expand Down