Annotate com.google.auto.factory for null hygiene.

RELNOTES=n/a PiperOrigin-RevId: 383872394
google · Jul 9, 2021 · f09743c · f09743c
1 parent 895a4d5
commit f09743c
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 18 deletions.
diff --git a/factory/src/main/java/com/google/auto/factory/package-info.java b/factory/src/main/java/com/google/auto/factory/package-info.java
@@ -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;
+
diff --git a/factory/src/main/java/com/google/auto/factory/processor/AutoFactoryDeclaration.java b/factory/src/main/java/com/google/auto/factory/processor/AutoFactoryDeclaration.java
@@ -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;
@@ -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(

diff --git a/factory/src/main/java/com/google/auto/factory/processor/AutoFactoryProcessor.java b/factory/src/main/java/com/google/auto/factory/processor/AutoFactoryProcessor.java
@@ -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;
@@ -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());
@@ -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,
@@ -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(

diff --git a/factory/src/main/java/com/google/auto/factory/processor/FactoryDescriptor.java b/factory/src/main/java/com/google/auto/factory/processor/FactoryDescriptor.java
@@ -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 {

diff --git a/factory/src/main/java/com/google/auto/factory/processor/FactoryDescriptorGenerator.java b/factory/src/main/java/com/google/auto/factory/processor/FactoryDescriptorGenerator.java
@@ -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;
@@ -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())

diff --git a/factory/src/main/java/com/google/auto/factory/processor/FactoryWriter.java b/factory/src/main/java/com/google/auto/factory/processor/FactoryWriter.java
@@ -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;
@@ -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.

diff --git a/factory/src/main/java/com/google/auto/factory/processor/ProvidedChecker.java b/factory/src/main/java/com/google/auto/factory/processor/ProvidedChecker.java
@@ -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;
@@ -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(

diff --git a/factory/src/main/java/com/google/auto/factory/processor/package-info.java b/factory/src/main/java/com/google/auto/factory/processor/package-info.java
@@ -16,3 +16,4 @@
  * {@link com.google.auto.factory.AutoFactory} API.
  */
 package com.google.auto.factory.processor;
+