Sanitise Lua method names where they are assigned

.gitignore

@@ -60,3 +60,5 @@ de.peeeq.wurstscript/output.txt
 /HelperScripts/.gradle
 /.gradle-user-home
 /gradle-home-temp
+/.claude/
+/LOOP.md

BACKLOG.md

@@ -0,0 +1,115 @@
+# Language feature backlog
+
+Working notes for ongoing work on type class bounds for `T:` generics
+(shipped in #1226, #1228, #1229). Ordered: take the top unblocked item.
+
+Keep this file current. It is the only memory that survives between sessions.
+When an item is finished, move it to Done with one line on what actually
+happened. When something is learned that would have saved time, write it under
+Notes rather than leaving it in a commit message.
+
+## Todo
+
+Numbering is stable: finished items leave a gap rather than shifting the ones below,
+because `LOOP.md` refers to items by number.
+
+3. **`slotFor` is bound to `get`'s implementation** in the emitted Lua — confirmed real, and
+   not an artefact of item 1: it survives sanitisation unchanged. Diagnosed, not yet fixed.
+   The alias sets in `LuaDispatchPreparation` decide which slots a method claims, and
+   `sharesSemanticName` accepts a match on *either* of two names: the source name (`get`,
+   `slotFor`) or `semanticNameFromMethodName`, which is the substring after the last
+   underscore. For a specialised method that substring is a type-argument fragment —
+   `FastHashMap_get_specialized__integer__integer___integer` and the `slotFor` one both yield
+   `integer` — so two unrelated methods count as sharing a name. They also share a dispatch
+   signature here (`(pos) returns int` both), which is the other half of the guard, so `get`
+   claims `slotFor`'s slot. Fix: when both methods have a real source name, that should decide;
+   the substring heuristic is a fallback for when there is no trace to ask, not an alternative.
+   Watch the closure and bridge cases in `TypeClassTests`/`LuaBackendAuditTests` — they are what
+   the loose match was presumably widened for.
+
+4. **Finish the FastHashMap proof.** `FastHashMapTests` is the first real use of bounds.
+   Add `remove` with tombstones, and an assertion that the emitted code stays cheap: no
+   dispatch node, no instance dictionary, and no WC3 hashtable natives — array access only,
+   which is the whole point versus `HashMap extends Table`.
+
+5. **Lua dispatch inside a closure.** Works on Jass since #1229. On Lua the specialised class
+   is built correctly but nothing calls it, because the closure is reached through its
+   interface and `specializeMethod` renames the method out of its dispatch slot.
+   `TypeClassTests.dispatchInsideClosureIsRejectedForLua` pins the current diagnostic and
+   should become a success test. Related to item 1; AGENTS.md flags this machinery.
+
+6. **Lua dispatch inside the constructor** of a bounded generic class. Works on Jass.
+
+7. **Module bounds.** `module M<T: Show>` is rejected with a clear message today. Needs
+   receiver rewriting during expansion, or type parameters on `ModuleInstanciation`.
+
+8. **`MOD_INT`/`DIV_INT` return the left operand's type** rather than `int`
+   (`AttrExprType.java`, the `case MOD_INT` branch), where `caseMathOperation` returns
+   `WurstTypeInt.instance()` for `+`, `-`, `*`. Only observable if something is a proper
+   subtype of int, so it may be harmless — establish whether it is reachable, then either fix
+   it or leave a comment saying why the asymmetry is intended. Small.
+
+9. **Keep `WURST_LANGUAGE.md` and `CHANGELOG.md` current** as items land. The bounds section
+   says nothing about closures, which now work on Jass. Fold this into whichever item changes
+   the behaviour rather than doing it as a separate pass.
+
+10. **One `ImTypeVar` per type parameter.** Name-tolerant lookups remain in
+   `EliminateGenerics.indexOfTypeVar`, `inheritTypeClassBinding` and
+   `ProgramState.getCurrentTypeArgument`, compensating for several nodes standing for one
+   source parameter. Making the node canonical lets all three compare by identity and removes
+   a class of silent wrong dispatch. Mechanical, well covered by the suite.
+
+11. **Jass temp counter is not reset between compilations.** Two runs of the same commit emit
+    different `.j` (`temp151` vs `temp8`) because the counter is JVM-wide and depends on how
+    many tests ran before. Not wrong for compiling one map, but it means `.j` cannot be diffed
+    across runs to validate a change — only `.lua` can. Fixing it would make Jass diffable.
+
+12. **Standing item, never finished.** When nothing above is left, find the next thing worth
+    doing and add it here rather than stopping. Good sources, in order: a test that would have
+    caught a bug already found; a place where two mechanisms do the same job and disagree; a
+    comment claiming something the code no longer does; a path where a wrong result is silent
+    rather than loud. Add what is found as a numbered item and start on it.
+
+## Blocked on a decision
+
+- **Eliminating the remaining `castTo int`.** The motivating case is timer data attachment
+  (`ClosureTimers.wurst`), and the containers behind it: `Table` has 81 casts, `HashList` 13,
+  `HashSet` 6, `HashMap` 4. None can adopt bounds as things stand, because an instance is
+  declared one type at a time and these accept any type. It needs a way to give an instance
+  for a whole family — every class type, or every handle type — which is a language design
+  question: what the syntax is, where such an instance may be declared under the orphan rule,
+  and whether a specific instance always beats a family one. Do not start this autonomously.
+
+## Out of scope
+
+- The stdlib itself. `de.peeeq.wurstscript/temp/WurstStdlib2` is a fetched artefact for tests;
+  editing it changes nothing real. `FastHashMap` ships from the WurstStdlib2 repo once the
+  compiler-side proof is complete, and that is a separate decision.
+
+## Done
+
+- 1 + 2. Lua method names are sanitised where they are assigned, not where they are printed.
+  `LuaDispatchPreparation.normalizeMethodNames` is the pass that gives one name to a whole
+  dispatch group, so it now sanitises before uniquing — two names differing only in characters
+  Lua has no place for still get a slot each. `LuaTranslator` maps every slot key and every
+  `LuaMethod` name through the same function, so call sites and class tables agree. Lua's
+  identifier rule now lives in one place, `LuaIdentifiers`. `LuaAssertions.assertNamesAreValidIdentifiers`
+  walks the emitted Lua and fails on any name that is not an identifier; it runs for every
+  `testLua` compile, so the silent two-target-assignment case cannot come back.
+- Substitution now carries the type class binding with the type (#1229). Also fixed the
+  type-variable reference on `ImTypeVarDispatch`, which a walk over types alone missed.
+
+## Notes
+
+- `%` is real modulo in Wurst; `mod` is integer modulo. `int % 8` types as `real`.
+- Emitted Lua must be byte-identical for identical input (AGENTS.md §8). It is the only
+  emitted output that can be diffed across runs — see item 11.
+- Method names are not what the frontend called them. `LuaDispatchPreparation` renames a whole
+  dispatch group to one name and attaches alias sets, and only then does the backend run. A
+  question about which Lua slot something lands in is a question about that pass, not about
+  `LuaTranslator`.
+- Tests run five Jass configurations plus the interpreter, then the Lua target separately.
+  `testAssertOkLines(true, ...)` covers both the pre-transform interpreter and full
+  monomorphisation, so it is a stronger check than it looks.
+- The stdlib copy under `de.peeeq.wurstscript/temp/WurstStdlib2` is a fetched artefact for
+  tests. Real stdlib changes belong in the WurstStdlib2 repo, not here.

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/LuaDispatchPreparation.java

@@ -11,6 +11,7 @@
 import de.peeeq.wurstscript.jassIm.ImProg;
 import de.peeeq.wurstscript.jassIm.ImType;
 import de.peeeq.wurstscript.jassIm.ImVars;
+import de.peeeq.wurstscript.translation.lua.translation.LuaIdentifiers;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -99,7 +100,9 @@ private static void normalizeMethodNames(ImProg prog, List<ImMethod> allMethods)
                 continue;
             }
             group.sort(Comparator.comparing(LuaDispatchPreparation::methodSortKey));
-            String name = uniqueName(group.get(0).getName(), usedNames);
+            // The name is about to become a Lua table key. Sanitising before uniquing means two
+            // names that only differed in characters Lua has no place for still get one slot each.
+            String name = uniqueName(LuaIdentifiers.toIdentifier(group.get(0).getName()), usedNames);
             for (ImMethod method : group) {
                 method.setName(name);
             }

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaAssertions.java

@@ -1,9 +1,19 @@
 package de.peeeq.wurstscript.translation.lua.translation;
 
+import de.peeeq.wurstscript.luaAst.Element;
+import de.peeeq.wurstscript.luaAst.LuaCompilationUnit;
+import de.peeeq.wurstscript.luaAst.LuaExprFieldAccess;
+import de.peeeq.wurstscript.luaAst.LuaExprFunctionCallByName;
+import de.peeeq.wurstscript.luaAst.LuaFunction;
+import de.peeeq.wurstscript.luaAst.LuaMethod;
+import de.peeeq.wurstscript.luaAst.LuaTableNamedField;
+import de.peeeq.wurstscript.luaAst.LuaVariable;
+
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.TreeSet;
 
 /**
  * Static assertion helpers for the Lua backend.
@@ -15,6 +25,66 @@ public class LuaAssertions {
 
     private LuaAssertions() {}
 
+    /**
+     * Asserts that every name the backend emits is a Lua identifier.
+     *
+     * <p>A name that is not one usually breaks the syntax check, but not always: a table key
+     * containing a comma parses as an assignment to two targets and quietly stores the value in
+     * the wrong place. Checking the names themselves catches that case at the point it is
+     * introduced, rather than as a wrong result at runtime.
+     */
+    public static void assertNamesAreValidIdentifiers(LuaCompilationUnit luaCode) {
+        Set<String> invalid = new TreeSet<>();
+        luaCode.accept(new Element.DefaultVisitor() {
+            private void check(String kind, String name) {
+                // A vararg parameter is the one name that is legal without being an identifier.
+                if (!LuaIdentifiers.isValid(name) && !LuaIdentifiers.VARARG.equals(name)) {
+                    invalid.add(kind + " '" + name + "'");
+                }
+            }
+
+            @Override
+            public void visit(LuaFunction f) {
+                super.visit(f);
+                check("function", f.getName());
+            }
+
+            @Override
+            public void visit(LuaMethod m) {
+                super.visit(m);
+                check("method", m.getName());
+            }
+
+            @Override
+            public void visit(LuaVariable v) {
+                super.visit(v);
+                check("variable", v.getName());
+            }
+
+            @Override
+            public void visit(LuaExprFieldAccess fa) {
+                super.visit(fa);
+                check("field", fa.getFieldName());
+            }
+
+            @Override
+            public void visit(LuaTableNamedField f) {
+                super.visit(f);
+                check("field", f.getFieldName());
+            }
+
+            @Override
+            public void visit(LuaExprFunctionCallByName call) {
+                super.visit(call);
+                check("call to", call.getFuncName());
+            }
+        });
+        if (!invalid.isEmpty()) {
+            throw new RuntimeException("Wurst Lua backend assertion failed: emitted names are not Lua identifiers: "
+                + String.join(", ", invalid));
+        }
+    }
+
     /**
      * Asserts that every emitted call to {@code __wurst_GetHandleId} has a helper definition.
      *

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaIdentifiers.java

@@ -0,0 +1,67 @@
+package de.peeeq.wurstscript.translation.lua.translation;
+
+/**
+ * Lua's rule for what a generated name may look like, kept in one place.
+ *
+ * <p>Names from the intermediate language are not constrained to Lua's identifier syntax;
+ * specialised generics, for example, are named after their type arguments. Sanitising in the
+ * backend keeps that rule where it belongs rather than requiring every earlier pass to know
+ * about Lua. Any collisions the mapping introduces are resolved by the usual uniquing.
+ */
+public final class LuaIdentifiers {
+
+    /** Lua's vararg parameter, which is a legal parameter name but not an identifier. */
+    public static final String VARARG = "...";
+
+    /**
+     * Whether {@code name} can be used as-is as a Lua identifier or table key.
+     *
+     * <p>A keyword is spelled like an identifier and is not one. Wurst reserves a different set, so
+     * a method can be declared {@code repeat} or {@code goto} and reach the backend under that
+     * name; emitted as a table key it is a syntax error rather than a wrong result, but this is the
+     * check that is supposed to catch it first.
+     */
+    public static boolean isValid(String name) {
+        if (name == null || name.isEmpty() || isDigit(name.charAt(0))) {
+            return false;
+        }
+        for (int i = 0; i < name.length(); i++) {
+            if (!isIdentifierPart(name.charAt(i))) {
+                return false;
+            }
+        }
+        return !LuaReservedNames.LUA_KEYWORDS.contains(name);
+    }
+
+    /** Maps any name onto a Lua identifier, leaving names that already are one untouched. */
+    public static String toIdentifier(String name) {
+        if (isValid(name)) {
+            return name;
+        }
+        StringBuilder sb = new StringBuilder(name.length());
+        for (int i = 0; i < name.length(); i++) {
+            char c = name.charAt(i);
+            sb.append(isIdentifierPart(c) ? c : '_');
+        }
+        if (sb.length() == 0 || isDigit(sb.charAt(0))) {
+            sb.insert(0, '_');
+        }
+        // A trailing underscore rather than a counter, so the name a keyword maps to is the same
+        // wherever it is derived - call sites and class tables have to agree without consulting
+        // each other.
+        while (LuaReservedNames.LUA_KEYWORDS.contains(sb.toString())) {
+            sb.append('_');
+        }
+        return sb.toString();
+    }
+
+    private static boolean isIdentifierPart(char c) {
+        return c == '_' || (c < 128 && Character.isLetterOrDigit(c));
+    }
+
+    private static boolean isDigit(char c) {
+        return c >= '0' && c <= '9';
+    }
+
+    private LuaIdentifiers() {}
+}

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java

@@ -135,7 +135,9 @@ public LuaFunction initFor(ImFunction a) {
         @Override
         public LuaMethod initFor(ImMethod a) {
             LuaExpr receiver = LuaAst.LuaExprVarAccess(luaClassVar.getFor(a.attrClass()));
-            return LuaAst.LuaMethod(receiver, a.getName(), LuaAst.LuaParams(), LuaAst.LuaStatements());
+            // A method name is a table key, so it must be an identifier - but unlike a variable
+            // it must not be uniqued: every override has to keep landing in the same slot.
+            return LuaAst.LuaMethod(receiver, dispatchSlotName(a.getName()), LuaAst.LuaParams(), LuaAst.LuaStatements());
         }
     };
 
@@ -196,28 +198,8 @@ public LuaTranslator(ImProg prog, ImTranslator imTr) {
         luaModel = LuaAst.LuaCompilationUnit();
     }
 
-    /**
-     * Makes an intermediate-language name usable as a Lua identifier.
-     * <p>
-     * Names from the IM are not constrained to Lua's identifier syntax; specialised generics, for
-     * example, are named after their type arguments. Sanitising here keeps that rule where it
-     * belongs, in the backend, rather than requiring every earlier pass to know about Lua. Any
-     * collisions the mapping introduces are resolved by the usual uniquing.
-     */
-    private static String toLuaIdentifier(String name) {
-        StringBuilder sb = new StringBuilder(name.length());
-        for (int i = 0; i < name.length(); i++) {
-            char c = name.charAt(i);
-            sb.append(c == '_' || Character.isLetterOrDigit(c) && c < 128 ? c : '_');
-        }
-        if (sb.length() == 0 || Character.isDigit(sb.charAt(0))) {
-            sb.insert(0, '_');
-        }
-        return sb.toString();
-    }
-
     protected String uniqueName(String rawName) {
-        String name = toLuaIdentifier(rawName);
+        String name = LuaIdentifiers.toIdentifier(rawName);
         Integer nextIndex = uniqueNameCounters.get(name);
         if (nextIndex == null) {
             uniqueNameCounters.put(name, 1);
@@ -462,7 +444,7 @@ private void collectMethodNames(ImClass c, Set<String> methodNames, Set<ImClass>
         }
         visited.add(c);
         for (ImMethod method : c.getMethods()) {
-            methodNames.add(method.getName());
+            methodNames.add(dispatchSlotName(method.getName()));
         }
         for (ImClassType sc : c.getSuperClasses()) {
             collectMethodNames(sc.getClassDef(), methodNames, visited);
@@ -914,7 +896,7 @@ private void createMethods(ImClass c, LuaVariable classVar) {
             ImMethod chosen = chosenByGroup.get(groupMethods);
             Set<String> memberNames = new HashSet<>();
             for (ImMethod m : groupMethods) {
-                memberNames.add(m.getName());
+                memberNames.add(dispatchSlotName(m.getName()));
             }
             Set<String> slotNames = collectDispatchSlotNames(c, groupMethods);
             for (String slotName : slotNames) {
@@ -934,7 +916,7 @@ private void createMethods(ImClass c, LuaVariable classVar) {
             ImMethod chosen = chosenByGroup.get(groupMethods);
             Set<String> memberNames = new HashSet<>();
             for (ImMethod m : groupMethods) {
-                memberNames.add(m.getName());
+                memberNames.add(dispatchSlotName(m.getName()));
             }
             for (String slotName : collectDispatchSlotNames(c, groupMethods)) {
                 if (memberNames.contains(slotName)) {
@@ -973,6 +955,15 @@ && implArity(chosen) != implArity(current)) {
         }
     }
 
+    /**
+     * The Lua table key a dispatch slot is emitted under. Aliases and class-qualified names are
+     * built from IM names, which may contain characters Lua has no place for; the mapping has to
+     * be the same one call sites go through, so that a slot is still found under its new name.
+     */
+    private String dispatchSlotName(String rawName) {
+        return LuaIdentifiers.toIdentifier(rawName);
+    }
+
     private Set<String> collectDispatchSlotNames(ImClass receiverClass, List<ImMethod> groupMethods) {
         Set<String> slotNames = new TreeSet<>();
         Set<String> semanticNames = new TreeSet<>();
@@ -982,7 +973,7 @@ private Set<String> collectDispatchSlotNames(ImClass receiverClass, List<ImMetho
             }
             for (String alias : m.getLuaMethodDispatchAliases()) {
                 if (alias != null && !alias.isEmpty()) {
-                    slotNames.add(alias);
+                    slotNames.add(dispatchSlotName(alias));
                 }
             }
             String semanticName = semanticNameFromMethodName(m.getName());
@@ -999,7 +990,7 @@ private Set<String> collectDispatchSlotNames(ImClass receiverClass, List<ImMetho
             collectClassNamesInHierarchy(receiverClass, classNames, new HashSet<>());
             for (String className : classNames) {
                 for (String semanticName : semanticNames) {
-                    slotNames.add(className + "_" + semanticName);
+                    slotNames.add(dispatchSlotName(className + "_" + semanticName));
                 }
             }
         }