Fix cacheing of module type (#1393)

This fixes an issue where the `module` type is incorrectly cached.

pkl-core/src/main/java/org/pkl/core/ast/member/PropertyTypeNode.java

@@ -69,7 +69,11 @@ public final class PropertyTypeNode extends PklRootNode {
       defaultValue =
           typeNode.createDefaultValue(
               frame, VmLanguage.get(this), getSourceSection(), qualifiedPropertyName);
-      defaultValueInitialized = true;
+      // can't cache default value for `module` type in a non-final module because it's a self-type
+      // (the default value changes when inherited).
+      if (typeNode.isFinalType()) {
+        defaultValueInitialized = true;
+      }
     }
     return defaultValue;
   }

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

@@ -50,15 +50,20 @@ public final class GetParentForTypeNode extends ExpressionNode {
   @Override
   public Object executeGeneric(VirtualFrame frame) {
     if (defaultValue != null) return defaultValue;
-
     CompilerDirectives.transferToInterpreterAndInvalidate();
 
     var typeNode = getTypeNode(frame);
-    defaultValue =
+    var defaultValue =
         typeNode.createDefaultValue(frame, VmLanguage.get(this), sourceSection, qualifiedName);
 
-    if (defaultValue != null) {
+    // can't cache default value for `module` type in a non-final module because it's a self-type
+    // (the default value changes when inherited).
+    if (typeNode.isFinalType() && defaultValue != null) {
       unresolvedTypeNode = null;
+      this.defaultValue = defaultValue;
+    }
+
+    if (defaultValue != null) {
       return defaultValue;
     }
 

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

@@ -125,11 +125,23 @@ public abstract class TypeNode extends PklNode {
     return null;
   }
 
-  /**
+  public final boolean isFinalType() {
-   * Visit child type nodes; but not parameterized types (does not visit {@code String} in {@code
+    var ret = new MutableBoolean(true);
-   * Listing<String>}).
+    acceptTypeNode(
-   */
+        true,
-  protected abstract boolean acceptTypeNode(TypeNodeConsumer consumer);
+        typeNode -> {
+          // assumption: don't need to worry about `NonFinalClassTypeNode`
+          if (typeNode instanceof NonFinalModuleTypeNode) {
+            ret.set(false);
+            return false;
+          }
+          return true;
+        });
+    return ret.get();
+  }
+
+  /** Visit child type nodes of this type. */
+  protected abstract boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer);
 
   public static TypeNode forClass(SourceSection sourceSection, VmClass clazz) {
     return clazz.isClosed()
@@ -217,7 +229,6 @@ public abstract class TypeNode extends PklNode {
     public TypeNode initWriteSlotNode(int slot) {
       CompilerDirectives.transferToInterpreterAndInvalidate();
       this.slot = slot;
-      //noinspection DataFlowIssue
       writeFrameSlotNode = WriteFrameSlotNodeGen.create(sourceSection, slot, null);
       return this;
     }
@@ -322,7 +333,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -374,7 +385,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -419,9 +430,18 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
+
+    @Override
+    public @Nullable Object createDefaultValue(
+        VirtualFrame frame,
+        VmLanguage language,
+        SourceSection headerSection,
+        String qualifiedName) {
+      return TypeNode.createDefaultValue(moduleClass);
+    }
   }
 
   /** The `module` type for an open module. */
@@ -471,7 +491,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
 
@@ -534,7 +554,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -562,7 +582,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -600,7 +620,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -656,14 +676,15 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
 
   /**
-   * An `open` or `abstract` class type. Since this node is not used for String/Boolean/Int/Float
+   * An {@code open} or {@code abstract} class type. Since this node is not used for
-   * and their supertypes, only `VmValue`s can possibly pass its type check.
+   * String/Boolean/Int/Float and their supertypes, only {@link VmValue}s can possibly pass its type
+   * check.
    */
   public abstract static class NonFinalClassTypeNode extends ObjectSlotTypeNode {
     protected final VmClass clazz;
@@ -728,7 +749,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -798,11 +819,8 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
-      if (!consumer.accept(this)) {
+      return consumer.accept(this) && elementTypeNode.acceptTypeNode(visitTypeArguments, consumer);
-        return false;
-      }
-      return elementTypeNode.acceptTypeNode(consumer);
     }
   }
 
