Fix an issue where @OptionalInject activitiy/fragment classes could n…

…ot be used with non-Hilt ViewModels. RELNOTES=Fix an issue where @OptionalInject activitiy/fragment classes could not be used with non-Hilt ViewModels. PiperOrigin-RevId: 402432165
google · Oct 12, 2021 · 8ef08fa · 8ef08fa
1 parent f200eb9
commit 8ef08fa
Show file tree

Hide file tree

Showing 8 changed files with 728 additions and 32 deletions.
diff --git a/java/dagger/hilt/android/processor/internal/androidentrypoint/ActivityGenerator.java b/java/dagger/hilt/android/processor/internal/androidentrypoint/ActivityGenerator.java
@@ -113,10 +113,19 @@ private MethodSpec init() {
   //       this, super.getDefaultViewModelProviderFactory());
   // }
   private MethodSpec getDefaultViewModelProviderFactory() {
-    return MethodSpec.methodBuilder("getDefaultViewModelProviderFactory")
+    MethodSpec.Builder builder = MethodSpec.methodBuilder("getDefaultViewModelProviderFactory")
         .addAnnotation(Override.class)
         .addModifiers(Modifier.PUBLIC)
-        .returns(AndroidClassNames.VIEW_MODEL_PROVIDER_FACTORY)
+        .returns(AndroidClassNames.VIEW_MODEL_PROVIDER_FACTORY);
+
+    if (metadata.allowsOptionalInjection()) {
+      builder
+          .beginControlFlow("if (!optionalInjectParentUsesHilt(optionalInjectGetParent()))")
+          .addStatement("return super.getDefaultViewModelProviderFactory()")
+          .endControlFlow();
+    }
+
+    return builder
         .addStatement(
             "return $T.getActivityFactory(this, super.getDefaultViewModelProviderFactory())",
             AndroidClassNames.DEFAULT_VIEW_MODEL_FACTORIES)

diff --git a/java/dagger/hilt/android/processor/internal/androidentrypoint/FragmentGenerator.java b/java/dagger/hilt/android/processor/internal/androidentrypoint/FragmentGenerator.java
@@ -259,10 +259,19 @@ private MethodSpec inflatorMethod() {
   //       this, super.getDefaultViewModelProviderFactory());
   // }
   private MethodSpec getDefaultViewModelProviderFactory() {
-    return MethodSpec.methodBuilder("getDefaultViewModelProviderFactory")
+    MethodSpec.Builder builder = MethodSpec.methodBuilder("getDefaultViewModelProviderFactory")
         .addAnnotation(Override.class)
         .addModifiers(Modifier.PUBLIC)
-        .returns(AndroidClassNames.VIEW_MODEL_PROVIDER_FACTORY)
+        .returns(AndroidClassNames.VIEW_MODEL_PROVIDER_FACTORY);
+
+    if (metadata.allowsOptionalInjection()) {
+      builder
+          .beginControlFlow("if (!optionalInjectParentUsesHilt(optionalInjectGetParent()))")
+          .addStatement("return super.getDefaultViewModelProviderFactory()")
+          .endControlFlow();
+    }
+
+    return builder
         .addStatement(
             "return $T.getFragmentFactory(this, super.getDefaultViewModelProviderFactory())",
             AndroidClassNames.DEFAULT_VIEW_MODEL_FACTORIES)

diff --git a/java/dagger/hilt/android/processor/internal/androidentrypoint/Generators.java b/java/dagger/hilt/android/processor/internal/androidentrypoint/Generators.java
@@ -35,6 +35,7 @@
 import com.squareup.javapoet.TypeName;
 import com.squareup.javapoet.TypeSpec;
 import dagger.hilt.android.processor.internal.AndroidClassNames;
+import dagger.hilt.android.processor.internal.androidentrypoint.AndroidEntryPointMetadata.AndroidType;
 import dagger.hilt.processor.internal.ClassNames;
 import dagger.hilt.processor.internal.Processors;
 import java.util.List;
@@ -216,7 +217,7 @@ static void addInjectionMethods(AndroidEntryPointMetadata metadata, TypeSpec.Bui
         addComponentManagerMethods(metadata, builder);
         // fall through
       case BROADCAST_RECEIVER:
-        addInjectMethod(metadata, builder);
+        addInjectAndMaybeOptionalInjectMethod(metadata, builder);
         break;
       default:
         throw new AssertionError();
@@ -323,30 +324,52 @@ private static FieldSpec componentManagerLockField() {
   //     injected = true;
   //   }
   // }
-  private static void addInjectMethod(
+  private static void addInjectAndMaybeOptionalInjectMethod(
       AndroidEntryPointMetadata metadata, TypeSpec.Builder typeSpecBuilder) {
     MethodSpec.Builder methodSpecBuilder = MethodSpec.methodBuilder("inject")
         .addModifiers(Modifier.PROTECTED);
 
     // Check if the parent is a Hilt type. If it isn't or if it is but it
     // wasn't injected by hilt, then return.
     // Object parent = ...depends on type...
-    // if (!(parent instanceof GeneratedComponentManager)
-    //     || ((parent instanceof InjectedByHilt) &&
-    //         !((InjectedByHilt) parent).wasInjectedByHilt())) {
+    // if (!optionalInjectParentUsesHilt()) {
     //   return;
     //
     if (metadata.allowsOptionalInjection()) {
+      CodeBlock parentCodeBlock;
+      if (metadata.androidType() != AndroidType.BROADCAST_RECEIVER) {
+        parentCodeBlock = CodeBlock.of("optionalInjectGetParent()");
+
+        // Also, add the optionalInjectGetParent method we just used. This is a separate method so
+        // other parts of the code when dealing with @OptionalInject. BroadcastReceiver can't have
+        // this method since the context is only accessible as a parameter to receive()/inject().
+        typeSpecBuilder.addMethod(MethodSpec.methodBuilder("optionalInjectGetParent")
+            .addModifiers(Modifier.PRIVATE)
+            .returns(TypeName.OBJECT)
+            .addStatement("return $L", getParentCodeBlock(metadata))
+            .build());
+      } else {
+        // For BroadcastReceiver, use the "context" field that is on the stack.
+        parentCodeBlock = CodeBlock.of(
+            "$T.getApplication(context.getApplicationContext())", ClassNames.CONTEXTS);
+      }
+
       methodSpecBuilder
-          .addStatement("$T parent = $L", ClassNames.OBJECT, getParentCodeBlock(metadata))
-          .beginControlFlow(
-              "if (!(parent instanceof $T) "
-              + "|| ((parent instanceof $T) && !(($T) parent).wasInjectedByHilt()))",
+          .beginControlFlow("if (!optionalInjectParentUsesHilt($L))", parentCodeBlock)
+          .addStatement("return")
+          .endControlFlow();
+
+      // Add the optionalInjectParentUsesHilt used above.
+      typeSpecBuilder.addMethod(MethodSpec.methodBuilder("optionalInjectParentUsesHilt")
+          .addModifiers(Modifier.PRIVATE)
+          .addParameter(TypeName.OBJECT, "parent")
+          .returns(TypeName.BOOLEAN)
+          .addStatement("return (parent instanceof $T) "
+              + "&& (!(parent instanceof $T) || (($T) parent).wasInjectedByHilt())",
               ClassNames.GENERATED_COMPONENT_MANAGER,
               AndroidClassNames.INJECTED_BY_HILT,
               AndroidClassNames.INJECTED_BY_HILT)
-          .addStatement("return")
-          .endControlFlow();
+          .build());
     }
 
     // Only add @Override if an ancestor extends a generated Hilt class.
@@ -430,10 +453,9 @@ private static CodeBlock getParentCodeBlock(AndroidEntryPointMetadata metadata)
         return CodeBlock.of(
             "$L.maybeGetParentComponentManager()", componentManagerCallBlock(metadata));
       case BROADCAST_RECEIVER:
-        // Broadcast receivers receive a "context" parameter
-        return CodeBlock.of(
-            "$T.getApplication(context.getApplicationContext())",
-            ClassNames.CONTEXTS);
+        // Broadcast receivers receive a "context" parameter that make it so this code block
+        // isn't really usable anywhere
+        throw new AssertionError("BroadcastReceiver types should not get here");
       default:
         throw new AssertionError();
     }

diff --git a/javatests/dagger/hilt/android/AndroidManifest.xml b/javatests/dagger/hilt/android/AndroidManifest.xml
@@ -1,47 +1,77 @@
 <manifest xmlns:android="http://schemas.android.com/apk/res/android"
+    xmlns:tools="http://schemas.android.com/tools"
     package="dagger.hilt.android">
 
   <uses-sdk android:minSdkVersion="14" />
 
   <application>
     <activity
         android:name=".ActivityInjectedSavedStateViewModelTest$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".ActivityInjectedSavedStateViewModelTest$TestActivityWithSuperActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".ActivityInjectedViewModelTest$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".ActivityRetainedClearedListenerTest$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".ActivityScenarioRuleTest$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".DefaultViewModelFactoryTest$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
+    <activity
+        android:name=".OptionalInjectTestClasses$TestActivity"
+        android:exported="false"
+        tools:ignore="MissingClass"/>
+    <activity
+        android:name=".OptionalInjectTestClasses$NonOptionalSubclassActivity"
+        android:exported="false"
+        tools:ignore="MissingClass"/>
+    <activity
+        android:name=".OptionalInjectTestClasses$OptionalSubclassActivity"
+        android:exported="false"
+        tools:ignore="MissingClass"/>
+    <activity
+        android:name=".OptionalInjectTestClasses$TestActivity"
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".QualifierInKotlinFieldsTest$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".UsesSharedComponent1Test$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".UsesSharedComponent2Test$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".UsesSharedComponentEnclosedTest$EnclosedTest$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".ViewModelScopedTest$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name=".ViewModelWithBaseTest$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
     <activity
         android:name="dagger.hilt.android.testsubpackage.UsesSharedComponent1Test$TestActivity"
-        android:exported="false"/>
+        android:exported="false"
+        tools:ignore="MissingClass"/>
   </application>
 </manifest>
diff --git a/javatests/dagger/hilt/android/BUILD b/javatests/dagger/hilt/android/BUILD
@@ -387,6 +387,75 @@ kt_android_library(
     ],
 )
 
+android_local_test(
+    name = "OptionalInjectWithHiltTest",
+    size = "small",
+    srcs = [
+        "OptionalInjectWithHiltTest.java",
+    ],
+    manifest = "AndroidManifest.xml",
+    manifest_values = {
+        "minSdkVersion": "14",
+    },
+    deps = [
+        ":OptionalInjectTestClasses",
+        "//:android_local_test_exports",
+        "//java/dagger/hilt/android:android_entry_point",
+        "//java/dagger/hilt/android:package_info",
+        "//java/dagger/hilt/android/migration:optional_inject",
+        "//java/dagger/hilt/android/testing:hilt_android_test",
+        "@google_bazel_common//third_party/java/truth",
+        "@maven//:androidx_activity_activity",
+        "@maven//:androidx_fragment_fragment",
+        "@maven//:androidx_lifecycle_lifecycle_common",
+        "@maven//:androidx_lifecycle_lifecycle_viewmodel",
+        "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate",
+    ],
+)
+
+android_local_test(
+    name = "OptionalInjectWithoutHiltTest",
+    size = "small",
+    srcs = [
+        "OptionalInjectWithoutHiltTest.java",
+    ],
+    manifest = "AndroidManifest.xml",
+    manifest_values = {
+        "minSdkVersion": "14",
+    },
+    deps = [
+        ":OptionalInjectTestClasses",
+        "//:android_local_test_exports",
+        "//java/dagger/hilt/android:package_info",
+        "//java/dagger/hilt/android/migration:optional_inject",
+        "@google_bazel_common//third_party/java/truth",
+        "@maven//:androidx_activity_activity",
+        "@maven//:androidx_fragment_fragment",
+        "@maven//:androidx_lifecycle_lifecycle_common",
+        "@maven//:androidx_lifecycle_lifecycle_viewmodel",
+        "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate",
+    ],
+)
+
+android_library(
+    name = "OptionalInjectTestClasses",
+    srcs = ["OptionalInjectTestClasses.java"],
+    manifest = "AndroidManifest.xml",
+    deps = [
+        "//:dagger_with_compiler",
+        "//java/dagger/hilt:install_in",
+        "//java/dagger/hilt/android:android_entry_point",
+        "//java/dagger/hilt/android:package_info",
+        "//java/dagger/hilt/android/migration:optional_inject",
+        "@google_bazel_common//third_party/java/jsr330_inject",
+        "@maven//:androidx_activity_activity",
+        "@maven//:androidx_fragment_fragment",
+        "@maven//:androidx_lifecycle_lifecycle_common",
+        "@maven//:androidx_lifecycle_lifecycle_viewmodel",
+        "@maven//:androidx_lifecycle_lifecycle_viewmodel_savedstate",
+    ],
+)
+
 android_local_test(
     name = "ActivityRetainedClearedListenerTest",
     srcs = ["ActivityRetainedClearedListenerTest.java"],