Improve function arguments parameter expression detection (#2060)

Jint.Benchmark/FunctionBenchmark.cs

@@ -0,0 +1,27 @@
+using BenchmarkDotNet.Attributes;
+
+namespace Jint.Benchmark;
+
+[MemoryDiagnoser]
+public class FunctionBenchmark
+{
+    private readonly Engine _engine;
+
+    public FunctionBenchmark()
+    {
+        _engine = new Engine();
+        _engine.Execute("function objectPattern({ toMessage: t, code: e, reasonCode: s, syntaxPlugin: r }) {  return \"MissingPlugin\" === s || \"MissingOneOfPlugins\" === s; }");
+    }
+
+    [Benchmark]
+    public bool ObjectPattern()
+    {
+        var b = true;
+        for (var i = 0; i < 100; ++i)
+        {
+            b &= _engine.Evaluate("objectPattern({\"reasonCode\": \"MissingPlugin\"})").AsBoolean();
+        }
+
+        return b;
+    }
+}

Jint.Tests/Runtime/Interpreter/JintFunctionDefinitionTest.cs

@@ -0,0 +1,22 @@
+using Jint.Runtime.Interpreter;
+
+namespace Jint.Tests.Runtime.Interpreter;
+
+public class JintFunctionDefinitionTest
+{
+    [Theory]
+    [InlineData("function f(_ = probeParams = function() { return 42; }) { }", true)]
+    [InlineData("function* g(_ = probeParams = function() { return 42; }) { }", true)]
+    [InlineData("function x(t = {}) {}", false)]
+    [InlineData("function x(e, t = {}) {}", false)]
+    [InlineData("function x([t, e]) { }", false)]
+    public void ShouldDetectParameterExpression(string functionCode, bool hasExpressions)
+    {
+        var parser = new Parser();
+        var script = parser.ParseScript(functionCode);
+        var function = (IFunction) script.Body.First();
+
+        var state = JintFunctionDefinition.BuildState(function);
+        state.HasParameterExpressions.Should().Be(hasExpressions);
+    }
+}

Jint/Runtime/Interpreter/JintFunctionDefinition.cs

@@ -285,10 +285,7 @@ internal sealed class JintFunctionDefinition
                 var n = state.VarNames[i];
                 if (instantiatedVarNames.Add(n))
                 {
-                    varsToInitialize.Add(new State.VariableValuePair
-                    {
-                        Name = n
-                    });
+                    varsToInitialize.Add(new State.VariableValuePair(Name: n, InitialValue: null));
                 }
             }
         }
@@ -309,11 +306,7 @@ internal sealed class JintFunctionDefinition
                         initialValue = JsValue.Undefined;
                     }
 
-                    varsToInitialize.Add(new State.VariableValuePair
-                    {
-                        Name = n,
-                        InitialValue = initialValue
-                    });
+                    varsToInitialize.Add(new State.VariableValuePair(Name: n, InitialValue: initialValue));
                 }
             }
         }
