fix: stop JavaScript renaming a private member its references still name [patch]

Coder.Test/Ast/ConstantDeclarationTests.cs

@@ -100,10 +100,12 @@ public void JavaScript_WritesStaticForAMember()
 	}
 
 	/// <summary>
-	/// Tests that a private constant member is spelled with both, since the two are independent.
+	/// Tests that a private constant member is still spelled <c>static</c>, and under the name it was
+	/// declared with: constancy is a modifier JavaScript has, and visibility is one it does not, so
+	/// asking for the second leaves the first alone rather than renaming what it modifies.
 	/// </summary>
 	[TestMethod]
-	public void JavaScript_CombinesStaticWithAPrivateName()
+	public void JavaScript_WritesStaticOnAPrivateConstantWithoutRenamingIt()
 	{
 		ClassDeclaration declaration = new("Limits");
 		declaration.Members.Add(new VariableDeclaration("MAX", "int", Literal.Number(10))
@@ -112,7 +114,10 @@ public void JavaScript_CombinesStaticWithAPrivateName()
 			Visibility = Visibility.Private,
 		});
 
-		StringAssert.Contains(new JavaScriptGenerator().Generate(declaration), "static #MAX = 10;", StringComparison.Ordinal);
+		string code = new JavaScriptGenerator().Generate(declaration);
+
+		Assert.Contains("static MAX = 10;", code, StringComparison.Ordinal);
+		Assert.DoesNotContain("#MAX", code, StringComparison.Ordinal);
 	}
 
 	/// <summary>

Coder.Test/Ast/FunctionModifierTests.cs

@@ -139,18 +139,20 @@ public void JavaScript_WritesStaticOnAMethod()
 	}
 
 	/// <summary>
-	/// A static private member keeps both spellings, which are separate parts of the same declaration.
+	/// A static private member keeps the one spelling JavaScript has for it. <c>static</c> is a
+	/// modifier in front of the name and survives; privacy is a <c>#</c> on the name itself, so
+	/// writing it would rename the method and leave every call to it naming the old one.
 	/// </summary>
 	[TestMethod]
-	public void JavaScript_WritesStaticAlongsideThePrivatePrefix()
+	public void JavaScript_WritesStaticOnAPrivateMethodWithoutRenamingIt()
 	{
 		ClassDeclaration declaration = SampleClass();
 		((FunctionDeclaration)declaration.Members[0]).Visibility = Visibility.Private;
 
-		Assert.Contains(
-			"static #distance(scale)",
-			new JavaScriptGenerator().Generate(declaration),
-			StringComparison.Ordinal);
+		string code = new JavaScriptGenerator().Generate(declaration);
+
+		Assert.Contains("static distance(scale)", code, StringComparison.Ordinal);
+		Assert.DoesNotContain("#distance", code, StringComparison.Ordinal);
 	}
 
 	/// <summary>

Coder.Test/Ast/VisibilityTests.cs

@@ -14,8 +14,9 @@ namespace ktsu.Coder.Test.Ast;
 /// </summary>
 /// <remarks>
 /// Visibility is the first thing the AST models that no two languages spell the same way — a C#
-/// keyword, a C++ access label, a JavaScript name prefix, and nothing at all in Python — so these
-/// cover each spelling rather than asserting one shape four times.
+/// keyword, a C++ access label, and nothing at all in Python or JavaScript, both of whose only
+/// convention is a spelling of the name itself and so would rename a declaration out from under
+/// every reference to it — so these cover each spelling rather than asserting one shape four times.
 /// </remarks>
 [TestClass]
 public class VisibilityTests
@@ -144,32 +145,68 @@ public void Cpp_TreatsInternalAsPublic()
 	}
 
 	/// <summary>
-	/// Tests that JavaScript spells a private member with the <c>#</c> prefix its own private syntax
-	/// uses, and leaves the others as ordinary members.
+	/// Tests that JavaScript drops visibility rather than renaming the declaration, since its private
+	/// syntax is a <c>#</c> on the name rather than a modifier in front of it.
 	/// </summary>
 	[TestMethod]
-	public void JavaScript_SpellsAPrivateMemberWithAHash()
+	public void JavaScript_DropsVisibilityRatherThanRenaming()
 	{
 		string code = new JavaScriptGenerator().Generate(SampleClass());
 
-		StringAssert.Contains(code, "#x = 0;", StringComparison.Ordinal);
-		StringAssert.Contains(code, "origin = 0;", StringComparison.Ordinal);
+		Assert.Contains("x = 0;", code, StringComparison.Ordinal);
+		Assert.Contains("origin = 0;", code, StringComparison.Ordinal);
 
-		// protected has no JavaScript spelling, so the method is an ordinary one.
-		StringAssert.Contains(code, "area() {", StringComparison.Ordinal);
+		// protected has no JavaScript spelling either, so the method is an ordinary one.
+		Assert.Contains("area() {", code, StringComparison.Ordinal);
+
+		Assert.DoesNotContain("#", code, StringComparison.Ordinal);
+		Assert.DoesNotContain("private", code, StringComparison.Ordinal);
 	}
 
 	/// <summary>
