From e2989f5de9f6de8a282752a013a0386bc8163c0c Mon Sep 17 00:00:00 2001 From: cpovirk Date: Thu, 5 Dec 2024 10:43:19 -0800 Subject: [PATCH] Make Hilt copy type-use nullness annotations, too. RELNOTES=n/a PiperOrigin-RevId: 703169716 --- .../androidentrypoint/Generators.java | 27 ++++- .../processor/internal/GeneratorsTest.java | 99 +++++++++++++++++++ 2 files changed, 121 insertions(+), 5 deletions(-) diff --git a/java/dagger/hilt/android/processor/internal/androidentrypoint/Generators.java b/java/dagger/hilt/android/processor/internal/androidentrypoint/Generators.java index a79a3e90733..6213a4a5744 100644 --- a/java/dagger/hilt/android/processor/internal/androidentrypoint/Generators.java +++ b/java/dagger/hilt/android/processor/internal/androidentrypoint/Generators.java @@ -28,6 +28,7 @@ import androidx.room.compiler.processing.XAnnotation; import androidx.room.compiler.processing.XConstructorElement; import androidx.room.compiler.processing.XElement; +import androidx.room.compiler.processing.XType; import androidx.room.compiler.processing.XTypeElement; import androidx.room.compiler.processing.XVariableElement; import com.google.common.base.Preconditions; @@ -130,14 +131,30 @@ private static Optional getNullableAnnotationSpec(XElement eleme return Optional.empty(); } + /** Returns a TypeName for the given type, including any @Nullable annotations on it. */ + private static TypeName withAnyNullnessAnnotation(XType type) { + for (XAnnotation annotation : type.getAllAnnotations()) { + if (annotation.getClassName().simpleName().contentEquals("Nullable")) { + return type.getTypeName().annotated(toAnnotationSpec(annotation)); + } + } + return type.getTypeName(); + } + /** - * Returns a ParameterSpec of the input parameter, @Nullable annotated if existing in original - * (this does not handle Nullable type annotations). + * Returns a ParameterSpec of the input parameter, @Nullable annotated if existing in original. */ private static ParameterSpec getParameterSpecWithNullable(XVariableElement parameter) { - ParameterSpec.Builder builder = - ParameterSpec.builder(parameter.getType().getTypeName(), getSimpleName(parameter)); - getNullableAnnotationSpec(parameter).ifPresent(builder::addAnnotation); + TypeName type = withAnyNullnessAnnotation(parameter.getType()); + ParameterSpec.Builder builder = ParameterSpec.builder(type, getSimpleName(parameter)); + /* + * If we already have a type-use Nullable, don't consider also adding a declaration Nullable, + * which could be a duplicate in the case of "hybrid" annotations that support both type-use and + * declaration targets. + */ + if (!type.isAnnotated()) { + getNullableAnnotationSpec(parameter).ifPresent(builder::addAnnotation); + } return builder.build(); } diff --git a/javatests/dagger/hilt/android/processor/internal/GeneratorsTest.java b/javatests/dagger/hilt/android/processor/internal/GeneratorsTest.java index 4fbec681b8c..5705c91d194 100644 --- a/javatests/dagger/hilt/android/processor/internal/GeneratorsTest.java +++ b/javatests/dagger/hilt/android/processor/internal/GeneratorsTest.java @@ -140,6 +140,105 @@ public void copyConstructorParametersConvertsAndroidInternalNullableToExternal() }); } + // This is a regression test for b/382104423 + @Test + public void typeUseNullableCopiedFromSuperConstructor() { + Source baseView = + HiltCompilerTests.javaSource( + "test.BaseView", + "package test;", + "", + "import android.content.Context;", + "import android.util.AttributeSet;", + "import android.view.View;", + "import org.jspecify.annotations.Nullable;", + "", + "public class BaseView extends View {", + " public BaseView(Context context, @Nullable AttributeSet attrs) {", + " super(context, attrs);", + " }", + "}"); + Source myView = + HiltCompilerTests.javaSource( + "test.MyView", + "package test;", + "", + "import android.content.Context;", + "import android.util.AttributeSet;", + "import android.view.View;", + "import dagger.hilt.android.AndroidEntryPoint;", + "import org.jspecify.annotations.Nullable;", + "", + "@AndroidEntryPoint(BaseView.class)", + "public class MyView extends Hilt_MyView {", + " public MyView(Context context, @Nullable AttributeSet attrs) {", + " super(context, attrs);", + " }", + "}"); + HiltCompilerTests.hiltCompiler(baseView, myView) + .compile( + subject -> { + subject.hasErrorCount(0); + StringSubject stringSubject = + subject.generatedSourceFileWithPath("test/Hilt_MyView.java"); + stringSubject.contains("org.jspecify.annotations.Nullable"); + }); + } + + @Test + public void hybridTypeUseAndDeclarationNullableNotDuplicated() { + Source hybridNullable = + HiltCompilerTests.javaSource( + "test.Nullable", + "package test;", + "", + "import static java.lang.annotation.ElementType.PARAMETER;", + "import static java.lang.annotation.ElementType.TYPE_USE;", + "", + "import java.lang.annotation.Target;", + "", + "@Target({TYPE_USE, PARAMETER})", + "public @interface Nullable {}"); + Source baseView = + HiltCompilerTests.javaSource( + "test.BaseView", + "package test;", + "", + "import android.content.Context;", + "import android.util.AttributeSet;", + "import android.view.View;", + "", + "public class BaseView extends View {", + " public BaseView(Context context, @Nullable AttributeSet attrs) {", + " super(context, attrs);", + " }", + "}"); + Source myView = + HiltCompilerTests.javaSource( + "test.MyView", + "package test;", + "", + "import android.content.Context;", + "import android.util.AttributeSet;", + "import android.view.View;", + "import dagger.hilt.android.AndroidEntryPoint;", + "", + "@AndroidEntryPoint(BaseView.class)", + "public class MyView extends Hilt_MyView {", + " public MyView(Context context, @Nullable AttributeSet attrs) {", + " super(context, attrs);", + " }", + "}"); + HiltCompilerTests.hiltCompiler(hybridNullable, baseView, myView) + .compile( + subject -> { + subject.hasErrorCount(0); + StringSubject stringSubject = + subject.generatedSourceFileWithPath("test/Hilt_MyView.java"); + stringSubject.contains("@Nullable"); + }); + } + // This is a regression test for https://github.com/google/dagger/issues/3296 @Test public void isRestrictedApiConstructorWithPrimitiveParameterTest() {