Skip to content

Commit

Permalink
Discover companion object bindings of @module annotated classes.
Browse files Browse the repository at this point in the history 
If a Kotlin class annotated with @module has a companion object, then
the functions in it are considered as part of the bindings declared by
the module. The bindings are effectively static and don't require an
instance. For backwards compatibility, it is OK to still annotated the
companion object with @module and its functions with @JvmStatic, but
this is no longer needed.

RELNOTES=Support binding declarations in Kotlin companion objects of @module annotated classes.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=288711739
  • Loading branch information
@danysantiago @netdpb
danysantiago authored and netdpb committed Jan 9, 2020
1 parent 124fd04 commit 8190c7c
Show file tree
Hide file tree
Showing 22 changed files with 442 additions and 71 deletions.
39 changes: 29 additions & 10 deletions java/dagger/internal/codegen/ModuleProcessingStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import dagger.internal.codegen.binding.DelegateDeclaration.Factory;
import dagger.internal.codegen.binding.ProductionBinding;
import dagger.internal.codegen.binding.ProvisionBinding;
import dagger.internal.codegen.kotlin.KotlinMetadataUtil;
import dagger.internal.codegen.validation.ModuleValidator;
import dagger.internal.codegen.validation.TypeCheckingProcessingStep;
import dagger.internal.codegen.validation.ValidationReport;
Expand Down Expand Up @@ -64,6 +65,7 @@ final class ModuleProcessingStep extends TypeCheckingProcessingStep<TypeElement>
private final SourceFileGenerator<TypeElement> moduleConstructorProxyGenerator;
private final InaccessibleMapKeyProxyGenerator inaccessibleMapKeyProxyGenerator;
private final DelegateDeclaration.Factory delegateDeclarationFactory;
private final KotlinMetadataUtil metadataUtil;
private final Set<TypeElement> processedModuleElements = Sets.newLinkedHashSet();

@Inject
Expand All @@ -75,7 +77,8 @@ final class ModuleProcessingStep extends TypeCheckingProcessingStep<TypeElement>
SourceFileGenerator<ProductionBinding> producerFactoryGenerator,
@ModuleGenerator SourceFileGenerator<TypeElement> moduleConstructorProxyGenerator,
InaccessibleMapKeyProxyGenerator inaccessibleMapKeyProxyGenerator,
Factory delegateDeclarationFactory) {
Factory delegateDeclarationFactory,
KotlinMetadataUtil metadataUtil) {
super(MoreElements::asType);
this.messager = messager;
this.moduleValidator = moduleValidator;
Expand All @@ -85,6 +88,7 @@ final class ModuleProcessingStep extends TypeCheckingProcessingStep<TypeElement>
this.moduleConstructorProxyGenerator = moduleConstructorProxyGenerator;
this.inaccessibleMapKeyProxyGenerator = inaccessibleMapKeyProxyGenerator;
this.delegateDeclarationFactory = delegateDeclarationFactory;
this.metadataUtil = metadataUtil;
}

