Replace some string usage with Key when suitable (#1737)

Jint/Engine.Ast.cs

@@ -88,14 +88,14 @@ internal sealed class CachedHoistingScope
     {
         Scope = HoistingScope.GetProgramLevelDeclarations(program);
 
-        VarNames = new List<string>();
+        VarNames = new List<Key>();
         GatherVarNames(Scope, VarNames);
 
         LexNames = new List<CachedLexicalName>();
         GatherLexNames(Scope, LexNames);
     }
 
-    internal static void GatherVarNames(HoistingScope scope, List<string> boundNames)
+    internal static void GatherVarNames(HoistingScope scope, List<Key> boundNames)
     {
         var varDeclarations = scope._variablesDeclarations;
         if (varDeclarations != null)
@@ -113,15 +113,15 @@ internal sealed class CachedHoistingScope
         var lexDeclarations = scope._lexicalDeclarations;
         if (lexDeclarations != null)
         {
-            var temp = new List<string>();
+            var temp = new List<Key>();
             for (var i = 0; i < lexDeclarations.Count; i++)
             {
                 var d = lexDeclarations[i];
                 temp.Clear();
                 d.GetBoundNames(temp);
-                foreach (var name in temp)
+                for (var j = 0; j < temp.Count; j++)
                 {
-                    boundNames.Add(new CachedLexicalName(name, d.IsConstantDeclaration()));
+                    boundNames.Add(new CachedLexicalName(temp[j], d.IsConstantDeclaration()));
                 }
             }
         }
@@ -131,7 +131,7 @@ internal sealed class CachedHoistingScope
     internal readonly record struct CachedLexicalName(Key Name, bool Constant);
 
     public HoistingScope Scope { get; }
-    public List<string> VarNames { get; }
+    public List<Key> VarNames { get; }
     public List<CachedLexicalName> LexNames { get; }
 }
 
@@ -142,16 +142,16 @@ internal static class AstPreparationExtensions
         return program.AssociatedData is CachedHoistingScope cached ? cached.Scope : HoistingScope.GetProgramLevelDeclarations(program);
     }
 