@@ -329,80 +322,86 @@ internal sealed class JintFunctionDefinition
     }
 
     private static void GetBoundNames(
-        Node? parameter,
+        Node parameter,
         List<Key> target,
         bool checkDuplicates,
-        ref bool _hasRestParameter,
-        ref bool _hasParameterExpressions,
-        ref bool _hasDuplicates,
+        ref bool hasRestParameter,
+        ref bool hasParameterExpressions,
+        ref bool hasDuplicates,
         ref bool hasArguments)
     {
-        if (parameter is Identifier identifier)
+        Start:
+        if (parameter.Type == NodeType.Identifier)
         {
-            _hasDuplicates |= checkDuplicates && target.Contains(identifier.Name);
-            target.Add(identifier.Name);
-            hasArguments |= string.Equals(identifier.Name, "arguments", StringComparison.Ordinal);
+            var key = (Key) ((Identifier) parameter).Name;
+            target.Add(key);
+            hasDuplicates |= checkDuplicates && target.Contains(key);
+            hasArguments |= key == KnownKeys.Arguments;
             return;
         }
 
         while (true)
         {
-            if (parameter is RestElement restElement)
+            if (parameter.Type == NodeType.RestElement)
             {
-                _hasRestParameter = true;
-                _hasParameterExpressions = true;
-                parameter = restElement.Argument;
+                hasRestParameter = true;
+                parameter = ((RestElement) parameter).Argument;
                 continue;
             }
 
-            if (parameter is ArrayPattern arrayPattern)
+            if (parameter.Type == NodeType.ArrayPattern)
             {
-                _hasParameterExpressions = true;
-                ref readonly var arrayPatternElements = ref arrayPattern.Elements;
-                for (var i = 0; i < arrayPatternElements.Count; i++)
+                foreach (var element in ((ArrayPattern) parameter).Elements.AsSpan())
                 {
-                    var expression = arrayPatternElements[i];
+                    if (element is null)
+                    {
+                        continue;
+                    }
+
+                    if (element.Type == NodeType.RestElement)
+                    {
+                        hasRestParameter = true;
+                        parameter = ((RestElement) element).Argument;
+                        goto Start;
+                    }
+
                     GetBoundNames(
-                        expression,
+                        element,
                         target,
                         checkDuplicates,
-                        ref _hasRestParameter,
-                        ref _hasParameterExpressions,
-                        ref _hasDuplicates,
+                        ref hasRestParameter,
+                        ref hasParameterExpressions,
+                        ref hasDuplicates,
                         ref hasArguments);
                 }
             }
-            else if (parameter is ObjectPattern objectPattern)
+            else if (parameter.Type == NodeType.ObjectPattern)
             {
-                _hasParameterExpressions = true;
-                ref readonly var objectPatternProperties = ref objectPattern.Properties;
-                for (var i = 0; i < objectPatternProperties.Count; i++)
+                foreach (var property in ((ObjectPattern) parameter).Properties.AsSpan())
                 {
-                    var property = objectPatternProperties[i];
-                    if (property is AssignmentProperty p)
-                    {
-                        GetBoundNames(
-                            p.Value,
-                            target,
-                            checkDuplicates,
-                            ref _hasRestParameter,
-                            ref _hasParameterExpressions,
-                            ref _hasDuplicates,
-                            ref hasArguments);
-                    }
-                    else
+                    if (property.Type == NodeType.RestElement)
                     {
-                        _hasRestParameter = true;
-                        _hasParameterExpressions = true;
+                        hasRestParameter = true;
                         parameter = ((RestElement) property).Argument;
-                        continue;
+                        goto Start;
                     }
+
+                    GetBoundNames(
+                        ((AssignmentProperty) property).Value,
+                        target,
+                        checkDuplicates,
+                        ref hasRestParameter,
+                        ref hasParameterExpressions,
+                        ref hasDuplicates,
+                        ref hasArguments);
                 }
             }
-            else if (parameter is AssignmentPattern assignmentPattern)
+            else if (parameter.Type == NodeType.AssignmentPattern)
             {
-                _hasParameterExpressions = true;
+                var assignmentPattern = (AssignmentPattern) parameter;
+                hasParameterExpressions |= ExpressionAstVisitor.HasExpression(assignmentPattern.ChildNodes);
                 parameter = assignmentPattern.Left;
+
                 continue;
             }
 
@@ -416,23 +415,22 @@ internal sealed class JintFunctionDefinition
         out bool hasArguments)
     {
         hasArguments = false;
-        state.IsSimpleParameterList  = true;
+        state.IsSimpleParameterList = true;
 
         var countParameters = true;
         ref readonly var functionDeclarationParams = ref function.Params;
         var count = functionDeclarationParams.Count;
         var parameterNames = new List<Key>(count);
-        for (var i = 0; i < count; i++)
+        foreach (var parameter in function.Params.AsSpan())
         {
-            var parameter = functionDeclarationParams[i];
             var type = parameter.Type;
 
             if (type == NodeType.Identifier)
             {
-                var id = (Identifier) parameter;
-                state.HasDuplicates |= parameterNames.Contains(id.Name);
-                hasArguments = string.Equals(id.Name, "arguments", StringComparison.Ordinal);
-                parameterNames.Add(id.Name);
+                var key = (Key) ((Identifier) parameter).Name;
+                state.HasDuplicates |= parameterNames.Contains(key);
+                hasArguments |= key == KnownKeys.Arguments;
+                parameterNames.Add(key);
             }
             else if (type != NodeType.Literal)
             {
@@ -466,10 +464,9 @@ internal sealed class JintFunctionDefinition
                 return true;
             }
 
-            ref readonly var parameters = ref function.Params;
-            for (var i = 0; i < parameters.Count; ++i)
+            foreach (var parameter in function.Params.AsSpan())
             {
-                if (HasArgumentsReference(parameters[i]))
+                if (HasArgumentsReference(parameter))
                 {
                     return true;
                 }
@@ -544,4 +541,37 @@ internal sealed class JintFunctionDefinition
             return false;
         }
     }
+
+    private static class ExpressionAstVisitor
+    {
+        internal static bool HasExpression(ChildNodes nodes)
+        {
+            foreach (var childNode in nodes)
+            {
+                switch (childNode.Type)
+                {
+                    case NodeType.ArrowFunctionExpression:
+                    case NodeType.FunctionExpression:
+                    case NodeType.CallExpression:
+                    case NodeType.AssignmentExpression:
+                        return true;
+                    case NodeType.Identifier:
+                    case NodeType.Literal:
+                        continue;
+                    default:
+                        if (!childNode.ChildNodes.IsEmpty())
+                        {
+                            if (HasExpression(childNode.ChildNodes))
+                            {
+                                return true;
+                            }
+                        }
+
+                        break;
+                }
+            }
+
+            return false;
+        }
+    }
 }