Kapt: Exclude all primitive Java constants from inlining (String is still inlined) (KT-20878)

2017-11-03 13:43:53 +09:00
parent 6ca5787799
commit 84e5fa38bb
8 changed files with 350 additions and 32 deletions
@@ -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
@@ -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<GenerationState>,
+            javaFiles: List<File>,
            generateNonExistentClass: Boolean,
            correctErrorTypes: Boolean
    ): JavacList<JCCompilationUnit> {
        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<GenerationState>,
+            javaFiles: List<File>,
            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<GenerationState>, txtFile: File, wholeFile: File) {
+    override fun check(kaptContext: KaptContext<GenerationState>, javaFiles: List<File>, 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<GenerationState>, txtFile: File, wholeFile: File) {
-        val compilationUnits = convert(kaptContext, generateNonExistentClass = false, correctErrorTypes = true)
+    override fun check(kaptContext: KaptContext<GenerationState>, javaFiles: List<File>, 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()),
@@ -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() {}
 }
@@ -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() {
@@ -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 {
@@ -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()
@@ -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;

@@ -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()