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

Jint/Engine.Ast.cs

@@ -88,14 +88,14 @@ public CachedHoistingScope(Program program)
     {
         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 static void GatherLexNames(HoistingScope scope, List<CachedLexicalName>
         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 static void GatherLexNames(HoistingScope scope, List<CachedLexicalName>
     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 HoistingScope GetHoistingScope(this Program program)
         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 @@ private void GlobalDeclarationInstantiation(
 
             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 @@ private void GlobalDeclarationInstantiation(
             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 @@ private void GlobalDeclarationInstantiation(
             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 @@ private void GlobalDeclarationInstantiation(
                 {
                     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 @@ internal void EvalDeclarationInstantiation(
                 }
             }
 
-            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 @@ internal void EvalDeclarationInstantiation(
 
             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 @@ internal static string LiteralKeyToString(Literal literal)
             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 @@ internal static void GetBoundNames(this VariableDeclaration variableDeclaration,
             }
         }
 
-        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 @@ private static void GetExportEntries(bool defaultExport, StatementListItem decla
                 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 @@ private Key(string name)
             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,12 +1,10 @@
-using System.ComponentModel;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Runtime.CompilerServices;
 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 @@ public bool Set(JsValue property, JsValue value)
         {
             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 @@ internal sealed override bool DeleteBinding(Key name)
 
         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 @@ internal sealed override string[] GetAllBindingNames()
             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 @@ public GlobalDeclarativeEnvironment(Engine engine) : base(engine)
 
         // 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 @@ internal override bool DeleteBinding(Key name)
             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 @@ internal bool HasRestrictedGlobalProperty(Key name)
             return !existingProp.Configurable;
         }
 
-        public bool CanDeclareGlobalVar(string name)
+        public bool CanDeclareGlobalVar(Key name)
         {
             if (_global._properties!.ContainsKey(name))
             {
@@ -323,7 +311,7 @@ public bool CanDeclareGlobalVar(string name)
             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,18 +332,17 @@ public bool CanDeclareGlobalFunction(string name)
             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)))
             {
                 _varNames.Add(name);
             }
         }
 
-        internal void CreateGlobalVarBindings(List<string> names, bool canBeDeleted)
+        internal void CreateGlobalVarBindings(List<Key> names, bool canBeDeleted)
         {
             if (!_global.Extensible)
             {
@@ -366,7 +353,7 @@ internal void CreateGlobalVarBindings(List<string> names, bool canBeDeleted)
             {
                 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 @@ internal void CreateGlobalVarBindings(List<string> names, bool canBeDeleted)
         /// <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 @@ public void CreateGlobalFunctionBinding(string name, JsValue value, bool canBeDe
             _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 struct VariableValuePair
         internal struct LexicalVariableDeclaration
         {
             public bool IsConstantDeclaration;
-            public List<string> BoundNames;
+            public List<Key> BoundNames;
         }
     }
 
@@ -316,7 +316,7 @@ internal static State BuildState(IFunction function)
             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 @@ private static Completion CreateThrowCompletion(JintStatement? s, JavaScriptExce
         /// <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 @@ public Completion ExecuteBlock(EvaluationContext context)
             {
                 oldEnv = engine.ExecutionContext.LexicalEnvironment;
                 var blockEnv = JintEnvironment.NewDeclarativeEnvironment(engine, engine.ExecutionContext.LexicalEnvironment);
-                JintStatementList.BlockDeclarationInstantiation(engine, blockEnv, _lexicalDeclarations);
+                JintStatementList.BlockDeclarationInstantiation(blockEnv, _lexicalDeclarations);
                 engine.UpdateLexicalEnvironment(blockEnv);
             }