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

Fix Promise.all behavior under interop #911

Merged
merged 4 commits into from
Jun 9, 2021
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 7 additions & 0 deletions Jint.Tests/Runtime/InteropTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2556,5 +2556,12 @@ public void ObjectWrapperOverridingEquality()
Assert.Equal(1, _engine.Evaluate("arr.indexOf(b)").AsNumber());
Assert.True(_engine.Evaluate("arr.includes(b)").AsBoolean());
}

[Fact]
public void ObjectWrapperWrappingDictionaryShouldNotBeArrayLike()
{
var wrapper = new ObjectWrapper(_engine, new Dictionary<string, object>());
Assert.False(wrapper.IsArrayLike);
}
}
}
29 changes: 29 additions & 0 deletions Jint.Tests/Runtime/PromiseTests.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Collections.Generic;
using Jint.Native;
using Jint.Native.Promise;
using Jint.Runtime;
Expand Down Expand Up @@ -496,5 +497,33 @@ public void PromiseRaceMixturePromisesNoPromises_ResolvesCorrectly4()
}

#endregion

#region Regression

[Fact(Timeout = 5000)]
public void PromiseRegression_SingleElementArrayWithClrDictionaryInPromiseAll()
{
var engine = new Engine();
var dictionary = new Dictionary<string, object>
{
{ "Value 1", 1 },
{ "Value 2", "a string" }
};
engine.SetValue("clrDictionary", dictionary);

var resultAsObject = engine
.Evaluate(@"
const promiseArray = [clrDictionary];
return Promise.all(promiseArray);") // Returning and array through Promise.any()
.UnwrapIfPromise()
.ToObject();

var result = (object[]) resultAsObject;

Assert.Single(result);
Assert.IsType<Dictionary<string, object>>(result[0]);
}

#endregion
}
}
20 changes: 20 additions & 0 deletions Jint/Native/Array/ArrayConstructor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,26 @@ private ArrayInstance ConstructArrayFromIEnumerable(IEnumerable enumerable)
return jsArray;
}

public ArrayInstance ConstructFast(JsValue[] contents)
{
var instance = ConstructFast((ulong) contents.Length);
for (var i = 0; i < contents.Length; i++)
{
instance.SetIndexValue((uint) i, contents[i], updateLength: false);
}
return instance;
}

internal ArrayInstance ConstructFast(List<JsValue> contents)
{
var instance = ConstructFast((ulong) contents.Count);
for (var i = 0; i < contents.Count; i++)
{
instance.SetIndexValue((uint) i, contents[i], updateLength: false);
}
return instance;
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal ArrayInstance ConstructFast(ulong length)
{
Expand Down
11 changes: 1 addition & 10 deletions Jint/Native/Json/JsonParser.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.Collections.Generic;
using System.Globalization;
using System.Linq;
using Esprima;
using Esprima.Ast;
using Jint.Native.Object;
Expand Down Expand Up @@ -600,14 +599,6 @@ public Node PostProcess(Node node)
return node;
}

public ObjectInstance CreateArrayInstance(IEnumerable<JsValue> values)
{
var jsValues = values.ToArray();
var jsArray = _engine.Array.Construct(jsValues.Length);
_engine.Array.PrototypeObject.Push(jsArray, jsValues);
return jsArray;
}

// Throw an exception

private void ThrowError(Token token, string messageFormat, params object[] arguments)
Expand Down Expand Up @@ -693,7 +684,7 @@ private ObjectInstance ParseJsonArray()

Expect("]");

return CreateArrayInstance(elements);
return _engine.Array.ConstructFast(elements);
}

public ObjectInstance ParseJsonObject()
Expand Down
4 changes: 2 additions & 2 deletions Jint/Native/Object/ObjectConstructor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -254,14 +254,14 @@ public JsValue GetOwnPropertyNames(JsValue thisObject, JsValue[] arguments)
{
var o = TypeConverter.ToObject(_engine, arguments.At(0));
var names = o.GetOwnPropertyKeys(Types.String);
return _engine.Array.Construct(names.ToArray());
return _engine.Array.ConstructFast(names);
}

private JsValue GetOwnPropertySymbols(JsValue thisObject, JsValue[] arguments)
{
var o = TypeConverter.ToObject(_engine, arguments.At(0));
var keys = o.GetOwnPropertyKeys(Types.Symbol);
return _engine.Array.Construct(keys.ToArray());
return _engine.Array.ConstructFast(keys);
}

private JsValue Create(JsValue thisObject, JsValue[] arguments)
Expand Down
9 changes: 4 additions & 5 deletions Jint/Native/Object/ObjectInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1260,11 +1260,10 @@ internal ArrayInstance EnumerableOwnPropertyNames(EnumerableOwnPropertyNamesKind
}
else
{
array.SetIndexValue(index, _engine.Array.Construct(new[]
{
property,
value
}), updateLength: false);
var objectInstance = _engine.Array.ConstructFast(2);
objectInstance.SetIndexValue(0, property, updateLength: false);
objectInstance.SetIndexValue(1, value, updateLength: false);
array.SetIndexValue(index, objectInstance, updateLength: false);
}
}

Expand Down
5 changes: 2 additions & 3 deletions Jint/Native/Promise/PromiseConstructor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ internal sealed record PromiseCapability(
JsValue RejectObj
);


public sealed class PromiseConstructor : FunctionInstance, IConstructor
{
private static readonly JsString _functionName = new JsString("Promise");
Expand Down Expand Up @@ -230,8 +229,8 @@ void ResolveIfFinished()
// if "then" method is sync then it will be resolved BEFORE the next iteration cycle
if (results.TrueForAll(static x => x != null) && doneIterating)
{
resolve.Call(Undefined,
new JsValue[] {Engine.Array.Construct(results.ToArray())});
var array = _engine.Array.ConstructFast(results);
resolve.Call(Undefined, new JsValue[] { array });
}
}

Expand Down
2 changes: 1 addition & 1 deletion Jint/Native/Proxy/ProxyInstance.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class ProxyInstance : ObjectInstance, IConstructor, ICallable

public JsValue Call(JsValue thisObject, JsValue[] arguments)
{
var jsValues = new[] { _target, thisObject, _engine.Array.Construct(arguments) };
var jsValues = new[] { _target, thisObject, _engine.Array.ConstructFast(arguments) };
if (TryCallHandler(TrapApply, jsValues, out var result))
{
return result;
Expand Down
8 changes: 2 additions & 6 deletions Jint/Native/String/StringPrototype.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using System.Runtime.CompilerServices;
using System.Text;
using Jint.Collections;
using Jint.Native.Array;
using Jint.Native.Object;
using Jint.Native.RegExp;
using Jint.Native.Symbol;
Expand Down Expand Up @@ -335,7 +334,6 @@ private JsValue Split(JsValue thisObj, JsValue[] arguments)

// Coerce into a number, true will become 1
var lim = limit.IsUndefined() ? uint.MaxValue : TypeConverter.ToUint32(limit);
var len = s.Length;

if (lim == 0)
{
Expand All @@ -348,10 +346,8 @@ private JsValue Split(JsValue thisObj, JsValue[] arguments)
}
else if (separator.IsUndefined())
{
var jsValues = _engine._jsValueArrayPool.RentArray(1);
jsValues[0] = s;
var arrayInstance = (ArrayInstance)Engine.Array.Construct(jsValues);
_engine._jsValueArrayPool.ReturnArray(jsValues);
var arrayInstance = Engine.Array.ConstructFast(1);
arrayInstance.SetIndexValue(0, s, updateLength: false);
return arrayInstance;
}
else
Expand Down
6 changes: 6 additions & 0 deletions Jint/Runtime/Interop/TypeDescriptor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ public static TypeDescriptor Get(Type type)

private static bool DetermineIfObjectIsArrayLikeClrCollection(Type type)
{
if (typeof(IDictionary).IsAssignableFrom(type))
{
// dictionaries are considered normal-object-like
return false;
}

if (typeof(ICollection).IsAssignableFrom(type))
{
return true;
Expand Down