Introduce TypeResolver with member filter and name comparer

[lahma]

Setting a filter allows to reuse all resolution logic and wrapping and on top of that just filter with MemberInfo. Now also caching resolution per engine as options are per engine.

Also adding custom property name comparer.

fixes #275
fixes #342

[SebastianStehle]

Given the fact that engines cannot be reused for parallel access I think the caching per engine could cause a downgrade in performance. Perhaps we should get rid of the options class and use dedicated class instances for the different tasks. E.g. a "TypeResolver" or so, that is passed to the engine. Then you can decide how you want to do the caching.

[lahma]

I think that might get quite hard for end user to grasp how things affect performance based on instances provided. I don't know if there's a silver bullet here, unless we move the configuration to code (decorate members with attributes).

[SebastianStehle]

I think very often members are only accessed once and I am afraid that the cache has very little impact if it is not shared.

But lets say you define the options like this:

class TypeResolver {
  public static readonly TypeResolve Default = new TypeResolver();

  public static TypeResolver WithFiltering(Predicate<MemberInfo> filter)
  {
     return new FilteringTypeResolver { Filter = filter };
  }

  protected virtual bool ExposeMember(MemberInfo member) {
    return true;
  }
}

class FilteringTypeResolver : TypeResolver {
  public Predicate<MemberInfo> Filter { get; init; }

  protected virtual bool ExposeMember(MemberInfo member) {
    return Filter(member)
  }
}

class EngineOptions {
   public TypeResolver TypeResolver { get; set; } = TypeResolver.Default;
}

Then...

• You can easily override default behaviors.
• You do not have to configure anything.
• You can implement your own caching by reusing the type resolver in a way that fits to your use case.

[lahma]

I've pushed changes to add TypeResolver, this now combines the possibility to configure property name matcher from separate PR that I've closed in favor of this. TypeResolver is currently sealed as I think most can be achieved just by setting required properties.

[lahma]

@SebastianStehle thanks for your valuable input, I'd be happy to refine more if needed.

[SebastianStehle]

Lgtm

[luttje]

Looks great y'all! Is there a chance this could work with static members (exposed with TypeReference) as well? Or is that outside the scope of this PR / project?

For example, I expose a static Log class which has a static "Debug" method (to print messages):

public static class Log
{
    [ScriptAllowed] // With the proposed TypeResolver.MemberFilter I can allow this method to be accessed (awesome!)
    public static void Debug(string message)
        => UnityEngine.Debug.Log(message);
// ... etc

engine.SetValue("Log", TypeReference.CreateTypeReference(engine, typeof(Log)));

In JavaScript I wish to call (name comparison is already handled by the currently proposed TypeResolver.MemberNameComparer):

Log.debug("Hello World")

(instead of Log.Debug)

[lahma]

@luttje I've changed TypeReference to reuse the same resolution logic and configuration in latest commit. Included test case shows that TypeReference now also follows the same logic as object wrapping.

[luttje]

Awesome work @lahma, kudos! Great to see this picked up swiftly and implemented cleanly.

I don't want to seem demanding, but if you have some spare time would you mind publishing this to NuGet?

I tried using the MyGet feed, but that seems to be outdated. Or am I doing something wrong?

[lahma]

@luttje working on it. Unfortunately new GitHub Actions pipeline doesn't push to MyGet so NuGet is needed, Hopefully a new release won't take that long.

[lahma]

@luttje new releases should be now available on NuGet

[luttje]

Amazing work @lahma! I think I'm going to cry though, after all your effort I can't use Jint as is. I'm using Unity (currently on C# 8.0) and your use of the init keyword on the TypeResolver properties is not backwards compatible below C# 9.0 😭

[lahma]

I had no idea that there's some backwards usage restriction for such properties. My guess would have been compiler would ignore non-init usage. Usually compiler features do not reflect to actual IL. The init-only usage was to ensure nobody goes doing crazy stuff w.r.t. threading (setting properties outside of engine), but it's not mandatory. I'm still confused how this can cause trouble based on library version.

[luttje]

I think the logic is good, but for some reason Unity won't accept me setting the members.

Copying from this test in Unity gives this error:
CS1913: Member 'MemberNameComparer' cannot be initialized. It is not a field or property.

[luttje]

A smaller test-case without Unity and just a C# 8.0 Console Application yields a more explicit error (telling me to go use 9.0) when using the latest Jint pre-release.

Don't look at this ugly workaround for C# < 9.0 that works 😅

var typeResolver = new TypeResolver();
var type = typeResolver.GetType();

var memberNameProperty = type.GetProperty("MemberNameComparer");
memberNameProperty.SetValue(typeResolver, StringComparer.OrdinalIgnoreCase);

Predicate<MemberInfo> filter = member =>
{
    return !Attribute.IsDefined(member, typeof(ObsoleteAttribute));
};
var memberFilterProperty = type.GetProperty("MemberFilter");
memberFilterProperty.SetValue(typeResolver, filter);

engine = new Engine(options =>
{
    options.SetTypeResolver(typeResolver);
});

I can live with this until Unity upgrades to C# 9.0.

[lahma]

The issue with Unity/C# 8 has been now fixed in main. I think you've found a workaround for now, also make sure to share the typeResolver instance between engines if you want the best performance (type information can be cached).

[luttje]

Oh awesome. Thank you very much for your work! ✌️

I'll keep in mind to share the TypeResolver instance if I have more Engines.

[kkohno]

ok. it is allow to enable and disable whole property. And what about disabling only get or set method of property?