Skip to content

Commit

Permalink
Migrate dagger.spi.model.Key.MultibindingContributionIdentifier to …
Browse files Browse the repository at this point in the history
…use XProcessing.

This CL also makes some API simplifications, e.g. it replaces the more general `toBuilder()` method with more specific `with*()` methods as described in (https://github.com/google/auto/blob/master/value/userguide/builders-howto.md#-include-with--methods-on-my-value-class-for-creating-slightly-altered-instances). This CL also converts `MultibindingContributionIdentifier` to use auto value and removes its public constructors.

RELNOTES=N/A
PiperOrigin-RevId: 463420943
  • Loading branch information
bcorso authored and Dagger Team committed Jul 26, 2022
1 parent 3445275 commit 56335c1
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 109 deletions.
10 changes: 2 additions & 8 deletions java/dagger/internal/codegen/binding/BindingGraphFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ private LegacyBindingGraph createLegacyBindingGraph(
// identifier so that the multibinding itself is resolved.
modules(componentDescriptor, parentResolver).stream()
.flatMap(module -> module.allBindingKeys().stream())
.map(key -> key.toBuilder().multibindingContributionIdentifier(Optional.empty()).build())
.map(Key::withoutMultibindingContributionIdentifier)
.forEach(requestResolver::resolve);
}

Expand Down Expand Up @@ -965,13 +965,7 @@ ImmutableSetMultimap<Key, T> multibindingContributionsByMultibindingKey(
ImmutableSetMultimap.Builder<Key, T> builder = ImmutableSetMultimap.builder();
for (T declaration : declarations) {
if (declaration.key().multibindingContributionIdentifier().isPresent()) {
builder.put(
declaration
.key()
.toBuilder()
.multibindingContributionIdentifier(Optional.empty())
.build(),
declaration);
builder.put(declaration.key().withoutMultibindingContributionIdentifier(), declaration);
}
}
return builder.build();
Expand Down
32 changes: 11 additions & 21 deletions java/dagger/internal/codegen/binding/KeyFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@
import dagger.internal.codegen.xprocessing.XAnnotations;
import dagger.multibindings.Multibinds;
import dagger.spi.model.DaggerAnnotation;
import dagger.spi.model.DaggerExecutableElement;
import dagger.spi.model.DaggerType;
import dagger.spi.model.DaggerTypeElement;
import dagger.spi.model.Key;
import dagger.spi.model.Key.MultibindingContributionIdentifier;
import dagger.spi.model.RequestKind;
import java.util.Map;
import java.util.Optional;
Expand Down Expand Up @@ -153,10 +154,8 @@ private Key forBindingMethod(
Key key = forMethod(method, keyType);
return contributionType.equals(ContributionType.UNIQUE)
? key
: key.toBuilder()
.multibindingContributionIdentifier(
new MultibindingContributionIdentifier(method, contributingModule))
.build();
: key.withMultibindingContributionIdentifier(
DaggerTypeElement.from(contributingModule), DaggerExecutableElement.from(method));
}

/**
Expand Down Expand Up @@ -319,11 +318,9 @@ public Key unwrapMapValueType(Key key) {
for (ClassName frameworkClass :
asList(TypeNames.PROVIDER, TypeNames.PRODUCER, TypeNames.PRODUCED)) {
if (mapType.valuesAreTypeOf(frameworkClass)) {
return key.toBuilder()
.type(
DaggerType.from(
mapOf(mapType.keyType(), mapType.unwrappedValueType(frameworkClass))))
.build();
return key.withType(
DaggerType.from(
mapOf(mapType.keyType(), mapType.unwrappedValueType(frameworkClass))));
}
}
}
Expand Down Expand Up @@ -363,9 +360,7 @@ public Optional<Key> rewrapMapKey(
processingEnv.getDeclaredType(
wrappingElement, mapType.unwrappedValueType(currentWrappingClassName));
return Optional.of(
possibleMapKey.toBuilder()
.type(DaggerType.from(mapOf(mapType.keyType(), wrappedValueType)))
.build());
possibleMapKey.withType(DaggerType.from(mapOf(mapType.keyType(), wrappedValueType))));
}
}
return Optional.empty();
Expand All @@ -391,9 +386,7 @@ private Optional<Key> wrapMapKey(Key possibleMapKey, ClassName wrappingClassName
XType wrappedValueType =
processingEnv.getDeclaredType(wrappingElement, mapType.valueType());
return Optional.of(
possibleMapKey.toBuilder()
.type(DaggerType.from(mapOf(mapType.keyType(), wrappedValueType)))
.build());
possibleMapKey.withType(DaggerType.from(mapOf(mapType.keyType(), wrappedValueType))));
}
}
return Optional.empty();
Expand All @@ -408,9 +401,7 @@ Optional<Key> unwrapSetKey(Key key, ClassName wrappingClassName) {
SetType setType = SetType.from(key);
if (!setType.isRawType() && setType.elementsAreTypeOf(wrappingClassName)) {
return Optional.of(
key.toBuilder()
.type(DaggerType.from(setOf(setType.unwrappedElementType(wrappingClassName))))
.build());
key.withType(DaggerType.from(setOf(setType.unwrappedElementType(wrappingClassName)))));
}
}
return Optional.empty();
Expand All @@ -427,7 +418,6 @@ Optional<Key> unwrapOptional(Key key) {
}

XType optionalValueType = OptionalType.from(key).valueType();
return Optional.of(
key.toBuilder().type(DaggerType.from(extractKeyType(optionalValueType))).build());
return Optional.of(key.withType(DaggerType.from(extractKeyType(optionalValueType))));
}
}
2 changes: 1 addition & 1 deletion java/dagger/internal/codegen/binding/KeyVariableNamer.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ private KeyVariableNamer() {}

public static String name(Key key) {
if (key.multibindingContributionIdentifier().isPresent()) {
return key.multibindingContributionIdentifier().get().bindingElement();
return key.multibindingContributionIdentifier().get().bindingMethod();
}

StringBuilder builder = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,8 @@ private static Edge fromSpiModel(dagger.spi.model.BindingGraph.Edge edge) {

private static MultibindingContributionIdentifier fromSpiModel(
dagger.spi.model.Key.MultibindingContributionIdentifier identifier) {
return new MultibindingContributionIdentifier(identifier.bindingElement(), identifier.module());
return new MultibindingContributionIdentifier(
identifier.bindingMethod(), identifier.contributingModule());
}

private static Key fromSpiModel(dagger.spi.model.Key key) {
Expand Down
118 changes: 43 additions & 75 deletions java/dagger/spi/model/Key.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,17 +16,12 @@

package dagger.spi.model;

import static androidx.room.compiler.processing.compat.XConverters.toJavac;
import static dagger.internal.codegen.xprocessing.XElements.getSimpleName;

import androidx.room.compiler.processing.XMethodElement;
import androidx.room.compiler.processing.XTypeElement;
import com.google.auto.value.AutoValue;
import com.google.auto.value.extension.memoized.Memoized;
import com.google.common.base.Joiner;
import java.util.Objects;
import java.util.Optional;
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.TypeElement;

/**
* A {@linkplain DaggerType type} and an optional {@linkplain javax.inject.Qualifier qualifier} that
Expand Down Expand Up @@ -55,7 +50,28 @@ public abstract class Key {
public abstract Optional<MultibindingContributionIdentifier> multibindingContributionIdentifier();

/** Returns a {@link Builder} that inherits the properties of this key. */
public abstract Builder toBuilder();
abstract Builder toBuilder();

/** Returns a copy of this key with the type replaced with the given type. */
public Key withType(DaggerType newType) {
return toBuilder().type(newType).build();
}

/**
* Returns a copy of this key with the multibinding contribution identifier replaced with the
* given multibinding contribution identifier.
*/
public Key withMultibindingContributionIdentifier(
DaggerTypeElement contributingModule, DaggerExecutableElement bindingMethod) {
return toBuilder()
.multibindingContributionIdentifier(contributingModule, bindingMethod)
.build();
}

/** Returns a copy of this key with the multibinding contribution identifier, if any, removed. */
public Key withoutMultibindingContributionIdentifier() {
return toBuilder().multibindingContributionIdentifier(Optional.empty()).build();
}

// The main hashCode/equality bottleneck is in MoreTypes.equivalence(). It's possible that we can
// avoid this by tuning that method. Perhaps we can also avoid the issue entirely by interning all
Expand Down Expand Up @@ -91,11 +107,15 @@ public abstract static class Builder {

public abstract Builder qualifier(DaggerAnnotation qualifier);

public abstract Builder multibindingContributionIdentifier(
Optional<MultibindingContributionIdentifier> identifier);
public final Builder multibindingContributionIdentifier(
DaggerTypeElement contributingModule, DaggerExecutableElement bindingMethod) {
return multibindingContributionIdentifier(
Optional.of(
MultibindingContributionIdentifier.create(contributingModule, bindingMethod)));
}

public abstract Builder multibindingContributionIdentifier(
MultibindingContributionIdentifier identifier);
abstract Builder multibindingContributionIdentifier(
Optional<MultibindingContributionIdentifier> identifier);

public abstract Key build();
}
Expand All @@ -106,58 +126,20 @@ public abstract Builder multibindingContributionIdentifier(
*
* @see #multibindingContributionIdentifier()
*/
public static final class MultibindingContributionIdentifier {
private final String module;
private final String bindingElement;

/**
* @deprecated This is only meant to be called from code in {@code dagger.internal.codegen}. It
* is not part of a specified API and may change at any point.
*/
@Deprecated
public MultibindingContributionIdentifier(
// TODO(ronshapiro): reverse the order of these parameters
XMethodElement bindingMethod, XTypeElement contributingModule) {
this(toJavac(bindingMethod), toJavac(contributingModule));
}

/**
* @deprecated This is only meant to be called from code in {@code dagger.internal.codegen}.
* It is not part of a specified API and may change at any point.
*/
@Deprecated
public MultibindingContributionIdentifier(
// TODO(ronshapiro): reverse the order of these parameters
ExecutableElement bindingMethod, TypeElement contributingModule) {
this(
bindingMethod.getSimpleName().toString(),
contributingModule.getQualifiedName().toString());
}

// TODO(ronshapiro,dpb): create KeyProxies so that these constructors don't need to be public.
@Deprecated
public MultibindingContributionIdentifier(String bindingElement, String module) {
this.module = module;
this.bindingElement = bindingElement;
@AutoValue
public abstract static class MultibindingContributionIdentifier {
private static MultibindingContributionIdentifier create(
DaggerTypeElement contributingModule, DaggerExecutableElement bindingMethod) {
return new AutoValue_Key_MultibindingContributionIdentifier(
contributingModule.xprocessing().getQualifiedName(),
getSimpleName(bindingMethod.xprocessing()));
}

/**
* @deprecated This is only meant to be called from code in {@code dagger.internal.codegen}.
* It is not part of a specified API and may change at any point.
*/
@Deprecated
public String module() {
return module;
}
/** Returns the module containing the multibinding method. */
public abstract String contributingModule();

/**
* @deprecated This is only meant to be called from code in {@code dagger.internal.codegen}.
* It is not part of a specified API and may change at any point.
*/
@Deprecated
public String bindingElement() {
return bindingElement;
}
/** Returns the multibinding method that defines teh multibinding contribution. */
public abstract String bindingMethod();

/**
* {@inheritDoc}
Expand All @@ -167,21 +149,7 @@ public String bindingElement() {
*/
@Override
public String toString() {
return String.format("%s#%s", module, bindingElement);
}

@Override
public boolean equals(Object obj) {
if (obj instanceof MultibindingContributionIdentifier) {
MultibindingContributionIdentifier other = (MultibindingContributionIdentifier) obj;
return module.equals(other.module) && bindingElement.equals(other.bindingElement);
}
return false;
}

@Override
public int hashCode() {
return Objects.hash(module, bindingElement);
return String.format("%s#%s", contributingModule(), bindingMethod());
}
}
}
9 changes: 6 additions & 3 deletions javatests/dagger/internal/codegen/KeyFactoryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,10 @@
import dagger.producers.ProducerModule;
import dagger.producers.Produces;
import dagger.spi.model.DaggerAnnotation;
import dagger.spi.model.DaggerExecutableElement;
import dagger.spi.model.DaggerType;
import dagger.spi.model.DaggerTypeElement;
import dagger.spi.model.Key;
import dagger.spi.model.Key.MultibindingContributionIdentifier;
import java.lang.annotation.Retention;
import java.util.Set;
import javax.inject.Inject;
Expand Down Expand Up @@ -209,7 +210,8 @@ public void forProvidesMethod_sets() {
.isEqualTo(
Key.builder(DaggerType.from(setOfStringsType))
.multibindingContributionIdentifier(
new MultibindingContributionIdentifier(providesMethod, moduleElement))
DaggerTypeElement.from(moduleElement),
DaggerExecutableElement.from(providesMethod))
.build());
assertThat(key.toString())
.isEqualTo(
Expand Down Expand Up @@ -300,7 +302,8 @@ static final class ProducesMethodsModule {
.isEqualTo(
Key.builder(DaggerType.from(setOfStringsType))
.multibindingContributionIdentifier(
new MultibindingContributionIdentifier(producesMethod, moduleElement))
DaggerTypeElement.from(moduleElement),
DaggerExecutableElement.from(producesMethod))
.build());
assertThat(key.toString())
.isEqualTo(
Expand Down

0 comments on commit 56335c1

Please sign in to comment.