From 8078b4a7984f8b0d3aea391f0ca8e69fc92a86ea Mon Sep 17 00:00:00 2001 From: Marko Lahma Date: Sat, 16 Mar 2024 11:21:40 +0200 Subject: [PATCH] Create separate ObjectWrapper.Create factory method (#1810) --- Directory.Packages.props | 2 +- Jint.Benchmark/ListInteropBenchmark.cs | 4 ++-- .../InteropTests.SystemTextJson.cs | 11 +++++------ Jint.Tests/Runtime/InteropTests.cs | 6 +++--- Jint.Tests/Runtime/ModuleTests.cs | 2 +- Jint.Tests/Runtime/PropertyDescriptorTests.cs | 2 +- Jint/Options.cs | 4 ++-- Jint/Runtime/Interop/ClrHelper.cs | 4 ++-- Jint/Runtime/Interop/DefaultTypeConverter.cs | 2 +- Jint/Runtime/Interop/IObjectWrapper.cs | 9 ++++----- Jint/Runtime/Interop/ObjectWrapper.cs | 9 +++++++++ Jint/Runtime/Interop/TypeReference.cs | 2 +- 12 files changed, 32 insertions(+), 25 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index ac06b5d118..70e4f1ed13 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -21,7 +21,7 @@ - + diff --git a/Jint.Benchmark/ListInteropBenchmark.cs b/Jint.Benchmark/ListInteropBenchmark.cs index 3ff8375089..51533308b8 100644 --- a/Jint.Benchmark/ListInteropBenchmark.cs +++ b/Jint.Benchmark/ListInteropBenchmark.cs @@ -49,8 +49,8 @@ public void Setup() options .SetWrapObjectHandler((engine, target, type) => { - var instance = new ObjectWrapper(engine, target); - var isArrayLike = IsArrayLike(instance.Target.GetType()); + var instance = ObjectWrapper.Create(engine, target); + var isArrayLike = IsArrayLike(target.GetType()); if (isArrayLike) { instance.Prototype = engine.Intrinsics.Array.PrototypeObject; diff --git a/Jint.Tests.PublicInterface/InteropTests.SystemTextJson.cs b/Jint.Tests.PublicInterface/InteropTests.SystemTextJson.cs index ac97e4cf50..ef36ef549b 100644 --- a/Jint.Tests.PublicInterface/InteropTests.SystemTextJson.cs +++ b/Jint.Tests.PublicInterface/InteropTests.SystemTextJson.cs @@ -3,10 +3,10 @@ using Jint.Native; using Jint.Runtime.Interop; using System.Text.Json; + namespace Jint.Tests.PublicInterface; -#if NET8_0_OR_GREATER -public class TestJsonValueConverter : IObjectConverter +public sealed class SystemTextJsonValueConverter : IObjectConverter { public bool TryConvert(Engine engine, object value, out JsValue result) { @@ -95,15 +95,15 @@ public void AccessingJsonNodeShouldWork() { if (target is JsonArray) { - var wrapped = new ObjectWrapper(e, target); + var wrapped = ObjectWrapper.Create(e, target); wrapped.Prototype = e.Intrinsics.Array.PrototypeObject; return wrapped; } - return new ObjectWrapper(e, target); + return ObjectWrapper.Create(e, target); }; - options.AddObjectConverter(new TestJsonValueConverter()); + options.AddObjectConverter(new SystemTextJsonValueConverter()); // we cannot access this[string] with anything else than JsonObject, otherwise itw will throw options.Interop.TypeResolver = new TypeResolver { @@ -193,4 +193,3 @@ function populateFullName() { Assert.True(engine.Evaluate("variables.employees.other == 'def'").AsBoolean()); } } -#endif diff --git a/Jint.Tests/Runtime/InteropTests.cs b/Jint.Tests/Runtime/InteropTests.cs index 40aa14be1c..708a5afbc6 100644 --- a/Jint.Tests/Runtime/InteropTests.cs +++ b/Jint.Tests/Runtime/InteropTests.cs @@ -850,7 +850,7 @@ public void CanAddArrayPrototypeForArrayLikeClrObjects() .AllowClr(typeof(Person).Assembly) .SetWrapObjectHandler((engine, target, type) => { - var instance = new ObjectWrapper(engine, target); + var instance = ObjectWrapper.Create(engine, target); if (instance.IsArrayLike) { instance.SetPrototypeOf(engine.Realm.Intrinsics.Array.PrototypeObject); @@ -885,7 +885,7 @@ public void CanSetIsConcatSpreadableForArrays() { opt.SetWrapObjectHandler((eng, obj, type) => { - var wrapper = new ObjectWrapper(eng, obj); + var wrapper = ObjectWrapper.Create(eng, obj); if (wrapper.IsArrayLike) { wrapper.SetPrototypeOf(eng.Realm.Intrinsics.Array.PrototypeObject); @@ -2633,7 +2633,7 @@ public void ObjectWrapperOverridingEquality() [Fact] public void ObjectWrapperWrappingDictionaryShouldNotBeArrayLike() { - var wrapper = new ObjectWrapper(_engine, new Dictionary()); + var wrapper = ObjectWrapper.Create(_engine, new Dictionary()); Assert.False(wrapper.IsArrayLike); } diff --git a/Jint.Tests/Runtime/ModuleTests.cs b/Jint.Tests/Runtime/ModuleTests.cs index 9807b47066..260df9c71a 100644 --- a/Jint.Tests/Runtime/ModuleTests.cs +++ b/Jint.Tests/Runtime/ModuleTests.cs @@ -631,7 +631,7 @@ public void EngineShouldTransmitSourceModuleForModuleLoader() } public class ModuleLoaderForEngineShouldTransmitSourceModuleForModuleLoaderTest : ModuleLoader { - public override ResolvedSpecifier Resolve(string? referencingModuleLocation, ModuleRequest moduleRequest) + public override ResolvedSpecifier Resolve(string referencingModuleLocation, ModuleRequest moduleRequest) { var moduleSpec = moduleRequest.Specifier; diff --git a/Jint.Tests/Runtime/PropertyDescriptorTests.cs b/Jint.Tests/Runtime/PropertyDescriptorTests.cs index 41329a674f..42bb553524 100644 --- a/Jint.Tests/Runtime/PropertyDescriptorTests.cs +++ b/Jint.Tests/Runtime/PropertyDescriptorTests.cs @@ -167,7 +167,7 @@ public void ClrAccessDescriptor() JsValue ExtractClrAccessDescriptor(JsValue jsArugments) { var pd = ((JsArguments) jsArugments).ParameterMap.GetOwnProperty("0"); - return new ObjectWrapper(_engine, pd); + return ObjectWrapper.Create(_engine, pd); } _engine.SetValue("ExtractClrAccessDescriptor", ExtractClrAccessDescriptor); var pdobj = _engine.Evaluate(""" diff --git a/Jint/Options.cs b/Jint/Options.cs index c786d53399..546b0ab260 100644 --- a/Jint/Options.cs +++ b/Jint/Options.cs @@ -125,7 +125,7 @@ internal void Apply(Engine engine) new NamespaceReference(engine, TypeConverter.ToString(arguments.At(0)))), PropertyFlag.AllForbidden)); - engine.Realm.GlobalObject.SetProperty("clrHelper", new PropertyDescriptor(new ObjectWrapper(engine, new ClrHelper(Interop)), PropertyFlag.AllForbidden)); + engine.Realm.GlobalObject.SetProperty("clrHelper", new PropertyDescriptor(ObjectWrapper.Create(engine, new ClrHelper(Interop)), PropertyFlag.AllForbidden)); #pragma warning restore IL2026 } @@ -291,7 +291,7 @@ public class InteropOptions /// ObjectInstance using class ObjectWrapper. This function can be used to /// change the behavior. /// - public WrapObjectDelegate WrapObjectHandler { get; set; } = static (engine, target, type) => new ObjectWrapper(engine, target, type); + public WrapObjectDelegate WrapObjectHandler { get; set; } = static (engine, target, type) => ObjectWrapper.Create(engine, target, type); /// /// diff --git a/Jint/Runtime/Interop/ClrHelper.cs b/Jint/Runtime/Interop/ClrHelper.cs index 693350d859..05ee1db80a 100644 --- a/Jint/Runtime/Interop/ClrHelper.cs +++ b/Jint/Runtime/Interop/ClrHelper.cs @@ -30,7 +30,7 @@ public JsValue ToString(JsValue value) public JsValue Unwrap(ObjectWrapper obj) #pragma warning restore CA1822 { - return new ObjectWrapper(obj.Engine, obj.Target); + return ObjectWrapper.Create(obj.Engine, obj.Target); } /// @@ -44,7 +44,7 @@ public JsValue Wrap(ObjectWrapper obj, TypeReference type) { ExceptionHelper.ThrowTypeError(type.Engine.Realm, "Argument obj must be an instance of type"); } - return new ObjectWrapper(obj.Engine, obj.Target, type.ReferenceType); + return ObjectWrapper.Create(obj.Engine, obj.Target, type.ReferenceType); } /// diff --git a/Jint/Runtime/Interop/DefaultTypeConverter.cs b/Jint/Runtime/Interop/DefaultTypeConverter.cs index b60bd971a1..b530a9d387 100644 --- a/Jint/Runtime/Interop/DefaultTypeConverter.cs +++ b/Jint/Runtime/Interop/DefaultTypeConverter.cs @@ -395,7 +395,7 @@ internal static class ObjectExtensions public static void SetHiddenClrObjectProperty(this ObjectInstance obj, string name, object value) { - obj.SetOwnProperty(name, new PropertyDescriptor(new ObjectWrapper(obj.Engine, value), PropertyFlag.AllForbidden)); + obj.SetOwnProperty(name, new PropertyDescriptor(ObjectWrapper.Create(obj.Engine, value), PropertyFlag.AllForbidden)); } } } diff --git a/Jint/Runtime/Interop/IObjectWrapper.cs b/Jint/Runtime/Interop/IObjectWrapper.cs index cdc88ede8e..9d6c03567a 100644 --- a/Jint/Runtime/Interop/IObjectWrapper.cs +++ b/Jint/Runtime/Interop/IObjectWrapper.cs @@ -1,7 +1,6 @@ -namespace Jint.Runtime.Interop +namespace Jint.Runtime.Interop; + +public interface IObjectWrapper { - public interface IObjectWrapper - { - object Target { get; } - } + object Target { get; } } diff --git a/Jint/Runtime/Interop/ObjectWrapper.cs b/Jint/Runtime/Interop/ObjectWrapper.cs index 73a76d58f3..17b05c6100 100644 --- a/Jint/Runtime/Interop/ObjectWrapper.cs +++ b/Jint/Runtime/Interop/ObjectWrapper.cs @@ -21,6 +21,7 @@ public sealed class ObjectWrapper : ObjectInstance, IObjectWrapper, IEquatable + /// Creates a new object wrapper for given object instance and exposed type. + /// + public static ObjectInstance Create(Engine engine, object obj, Type? type = null) +#pragma warning disable CS0618 // Type or member is obsolete + => new ObjectWrapper(engine, obj, type); +#pragma warning restore CS0618 // Type or member is obsolete + public object Target { get; } internal Type ClrType { get; } diff --git a/Jint/Runtime/Interop/TypeReference.cs b/Jint/Runtime/Interop/TypeReference.cs index bc6dc74cd5..a48a43bf38 100644 --- a/Jint/Runtime/Interop/TypeReference.cs +++ b/Jint/Runtime/Interop/TypeReference.cs @@ -356,7 +356,7 @@ private static JsBoolean HasInstance(JsValue thisObject, JsValue[] arguments) var derivedType = other switch { - ObjectWrapper wrapper => wrapper.Target.GetType(), + IObjectWrapper wrapper => wrapper.Target.GetType(), TypeReferencePrototype otherTypeReference => otherTypeReference.TypeReference.ReferenceType, _ => null };