Escape keyword-named bindings in the JavaScript generator [patch]

Coder.Test/Languages/JavaScriptReservedWordIdentifierTests.cs

@@ -0,0 +1,153 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.Coder.Test.Languages;
+
+using ktsu.Coder.Ast;
+using ktsu.Coder.Languages;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+/// <summary>
+/// Runs what the JavaScript generator writes for names that are JavaScript keywords. A binding — a
+/// parameter, local, function, class or top-level name — has no escape, so it has to come out with an
+/// underscore wherever it is declared and used. A member keeps its name, because JavaScript reads a
+/// keyword after a <c>.</c> and as a field, accessor or method name.
+/// </summary>
+/// <remarks>
+/// The run is inconclusive rather than failing where no node is on the path, which is the honest
+/// result: nothing was checked.
+/// </remarks>
+[TestClass]
+public class JavaScriptReservedWordIdentifierTests
+{
+	/// <summary>
+	/// The JavaScript driver. A member keeps its keyword name, which JavaScript reads after a
+	/// <c>.</c>, and a top-level name, which no keyword can be, has had an underscore appended.
+	/// </summary>
+	private const string JavaScriptDriver = """
+
+
+		const holder = new Holder();
+		if (holder.class !== 3 || holder.default !== 4) { process.exit(1); }
+		if (holder.delete !== 1) { process.exit(2); }
+		if (holder.total(5, 6) !== 14) { process.exit(3); }
+		if (holder.twice() !== 3) { process.exit(4); }
+		if (Flag.null !== 0 || Flag.All !== 1) { process.exit(5); }
+		if (return_(7) !== 7 || relay() !== 5) { process.exit(6); }
+		if (var_ !== 7) { process.exit(7); }
+		if (!(new Child() instanceof import_)) { process.exit(8); }
+
+		""";
+
+	/// <summary>
+	/// Tests that JavaScript appends an underscore to a keyword-named parameter, local, function,
+	/// class and top-level name, at their declarations and where they are used, and leaves a
+	/// keyword-named field, accessor, method and enumeration member as declared, which is how
+	/// JavaScript reads a keyword after a <c>.</c>.
+	/// </summary>
+	[TestMethod]
+	public void JavaScript_KeywordNamesRunAndResolve()
+	{
+		string? node = ToolchainHarness.FindOnPath("--version", "node");
+		if (node is null)
+		{
+			Assert.Inconclusive("No node on the path, so nothing was run.");
+			return;
+		}
+
+		ToolchainHarness.InTemporaryDirectory(directory =>
+		{
+			string source = new JavaScriptGenerator().Generate(JavaScriptTypes()) + JavaScriptDriver;
+			File.WriteAllText(Path.Combine(directory, "holder.js"), source);
+
+			(int ran, string output) = ToolchainHarness.Run(node, "holder.js", directory);
+			Assert.AreEqual(0, ran, $"the generated script failed, or a keyword-named name read back wrongly (driver check {ran}):{Environment.NewLine}{output}{Environment.NewLine}{source}");
+		});
+	}
+
+	/// <summary>
+	/// Tests that a JavaScript reference escapes the binding it leads with but not the member after a
+	/// <c>.</c>, and keeps <c>this</c> and the literals as written.
+	/// </summary>
+	[TestMethod]
+	public void JavaScript_ReferenceEscapesTheBindingButNotTheMember()
+	{
+		string source = new JavaScriptGenerator().Generate(JavaScriptTypes());
+
+		Assert.Contains("this.class", source, StringComparison.Ordinal, source);
+		Assert.Contains("this.switch()", source, StringComparison.Ordinal, source);
+		Assert.Contains("get delete()", source, StringComparison.Ordinal, source);
+		Assert.Contains("total(in_, typeof_)", source, StringComparison.Ordinal, source);
+		Assert.Contains("return_(5)", source, StringComparison.Ordinal, source);
+		Assert.DoesNotContain("this_", source, StringComparison.Ordinal, source);
+		Assert.DoesNotContain("class_", source, StringComparison.Ordinal, source);
+		Assert.DoesNotContain("switch_", source, StringComparison.Ordinal, source);
+		Assert.DoesNotContain("null_", source, StringComparison.Ordinal, source);
+	}
+
+	/// <summary>
+	/// Builds classes, an enumeration, functions and a top-level name whose names are JavaScript
+	/// keywords: members, which keep their names, and bindings, which cannot.
+	/// </summary>
+	/// <returns>The file to generate.</returns>
+	private static SourceFile JavaScriptTypes()
+	{
+		ClassDeclaration holder = new("Holder");
+		holder.Members.Add(new VariableDeclaration("class", "int", new LiteralExpression<int>(3)));
+		holder.Members.Add(new VariableDeclaration("default", "int", new LiteralExpression<int>(4)));
+		holder.Members.Add(new PropertyDeclaration("delete", "int")
+		{
+			GetterBody = { new ReturnStatement(new LiteralExpression<int>(1)) },
+		});
+
+		FunctionDeclaration keywordMethod = new("switch") { ReturnType = "int" };
+		keywordMethod.Body.Add(new ReturnStatement(new LiteralExpression<int>(2)));
+		holder.Members.Add(keywordMethod);
+
+		FunctionDeclaration twice = new("twice") { ReturnType = "int" };
+		twice.Body.Add(new ReturnStatement(new BinaryExpression(
+			new CallExpression(new VariableReference("this"), "switch"),
+			BinaryOperator.Add,
+			new LiteralExpression<int>(1))));
+		holder.Members.Add(twice);
+
+		FunctionDeclaration total = new("total") { ReturnType = "int" };
+		total.Parameters.Add(new Parameter("in", "int"));
+		total.Parameters.Add(new Parameter("typeof", "int"));
+		total.Body.Add(new VariableDeclaration(
+			"let",
+			"int",
+			new BinaryExpression(new VariableReference("in"), BinaryOperator.Add, new VariableReference("typeof"))));
+		total.Body.Add(new VariableDeclaration(
+			"new",
+			"int",
+			new BinaryExpression(new VariableReference("let"), BinaryOperator.Add, new VariableReference("this.class"))));
+		total.Body.Add(new ReturnStatement(new VariableReference("new")));
+		holder.Members.Add(total);
+
+		EnumDeclaration flag = new("Flag");
+		flag.Members.Add(new EnumMember("null"));
+		flag.Members.Add(new EnumMember("All"));
+
+		FunctionDeclaration identity = new("return") { ReturnType = "int" };
+		identity.Parameters.Add(new Parameter("with", "int"));
+		identity.Body.Add(new ReturnStatement(new VariableReference("with")));
+
+		CallExpression callIdentity = new("return");
+		callIdentity.Arguments.Add(new LiteralExpression<int>(5));
+		FunctionDeclaration relay = new("relay") { ReturnType = "int" };
+		relay.Body.Add(new ReturnStatement(callIdentity));
+
+		ClassDeclaration keywordBase = new("import");
+		ClassDeclaration child = new("Child") { BaseType = new TypeReference("import") };
+
+		SourceFile file = new("holder");
+		file.Members.Add(holder);
+		file.Members.Add(flag);
+		file.Members.Add(identity);
+		file.Members.Add(relay);
+		file.Members.Add(new VariableDeclaration("var", "int", new LiteralExpression<int>(7)));
+		file.Members.Add(keywordBase);
+		file.Members.Add(child);
+		return file;
+	}
+}

Coder/Languages/JavaScriptGenerator.cs

@@ -27,6 +27,33 @@
 	/// </summary>
 	private const string StaticKeyword = "static ";
 
+	/// <summary>
+	/// Gets the words JavaScript will not bind as a name in a class body or a module, which is strict
+	/// code: the reserved words, the strict-mode future reserved words, the literals, and the two
+	/// names strict code may not bind.
+	/// </summary>
+	/// <remarks>
+	/// A binding — a function, class, parameter, local or top-level name — has no escape here, so it
+	/// gets an underscore: <c>delete</c> becomes <c>delete_</c>. A member keeps its name, because
+	/// JavaScript reads a keyword after a <c>.</c> and as a method, accessor or field name as written
+	/// (<see cref="EscapesMemberNames"/>), and renaming one would change the object's shape for no
+	/// reason.
+	/// </remarks>
+	protected override IReadOnlySet<string> ReservedWords { get; } = new HashSet<string>(StringComparer.Ordinal)
+	{
+		"arguments", "await", "break", "case", "catch", "class", "const", "continue", "debugger",
+		"default", "delete", "do", "else", "enum", "eval", "export", "extends", "false", "finally",
+		"for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let",
+		"new", "null", "package", "private", "protected", "public", "return", "static", "super",
+		"switch", "this", "throw", "true", "try", "typeof", "var", "void", "while", "with", "yield",
+	};
+
+	/// <inheritdoc/>
+	protected override bool IsReceiverWord(string word) => word is "this" or "super" or "true" or "false" or "null";
+
+	/// <inheritdoc/>
+	protected override bool EscapesMemberNames => false;
+
 	/// <summary>
 	/// Gets the unique identifier for this language generator.
 	/// </summary>
