From 698f6cbd20f839121d0f8fb8ac4cb8f9105ee671 Mon Sep 17 00:00:00 2001 From: Kuan-Ying Chou Date: Thu, 10 Aug 2023 13:43:32 -0700 Subject: [PATCH] Fix crash when there are multiple injected constructors RELNOTES=N/A PiperOrigin-RevId: 555635748 --- .../binding/AssistedInjectionAnnotations.java | 2 ++ .../AssistedInjectProcessingStep.java | 13 +++++-- .../internal/codegen/AssistedErrorsTest.java | 34 +++++++++++++++++++ .../codegen/AssistedInjectErrorsTest.java | 29 ++++++++++++++++ 4 files changed, 76 insertions(+), 2 deletions(-) diff --git a/java/dagger/internal/codegen/binding/AssistedInjectionAnnotations.java b/java/dagger/internal/codegen/binding/AssistedInjectionAnnotations.java index a8bca0956df..3ad5105b3c4 100644 --- a/java/dagger/internal/codegen/binding/AssistedInjectionAnnotations.java +++ b/java/dagger/internal/codegen/binding/AssistedInjectionAnnotations.java @@ -253,6 +253,8 @@ public static ImmutableList assistedInjectAssistedParameters( XType assistedInjectType) { // We keep track of the constructor both as an ExecutableElement to access @Assisted // parameters and as an ExecutableType to access the resolved parameter types. + ImmutableSet assistedInjectConstructors = + assistedInjectedConstructors(assistedInjectType.getTypeElement()); XConstructorElement assistedInjectConstructor = getOnlyElement(assistedInjectedConstructors(assistedInjectType.getTypeElement())); XConstructorType assistedInjectConstructorType = diff --git a/java/dagger/internal/codegen/processingstep/AssistedInjectProcessingStep.java b/java/dagger/internal/codegen/processingstep/AssistedInjectProcessingStep.java index 8d73f477477..cfd0bec6e04 100644 --- a/java/dagger/internal/codegen/processingstep/AssistedInjectProcessingStep.java +++ b/java/dagger/internal/codegen/processingstep/AssistedInjectProcessingStep.java @@ -26,6 +26,7 @@ import com.squareup.javapoet.ClassName; import dagger.internal.codegen.binding.AssistedInjectionAnnotations.AssistedParameter; import dagger.internal.codegen.javapoet.TypeNames; +import dagger.internal.codegen.validation.InjectValidator; import dagger.internal.codegen.validation.ValidationReport; import java.util.HashSet; import java.util.Set; @@ -34,10 +35,12 @@ /** An annotation processor for {@link dagger.assisted.AssistedInject}-annotated elements. */ final class AssistedInjectProcessingStep extends TypeCheckingProcessingStep { private final XMessager messager; + private final InjectValidator injectValidator; @Inject - AssistedInjectProcessingStep(XMessager messager) { + AssistedInjectProcessingStep(XMessager messager, InjectValidator injectValidator) { this.messager = messager; + this.injectValidator = injectValidator; } @Override @@ -48,7 +51,13 @@ public ImmutableSet annotationClassNames() { @Override protected void process( XConstructorElement assistedInjectElement, ImmutableSet annotations) { - new AssistedInjectValidator().validate(assistedInjectElement).printMessagesTo(messager); + // The InjectValidator has already run and reported its errors in InjectProcessingStep, so no + // need to report its errors. However, the AssistedInjectValidator relies on the InjectValidator + // returning a clean report, so we check that first before running AssistedInjectValidator. This + // shouldn't be expensive since InjectValidator caches its results after validating. + if (injectValidator.validate(assistedInjectElement.getEnclosingElement()).isClean()) { + new AssistedInjectValidator().validate(assistedInjectElement).printMessagesTo(messager); + } } private final class AssistedInjectValidator { diff --git a/javatests/dagger/internal/codegen/AssistedErrorsTest.java b/javatests/dagger/internal/codegen/AssistedErrorsTest.java index 5a4ebe7a995..1027212a6ce 100644 --- a/javatests/dagger/internal/codegen/AssistedErrorsTest.java +++ b/javatests/dagger/internal/codegen/AssistedErrorsTest.java @@ -97,4 +97,38 @@ public void testNestedFactoryNotStatic() { .onLine(12); }); } + + + @Test + public void testMultipleInjectedConstructors() { + Source foo = + CompilerTests.kotlinSource( + "test.Foo.kt", + "package test;", + "", + "import dagger.assisted.Assisted", + "import dagger.assisted.AssistedInject", + "import dagger.assisted.AssistedFactory", + "import javax.inject.Inject", + "", + "class Foo @AssistedInject constructor(@Assisted i: Int) {", + "", + " @Inject", + " constructor(s: String, @Assisted i: Int): this(i) {}", + "}"); + + CompilerTests.daggerCompiler(foo) + .withProcessingOptions(compilerMode.processorOptions()) + .compile( + subject -> { + subject.hasErrorCount(2); + subject.hasErrorContaining( + "Type test.Foo may only contain one injected constructor." + + " Found: [@Inject test.Foo(String, int)," + + " @dagger.assisted.AssistedInject test.Foo(int)]"); + subject.hasErrorContaining( + "@Assisted parameters can only be used within an" + + " @AssistedInject-annotated constructor."); + }); + } } diff --git a/javatests/dagger/internal/codegen/AssistedInjectErrorsTest.java b/javatests/dagger/internal/codegen/AssistedInjectErrorsTest.java index 1d3628e46a6..e29bc456866 100644 --- a/javatests/dagger/internal/codegen/AssistedInjectErrorsTest.java +++ b/javatests/dagger/internal/codegen/AssistedInjectErrorsTest.java @@ -243,4 +243,33 @@ public void testAssistedInjectWithUniqueQualifiedTypesPasses() { .withProcessingOptions(compilerMode.processorOptions()) .compile(subject -> subject.hasErrorCount(0)); } + + @Test + public void testMultipleAssistedInjectedConstructors() { + Source foo = + CompilerTests.kotlinSource( + "test.Foo.kt", + "package test;", + "", + "import dagger.assisted.Assisted", + "import dagger.assisted.AssistedInject", + "import dagger.assisted.AssistedFactory", + "", + "class Foo @AssistedInject constructor(@Assisted i: Int) {", + "", + " @AssistedInject", + " constructor(s: String, @Assisted i: Int): this(i) {}", + "}"); + + CompilerTests.daggerCompiler(foo) + .withProcessingOptions(compilerMode.processorOptions()) + .compile( + subject -> { + subject.hasErrorCount(1); + subject.hasErrorContaining( + "Type test.Foo may only contain one injected constructor." + + " Found: [@dagger.assisted.AssistedInject test.Foo(int)," + + " @dagger.assisted.AssistedInject test.Foo(String, int)]"); + }); + } }