Flip the runtime flag default and remove the compiler option for the …

…Fragment.getContext() fix since the compiler option has to be set on every target that compiles a Hilt fragment which can be difficult to set in practice.

RELNOTES=Flip the runtime flag default and remove the compiler option for the Fragment.getContext() fix. Also, remember to update the dagger.dev site before releasing this.
PiperOrigin-RevId: 404636177

java/dagger/hilt/android/flags/FragmentGetContextFix.java

@@ -36,15 +36,11 @@
  * regular, non-Hilt fragment and can help catch issues where a removed or leaked fragment is
  * incorrectly used.
  *
- * <p>This flag is paired with the compiler option flag
- * dagger.hilt.android.useFragmentGetContextFix. When that flag is false, this runtime flag has no
- * effect on behavior (e.g. the compiler flag being off takes precedence). When the compiler flag is
- * on, then the runtime flag may be used to disable the behavior at runtime.
- *
  * <p>In order to set the flag, bind a boolean value qualified with
  * {@link DisableFragmentGetContextFix} into a set in the {@code SingletonComponent}. A set is used
  * instead of an optional binding to avoid a dependency on Guava. Only one value may be bound into
- * the set within a given app. Example for binding the value:
+ * the set within a given app. If this is not set, the default is to not use the fix. Example for
+ * binding the value:
  *
  * <pre><code>
  * {@literal @}Module
@@ -76,7 +72,7 @@ public static boolean isFragmentGetContextFixDisabled(Context context) {
         "Cannot bind the flag @DisableFragmentGetContextFix more than once.");
 
     if (flagSet.isEmpty()) {
-      return false;
+      return true;
     } else {
       return flagSet.iterator().next();
     }

java/dagger/hilt/android/processor/internal/androidentrypoint/FragmentGenerator.java

@@ -16,8 +16,6 @@
 
 package dagger.hilt.android.processor.internal.androidentrypoint;
 
-import static dagger.hilt.processor.internal.HiltCompilerOptions.useFragmentGetContextFix;
-
 import com.squareup.javapoet.AnnotationSpec;
 import com.squareup.javapoet.ClassName;
 import com.squareup.javapoet.FieldSpec;