@@ -890,7 +908,7 @@ public abstract class TypeNode extends PklNode {
 
     @Override
     @ExplodeLoop
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       if (!consumer.accept(this)) {
         return false;
       }
@@ -901,7 +919,7 @@ public abstract class TypeNode extends PklNode {
         if (!ret) {
           continue;
         }
-        if (!elementTypeNodes[i].acceptTypeNode(consumer)) {
+        if (!elementTypeNodes[i].acceptTypeNode(visitTypeArguments, consumer)) {
           ret = false;
         }
       }
@@ -921,6 +939,7 @@ public abstract class TypeNode extends PklNode {
       var seenParameterizedClasses = EconomicSets.<VmClass>create();
       var ret = new MutableBoolean(false);
       this.acceptTypeNode(
+          false,
           (typeNode) -> {
             if (!typeNode.isParametric()) {
               return true;
@@ -1045,7 +1064,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
 
@@ -1123,7 +1142,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
 
@@ -1180,7 +1199,10 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
+      if (visitTypeArguments) {
+        return consumer.accept(this) && elementTypeNode.acceptTypeNode(true, consumer);
+      }
       return consumer.accept(this);
     }
 
@@ -1313,7 +1335,10 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
+      if (visitTypeArguments) {
+        return consumer.accept(this) && elementTypeNode.acceptTypeNode(true, consumer);
+      }
       return consumer.accept(this);
     }
 
@@ -1410,7 +1435,12 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
+      if (visitTypeArguments) {
+        return consumer.accept(this)
+            && keyTypeNode.acceptTypeNode(true, consumer)
+            && valueTypeNode.acceptTypeNode(true, consumer);
+      }
       return consumer.accept(this);
     }
 
@@ -1505,7 +1535,10 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
+      if (visitTypeArguments) {
+        return consumer.accept(this) && valueTypeNode.acceptTypeNode(true, consumer);
+      }
       return consumer.accept(this);
     }
   }
@@ -1578,7 +1611,11 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
+      if (visitTypeArguments) {
+        assert keyTypeNode != null;
+        return consumer.accept(this) && valueTypeNode.acceptTypeNode(true, consumer);
+      }
       return consumer.accept(this);
     }
   }
@@ -1851,7 +1888,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
 
@@ -1922,7 +1959,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
 
@@ -1999,7 +2036,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
 
@@ -2070,7 +2107,12 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
+      if (visitTypeArguments) {
+        return consumer.accept(this)
+            && firstTypeNode.acceptTypeNode(true, consumer)
+            && secondTypeNode.acceptTypeNode(true, consumer);
+      }
       return consumer.accept(this);
     }
 
@@ -2123,7 +2165,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
 
@@ -2177,7 +2219,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2212,7 +2254,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2255,7 +2297,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected final boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected final boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2324,7 +2366,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2368,7 +2410,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2412,7 +2454,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2569,11 +2611,8 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
-      if (!consumer.accept(this)) {
+      return consumer.accept(this) && aliasedTypeNode.acceptTypeNode(visitTypeArguments, consumer);
-        return false;
-      }
-      return aliasedTypeNode.acceptTypeNode(consumer);
     }
 
     @Override
@@ -2696,11 +2735,11 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       if (!consumer.accept(this)) {
         return false;
       }
-      return childNode.acceptTypeNode(consumer);
+      return childNode.acceptTypeNode(visitTypeArguments, consumer);
     }
 
     public VmTyped getMirror() {
@@ -2736,7 +2775,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2764,7 +2803,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2823,7 +2862,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2851,7 +2890,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2891,7 +2930,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }
@@ -2931,7 +2970,7 @@ public abstract class TypeNode extends PklNode {
     }
 
     @Override
-    protected boolean acceptTypeNode(TypeNodeConsumer consumer) {
+    protected boolean acceptTypeNode(boolean visitTypeArguments, TypeNodeConsumer consumer) {
       return consumer.accept(this);
     }
   }

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

@@ -0,0 +1,34 @@
+open module Foo
+
+prop: String
+
+myself: module?
+
+class Bar extends module {
+  prop2: Int
+}
+
+class Baz extends module {
+  prop2: String
+}
+
+output {
+  value = new Dynamic {
+    bar = new Bar {
+      prop = "hi"
+      prop2 = 15
+      myself {
+        prop = "hi again"
+        prop2 = 15
+      }
+    }
+    baz = new Baz {
+      prop = "hi"
+      prop2 = "hihi"
+      myself {
+        prop = "hihihi"
+        prop2 = "hihihihi"
+      }
+    }
+  }
+}

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

@@ -0,0 +1,18 @@
+open module TheModule
+
+mod: module? = Null(new module {})
+
+class Bar extends module {
+  num: Int
+}
+
+output {
+  value = new Dynamic {
+    bar = new Bar {
+      num = 5
+      mod {
+        num = 6
+      }
+    }
+  }
+}

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

@@ -0,0 +1,28 @@
+open module TheModule
+
+mod: module? = Null(new module {})
+
+open class Bar extends module {
+  num: Int
+}
+
+open class Baz extends module {
+  num2: Int
+}
+
+output {
+  value = new Dynamic {
+    bar = new Bar {
+      num = 5
+      mod {
+        num = 5
+      }
+    }
+    baz = new Baz {
+      num2 = 5
+      mod {
+        num2 = 5
+      }
+    }
+  }
+}

pkl-core/src/test/files/LanguageSnippetTests/output/types/moduleType3.pcf

@@ -0,0 +1,18 @@
+bar {
+  prop = "hi"
+  myself {
+    prop = "hi again"
+    myself = null
+    prop2 = 15
+  }
+  prop2 = 15
+}
+baz {
+  prop = "hi"
+  myself {
+    prop = "hihihi"
+    myself = null
+    prop2 = "hihihihi"
+  }
+  prop2 = "hihi"
+}

pkl-core/src/test/files/LanguageSnippetTests/output/types/moduleType4.pcf

@@ -0,0 +1,7 @@
+bar {
+  mod {
+    mod = null
+    num = 6
+  }
+  num = 5
+}

pkl-core/src/test/files/LanguageSnippetTests/output/types/moduleType5.pcf

@@ -0,0 +1,14 @@
+bar {
+  mod {
+    mod = null
+    num = 5
+  }
+  num = 5
+}
+baz {
+  mod {
+    mod = null
+    num2 = 5
+  }
+  num2 = 5
+}