Select a type class instance by type identity, not by printed name

The fallback registry keyed implementations on the printed type. A class prints
as its simple name, so two classes of the same name in different packages
collided: the second registration was dropped and its values silently dispatched
through the first's implementation, with no error. Implementations are now held
per requirement and matched by structural type equality.

The printed form also fed those keys and reaches specialization names, while the
binding it prints is a hash map, so identical input could produce different
symbols between runs. The names are sorted, and with the keys no longer built
from text the printing is once again only for reading.

Author: Frotty

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

@@ -3,6 +3,8 @@
 import de.peeeq.wurstscript.jassIm.*;
 
 import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.StringJoiner;
 import java.util.stream.Collectors;
@@ -545,15 +547,16 @@ private static void printTypeArguments(ImTypeArguments typeArguments, int indent
                 // Show the type class instances travelling with the argument: whether they are
                 // present is the whole question when a bound fails to dispatch.
                 if (!ta.getTypeClassBinding().isEmpty()) {
-                    append(sb, "{");
-                    boolean firstBinding = true;
+                    // Sorted: this printing reaches specialization names and so the emitted symbols,
+                    // which have to be identical for identical input. The binding is a hash map, so
+                    // its iteration order is not.
+                    List<String> requirements = new ArrayList<>();
                     for (ImTypeClassFunc requirement : ta.getTypeClassBinding().keySet()) {
-                        if (!firstBinding) {
-                            append(sb, ", ");
-                        }
-                        append(sb, requirement.getName());
-                        firstBinding = false;
+                        requirements.add(requirement.getName());
                     }
+                    Collections.sort(requirements);
+                    append(sb, "{");
+                    append(sb, String.join(", ", requirements));
                     append(sb, "}");
                 }
                 first = false;

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

@@ -1714,29 +1714,45 @@ public ImTypeClassFunc getTypeClassFunc(FuncDef method) {
     private final Map<FuncDef, ImTypeClassFunc> typeClassFuncs = new LinkedHashMap<>();
 
     /**
-     * Every type class implementation in the program, keyed by requirement and the type it is for.
+     * Every type class implementation in the program, held per requirement against the type it is
+     * for.
      * <p>
      * A type argument can carry its instances directly, but that binding lives on the argument
      * position and is lost as soon as a type variable is substituted into a plain type, which is
      * what happens when a generic class type travels through a return type or a receiver. Instance
-     * selection is static, so keying on the concrete type is both simpler and robust: the type at a
-     * dispatch site is enough to find the implementation, however it got there.
+     * selection is static, so recording the type is enough to recover it.
+     * <p>
+     * Matched by structural type equality rather than by printed name: a class prints as its simple
+     * name, so two classes of the same name in different packages would otherwise collide and the
+     * second would silently dispatch through the first. The lists hold one entry per instance of a
+     * requirement, so scanning them is cheaper than the printing it replaces.
      */
-    private final Map<ImTypeClassFunc, Map<String, ImFunction>> typeClassImpls = new LinkedHashMap<>();
+    private final Map<ImTypeClassFunc, List<TypeClassImpl>> typeClassImpls = new LinkedHashMap<>();
 
-    public void registerTypeClassImpl(ImTypeClassFunc requirement, ImType instanceType, ImFunction impl) {
-        typeClassImpls.computeIfAbsent(requirement, r -> new LinkedHashMap<>())
-                .putIfAbsent(typeClassKey(instanceType), impl);
+    private record TypeClassImpl(ImType instanceType, ImFunction impl) {
     }
 
-    public @Nullable ImFunction lookupTypeClassImpl(ImTypeClassFunc requirement, ImType instanceType) {
-        Map<String, ImFunction> byType = typeClassImpls.get(requirement);
-        return byType == null ? null : byType.get(typeClassKey(instanceType));
+    public void registerTypeClassImpl(ImTypeClassFunc requirement, ImType instanceType, ImFunction impl) {
+        List<TypeClassImpl> impls = typeClassImpls.computeIfAbsent(requirement, r -> new ArrayList<>());
+        for (TypeClassImpl existing : impls) {
+            if (existing.instanceType().equalsType(instanceType)) {
+                return;
+            }
+        }
+        impls.add(new TypeClassImpl(instanceType, impl));
     }
 
-    /** Structural key for an instance type; concrete types print stably. */
-    private static String typeClassKey(ImType type) {
-        return type.toString();
+    public @Nullable ImFunction lookupTypeClassImpl(ImTypeClassFunc requirement, ImType instanceType) {
+        List<TypeClassImpl> impls = typeClassImpls.get(requirement);
+        if (impls == null) {
+            return null;
+        }
+        for (TypeClassImpl candidate : impls) {
+            if (candidate.instanceType().equalsType(instanceType)) {
+                return candidate.impl();
+            }
+        }
+        return null;
     }
 
     public ImTypeVar getTypeVar(TypeParamDef tp) {

de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/TypeClassTests.java

@@ -800,6 +800,53 @@ public void genericRequirement() {
         );
     }
 
+    /**
+     * Two classes with the same simple name in different packages, dispatched through a bounded
+     * generic class so the lookup by type is the path used. Selecting by printed name made the
+     * second silently dispatch through the first's implementation.
+     */
+    @Test
+    public void sameSimpleNameThroughRegistryFallback() {
+        testAssertOkLines(true,
+            "package Iface",
+            "public interface Show<T:>",
+            "    function show(T x) returns string",
+            "public class Renderer<Q: Show>",
+            "    function render(Q x) returns string",
+            "        return Q.show(x)",
+            "endpackage",
+            "",
+            "package First",
+            "import public Iface",
+            "public class Item",
+            "implements Show<Item>",
+            "    function show(Item x) returns string",
+            "        return \"first\"",
+            "public function firstResult() returns string",
+            "    return new Renderer<Item>().render(new Item())",
+            "endpackage",
+            "",
+            "package Second",
+            "import public Iface",
+            "public class Item",
+            "implements Show<Item>",
+            "    function show(Item x) returns string",
+            "        return \"second\"",
+            "public function secondResult() returns string",
+            "    return new Renderer<Item>().render(new Item())",
+            "endpackage",
+            "",
+            "package test",
+            "import First",
+            "import Second",
+            "native testSuccess()",
+            "init",
+            "    if firstResult() == \"first\" and secondResult() == \"second\"",
+            "        testSuccess()",
+            "endpackage"
+        );
+    }
+
     /** A type parameter is not a value, so it may only appear as the receiver of a requirement. */
     @Test
     public void typeParameterIsNotAValue() {