@Override
Expand All @@ -106,23 +110,38 @@ protected void process(
if (processedModuleElements.contains(module)) {
return;
}
// For backwards compatibility, we allow a companion object to be annotated with @Module even
// though it's no longer required. However, we skip processing the companion object itself
// because it will now be processed when processing the companion object's enclosing class.
if (metadataUtil.isCompanionObjectClass(module)) {
// TODO(user): Be strict about annotating companion objects with @Module,
// i.e. tell user to annotate parent instead.
return;
}
ValidationReport<TypeElement> report = moduleValidator.validate(module);
report.printMessagesTo(messager);
if (report.isClean()) {
for (ExecutableElement method : methodsIn(module.getEnclosedElements())) {
if (isAnnotationPresent(method, Provides.class)) {
generate(factoryGenerator, bindingFactory.providesMethodBinding(method, module));
} else if (isAnnotationPresent(method, Produces.class)) {
generate(producerFactoryGenerator, bindingFactory.producesMethodBinding(method, module));
} else if (isAnnotationPresent(method, Binds.class)) {
inaccessibleMapKeyProxyGenerator.generate(bindsMethodBinding(module, method), messager);
}
generateForMethodsIn(module);
if (metadataUtil.hasEnclosedCompanionObject(module)) {
generateForMethodsIn(metadataUtil.getEnclosedCompanionObject(module));
}
moduleConstructorProxyGenerator.generate(module, messager);
}
processedModuleElements.add(module);
}

private void generateForMethodsIn(TypeElement module) {
for (ExecutableElement method : methodsIn(module.getEnclosedElements())) {
if (isAnnotationPresent(method, Provides.class)) {
generate(factoryGenerator, bindingFactory.providesMethodBinding(method, module));
} else if (isAnnotationPresent(method, Produces.class)) {
generate(producerFactoryGenerator, bindingFactory.producesMethodBinding(method, module));
} else if (isAnnotationPresent(method, Binds.class)) {
inaccessibleMapKeyProxyGenerator.generate(bindsMethodBinding(module, method), messager);
}
}
moduleConstructorProxyGenerator.generate(module, messager);
}

private <B extends ContributionBinding> void generate(
SourceFileGenerator<B> generator, B binding) {
generator.generate(binding, messager);
Expand Down
11 changes: 8 additions & 3 deletions java/dagger/internal/codegen/binding/BindingFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,14 @@ B setMethodBindingProperties(
if (!types.isSameType(methodType, method.asType())) {
builder.unresolved(create.apply(method, MoreElements.asType(method.getEnclosingElement())));
}
boolean isKotlinObject =
metadataUtil.isObjectClass(contributedBy)
|| metadataUtil.isCompanionObjectClass(contributedBy);
return builder
.contributionType(ContributionType.fromBindingElement(method))
.bindingElement(method)
.contributingModule(contributedBy)
.isModuleKotlinObject(metadataUtil.isObjectClass(contributedBy))
.isContributingModuleKotlinObject(isKotlinObject)
.key(key)
.dependencies(
dependencyRequestFactory.forRequiredResolvedVariables(
Expand Down Expand Up @@ -420,12 +423,14 @@ private ContributionBinding buildDelegateBinding(
ContributionBinding.Builder<?, ?> builder,
DelegateDeclaration delegateDeclaration,
Class<?> frameworkType) {
boolean isKotlinObject =
metadataUtil.isObjectClass(delegateDeclaration.contributingModule().get())
|| metadataUtil.isCompanionObjectClass(delegateDeclaration.contributingModule().get());
return builder
.contributionType(delegateDeclaration.contributionType())
.bindingElement(delegateDeclaration.bindingElement().get())
.contributingModule(delegateDeclaration.contributingModule().get())
.isModuleKotlinObject(
metadataUtil.isObjectClass(delegateDeclaration.contributingModule().get()))
.isContributingModuleKotlinObject(isKotlinObject)
.key(keyFactory.forDelegateBinding(delegateDeclaration, frameworkType))
.dependencies(delegateDeclaration.delegateRequest())
.wrappedMapKeyAnnotation(delegateDeclaration.wrappedMapKey())
Expand Down
19 changes: 13 additions & 6 deletions java/dagger/internal/codegen/binding/ComponentRequirement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,13 +166,19 @@ public boolean requiresAPassedInstance(
*/
private boolean requiresModuleInstance(
DaggerElements elements, DaggerTypes types, KotlinMetadataUtil metadataUtil) {
boolean isKotlinObject =
metadataUtil.isObjectClass(typeElement())
|| metadataUtil.isCompanionObjectClass(typeElement());
if (isKotlinObject) {
return false;
}

ImmutableSet<ExecutableElement> methods =
getLocalAndInheritedMethods(typeElement(), types, elements);
return !metadataUtil.isObjectClass(typeElement())
&& methods.stream()
.filter(this::isBindingMethod)
.map(ExecutableElement::getModifiers)
.anyMatch(modifiers -> !modifiers.contains(ABSTRACT) && !modifiers.contains(STATIC));
return methods.stream()
.filter(this::isBindingMethod)
.map(ExecutableElement::getModifiers)
.anyMatch(modifiers -> !modifiers.contains(ABSTRACT) && !modifiers.contains(STATIC));
}

private boolean isBindingMethod(ExecutableElement method) {
Expand Down Expand Up @@ -261,7 +267,8 @@ public static boolean componentCanMakeNewInstances(
return false;
}

if (metadataUtil.isObjectClass(typeElement)) {
if (metadataUtil.isObjectClass(typeElement)
|| metadataUtil.isCompanionObjectClass(typeElement)) {
return false;
}

Expand Down
13 changes: 9 additions & 4 deletions java/dagger/internal/codegen/binding/ContributionBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,15 +67,19 @@ public final Optional<TypeMirror> contributedPrimitiveType() {

@Override
public boolean requiresModuleInstance() {
return !isModuleKotlinObject().orElse(false) && super.requiresModuleInstance();
return !isContributingModuleKotlinObject().orElse(false) && super.requiresModuleInstance();
}

@Override
public final boolean isNullable() {
return nullableType().isPresent();
}

abstract Optional<Boolean> isModuleKotlinObject();
/**
* Returns {@code true} if the contributing module is a Kotlin object. Note that a companion
* object is also considered a Kotlin object.
*/
abstract Optional<Boolean> isContributingModuleKotlinObject();

/** The strategy for getting an instance of a factory for a {@link ContributionBinding}. */
public enum FactoryCreationStrategy {
Expand Down Expand Up @@ -168,7 +172,7 @@ public B dependencies(DependencyRequest... dependencies) {

abstract B contributingModule(TypeElement contributingModule);

abstract B isModuleKotlinObject(boolean isModuleKotlinObject);
abstract B isContributingModuleKotlinObject(boolean isModuleKotlinObject);

public abstract B key(Key key);

Expand All @@ -186,7 +190,8 @@ abstract B wrappedMapKeyAnnotation(
public C build() {
C binding = autoBuild();
Preconditions.checkState(
binding.contributingModule().isPresent() == binding.isModuleKotlinObject().isPresent(),
binding.contributingModule().isPresent()
== binding.isContributingModuleKotlinObject().isPresent(),
"The contributionModule and isModuleKotlinObject must both be set together.");
return binding;
}
Expand Down
39 changes: 39 additions & 0 deletions java/dagger/internal/codegen/binding/ModuleDescriptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,19 @@

package dagger.internal.codegen.binding;

import static com.google.auto.common.MoreElements.asExecutable;
import static com.google.auto.common.MoreElements.getPackage;
import static com.google.auto.common.MoreElements.isAnnotationPresent;
import static com.google.common.base.CaseFormat.LOWER_CAMEL;
import static com.google.common.base.CaseFormat.UPPER_CAMEL;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Verify.verify;
import static com.google.common.collect.Iterables.transform;
import static dagger.internal.codegen.base.ModuleAnnotation.moduleAnnotation;
import static dagger.internal.codegen.base.Util.reentrantComputeIfAbsent;
import static dagger.internal.codegen.binding.SourceFiles.classFileName;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet;
import static dagger.internal.codegen.langmodel.DaggerElements.getMethodDescriptor;
import static dagger.internal.codegen.langmodel.DaggerElements.isAnnotationPresent;
import static javax.lang.model.type.TypeKind.DECLARED;
import static javax.lang.model.type.TypeKind.NONE;
Expand All @@ -44,6 +47,7 @@
import dagger.Module;
import dagger.Provides;
import dagger.internal.codegen.base.ClearableCache;
import dagger.internal.codegen.kotlin.KotlinMetadataUtil;
import dagger.internal.codegen.langmodel.DaggerElements;
import dagger.model.Key;
import dagger.multibindings.Multibinds;
Expand Down Expand Up @@ -104,6 +108,7 @@ ImmutableSet<Key> allBindingKeys() {
@Singleton
public static final class Factory implements ClearableCache {
private final DaggerElements elements;
private final KotlinMetadataUtil metadataUtil;
private final BindingFactory bindingFactory;
private final MultibindingDeclaration.Factory multibindingDeclarationFactory;
private final DelegateDeclaration.Factory bindingDelegateDeclarationFactory;
Expand All @@ -114,12 +119,14 @@ public static final class Factory implements ClearableCache {
@Inject
Factory(
DaggerElements elements,
KotlinMetadataUtil metadataUtil,
BindingFactory bindingFactory,
MultibindingDeclaration.Factory multibindingDeclarationFactory,
DelegateDeclaration.Factory bindingDelegateDeclarationFactory,
SubcomponentDeclaration.Factory subcomponentDeclarationFactory,
OptionalBindingDeclaration.Factory optionalBindingDeclarationFactory) {
this.elements = elements;
this.metadataUtil = metadataUtil;
this.bindingFactory = bindingFactory;
this.multibindingDeclarationFactory = multibindingDeclarationFactory;
this.bindingDelegateDeclarationFactory = bindingDelegateDeclarationFactory;
Expand Down Expand Up @@ -159,6 +166,10 @@ public ModuleDescriptor createUncached(TypeElement moduleElement) {
}
}

if (metadataUtil.hasEnclosedCompanionObject(moduleElement)) {
collectCompanionModuleBindings(moduleElement, bindings);
}

return new AutoValue_ModuleDescriptor(
moduleElement,
ImmutableSet.copyOf(collectIncludedModules(new LinkedHashSet<>(), moduleElement)),
Expand All @@ -170,6 +181,34 @@ public ModuleDescriptor createUncached(TypeElement moduleElement) {
ModuleKind.forAnnotatedElement(moduleElement).get());
}

private void collectCompanionModuleBindings(
TypeElement moduleElement, ImmutableSet.Builder<ContributionBinding> bindings) {
checkArgument(metadataUtil.hasEnclosedCompanionObject(moduleElement));
TypeElement companionModule = metadataUtil.getEnclosedCompanionObject(moduleElement);
ImmutableSet<String> bindingElementDescriptors =
bindings.build().stream()
.map(binding -> getMethodDescriptor(asExecutable(binding.bindingElement().get())))
.collect(toImmutableSet());
methodsIn(elements.getAllMembers(companionModule)).stream()
// Binding methods in companion objects with @JvmStatic are mirrored in the enclosing
// class, therefore we should ignore it or else it'll be a duplicate binding.
.filter(method -> !KotlinMetadataUtil.isJvmStaticPresent(method))
// Fallback strategy for de-duping contributing bindings in the companion module with
// @JvmStatic by comparing descriptors. Contributing bindings are the only valid bindings
// a companion module can declare. See: https://youtrack.jetbrains.com/issue/KT-35104
// TODO(user): Checks qualifiers too.
.filter(method -> !bindingElementDescriptors.contains(getMethodDescriptor(method)))
.forEach(
method -> {
if (isAnnotationPresent(method, Provides.class)) {
bindings.add(bindingFactory.providesMethodBinding(method, companionModule));
}
if (isAnnotationPresent(method, Produces.class)) {
bindings.add(bindingFactory.producesMethodBinding(method, companionModule));
}
});
}

/** Returns all the modules transitively included by given modules, including the arguments. */
ImmutableSet<ModuleDescriptor> transitiveModules(Iterable<TypeElement> modules) {
return ImmutableSet.copyOf(
Expand Down
12 changes: 10 additions & 2 deletions java/dagger/internal/codegen/binding/ModuleKind.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package dagger.internal.codegen.binding;

import static com.google.auto.common.MoreElements.asType;
import static com.google.common.base.Preconditions.checkArgument;
import static dagger.internal.codegen.extension.DaggerStreams.toImmutableSet;
import static dagger.internal.codegen.langmodel.DaggerElements.getAnnotationMirror;
Expand All @@ -24,6 +25,7 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Sets;
import dagger.Module;
import dagger.internal.codegen.kotlin.KotlinMetadataUtil;
import dagger.producers.ProducerModule;
import java.lang.annotation.Annotation;
import java.util.EnumSet;
Expand Down Expand Up @@ -67,8 +69,14 @@ public static Optional<ModuleKind> forAnnotatedElement(TypeElement element) {
return kinds.stream().findAny();
}

public static void checkIsModule(TypeElement moduleElement) {
checkArgument(forAnnotatedElement(moduleElement).isPresent());
public static void checkIsModule(TypeElement moduleElement, KotlinMetadataUtil metadataUtil) {
// If the type element is a Kotlin companion object, then assert it is a module if its enclosing
// type is a module.
if (metadataUtil.isCompanionObjectClass(moduleElement)) {
checkArgument(forAnnotatedElement(asType(moduleElement.getEnclosingElement())).isPresent());
} else {
checkArgument(forAnnotatedElement(moduleElement).isPresent());
}
}

private final Class<? extends Annotation> moduleAnnotation;
Expand Down
1 change: 1 addition & 0 deletions java/dagger/internal/codegen/kotlin/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ java_library(
"//java/dagger/internal/guava:base",
"//java/dagger/internal/guava:collect",
"@google_bazel_common//third_party/java/auto:common",
"@google_bazel_common//third_party/java/jsr305_annotations",
"@google_bazel_common//third_party/java/jsr330_inject",
"@maven//:org_jetbrains_kotlin_kotlin_stdlib",
"@maven//:org_jetbrains_kotlinx_kotlinx_metadata_jvm",
Expand Down
Loading

0 comments on commit 8190c7c

Please sign in to comment.