Merge pull request #141 from usethesource/issue140

Fixing issue 140, slowly.
usethesource · Jan 26, 2022 · c956e17 · c956e17
2 parents 767bde5 + ab36f9e
commit c956e17
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 29 deletions.
diff --git a/src/main/java/io/usethesource/vallang/type/AbstractDataType.java b/src/main/java/io/usethesource/vallang/type/AbstractDataType.java
@@ -374,23 +374,22 @@ public boolean equals(@Nullable Object o) {
 
     @Override
     public Type instantiate(Map<Type, Type> bindings) {
-        if (bindings.isEmpty()) {
+        if (bindings.isEmpty() || !isParameterized()) {
             return this;
         }
 
-        Type[] params = new Type[0];
-        if (isParameterized()) {
-            params = new Type[fParameters.getArity()];
-            int i = 0;
-            for (Type p : fParameters) {
-                params[i++] = p.instantiate(bindings);
-            }
-        }
-
         TypeStore store = new TypeStore();
-        store.declareAbstractDataType(this);
 
-        return TypeFactory.getInstance().abstractDataType(store, fName, params);
+        // TODO: find out why we had this call
+        // store.declareAbstractDataType(this);
+
+        // Here it is important _not_ to reuse TupleType.instantiate, since
+        // that has a normalizing feature to `void` if one of the parameters
+        // reduced to `void`. The type parameters of an ADT
+        // can be void and still the ADT type can have values,
+        Type params = instantiateTuple((TupleType) fParameters, bindings);
+
+        return TypeFactory.getInstance().abstractDataTypeFromTuple(store, fName, params);
     }
 
     @Override

diff --git a/src/main/java/io/usethesource/vallang/type/FunctionType.java b/src/main/java/io/usethesource/vallang/type/FunctionType.java
@@ -418,19 +418,6 @@ public Type instantiate(Map<Type, Type> bindings) {
 		return TF.functionType(returnType.instantiate(bindings), instantiateTuple(argumentTypes, bindings), keywordParameters != null ? instantiateTuple(keywordParameters, bindings) : TypeFactory.getInstance().tupleEmpty());
     }
 
-    /**
-     * Instantiate a tuple but do not reduce to void like TupleType.instantiate would
-     * if one of the parameters was substituted by void
-     */
-    private Type instantiateTuple(TupleType t, Map<Type, Type> bindings) {
-        Type[] fChildren = new Type[t.getArity()];
-        for (int i = t.fFieldTypes.length - 1; i >= 0; i--) {
-            fChildren[i] = t.getFieldType(i).instantiate(bindings);
-        }
-
-        return TypeFactory.getInstance().getFromCache(new TupleType(fChildren));
-    }
-
 	@Override
 	public boolean isOpen() {
 		return returnType.isOpen() || argumentTypes.isOpen();

diff --git a/src/main/java/io/usethesource/vallang/type/Type.java b/src/main/java/io/usethesource/vallang/type/Type.java
@@ -664,6 +664,20 @@ protected final boolean isSubtypeOfAlias(Type type) {
     return isSubtypeOf(type.getAliased());
   }
 
+  /**
+   * Instantiate a tuple but do not reduce to void like TupleType.instantiate would
+   * if one of the parameters was substituted by void. This is needed for other types
+   * which use TupleType as an array of Types (i.e. for type parameters, fields and keyword fields)
+   */
+  protected Type instantiateTuple(TupleType t, Map<Type, Type> bindings) {
+      Type[] fChildren = new Type[t.getArity()];
+      for (int i = t.fFieldTypes.length - 1; i >= 0; i--) {
+          fChildren[i] = t.getFieldType(i).instantiate(bindings);
+      }
+
+      return TypeFactory.getInstance().getFromCache(new TupleType(fChildren));
+  }
+
   abstract protected boolean isSubtypeOfReal(Type type);
   abstract protected boolean isSubtypeOfInteger(Type type);
   abstract protected boolean isSubtypeOfRational(Type type);

diff --git a/src/test/java/io/usethesource/vallang/basic/TypeFactorySmokeTest.java b/src/test/java/io/usethesource/vallang/basic/TypeFactorySmokeTest.java
@@ -12,6 +12,7 @@
 
 package io.usethesource.vallang.basic;
 
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.junit.jupiter.api.Assertions.fail;
 
@@ -51,8 +52,15 @@ public void testGetInstance() {
 
     @ParameterizedTest
     @ArgumentsSource(ValueProvider.class)
-    public void testGetTypeByDescriptor() {
-        // TODO: needs to be tested, after we've implemented it
+    public void testParametrizedAbstractDatatypeArityInvariant(TypeStore store) {
+        Type param = ft.parameterType("T");
+        Type adt = ft.abstractDataType(store, "Test", ft.tupleType(param));
+        Map<Type,Type> bindings = new HashMap<>();
+        bindings.put(param, ft.voidType());
+        Type instant = adt.instantiate(bindings);
+
+        assertTrue(instant.isParameterized());
+        assertFalse(instant.getTypeParameters().isBottom());
     }
 
     @ParameterizedTest

diff --git a/src/test/java/io/usethesource/vallang/specification/TypeTest.java b/src/test/java/io/usethesource/vallang/specification/TypeTest.java
@@ -349,7 +349,7 @@ public void testGetTypeDescriptor(Type t1, Type t2) {
     }
 
     @ParameterizedTest @ArgumentsSource(ValueProvider.class)
-    public void testMatchAndInstantiate(TypeFactory ft) {
+    public void testMatchAndInstantiate(TypeFactory ft, TypeStore store) {
         Type X = ft.parameterType("X");
         Map<Type, Type> bindings = new HashMap<>();
 
@@ -390,8 +390,15 @@ public void testMatchAndInstantiate(TypeFactory ft) {
             fail("instantiate failed");
         }
 
+        bindings.clear();
+        Type adtX = ft.abstractDataType(store, "A", X);
+        bindings.put(X, ft.voidType());
+        assertTrue(adtX.instantiate(bindings).isSubtypeOf(adtX));
+
+        bindings.put(X, ft.integerType());
+        assertTrue(adtX.instantiate(bindings).isSubtypeOf(adtX));
     }
-
+  
     @ParameterizedTest @ArgumentsSource(ValueProvider.class)
     public void testAlias(TypeFactory ft) {
         Type alias = ft.aliasType(new TypeStore(), "myValue", ft.valueType());