Correctly type check Class<T>

docs/modules/release-notes/pages/0.33.adoc

@@ -23,6 +23,16 @@ Native `pkl` and `pkldoc` binaries for Intel Mac systems are no longer published
 
 To continue running new Pkl releases on these systems, use an appropriate Java runtime and the `jpkl` and `jpkldoc` Java executables.
 
+=== Type check changes for `Class<T>`
+
+In prior versions of Pkl, type arguments to the `Class` type were erased.
+Any `Class` value would typecheck against `Class<T>` for any value of `T`.
+
+In Pkl 0.33, this erasure has been removed.
+A `Class` value typechecked against `Class<T>` must be a subclass of `T`.
+
+If `T` does not resolve to a class type (i.e. it is a union type, nullable type, string literal type, parameterized type, or `nothing`), the type check will always fail.
+
 === XXX
 
 == Bug Fixes [small]#🐜#

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

@@ -33,6 +33,7 @@
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import org.jspecify.annotations.NonNull;
@@ -61,10 +62,18 @@
 
 public abstract class TypeNode extends PklNode {
 
-  public interface ClassTypeNode {
+  /**
+   * Type node that corresponds to a simple, unparameterized {@link VmClass}.
+   *
+   * <p>This includes generic classes written without any type arguments like {@code List}.
+   */
+  public interface SimpleClassTypeNode {
     VmClass getVmClass();
   }
 
+  /** Type node that corresponds to a user-defined class (or module class). */
+  public interface UserClassTypeNode extends SimpleClassTypeNode {}
+
   protected TypeNode(SourceSection sourceSection) {
     super(sourceSection);
   }
@@ -411,7 +420,7 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
 
   /** The `module` type for a final module. */
   public static final class FinalModuleTypeNode extends ObjectSlotTypeNode
-      implements ClassTypeNode {
+      implements UserClassTypeNode {
     private final VmClass moduleClass;
 
     public FinalModuleTypeNode(SourceSection sourceSection, VmClass moduleClass) {
@@ -466,7 +475,7 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
 
   /** The `module` type for an open module. */
   public static final class NonFinalModuleTypeNode extends ObjectSlotTypeNode
-      implements ClassTypeNode {
+      implements UserClassTypeNode {
     private final VmClass moduleClass; // only used by getVmClass()
     @Child private ExpressionNode getModuleNode;
 
@@ -580,7 +589,8 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
     }
   }
 
-  public static final class TypedTypeNode extends ObjectSlotTypeNode {
+  public static final class TypedTypeNode extends ObjectSlotTypeNode
+      implements SimpleClassTypeNode {
     public TypedTypeNode(SourceSection sourceSection) {
       super(sourceSection);
     }
@@ -608,7 +618,8 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
     }
   }
 
-  public static final class DynamicTypeNode extends ObjectSlotTypeNode {
+  public static final class DynamicTypeNode extends ObjectSlotTypeNode
+      implements SimpleClassTypeNode {
     public DynamicTypeNode(SourceSection sourceSection) {
       super(sourceSection);
     }
@@ -651,7 +662,8 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
    * String/Boolean/Int/Float and their supertypes, only `VmValue`s can possibly pass its type
    * check.
    */
-  public static final class FinalClassTypeNode extends ObjectSlotTypeNode implements ClassTypeNode {
+  public static final class FinalClassTypeNode extends ObjectSlotTypeNode
+      implements UserClassTypeNode {
     private final VmClass clazz;
 
     public FinalClassTypeNode(SourceSection sourceSection, VmClass clazz) {
@@ -708,7 +720,7 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
    * check.
    */
   public abstract static class NonFinalClassTypeNode extends ObjectSlotTypeNode
-      implements ClassTypeNode {
+      implements UserClassTypeNode {
     protected final VmClass clazz;
 
     public NonFinalClassTypeNode(SourceSection sourceSection, VmClass clazz) {
@@ -2937,7 +2949,8 @@ public VmTyped getMirror() {
     }
   }
 
-  public static final class AnyTypeNode extends WriteFrameSlotTypeNode {
+  public static final class AnyTypeNode extends WriteFrameSlotTypeNode
+      implements SimpleClassTypeNode {
     public AnyTypeNode(SourceSection sourceSection) {
       super(sourceSection);
     }
@@ -2969,7 +2982,8 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
     }
   }
 
-  public static final class StringTypeNode extends ObjectSlotTypeNode {
+  public static final class StringTypeNode extends ObjectSlotTypeNode
+      implements SimpleClassTypeNode {
     public StringTypeNode(SourceSection sourceSection) {
       super(sourceSection);
     }
@@ -2997,7 +3011,8 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
     }
   }
 
-  public static final class NumberTypeNode extends FrameSlotTypeNode {
+  public static final class NumberTypeNode extends FrameSlotTypeNode
+      implements SimpleClassTypeNode {
     public NumberTypeNode(SourceSection sourceSection) {
       super(sourceSection);
     }
@@ -3056,7 +3071,7 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
     }
   }
 
-  public static final class IntTypeNode extends IntSlotTypeNode {
+  public static final class IntTypeNode extends IntSlotTypeNode implements SimpleClassTypeNode {
     public IntTypeNode(SourceSection sourceSection) {
       super(sourceSection);
     }
@@ -3084,7 +3099,7 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
     }
   }
 
-  public static final class FloatTypeNode extends FrameSlotTypeNode {
+  public static final class FloatTypeNode extends FrameSlotTypeNode implements SimpleClassTypeNode {
     public FloatTypeNode(SourceSection sourceSection) {
       super(sourceSection);
     }
@@ -3124,7 +3139,8 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
     }
   }
 
-  public static final class BooleanTypeNode extends FrameSlotTypeNode {
+  public static final class BooleanTypeNode extends FrameSlotTypeNode
+      implements SimpleClassTypeNode {
     public BooleanTypeNode(SourceSection sourceSection) {
       super(sourceSection);
     }
@@ -3164,6 +3180,104 @@ protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer co
     }
   }
 
+  public abstract static class ClassClassTypeNode extends ValidatingObjectSlotTypeNode {
+
+    @Child private TypeNode typeNode;
+    @CompilationFinal private @Nullable VmClass clazz;
+
+    public ClassClassTypeNode(SourceSection sourceSection, TypeNode typeNode) {
+      super(sourceSection);
+      this.typeNode = typeNode;
+      validate();
+    }
+
+    @Override
+    public String getValidationErrorKey() {
+      // getViolatingNode() always returns null, it's only used to re-calculate clazz
+      throw PklBugException.unreachableCode();
+    }
+
+    @Override
+    protected boolean isIncludedInTrace(Node node) {
+      // getViolatingNode() always returns null, it's only used to re-calculate clazz
+      throw PklBugException.unreachableCode();
+    }
+
+    @Override
+    public @Nullable Node getViolatingNode() {
+      // use the validation hook to recalculate clazz after typealias instantiation
+      CompilerDirectives.transferToInterpreterAndInvalidate();
+
+      var node = typeNode;
+      while (node instanceof TypeAliasTypeNode typeAliasTypeNode) {
+        node = typeAliasTypeNode.getAliasedTypeNode();
+      }
+
+      if (node instanceof SimpleClassTypeNode simpleClassTypeNode) {
+        clazz = simpleClassTypeNode.getVmClass();
+      } else if (node instanceof UnknownTypeNode || node instanceof TypeVariableNode) {
+        clazz = BaseModule.getAnyClass();
+      } else {
+        clazz = null;
+      }
+      return null;
+    }
+
+    @Override
+    public VmClass getVmClass() {
+      return BaseModule.getClassClass();
+    }
+
+    @Specialization
+    protected Object eval(VmClass value) {
+      // Fast path: all classes match Class<Any> / Class<unknown> / Class<type arg>.
+      // In this case, skip the subclass check and behave like a bare `Class` type annotation.
+      if (clazz == BaseModule.getAnyClass()) {
+        return value;
+      }
+
+      // clazz will be null iff the type arg is a not a valid class type
+      if (clazz == null) {
+        throw new VmTypeMismatchException.Class(sourceSection, value, typeNode.doExport());
+      }
+
+      if (!value.isSubclassOf(clazz)) {
+        throw new VmTypeMismatchException.Class(sourceSection, value, clazz);
+      }
+
+      return value;
+    }
+
+    @Fallback
+    protected Object fallback(Object value) {
+      throw typeMismatch(value, BaseModule.getClassClass());
+    }
+
+    @Override
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
+      return consumer.accept(this);
+    }
+
+    @Override
+    protected boolean doIsEquivalentTo(TypeNode other) {
+      if (!(other instanceof ClassClassTypeNode classClassTypeNode)) {
+        return false;
+      }
+
+      return Objects.equals(clazz, classClassTypeNode.clazz);
+    }
+
+    @Override
+    public VmList getTypeArgumentMirrors() {
+      return VmList.of(typeNode.getMirror());
+    }
+
+    @Override
+    protected PType doExport() {
+      return new PType.Class(BaseModule.getClassClass().export(), typeNode.doExport());
+    }
+  }
+
   public abstract static class ValidatingObjectSlotTypeNode extends ObjectSlotTypeNode {
 
     protected ValidatingObjectSlotTypeNode(SourceSection sourceSection) {

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

@@ -278,9 +278,8 @@ public TypeNode execute(VirtualFrame frame) {
           return FunctionNClassTypeNodeGen.create(sourceSection, resolvedTypeArgumentNodes);
         }
 
-        // erase `x: Class<Foo>` to `x: Class` for now (cf. function types)
         if (clazz.isClassClass()) {
-          return new FinalClassTypeNode(sourceSection, clazz);
+          return ClassClassTypeNodeGen.create(sourceSection, typeArgumentNodes[0].execute(frame));
         }
 
         if (clazz.isVarArgsClass()) {

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

@@ -169,6 +169,69 @@ protected Boolean hasHint() {
     }
   }
 
+  public static final class Class extends VmTypeMismatchException {
+
+    private final String renderedExpected;
+    private final @Nullable VmClass expectedClass;
+
+    public Class(SourceSection sourceSection, VmClass actualClass, VmClass expectedClass) {
+      super(sourceSection, actualClass);
+      this.expectedClass = expectedClass;
+      renderedExpected = "Class<" + expectedClass + ">";
+    }
+
+    public Class(SourceSection sourceSection, VmClass actualClass, PType expectedType) {
+      super(sourceSection, actualClass);
+      this.expectedClass = null;
+      renderedExpected = "Class<" + expectedType + ">";
+    }
+
+    @Override
+    @TruffleBoundary
+    public void buildMessage(
+        AnsiStringBuilder builder, String indent, boolean withPowerAssertions) {
+      var actualClass = (VmClass) actualValue;
+      var renderedActualClass = "Class<" + actualClass + ">";
+
+      // give better error than "expected Class<foo.Bar>, but got Class<foo.Bar>" in case of naming
+      // conflict
+      if (expectedClass != null
+          && actualClass.getQualifiedName().equals(expectedClass.getQualifiedName())) {
+        var actualModuleUri = actualClass.getModule().getModuleInfo().getModuleKey().getUri();
+        var expectedModuleUri = expectedClass.getModule().getModuleInfo().getModuleKey().getUri();
+
+        builder
+            .append(
+                ErrorMessages.createIndented(
+                    actualClass.getPClassInfo().isModuleClass()
+                        ? "typeMismatchVersionConflict1"
+                        : "typeMismatchVersionConflict2",
+                    indent,
+                    renderedExpected,
+                    expectedModuleUri,
+                    actualModuleUri))
+            .append("\n");
+        return;
+      }
+
+      builder.append(
+          ErrorMessages.createIndented(
+              "typeMismatch", indent, renderedExpected, renderedActualClass));
+    }
+
+    @Override
+    protected Boolean hasHint() {
+      return expectedClass == null;
+    }
+
+    @Override
+    public void buildHint(AnsiStringBuilder builder, String indent, boolean withPowerAssertions) {
+      if (expectedClass != null) return;
+      builder.append(
+          "A `Class` type check can only succeed when its type argument is an un-parameterized class, a module, `unknown`, `module`, or an alias to one of those types.");
+    }
+  }
+
   public static final class Constraint extends VmTypeMismatchException {
 
     private final SourceSection constraintBodySourceSection;

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

@@ -173,7 +173,7 @@ private VmClass getOptionsClass(VmTyped command) {
     if (optionsTypeNode instanceof TypeNode.TypedTypeNode) {
       return BaseModule.getTypedClass();
     }
-    if (!(optionsTypeNode instanceof TypeNode.ClassTypeNode node)) {
+    if (!(optionsTypeNode instanceof TypeNode.UserClassTypeNode node)) {
       throw exceptionBuilder()
           .withSourceSection(optionsTypeNode.getSourceSection())
           .evalError(

pkl-core/src/test/files/LanguageSnippetTests/input-helper/types/classTypeA.pkl

@@ -0,0 +1,2 @@
+module classType
+class Foo

pkl-core/src/test/files/LanguageSnippetTests/input-helper/types/classTypeB.pkl

@@ -0,0 +1,2 @@
+module classType
+class Foo

pkl-core/src/test/files/LanguageSnippetTests/input/types/classType.pkl

@@ -0,0 +1,26 @@
+open module classType
+
+open class A
+open class B
+class C extends A
+class D extends module
+typealias E = A
+typealias BB = B
+
+res0 = C is Class<C>
+res1 = C is Class<A>
+res2 = C is Class<E>
+
+res3 = C is Class
+res4 = C is Class<unknown>
+res5 = C is Class<Any>
+
+res15 = D is Class<module>
+res16 = D is Class<Object>
+res17 = D is Class<Typed>
+res18 = D is Class<Dynamic>
+res19 = D is Class<Int>
+
+typealias F<T> = List<Class<T>>
+res20 = List(A, C) is F<A>
+res21 = List(new A {}, new B {}, new C {}).filterIsInstance(A).length

pkl-core/src/test/files/LanguageSnippetTests/input/types/classTypeErr1.pkl

@@ -0,0 +1,3 @@
+extends "classType.pkl"
+
+res2 = C as Class<B>

pkl-core/src/test/files/LanguageSnippetTests/input/types/classTypeErr10.pkl

@@ -0,0 +1,3 @@
+extends "classType.pkl"
+
+res14 = C as Class<module>

pkl-core/src/test/files/LanguageSnippetTests/input/types/classTypeErr11.pkl

@@ -0,0 +1,3 @@
+extends "classType.pkl"
+
+res22 = List(A, C) as F<A | B>