Skip to content

Commit

Permalink
Tweak object wrapper reported members logic (#1956)
Browse files Browse the repository at this point in the history
  • Loading branch information
lahma authored Aug 26, 2024
1 parent 2f3a801 commit eb091eb
Show file tree
Hide file tree
Showing 5 changed files with 77 additions and 56 deletions.
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

0 comments on commit eb091eb

Please sign in to comment.