Skip to content

Commit

Permalink
Remove dagger-android-processor's dependency on dagger-android-jarimp…
Browse files Browse the repository at this point in the history
…l artifact.

Due mainly to BasicAnnotationProcessor.ProcessingStep, the dagger-android-processor required a dependency on the dagger-android annotation classes. However, due to restrictions on java_plugin depending on an android_library, mentioned in bazelbuild/bazel#2517, we had to create a java_import to work around this issue. However, soon even the java_import workaround will no longer work.

This CL fixes the above issues by switching to the new BasicAnnotaitonProcessor.Step, which allows processing annotations via String instead of Class, which allows us to remove the processors dependency on the dagger-android API altogether.

RELNOTES=the Dagger artifact, "dagger-android-jarimpl", has been removed. This was an internal-only artifact, so its removal shouldn't affect users.
PiperOrigin-RevId: 375962581
  • Loading branch information
bcorso authored and Dagger Team committed May 26, 2021
1 parent 5ce0cea commit 6da2e7e
Show file tree
Hide file tree
Showing 12 changed files with 294 additions and 124 deletions.
8 changes: 0 additions & 8 deletions java/dagger/android/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,6 @@ pom_file(
targets = [":android"],
)

# b/37741866 and https://github.com/google/dagger/issues/715
pom_file(
name = "jarimpl-pom",
artifact_id = "dagger-android-jarimpl",
artifact_name = "Dagger Android",
targets = [":android"],
)

