Address review: check instance signatures, abstract bounds and invalid bounds

Instance methods were matched by name only, so an implementation with the wrong
parameter types, parameter count or return type was accepted and emitted, leaving
the error to pjass. Compare against the requirement with the interface's type
parameter replaced by the instance type, and report the expected signature.

A type argument which is itself a type parameter was accepted unconditionally.
That let a generic without the bound pass its parameter to one that needs it: the
call type checked but had no instance to dispatch to at runtime. Such an argument
now has to declare the bound itself.

Bounds which do not name a usable type class were silently dropped, so a bound on
a class or an applied interface became a no-op. They are reported at the bound.

Interfaces which extend another interface are rejected as bounds, at the bound and
at the instance: the requirements of a bound are the interface's own functions, so
allowing inheritance would silently omit the inherited ones.

Author: Frotty

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/types/TypeClassConstraints.java

@@ -21,8 +21,8 @@ private TypeClassConstraints() {
 
     /**
      * The interfaces named as bounds of the given type parameter, in source order.
-     * Bounds which do not resolve to a single-parameter interface are skipped here; the validator
-     * reports those separately so that a bad bound does not cascade into every use site.
+     * Unusable bounds are skipped; {@link #invalidBoundReason} explains why, and the validator
+     * reports it at the bound itself so a bad bound does not cascade into every use site.
      */
     public static List<InterfaceDef> boundInterfaces(TypeParamDef tp) {
         List<TypeExpr> exprs = boundExprs(tp);
@@ -58,22 +58,49 @@ public static boolean hasBounds(TypeParamDef tp) {
      * plain reference to a single-parameter interface.
      */
     public static @Nullable InterfaceDef resolveBound(TypeExpr boundExpr) {
+        return invalidBoundReason(boundExpr) == null ? namedInterface(boundExpr) : null;
+    }
+
+    /**
+     * Explains why a bound cannot be used as a type class, or null when it is usable.
+     * <p>
+     * A bound must name an interface, unapplied, with exactly one type parameter, and that
+     * interface must not extend another. The last restriction keeps the set of requirements equal
+     * to the interface's own methods, so an instance cannot silently miss an inherited one.
+     */
+    public static @Nullable String invalidBoundReason(TypeExpr boundExpr) {
         if (!(boundExpr instanceof TypeExprSimple simple)) {
-            return null;
+            return "A bound must name an interface.";
         }
         // A bound names the interface unapplied: writing the type argument would be redundant,
         // because it is always the type parameter being constrained.
         if (!simple.getTypeArgs().isEmpty()) {
-            return null;
+            return "A bound must name the interface without type arguments, because the argument is"
+                    + " always the type parameter being constrained.";
         }
         TypeDef def = boundExpr.lookupType(simple.getTypeName(), false);
+        if (def == null) {
+            return "Could not find " + simple.getTypeName() + ".";
+        }
         if (!(def instanceof InterfaceDef i)) {
-            return null;
+            return simple.getTypeName() + " is not an interface, so it cannot be used as a bound.";
         }
         if (i.getTypeParameters().size() != 1) {
+            return i.getName() + " must have exactly one type parameter to be used as a bound, but has "
+                    + i.getTypeParameters().size() + ".";
+        }
+        if (!i.getExtendsList().isEmpty()) {
+            return i.getName() + " extends another interface, which is not supported for bounds:"
+                    + " the requirements of a bound are the interface's own functions.";
+        }
+        return null;
+    }
+
+    private static @Nullable InterfaceDef namedInterface(TypeExpr boundExpr) {
+        if (!(boundExpr instanceof TypeExprSimple simple)) {
             return null;
         }
-        return i;
+        return boundExpr.lookupType(simple.getTypeName(), false) instanceof InterfaceDef i ? i : null;
     }
 
     /**

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/validation/WurstValidator.java

@@ -405,6 +405,8 @@ private void check(Element e) {
                 checkConstructorsUnique((ClassOrModule) e);
             if (e instanceof InstanceDecl)
                 checkInstanceDecl((InstanceDecl) e);
+            if (e instanceof TypeParamDef)
+                checkTypeParamBounds((TypeParamDef) e);
             if (e instanceof StmtCall)
                 checkCallBounds((StmtCall) e);
             if (e instanceof CompilationUnit)
@@ -2600,16 +2602,51 @@ private void checkCallBounds(StmtCall call) {
      * where the type argument is chosen so the message can name both.
      */
     private void checkBoundsSatisfied(Element location, TypeParamDef tp, WurstType typ) {
-        if (typ instanceof WurstTypeUnknown || typ instanceof WurstTypeTypeParam
-                || typ instanceof WurstTypeBoundTypeParam) {
-            // still abstract, or already broken: checked once it is bound to something concrete
+        List<InterfaceDef> bounds = TypeClassConstraints.boundInterfaces(tp);
+        if (bounds.isEmpty() || typ instanceof WurstTypeUnknown) {
             return;
         }
-        for (InterfaceDef bound : TypeClassConstraints.boundInterfaces(tp)) {
-            if (TypeClassInstances.find(bound, typ) == null) {
-                location.addError("Type " + typ + " does not satisfy the bound " + tp.getName() + ": "
-                        + bound.getName() + ".\nDeclare 'implements " + bound.getName() + "<" + typ + ">' in the package of "
-                        + bound.getName() + " or of " + typ + ".");
+        WurstType normalized = typ.normalize();
+        TypeParamDef abstractArg = asTypeParam(normalized);
+        if (abstractArg != null) {
+            // The argument is another type parameter, so no instance exists yet. It can only supply
+            // the bound if it declares it itself; otherwise the call is unsatisfiable no matter what
+            // the outer generic is later instantiated with.
+            for (InterfaceDef bound : bounds) {
+                if (!TypeClassConstraints.boundInterfaces(abstractArg).contains(bound)) {
+                    location.addError("Type parameter " + abstractArg.getName() + " does not satisfy the bound "
+                            + tp.getName() + ": " + bound.getName() + ".\nAdd the bound to " + abstractArg.getName()
+                            + ", as in <" + abstractArg.getName() + ": " + bound.getName() + ">.");
+                }
+            }
+            return;
+        }
+        for (InterfaceDef bound : bounds) {
+            if (TypeClassInstances.find(bound, normalized) == null) {
+                location.addError("Type " + normalized + " does not satisfy the bound " + tp.getName() + ": "
+                        + bound.getName() + ".\nDeclare 'implements " + bound.getName() + "<" + normalized
+                        + ">' in the package of " + bound.getName() + " or of " + normalized + ".");
+            }
+        }
+    }
+
+    /** The type parameter this type stands for, when it is still abstract. */
+    private static @Nullable TypeParamDef asTypeParam(WurstType typ) {
+        if (typ instanceof WurstTypeTypeParam tp) {
+            return tp.getDef();
+        }
+        if (typ instanceof WurstTypeBoundTypeParam bound) {
+            return asTypeParam(bound.getBaseType().normalize());
+        }
+        return null;
+    }
+
+    /** Every bound written on a type parameter must be usable as a type class. */
+    private void checkTypeParamBounds(TypeParamDef tp) {
+        for (TypeExpr boundExpr : TypeClassConstraints.boundExprs(tp)) {
+            String reason = TypeClassConstraints.invalidBoundReason(boundExpr);
+            if (reason != null) {
+                boundExpr.addError("Invalid bound on type parameter " + tp.getName() + ": " + reason);
             }
         }
     }
@@ -2637,9 +2674,15 @@ private void checkInstanceDecl(InstanceDecl decl) {
             return;
         }
 
+        if (!iface.getExtendsList().isEmpty()) {
+            decl.addError("Interface " + iface.getName() + " extends another interface, which is not supported"
+                    + " for type classes: the requirements of a bound are the interface's own functions.");
+            return;
+        }
+
         checkInstanceIsNotOrphan(decl, iface, instanceType);
         checkInstanceIsUnique(decl, iface, instanceType);
-        checkInstanceIsComplete(decl, iface);
+        checkInstanceIsComplete(decl, iface, instanceType);
     }
 
     private void checkInstanceIsNotOrphan(InstanceDecl decl, InterfaceDef iface, WurstType instanceType) {
@@ -2673,17 +2716,24 @@ private void checkInstanceIsUnique(InstanceDecl decl, InterfaceDef iface, WurstT
         }
     }
 
-    private void checkInstanceIsComplete(InstanceDecl decl, InterfaceDef iface) {
+    private void checkInstanceIsComplete(InstanceDecl decl, InterfaceDef iface, WurstType instanceType) {
+        // The requirement is written in terms of the interface's type parameter, so compare against
+        // it with that parameter replaced by the type this instance is for.
+        TypeParamDef ifaceParam = iface.getTypeParameters().get(0);
+        VariableBinding binding = VariableBinding.emptyMapping()
+                .set(ifaceParam, new WurstTypeBoundTypeParam(ifaceParam, instanceType, decl));
+
         StringBuilder missing = new StringBuilder();
         for (FuncDef requirement : iface.getMethods()) {
             int found = 0;
             for (FuncDef provided : decl.getMethods()) {
                 if (provided.getName().equals(requirement.getName())) {
                     found++;
+                    checkInstanceMethodSignature(provided, requirement, binding, iface);
                 }
             }
             if (found == 0) {
-                missing.append("\n    ").append(requirement.getName());
+                missing.append("\n    ").append(requirementTemplate(requirement, binding, decl));
             } else if (found > 1) {
                 decl.addError("This instance defines " + requirement.getName() + " more than once.");
             }
@@ -2706,6 +2756,61 @@ private void checkInstanceIsComplete(InstanceDecl decl, InterfaceDef iface) {
         }
     }
 
+    /**
+     * An instance method must match its requirement exactly once the interface's type parameter is
+     * replaced by the instance type. Matching on the name alone would let a wrongly typed
+     * implementation be selected and emitted, which the backend cannot catch.
+     */
+    private void checkInstanceMethodSignature(FuncDef provided, FuncDef requirement,
+                                              VariableBinding binding, InterfaceDef iface) {
+        List<WurstType> expectedParams = new ArrayList<>();
+        for (WParameter p : requirement.getParameters()) {
+            expectedParams.add(p.attrTyp().setTypeArgs(binding));
+        }
+        if (provided.getParameters().size() != expectedParams.size()) {
+            provided.addError(provided.getName() + " must take " + expectedParams.size()
+                    + " parameter(s) to implement " + iface.getName() + "."
+                    + "\nExpected: " + signatureText(requirement, binding));
+            return;
+        }
+        for (int i = 0; i < expectedParams.size(); i++) {
+            WParameter actual = provided.getParameters().get(i);
+            WurstType expected = expectedParams.get(i);
+            if (!actual.attrTyp().equalsType(expected, actual)) {
+                actual.addError("Parameter " + actual.getName() + " should have type " + expected
+                        + " to implement " + iface.getName() + "." + provided.getName() + ", but has "
+                        + actual.attrTyp() + ".");
+            }
+        }
+        WurstType expectedReturn = requirement.attrReturnTyp().setTypeArgs(binding);
+        if (!provided.attrReturnTyp().equalsType(expectedReturn, provided)) {
+            provided.addError(provided.getName() + " should return " + expectedReturn + " to implement "
+                    + iface.getName() + ", but returns " + provided.attrReturnTyp() + ".");
+        }
+    }
+
+    private String requirementTemplate(FuncDef requirement, VariableBinding binding, Element context) {
+        return signatureText(requirement, binding);
+    }
+
+    private String signatureText(FuncDef requirement, VariableBinding binding) {
+        StringBuilder sb = new StringBuilder("function ").append(requirement.getName()).append("(");
+        boolean first = true;
+        for (WParameter p : requirement.getParameters()) {
+            if (!first) {
+                sb.append(", ");
+            }
+            first = false;
+            sb.append(p.attrTyp().setTypeArgs(binding)).append(" ").append(p.getName());
+        }
+        sb.append(")");
+        WurstType returnType = requirement.attrReturnTyp().setTypeArgs(binding);
+        if (!(returnType instanceof WurstTypeVoid)) {
+            sb.append(" returns ").append(returnType);
+        }
+        return sb.toString();
+    }
+
     private void checkFuncRef(FuncRef ref) {
         if (isConstructorThisCall(ref)) {
             return;

de.peeeq.wurstscript/src/main/resources/agent-docs/WURST_LANGUAGE.md

@@ -184,6 +184,18 @@ Unlike an interface used as a supertype, a bound works for `int`, `real`, `strin
 
 Instances are unique and must be declared next to what they relate. An instance of `I` for type `X` may only live in the package declaring `I` or the package declaring `X`, and there may be only one. This makes `I` for `X` mean the same thing everywhere, independent of imports. Instances have no type parameters of their own in this version, so there is no way to write "every `List<T>` is `Indexable` when `T` is".
 
+An instance must implement each requirement with the signature it has after the interface's type parameter is replaced by the instance type; a matching name is not enough. An interface used as a bound must not extend another interface, because the requirements of a bound are the interface's own functions.
+
+A generic which passes its own type parameter to another bounded generic must declare that bound itself:
+
+```wurst
+function inner<Q: Show>(Q x) returns string
+	return Q.show(x)
+
+function outer<R: Show>(R x) returns string   // <R:> alone would not compile
+	return inner(x)
+```
+
 ## Lua and Jass targets
 
 The target is selected by the project `wurst.build` `scriptMode` field. `wc3Patch` separately selects the compatible core Jass and standard-library era.