From bb0a46ac17e4e030f3ba9c08b322039f1afb29c7 Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Mon, 27 Feb 2017 00:33:38 +0300 Subject: [PATCH] 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 | 1 + .../stubs/ClassFileToSourceStubConverter.kt | 56 +++++++++- .../kapt3/testData/converter/nestedClasses.kt | 43 ++++++++ .../testData/converter/nestedClasses.txt | 100 ++++++++++++++++++ 4 files changed, 198 insertions(+), 2 deletions(-) diff --git a/compiler/compiler.pro b/compiler/compiler.pro index 6098c12103a..e6a0d6f6c5b 100644 --- a/compiler/compiler.pro +++ b/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 { diff --git a/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt b/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt index b18c0c33fd4..19895d52cfe 100644 --- a/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt +++ b/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 diff --git a/plugins/kapt3/testData/converter/nestedClasses.kt b/plugins/kapt3/testData/converter/nestedClasses.kt index 5f6d8b307e2..ea2c7cb2bc7 100644 --- a/plugins/kapt3/testData/converter/nestedClasses.kt +++ b/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 + } + } + } } \ No newline at end of file diff --git a/plugins/kapt3/testData/converter/nestedClasses.txt b/plugins/kapt3/testData/converter/nestedClasses.txt index 64c19c0178b..28f27a0b341 100644 --- a/plugins/kapt3/testData/converter/nestedClasses.txt +++ b/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() {