[patch] Honour IsStatic and IsConstant on JavaScript fields

Coder.Test/Languages/JavaScriptGeneratorTests.cs

@@ -160,6 +160,7 @@
 		Assert.IsFalse(Generator.CanGenerate(new AstLeafNode<double>(1.0)));
 		Assert.ThrowsExactly<NotSupportedException>(() => Generator.Generate(new AstLeafNode<double>(1.0)));
 	}
+
 	/// <summary>
 	/// Builds a <c>Shape</c> base class and a <c>Circle</c> that extends it with a constructor, the
 	/// shape that throws on construction when the derived constructor never calls <c>super</c>.
@@ -252,4 +253,85 @@
 			StringAssert.Contains(output, "2 shape", StringComparison.Ordinal);
 		});
 	}
+
+	/// <summary>
+	/// Builds the file from issue #104: a file-scope constant, and a class with a static field and a
+	/// static constant.
+	/// </summary>
+	/// <returns>The source file.</returns>
+	private static SourceFile StaticFieldFile()
+	{
+		SourceFile file = new("Sample");
+		file.Members.Add(new FieldDeclaration("Limit", "int") { IsConstant = true, InitialValue = new LiteralExpression<int>(10) });
+		ClassDeclaration counter = new("Counter");
+		counter.Members.Add(new FieldDeclaration("created", "int") { IsStatic = true, InitialValue = new LiteralExpression<int>(0) });
+		counter.Members.Add(new FieldDeclaration("MAX", "int") { IsStatic = true, IsConstant = true, InitialValue = new LiteralExpression<int>(99) });
+		counter.Members.Add(new FieldDeclaration("count", "int") { InitialValue = new LiteralExpression<int>(1) });
+		file.Members.Add(counter);
+		return file;
+	}
+
+	/// <summary>
+	/// Tests that static and constant class fields are written <c>static</c>, and an instance field is not.
+	/// </summary>
+	[TestMethod]
+	public void ClassField_StaticOrConstant_IsWrittenStatic()
+	{
+		ClassDeclaration counter = new("Counter");
+		counter.Members.Add(new FieldDeclaration("created", "int") { IsStatic = true, InitialValue = new LiteralExpression<int>(0) });
+		counter.Members.Add(new FieldDeclaration("LIMIT", "int") { IsConstant = true, InitialValue = new LiteralExpression<int>(5) });
+		counter.Members.Add(new FieldDeclaration("count", "int") { InitialValue = new LiteralExpression<int>(1) });
+
+		string code = Generator.Generate(counter);
+
+		StringAssert.Contains(code, "static created = 0;", StringComparison.Ordinal);
+		StringAssert.Contains(code, "static LIMIT = 5;", StringComparison.Ordinal);
+		StringAssert.Contains(code, "count = 1;", StringComparison.Ordinal);
+		Assert.DoesNotContain("static count", code);
+	}
+
+	/// <summary>
+	/// Tests that a field outside a class is declared, <c>const</c> when constant and <c>let</c> otherwise,
+	/// rather than assigned to an undeclared name.
+	/// </summary>
+	[TestMethod]
+	public void FileScopeField_IsDeclaredConstOrLet()
+	{
+		SourceFile file = new("Sample");
+		file.Members.Add(new FieldDeclaration("Limit", "int") { IsConstant = true, InitialValue = new LiteralExpression<int>(10) });
+		file.Members.Add(new FieldDeclaration("total", "int") { InitialValue = new LiteralExpression<int>(0) });
+		file.Members.Add(new FieldDeclaration("Pending", "int") { IsConstant = true });
+
+		string code = Generator.Generate(file);
+
+		StringAssert.Contains(code, "const Limit = 10;", StringComparison.Ordinal);
+		StringAssert.Contains(code, "let total = 0;", StringComparison.Ordinal);
+		StringAssert.Contains(code, "let Pending;", StringComparison.Ordinal);
+	}
+
+	/// <summary>
+	/// Tests that the issue's file loads as an ES module under Node.js, the static constant is on the
+	/// class, and the file-scope constant does not leak onto the global object.
+	/// </summary>
+	[TestMethod]
+	public void StaticAndFileScopeFields_RunUnderNode()
+	{
+		string? node = ToolchainHarness.FindOnPath("--version", "node");
+		if (node is null)
+		{
+			Assert.Inconclusive("No Node.js runtime on the path, so nothing was run.");
+			return;
+		}
+
+		string generated = Generator.Generate(StaticFieldFile());
+
+		ToolchainHarness.InTemporaryDirectory(directory =>
+		{
+			string path = Path.Combine(directory, "sample.mjs");
+			File.WriteAllText(path, generated + "console.log(Counter.MAX === 99, Counter.created, new Counter().MAX, Limit, globalThis.Limit);\n");
+			(int exitCode, string output) = ToolchainHarness.Run(node, path, directory);
+			Assert.AreEqual(0, exitCode, $"Node.js could not load the generated module:{Environment.NewLine}{output}{Environment.NewLine}{generated}");
+			StringAssert.Contains(output, "true 0 undefined 10 undefined", StringComparison.Ordinal);
+		});
+	}
 }

Coder/Languages/JavaScriptGenerator.cs

@@ -269,16 +269,45 @@ protected override void GenerateEnumDeclaration(EnumDeclaration enumDecl, CodeBl
 
 	/// <inheritdoc/>
 	/// <remarks>
-	/// A class field, which JavaScript writes without a type since it has none to write. A field with
-	/// no initialiser is still declared: the property then exists on every instance, which is what
-	/// makes the shape of an object predictable rather than growing as it is assigned to.
+	/// A field outside a class, in a file or a namespace, is a module-level binding, so it is declared
+	/// the way a local is. A bare assignment would be to an undeclared name, which throws in an ES
+	/// module and creates a global in a script. <c>const</c> needs an initializer, so an uninitialized
+	/// constant is declared with <c>let</c>. Fields inside a class go through
+	/// <see cref="GenerateClassField"/> instead.
 	/// </remarks>
 	protected override void GenerateFieldDeclaration(FieldDeclaration field, CodeBlocker code)
 	{
 		Ensure.NotNull(field);
 		Ensure.NotNull(code);
 
+		WriteField(field, field.IsConstant && field.InitialValue is not null ? "const " : "let ", code);
+	}
+
+	/// <summary>
+	/// Emits a field declaration as a class field.
+	/// </summary>
+	/// <param name="field">The declaration to emit.</param>
+	/// <param name="code">The writer to emit into.</param>
+	/// <remarks>
+	/// JavaScript writes a class field without a type, since it has none to write. A field with no
+	/// initialiser is still declared: the property then exists on every instance, which is what makes
+	/// the shape of an object predictable rather than growing as it is assigned to. A static field, and
+	/// a constant one, which is static whether or not it says so, is written <c>static</c>, so the
+	/// 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);
+
+	/// <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="code">The writer to emit into.</param>
+	private void WriteField(FieldDeclaration field, string keyword, CodeBlocker code)
+	{
 		WritePreamble(field, code, PreambleSite.Field);
+		code.Write(keyword);
 		code.Write(field.Name ?? "unnamed");
 
 		if (field.InitialValue is not null)
@@ -373,6 +402,10 @@ protected override void GenerateClassDeclaration(ClassDeclaration classDecl, Cod
 					GenerateNestedEnum(nested, code);
 					break;
 
+				case FieldDeclaration classField:
+					GenerateClassField(classField, code);
+					break;
+
 				// A field is not a variable: `let` is a statement keyword and a syntax error in a
 				// class body, so the declaration is emitted as the name and its initializer alone.
 				case VariableDeclaration field: