Kotlin nullability for collection-wrapped types

common/schema-builder/src/main/java/io/smallrye/graphql/schema/creator/OperationCreator.java

@@ -1,16 +1,20 @@
 package io.smallrye.graphql.schema.creator;
 
 import java.lang.reflect.Modifier;
+import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
 import org.jboss.jandex.AnnotationInstance;
 import org.jboss.jandex.DotName;
 import org.jboss.jandex.MethodInfo;
+import org.jboss.jandex.ParameterizedType;
 import org.jboss.jandex.Type;
 import org.jboss.logging.Logger;
 
 import io.smallrye.graphql.schema.Annotations;
+import io.smallrye.graphql.schema.Classes;
 import io.smallrye.graphql.schema.SchemaBuilderException;
 import io.smallrye.graphql.schema.helper.DeprecatedDirectivesHelper;
 import io.smallrye.graphql.schema.helper.Direction;
@@ -105,9 +109,7 @@ public Operation createOperation(MethodInfo methodInfo, OperationType operationT
         addDirectiveForDeprecated(annotationsForMethod, operation);
         populateField(Direction.OUT, operation, fieldType, annotationsForMethod);
 
-        if (operation.hasWrapper()) {
-            checkWrappedTypeKotlinNullability(methodInfo, annotationsForClass, operation);
-        }
+        checkWrappedTypeKotlinNullability(methodInfo, annotationsForClass, operation);
         return operation;
     }
 
@@ -130,36 +132,126 @@ private void checkWrappedTypeKotlinNullability(MethodInfo methodInfo,
                     .filter(f -> compareParameterLists(f.getValueParameters(), methodInfo.parameterTypes()))
                     .findAny();
             if (function.isPresent()) {
-                KmType returnType = function.get().getReturnType();
-                KmTypeProjection arg = returnType.getArguments().get(0);
-                int flags = arg.getType().getFlags();
-                boolean nullable = Flag.Type.IS_NULLABLE.invoke(flags);
-                if (nullable) {
-                    operation.setNotNull(false);
+                // handle return type
+                if (operation.hasWrapper()) {
+                    KmType returnType = function.get().getReturnType();
+                    var nullable = isKotlinWrappedTypeNullable(returnType);
+                    if (operation.getWrapper().isCollectionOrArrayOrMap()) {
+                        operation.getWrapper().setNotEmpty(!nullable);
+                        // workaround: consistent behavior to java: if wrapped type is non null, collection will be marked as non null
+                        if (!nullable && !operation.isNotNull()) {
+                            operation.setNotNull(true);
+                        }
+                    } else {
+                        // Uni, etc
+                        if (nullable) {
+                            operation.setNotNull(false);
+                        }
+                    }
+                }
+
+                // handle arguments
+                List<Argument> arguments = operation.getArguments();
+                List<KmValueParameter> valueParameters = function.get().getValueParameters();
+                if (arguments.size() == valueParameters.size()) {
+                    for (int i = 0; i < arguments.size(); i++) {
+                        Argument argument = arguments.get(i);
+                        if (argument.hasWrapper() && argument.getWrapper().isCollectionOrArrayOrMap()) {
+                            KmValueParameter typeParameter = valueParameters.get(i);
+                            var paramNullable = isKotlinWrappedTypeNullable(typeParameter.getType());
+                            argument.getWrapper().setNotEmpty(!paramNullable);
+                            // workaround:  consistent behavior to java: if wrapped type is not null, collection will be marked as not null
+                            if (!paramNullable && !argument.isNotNull()) {
+                                argument.setNotNull(true);
+                            }
+                        }
+                    }
                 }
             }
         }
     }
 