-	/// Tests that a private method is spelled with the prefix too, since JavaScript's <c>#</c> applies
-	/// to any class member rather than to fields alone.
+	/// Tests that a private method is left alone too, since the prefix applies to any class member
+	/// rather than to fields alone and so would strand a call to one just as it strands a read.
 	/// </summary>
 	[TestMethod]
-	public void JavaScript_SpellsAPrivateMethodWithAHash()
+	public void JavaScript_DropsVisibilityOnAMethodToo()
 	{
 		ClassDeclaration declaration = new("Point");
 		declaration.Members.Add(new FunctionDeclaration("recompute") { ReturnType = "void", Visibility = Visibility.Private });
 
-		StringAssert.Contains(new JavaScriptGenerator().Generate(declaration), "#recompute() {", StringComparison.Ordinal);
+		string code = new JavaScriptGenerator().Generate(declaration);
+
+		Assert.Contains("recompute() {", code, StringComparison.Ordinal);
+		Assert.DoesNotContain("#", code, StringComparison.Ordinal);
+	}
+
+	/// <summary>
+	/// Tests that a private member and the body that reads it come out naming the same identifier.
+	/// </summary>
+	/// <remarks>
+	/// This is the half the declaration tests above cannot see. <c>#</c> is a part of the name, so
+	/// prefixing a declaration renames it, and nothing prefixes a <see cref="VariableReference"/> —
+	/// the shared path writes one verbatim. A class whose method reads a private field would then
+	/// declare <c>#x</c> and read <c>x</c>, which resolves to nothing and throws a
+	/// <c>ReferenceError</c> under a module's implicit strict mode. Every name the body mentions has
+	/// to be one the class body declares, whichever way the generator spells it.
+	/// </remarks>
+	[TestMethod]
+	public void JavaScript_ReferencesThePrivateMemberItDeclared()
+	{
+		ClassDeclaration declaration = new("Counter");
+		declaration.Members.Add(new VariableDeclaration("count", "int", Literal.Number(0)) { Visibility = Visibility.Private });
+
+		FunctionDeclaration increment = new("increment") { ReturnType = "void" };
+		increment.Body.Add(new ReturnStatement(new VariableReference("count")));
+		declaration.Members.Add(increment);
+
+		string code = new JavaScriptGenerator().Generate(declaration);
+
+		// The declared name and the referenced name are the same one, so the body reads what the
+		// class holds rather than an identifier nothing declares.
+		Assert.Contains("count = 0;", code, StringComparison.Ordinal);
+		Assert.Contains("return count;", code, StringComparison.Ordinal);
+		Assert.DoesNotContain("#count", code, StringComparison.Ordinal);
 	}
 
 	/// <summary>

Coder/Languages/JavaScriptGenerator.cs