@@ -78,11 +76,8 @@ TypeSpec createTypeSpec() {
             .addMethod(onAttachActivityMethod())
             .addMethod(initializeComponentContextMethod())
             .addMethod(getContextMethod())
-            .addMethod(inflatorMethod());
-
-    if (useFragmentGetContextFix(env)) {
-      builder.addField(DISABLE_GET_CONTEXT_FIX_FIELD);
-    }
+            .addMethod(inflatorMethod())
+            .addField(DISABLE_GET_CONTEXT_FIX_FIELD);
 
     Generators.addGeneratedBaseClassJavadoc(builder, AndroidClassNames.ANDROID_ENTRY_POINT);
     Processors.addGeneratedAnnotation(builder, env, getClass());
@@ -184,11 +179,25 @@ private MethodSpec initializeComponentContextMethod() {
             "$N = $T.createContextWrapper(super.getContext(), this)",
             COMPONENT_CONTEXT_FIELD,
             metadata.componentManager());
+    if (metadata.allowsOptionalInjection()) {
+      // When optionally injected, since the runtime flag is only available in Hilt, we need to
+      // check that the parent uses Hilt first.
+      builder.beginControlFlow("if (optionalInjectParentUsesHilt(optionalInjectGetParent()))");
+    }
+
+    builder
+        .addStatement("$N = $T.isFragmentGetContextFixDisabled(super.getContext())",
+            DISABLE_GET_CONTEXT_FIX_FIELD,
+            AndroidClassNames.FRAGMENT_GET_CONTEXT_FIX);
 
-    if (useFragmentGetContextFix(env)) {
-      builder.addStatement("$N = $T.isFragmentGetContextFixDisabled(super.getContext())",
-          DISABLE_GET_CONTEXT_FIX_FIELD,
-          AndroidClassNames.FRAGMENT_GET_CONTEXT_FIX);
+    if (metadata.allowsOptionalInjection()) {
+      // If not attached to a Hilt parent, just disable the fix for now since this is the current
+      // default. There's not a good way to flip this at runtime without Hilt, so after we flip
+      // the default we may just have to flip this and hope that the Hilt usage is already enough
+      // coverage as this should be a fairly rare case.
+      builder.nextControlFlow("else")
+          .addStatement("$N = true", DISABLE_GET_CONTEXT_FIX_FIELD)
+          .endControlFlow();
     }
 
     return builder
@@ -205,26 +214,15 @@ private MethodSpec initializeComponentContextMethod() {
   //   return componentContext;
   // }
   private MethodSpec getContextMethod() {
-    MethodSpec.Builder builder = MethodSpec.methodBuilder("getContext")
+    return MethodSpec.methodBuilder("getContext")
         .returns(AndroidClassNames.CONTEXT)
         .addAnnotation(Override.class)
-        .addModifiers(Modifier.PUBLIC);
-
-    if (useFragmentGetContextFix(env)) {
-      builder
-          // Note that disableGetContext can only be true if componentContext is set, so if it is
-          // true we don't need to check whether componentContext is set or not.
-          .beginControlFlow(
-              "if (super.getContext() == null && !$N)",
-              DISABLE_GET_CONTEXT_FIX_FIELD);
-    } else {
-      builder
-          .beginControlFlow(
-              "if (super.getContext() == null && $N == null)",
-              COMPONENT_CONTEXT_FIELD);
-    }
-
-    return builder
+        .addModifiers(Modifier.PUBLIC)
+        // Note that disableGetContext can only be true if componentContext is set, so if it is
+        // true we don't need to check whether componentContext is set or not.
+        .beginControlFlow(
+            "if (super.getContext() == null && !$N)",
+            DISABLE_GET_CONTEXT_FIX_FIELD)
         .addStatement("return null")
         .endControlFlow()
         .addStatement("initializeComponentContext()")

java/dagger/hilt/processor/internal/BaseProcessor.java

@@ -159,6 +159,7 @@ public synchronized void init(ProcessingEnvironment processingEnvironment) {
     this.elements = processingEnv.getElementUtils();
     this.types = processingEnv.getTypeUtils();
     this.errorHandler = new ProcessorErrorHandler(processingEnvironment);
+    HiltCompilerOptions.checkWrongAndDeprecatedOptions(processingEnvironment);
   }
 
   @Override

java/dagger/hilt/processor/internal/HiltCompilerOptions.java

@@ -23,6 +23,7 @@
 import java.util.stream.Collectors;
 import javax.annotation.processing.ProcessingEnvironment;
 import javax.lang.model.element.TypeElement;
+import javax.tools.Diagnostic.Kind;
 
 /** Hilt annotation processor options. */
 // TODO(danysantiago): Consider consolidating with Dagger compiler options logic.
@@ -85,16 +86,6 @@ public static boolean useAggregatingRootProcessor(ProcessingEnvironment env) {
     return BooleanOption.USE_AGGREGATING_ROOT_PROCESSOR.get(env);
   }
 
-  /**
-   * Returns {@code true} if fragment code should use the fixed getContext() behavior where it
-   * correctly returns null after a fragment is removed. This fixed behavior matches the behavior
-   * of a regular fragment and can help catch issues where a removed or leaked fragment is
-   * incorrectly used.
-   */
-  public static boolean useFragmentGetContextFix(ProcessingEnvironment env) {
-    return BooleanOption.USE_FRAGMENT_GET_CONTEXT_FIX.get(env);
-  }
-
   /** Processor options which can have true or false values. */
   private enum BooleanOption {
     /** Do not use! This is for internal use only. */
@@ -109,9 +100,7 @@ private enum BooleanOption {
     DISABLE_MODULES_HAVE_INSTALL_IN_CHECK("disableModulesHaveInstallInCheck", false),
 
     SHARE_TEST_COMPONENTS(
-        "shareTestComponents", true),
-
-    USE_FRAGMENT_GET_CONTEXT_FIX("android.useFragmentGetContextFix", false);
+        "shareTestComponents", true);
 
     private final String name;
     private final boolean defaultValue;
@@ -149,6 +138,32 @@ String getQualifiedName() {
     }
   }
 
+  private static final ImmutableSet<String> DEPRECATED_OPTIONS = ImmutableSet.of(
+      "dagger.hilt.android.useFragmentGetContextFix");
+
+  public static void checkWrongAndDeprecatedOptions(ProcessingEnvironment env) {
+    Set<String> knownOptions = getProcessorOptions();
+    for (String option : env.getOptions().keySet()) {
+      if (knownOptions.contains(option)) {
+        continue;
+      }
+
+      if (DEPRECATED_OPTIONS.contains(option)) {
+        env.getMessager().printMessage(
+            Kind.ERROR,
+            "The compiler option " + option + " is deprecated and no longer does anything. "
+            + "Please do not set this option.");
+        continue;
+      }
+
+      if (option.startsWith("dagger.hilt.")) {
+        env.getMessager().printMessage(
+            Kind.ERROR,
+            "The compiler option " + option + " is not a recognized Hilt option. Is there a typo?");
+      }
+    }
+  }
+
   public static Set<String> getProcessorOptions() {
     return Arrays.stream(BooleanOption.values())
         .map(BooleanOption::getQualifiedName)

javatests/dagger/hilt/android/BUILD

@@ -189,9 +189,6 @@ android_local_test(
     name = "FragmentContextOnAttachTest",
     size = "small",
     srcs = ["FragmentContextOnAttachTest.java"],
-    javacopts = [
-        "-Adagger.hilt.android.useFragmentGetContextFix=true",
-    ],
     manifest_values = {
         "minSdkVersion": "14",
     },