Skip to content

Commit

Permalink
Migrate HiltGradlePlugin to use AGP's new ASM pipeline for bytecode t…
Browse files Browse the repository at this point in the history
…ransformation.

Updates the HiltGradlePlugin to use AGP's new ASM pipeline if the developer is on AGP 4.2.0+, otherwise the plugin will use the older transform APIs. Using the new APIs allows for the plugin to not need a custom task to transform local tests as this is now supported with the new pipeline. If the developer is using AGP 4.2.0 then having 'enableTransformForLocalTests' is no longer needed and a warning will be shown with the hopes of migrating user away and once 4.2 is stable completely removing and deprecating the option. AGP's new transform pipeline is available in AGP 4.2.0 which is currently in beta.

Fixes: #2118
RELNOTES=Use AGP's new transform pipeline when developer is on AGP 4.2.0+.
PiperOrigin-RevId: 348818076
  • Loading branch information
java-team-github-bot authored and Dagger Team committed Dec 23, 2020
1 parent bf18e2f commit d69b00f
Show file tree
Hide file tree
Showing 19 changed files with 417 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,6 @@ android {
}
}

hilt {
enableTransformForLocalTests = true
}

dependencies {
implementation project(':feature')
implementation 'androidx.appcompat:appcompat:1.2.0'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,22 @@ public void verifyBaseReceiverIsNotDoubleInjected() throws InterruptedException
assertThat(receiver.onReceiveCalled).isEqualTo(1);
}

@Test
public void verifyComplexReceiverInjectedValue() throws InterruptedException {
Context context = InstrumentationRegistry.getInstrumentation().getTargetContext();

TestReceiverFour receiver = new TestReceiverFour();
IntentFilter intentFilter = new IntentFilter("test-action");
context.registerReceiver(receiver, intentFilter);

Intent intent = new Intent();
intent.setAction("test-action");
context.sendBroadcast(intent);

receiver.latch.await(2, TimeUnit.SECONDS);
assertThat(receiver.injectedValue).isNotEmpty();
}

/** Test receiver */
@AndroidEntryPoint
static class TestReceiverOne extends BroadcastReceiver {
Expand Down Expand Up @@ -139,6 +155,35 @@ public void onReceive(Context context, Intent intent) {
}
}

/** Complex-ish test receiver */
@AndroidEntryPoint
static class TestReceiverFour extends BroadcastReceiver {

final CountDownLatch latch = new CountDownLatch(1);

@Inject String injectedValue;

@Override
public void onReceive(Context context, Intent intent) {
// Weird code, but it tests that the exception table and stack table frames are correctly
// updated in a transformation.
boolean var0;
if (context != null) {
var0 = false;
Object var1 = context.getClass();
try {
throw new IllegalStateException();
} catch (IllegalStateException ex) {
var0 = true;
}
} else {
BroadcastReceiver myself = this;
var0 = false;
}
latch.countDown();
}
}

