Skip to content

Commit

Permalink
Propagate parameter annotations in generated factory code.
Browse files Browse the repository at this point in the history
Example:
```java
@autofactory
final class SomeClass {
  ...
  SomeClass(@provided @AQualifier @foo String provideDepA, @bar String depB) {...}
}
```

The generated `SomeClassFactory` will have a `create` method like this:
```java
  SomeClass create(@bar String depB) {...}
```

This was already true if `@Bar` was a `TYPE_USE` annotation, but now it is also true for other annotations.

This may affect compilation. For example, if `@Bar` is Kotlin's `@NotNull`,
this change plugs a hole in Kotlin's nullness checking and will break callers
that were previously passing a `@Nullable` value. The generated factory already
null-checked such parameters, so in Kotlin you can add `!!` with no change in
effect. If the `!!` causes a `NullPointerException`, there would already have
been a `NullPointerException` from the generated factory.

We do not currently copy annotations, other than `@Qualifier` annotations, from `@Provided` parameters to the generated factory constructor. In the example, `@Foo` would not be copied.

PiperOrigin-RevId: 481174930
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Oct 14, 2022
1 parent dffe1f8 commit 4bb91ca
Show file tree
Hide file tree
Showing 5 changed files with 121 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package com.google.auto.factory.processor;

import static com.google.auto.common.GeneratedAnnotationSpecs.generatedAnnotationSpec;
import static com.google.auto.common.MoreStreams.toImmutableList;
import static com.squareup.javapoet.MethodSpec.constructorBuilder;
import static com.squareup.javapoet.MethodSpec.methodBuilder;
import static com.squareup.javapoet.TypeSpec.classBuilder;
Expand All @@ -27,15 +28,12 @@
import static javax.lang.model.element.Modifier.PUBLIC;
import static javax.lang.model.element.Modifier.STATIC;

import com.google.auto.common.AnnotationMirrors;
import com.google.auto.common.AnnotationValues;
import com.google.auto.common.MoreTypes;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.common.collect.Streams;
import com.squareup.javapoet.AnnotationSpec;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
Expand All @@ -47,19 +45,13 @@
import com.squareup.javapoet.TypeSpec;
import com.squareup.javapoet.TypeVariableName;
import java.io.IOException;
import java.lang.annotation.Target;
import java.util.Iterator;
import java.util.List;
import java.util.Optional;
import java.util.stream.Stream;
import javax.annotation.processing.Filer;
import javax.annotation.processing.ProcessingEnvironment;
import javax.inject.Inject;
import javax.inject.Provider;
import javax.lang.model.SourceVersion;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.Element;
import javax.lang.model.element.VariableElement;
import javax.lang.model.type.DeclaredType;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;
Expand Down Expand Up @@ -228,42 +220,17 @@ private ImmutableList<ParameterSpec> parameters(Iterable<Parameter> parameters)
ImmutableList.Builder<ParameterSpec> builder = ImmutableList.builder();
for (Parameter parameter : parameters) {
TypeName type = resolveTypeName(parameter.type().get());
// Remove TYPE_USE annotations, since resolveTypeName will already have included those in
// the TypeName it returns.
List<AnnotationSpec> annotations =
Stream.of(parameter.nullable(), parameter.key().qualifier())
.flatMap(Streams::stream)
.filter(a -> !isTypeUseAnnotation(a))
ImmutableList<AnnotationSpec> annotations =
parameter.annotations().stream()
.map(AnnotationSpec::get)
.collect(toList());
.collect(toImmutableList());
ParameterSpec parameterSpec =
ParameterSpec.builder(type, parameter.name()).addAnnotations(annotations).build();
builder.add(parameterSpec);
}
return builder.build();
}

private static boolean isTypeUseAnnotation(AnnotationMirror mirror) {
Element annotationElement = mirror.getAnnotationType().asElement();
// This is basically equivalent to:
// Target target = annotationElement.getAnnotation(Target.class);
// return target != null
// && Arrays.asList(annotationElement.getAnnotation(Target.class)).contains(TYPE_USE);
// but that might blow up if the annotation is being compiled at the same time and has an
// undefined identifier in its @Target values. The rigmarole below avoids that problem.
Optional<AnnotationMirror> maybeTargetMirror =
Mirrors.getAnnotationMirror(annotationElement, Target.class);
return maybeTargetMirror
.map(
targetMirror ->
AnnotationValues.getEnums(
AnnotationMirrors.getAnnotationValue(targetMirror, "value"))
.stream()
.map(VariableElement::getSimpleName)
.anyMatch(name -> name.contentEquals("TYPE_USE")))
.orElse(false);
}

