From dd17adf64de73c3dc5db2a8857d0c616670bc2f8 Mon Sep 17 00:00:00 2001 From: Brad Corso Date: Tue, 19 Nov 2024 15:51:27 -0800 Subject: [PATCH] [Refactor]: Change `FrameworkField#create()` to better align with the XPoet migration. During the XPoet migrations `FrameworkField#create()` will be called by both migrated and non-migrated code. Due to b/379934406, it's currently not feasible to generally go from JTypeName to XTypeName so I'm changing up the `create` method to take in the framework `ClassName` separately. Unlike `TypeName`, a `ClassName` can be converted to an `XClassName` which will make the interopt between the two usages easier during the migration. This CL also updates FrameworkFieldTest to use the new API. RELNOTES=N/A PiperOrigin-RevId: 698172259 --- .../codegen/binding/FrameworkField.java | 63 +++++++++++-------- .../internal/codegen/binding/SourceFiles.java | 7 +-- .../internal/codegen/FrameworkFieldTest.java | 46 +++++++++----- 3 files changed, 68 insertions(+), 48 deletions(-) diff --git a/java/dagger/internal/codegen/binding/FrameworkField.java b/java/dagger/internal/codegen/binding/FrameworkField.java index a9f3bbfbd53..5a59e7f62da 100644 --- a/java/dagger/internal/codegen/binding/FrameworkField.java +++ b/java/dagger/internal/codegen/binding/FrameworkField.java @@ -24,14 +24,13 @@ import static dagger.internal.codegen.xprocessing.XElements.getSimpleName; import androidx.room.compiler.processing.XElement; +import androidx.room.compiler.processing.XType; import com.google.auto.value.AutoValue; import com.google.common.base.CaseFormat; -import com.google.common.base.Preconditions; import com.squareup.javapoet.ClassName; import com.squareup.javapoet.ParameterizedTypeName; import com.squareup.javapoet.TypeName; import dagger.internal.codegen.base.MapType; -import dagger.internal.codegen.javapoet.TypeNames; import java.util.Optional; /** @@ -51,18 +50,13 @@ public abstract class FrameworkField { /** * Creates a framework field. * - * @param fieldType the type of the framework field (e.g., {@code Provider}). * @param fieldName the base name of the field. The name of the raw type of the field will be * added as a suffix + * @param frameworkClassName the framework class that wraps the type (e.g., {@code Provider}). + * @param type the base type of the field (e.g., {@code Foo}). */ - public static FrameworkField create(TypeName fieldType, String fieldName) { - Preconditions.checkState( - fieldType instanceof ClassName || fieldType instanceof ParameterizedTypeName, - "Can only create a field with a class name or parameterized type name"); - String suffix = ((ClassName) TypeNames.rawTypeName(fieldType)).simpleName(); - return new AutoValue_FrameworkField( - fieldType, - fieldName.endsWith(suffix) ? fieldName : fieldName + suffix); + public static FrameworkField create(String fieldName, ClassName frameworkClassName, XType type) { + return createInternal(fieldName, frameworkClassName, Optional.of(type)); } /** @@ -73,14 +67,23 @@ public static FrameworkField create(TypeName fieldType, String fieldName) { */ public static FrameworkField forBinding( ContributionBinding binding, Optional frameworkClassName) { - return create( - fieldType(binding, frameworkClassName.orElse(binding.frameworkType().frameworkClassName())), - frameworkFieldName(binding)); + return createInternal( + bindingName(binding), + frameworkClassName.orElse(binding.frameworkType().frameworkClassName()), + bindingType(binding)); + } + + private static String bindingName(ContributionBinding binding) { + if (binding.bindingElement().isPresent()) { + String name = bindingElementName(binding.bindingElement().get()); + return binding.kind().equals(MEMBERS_INJECTOR) ? name + "MembersInjector" : name; + } + return KeyVariableNamer.name(binding.key()); } - private static TypeName fieldType(ContributionBinding binding, ClassName frameworkClassName) { + private static Optional bindingType(ContributionBinding binding) { if (binding.contributionType().isMultibinding()) { - return ParameterizedTypeName.get(frameworkClassName, binding.contributedType().getTypeName()); + return Optional.of(binding.contributedType()); } // If the binding key type is a Map>, we need to change field type to a raw @@ -89,19 +92,25 @@ private static TypeName fieldType(ContributionBinding binding, ClassName framewo // Map>. We could add casts everywhere, but it is easier to just // make the field itself a raw type. if (MapType.isMapOfProvider(binding.contributedType())) { - return frameworkClassName; + return Optional.empty(); } - return ParameterizedTypeName.get( - frameworkClassName, binding.key().type().xprocessing().getTypeName()); + return Optional.of(binding.key().type().xprocessing()); } - private static String frameworkFieldName(ContributionBinding binding) { - if (binding.bindingElement().isPresent()) { - String name = bindingElementName(binding.bindingElement().get()); - return binding.kind().equals(MEMBERS_INJECTOR) ? name + "MembersInjector" : name; - } - return KeyVariableNamer.name(binding.key()); + private static FrameworkField createInternal( + String fieldName, ClassName frameworkClassName, Optional type) { + return new AutoValue_FrameworkField( + frameworkFieldName(fieldName, frameworkClassName), + type.isPresent() + ? ParameterizedTypeName.get(frameworkClassName, type.get().getTypeName()) + // Use a raw framework classname, e.g. Provider + : frameworkClassName); + } + + private static String frameworkFieldName(String fieldName, ClassName frameworkClassName) { + String suffix = frameworkClassName.simpleName(); + return fieldName.endsWith(suffix) ? fieldName : fieldName + suffix; } private static String bindingElementName(XElement bindingElement) { @@ -118,7 +127,7 @@ private static String bindingElementName(XElement bindingElement) { } } - public abstract TypeName type(); - public abstract String name(); + + public abstract TypeName type(); } diff --git a/java/dagger/internal/codegen/binding/SourceFiles.java b/java/dagger/internal/codegen/binding/SourceFiles.java index d776eb4a9a7..74659017f7e 100644 --- a/java/dagger/internal/codegen/binding/SourceFiles.java +++ b/java/dagger/internal/codegen/binding/SourceFiles.java @@ -107,10 +107,9 @@ public final class SourceFiles { frameworkClassName = TypeNames.PROVIDER; } return FrameworkField.create( - ParameterizedTypeName.get( - frameworkClassName, - dependency.key().type().xprocessing().getTypeName()), - DependencyVariableNamer.name(dependency)); + DependencyVariableNamer.name(dependency), + frameworkClassName, + dependency.key().type().xprocessing()); }); } diff --git a/javatests/dagger/internal/codegen/FrameworkFieldTest.java b/javatests/dagger/internal/codegen/FrameworkFieldTest.java index ba5b88f8c5c..6b9db85b2e2 100644 --- a/javatests/dagger/internal/codegen/FrameworkFieldTest.java +++ b/javatests/dagger/internal/codegen/FrameworkFieldTest.java @@ -22,11 +22,14 @@ import static dagger.internal.codegen.javapoet.TypeNames.membersInjectorOf; import static dagger.internal.codegen.javapoet.TypeNames.providerOf; +import androidx.room.compiler.processing.XProcessingEnv; +import androidx.room.compiler.processing.XType; import com.google.testing.compile.CompilationRule; -import com.squareup.javapoet.ClassName; -import com.squareup.javapoet.ParameterizedTypeName; +import dagger.Component; import dagger.internal.codegen.binding.FrameworkField; +import dagger.internal.codegen.javac.JavacPluginModule; import javax.inject.Inject; +import javax.inject.Singleton; import org.junit.Before; import org.junit.Rule; import org.junit.Test; @@ -40,32 +43,41 @@ public class FrameworkFieldTest { @Rule public CompilationRule compilationRule = new CompilationRule(); - private ClassName xTypeName; + @Inject XProcessingEnv processingEnv; - @Before public void setUp() { - xTypeName = - ClassName.get(compilationRule.getElements().getTypeElement(X.class.getCanonicalName())); + private XType type; + + @Before + public void setUp() { + DaggerFrameworkFieldTest_TestComponent.builder() + .javacPluginModule( + new JavacPluginModule(compilationRule.getElements(), compilationRule.getTypes())) + .build() + .inject(this); + type = processingEnv.requireType(X.class.getCanonicalName()); } @Test public void frameworkType() { - assertThat(FrameworkField.create(ParameterizedTypeName.get(PROVIDER, xTypeName), "test").type()) - .isEqualTo(providerOf(xTypeName)); - assertThat( - FrameworkField.create(ParameterizedTypeName.get(MEMBERS_INJECTOR, xTypeName), "test") - .type()) - .isEqualTo(membersInjectorOf(xTypeName)); + assertThat(FrameworkField.create("test", PROVIDER, type).type()) + .isEqualTo(providerOf(type.getTypeName())); + assertThat(FrameworkField.create("test", MEMBERS_INJECTOR, type).type()) + .isEqualTo(membersInjectorOf(type.getTypeName())); } @Test public void nameSuffix() { - assertThat(FrameworkField.create(ParameterizedTypeName.get(PROVIDER, xTypeName), "foo").name()) + assertThat(FrameworkField.create("foo", PROVIDER, type).name()) .isEqualTo("fooProvider"); - assertThat( - FrameworkField.create(ParameterizedTypeName.get(PROVIDER, xTypeName), "fooProvider") - .name()) + assertThat(FrameworkField.create("fooProvider", PROVIDER, type).name()) .isEqualTo("fooProvider"); } static final class X { @Inject X() {} } -} + + @Singleton + @Component(modules = JavacPluginModule.class) + interface TestComponent { + void inject(FrameworkFieldTest test); + } +} \ No newline at end of file