Merge pull request #9704 from smowton/smowton/fix/generic-inner-class

Kotlin: note that raw inner classes nest within a raw outer

Author: smowton

java/kotlin-extractor/src/main/kotlin/KotlinFileExtractor.kt

@@ -6,6 +6,7 @@ import com.github.codeql.utils.versions.functionN
 import com.github.codeql.utils.versions.getIrStubFromDescriptor
 import com.semmle.extractor.java.OdasaOutput
 import org.jetbrains.kotlin.backend.common.extensions.IrPluginContext
+import org.jetbrains.kotlin.backend.common.lower.parents
 import org.jetbrains.kotlin.backend.common.pop
 import org.jetbrains.kotlin.builtins.functions.BuiltInFunctionArity
 import org.jetbrains.kotlin.descriptors.*
@@ -314,8 +315,21 @@ open class KotlinFileExtractor(
             val locId = getLocation(c, argsIncludingOuterClasses)
             tw.writeHasLocation(id, locId)
 
-            // Extract the outer <-> inner class relationship, passing on any type arguments in excess to this class' parameters.
-            extractEnclosingClass(c, id, locId, argsIncludingOuterClasses?.drop(c.typeParameters.size) ?: listOf())
+            // Extract the outer <-> inner class relationship, passing on any type arguments in excess to this class' parameters if this is an inner class.
+            // For example, in `class Outer<T> { inner class Inner<S> { } }`, `Inner<Int, String>` nests within `Outer<Int>` and raw `Inner<>` within `Outer<>`,
+            // but for a similar non-`inner` (in Java terms, static nested) class both `Inner<Int>` and `Inner<>` nest within the unbound type `Outer`.
+            val useBoundOuterType = (c.isInner || c.isLocal) && (c.parents.map { // Would use `firstNotNullOfOrNull`, but this doesn't exist in Kotlin 1.4
+                when(it) {
+                    is IrClass -> when {
+                        it.typeParameters.isNotEmpty() -> true // Type parameters visible to this class -- extract an enclosing bound or raw type.
+                        !(it.isInner || it.isLocal) -> false // No type parameters seen yet, and this is a static class -- extract an enclosing unbound type.
+                        else -> null // No type parameters seen here, but may be visible enclosing type parameters; keep searching.
+                    }
+                    else -> null // Look through enclosing non-class entities (this may need to change)
+                }
+            }.firstOrNull { it != null } ?: false)
+
+            extractEnclosingClass(c, id, locId, if (useBoundOuterType) argsIncludingOuterClasses?.drop(c.typeParameters.size) else listOf())
 
             return id
         }
@@ -458,7 +472,8 @@ open class KotlinFileExtractor(
         }
     }
 