private static void addCheckNotNullMethod(
TypeSpec.Builder factory, FactoryDescriptor descriptor) {
if (shouldGenerateCheckNotNull(descriptor)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,17 @@
*/
package com.google.auto.factory.processor;

import static com.google.auto.common.MoreStreams.toImmutableList;
import static com.google.auto.factory.processor.Mirrors.unwrapOptionalEquivalence;
import static com.google.auto.factory.processor.Mirrors.wrapOptionalInEquivalence;
import static com.google.common.base.Preconditions.checkArgument;
import static java.util.stream.Collectors.toList;

import com.google.auto.common.AnnotationMirrors;
import com.google.auto.common.MoreElements;
import com.google.auto.common.MoreTypes;
import com.google.auto.value.AutoValue;
import com.google.common.base.Equivalence;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
Expand Down Expand Up @@ -65,26 +66,37 @@ boolean isPrimitive() {

abstract Key key();

/** Annotations on the parameter (not its type). */
abstract ImmutableList<Equivalence.Wrapper<AnnotationMirror>> annotationWrappers();

ImmutableList<AnnotationMirror> annotations() {
return annotationWrappers().stream().map(Equivalence.Wrapper::get).collect(toImmutableList());
}

abstract Optional<Equivalence.Wrapper<AnnotationMirror>> nullableWrapper();

Optional<AnnotationMirror> nullable() {
return unwrapOptionalEquivalence(nullableWrapper());
}

private static Parameter forVariableElement(
VariableElement variable, TypeMirror type, Types types) {
List<AnnotationMirror> annotations =
ImmutableList<AnnotationMirror> annotations =
Stream.of(variable.getAnnotationMirrors(), type.getAnnotationMirrors())
.flatMap(List::stream)
.collect(toList());
.collect(toImmutableList());
Optional<AnnotationMirror> nullable =
annotations.stream().filter(Parameter::isNullable).findFirst();
ImmutableList<Equivalence.Wrapper<AnnotationMirror>> annotationWrappers =
variable.getAnnotationMirrors().stream()
.map(AnnotationMirrors.equivalence()::wrap)
.collect(toImmutableList());

Key key = Key.create(type, annotations, types);
return new AutoValue_Parameter(
MoreTypes.equivalence().wrap(type),
variable.getSimpleName().toString(),
key,
annotationWrappers,
wrapOptionalInEquivalence(AnnotationMirrors.equivalence(), nullable));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -524,6 +524,16 @@ public void generics() {
.hasSourceEquivalentTo(loadExpectedFile("expected/Generics_FooImplWithClassFactory.java"));
}

@Test
public void parameterAnnotations() {
JavaFileObject file = JavaFileObjects.forResource("good/ParameterAnnotations.java");
Compilation compilation = javac.compile(file);
assertThat(compilation).succeededWithoutWarnings();
assertThat(compilation)
.generatedSourceFile("tests.ParameterAnnotationsFactory")
.hasSourceEquivalentTo(loadExpectedFile("expected/ParameterAnnotationsFactory.java"));
}

private JavaFileObject loadExpectedFile(String resourceName) {
if (isJavaxAnnotationProcessingGeneratedAvailable()) {
return JavaFileObjects.forResource(resourceName);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package tests;

import javax.annotation.processing.Generated;
import javax.inject.Inject;
import javax.inject.Provider;

@Generated(
value = "com.google.auto.factory.processor.AutoFactoryProcessor",
comments = "https://github.com/google/auto/tree/master/factory"
)
final class ParameterAnnotationsFactory {
private final Provider<@ParameterAnnotations.NullableType String> fooProvider;

@Inject
ParameterAnnotationsFactory(Provider<@ParameterAnnotations.NullableType String> fooProvider) {
this.fooProvider = checkNotNull(fooProvider, 1);
}

ParameterAnnotations create(@ParameterAnnotations.NullableParameter Integer bar, @ParameterAnnotations.Nullable Long baz, @ParameterAnnotations.NullableType Thread buh) {
return new ParameterAnnotations(checkNotNull(fooProvider.get(), 1), checkNotNull(bar, 2), baz, checkNotNull(buh, 4));
}

private static <T> T checkNotNull(T reference, int argumentIndex) {
if (reference == null) {
throw new NullPointerException("@AutoFactory method argument is null but is not marked @Nullable. Argument index: " + argumentIndex);
}
return reference;
}
}
47 changes: 47 additions & 0 deletions factory/src/test/resources/good/ParameterAnnotations.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package tests;

import static java.lang.annotation.ElementType.PARAMETER;
import static java.lang.annotation.ElementType.TYPE_USE;
import static java.lang.annotation.RetentionPolicy.RUNTIME;

import com.google.auto.factory.AutoFactory;
import com.google.auto.factory.Provided;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;

@AutoFactory
final class ParameterAnnotations {
@Retention(RUNTIME)
@Target(PARAMETER)
@interface NullableParameter {}

// We have special treatment of @Nullable; make sure it doesn't get copied twice.
@Retention(RUNTIME)
@Target(PARAMETER)
@interface Nullable {}

@Retention(RUNTIME)
@Target(TYPE_USE)
@interface NullableType {}

ParameterAnnotations(
@Provided @NullableParameter @NullableType String foo,
@NullableParameter Integer bar,
@Nullable Long baz,
@NullableType Thread buh) {}
}

0 comments on commit 4bb91ca

Please sign in to comment.