Android Extensions: Fixes after review

plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/codegen/AndroidContainerType.kt

@@ -37,7 +37,7 @@ enum class AndroidContainerType(className: String, val doesSupportCache: Boolean
         private val LAYOUT_CONTAINER_FQNAME = LayoutContainer::class.java.canonicalName
 
         fun get(descriptor: ClassifierDescriptor): AndroidContainerType {
-            fun getClassTypeInternal(name: String): AndroidContainerType? = when (name) {
+            fun getContainerTypeInternal(name: String): AndroidContainerType? = when (name) {
                 AndroidConst.ACTIVITY_FQNAME -> AndroidContainerType.ACTIVITY
                 AndroidConst.FRAGMENT_FQNAME -> AndroidContainerType.FRAGMENT
                 AndroidConst.DIALOG_FQNAME -> AndroidContainerType.DIALOG
@@ -48,13 +48,13 @@ enum class AndroidContainerType(className: String, val doesSupportCache: Boolean
                 else -> null
             }
 
-            getClassTypeInternal(DescriptorUtils.getFqName(descriptor).asString())?.let { return it }
+            getContainerTypeInternal(DescriptorUtils.getFqName(descriptor).asString())?.let { return it }
 
             for (supertype in descriptor.typeConstructor.supertypes) {
                 val declarationDescriptor = supertype.constructor.declarationDescriptor
                 if (declarationDescriptor != null) {
-                    val androidClassType = get(declarationDescriptor)
+                    val containerType = get(declarationDescriptor)
-                    if (androidClassType != AndroidContainerType.UNKNOWN) return androidClassType
+                    if (containerType != AndroidContainerType.UNKNOWN) return containerType
                 }
             }
 

plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/codegen/AndroidExpressionCodegenExtension.kt

@@ -50,7 +50,7 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
         val ON_DESTROY_METHOD_NAME = "onDestroyView"
 
         fun shouldCacheResource(resource: PropertyDescriptor): Boolean {
-            return (resource as? AndroidSyntheticProperty)?.shouldBeCached ?: false
+            return (resource as? AndroidSyntheticProperty)?.shouldBeCached == true
         }
     }
 
@@ -87,13 +87,13 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
             container: ClassDescriptor,
             c: ExpressionCodegenExtension.Context
     ): StackValue? {
-        val containerOptions = ContainerOptionsProxy.get(container)
+        val containerOptions = ContainerOptionsProxy.create(container)
 
         if (!containerOptions.cache.hasCache) {
             return StackValue.functionCall(Type.VOID_TYPE) {}
         }
 
-        if (containerOptions.classType == AndroidContainerType.UNKNOWN) return null
+        if (containerOptions.containerType == AndroidContainerType.UNKNOWN) return null
 
         return StackValue.functionCall(Type.VOID_TYPE) {
             val bytecodeClassName = c.typeMapper.mapType(container).internalName
@@ -114,7 +114,7 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
         val androidPackage = packageFragment.packageData.moduleData.module.applicationPackage
         val container = resolvedCall.getReceiverDeclarationDescriptor() as? ClassDescriptor ?: return null
 
-        val containerOptions = ContainerOptionsProxy.get(container)
+        val containerOptions = ContainerOptionsProxy.create(container)
         return ResourcePropertyStackValue(receiver, c.typeMapper, resource, container, containerOptions, androidPackage)
     }
 
@@ -129,7 +129,7 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
         val container = codegen.descriptor
         if (container.kind != ClassKind.CLASS || container.isInner || DescriptorUtils.isLocal(container)) return
 
-        val containerOptions = ContainerOptionsProxy.get(container)
+        val containerOptions = ContainerOptionsProxy.create(container)
         if (containerOptions.cache == NO_CACHE) return
 
         val context = SyntheticPartsGenerateContext(classBuilder, codegen.state, container, targetClass, containerOptions)
@@ -137,7 +137,7 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
         context.generateClearCacheFunction()
         context.generateCacheField()
 
-        if (containerOptions.classType.isFragment) {
+        if (containerOptions.containerType.isFragment) {
             val classMembers = container.unsubstitutedMemberScope.getContributedDescriptors()
             val onDestroy = classMembers.firstOrNull { it is FunctionDescriptor && it.isOnDestroyFunction() }
             if (onDestroy == null) {
@@ -161,9 +161,9 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
         methodVisitor.visitCode()
         val iv = InstructionAdapter(methodVisitor)
 
-        val classType = state.typeMapper.mapClass(container)
+        val containerType = state.typeMapper.mapClass(container)
 
-        iv.load(0, classType)
+        iv.load(0, containerType)
         iv.invokespecial(state.typeMapper.mapClass(container.getSuperClassOrAny()).internalName, ON_DESTROY_METHOD_NAME, "()V", false)
         iv.areturn(Type.VOID_TYPE)
 
@@ -196,7 +196,7 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
     }
 
     private fun SyntheticPartsGenerateContext.generateCachedFindViewByIdFunction() {
-        val containerType = state.typeMapper.mapClass(container)
+        val containerAsmType = state.typeMapper.mapClass(container)
 
         val viewType = Type.getObjectType("android/view/View")
 
@@ -205,7 +205,7 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
         methodVisitor.visitCode()
         val iv = InstructionAdapter(methodVisitor)
 
-        val cacheImpl = CacheMechanism.get(containerOptions, iv, containerType)
+        val cacheImpl = CacheMechanism.get(containerOptions, iv, containerAsmType)
 
         fun loadId() = iv.load(1, Type.INT_TYPE)
 
@@ -231,23 +231,21 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
         iv.ifnonnull(lViewNonNull)
 
         // Resolve View via findViewById if not in cache
-        iv.load(0, containerType)
+        iv.load(0, containerAsmType)
-        when (containerOptions.classType) {
+
+        val containerType = containerOptions.containerType
+        when (containerType) {
             AndroidContainerType.ACTIVITY, AndroidContainerType.SUPPORT_FRAGMENT_ACTIVITY, AndroidContainerType.VIEW, AndroidContainerType.DIALOG -> {
                 loadId()
-                iv.invokevirtual(containerOptions.classType.internalClassName, "findViewById", "(I)Landroid/view/View;", false)
+                iv.invokevirtual(containerType.internalClassName, "findViewById", "(I)Landroid/view/View;", false)
             }
             AndroidContainerType.FRAGMENT, AndroidContainerType.SUPPORT_FRAGMENT, AndroidContainerType.LAYOUT_CONTAINER -> {
-                val methodName: String
+                if (containerType == AndroidContainerType.LAYOUT_CONTAINER) {
-                val targetClassName: String
+                    iv.invokeinterface(containerType.internalClassName, "getContainerView", "()Landroid/view/View;")
-                if (containerOptions.classType == AndroidContainerType.LAYOUT_CONTAINER) {
-                    methodName = "getEntityView"
-                    targetClassName = containerType.internalName
                 } else {
-                    methodName = "getView"
+                    iv.invokevirtual(containerType.internalClassName, "getView", "()Landroid/view/View;", false)
-                    targetClassName = containerOptions.classType.internalClassName
                 }
-                iv.invokevirtual(targetClassName, methodName, "()Landroid/view/View;", false)
+
                 iv.dup()
                 val lgetViewNotNull = Label()
                 iv.ifnonnull(lgetViewNotNull)
@@ -262,7 +260,7 @@ class AndroidExpressionCodegenExtension : ExpressionCodegenExtension {
                 loadId()
                 iv.invokevirtual("android/view/View", "findViewById", "(I)Landroid/view/View;", false)
             }
-            else -> throw IllegalStateException("Can't generate code for ${containerOptions.classType}")
+            else -> throw IllegalStateException("Can't generate code for $containerType")
         }
         iv.store(2, viewType)
 

plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/codegen/AndroidOnDestroyClassBuilderInterceptorExtension.kt

@@ -17,7 +17,6 @@
 package org.jetbrains.kotlin.android.synthetic.codegen
 
 import com.intellij.psi.PsiElement
-import kotlinx.android.extensions.CacheImplementation.NO_CACHE
 import org.jetbrains.kotlin.android.synthetic.descriptors.ContainerOptionsProxy
 import org.jetbrains.kotlin.codegen.ClassBuilder
 import org.jetbrains.kotlin.codegen.ClassBuilderFactory
@@ -111,14 +110,14 @@ class AndroidOnDestroyClassBuilderInterceptorExtension : ClassBuilderInterceptor
                     if (Type.getArgumentTypes(desc).isNotEmpty()) return
                     if (Type.getReturnType(desc) != Type.VOID_TYPE) return
 
-                    val classType = currentClassName?.let { Type.getObjectType(it) } ?: return
+                    val containerType = currentClassName?.let { Type.getObjectType(it) } ?: return
 
                     val container = bindingContext.get(BindingContext.CLASS, currentClass) ?: return
-                    val containerOptions = ContainerOptionsProxy.get(container)
+                    val entityOptions = ContainerOptionsProxy.create(container)
-                    if (!containerOptions.classType.isFragment || containerOptions.cache == NO_CACHE) return
+                    if (!entityOptions.containerType.isFragment || !entityOptions.cache.hasCache) return
 
                     val iv = InstructionAdapter(this)
-                    iv.load(0, classType)
+                    iv.load(0, containerType)
                     iv.invokevirtual(currentClassName, AndroidExpressionCodegenExtension.CLEAR_CACHE_METHOD_NAME, "()V", false)
                 }
             }

plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/codegen/ResourcePropertyStackValue.kt

@@ -30,16 +30,16 @@ import org.jetbrains.org.objectweb.asm.commons.InstructionAdapter
 
 class ResourcePropertyStackValue(
         val receiver: StackValue,
-        val typeMapper: KotlinTypeMapper,
+        private val typeMapper: KotlinTypeMapper,
         val resource: PropertyDescriptor,
         val container: ClassDescriptor,
-        val containerOptions: ContainerOptionsProxy,
+        private val containerOptions: ContainerOptionsProxy,
-        val androidPackage: String
+        private val androidPackage: String
 ) : StackValue(typeMapper.mapType(resource.returnType!!)) {
-    private val androidClassType get() = containerOptions.classType
+    private val containerType get() = containerOptions.containerType
 
     init {
-        assert(containerOptions.classType != AndroidContainerType.UNKNOWN)
+        assert(containerOptions.containerType != AndroidContainerType.UNKNOWN)
     }
 
     override fun putSelector(type: Type, v: InstructionAdapter) {
@@ -61,25 +61,25 @@ class ResourcePropertyStackValue(
             v.invokevirtual(declarationDescriptorType.internalName, AndroidExpressionCodegenExtension.CACHED_FIND_VIEW_BY_ID_METHOD_NAME, "(I)Landroid/view/View;", false)
         }
         else {
-            when (androidClassType) {
+            when (containerType) {
                 AndroidContainerType.ACTIVITY, AndroidContainerType.SUPPORT_FRAGMENT_ACTIVITY, AndroidContainerType.VIEW, AndroidContainerType.DIALOG -> {
-                    receiver.put(Type.getType("L${androidClassType.internalClassName};"), v)
+                    receiver.put(Type.getType("L${containerType.internalClassName};"), v)
                     getResourceId(v)
-                    v.invokevirtual(androidClassType.internalClassName, "findViewById", "(I)Landroid/view/View;", false)
+                    v.invokevirtual(containerType.internalClassName, "findViewById", "(I)Landroid/view/View;", false)
                 }
                 AndroidContainerType.FRAGMENT, AndroidContainerType.SUPPORT_FRAGMENT -> {
-                    receiver.put(Type.getType("L${androidClassType.internalClassName};"), v)
+                    receiver.put(Type.getType("L${containerType.internalClassName};"), v)
-                    v.invokevirtual(androidClassType.internalClassName, "getView", "()Landroid/view/View;", false)
+                    v.invokevirtual(containerType.internalClassName, "getView", "()Landroid/view/View;", false)
                     getResourceId(v)
                     v.invokevirtual("android/view/View", "findViewById", "(I)Landroid/view/View;", false)
                 }
                 AndroidContainerType.LAYOUT_CONTAINER -> {
-                    receiver.put(Type.getType("L${androidClassType.internalClassName};"), v)
+                    receiver.put(Type.getType("L${containerType.internalClassName};"), v)
-                    v.invokevirtual(androidClassType.internalClassName, "getEntityView", "()Landroid/view/View;", false)
+                    v.invokevirtual(containerType.internalClassName, "getEntityView", "()Landroid/view/View;", false)
                     getResourceId(v)
                     v.invokevirtual("android/view/View", "findViewById", "(I)Landroid/view/View;", false)
                 }
-                else -> throw IllegalStateException("Invalid Android class type: $androidClassType") // Should never occur
+                else -> throw IllegalStateException("Invalid Android class type: $containerType") // Should never occur
             }
         }
 
@@ -87,31 +87,31 @@ class ResourcePropertyStackValue(
     }
 
     private fun putSelectorForFragment(v: InstructionAdapter) {
-        receiver.put(Type.getType("L${androidClassType.internalClassName};"), v)
+        receiver.put(Type.getType("L${containerType.internalClassName};"), v)
 
-        when (androidClassType) {
+        when (containerType) {
             AndroidContainerType.ACTIVITY, AndroidContainerType.FRAGMENT -> {
-                v.invokevirtual(androidClassType.internalClassName, "getFragmentManager", "()Landroid/app/FragmentManager;", false)
+                v.invokevirtual(containerType.internalClassName, "getFragmentManager", "()Landroid/app/FragmentManager;", false)
                 getResourceId(v)
                 v.invokevirtual("android/app/FragmentManager", "findFragmentById", "(I)Landroid/app/Fragment;", false)
             }
             AndroidContainerType.SUPPORT_FRAGMENT -> {
-                v.invokevirtual(androidClassType.internalClassName, "getFragmentManager", "()Landroid/support/v4/app/FragmentManager;", false)
+                v.invokevirtual(containerType.internalClassName, "getFragmentManager", "()Landroid/support/v4/app/FragmentManager;", false)
                 getResourceId(v)
                 v.invokevirtual("android/support/v4/app/FragmentManager", "findFragmentById", "(I)Landroid/support/v4/app/Fragment;", false)
             }
             AndroidContainerType.SUPPORT_FRAGMENT_ACTIVITY -> {
-                v.invokevirtual(androidClassType.internalClassName, "getSupportFragmentManager", "()Landroid/support/v4/app/FragmentManager;", false)
+                v.invokevirtual(containerType.internalClassName, "getSupportFragmentManager", "()Landroid/support/v4/app/FragmentManager;", false)
                 getResourceId(v)
                 v.invokevirtual("android/support/v4/app/FragmentManager", "findFragmentById", "(I)Landroid/support/v4/app/Fragment;", false)
             }
-            else -> throw IllegalStateException("Invalid Android class type: $androidClassType") // Should never occur
+            else -> throw IllegalStateException("Invalid Android class type: $containerType") // Should never occur
         }
 
         v.checkcast(this.type)
     }
 
-    fun getResourceId(v: InstructionAdapter) {
+    private fun getResourceId(v: InstructionAdapter) {
         v.getstatic(androidPackage.replace(".", "/") + "/R\$id", resource.name.asString(), "I")
     }
 }

plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/descriptors/ContainerOptionsProxy.kt

@@ -28,33 +28,33 @@ import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.resolve.constants.EnumValue
 import org.jetbrains.kotlin.resolve.source.KotlinSourceElement
 
-class ContainerOptionsProxy(val classType: AndroidContainerType, val cache: CacheImplementation) {
+class ContainerOptionsProxy(val containerType: AndroidContainerType, val cache: CacheImplementation) {
     companion object {
         private val CONTAINER_OPTIONS_FQNAME = FqName(ContainerOptions::class.java.canonicalName)
         private val CACHE_NAME = ContainerOptions::cache.name
 
         private val DEFAULT_CACHE_IMPL = SPARSE_ARRAY
 
-        fun get(container: ClassDescriptor): ContainerOptionsProxy {
+        fun create(container: ClassDescriptor): ContainerOptionsProxy {
             if (container.kind != ClassKind.CLASS) {
                 return ContainerOptionsProxy(AndroidContainerType.UNKNOWN, NO_CACHE)
             }
 
-            val classType = AndroidContainerType.get(container)
+            val containerType = AndroidContainerType.get(container)
 
             val anno = container.annotations.findAnnotation(CONTAINER_OPTIONS_FQNAME)
 
             if (anno == null) {
                 // Java classes (and Kotlin classes from other modules) does not support cache by default
-                val supportsCache = container.source is KotlinSourceElement && classType.doesSupportCache
+                val supportsCache = container.source is KotlinSourceElement && containerType.doesSupportCache
                 return ContainerOptionsProxy(
-                        classType,
+                        containerType,
                         if (supportsCache) DEFAULT_CACHE_IMPL else NO_CACHE)
             }
 
             val cache = anno.getEnumValue(CACHE_NAME, DEFAULT_CACHE_IMPL) { valueOf(it) }
 
-            return ContainerOptionsProxy(classType, cache)
+            return ContainerOptionsProxy(containerType, cache)
         }
     }
 }
@@ -68,7 +68,8 @@ private fun <E: Enum<E>> AnnotationDescriptor.getEnumValue(name: String, default
 
     return try {
         factory(valueName)
-    } catch (e: Exception) {
+    } catch (e: IllegalArgumentException) {
+        // Enum.valueOf() may throw this
         defaultValue
     }
 }

plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/res/AndroidPackageFragmentProviderExtension.kt

@@ -79,7 +79,7 @@ abstract class AndroidPackageFragmentProviderExtension : PackageFragmentProvider
         // Package with clearFindViewByIdCache()
         AndroidConst.SYNTHETIC_SUBPACKAGES.last().let { s ->
             val packageDescriptor = PredefinedPackageFragmentDescriptor(s, module, storageManager, packagesToLookupInCompletion) { descriptor ->
-                (lazyContext().getWidgetReceivers(false) + lazyContext().getWidgetReceivers(true))
+                (lazyContext().getWidgetReceivers(forView = false) + lazyContext().getWidgetReceivers(forView = true))
                         .filter { it.mayHaveCache }
                         .map { genClearCacheFunction(descriptor, it.type) }
             }

plugins/android-extensions/android-extensions-compiler/src/org/jetbrains/kotlin/android/synthetic/res/syntheticDescriptorGeneration.kt

@@ -145,10 +145,7 @@ private fun genProperty(
 private val SimpleType.shouldBeCached: Boolean
     get() {
         val viewClassFqName = constructor.declarationDescriptor?.fqNameUnsafe?.asString() ?: return false
-        return when (viewClassFqName) {
+        return viewClassFqName != AndroidConst.VIEWSTUB_FQNAME
-            AndroidConst.VIEWSTUB_FQNAME -> false
-            else -> true
-        }
     }
 
 interface AndroidSyntheticFunction

plugins/plugins-tests/tests/org/jetbrains/kotlin/android/synthetic/test/AbstractAndroidBytecodeShapeTest.kt

@@ -30,7 +30,7 @@ abstract class AbstractAndroidBytecodeShapeTest : AbstractBytecodeTextTest() {
     private fun createEnvironmentForConfiguration(configuration: CompilerConfiguration, path: String) {
         val layoutPaths = getResPaths(path)
         myEnvironment = createTestEnvironment(configuration, layoutPaths)
-        addAERuntimeLibrary(myEnvironment)
+        addAndroidExtensionsRuntimeLibrary(myEnvironment)
     }
 
     override fun doTest(path: String) {

plugins/plugins-tests/tests/org/jetbrains/kotlin/android/synthetic/test/CompilerTestUtils.kt

@@ -51,12 +51,12 @@ fun KtUsefulTestCase.createTestEnvironment(configuration: CompilerConfiguration,
     ClassBuilderInterceptorExtension.registerExtension(project, AndroidOnDestroyClassBuilderInterceptorExtension())
     PackageFragmentProviderExtension.registerExtension(project, CliAndroidPackageFragmentProviderExtension())
 
-    addAERuntimeLibrary(myEnvironment)
+    addAndroidExtensionsRuntimeLibrary(myEnvironment)
 
     return myEnvironment
 }
 
-fun addAERuntimeLibrary(environment: KotlinCoreEnvironment) {
+fun addAndroidExtensionsRuntimeLibrary(environment: KotlinCoreEnvironment) {
     environment.apply {
         val runtimeLibrary = File("out/production/android-extensions-runtime")
         updateClasspath(listOf(JvmClasspathRoot(runtimeLibrary)))