@@ -279,7 +279,7 @@ protected override void GenerateFieldDeclaration(FieldDeclaration field, CodeBlo
 		Ensure.NotNull(code);
 
 		GenerateDocumentation(field, code);
-		code.Write(MemberName(field.Name ?? "unnamed", field.Visibility));
+		code.Write(field.Name ?? "unnamed");
 
 		if (field.InitialValue is not null)
 		{
@@ -317,9 +317,12 @@ protected override void GenerateFunctionDeclaration(FunctionDeclaration funcDecl
 	/// JavaScript's <c>function</c> keyword is a syntax error there. That is why the members are
 	/// emitted here rather than through <see cref="StandardLanguageGenerator.GenerateClassMembers"/>.
 	/// <para>
-	/// A private member is spelled with the <c>#</c> prefix, which is JavaScript's own private syntax
-	/// and enforced by the runtime. The other three visibilities have no spelling: JavaScript draws
-	/// the line at private, and a <c>protected</c> or <c>internal</c> member is an ordinary one.
+	/// No visibility is spelled, private included. JavaScript's <c>#</c> is a part of the name rather
+	/// than a modifier in front of it, so writing it renames the declaration, and nothing renames the
+	/// references: a <see cref="VariableReference"/> is emitted verbatim by the shared path, leaving
+	/// a body that names an identifier the class no longer declares. That is the reason
+	/// <see cref="PythonGenerator"/> drops its leading underscore and <see cref="GoGenerator"/> notes
+	/// the case of a name rather than changing it, and it is the same reason here.
 	/// </para>
 	/// </remarks>
 	protected override void GenerateClassDeclaration(ClassDeclaration classDecl, CodeBlocker code)
@@ -398,7 +401,7 @@ private void GenerateField(VariableDeclaration field, CodeBlocker code)
 			code.Write(StaticKeyword);
 		}
 
-		code.Write(MemberName(field.Name, field.Visibility));
+		code.Write(field.Name);
 
 		if (field.InitialValue is not null)
 		{
@@ -438,7 +441,7 @@ private void GenerateMethod(FunctionDeclaration method, CodeBlocker code)
 
 		code.Write(method.Kind == FunctionKind.Constructor
 			? "constructor"
-			: MemberName(method.Name ?? "unnamedMethod", method.Visibility));
+			: method.Name ?? "unnamedMethod");
 
 		code.Write("(");
 		GenerateParameterList(method.Parameters, code);
@@ -503,19 +506,6 @@ protected override void GenerateVariableDeclaration(VariableDeclaration varDecl,
 		EndStatement(code);
 	}
 
-	/// <summary>
-	/// Spells a class member's name for its visibility.
-	/// </summary>
-	/// <param name="name">The member's name in the AST.</param>
-	/// <param name="visibility">The visibility it was declared with.</param>
-	/// <returns>The name as the class body should spell it.</returns>
-	/// <remarks>
-	/// <c>#</c> is a part of the name in JavaScript rather than a modifier in front of it, so private
-	/// members are spelled here rather than by writing a keyword before the declaration.
-	/// </remarks>
-	private static string MemberName(string name, Visibility visibility) =>
-		visibility == Visibility.Private ? $"#{name}" : name;
-
 	/// <inheritdoc/>
 	/// <remarks>
 	/// JavaScript has no entry point of its own — a module runs top to bottom — so the function is

README.md

@@ -91,7 +91,7 @@ rather than the modifier's text because no two languages spell visibility the sa
 | C | Nothing inside a struct, which has no access control; a `Private` declaration at file scope is `static`, which is the internal linkage C has instead |
 | Rust | `pub`, or `pub(crate)` for `Internal` and `Protected`; `Private` writes nothing, which is already Rust's default. `Unspecified` is `pub`, since a generated type nothing outside the module can read is not what saying nothing asked for |
 | Go | Nothing. Go exports a name whose first letter is a capital and has no keyword at all, so a declaration whose name disagrees with what it asked for gets a note — renaming it would not rename the references to it. `Internal` is exactly Go's unexported, and `Private` and `Protected` are as near as there is |
-| JavaScript | A private class member takes the `#` prefix, which is JavaScript's own private syntax; nothing for the rest |
+| JavaScript | Nothing — the `#` that makes a class member private is a part of the name rather than a modifier in front of it, so writing one renames the declaration and leaves every reference to it naming something the class no longer declares |
 | Python | Nothing — Python has no access modifiers, and its leading-underscore convention renames the declaration rather than modifying it |
 
 ### Constants and entry points
@@ -211,7 +211,7 @@ directly.
 |---|---|---|---|
 | `python` | `PythonGenerator` | `py` | Type hints, `None` for void, `pass` for an empty body or class; `self` on methods; `main` with its `__main__` guard |
 | `csharp` | `CSharpGenerator` | `cs` | Mapped type names, `var` for inferred declarations, visibility keywords, `const`, `static Main` |
-| `javascript` | `JavaScriptGenerator` | `js` | Untyped; `const`/`let`; strict `===` and `!==`; method, `static` and `#private` syntax inside a class |
+| `javascript` | `JavaScriptGenerator` | `js` | Untyped; `const`/`let`; strict `===` and `!==`; method and `static` syntax inside a class |
 | `cpp` | `CppGenerator` | `cpp` | Mapped type spellings (`str` → `std::string`); `auto` for inferred declarations; access labels, `static constexpr` members and a terminating `;` on a class |
 | `c` | `CGenerator` | `c` | `typedef struct` for every kind of type; a member function is a free `Type_name(Type* self, …)`; an interface is a struct of function pointers; a base type is the first member; enumeration members are qualified by their enumeration; `_Static_assert`, `main(void)`, and `static const` for a constant |
 | `rust` | `RustGenerator` | `rs` | A `struct` for the data and an `impl` block for the behaviour; an interface is a `trait` and a base type on one is a supertrait; a destructor is `impl Drop`, an operator is its `std::ops` trait, a conversion is `impl From`, and a specialisation is `impl Trait for Type`; `#[repr]`, `#[must_use]`, `const fn`, and `const _: () = assert!(…)` |