-    private fun extractEnclosingClass(innerDeclaration: IrDeclaration, innerId: Label<out DbClassorinterface>, innerLocId: Label<DbLocation>, parentClassTypeArguments: List<IrTypeArgument>) {
+    // If `parentClassTypeArguments` is null, the parent class is a raw type.
+    private fun extractEnclosingClass(innerDeclaration: IrDeclaration, innerId: Label<out DbClassorinterface>, innerLocId: Label<DbLocation>, parentClassTypeArguments: List<IrTypeArgument>?) {
         with("enclosing class", innerDeclaration) {
             var parent: IrDeclarationParent? = innerDeclaration.parent
             while (parent != null) {

java/ql/integration-tests/posix-only/kotlin/nested_generic_types/JavaUser.java

@@ -21,6 +21,15 @@ public static void test() {
     String result4 = d.identity("goodbye");
     Short result5 = e.returnSixth(1, "hello", 1.0f, 1.0, 1L, (short)1);
 
+    OuterGeneric<String>.InnerNotGeneric innerGetterTest = (new OuterGeneric<String>()).getInnerNotGeneric();
+    OuterNotGeneric.InnerGeneric<String> innerGetterTest2 = (new OuterNotGeneric()).getInnerGeneric();
+
+    TypeParamVisibility<String> tpv = new TypeParamVisibility<String>();
+    TypeParamVisibility<String>.VisibleBecauseInner<String> visibleBecauseInner = tpv.getVisibleBecauseInner();
+    TypeParamVisibility<String>.VisibleBecauseInnerIndirectContainer.VisibleBecauseInnerIndirect<String> visibleBecauseInnerIndirect = tpv.getVisibleBecauseInnerIndirect();
+    TypeParamVisibility.NotVisibleBecauseStatic<String> notVisibleBecauseStatic = tpv.getNotVisibleBecauseStatic();
+    TypeParamVisibility.NotVisibleBecauseStaticIndirectContainer.NotVisibleBecauseStaticIndirect<String> notVisibleBecauseStaticIndirect = tpv.getNotVisibleBecauseStaticIndirect();
+
   }
 
 }

java/ql/integration-tests/posix-only/kotlin/nested_generic_types/KotlinUser.kt

@@ -22,6 +22,15 @@ class User {
     val result4 = d.identity("goodbye")
     val result5 = e.returnSixth(1, "hello", 1.0f, 1.0, 1L, 1.toShort())
 
+    val innerGetterTest = OuterGeneric<String>().getInnerNotGeneric()
+    val innerGetterTest2 = OuterNotGeneric().getInnerGeneric()
+
+    val tpv = TypeParamVisibility<String>()
+    val visibleBecauseInner = tpv.getVisibleBecauseInner();
+    val visibleBecauseInnerIndirect = tpv.getVisibleBecauseInnerIndirect()
+    val notVisibleBecauseStatic = tpv.getNotVisibleBecauseStatic()
+    val notVisibleBecauseStaticIndirect = tpv.getNotVisibleBecauseStaticIndirect()
+
   }
 
 }

java/ql/integration-tests/posix-only/kotlin/nested_generic_types/libsrc/extlib/OuterGeneric.java

@@ -8,6 +8,8 @@ public class InnerNotGeneric {
 
   }
 
+  public InnerNotGeneric getInnerNotGeneric() { return null; }
+
   public class InnerGeneric<S> {
 
     public <R> InnerGeneric(R r) { }

java/ql/integration-tests/posix-only/kotlin/nested_generic_types/libsrc/extlib/OuterNotGeneric.java

@@ -8,4 +8,10 @@ public class InnerGeneric<S> {
 
   }
 
+  public InnerGeneric<String> getInnerGeneric() {
+
+    return new InnerGeneric<String>();
+
+  }
+
 }

java/ql/integration-tests/posix-only/kotlin/nested_generic_types/libsrc/extlib/TypeParamVisibility.java

@@ -0,0 +1,29 @@
+package extlib;
+
+public class TypeParamVisibility<T> {
+
+  public class VisibleBecauseInner<S> { }
+
+  public class VisibleBecauseInnerIndirectContainer {
+
+    public class VisibleBecauseInnerIndirect<S> { }
+
+  }
+
+  public static class NotVisibleBecauseStatic<S> { }
+
+  public static class NotVisibleBecauseStaticIndirectContainer {
+
+    public class NotVisibleBecauseStaticIndirect<S> { }
+
+  }
+
+  public VisibleBecauseInner<String> getVisibleBecauseInner() { return new VisibleBecauseInner<String>(); }
+
+  public VisibleBecauseInnerIndirectContainer.VisibleBecauseInnerIndirect<String> getVisibleBecauseInnerIndirect() { return (new VisibleBecauseInnerIndirectContainer()).new VisibleBecauseInnerIndirect<String>(); }
+
+  public NotVisibleBecauseStatic<String> getNotVisibleBecauseStatic() { return new NotVisibleBecauseStatic(); }
+
+  public NotVisibleBecauseStaticIndirectContainer.NotVisibleBecauseStaticIndirect<String> getNotVisibleBecauseStaticIndirect() { return (new NotVisibleBecauseStaticIndirectContainer()).new NotVisibleBecauseStaticIndirect<String>(); }
+
+}