From 84e5fa38bb1182156bd13b6b40084e428e6b0ef1 Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Fri, 3 Nov 2017 13:43:53 +0900 Subject: [PATCH] Kapt: Exclude all primitive Java constants from inlining (String is still inlined) (KT-20878) --- .../stubs/ClassFileToSourceStubConverter.kt | 34 ++--- .../kapt3/test/AbstractKotlinKapt3Test.kt | 27 ++-- .../testData/converter/kt18791.kt | 41 +++++- .../testData/converter/kt18791.txt | 121 +++++++++++++++++- .../testData/converter/nestedClasses2.kt | 3 + .../testData/converter/nestedClasses2.txt | 76 +++++++++++ .../converter/nestedClassesNonRootPackage.kt | 2 + .../converter/nestedClassesNonRootPackage.txt | 78 +++++++++++ 8 files changed, 350 insertions(+), 32 deletions(-) diff --git a/plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt b/plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt index 29c6e571561..5741c79490e 100644 --- a/plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt +++ b/plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt @@ -32,6 +32,7 @@ import org.jetbrains.kotlin.kapt3.javac.KaptTreeMaker import org.jetbrains.kotlin.kapt3.javac.KaptJavaFileObject import org.jetbrains.kotlin.kapt3.javac.kaptError import org.jetbrains.kotlin.kapt3.util.* +import org.jetbrains.kotlin.load.java.sources.JavaSourceElement import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.BindingContext @@ -678,9 +679,9 @@ class ClassFileToSourceStubConverter( val args = value?.arguments?.mapNotNull { it.getArgumentExpression() } ?: emptyList() val singleArg by lazy { args.singleOrNull() } - if (constantValue is Int) { - // This may be a resource identifier, we should not inline the id int - tryParseReferenceToAndroidResource(singleArg)?.let { return it } + if (constantValue.isOfPrimiviteType()) { + // Do not inline primitive constants + tryParseReferenceToIntConstant(singleArg)?.let { return it } } fun tryParseTypeExpression(expression: KtExpression?): JCExpression? { @@ -741,7 +742,7 @@ class ClassFileToSourceStubConverter( return convertLiteralExpression(constantValue) } - private fun tryParseReferenceToAndroidResource(expression: KtExpression?): JCExpression? { + private fun tryParseReferenceToIntConstant(expression: KtExpression?): JCExpression? { val bindingContext = kaptContext.bindingContext val expressionToResolve = when (expression) { @@ -750,22 +751,10 @@ class ClassFileToSourceStubConverter( } val resolvedCall = expressionToResolve.getResolvedCall(bindingContext) ?: return null - val fqName = resolvedCall.resultingDescriptor.fqNameOrNull() ?: return null - val jcExpression = treeMaker.FqName(fqName) - - return if (doesLookLikeAndroidIdentifier(jcExpression)) jcExpression else null - } - - private fun doesLookLikeAndroidIdentifier(expression: JCExpression): Boolean { - // 'expression' here is always a fqName, so we can forget about imports here - val rId = (expression as? JCFieldAccess)?.selected ?: return false - val r = (rId as? JCFieldAccess)?.selected ?: return false - - return when { - r is JCIdent && r.name.toString() == "R" -> true - r is JCFieldAccess && r.name.toString() == "R" -> true - else -> false - } + // Disable inlining only for Java statics + val resultingDescriptor = resolvedCall.resultingDescriptor.takeIf { it.source is JavaSourceElement } ?: return null + val fqName = resultingDescriptor.fqNameOrNull()?.takeIf { isValidQualifiedName(it) } ?: return null + return treeMaker.FqName(fqName) } private fun convertValueOfPrimitiveTypeOrString(value: Any?): JCExpression? { @@ -823,5 +812,10 @@ class ClassFileToSourceStubConverter( } } +private fun Any?.isOfPrimiviteType(): Boolean = when(this) { + is Boolean, is Byte, is Int, is Long, is Short, is Char, is Float, is Double -> true + else -> false +} + private val ClassDescriptor.isNested: Boolean get() = containingDeclaration is ClassDescriptor \ No newline at end of file diff --git a/plugins/kapt3/kapt3-compiler/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3Test.kt b/plugins/kapt3/kapt3-compiler/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3Test.kt index 420605df89d..5dafe3800bc 100644 --- a/plugins/kapt3/kapt3-compiler/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3Test.kt +++ b/plugins/kapt3/kapt3-compiler/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3Test.kt @@ -69,24 +69,33 @@ abstract class AbstractKotlinKapt3Test : CodegenTestCase() { val logger = KaptLogger(isVerbose = true, messageCollector = messageCollector) val kaptContext = KaptContext(logger, generationState.project, generationState.bindingContext, classBuilderFactory.compiledClasses, classBuilderFactory.origins, generationState, processorOptions = emptyMap()) + + val javaFiles = files + .filter { it.name.toLowerCase().endsWith(".java") } + .map { File.createTempFile(it.name.substringBeforeLast('.'), ".java").apply { writeText(it.content) } } + try { - check(kaptContext, txtFile, wholeFile) + check(kaptContext, javaFiles, txtFile, wholeFile) } finally { + javaFiles.forEach { it.delete() } kaptContext.close() } } protected fun convert( kaptContext: KaptContext, + javaFiles: List, generateNonExistentClass: Boolean, correctErrorTypes: Boolean ): JavacList { val converter = ClassFileToSourceStubConverter(kaptContext, generateNonExistentClass, correctErrorTypes) - return converter.convert() + val parsedJavaFiles = kaptContext.compiler.parseFiles(kaptContext.fileManager.getJavaFileObjectsFromFiles(javaFiles)) + return converter.convert().appendList(parsedJavaFiles) } protected abstract fun check( kaptContext: KaptContext, + javaFiles: List, txtFile: File, wholeFile: File) } @@ -99,7 +108,7 @@ abstract class AbstractClassFileToSourceStubConverterTest : AbstractKotlinKapt3T fun removeMetadataAnnotationContents(s: String): String = s.replace(KOTLIN_METADATA_REGEX, "@kotlin.Metadata()") } - override fun check(kaptContext: KaptContext, txtFile: File, wholeFile: File) { + override fun check(kaptContext: KaptContext, javaFiles: List, txtFile: File, wholeFile: File) { fun isOptionSet(name: String) = wholeFile.useLines { lines -> lines.any { it.trim() == "// $name" } } fun getOptionValues(name: String) = wholeFile.useLines { lines -> @@ -114,12 +123,12 @@ abstract class AbstractClassFileToSourceStubConverterTest : AbstractKotlinKapt3T val validate = !isOptionSet("NO_VALIDATION") val expectedErrors = getOptionValues("EXPECTED_ERROR").sorted() - val javaFiles = convert(kaptContext, generateNonExistentClass, correctErrorTypes) + val convertedFiles = convert(kaptContext, javaFiles, generateNonExistentClass, correctErrorTypes) - kaptContext.javaLog.interceptorData.files = javaFiles.map { it.sourceFile to it }.toMap() - if (validate) kaptContext.compiler.enterTrees(javaFiles) + kaptContext.javaLog.interceptorData.files = convertedFiles.map { it.sourceFile to it }.toMap() + if (validate) kaptContext.compiler.enterTrees(convertedFiles) - val actualRaw = javaFiles.sortedBy { it.sourceFile.name }.joinToString (FILE_SEPARATOR) + val actualRaw = convertedFiles.sortedBy { it.sourceFile.name }.joinToString (FILE_SEPARATOR) val actual = StringUtil.convertLineSeparators(actualRaw.trim({ it <= ' ' })) .trimTrailingWhitespacesAndAddNewlineAtEOF() .let { removeMetadataAnnotationContents(it) } @@ -148,8 +157,8 @@ abstract class AbstractClassFileToSourceStubConverterTest : AbstractKotlinKapt3T } abstract class AbstractKotlinKaptContextTest : AbstractKotlinKapt3Test() { - override fun check(kaptContext: KaptContext, txtFile: File, wholeFile: File) { - val compilationUnits = convert(kaptContext, generateNonExistentClass = false, correctErrorTypes = true) + override fun check(kaptContext: KaptContext, javaFiles: List, txtFile: File, wholeFile: File) { + val compilationUnits = convert(kaptContext, javaFiles, generateNonExistentClass = false, correctErrorTypes = true) val sourceOutputDir = Files.createTempDirectory("kaptRunner").toFile() try { kaptContext.doAnnotationProcessing(emptyList(), listOf(JavaKaptContextTest.simpleProcessor()), diff --git a/plugins/kapt3/kapt3-compiler/testData/converter/kt18791.kt b/plugins/kapt3/kapt3-compiler/testData/converter/kt18791.kt index c5a9c833d84..d058ae80e15 100644 --- a/plugins/kapt3/kapt3-compiler/testData/converter/kt18791.kt +++ b/plugins/kapt3/kapt3-compiler/testData/converter/kt18791.kt @@ -1,3 +1,8 @@ +// EXPECTED_ERROR class B is public, should be declared in a file named B.java +// EXPECTED_ERROR class R is public, should be declared in a file named R.java +// EXPECTED_ERROR class R is public, should be declared in a file named R.java +// EXPECTED_ERROR class R2 is public, should be declared in a file named R2.java + //FILE: lib/R.java package lib; @@ -16,6 +21,15 @@ public class R { } } +//FILE: app/R2.java +package app; + +public class R2 { // For ButterKnife library project support + public static class layout { + public final static int mainActivity = 100; + } +} + //FILE: app/B.java package app; @@ -23,6 +37,16 @@ public class B { public static class id { public final static int textView = 200; } + + public final static boolean a1 = false; + public final static byte a2 = 1; + public final static int a3 = 2; + public final static short a4 = 3; + public final static long a5 = 4L; + public final static char a6 = '5'; + public final static float a7 = 6.0f; + public final static double a8 = 7.0; + public final static String a9 = "A"; } //FILE: test.kt @@ -33,6 +57,17 @@ import lib.R.id.textView annotation class Bind(val id: Int) +annotation class Anno( + val a1: Boolean, + val a2: Byte, + val a3: Int, + val a4: Short, + val a5: Long, + val a6: Char, + val a7: Float, + val a8: Double, + val a9: String) + class MyActivity { @Bind(LibR.id.textView) fun foo() {} @@ -46,6 +81,10 @@ class MyActivity { @Bind(R.layout.mainActivity) fun foo4() {} + @Bind(R2.layout.mainActivity) + @Anno(a1 = B.a1, a2 = B.a2, a3 = B.a3, a4 = B.a4, a5 = B.a5, a6 = B.a6, a7 = B.a7, a8 = B.a8, a9 = B.a9) + fun foo5() {} + @Bind(B.id.textView) - fun notResource() {} + fun plainIntConstant() {} } \ No newline at end of file diff --git a/plugins/kapt3/kapt3-compiler/testData/converter/kt18791.txt b/plugins/kapt3/kapt3-compiler/testData/converter/kt18791.txt index f56e2542bf0..24fe2663978 100644 --- a/plugins/kapt3/kapt3-compiler/testData/converter/kt18791.txt +++ b/plugins/kapt3/kapt3-compiler/testData/converter/kt18791.txt @@ -1,5 +1,117 @@ package app; +public class B { + + public B() { + super(); + } + + public static class id { + + public id() { + super(); + } + public static final int textView = 200; + } + public static final boolean a1 = false; + public static final byte a2 = 1; + public static final int a3 = 2; + public static final short a4 = 3; + public static final long a5 = 4L; + public static final char a6 = '5'; + public static final float a7 = 6.0F; + public static final double a8 = 7.0; + public static final String a9 = "A"; +} + +//////////////////// + +package app; + +public class R2 { + + public R2() { + super(); + } + + public static class layout { + + public layout() { + super(); + } + public static final int mainActivity = 100; + } +} + +//////////////////// + +package lib; + +public class R { + + public R() { + super(); + } + + public static class id { + + public id() { + super(); + } + public static final int textView = 100; + } +} + +//////////////////// + +package app; + +public class R { + + public R() { + super(); + } + + public static class layout { + + public layout() { + super(); + } + public static final int mainActivity = 100; + } +} + +//////////////////// + +package app; + +@kotlin.Metadata() +@java.lang.annotation.Retention(value = java.lang.annotation.RetentionPolicy.RUNTIME) +public abstract @interface Anno { + + public abstract boolean a1(); + + public abstract byte a2(); + + public abstract int a3(); + + public abstract short a4(); + + public abstract long a5(); + + public abstract char a6(); + + public abstract float a7(); + + public abstract double a8(); + + public abstract java.lang.String a9(); +} + +//////////////////// + +package app; + @kotlin.Metadata() @java.lang.annotation.Retention(value = java.lang.annotation.RetentionPolicy.RUNTIME) public abstract @interface Bind { @@ -30,8 +142,13 @@ public final class MyActivity { public final void foo4() { } - @Bind(id = 200) - public final void notResource() { + @Anno(a1 = app.B.a1, a2 = app.B.a2, a3 = app.B.a3, a4 = app.B.a4, a5 = app.B.a5, a6 = app.B.a6, a7 = app.B.a7, a8 = app.B.a8, a9 = "A") + @Bind(id = app.R2.layout.mainActivity) + public final void foo5() { + } + + @Bind(id = app.B.id.textView) + public final void plainIntConstant() { } public MyActivity() { diff --git a/plugins/kapt3/kapt3-compiler/testData/converter/nestedClasses2.kt b/plugins/kapt3/kapt3-compiler/testData/converter/nestedClasses2.kt index e156a4841e3..7d453bed8ea 100644 --- a/plugins/kapt3/kapt3-compiler/testData/converter/nestedClasses2.kt +++ b/plugins/kapt3/kapt3-compiler/testData/converter/nestedClasses2.kt @@ -1,3 +1,6 @@ +// EXPECTED_ERROR class J$B is public, should be declared in a file named J$B.java +// EXPECTED_ERROR class JavaClass is public, should be declared in a file named JavaClass.java + // FILE: JavaClass.java public class JavaClass { public class Foo { diff --git a/plugins/kapt3/kapt3-compiler/testData/converter/nestedClasses2.txt b/plugins/kapt3/kapt3-compiler/testData/converter/nestedClasses2.txt index aea2d1e950c..b69062897f5 100644 --- a/plugins/kapt3/kapt3-compiler/testData/converter/nestedClasses2.txt +++ b/plugins/kapt3/kapt3-compiler/testData/converter/nestedClasses2.txt @@ -1,3 +1,79 @@ +public class J$B { + + public J$B() { + super(); + } + + public class C { + + public C() { + super(); + } + } + + public class D$E { + + public D$E() { + super(); + } + + class F { + + F() { + super(); + } + } + + class F$G { + + F$G() { + super(); + } + } + } + + public class D$$E { + + public D$$E() { + super(); + } + } + + public class D$$$E { + + public D$$$E() { + super(); + } + } +} + +//////////////////// + + +public class JavaClass { + + public JavaClass() { + super(); + } + + public class Foo { + + public Foo() { + super(); + } + + public class Bar { + + public Bar() { + super(); + } + } + } +} + +//////////////////// + + @kotlin.Metadata() public final class A$B { @org.jetbrains.annotations.NotNull() diff --git a/plugins/kapt3/kapt3-compiler/testData/converter/nestedClassesNonRootPackage.kt b/plugins/kapt3/kapt3-compiler/testData/converter/nestedClassesNonRootPackage.kt index 9a8218aba97..14f1f33ed57 100644 --- a/plugins/kapt3/kapt3-compiler/testData/converter/nestedClassesNonRootPackage.kt +++ b/plugins/kapt3/kapt3-compiler/testData/converter/nestedClassesNonRootPackage.kt @@ -1,3 +1,5 @@ +// EXPECTED_ERROR class J$B is public, should be declared in a file named J$B.java + // FILE: test/JavaClass.java package test; diff --git a/plugins/kapt3/kapt3-compiler/testData/converter/nestedClassesNonRootPackage.txt b/plugins/kapt3/kapt3-compiler/testData/converter/nestedClassesNonRootPackage.txt index a9041858fc6..cfcd1a772e9 100644 --- a/plugins/kapt3/kapt3-compiler/testData/converter/nestedClassesNonRootPackage.txt +++ b/plugins/kapt3/kapt3-compiler/testData/converter/nestedClassesNonRootPackage.txt @@ -1,5 +1,83 @@ package test; +public class J$B { + + public J$B() { + super(); + } + + public class C { + + public C() { + super(); + } + } + + public class D$E { + + public D$E() { + super(); + } + + class F { + + F() { + super(); + } + } + + class F$G { + + F$G() { + super(); + } + } + } + + public class D$$E { + + public D$$E() { + super(); + } + } + + public class D$$$E { + + public D$$$E() { + super(); + } + } +} + +//////////////////// + +package test; + +class JavaClass { + + JavaClass() { + super(); + } + + class Foo { + + Foo() { + super(); + } + + class Bar { + + Bar() { + super(); + } + } + } +} + +//////////////////// + +package test; + @kotlin.Metadata() public final class A$B { @org.jetbrains.annotations.NotNull()