/** Base test receiver */
abstract static class BaseReceiverAbstractMethod extends BroadcastReceiver {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
buildscript {
ext {
kotlin_version = '1.3.61'
agp_version = System.getenv('AGP_VERSION') ?: "4.1.1"
agp_version = "4.2.0-beta01"
}
repositories {
google()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ kapt {
correctErrorTypes true
}

hilt {
enableTransformForLocalTests = true
}

dependencies {
// This is api instead of implementation since Kotlin modules here consumed
// by the app need to expose @kotlin.Metadata
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ android {
}
}

hilt {
enableTransformForLocalTests = true
}

dependencies {
implementation fileTree(dir: "libs", include: ["*.jar"])
implementation "org.jetbrains.kotlin:kotlin-stdlib:$kotlin_version"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
buildscript {
ext {
kotlin_version = '1.3.61'
agp_version = System.getenv('AGP_VERSION') ?: "4.1.1"
agp_version = "4.2.0-beta01"
}
repositories {
google()
Expand Down
24 changes: 21 additions & 3 deletions java/dagger/hilt/android/plugin/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ buildscript {
}

plugins {
id 'org.jetbrains.kotlin.jvm' version '1.3.61'
id 'org.jetbrains.kotlin.jvm' version '1.4.20'
id 'java-gradle-plugin'
id 'maven-publish'
}
Expand All @@ -32,15 +32,33 @@ repositories {
jcenter()
}

configurations {
additionalTestPlugin {
canBeConsumed = false
canBeResolved = true
extendsFrom implementation
}
}

dependencies {
implementation gradleApi()
implementation 'com.android.tools.build:gradle:3.6.3'
implementation 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.3.61'
compileOnly 'com.android.tools.build:gradle:4.2.0-beta01'
implementation 'org.jetbrains.kotlin:kotlin-gradle-plugin:1.4.20'
implementation 'org.javassist:javassist:3.26.0-GA'
implementation 'org.ow2.asm:asm:9.0'

testImplementation gradleTestKit()
testImplementation 'junit:junit:4.12'
testImplementation 'com.google.truth:truth:1.0.1'
additionalTestPlugin 'com.android.tools.build:gradle:4.2.0-beta01'
}

// Configure the generating task of plugin-under-test-metadata.properties to
// include additional dependencies for the injected plugin classpath that
// are not present in the main runtime dependencies. This allows us to test
// the desired AGP version while keeping a compileOnly dep on the main source.
tasks.withType(PluginUnderTestMetadata.class).named("pluginUnderTestMetadata").configure {
it.pluginClasspath.from(configurations.additionalTestPlugin)
}

// Create sources Jar from main kotlin sources
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
package dagger.hilt.android.plugin

import com.android.build.api.instrumentation.AsmClassVisitorFactory
import com.android.build.api.instrumentation.ClassContext
import com.android.build.api.instrumentation.ClassData
import com.android.build.api.instrumentation.InstrumentationParameters
import java.io.File
import org.gradle.api.provider.Property
import org.gradle.api.tasks.Input
import org.objectweb.asm.ClassReader
import org.objectweb.asm.ClassVisitor
import org.objectweb.asm.FieldVisitor
import org.objectweb.asm.MethodVisitor
import org.objectweb.asm.Opcodes

/**
* ASM Adapter that transforms @AndroidEntryPoint-annotated classes to extend the Hilt
* generated android class, including the @HiltAndroidApp application class.
*/
@Suppress("UnstableApiUsage")
class AndroidEntryPointClassVisitor(
private val apiVersion: Int,
nextClassVisitor: ClassVisitor,
private val additionalClasses: File
) : ClassVisitor(Opcodes.ASM8, nextClassVisitor) {

interface AndroidEntryPointParams : InstrumentationParameters {
@get:Input
val additionalClassesDir: Property<File>
}

abstract class Factory : AsmClassVisitorFactory<AndroidEntryPointParams> {
override fun createClassVisitor(
classContext: ClassContext,
nextClassVisitor: ClassVisitor
): ClassVisitor {
return AndroidEntryPointClassVisitor(
apiVersion = instrumentationContext.apiVersion.get(),
nextClassVisitor = nextClassVisitor,
additionalClasses = parameters.get().additionalClassesDir.get()
)
}

/**
* Check if a class should be transformed.
*
* Only classes that are an Android entry point should be transformed.
*/
override fun isInstrumentable(classData: ClassData) =
classData.classAnnotations.any { ANDROID_ENTRY_POINT_ANNOTATIONS.contains(it) }
}

// The name of the Hilt generated superclass in it internal form.
// e.g. "my/package/Hilt_MyActivity"
lateinit var newSuperclassName: String

lateinit var oldSuperclassName: String

override fun visit(
version: Int,
access: Int,
name: String,
signature: String?,
superName: String?,
interfaces: Array<out String>?
) {
val packageName = name.substringBeforeLast('/')
val className = name.substringAfterLast('/')
newSuperclassName =
packageName + "/Hilt_" + className.replace("$", "_")
oldSuperclassName = superName ?: error { "Superclass of $name is null!" }
super.visit(version, access, name, signature, newSuperclassName, interfaces)
}

override fun visitMethod(
access: Int,
name: String,
descriptor: String,
signature: String?,
exceptions: Array<out String>?
): MethodVisitor {
val nextMethodVisitor = super.visitMethod(access, name, descriptor, signature, exceptions)
val invokeSpecialVisitor = InvokeSpecialAdapter(apiVersion, nextMethodVisitor)
if (name == ON_RECEIVE_METHOD_NAME &&
descriptor == ON_RECEIVE_METHOD_DESCRIPTOR &&
hasOnReceiveBytecodeInjectionMarker()
) {
return OnReceiveAdapter(apiVersion, invokeSpecialVisitor)
}
return invokeSpecialVisitor
}

/**
* Adapter for super calls (e.g. super.onCreate()) that rewrites the owner reference of the
* invokespecial instruction to use the new superclass.
*
* The invokespecial instruction is emitted for code that between other things also invokes a
* method of a superclass of the current class. The opcode invokespecial takes two operands, each
* of 8 bit, that together represent an address in the constant pool to a method reference. The
* method reference is computed at compile-time by looking the direct superclass declaration, but
* at runtime the code behaves like invokevirtual, where as the actual method invoked is looked up
* based on the class hierarchy.
*
* However, it has been observed that on APIs 19 to 22 the Android Runtime (ART) jumps over the
* direct superclass and into the method reference class, causing unexpected behaviours.
* Therefore, this method performs the additional transformation to rewrite direct super call
* invocations to use a method reference whose class in the pool is the new superclass. Note that
* this is not necessary for constructor calls since the Javassist library takes care of those.
*
* @see: https://docs.oracle.com/javase/specs/jvms/se11/html/jvms-6.html#jvms-6.5.invokespecial
* @see: https://source.android.com/devices/tech/dalvik/dalvik-bytecode
*/
inner class InvokeSpecialAdapter(
apiVersion: Int,
nextClassVisitor: MethodVisitor
) : MethodVisitor(apiVersion, nextClassVisitor) {
override fun visitMethodInsn(
opcode: Int,
owner: String,
name: String,
descriptor: String,
isInterface: Boolean
) {
if (opcode == Opcodes.INVOKESPECIAL && owner == oldSuperclassName) {
// Update the owner of all INVOKESPECIAL instructions, including those found in
// constructors.
super.visitMethodInsn(opcode, newSuperclassName, name, descriptor, isInterface)
} else {
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface)
}
}
}

/**
* Method adapter for a BroadcastReceiver's onReceive method to insert a super call since with
* its new superclass, onReceive will no longer be abstract (it is implemented by Hilt generated
* receiver).
*/
inner class OnReceiveAdapter(
apiVersion: Int,
nextClassVisitor: MethodVisitor
) : MethodVisitor(apiVersion, nextClassVisitor) {
override fun visitCode() {
super.visitCode()
super.visitIntInsn(Opcodes.ALOAD, 0) // Load 'this'
super.visitIntInsn(Opcodes.ALOAD, 1) // Load method param 1 (Context)
super.visitIntInsn(Opcodes.ALOAD, 2) // Load method param 2 (Intent)
super.visitMethodInsn(
Opcodes.INVOKESPECIAL,
newSuperclassName,
ON_RECEIVE_METHOD_NAME,
ON_RECEIVE_METHOD_DESCRIPTOR,
false
)
}
}

/**
* Check if Hilt generated class is a BroadcastReceiver with the marker field which means
* a super.onReceive invocation has to be inserted in the implementation.
*/
private fun hasOnReceiveBytecodeInjectionMarker() =
findAdditionalClassFile(newSuperclassName).inputStream().use {
var hasMarker = false
ClassReader(it).accept(
object : ClassVisitor(apiVersion) {
override fun visitField(
access: Int,
name: String,
descriptor: String,
signature: String?,
value: Any?
): FieldVisitor? {
if (name == "onReceiveBytecodeInjectionMarker") {
hasMarker = true
}
return null
}
},
ClassReader.SKIP_CODE or ClassReader.SKIP_DEBUG or ClassReader.SKIP_FRAMES
)
return@use hasMarker
}

private fun findAdditionalClassFile(className: String) =
File(additionalClasses, "$className.class")

companion object {
val ANDROID_ENTRY_POINT_ANNOTATIONS = setOf(
"dagger.hilt.android.AndroidEntryPoint",
"dagger.hilt.android.HiltAndroidApp"
)
const val ON_RECEIVE_METHOD_NAME = "onReceive"
const val ON_RECEIVE_METHOD_DESCRIPTOR =
"(Landroid/content/Context;Landroid/content/Intent;)V"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ interface HiltExtension {
* If set to `true`, Hilt will register a transform task that will rewrite `@AndroidEntryPoint`
* annotated classes before the host-side JVM tests run. You should enable this option if you are
* running Robolectric UI tests as part of your JUnit tests.
*
* This flag is not necessary if when com.android.tools.build:gradle:4.2.0+ is used and will be
* deprecated in a future version.
*/
var enableTransformForLocalTests: Boolean
}
Expand Down
Loading

0 comments on commit d69b00f

Please sign in to comment.