Kapt3: Ignore inner classes with clashing names (KT-16458)

Ignore classes (and references to such classes in declaration signatures) if one of the containing classes has the same simple name. This is forbidden in Java, thus should not be present in kapt source stubs.

compiler/compiler.pro

@@ -187,6 +187,7 @@ messages/**)
 
 -keepclassmembers class org.jetbrains.org.objectweb.asm.Type {
     *** ARRAY;
+    *** OBJECT;
 }
 
 -keepclassmembers class org.jetbrains.org.objectweb.asm.ClassReader {

plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt

@@ -281,6 +281,11 @@ class ClassFileToSourceStubConverter(
             if (enumValuesData.any { it.innerClass == innerClass }) return@mapJList null
             if (innerClass.outerName != clazz.name) return@mapJList null
             val innerClassNode = kaptContext.compiledClasses.firstOrNull { it.name == innerClass.name } ?: return@mapJList null
+            if (checkIfInnerClassNameConflictsWithOuter(innerClassNode, clazz)) {
+                kaptContext.logger.warn(innerClassNode.name.replace('/', '.').replace('$', '.')
+                        + " will be ignored (name is clashing with the outer class name)")
+                return@mapJList null
+            }
             convertClass(innerClassNode, packageFqName, false)
         }
 
@@ -293,6 +298,38 @@ class ClassFileToSourceStubConverter(
                 enumValues + fields + methods + nestedClasses)
     }
 
+    private tailrec fun checkIfShouldBeIgnored(type: Type): Boolean {
+        if (type.sort == Type.ARRAY) {
+            return checkIfShouldBeIgnored(type.elementType)
+        }
+
+        if (type.sort != Type.OBJECT) return false
+
+        val internalName = type.internalName
+        val clazz = kaptContext.compiledClasses.firstOrNull { it.name == internalName } ?: return false
+        return checkIfInnerClassNameConflictsWithOuter(clazz)
+    }
+
+    private fun findContainingClassNode(clazz: ClassNode): ClassNode? {
+        val innerClassForOuter = clazz.innerClasses.firstOrNull { it.name == clazz.name } ?: return null
+        return kaptContext.compiledClasses.firstOrNull { it.name == innerClassForOuter.outerName }
+    }
+
+    // Java forbids outer and inner class names to be the same. Check if the names are different
+    private tailrec fun checkIfInnerClassNameConflictsWithOuter(
+            clazz: ClassNode,
+            outerClass: ClassNode? = findContainingClassNode(clazz)
+    ): Boolean {
+        if (outerClass == null) return false
+        if (clazz.simpleName == outerClass.simpleName) return true
+        // Try to find the containing class for outerClassNode (to check the whole tree recursively)
+        val containingClassForOuterClass = findContainingClassNode(outerClass) ?: return false
+        return checkIfInnerClassNameConflictsWithOuter(clazz, containingClassForOuterClass)
+    }
+
+    private val ClassNode.simpleName: String
+        get() = name.substringAfterLast(".").substringAfterLast("/")
+
     private fun getClassAccessFlags(clazz: ClassNode, descriptor: DeclarationDescriptor, isInner: Boolean, isNested: Boolean) = when {
         (descriptor.containingDeclaration as? ClassDescriptor)?.kind == ClassKind.INTERFACE -> {
             // Classes inside interfaces should always be public and static.
@@ -315,14 +352,24 @@ class ClassFileToSourceStubConverter(
         }
     }
 
-    private fun convertField(field: FieldNode, packageFqName: String, explicitInitializer: JCExpression? = null): JCVariableDecl? {
+    private fun convertField(
+            field: FieldNode,
+            packageFqName: String,
+            explicitInitializer: JCExpression? = null
+    ): JCVariableDecl? {
         if (isSynthetic(field.access)) return null
-        val descriptor = kaptContext.origins[field]?.descriptor
+        // not needed anymore
+        val origin = kaptContext.origins[field]
+        val descriptor = origin?.descriptor
 
         val modifiers = convertModifiers(field.access, ElementKind.FIELD, packageFqName, field.visibleAnnotations, field.invisibleAnnotations)
         val name = getValidIdentifierName(field.name) ?: return null
         val type = Type.getType(field.desc)
 
+        if (checkIfShouldBeIgnored(type)) {
+            return null
+        }
+
         // Enum type must be an identifier (Javac requirement)
         val typeExpression = if (isEnum(field.access))
             treeMaker.SimpleName(type.className.substringAfterLast('.'))
@@ -371,6 +418,11 @@ class ClassFileToSourceStubConverter(
         val jcReturnType = if (isConstructor) null else treeMaker.Type(asmReturnType)
 
         val parametersInfo = method.getParametersInfo(containingClass)
+
+        if (checkIfShouldBeIgnored(asmReturnType) || parametersInfo.any { checkIfShouldBeIgnored(it.type) }) {
+            return null
+        }
+
         @Suppress("NAME_SHADOWING")
         val parameters = mapJListIndexed(parametersInfo) { index, info ->
             val lastParameter = index == parametersInfo.lastIndex

plugins/kapt3/testData/converter/nestedClasses.kt

@@ -12,4 +12,47 @@ class Test {
     enum class NestedEnum {
         BLACK, WHITE
     }
+}
+
+class Foo {
+    companion object Foo
+}
+
+class A {
+    val x: A? = null
+
+    fun f1(a: A, b: B): A? = null
+
+    interface B {
+        val y: B?
+
+        fun f2(a: A, b: B): A? = null
+
+        class A {
+            val x: A? = null
+            val y: B? = null
+
+            fun f3(a: A, b: B) {}
+
+            object B
+        }
+    }
+
+    object C {
+        interface C
+    }
+}
+
+class A2 {
+    class B {
+        class C {
+            class D {
+                class A2
+                class B
+                class Cme
+                class D
+                class E
+            }
+        }
+    }
 }

plugins/kapt3/testData/converter/nestedClasses.txt

@@ -1,3 +1,103 @@
+public final class A {
+    @org.jetbrains.annotations.Nullable()
+    private final A x = null;
+
+    @org.jetbrains.annotations.Nullable()
+    public final A getX() {
+        return null;
+    }
+
+    @org.jetbrains.annotations.Nullable()
+    public final A f1(@org.jetbrains.annotations.NotNull()
+    A a, @org.jetbrains.annotations.NotNull()
+    A.B b) {
+        return null;
+    }
+
+    public A() {
+        super();
+    }
+
+    public static abstract interface B {
+
+        @org.jetbrains.annotations.Nullable()
+        public abstract A.B getY();
+
+        public static final class DefaultImpls {
+
+            public DefaultImpls() {
+                super();
+            }
+        }
+    }
+
+    public static final class C {
+        public static final A.C INSTANCE = null;
+
+        private C() {
+            super();
+        }
+    }
+}
+
+////////////////////
+
+
+public final class A2 {
+
+    public A2() {
+        super();
+    }
+
+    public static final class B {
+
+        public B() {
+            super();
+        }
+
+        public static final class C {
+
+            public C() {
+                super();
+            }
+
+            public static final class D {
+
+                public D() {
+                    super();
+                }
+
+                public static final class Cme {
+
+                    public Cme() {
+                        super();
+                    }
+                }
+
+                public static final class E {
+
+                    public E() {
+                        super();
+                    }
+                }
+            }
+        }
+    }
+}
+
+////////////////////
+
+
+public final class Foo {
+
+    public Foo() {
+        super();
+    }
+}
+
+////////////////////
+
+
 public final class Test {
 
     public Test() {