dejetified_library(
name = "dejetified-android",
input = ":android.aar",
Expand Down
25 changes: 11 additions & 14 deletions java/dagger/android/processor/AndroidInjectorDescriptor.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

import static com.google.auto.common.AnnotationMirrors.getAnnotatedAnnotations;
import static com.google.auto.common.AnnotationMirrors.getAnnotationValue;
import static com.google.auto.common.MoreElements.getAnnotationMirror;
import static com.google.auto.common.MoreElements.isAnnotationPresent;
import static java.util.stream.Collectors.toList;
import static javax.lang.model.element.Modifier.ABSTRACT;

Expand All @@ -30,8 +28,6 @@
import com.squareup.javapoet.AnnotationSpec;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.TypeName;
import dagger.Module;
import dagger.android.ContributesAndroidInjector;
import java.util.List;
import java.util.Optional;
import javax.annotation.processing.Messager;
Expand All @@ -47,24 +43,24 @@
import javax.tools.Diagnostic.Kind;

/**
* A descriptor of a generated {@link Module} and {@link dagger.Subcomponent} to be generated from a
* {@link ContributesAndroidInjector} method.
* A descriptor of a generated {@link dagger.Module} and {@link dagger.Subcomponent} to be generated
* from a {@code ContributesAndroidInjector} method.
*/
@AutoValue
abstract class AndroidInjectorDescriptor {
/** The type to be injected; the return type of the {@link ContributesAndroidInjector} method. */
/** The type to be injected; the return type of the {@code ContributesAndroidInjector} method. */
abstract ClassName injectedType();

/** Scopes to apply to the generated {@link dagger.Subcomponent}. */
abstract ImmutableSet<AnnotationSpec> scopes();

/** @see ContributesAndroidInjector#modules() */
/** See {@code ContributesAndroidInjector#modules()} */
abstract ImmutableSet<ClassName> modules();

/** The {@link Module} that contains the {@link ContributesAndroidInjector} method. */
/** The {@link dagger.Module} that contains the {@code ContributesAndroidInjector} method. */
abstract ClassName enclosingModule();

/** The method annotated with {@link ContributesAndroidInjector}. */
/** The method annotated with {@code ContributesAndroidInjector}. */
abstract ExecutableElement method();

@AutoValue.Builder
Expand All @@ -90,7 +86,7 @@ static final class Validator {
}

/**
* Validates a {@link ContributesAndroidInjector} method, returning an {@link
* Validates a {@code ContributesAndroidInjector} method, returning an {@link
* AndroidInjectorDescriptor} if it is valid, or {@link Optional#empty()} otherwise.
*/
Optional<AndroidInjectorDescriptor> createIfValid(ExecutableElement method) {
Expand All @@ -107,7 +103,7 @@ Optional<AndroidInjectorDescriptor> createIfValid(ExecutableElement method) {
AndroidInjectorDescriptor.Builder builder =
new AutoValue_AndroidInjectorDescriptor.Builder().method(method);
TypeElement enclosingElement = MoreElements.asType(method.getEnclosingElement());
if (!isAnnotationPresent(enclosingElement, Module.class)) {
if (!MoreDaggerElements.isAnnotationPresent(enclosingElement, TypeNames.MODULE)) {
reporter.reportError("@ContributesAndroidInjector methods must be in a @Module");
}
builder.enclosingModule(ClassName.get(enclosingElement));
Expand All @@ -121,10 +117,11 @@ Optional<AndroidInjectorDescriptor> createIfValid(ExecutableElement method) {
}

AnnotationMirror annotation =
getAnnotationMirror(method, ContributesAndroidInjector.class).get();
MoreDaggerElements.getAnnotationMirror(method, TypeNames.CONTRIBUTES_ANDROID_INJECTOR)
.get();
for (TypeMirror module :
getAnnotationValue(annotation, "modules").accept(new AllTypesVisitor(), null)) {
if (isAnnotationPresent(MoreTypes.asElement(module), Module.class)) {
if (MoreDaggerElements.isAnnotationPresent(MoreTypes.asElement(module), TypeNames.MODULE)) {
builder.modulesBuilder().add((ClassName) TypeName.get(module));
} else {
reporter.reportError(String.format("%s is not a @Module", module), annotation);
Expand Down
54 changes: 25 additions & 29 deletions java/dagger/android/processor/AndroidMapKeyValidator.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,16 @@
package dagger.android.processor;

import static com.google.auto.common.AnnotationMirrors.getAnnotatedAnnotations;
import static com.google.auto.common.MoreElements.getAnnotationMirror;
import static com.google.auto.common.MoreElements.isAnnotationPresent;
import static com.google.common.collect.Iterables.getOnlyElement;
import static dagger.android.processor.AndroidMapKeys.injectedTypeFromMapKey;

import com.google.auto.common.BasicAnnotationProcessor.ProcessingStep;
import com.google.auto.common.BasicAnnotationProcessor.Step;
import com.google.auto.common.MoreElements;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.SetMultimap;
import dagger.Binds;
import com.google.common.collect.ImmutableSetMultimap;
import com.squareup.javapoet.ClassName;
import dagger.MapKey;
import dagger.android.AndroidInjectionKey;
import dagger.android.AndroidInjector;
import dagger.multibindings.ClassKey;
import java.lang.annotation.Annotation;
import java.util.Set;
import javax.annotation.processing.Messager;
import javax.inject.Qualifier;
import javax.inject.Scope;
Expand All @@ -46,8 +40,13 @@
import javax.lang.model.util.Types;
import javax.tools.Diagnostic.Kind;

/** Validates the correctness of {@link MapKey}s used with {@code dagger.android}. */
final class AndroidMapKeyValidator implements ProcessingStep {
/** Validates the correctness of {@link dagger.MapKey}s used with {@code dagger.android}. */
final class AndroidMapKeyValidator implements Step {
private static final ImmutableMap<String, ClassName> SUPPORTED_ANNOTATIONS =
ImmutableMap.of(
TypeNames.ANDROID_INJECTION_KEY.toString(), TypeNames.ANDROID_INJECTION_KEY,
TypeNames.CLASS_KEY.toString(), TypeNames.CLASS_KEY);

private final Elements elements;
private final Types types;
private final Messager messager;
Expand All @@ -59,16 +58,12 @@ final class AndroidMapKeyValidator implements ProcessingStep {
}

@Override
public Set<? extends Class<? extends Annotation>> annotations() {
return ImmutableSet.<Class<? extends Annotation>>builder()
.add(AndroidInjectionKey.class)
.add(ClassKey.class)
.build();
public ImmutableSet<String> annotations() {
return SUPPORTED_ANNOTATIONS.keySet();
}

@Override
public Set<Element> process(
SetMultimap<Class<? extends Annotation>, Element> elementsByAnnotation) {
public ImmutableSet<Element> process(ImmutableSetMultimap<String, Element> elementsByAnnotation) {
ImmutableSet.Builder<Element> deferredElements = ImmutableSet.builder();
elementsByAnnotation
.entries()
Expand All @@ -83,7 +78,7 @@ public Set<Element> process(
return deferredElements.build();
}

private void validateMethod(Class<? extends Annotation> annotation, ExecutableElement method) {
private void validateMethod(String annotation, ExecutableElement method) {
if (!getAnnotatedAnnotations(method, Qualifier.class).isEmpty()) {
return;
}
Expand All @@ -107,7 +102,7 @@ private void validateMethod(Class<? extends Annotation> annotation, ExecutableEl
Kind.ERROR,
String.format(
"%s bindings should not be scoped. Scoping this method may leak instances of %s.",
AndroidInjector.Factory.class.getCanonicalName(),
TypeNames.ANDROID_INJECTOR_FACTORY.canonicalName(),
mapKeyValueElement.getQualifiedName()),
method);
}
Expand All @@ -117,7 +112,8 @@ private void validateMethod(Class<? extends Annotation> annotation, ExecutableEl

// @Binds methods should only have one parameter, but we can't guarantee the order of Processors
// in javac, so do a basic check for valid form
if (isAnnotationPresent(method, Binds.class) && method.getParameters().size() == 1) {
if (MoreDaggerElements.isAnnotationPresent(method, TypeNames.BINDS)
&& method.getParameters().size() == 1) {
validateMapKeyMatchesBindsParameter(annotation, method);
}
}
Expand All @@ -138,10 +134,10 @@ private void validateReturnType(ExecutableElement method) {
}

/**
* A valid @Binds method could bind an {@link AndroidInjector.Factory} for one type, while giving
* A valid @Binds method could bind an {@code AndroidInjector.Factory} for one type, while giving
* it a map key of a different type. The return type and parameter type would pass typical @Binds
* validation, but the map lookup in {@link dagger.android.DispatchingAndroidInjector} would
* retrieve the wrong injector factory.
* validation, but the map lookup in {@code DispatchingAndroidInjector} would retrieve the wrong
* injector factory.
*
* <pre>{@code
* {@literal @Binds}
Expand All @@ -151,10 +147,10 @@ private void validateReturnType(ExecutableElement method) {
* BlueActivityComponent.Builder builder);
* }</pre>
*/
private void validateMapKeyMatchesBindsParameter(
Class<? extends Annotation> annotation, ExecutableElement method) {
private void validateMapKeyMatchesBindsParameter(String annotation, ExecutableElement method) {
TypeMirror parameterType = getOnlyElement(method.getParameters()).asType();
AnnotationMirror annotationMirror = getAnnotationMirror(method, annotation).get();
AnnotationMirror annotationMirror =
MoreDaggerElements.getAnnotationMirror(method, SUPPORTED_ANNOTATIONS.get(annotation)).get();
TypeMirror mapKeyType =
elements.getTypeElement(injectedTypeFromMapKey(annotationMirror).get()).asType();
if (!types.isAssignable(parameterType, injectorFactoryOf(mapKeyType))) {
Expand All @@ -172,6 +168,6 @@ private DeclaredType injectorFactoryOf(TypeMirror implementationType) {
}

private TypeElement factoryElement() {
return elements.getTypeElement(AndroidInjector.Factory.class.getCanonicalName());
return elements.getTypeElement(TypeNames.ANDROID_INJECTOR_FACTORY.canonicalName());
}
}
3 changes: 1 addition & 2 deletions java/dagger/android/processor/AndroidMapKeys.java
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,14 @@
import static com.google.auto.common.AnnotationMirrors.getAnnotationValue;

import com.google.auto.common.MoreTypes;
import dagger.android.AndroidInjectionKey;
import java.util.Optional;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.TypeMirror;

final class AndroidMapKeys {
/**
* If {@code mapKey} is {@link AndroidInjectionKey}, returns the string value for the map key. If
* If {@code mapKey} is {@code AndroidInjectionKey}, returns the string value for the map key. If
* it's {@link dagger.multibindings.ClassKey}, returns the fully-qualified class name of the
* annotation value. Otherwise returns {@link Optional#empty()}.
*/
Expand Down
2 changes: 1 addition & 1 deletion java/dagger/android/processor/AndroidProcessor.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ public final class AndroidProcessor extends BasicAnnotationProcessor {
"dagger.android.experimentalUseStringKeys";

@Override
protected Iterable<? extends ProcessingStep> initSteps() {
protected Iterable<? extends Step> steps() {
Filer filer = new FormattingFiler(processingEnv.getFiler());
Messager messager = processingEnv.getMessager();
Elements elements = processingEnv.getElementUtils();
Expand Down
26 changes: 5 additions & 21 deletions java/dagger/android/processor/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
# Description:
# Public Dagger API for Android

load("@rules_java//java:defs.bzl", "java_import", "java_library", "java_plugin")
load("@rules_java//java:defs.bzl", "java_library", "java_plugin")
load(
"//:build_defs.bzl",
"DOCLINT_HTML_AND_SYNTAX",
Expand All @@ -38,35 +38,19 @@ java_library(
javacopts = DOCLINT_HTML_AND_SYNTAX + DOCLINT_REFERENCES,
tags = ["maven_coordinates=com.google.dagger:dagger-android-processor:" + POM_VERSION],
deps = [
"//java/dagger:core",
"//java/dagger/internal/guava:base",
"//java/dagger/internal/guava:collect",
"//java/dagger/spi",
"@google_bazel_common//third_party/java/auto:service",
"@google_bazel_common//third_party/java/auto:value",
"@maven//:com_google_auto_auto_common",
"@google_bazel_common//third_party/java/google_java_format",
"@google_bazel_common//third_party/java/incap",
"@google_bazel_common//third_party/java/javapoet",
"@google_bazel_common//third_party/java/google_java_format",
"//java/dagger:core",
"//java/dagger/spi",
# https://github.com/bazelbuild/bazel/issues/2517
":dagger-android-jar",
"@maven//:com_google_auto_auto_common",
],
)

# https://github.com/bazelbuild/bazel/issues/2517
# This target serves two (related) purposes:
# 1. Bazel does not allow a java_library to depend on an android_library, even if that java_library
# will be used in a java_plugin.
# 2. It stores the metadata for the "jarimpl" target that we use to work-around Gradle not loading
# aar artifacts that are declared as deps of an annotation processor. Our pom.xml generator reads
# the tags and includes them apppropriately.
java_import(
name = "dagger-android-jar",
jars = ["//java/dagger/android:libandroid.jar"],
tags = ["maven_coordinates=com.google.dagger:dagger-android-jarimpl:" + POM_VERSION],
visibility = ["//visibility:private"],
)

pom_file(
name = "pom",
artifact_id = "dagger-android-processor",
Expand Down
Loading

0 comments on commit 6da2e7e

Please sign in to comment.