-    internal static List<string> GetVarNames(this Program program, HoistingScope hoistingScope)
+    internal static List<Key> GetVarNames(this Program program, HoistingScope hoistingScope)
     {
-        List<string> boundNames;
+        List<Key> boundNames;
         if (program.AssociatedData is CachedHoistingScope cached)
         {
             boundNames = cached.VarNames;
         }
         else
         {
-            boundNames = new List<string>();
+            boundNames = new List<Key>();
             CachedHoistingScope.GatherVarNames(hoistingScope, boundNames);
         }
 

Jint/Engine.cs

@@ -945,7 +945,7 @@ namespace Jint
 
             var functionToInitialize = new List<JintFunctionDefinition>();
             var declaredFunctionNames = new HashSet<Key>();
-            var declaredVarNames = new List<string>();
+            var declaredVarNames = new List<Key>();
 
             var realm = Realm;
 
@@ -972,7 +972,7 @@ namespace Jint
             var varNames = script.GetVarNames(hoistingScope);
             for (var j = 0; j < varNames.Count; j++)
             {
-                Key vn = varNames[j];
+                var vn = varNames[j];
                 if (env.HasLexicalDeclaration(vn))
                 {
                     ExceptionHelper.ThrowSyntaxError(realm, $"Identifier '{vn}' has already been declared");
@@ -1014,7 +1014,7 @@ namespace Jint
             for (var i = functionToInitialize.Count - 1; i > -1; i--)
             {
                 var f = functionToInitialize[i];
-                var fn = f.Name!;
+                Key fn = f.Name!;
 
                 if (env.HasLexicalDeclaration(fn))
                 {
@@ -1143,7 +1143,7 @@ namespace Jint
                 {
                     for (var j = 0; j < d.BoundNames.Count; j++)
                     {
-                        Key dn = d.BoundNames[j];
+                        var dn = d.BoundNames[j];
                         if (d.IsConstantDeclaration)
                         {
                             lexEnv.CreateImmutableBinding(dn, strict: true);
@@ -1284,7 +1284,7 @@ namespace Jint
                 }
             }
 
-            var boundNames = new List<string>();
+            var boundNames = new List<Key>();
             var declaredVarNames = new List<Key>();
             var variableDeclarations = hoistingScope._variablesDeclarations;
             var variableDeclarationsCount = variableDeclarations?.Count;
@@ -1335,14 +1335,14 @@ namespace Jint
 
             foreach (var f in functionsToInitialize)
             {
-                Key fn = f.Name!;
                 var fo = realm.Intrinsics.Function.InstantiateFunctionObject(f, lexEnv, privateEnv);
                 if (varEnvRec is GlobalEnvironment ger)
                 {
-                    ger.CreateGlobalFunctionBinding(fn, fo, canBeDeleted: true);
+                    ger.CreateGlobalFunctionBinding(f.Name!, fo, canBeDeleted: true);
                 }
                 else
                 {
+                    Key fn = f.Name!;
                     var bindingExists = varEnvRec.HasBinding(fn);
                     if (!bindingExists)
                     {

Jint/EsprimaExtensions.cs

@@ -166,7 +166,7 @@ namespace Jint
             return literal.Value as string ?? Convert.ToString(literal.Value, provider: null) ?? "";
         }
 
-        internal static void GetBoundNames(this VariableDeclaration variableDeclaration, List<string> target)
+        internal static void GetBoundNames(this VariableDeclaration variableDeclaration, List<Key> target)
         {
             ref readonly var declarations = ref variableDeclaration.Declarations;
             for (var i = 0; i < declarations.Count; i++)
@@ -176,7 +176,7 @@ namespace Jint
             }
         }
 
-        internal static void GetBoundNames(this Node? parameter, List<string> target)
+        internal static void GetBoundNames(this Node? parameter, List<Key> target)
         {
             if (parameter is null || parameter.Type == Nodes.Literal)
             {
@@ -430,15 +430,15 @@ namespace Jint
                 for (var i = 0; i < names.Count; i++)
                 {
                     var name = names[i];
-                    var exportName = defaultExport ? "default" : name;
+                    var exportName = defaultExport ? "default" : name.Name;
                     exportEntries.Add(new(exportName, moduleRequest, null, name));
                 }
             }
         }
 
-        private static List<string> GetExportNames(StatementListItem declaration)
+        private static List<Key> GetExportNames(StatementListItem declaration)
         {
-            var result = new List<string>();
+            var result = new List<Key>();
 
             switch (declaration)
             {

Jint/Key.cs

@@ -39,16 +39,6 @@ namespace Jint
             return a.HashCode != b.HashCode || !string.Equals(a.Name, b.Name, StringComparison.Ordinal);
         }
 
-        public static bool operator ==(in Key a, string b)
-        {
-            return string.Equals(a.Name, b, StringComparison.Ordinal);
-        }
-
-        public static bool operator !=(in Key a, string b)
-        {
-            return !string.Equals(a.Name, b, StringComparison.Ordinal);
-        }
-
         public bool Equals(Key other)
         {
             return HashCode == other.HashCode && string.Equals(Name, other.Name, StringComparison.Ordinal);

Jint/Native/Object/ObjectInstance.cs

@@ -1,4 +1,3 @@
-using System.ComponentModel;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Runtime.CompilerServices;
@@ -6,7 +5,6 @@ using Jint.Collections;
 using Jint.Native.Array;
 using Jint.Native.BigInt;
 using Jint.Native.Boolean;
-using Jint.Native.Function;
 using Jint.Native.Json;
 using Jint.Native.Number;
 using Jint.Native.RegExp;
@@ -527,8 +525,7 @@ namespace Jint.Native.Object
         {
             if ((_type & InternalTypes.PlainObject) != InternalTypes.Empty && property is JsString jsString)
             {
-                var key = (Key) jsString.ToString();
-                if (_properties?.TryGetValue(key, out var ownDesc) == true)
+                if (_properties?.TryGetValue(jsString.ToString(), out var ownDesc) == true)
                 {
                     if ((ownDesc._flags & PropertyFlag.Writable) != PropertyFlag.None)
                     {

Jint/Runtime/Environments/DeclarativeEnvironment.cs

@@ -143,10 +143,7 @@ namespace Jint.Runtime.Environments
 
         internal sealed override JsValue WithBaseObject() => Undefined;
 
-        internal sealed override bool HasBindings()
-        {
-            return _dictionary?.Count > 0;
-        }
+        internal sealed override bool HasBindings() => _dictionary?.Count > 0;
 
         /// <inheritdoc />
         internal sealed override string[] GetAllBindingNames()
@@ -166,10 +163,7 @@ namespace Jint.Runtime.Environments
             return keys;
         }
 
-        internal override JsValue GetThisBinding()
-        {
-            return Undefined;
-        }
+        internal override JsValue GetThisBinding() => Undefined;
 
         public void Clear()
         {

Jint/Runtime/Environments/GlobalEnvironment.cs

@@ -29,7 +29,7 @@ namespace Jint.Runtime.Environments
 
         // Environment records are needed by debugger
         internal readonly GlobalDeclarativeEnvironment _declarativeRecord;
-        private readonly HashSet<string> _varNames = new(StringComparer.Ordinal);
+        private readonly HashSet<Key> _varNames = [];
 
         public GlobalEnvironment(Engine engine, ObjectInstance global) : base(engine)
         {
@@ -272,25 +272,13 @@ namespace Jint.Runtime.Environments
             return true;
         }
 
-        internal override bool HasThisBinding()
-        {
-            return true;
-        }
+        internal override bool HasThisBinding() => true;
 
-        internal override bool HasSuperBinding()
-        {
-            return false;
-        }
+        internal override bool HasSuperBinding() => false;
 
-        internal override JsValue WithBaseObject()
-        {
-            return Undefined;
-        }
+        internal override JsValue WithBaseObject() => Undefined;
 
-        internal override JsValue GetThisBinding()
-        {
-            return _global;
-        }
+        internal override JsValue GetThisBinding() => _global;
 
         internal bool HasVarDeclaration(Key name) => _varNames.Contains(name);
 
@@ -313,7 +301,7 @@ namespace Jint.Runtime.Environments
             return !existingProp.Configurable;
         }
 
-        public bool CanDeclareGlobalVar(string name)
+        public bool CanDeclareGlobalVar(Key name)
         {
             if (_global._properties!.ContainsKey(name))
             {
@@ -323,7 +311,7 @@ namespace Jint.Runtime.Environments
             return _global.Extensible;
         }
 
-        public bool CanDeclareGlobalFunction(string name)
+        public bool CanDeclareGlobalFunction(Key name)
         {
             if (!_global._properties!.TryGetValue(name, out var existingProp)
                 || existingProp == PropertyDescriptor.Undefined)
@@ -344,10 +332,9 @@ namespace Jint.Runtime.Environments
             return false;
         }
 
-        public void CreateGlobalVarBinding(string name, bool canBeDeleted)
+        public void CreateGlobalVarBinding(Key name, bool canBeDeleted)
         {
-            Key key = name;
-            if (_global.Extensible && _global._properties!.TryAdd(key, new PropertyDescriptor(Undefined, canBeDeleted
+            if (_global.Extensible && _global._properties!.TryAdd(name, new PropertyDescriptor(Undefined, canBeDeleted
                     ? PropertyFlag.ConfigurableEnumerableWritable | PropertyFlag.MutableBinding
                     : PropertyFlag.NonConfigurable | PropertyFlag.MutableBinding)))
             {
@@ -355,7 +342,7 @@ namespace Jint.Runtime.Environments
             }
         }
 
-        internal void CreateGlobalVarBindings(List<string> names, bool canBeDeleted)
+        internal void CreateGlobalVarBindings(List<Key> names, bool canBeDeleted)
         {
             if (!_global.Extensible)
             {
@@ -366,7 +353,7 @@ namespace Jint.Runtime.Environments
             {
                 var name = names[i];
 
-                _global._properties!.TryAdd(name,new PropertyDescriptor(Undefined, canBeDeleted
+                _global._properties!.TryAdd(name, new PropertyDescriptor(Undefined, canBeDeleted
                     ? PropertyFlag.ConfigurableEnumerableWritable | PropertyFlag.MutableBinding
                     : PropertyFlag.NonConfigurable | PropertyFlag.MutableBinding));
 
@@ -377,7 +364,7 @@ namespace Jint.Runtime.Environments
         /// <summary>
         /// https://tc39.es/ecma262/#sec-createglobalfunctionbinding
         /// </summary>
-        public void CreateGlobalFunctionBinding(string name, JsValue value, bool canBeDeleted)
+        public void CreateGlobalFunctionBinding(Key name, JsValue value, bool canBeDeleted)
         {
             var jsString = new JsString(name);
             var existingProp = _global.GetOwnProperty(jsString);
@@ -397,7 +384,7 @@ namespace Jint.Runtime.Environments
             _varNames.Add(name);
         }
 
-        internal sealed override bool HasBindings()
+        internal override bool HasBindings()
         {
             return _declarativeRecord.HasBindings() || _globalObject?._properties?.Count > 0 || _global._properties?.Count > 0;
         }

Jint/Runtime/Interpreter/JintFunctionDefinition.cs

@@ -188,7 +188,7 @@ internal sealed class JintFunctionDefinition
         internal struct LexicalVariableDeclaration
         {
             public bool IsConstantDeclaration;
-            public List<string> BoundNames;
+            public List<Key> BoundNames;
         }
     }
 
@@ -316,7 +316,7 @@ internal sealed class JintFunctionDefinition
             for (var i = 0; i < lexicalDeclarationsCount; i++)
             {
                 var d = _lexicalDeclarations[i];
-                var boundNames = new List<string>();
+                var boundNames = new List<Key>();
                 d.GetBoundNames(boundNames);
                 declarations[i] = new State.LexicalVariableDeclaration
                 {

Jint/Runtime/Interpreter/JintStatementList.cs

@@ -181,13 +181,10 @@ namespace Jint.Runtime.Interpreter
         /// <summary>
         /// https://tc39.es/ecma262/#sec-blockdeclarationinstantiation
         /// </summary>
-        internal static void BlockDeclarationInstantiation(
-            Engine engine,
-            Environment env,
-            List<Declaration> declarations)
+        internal static void BlockDeclarationInstantiation(Environment env, List<Declaration> declarations)
         {
             var privateEnv = env._engine.ExecutionContext.PrivateEnvironment;
-            var boundNames = new List<string>();
+            var boundNames = new List<Key>();
             for (var i = 0; i < declarations.Count; i++)
             {
                 var d = declarations[i];

Jint/Runtime/Interpreter/Statements/JintBlockStatement.cs

@@ -43,7 +43,7 @@ namespace Jint.Runtime.Interpreter.Statements
             {
                 oldEnv = engine.ExecutionContext.LexicalEnvironment;
                 var blockEnv = JintEnvironment.NewDeclarativeEnvironment(engine, engine.ExecutionContext.LexicalEnvironment);
-                JintStatementList.BlockDeclarationInstantiation(engine, blockEnv, _lexicalDeclarations);
+                JintStatementList.BlockDeclarationInstantiation(blockEnv, _lexicalDeclarations);
                 engine.UpdateLexicalEnvironment(blockEnv);
             }
 

Jint/Runtime/Interpreter/Statements/JintForInForOfStatement.cs

@@ -23,7 +23,7 @@ namespace Jint.Runtime.Interpreter.Statements
         private JintExpression? _expr;
         private BindingPattern? _assignmentPattern;
         private JintExpression _right = null!;
-        private List<string>? _tdzNames;
+        private List<Key>? _tdzNames;
         private bool _destructuring;
         private LhsKind _lhsKind;
 
@@ -57,7 +57,7 @@ namespace Jint.Runtime.Interpreter.Statements
                 var id = variableDeclarationDeclaration.Id;
                 if (_lhsKind == LhsKind.LexicalBinding)
                 {
-                    _tdzNames = new List<string>(1);
+                    _tdzNames = new List<Key>(1);
                     id.GetBoundNames(_tdzNames);
                 }
 
@@ -336,7 +336,7 @@ namespace Jint.Runtime.Interpreter.Statements
         {
             var envRec = (DeclarativeEnvironment) environment;
             var variableDeclaration = (VariableDeclaration) _leftNode;
-            var boundNames = new List<string>();
+            var boundNames = new List<Key>();
             variableDeclaration.GetBoundNames(boundNames);
             for (var i = 0; i < boundNames.Count; i++)
             {

Jint/Runtime/Interpreter/Statements/JintForStatement.cs

@@ -18,7 +18,7 @@ namespace Jint.Runtime.Interpreter.Statements
         private JintExpression? _increment;
 
         private ProbablyBlockStatement _body;
-        private List<string>? _boundNames;
+        private List<Key>? _boundNames;
 
         private bool _shouldCreatePerIterationEnvironment;
 
@@ -38,7 +38,7 @@ namespace Jint.Runtime.Interpreter.Statements
                     var d = (VariableDeclaration) _statement.Init;
                     if (d.Kind != VariableDeclarationKind.Var)
                     {
-                        _boundNames = new List<string>();
+                        _boundNames = new List<Key>();
                         d.GetBoundNames(_boundNames);
                     }
                     _initStatement = new JintVariableDeclaration(d);
@@ -77,11 +77,11 @@ namespace Jint.Runtime.Interpreter.Statements
                     var name = _boundNames[i];
                     if (kind == VariableDeclarationKind.Const)
                     {
-                        loopEnvRec.CreateImmutableBinding(name, true);
+                        loopEnvRec.CreateImmutableBinding(name);
                     }
                     else
                     {
-                        loopEnvRec.CreateMutableBinding(name, false);
+                        loopEnvRec.CreateMutableBinding(name);
                     }
                 }
 

Jint/Runtime/Interpreter/Statements/JintSwitchBlock.cs

@@ -55,7 +55,7 @@ namespace Jint.Runtime.Interpreter.Statements
                     blockEnv ??= JintEnvironment.NewDeclarativeEnvironment(context.Engine, oldEnv);
                     blockEnv.Clear();
 
-                    JintStatementList.BlockDeclarationInstantiation(context.Engine, blockEnv, clause.LexicalDeclarations);
+                    JintStatementList.BlockDeclarationInstantiation(blockEnv, clause.LexicalDeclarations);
                     context.Engine.UpdateLexicalEnvironment(blockEnv);
                 }
 

Jint/Runtime/Interpreter/Statements/JintTryStatement.cs

@@ -69,12 +69,12 @@ namespace Jint.Runtime.Interpreter.Statements
                 var oldEnv = engine.ExecutionContext.LexicalEnvironment;
                 var catchEnv = JintEnvironment.NewDeclarativeEnvironment(engine, oldEnv, catchEnvironment: true);
 
-                var boundNames = new List<string>();
+                var boundNames = new List<Key>();
                 _statement.Handler.Param.GetBoundNames(boundNames);
 
-                foreach (var argName in boundNames)
+                for (var i = 0; i < boundNames.Count; i++)
                 {
-                    catchEnv.CreateMutableBinding(argName, false);
+                    catchEnv.CreateMutableBinding(boundNames[i]);
                 }
 
                 engine.UpdateLexicalEnvironment(catchEnv);

Jint/Runtime/KnownKeys.cs

@@ -1,5 +1,3 @@
-using Environment = Jint.Runtime.Environments.Environment;
-
 namespace Jint.Runtime;
 
 internal static class KnownKeys

Jint/Runtime/Modules/SourceTextModule.cs

@@ -250,10 +250,10 @@ internal class SourceTextModule : CyclicModule
         var hoistingScope = HoistingScope.GetModuleLevelDeclarations(_source);
 
         var varDeclarations = hoistingScope._variablesDeclarations;
-        var declaredVarNames = new HashSet<string>(StringComparer.Ordinal);
+        var declaredVarNames = new HashSet<Key>();
         if (varDeclarations != null)
         {
-            var boundNames = new List<string>();
+            var boundNames = new List<Key>();
             for (var i = 0; i < varDeclarations.Count; i++)
             {
                 var d = varDeclarations[i];
@@ -264,7 +264,7 @@ internal class SourceTextModule : CyclicModule
                     var dn = boundNames[j];
                     if (declaredVarNames.Add(dn))
                     {
-                        env.CreateMutableBinding(dn, false);
+                        env.CreateMutableBinding(dn);
                         env.InitializeBinding(dn, Undefined);
                     }
                 }
@@ -275,7 +275,7 @@ internal class SourceTextModule : CyclicModule
 
         if (lexDeclarations != null)
         {
-            var boundNames = new List<string>();
+            var boundNames = new List<Key>();
             for (var i = 0; i < lexDeclarations.Count; i++)
             {
                 var d = lexDeclarations[i];
@@ -286,11 +286,11 @@ internal class SourceTextModule : CyclicModule
                     var dn = boundNames[j];
                     if (d.IsConstantDeclaration())
                     {
-                        env.CreateImmutableBinding(dn, true);
+                        env.CreateImmutableBinding(dn);
                     }
                     else
                     {
-                        env.CreateMutableBinding(dn, false);
+                        env.CreateMutableBinding(dn);
                     }
                 }
             }