@@ -309,7 +336,7 @@
 	/// </remarks>
 	private static readonly Dictionary<string, string> BuiltInAliasValues = new(StringComparer.Ordinal)
 	{
 		{ "int", "Number" },
 		{ "long", "Number" },
 		{ "short", "Number" },
 		{ "ushort", "Number" },
@@ -334,11 +361,11 @@
 	/// </summary>
 	/// <param name="aliased">The type the alias names, if any.</param>
 	/// <returns>A built-in type's JavaScript constructor, an array's <c>Array</c>, or the name itself.</returns>
-	private static string AliasedValue(TypeReference? aliased) => aliased switch
+	private string AliasedValue(TypeReference? aliased) => aliased switch
 	{
 		null => "Object",
 		{ IsArray: true } => "Array",
-		_ => BuiltInAliasValues.TryGetValue(aliased.Name, out string? value) ? value : aliased.Name,
+		_ => BuiltInAliasValues.TryGetValue(aliased.Name, out string? value) ? value : EscapeReference(aliased.Name),
 	};
 
 	/// <inheritdoc/>
@@ -353,7 +380,7 @@
 		Ensure.NotNull(code);
 
 		WritePreamble(usingAlias, code, PreambleSite.Type);
-		code.Write($"const {usingAlias.Name} = {AliasedValue(usingAlias.AliasedType)}");
+		code.Write($"const {EscapeIdentifier(usingAlias.Name ?? string.Empty)} = {AliasedValue(usingAlias.AliasedType)}");
 		EndStatement(code);
 	}
 
@@ -377,7 +404,7 @@
 			return;
 		}
 
