Skip to content

Commit

Permalink
Annotate com.google.auto.factory for null hygiene.
Browse files Browse the repository at this point in the history
RELNOTES=n/a
PiperOrigin-RevId: 383872394
  • Loading branch information
eamonnmcmanus authored and Google Java Core Libraries committed Jul 9, 2021
1 parent 895a4d5 commit f09743c
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 18 deletions.
17 changes: 17 additions & 0 deletions factory/src/main/java/com/google/auto/factory/package-info.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/*
* Copyright 2021 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 com.google.auto.factory;

Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.base.Preconditions.checkState;
import static com.google.common.collect.Iterables.getOnlyElement;
import static java.util.Objects.requireNonNull;
import static javax.lang.model.element.ElementKind.PACKAGE;
import static javax.lang.model.util.ElementFilter.typesIn;
import static javax.tools.Diagnostic.Kind.ERROR;
Expand Down Expand Up @@ -111,8 +112,9 @@ Optional<AutoFactoryDeclaration> createIfValid(Element element) {
Mirrors.simplifyAnnotationValueMap(elements.getElementValuesWithDefaults(mirror));
checkState(values.size() == 4);

// className value is a string, so we can just call toString
AnnotationValue classNameValue = values.get("className");
// className value is a string, so we can just call toString. We know values.get("className")
// is non-null because @AutoFactory has an annotation element of that name.
AnnotationValue classNameValue = requireNonNull(values.get("className"));
String className = classNameValue.getValue().toString();
if (!className.isEmpty() && !isValidIdentifier(className)) {
messager.printMessage(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.io.IOException;
import java.util.Arrays;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
Expand Down Expand Up @@ -132,12 +133,17 @@ private void doProcess(RoundEnvironment roundEnv) {
.asMap()
.forEach(
(factoryName, methodDescriptors) -> {
if (methodDescriptors.isEmpty()) {
// This shouldn't happen, but check anyway to avoid an exception for
// methodDescriptors.iterator().next() below.
return;
}
// The sets of classes that are mentioned in the `extending` and `implementing`
// elements, respectively, of the @AutoFactory annotations for this factory.
ImmutableSet.Builder<TypeMirror> extending = newTypeSetBuilder();
ImmutableSortedSet.Builder<TypeMirror> implementing = newTypeSetBuilder();
boolean publicType = false;
Boolean allowSubclasses = null;
Set<Boolean> allowSubclassesSet = new HashSet<>();
boolean skipCreation = false;
for (FactoryMethodDescriptor methodDescriptor : methodDescriptors) {
extending.add(methodDescriptor.declaration().extendingType().asType());
Expand All @@ -146,10 +152,8 @@ private void doProcess(RoundEnvironment roundEnv) {
implementing.add(implementingType.asType());
}
publicType |= methodDescriptor.publicMethod();
if (allowSubclasses == null) {
allowSubclasses = methodDescriptor.declaration().allowSubclasses();
} else if (!allowSubclasses.equals(
methodDescriptor.declaration().allowSubclasses())) {
allowSubclassesSet.add(methodDescriptor.declaration().allowSubclasses());
if (allowSubclassesSet.size() > 1) {
skipCreation = true;
messager.printMessage(
Kind.ERROR,
Expand All @@ -159,6 +163,8 @@ private void doProcess(RoundEnvironment roundEnv) {
methodDescriptor.declaration().valuesMap().get("allowSubclasses"));
}
}
// The set can't be empty because we eliminated methodDescriptors.isEmpty() above.
boolean allowSubclasses = allowSubclassesSet.iterator().next();
if (!skipCreation) {
try {
factoryWriter.writeFactory(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ public boolean matches(char c) {
abstract ImmutableMap<Key, ProviderField> providers();

final AutoFactoryDeclaration declaration() {
return Iterables.getFirst(methodDescriptors(), null).declaration();
// There is always at least one method descriptor.
return methodDescriptors().iterator().next().declaration();
}

private static class UniqueNameSet {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static com.google.auto.common.MoreElements.isAnnotationPresent;
import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.base.Preconditions.checkNotNull;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.partitioningBy;
import static javax.lang.model.element.Modifier.ABSTRACT;
import static javax.lang.model.element.Modifier.PUBLIC;
Expand Down Expand Up @@ -128,10 +129,12 @@ FactoryMethodDescriptor generateDescriptorForConstructor(
Map<Boolean, List<VariableElement>> parameterMap =
constructor.getParameters().stream()
.collect(partitioningBy(parameter -> isAnnotationPresent(parameter, Provided.class)));
// The map returned by partitioningBy always has entries for both key values but our
// null-checker isn't yet smart enough to know that.
ImmutableSet<Parameter> providedParameters =
Parameter.forParameterList(parameterMap.get(true), types);
Parameter.forParameterList(requireNonNull(parameterMap.get(true)), types);
ImmutableSet<Parameter> passedParameters =
Parameter.forParameterList(parameterMap.get(false), types);
Parameter.forParameterList(requireNonNull(parameterMap.get(false)), types);
return FactoryMethodDescriptor.builder(declaration)
.name("create")
.returnType(classElement.asType())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import static com.squareup.javapoet.MethodSpec.constructorBuilder;
import static com.squareup.javapoet.MethodSpec.methodBuilder;
import static com.squareup.javapoet.TypeSpec.classBuilder;
import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import static javax.lang.model.element.Modifier.FINAL;
Expand Down Expand Up @@ -174,7 +175,7 @@ private void addFactoryMethods(
checkNotNull = false;
}
} else {
ProviderField provider = descriptor.providers().get(parameter.key());
ProviderField provider = requireNonNull(descriptor.providers().get(parameter.key()));
argument = CodeBlock.of(provider.name());
if (parameter.isProvider()) {
// Providers are checked for nullness in the Factory's constructor.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import javax.lang.model.element.ExecutableElement;
import javax.lang.model.element.VariableElement;
import javax.lang.model.util.ElementKindVisitor6;
import org.checkerframework.checker.nullness.qual.Nullable;

final class ProvidedChecker {
private final Messager messager;
Expand All @@ -38,28 +39,29 @@ void checkProvidedParameter(Element element) {
checkArgument(
isAnnotationPresent(element, Provided.class), "%s not annoated with @Provided", element);
element.accept(
new ElementKindVisitor6<Void, Void>() {
new ElementKindVisitor6<@Nullable Void, @Nullable Void>() {
@Override
protected Void defaultAction(Element e, Void p) {
protected @Nullable Void defaultAction(Element e, @Nullable Void p) {
throw new AssertionError("Provided can only be applied to parameters");
}

@Override
public Void visitVariableAsParameter(final VariableElement providedParameter, Void p) {
public @Nullable Void visitVariableAsParameter(
VariableElement providedParameter, @Nullable Void p) {
providedParameter
.getEnclosingElement()
.accept(
new ElementKindVisitor6<Void, Void>() {
new ElementKindVisitor6<@Nullable Void, @Nullable Void>() {
@Override
protected Void defaultAction(Element e, Void p) {
protected @Nullable Void defaultAction(Element e, @Nullable Void p) {
raiseError(
providedParameter, "@%s may only be applied to constructor parameters");
return null;
}

@Override
public Void visitExecutableAsConstructor(
ExecutableElement constructor, Void p) {
public @Nullable Void visitExecutableAsConstructor(
ExecutableElement constructor, @Nullable Void p) {
if (!(annotatedWithAutoFactory(constructor)
|| annotatedWithAutoFactory(constructor.getEnclosingElement()))) {
raiseError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,4 @@
* {@link com.google.auto.factory.AutoFactory} API.
*/
package com.google.auto.factory.processor;

0 comments on commit f09743c

Please sign in to comment.