Improve error message for aliased references (#1695)

pkl-core/src/main/java/org/pkl/core/ast/type/TypeNode.java

@@ -27,7 +27,10 @@ import com.oracle.truffle.api.frame.FrameSlotKind;
 import com.oracle.truffle.api.frame.VirtualFrame;
 import com.oracle.truffle.api.nodes.ExplodeLoop;
 import com.oracle.truffle.api.nodes.LoopNode;
+import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.api.nodes.NodeUtil;
 import com.oracle.truffle.api.source.SourceSection;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
@@ -38,6 +41,7 @@ import org.jspecify.annotations.Nullable;
 import org.pkl.core.PType;
 import org.pkl.core.PType.StringLiteral;
 import org.pkl.core.PklBugException;
+import org.pkl.core.StackFrame;
 import org.pkl.core.TypeParameter;
 import org.pkl.core.ast.*;
 import org.pkl.core.ast.builder.SymbolTable.CustomThisScope;
@@ -54,6 +58,7 @@ import org.pkl.core.util.EconomicMaps;
 import org.pkl.core.util.EconomicSets;
 import org.pkl.core.util.LateInit;
 import org.pkl.core.util.MutableBoolean;
+import org.pkl.core.util.MutableReference;
 
 public abstract class TypeNode extends PklNode {
 
@@ -2138,7 +2143,17 @@ public abstract class TypeNode extends PklNode {
       this.domainTypeNode = domainTypeNode;
       this.referentTypeNode = referentTypeNode;
       this.getModuleNode = new GetModuleNode(sourceSection);
-      validateTypeArguments(sourceSection);
+      // A type constraint anywhere in the referent is forbidden, including one reached through a
+      // type alias used in the referent.
+      var constraint = findReferentConstraint();
+      if (constraint != null) {
+        CompilerDirectives.transferToInterpreter();
+        throw exceptionBuilder()
+            .evalError("invalidReferenceTypeAnnotationWithConstraint")
+            .withLeadingStackFrames(
+                buildReferentConstraintFrames(constraint, getSourceSection(), null))
+            .build();
+      }
     }
 
     @Specialization
@@ -2173,26 +2188,69 @@ public abstract class TypeNode extends PklNode {
           sourceSection, value, TypeNode.export(domainTypeNode), referentType);
     }
 
-    public void validateTypeArguments(@Nullable SourceSection aliasSourceSection) {
+    /**
-      // constraints may not be used in Reference type annotation referents
+     * Type constraints may not appear anywhere in a {@code Reference}'s referent type argument.
-      // walk the type and throw if any part of the referent is constrained
+     * Walks the referent type and returns the first offending {@link ConstrainedTypeNode} , or
-
+     * {@code null} if the referent is constraint-free.
-      // TODO improve error message when this type node and/or referent constraint are behind type
+     */
-      // aliases
+    public @Nullable ConstrainedTypeNode findReferentConstraint() {
+      var found = new MutableReference<@Nullable ConstrainedTypeNode>(null);
       referentTypeNode.acceptTypeNode(
           true,
           (typeNode) -> {
-            if (typeNode instanceof ConstrainedTypeNode) {
+            if (typeNode instanceof ConstrainedTypeNode constrainedTypeNode) {
-              CompilerDirectives.transferToInterpreter();
+              found.set(constrainedTypeNode);
-              var err =
+              return false;
-                  exceptionBuilder().evalError("invalidReferenceTypeAnnotationWithConstraint");
-              if (aliasSourceSection != null) {
-                err.withSourceSection(aliasSourceSection);
-              }
-              throw err.build();
             }
             return true;
           });
+      return found.getOrNull();
+    }
+
+    /** Builds the frames to show ahead of an "invalid referent constraint" error. */
+    public static List<StackFrame> buildReferentConstraintFrames(
+        ConstrainedTypeNode constraintNode,
+        SourceSection usageSection,
+        @Nullable VmTypeAlias outermostAlias) {
+      var frames = new ArrayList<StackFrame>();
+      for (Node node = constraintNode; node != null; node = node.getParent()) {
+        if (!(node instanceof ConstrainedTypeNode
+            || node instanceof TypeAliasTypeNode
+            || node instanceof ReferenceTypeNode)) {
+          continue;
+        }
+        var section = node.getSourceSection();
+        //noinspection ConstantValue
+        if (section == null || !section.isAvailable() || isWithin(usageSection, section)) {
+          continue;
+        }
+        var owner = ownerAlias(node, outermostAlias);
+        if (owner != null) {
+          frames.add(VmUtils.createStackFrame(section, owner.getQualifiedName()));
+        }
+      }
+      return frames;
+    }
+
+    /**
+     * The type alias whose body contains {@code node}: the nearest enclosing alias, else the
+     * outermost alias being instantiated (which is {@code null} for a directly-used Reference).
+     */
+    @SuppressWarnings("DataFlowIssue")
+    private static @Nullable VmTypeAlias ownerAlias(
+        Node node, @Nullable VmTypeAlias outermostAlias) {
+      var parent = NodeUtil.findParent(node, TypeAliasTypeNode.class);
+      //noinspection ConstantValue
+      if (parent != null) {
+        return parent.typeAlias;
+      }
+      return outermostAlias;
+    }
+
+    private static boolean isWithin(SourceSection outer, SourceSection inner) {
+      return inner.getSource().equals(outer.getSource())
+          && inner.getCharIndex() >= outer.getCharIndex()
+          && inner.getCharEndIndex() <= outer.getCharEndIndex();
     }
 
     @Fallback
@@ -2703,13 +2761,42 @@ public abstract class TypeNode extends PklNode {
 
       this.typeAlias = typeAlias;
       this.typeArgumentNodes = typeArgumentNodes;
-      aliasedTypeNode = typeAlias.instantiate(typeArgumentNodes, sourceSection);
+      aliasedTypeNode = typeAlias.instantiate(typeArgumentNodes);
+      checkReferentConstraints(typeAlias);
+    }
+
+    /**
+     * Reports a forbidden type constraint that a type argument introduced into a {@code
+     * Reference}'s referent through this (generic) alias. The error is reported at this usage type
+     * expression, with leading frames for the constraint and every alias layer it passed through.
+     */
+    private void checkReferentConstraints(VmTypeAlias outermostAlias) {
+      aliasedTypeNode.accept(
+          node -> {
+            if (node instanceof ReferenceTypeNode referenceTypeNode) {
+              var constraint = referenceTypeNode.findReferentConstraint();
+              if (constraint != null) {
+                CompilerDirectives.transferToInterpreter();
+                throw exceptionBuilder()
+                    .evalError("invalidReferenceTypeAnnotationWithConstraint")
+                    .withLeadingStackFrames(
+                        ReferenceTypeNode.buildReferentConstraintFrames(
+                            constraint, getSourceSection(), outermostAlias))
+                    .build();
+              }
+            }
+            return true;
+          });
     }
 
     public TypeNode getAliasedTypeNode() {
       return aliasedTypeNode;
     }
 
+    public VmTypeAlias getTypeAlias() {
+      return typeAlias;
+    }
+
     @Override
     public FrameSlotKind getFrameSlotKind() {
       return aliasedTypeNode.getFrameSlotKind();

pkl-core/src/main/java/org/pkl/core/runtime/StackTraceGenerator.java

@@ -38,6 +38,10 @@ final class StackTraceGenerator {
   }
 
   private List<StackFrame> capture() {
+    // frames that aren't part of the runtime call stack are
+    // shown ahead of the captured frames.
+    frames.addAll(exception.getLeadingStackFrames());
+
     var truffleElements = TruffleStackTrace.getStackTrace(exception);
     if (truffleElements.isEmpty()) {
       addFrame(exception.getSourceSection(), exception.getMemberName());

pkl-core/src/main/java/org/pkl/core/runtime/VmBugException.java

@@ -38,7 +38,8 @@ public final class VmBugException extends VmException {
       @Nullable SourceSection sourceSection,
       @Nullable String memberName,
       @Nullable BiConsumer<AnsiStringBuilder, Boolean> hintBuilder,
-      Map<CallTarget, StackFrame> insertedStackFrames) {
+      Map<CallTarget, StackFrame> insertedStackFrames,
+      List<StackFrame> leadingStackFrames) {
 
     super(
         message,
@@ -51,7 +52,8 @@ public final class VmBugException extends VmException {
         sourceSection,
         memberName,
         hintBuilder,
-        insertedStackFrames);
+        insertedStackFrames,
+        leadingStackFrames);
   }
 
   @Override

pkl-core/src/main/java/org/pkl/core/runtime/VmEvalException.java

@@ -37,7 +37,8 @@ public class VmEvalException extends VmException {
       @Nullable SourceSection sourceSection,
       @Nullable String memberName,
       @Nullable BiConsumer<AnsiStringBuilder, Boolean> hintBuilder,
-      Map<CallTarget, StackFrame> insertedStackFrames) {
+      Map<CallTarget, StackFrame> insertedStackFrames,
+      List<StackFrame> leadingStackFrames) {
 
     super(
         message,
@@ -50,6 +51,7 @@ public class VmEvalException extends VmException {
         sourceSection,
         memberName,
         hintBuilder,
-        insertedStackFrames);
+        insertedStackFrames,
+        leadingStackFrames);
   }
 }

pkl-core/src/main/java/org/pkl/core/runtime/VmException.java

@@ -34,6 +34,7 @@ public abstract class VmException extends AbstractTruffleException {
   private final @Nullable SourceSection sourceSection;
   private final @Nullable String memberName;
   private final Map<CallTarget, StackFrame> insertedStackFrames;
+  private final List<StackFrame> leadingStackFrames;
   @Nullable private final BiConsumer<AnsiStringBuilder, Boolean> messageBuilder;
   @Nullable protected BiConsumer<AnsiStringBuilder, Boolean> hintBuilder;
 
@@ -48,7 +49,8 @@ public abstract class VmException extends AbstractTruffleException {
       @Nullable SourceSection sourceSection,
       @Nullable String memberName,
       @Nullable BiConsumer<AnsiStringBuilder, Boolean> hintBuilder,
-      Map<CallTarget, StackFrame> insertedStackFrames) {
+      Map<CallTarget, StackFrame> insertedStackFrames,
+      List<StackFrame> leadingStackFrames) {
     super(message, cause, UNLIMITED_STACK_TRACE, location);
     assert message != null || messageBuilder != null;
     this.messageBuilder = messageBuilder;
@@ -58,6 +60,7 @@ public abstract class VmException extends AbstractTruffleException {
     this.sourceSection = sourceSection;
     this.memberName = memberName;
     this.insertedStackFrames = insertedStackFrames;
+    this.leadingStackFrames = leadingStackFrames;
     this.hintBuilder = hintBuilder;
   }
 
@@ -89,6 +92,14 @@ public abstract class VmException extends AbstractTruffleException {
     return insertedStackFrames;
   }
 
+  /**
+   * Stack frames to prepend to the rendered stack trace, ahead of the captured frames. Used to show
+   * source locations that aren't part of the runtime call stack.
+   */
+  public final List<StackFrame> getLeadingStackFrames() {
+    return leadingStackFrames;
+  }
+
   public @Nullable BiConsumer<AnsiStringBuilder, Boolean> getMessageBuilder() {
     return messageBuilder;
   }

pkl-core/src/main/java/org/pkl/core/runtime/VmExceptionBuilder.java

@@ -99,6 +99,7 @@ public final class VmExceptionBuilder {
   private @Nullable Node location;
   private @Nullable SourceSection sourceSection;
   private @Nullable String memberName;
+  private List<StackFrame> leadingStackFrames = List.of();
 
   public VmExceptionBuilder typeMismatch(Object value, VmClass expectedType) {
     if (value instanceof VmNull) {
@@ -359,6 +360,15 @@ public final class VmExceptionBuilder {
     return this;
   }
 
+  /**
+   * Frames to show ahead of the captured stack trace (see {@link
+   * VmException#getLeadingStackFrames()}).
+   */
+  public VmExceptionBuilder withLeadingStackFrames(List<StackFrame> leadingStackFrames) {
+    this.leadingStackFrames = leadingStackFrames;
+    return this;
+  }
+
   public VmException build() {
     if (message != null && messageBuilder != null) {
       throw new IllegalStateException("Both message and messageBuilder are set");
@@ -387,7 +397,8 @@ public final class VmExceptionBuilder {
               sourceSection,
               memberName,
               hintBuilder,
-              effectiveInsertedStackFrames);
+              effectiveInsertedStackFrames,
+              leadingStackFrames);
       case UNDEFINED_VALUE ->
           new VmUndefinedValueException(
               message,
@@ -401,7 +412,8 @@ public final class VmExceptionBuilder {
               memberName,
               hintBuilder,
               receiver,
-              effectiveInsertedStackFrames);
+              effectiveInsertedStackFrames,
+              leadingStackFrames);
       case BUG ->
           new VmBugException(
               message,
@@ -414,7 +426,8 @@ public final class VmExceptionBuilder {
               sourceSection,
               memberName,
               hintBuilder,
-              effectiveInsertedStackFrames);
+              effectiveInsertedStackFrames,
+              leadingStackFrames);
       case WRAPPED -> {
         assert wrappedException != null;
         yield new VmWrappedEvalException(
@@ -429,6 +442,7 @@ public final class VmExceptionBuilder {
             memberName,
             hintBuilder,
             effectiveInsertedStackFrames,
+            leadingStackFrames,
             wrappedException);
       }
     };

pkl-core/src/main/java/org/pkl/core/runtime/VmStackOverflowException.java

@@ -32,6 +32,7 @@ public final class VmStackOverflowException extends VmException {
         null,
         null,
         null,
-        new HashMap<>());
+        new HashMap<>(),
+        List.of());
   }
 }

pkl-core/src/main/java/org/pkl/core/runtime/VmTypeAlias.java

@@ -32,7 +32,6 @@ import org.pkl.core.TypeParameter;
 import org.pkl.core.ast.VmModifier;
 import org.pkl.core.ast.type.TypeNode;
 import org.pkl.core.ast.type.TypeNode.ConstrainedTypeNode;
-import org.pkl.core.ast.type.TypeNode.ReferenceTypeNode;
 import org.pkl.core.ast.type.TypeNode.TypeVariableNode;
 import org.pkl.core.ast.type.TypeNode.UnknownTypeNode;
 import org.pkl.core.util.LateInit;
@@ -178,8 +177,7 @@ public final class VmTypeAlias extends VmValue {
   }
 
   @TruffleBoundary
-  public TypeNode instantiate(
+  public TypeNode instantiate(TypeNode[] typeArgumentNodes) {
-      TypeNode[] typeArgumentNodes, SourceSection typeAliasTypeNodeSourceSection) {
     // Cloning the type node means that the entire type check remains within a single root node,
     // which should be good for interpreted and compiled performance alike:
     // * Fewer root nodes to call
@@ -201,13 +199,6 @@ public final class VmTypeAlias extends VmValue {
           }
           return true;
         });
-    clone.accept(
-        node -> {
-          if (node instanceof ReferenceTypeNode referenceTypeNode) {
-            referenceTypeNode.validateTypeArguments(typeAliasTypeNodeSourceSection);
-          }
-          return true;
-        });
 
     return clone;
   }

pkl-core/src/main/java/org/pkl/core/runtime/VmUndefinedValueException.java

@@ -43,7 +43,8 @@ public final class VmUndefinedValueException extends VmEvalException {
       @Nullable String memberName,
       @Nullable BiConsumer<AnsiStringBuilder, Boolean> hintBuilder,
       @Nullable Object receiver,
-      @Nullable Map<CallTarget, StackFrame> insertedStackFrames) {
+      @Nullable Map<CallTarget, StackFrame> insertedStackFrames,
+      List<StackFrame> leadingStackFrames) {
 
     super(
         message,
@@ -56,7 +57,8 @@ public final class VmUndefinedValueException extends VmEvalException {
         sourceSection,
         memberName,
         hintBuilder,
-        insertedStackFrames == null ? Collections.emptyMap() : insertedStackFrames);
+        insertedStackFrames == null ? Collections.emptyMap() : insertedStackFrames,
+        leadingStackFrames);
 
     this.receiver = receiver;
   }

pkl-core/src/main/java/org/pkl/core/runtime/VmWrappedEvalException.java

@@ -41,6 +41,7 @@ public class VmWrappedEvalException extends VmEvalException {
       @Nullable String memberName,
       @Nullable BiConsumer<AnsiStringBuilder, Boolean> hintBuilder,
       Map<CallTarget, StackFrame> insertedStackFrames,
+      List<StackFrame> leadingStackFrames,
       VmException wrappedException) {
     super(
         message,
@@ -53,7 +54,8 @@ public class VmWrappedEvalException extends VmEvalException {
         sourceSection,
         memberName,
         hintBuilder,
-        insertedStackFrames);
+        insertedStackFrames,
+        leadingStackFrames);
     this.wrappedException = wrappedException;
   }
 

pkl-core/src/test/files/LanguageSnippetTests/input/errors/reference21.pkl

@@ -0,0 +1,7 @@
+import "pkl:ref"
+
+typealias Ref<T> = ref.Reference<ref.Domain, T>
+
+typealias Ref2<T> = Ref<T>
+
+foo: Ref2<String(isEmpty)>

pkl-core/src/test/files/LanguageSnippetTests/input/errors/reference22.pkl

@@ -0,0 +1,6 @@
+import "pkl:ref"
+
+typealias Ref<T> = ref.Reference<ref.Domain, T>
+typealias Ref2<T> = Ref<T(isEmpty)>
+
+foo: Ref2<String>

pkl-core/src/test/files/LanguageSnippetTests/input/errors/reference23.pkl

@@ -0,0 +1,7 @@
+
+import "pkl:ref"
+
+typealias Foo1 = Foo2
+typealias Foo2 = String(isEmpty)
+
+foo: ref.Reference<ref.Domain, Foo1>

pkl-core/src/test/files/LanguageSnippetTests/output/errors/reference10.err

@@ -1,6 +1,10 @@
 –– Pkl Error ––
 `Reference` referent type argument may not include type constraints.
 
+x | typealias Ref<T> = ref.Reference<D, T>
+                       ^^^^^^^^^^^^^^^^^^^
+at reference10#Ref (file:///$snippetsDir/input/errors/reference10.pkl)
+
 x | test = ref.Reference(d, String, "") as Ref<Listing<String(true)>>
                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^
 at reference10#test (file:///$snippetsDir/input/errors/reference10.pkl)

pkl-core/src/test/files/LanguageSnippetTests/output/errors/reference11.err

@@ -1,6 +1,18 @@
 –– Pkl Error ––
 `Reference` referent type argument may not include type constraints.
 
+xx | typealias Alias2 = String(length < 5)
+                        ^^^^^^^^^^^^^^^^^^
+at reference11#Alias2 (file:///$snippetsDir/input/errors/reference11.pkl)
+
+xx | typealias Alias1 = Int | Alias2?
+                              ^^^^^^
+at reference11#Alias1 (file:///$snippetsDir/input/errors/reference11.pkl)
+
+x | typealias Ref<T> = ref.Reference<D, T>
+                       ^^^^^^^^^^^^^^^^^^^
+at reference11#Ref (file:///$snippetsDir/input/errors/reference11.pkl)
+
 x | test = ref.Reference(d, String, "") as Ref<Alias1?>
                                            ^^^^^^^^^^^^
 at reference11#test (file:///$snippetsDir/input/errors/reference11.pkl)

pkl-core/src/test/files/LanguageSnippetTests/output/errors/reference12.err

@@ -1,6 +1,10 @@
 –– Pkl Error ––
 `Reference` referent type argument may not include type constraints.
 
+xx | typealias Alias2 = String(length < 5)
+                        ^^^^^^^^^^^^^^^^^^
+at reference12#Alias2 (file:///$snippetsDir/input/errors/reference12.pkl)
+
 x | typealias RefAlias1 = ref.Reference<D, Alias1?>
                           ^^^^^^^^^^^^^^^^^^^^^^^^^
 at reference12#RefAlias1 (file:///$snippetsDir/input/errors/reference12.pkl)

pkl-core/src/test/files/LanguageSnippetTests/output/errors/reference21.err

@@ -0,0 +1,14 @@
+–– Pkl Error ––
+`Reference` referent type argument may not include type constraints.
+
+x | typealias Ref<T> = ref.Reference<ref.Domain, T>
+                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+at reference21#Ref (file:///$snippetsDir/input/errors/reference21.pkl)
+
+x | typealias Ref2<T> = Ref<T>
+                        ^^^^^^
+at reference21#Ref2 (file:///$snippetsDir/input/errors/reference21.pkl)
+
+x | foo: Ref2<String(isEmpty)>
+         ^^^^^^^^^^^^^^^^^^^^^
+at reference21 (file:///$snippetsDir/input/errors/reference21.pkl)

pkl-core/src/test/files/LanguageSnippetTests/output/errors/reference22.err

@@ -0,0 +1,14 @@
+–– Pkl Error ––
+`Reference` referent type argument may not include type constraints.
+
+x | typealias Ref<T> = ref.Reference<ref.Domain, T>
+                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+at reference22#Ref (file:///$snippetsDir/input/errors/reference22.pkl)
+
+x | typealias Ref2<T> = Ref<T(isEmpty)>
+                        ^^^^^^^^^^^^^^^
+at reference22#Ref2 (file:///$snippetsDir/input/errors/reference22.pkl)
+
+x | foo: Ref2<String>
+         ^^^^
+at reference22 (file:///$snippetsDir/input/errors/reference22.pkl)

pkl-core/src/test/files/LanguageSnippetTests/output/errors/reference23.err

@@ -0,0 +1,10 @@
+–– Pkl Error ––
+`Reference` referent type argument may not include type constraints.
+
+x | typealias Foo2 = String(isEmpty)
+                     ^^^^^^^^^^^^^^^
+at reference23#Foo2 (file:///$snippetsDir/input/errors/reference23.pkl)
+
+x | foo: ref.Reference<ref.Domain, Foo1>
+         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+at reference23 (file:///$snippetsDir/input/errors/reference23.pkl)

pkl-core/src/test/files/LanguageSnippetTests/output/errors/reference8.err

@@ -1,6 +1,10 @@
 –– Pkl Error ––
 `Reference` referent type argument may not include type constraints.
 
+x | typealias Ref<T> = ref.Reference<D, T>
+                       ^^^^^^^^^^^^^^^^^^^
+at reference8#Ref (file:///$snippetsDir/input/errors/reference8.pkl)
+
 x | test = ref.Reference(d, String, "") as Ref<String(true)>
                                            ^^^^^^^^^^^^^^^^^
 at reference8#test (file:///$snippetsDir/input/errors/reference8.pkl)

pkl-core/src/test/files/LanguageSnippetTests/output/errors/reference9.err

@@ -1,6 +1,10 @@
 –– Pkl Error ––
 `Reference` referent type argument may not include type constraints.
 
+x | typealias Ref<T> = ref.Reference<D, T>
+                       ^^^^^^^^^^^^^^^^^^^
+at reference9#Ref (file:///$snippetsDir/input/errors/reference9.pkl)
+
 x | test = ref.Reference(d, String, "") as Ref<String(true) | Int>
                                            ^^^^^^^^^^^^^^^^^^^^^^^
 at reference9#test (file:///$snippetsDir/input/errors/reference9.pkl)