-		code.Write($"new {construction.Type.Name}(");
+		code.Write($"new {EscapeReference(construction.Type.Name)}(");
 
 		if (named)
 		{
@@ -443,7 +470,7 @@
 		Ensure.NotNull(code);
 
 		WritePreamble(enumDecl, code, PreambleSite.Enum);
-		code.WriteLine($"const {enumDecl.Name ?? "UnnamedEnum"} = Object.freeze({{");
+		code.WriteLine($"const {EscapeIdentifier(enumDecl.Name ?? "UnnamedEnum")} = Object.freeze({{");
 
 		using (IndentScope members = new(code))
 		{
@@ -466,7 +493,7 @@
 		Ensure.NotNull(field);
 		Ensure.NotNull(code);
 
-		WriteField(field, field.IsConstant && field.InitialValue is not null ? "const " : "let ", code);
+		WriteField(field, field.IsConstant && field.InitialValue is not null ? "const " : "let ", isBinding: true, code);
 	}
 
 	/// <summary>
@@ -484,19 +511,21 @@
 	/// class holds one value rather than each instance holding its own.
 	/// </remarks>
 	private void GenerateClassField(FieldDeclaration field, CodeBlocker code) =>
-		WriteField(field, field.IsStatic || field.IsConstant ? StaticKeyword : string.Empty, code);
+		WriteField(field, field.IsStatic || field.IsConstant ? StaticKeyword : string.Empty, isBinding: false, code);
 
 	/// <summary>
 	/// Writes a field declaration after the keyword its scope calls for.
 	/// </summary>
 	/// <param name="field">The declaration to emit.</param>
 	/// <param name="keyword">The keyword, with its trailing space, or empty for none.</param>
+	/// <param name="isBinding">True when the field is a top-level name, which a keyword cannot be; false for a class field, which keeps its name.</param>
 	/// <param name="code">The writer to emit into.</param>
-	private void WriteField(FieldDeclaration field, string keyword, CodeBlocker code)
+	private void WriteField(FieldDeclaration field, string keyword, bool isBinding, CodeBlocker code)
 	{
 		WritePreamble(field, code, PreambleSite.Field);
 		code.Write(keyword);
-		code.Write(field.Name ?? "unnamed");
+		string name = field.Name ?? "unnamed";
+		code.Write(isBinding ? EscapeIdentifier(name) : name);
 
 		if (field.InitialValue is not null)
 		{
@@ -541,7 +570,7 @@
 		// JavaScript has no generics, so what a function is written over is written down.
 		WriteTypeParametersDown(funcDecl.TypeParameters, code);
 
-		code.Write($"function {funcDecl.Name ?? "unnamedFunction"}(");
+		code.Write($"function {EscapeIdentifier(funcDecl.Name ?? "unnamedFunction")}(");
 		GenerateParameterList(funcDecl.Parameters, code);
 
 		// The line is left open, so the scope's brace lands on it: JavaScript braces hang.
@@ -634,11 +663,13 @@
 			WriteInexpressible(code, $"implements {string.Join(", ", classDecl.Interfaces.Select(contract => contract.Name))}");
 		}
 
-		code.Write($"{binding}class {classDecl.Name ?? "UnnamedClass"}");
+		// The class's own name is a binding even where a static field holds it, so it escapes; the
+		// field in front of it, a member, keeps the name as declared.
+		code.Write($"{binding}class {EscapeIdentifier(classDecl.Name ?? "UnnamedClass")}");
 
 		if (classDecl.BaseType is TypeReference baseType)
 		{
-			code.Write($" extends {baseType.Name}");
+			code.Write($" extends {EscapeReference(baseType.Name)}");
 		}
 	}
 
@@ -843,7 +874,7 @@
 		Ensure.NotNull(parameter);
 		Ensure.NotNull(code);
 
-		code.Write(parameter.Name ?? $"param{position}");
+		code.Write(EscapeIdentifier(parameter.Name ?? $"param{position}"));
 		AppendDefaultValue(parameter, code);
 	}
 
@@ -856,7 +887,7 @@
 
 		// `const` needs an initializer, so an uninitialized constant has to be declared with `let`.
 		string keyword = varDecl.IsConstant && varDecl.InitialValue is not null ? "const" : "let";
-		code.Write($"{keyword} {varDecl.Name}");
+		code.Write($"{keyword} {EscapeIdentifier(varDecl.Name)}");
 
 		if (varDecl.InitialValue is not null)
 		{
@@ -1074,7 +1105,7 @@
 
 		IntegerWidth? left = IntegerWidthOf(StaticType(binary.Left));
 		IntegerWidth? right = IntegerWidthOf(StaticType(binary.Right));
 		return left is null ? right : right is null ? left : (IntegerWidth)Math.Max((int)left, (int)right);
 	}
 
 	private static IntegerWidth? IntegerWidthOf(TypeReference? type) =>

Coder/Languages/LanguageGeneratorBase.cs