+    private static boolean isKotlinWrappedTypeNullable(KmType kotlinType) {
+        if (kotlinType.getArguments().isEmpty()) {
+            return false;
+        }
+        KmTypeProjection arg = kotlinType.getArguments().get(0);
+        int flags = arg.getType().getFlags();
+        boolean nullable = Flag.Type.IS_NULLABLE.invoke(flags);
+        return nullable;
+    }
+
     private boolean compareParameterLists(List<KmValueParameter> kotlinParameters,
             List<Type> jandexParameters) {
         if (kotlinParameters.size() != jandexParameters.size()) {
             return false;
         }
         for (int i = 0; i < kotlinParameters.size(); i++) {
-            // TODO: the matching of parameter types could use some improvements
-            // For example, it won't work for primitives.
-            // An Int parameter will be represented as kotlin.Int in the KotlinClassMetadata,
-            // but as "int" in the Jandex MethodInfo.
-            if (!((KmClassifier.Class) kotlinParameters.get(i).getType().classifier)
-                    .getName().replace("/", ".")
-                    .equals(jandexParameters.get(i).name().toString())) {
+            KmType kotlinType = kotlinParameters.get(i).getType();
+            Type jandexType = jandexParameters.get(i);
+            if (!compareJavaAndKotlinType(jandexType, kotlinType)) {
                 return false;
             }
         }
         return true;
     }
 
+    private boolean compareJavaAndKotlinType(Type javaType, kotlinx.metadata.KmType kotlinType) {
+        if (kotlinType == null) {
+            return false;
+        }
+        String kotlinTypeName = ((KmClassifier.Class) kotlinType.classifier).getName().replace("/", ".");
+
+        boolean signatureIsEqual = false;
+        if (kotlinTypeName.equals(javaType.name().toString())) {
+            signatureIsEqual = true;
+        } else if (Classes.isCollection(javaType)) {
+            signatureIsEqual = compareKotlinCollectionType(javaType, kotlinType);
+        }
+        // TODO: the matching of parameter types could use some improvements
+        // TODO: only some cases are handled, there are much more: see https://kotlinlang.org/docs/java-interop.html#mapped-types
+
+        boolean parametersAreEqual = true;
+        if (javaType instanceof ParameterizedType) {
+            List<Type> arguments = javaType.asParameterizedType().arguments();
+            for (int i = 0; i < arguments.size(); i++) {
+                Type javaArg = arguments.get(i);
+                KmType kotlinArg = kotlinType.getArguments().get(i).getType();
+
+                boolean argTypeEqual = compareJavaAndKotlinType(javaArg, kotlinArg);
+                if (!argTypeEqual) {
+                    parametersAreEqual = false;
+                    break;
+                }
+            }
+        }
+
+        return signatureIsEqual && parametersAreEqual;
+    }
+
+    private boolean compareKotlinCollectionType(Type javaType, KmType kotlinType) {
+        String javaCollectionType = javaType.name().toString();
+        String kotlinCollectionType = ((KmClassifier.Class) kotlinType.classifier).getName().replace("/", ".");
+        // TODO: naive implementation.
+        if (Collection.class.getName().equals(javaCollectionType)) {
+            return kotlinCollectionType.equals("kotlin.collections.Collection") ||
+                    kotlinCollectionType.equals("kotlin.collections.MutableCollection");
+        }
+        if (List.class.getName().equals(javaCollectionType)) {
+            return kotlinCollectionType.equals("kotlin.collections.List") ||
+                    kotlinCollectionType.equals("kotlin.collections.MutableList");
+        }
+        if (Set.class.getName().equals(javaCollectionType)) {
+            return kotlinCollectionType.equals("kotlin.collections.Set") ||
+                    kotlinCollectionType.equals("kotlin.collections.MutableSet");
+        }
+        if (Iterable.class.getName().equals(javaCollectionType)) {
+            return kotlinCollectionType.equals("kotlin.collections.Iterable") ||
+                    kotlinCollectionType.equals("kotlin.collections.MutableIterable");
+        }
+        return false;
+    }
+
     private KotlinClassMetadata.Class toKotlinClassMetadata(AnnotationInstance metadata) {
         KotlinClassHeader classHeader = new KotlinClassHeader(
                 metadata.value("k").asInt(),

common/schema-builder/src/test/java/io/smallrye/graphql/index/SchemaBuilderTest.java

@@ -324,6 +324,51 @@ public void testKotlinTypeNullability() {
         assertTrue(getQueryByName(schema, "zzz4").isNotNull());
     }
 
+    @Test
+    public void testKotlinTypeWrappedInCollectionNullability() {
+        Indexer indexer = new Indexer();
+        indexDirectory(indexer, "io/smallrye/graphql/kotlin");
+        IndexView index = indexer.complete();
+        Schema schema = SchemaBuilder.build(index);
+
+        var yyy1 = getQueryByName(schema, "yyy1");
+        assertTrue(yyy1.isNotNull());
+        assertTrue(yyy1.getWrapper().isNotEmpty());
+
+        var yyy2 = getQueryByName(schema, "yyy2");
+        assertTrue(yyy2.isNotNull());
+        assertFalse(yyy2.getWrapper().isNotEmpty());
+
+        var yyy3 = getQueryByName(schema, "yyy3");
+        assertTrue(yyy3.isNotNull());
+        assertTrue(yyy3.getWrapper().isNotEmpty());
+
+        var yyy4 = getQueryByName(schema, "yyy4");
+        assertFalse(yyy4.isNotNull());
+        var arguments = yyy4.getArguments();
+        assertTrue(arguments.get(0).isNotNull()); // i0
+
+        assertFalse(arguments.get(1).isNotNull()); // i1
+
+        assertTrue(arguments.get(2).isNotNull()); // i2
+        assertTrue(arguments.get(2).getWrapper().isNotEmpty()); // i2
+
+        assertTrue(arguments.get(3).isNotNull()); // i3
+        assertFalse(arguments.get(3).getWrapper().isNotEmpty()); // i3
+
+        assertFalse(arguments.get(4).isNotNull()); // i4
+        assertFalse(arguments.get(4).getWrapper().isNotEmpty()); // i4
+
+        // it is not consistent with java: if in java the wrapped list element is not null, the list will be marked as non-null in any case.
+        assertTrue(arguments.get(5).isNotNull()); // i5
+        assertTrue(arguments.get(5).getWrapper().isNotEmpty()); // i5
+
+        var yyy5 = getQueryByName(schema, "yyy5");
+        // it is not consistent with java: if in java the wrapped list element is not null, the list will be marked as non-null in any case.
+        assertTrue(yyy5.isNotNull());
+        assertTrue(yyy5.getWrapper().isNotEmpty());
+    }
+
     private Operation getQueryByName(Schema schema, String name) {
         return schema.getQueries()
                 .stream().filter(q -> q.getName().equals(name))

common/schema-builder/src/test/kotlin/io/smallrye/graphql/kotlin/Foo.kt

@@ -46,4 +46,18 @@ class Example {
   @Query("zzz4")
   fun zzz(x: Foo4): Uni<Foo> = Uni.createFrom().nullItem()
 
+  @Query("yyy1")
+  fun yyy1(): List<Foo> = listOf()
+
+  @Query("yyy2")
+  fun yyy2(): List<Foo?> = listOf()
+
+  @Query("yyy3")
+  fun yyy3(i: List<Foo>): List<Foo> = listOf()
+
+  @Query("yyy4")
+  fun yyy4(i0: Foo, i1: Foo?, i2: List<Foo>, i3: List<Foo?>, i4: List<Foo?>?, i5: List<Foo>?): Foo? = null
+
+  @Query("yyy5")
+  fun yyy5(): List<Foo>? = listOf()
 }