@@ -417,7 +417,7 @@
 			}
 		}
 
 		foreach (object item in expected)
 		{
 			if (!emittedPreambleItems!.Contains(item))
 			{
@@ -435,7 +435,7 @@
 	/// Read off the nodes' public properties rather than a list of node types, so a node added to the
 	/// AST is walked without anybody remembering to add it here.
 	/// </remarks>
 	protected static IEnumerable<AstNode> SelfAndDescendants(AstNode root)
 	{
 		HashSet<AstNode> visited = new(ReferenceEqualityComparer.Instance);
 		Stack<AstNode> pending = new();
@@ -911,7 +911,7 @@
 			code.Write(SpellMemberAccess(callExpr.Receiver));
 		}
 
-		code.Write(SpellCallee(callExpr.Callee));
+		code.Write(callExpr.Receiver is not null && !EscapesMemberNames ? callExpr.Callee : SpellCallee(callExpr.Callee));
 		code.Write("(");
 		for (int index = 0; index < callExpr.Arguments.Count; index++)
 		{
@@ -1213,6 +1213,18 @@
 	/// </remarks>
 	protected virtual bool IsReceiverWord(string word) => false;
 
+	/// <summary>
+	/// Gets whether a member's name, where it follows a <c>.</c>, is escaped like any other name.
+	/// </summary>
+	/// <remarks>
+	/// True by default, because most targets reserve a keyword everywhere: C++ cannot read
+	/// <c>holder.default</c>, so the member is declared <c>default_</c> and every access says so.
+	/// A target that accepts a keyword as a member name after a <c>.</c> — JavaScript reads
+	/// <c>holder.default</c> and <c>this.delete()</c> as written — returns false, and then only the
+	/// names a keyword would bind are escaped: what leads a reference, and a call with no receiver.
+	/// </remarks>
+	protected virtual bool EscapesMemberNames => true;
+
 	/// <summary>
 	/// Spells a reference so any segment naming a declared member escapes as its declaration did.
 	/// </summary>
@@ -1237,6 +1249,11 @@
 		string[] segments = reference.Split('.');
 		for (int i = 0; i < segments.Length; i++)
 		{
+			if (i > 0 && !EscapesMemberNames)
+			{
+				continue;
+			}
+
 			string[] path = segments[i].Split("::");
 			if (path.Length > 1)
 			{
@@ -1270,17 +1287,30 @@
 	/// keyword. A callee is otherwise text and stays verbatim: C++'s <c>sizeof</c> and <c>delete</c>
 	/// are reserved but not declared, and escaping them would break the call. Any function or method
 	/// of the name counts, whatever its receiver, because every one of them was escaped the same way
-	/// and no target accepts the bare keyword as a callee.
+	/// and no target accepts the bare keyword as a callee. A target that does not escape member names
+	/// (<see cref="EscapesMemberNames"/>) counts only free functions, since only they were escaped.
 	/// </remarks>
 	protected string SpellCallee(string callee)
 	{
 		Ensure.NotNull(callee);
 
-		return ReservedWords.Contains(callee)
-			&& GenerationRoot is AstNode root
-			&& SelfAndDescendants(root).OfType<FunctionDeclaration>().Any(function => function.Name == callee)
-			? EscapeIdentifier(callee)
-			: callee;
+		if (!ReservedWords.Contains(callee) || GenerationRoot is not AstNode root)
+		{
+			return callee;
+		}
+
+		IEnumerable<FunctionDeclaration> functions = SelfAndDescendants(root).OfType<FunctionDeclaration>();
+		if (!EscapesMemberNames)
+		{
+			// A method keeps its keyword name, so only a free function was escaped where it was declared.
+			HashSet<FunctionDeclaration> methods = SelfAndDescendants(root)
+				.OfType<ClassDeclaration>()
+				.SelectMany(type => type.Members.OfType<FunctionDeclaration>())
+				.ToHashSet<FunctionDeclaration>(ReferenceEqualityComparer.Instance);
+			functions = functions.Where(function => !methods.Contains(function));
+		}
+
+		return functions.Any(function => function.Name == callee) ? EscapeIdentifier(callee) : callee;
 	}
 
 	/// <summary>
@@ -1391,7 +1421,7 @@
 	/// the generator adds through <see cref="RequiredImports"/>.
 	/// </remarks>
 	protected virtual string SpellNonFiniteDouble(double value) =>
 		double.IsNaN(value) ? "NaN" : value > 0 ? "Infinity" : "-Infinity";
 
 	/// <summary>
 	/// Spells a float that is NaN or an infinity.
@@ -1502,7 +1532,7 @@
 		string? origin = null;
 		long offset = 0;
 
 		foreach (EnumMember member in enumDecl.Members)
 		{
 			if (member.Value is not null)
 			{