Clarify which argument is meant in an exception message.

"Argument index 1" could plausibly mean the second argument, but "Argument 1 of 3" is more obviously the first. It also makes it easier to count backwards from the end if there are a lot of arguments.

Fixes #1495.

RELNOTES=The exception message for a null value now indicates not only which number argument it is but also how many arguments there are in total. This may lead to a small increase in code size.
PiperOrigin-RevId: 529210616

factory/src/main/java/com/google/auto/factory/processor/FactoryWriter.java

@@ -29,6 +29,7 @@
 import static javax.lang.model.element.Modifier.STATIC;
 
 import com.google.auto.common.MoreTypes;
+import com.google.common.collect.ImmutableCollection;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSetMultimap;
@@ -45,7 +46,6 @@
 import com.squareup.javapoet.TypeSpec;
 import com.squareup.javapoet.TypeVariableName;
 import java.io.IOException;
-import java.util.Iterator;
 import java.util.List;
 import javax.annotation.processing.Filer;
 import javax.annotation.processing.ProcessingEnvironment;
@@ -121,9 +121,10 @@ private void addConstructorAndProviderFields(
     if (descriptor.publicType()) {
       constructor.addModifiers(PUBLIC);
     }
-    Iterator<ProviderField> providerFields = descriptor.providers().values().iterator();
-    for (int argumentIndex = 1; providerFields.hasNext(); argumentIndex++) {
-      ProviderField provider = providerFields.next();
+    ImmutableCollection<ProviderField> providerFields = descriptor.providers().values();
+    int argumentNumber = 0;
+    for (ProviderField provider : providerFields) {
+      ++argumentNumber;
       TypeName typeName = resolveTypeName(provider.key().type().get()).box();
       TypeName providerType = ParameterizedTypeName.get(ClassName.get(Provider.class), typeName);
       factory.addField(providerType, provider.name(), PRIVATE, FINAL);
@@ -132,7 +133,11 @@ private void addConstructorAndProviderFields(
         providerType = providerType.annotated(AnnotationSpec.get(provider.key().qualifier().get()));
       }
       constructor.addParameter(providerType, provider.name());
-      constructor.addStatement("this.$1L = checkNotNull($1L, $2L)", provider.name(), argumentIndex);
+      constructor.addStatement(
+          "this.$1L = checkNotNull($1L, $2L, $3L)",
+          provider.name(),
+          argumentNumber,
+          providerFields.size());
     }
 
     factory.addMethod(constructor.build());
@@ -158,9 +163,13 @@ private void addFactoryMethods(
           methodDescriptor.exceptions().stream().map(TypeName::get).collect(toList()));
       CodeBlock.Builder args = CodeBlock.builder();
       method.addParameters(parameters(methodDescriptor.passedParameters()));
-      Iterator<Parameter> parameters = methodDescriptor.creationParameters().iterator();
-      for (int argumentIndex = 1; parameters.hasNext(); argumentIndex++) {
-        Parameter parameter = parameters.next();
+      ImmutableSet<Parameter> parameters = methodDescriptor.creationParameters();
+      int argumentNumber = 0;
+      String sep = "";
+      for (Parameter parameter : parameters) {
+        ++argumentNumber;
+        args.add(sep);
+        sep = ", ";
         boolean checkNotNull = !parameter.nullable().isPresent();
         CodeBlock argument;
         if (methodDescriptor.passedParameters().contains(parameter)) {
@@ -179,12 +188,10 @@ private void addFactoryMethods(
           }
         }
         if (checkNotNull) {
-          argument = CodeBlock.of("checkNotNull($L, $L)", argument, argumentIndex);
+          argument =
+              CodeBlock.of("checkNotNull($L, $L, $L)", argument, argumentNumber, parameters.size());
         }
         args.add(argument);
-        if (parameters.hasNext()) {
-          args.add(", ");
-        }
       }
       method.addStatement("return new $T($L)", methodDescriptor.returnType(), args.build());
       factory.addMethod(method.build());
@@ -221,9 +228,7 @@ private ImmutableList<ParameterSpec> parameters(Iterable<Parameter> parameters)
     for (Parameter parameter : parameters) {
       TypeName type = resolveTypeName(parameter.type().get());
       ImmutableList<AnnotationSpec> annotations =
-          parameter.annotations().stream()
-              .map(AnnotationSpec::get)
-              .collect(toImmutableList());
+          parameter.annotations().stream().map(AnnotationSpec::get).collect(toImmutableList());
       ParameterSpec parameterSpec =
           ParameterSpec.builder(type, parameter.name()).addAnnotations(annotations).build();
       builder.add(parameterSpec);
@@ -241,13 +246,14 @@ private static void addCheckNotNullMethod(
               .addTypeVariable(typeVariable)
               .returns(typeVariable)
               .addParameter(typeVariable, "reference")
-              .addParameter(TypeName.INT, "argumentIndex")
+              .addParameter(TypeName.INT, "argumentNumber")
+              .addParameter(TypeName.INT, "argumentCount")
               .beginControlFlow("if (reference == null)")
               .addStatement(
-                  "throw new $T($S + argumentIndex)",
+                  "throw new $T($S + argumentNumber + $S + argumentCount)",
                   NullPointerException.class,
-                  "@AutoFactory method argument is null but is not marked @Nullable. Argument "
-                      + "index: ")
+                  "@AutoFactory method argument is null but is not marked @Nullable. Argument ",
+                  " of ")
               .endControlFlow()
               .addStatement("return reference")
               .build());
@@ -269,15 +275,14 @@ private static boolean shouldGenerateCheckNotNull(FactoryDescriptor descriptor)
   }
 
   /**
-   * Returns an appropriate {@code TypeName} for the given type. If the type is an
-   * {@code ErrorType}, and if it is a simple-name reference to one of the {@code *Factory}
-   * classes that we are going to generate, then we return its fully-qualified name. In every other
-   * case we just return {@code TypeName.get(type)}. Specifically, if it is an {@code ErrorType}
-   * referencing some other type, or referencing one of the classes we are going to generate but
-   * using its fully-qualified name, then we leave it as-is. JavaPoet treats {@code TypeName.get(t)}
-   * the same for {@code ErrorType} as for {@code DeclaredType}, which means that if this is a name
-   * that will eventually be generated then the code we write that references the type will in fact
-   * compile.
+   * Returns an appropriate {@code TypeName} for the given type. If the type is an {@code
+   * ErrorType}, and if it is a simple-name reference to one of the {@code *Factory} classes that we
+   * are going to generate, then we return its fully-qualified name. In every other case we just
+   * return {@code TypeName.get(type)}. Specifically, if it is an {@code ErrorType} referencing some
+   * other type, or referencing one of the classes we are going to generate but using its
+   * fully-qualified name, then we leave it as-is. JavaPoet treats {@code TypeName.get(t)} the same
+   * for {@code ErrorType} as for {@code DeclaredType}, which means that if this is a name that will
+   * eventually be generated then the code we write that references the type will in fact compile.
    *
    * <p>A simpler alternative would be to defer processing to a later round if we find an
    * {@code @AutoFactory} class that references undefined types, under the assumption that something

factory/src/test/resources/expected/CheckerFrameworkNullableFactory.java

@@ -36,8 +36,9 @@ final class CheckerFrameworkNullableFactory {
   CheckerFrameworkNullableFactory(
       Provider<String> java_lang_StringProvider,
       Provider<Map.@NullableType Entry<?, ?>> providedNestedNullableTypeProvider) {
-    this.java_lang_StringProvider = checkNotNull(java_lang_StringProvider, 1);
-    this.providedNestedNullableTypeProvider = checkNotNull(providedNestedNullableTypeProvider, 2);
+    this.java_lang_StringProvider = checkNotNull(java_lang_StringProvider, 1, 2);
+    this.providedNestedNullableTypeProvider =
+        checkNotNull(providedNestedNullableTypeProvider, 2, 2);
   }
 
   CheckerFrameworkNullable create(
@@ -53,11 +54,13 @@ CheckerFrameworkNullable create(
         providedNestedNullableTypeProvider.get());
   }
 
-  private static <T> T checkNotNull(T reference, int argumentIndex) {
+  private static <T> T checkNotNull(T reference, int argumentNumber, int argumentCount) {
     if (reference == null) {
       throw new NullPointerException(
-          "@AutoFactory method argument is null but is not marked @Nullable. Argument index: "
-              + argumentIndex);
+          "@AutoFactory method argument is null but is not marked @Nullable. Argument "
+              + argumentNumber
+              + " of "
+              + argumentCount);
     }
     return reference;
   }

factory/src/test/resources/expected/ClassUsingQualifierWithArgsFactory.java

@@ -29,18 +29,20 @@ final class ClassUsingQualifierWithArgsFactory {
   @Inject
   ClassUsingQualifierWithArgsFactory(
       @QualifierWithArgs(name = "Fred", count = 3) Provider<String> providedDepAProvider) {
-    this.providedDepAProvider = checkNotNull(providedDepAProvider, 1);
+    this.providedDepAProvider = checkNotNull(providedDepAProvider, 1, 1);
   }
 
   ClassUsingQualifierWithArgs create() {
-    return new ClassUsingQualifierWithArgs(checkNotNull(providedDepAProvider.get(), 1));
+    return new ClassUsingQualifierWithArgs(checkNotNull(providedDepAProvider.get(), 1, 1));
   }
 
-  private static <T> T checkNotNull(T reference, int argumentIndex) {
+  private static <T> T checkNotNull(T reference, int argumentNumber, int argumentCount) {
     if (reference == null) {
       throw new NullPointerException(
-          "@AutoFactory method argument is null but is not marked @Nullable. Argument index: "
-              + argumentIndex);
+          "@AutoFactory method argument is null but is not marked @Nullable. Argument "
+              + argumentNumber
+              + " of "
+              + argumentCount);
     }
     return reference;
   }

factory/src/test/resources/expected/ConstructorAnnotatedFactory.java

@@ -28,30 +28,32 @@ final class ConstructorAnnotatedFactory {
 
   @Inject
   ConstructorAnnotatedFactory(Provider<Object> objProvider) {
-    this.objProvider = checkNotNull(objProvider, 1);
+    this.objProvider = checkNotNull(objProvider, 1, 1);
   }
 
   ConstructorAnnotated create() {
     return new ConstructorAnnotated();
   }
 
   ConstructorAnnotated create(String s) {
-    return new ConstructorAnnotated(checkNotNull(s, 1));
+    return new ConstructorAnnotated(checkNotNull(s, 1, 1));
   }
 
   ConstructorAnnotated create(int i) {
-    return new ConstructorAnnotated(checkNotNull(objProvider.get(), 1), i);
+    return new ConstructorAnnotated(checkNotNull(objProvider.get(), 1, 2), i);
   }
 
   ConstructorAnnotated create(char c) {
-    return new ConstructorAnnotated(checkNotNull(objProvider.get(), 1), c);
+    return new ConstructorAnnotated(checkNotNull(objProvider.get(), 1, 2), c);
   }
 
-  private static <T> T checkNotNull(T reference, int argumentIndex) {
+  private static <T> T checkNotNull(T reference, int argumentNumber, int argumentCount) {
     if (reference == null) {
       throw new NullPointerException(
-          "@AutoFactory method argument is null but is not marked @Nullable. Argument index: "
-              + argumentIndex);
+          "@AutoFactory method argument is null but is not marked @Nullable. Argument "
+              + argumentNumber
+              + " of "
+              + argumentCount);
     }
     return reference;
   }

factory/src/test/resources/expected/ConstructorAnnotatedNonFinalFactory.java

@@ -28,30 +28,32 @@ class ConstructorAnnotatedNonFinalFactory {
 
   @Inject
   ConstructorAnnotatedNonFinalFactory(Provider<Object> objProvider) {
-    this.objProvider = checkNotNull(objProvider, 1);
+    this.objProvider = checkNotNull(objProvider, 1, 1);
   }
 
   ConstructorAnnotatedNonFinal create() {
     return new ConstructorAnnotatedNonFinal();
   }
 
   ConstructorAnnotatedNonFinal create(String s) {
-    return new ConstructorAnnotatedNonFinal(checkNotNull(s, 1));
+    return new ConstructorAnnotatedNonFinal(checkNotNull(s, 1, 1));
   }
 
   ConstructorAnnotatedNonFinal create(int i) {
-    return new ConstructorAnnotatedNonFinal(checkNotNull(objProvider.get(), 1), i);
+    return new ConstructorAnnotatedNonFinal(checkNotNull(objProvider.get(), 1, 2), i);
   }
 
   ConstructorAnnotatedNonFinal create(char c) {
-    return new ConstructorAnnotatedNonFinal(checkNotNull(objProvider.get(), 1), c);
+    return new ConstructorAnnotatedNonFinal(checkNotNull(objProvider.get(), 1, 2), c);
   }
 
-  private static <T> T checkNotNull(T reference, int argumentIndex) {
+  private static <T> T checkNotNull(T reference, int argumentNumber, int argumentCount) {
     if (reference == null) {
       throw new NullPointerException(
-          "@AutoFactory method argument is null but is not marked @Nullable. Argument index: "
-              + argumentIndex);
+          "@AutoFactory method argument is null but is not marked @Nullable. Argument "
+              + argumentNumber
+              + " of "
+              + argumentCount);
     }
     return reference;
   }

factory/src/test/resources/expected/ConstructorAnnotatedThrowsFactory.java

@@ -29,30 +29,32 @@ final class ConstructorAnnotatedThrowsFactory {
 
   @Inject
   ConstructorAnnotatedThrowsFactory(Provider<Object> objProvider) {
-    this.objProvider = checkNotNull(objProvider, 1);
+    this.objProvider = checkNotNull(objProvider, 1, 1);
   }
 
   ConstructorAnnotatedThrows create() throws IOException, InterruptedException {
     return new ConstructorAnnotatedThrows();
   }
 
   ConstructorAnnotatedThrows create(String s) {
-    return new ConstructorAnnotatedThrows(checkNotNull(s, 1));
+    return new ConstructorAnnotatedThrows(checkNotNull(s, 1, 1));
   }
 
   ConstructorAnnotatedThrows create(int i) throws IOException {
-    return new ConstructorAnnotatedThrows(checkNotNull(objProvider.get(), 1), i);
+    return new ConstructorAnnotatedThrows(checkNotNull(objProvider.get(), 1, 2), i);
   }
 
   ConstructorAnnotatedThrows create(char c) throws InterruptedException {
-    return new ConstructorAnnotatedThrows(checkNotNull(objProvider.get(), 1), c);
+    return new ConstructorAnnotatedThrows(checkNotNull(objProvider.get(), 1, 2), c);
   }
 
-  private static <T> T checkNotNull(T reference, int argumentIndex) {
+  private static <T> T checkNotNull(T reference, int argumentNumber, int argumentCount) {
     if (reference == null) {
       throw new NullPointerException(
-          "@AutoFactory method argument is null but is not marked @Nullable. Argument index: "
-              + argumentIndex);
+          "@AutoFactory method argument is null but is not marked @Nullable. Argument "
+              + argumentNumber
+              + " of "
+              + argumentCount);
     }
     return reference;
   }

factory/src/test/resources/expected/CustomNullableFactory.java

@@ -29,18 +29,20 @@ final class CustomNullableFactory {
 
   @Inject
   CustomNullableFactory(Provider<Object> objectProvider) {
-    this.objectProvider = checkNotNull(objectProvider, 1);
+    this.objectProvider = checkNotNull(objectProvider, 1, 1);
   }
 
   CustomNullable create(@CustomNullable.Nullable String string) {
     return new CustomNullable(string, objectProvider.get());
   }
 
-  private static <T> T checkNotNull(T reference, int argumentIndex) {
+  private static <T> T checkNotNull(T reference, int argumentNumber, int argumentCount) {
     if (reference == null) {
       throw new NullPointerException(
-          "@AutoFactory method argument is null but is not marked @Nullable. Argument index: "
-              + argumentIndex);
+          "@AutoFactory method argument is null but is not marked @Nullable. Argument "
+              + argumentNumber
+              + " of "
+              + argumentCount);
     }
     return reference;
   }

factory/src/test/resources/expected/FactoryImplementingCreateMethod_ConcreteClassFactory.java

@@ -43,18 +43,20 @@ public FactoryImplementingCreateMethod.ConcreteClass create(int aDifferentArgume
   public FactoryImplementingCreateMethod.ConcreteClass create(
       List<Integer> genericWithDifferentArgumentName) {
     return new FactoryImplementingCreateMethod.ConcreteClass(
-        checkNotNull(genericWithDifferentArgumentName, 1));
+        checkNotNull(genericWithDifferentArgumentName, 1, 1));
   }
 
   FactoryImplementingCreateMethod.ConcreteClass create(int a, boolean b) {
     return new FactoryImplementingCreateMethod.ConcreteClass(a, b);
   }
 
-  private static <T> T checkNotNull(T reference, int argumentIndex) {
+  private static <T> T checkNotNull(T reference, int argumentNumber, int argumentCount) {
     if (reference == null) {
       throw new NullPointerException(
-          "@AutoFactory method argument is null but is not marked @Nullable. Argument index: "
-              + argumentIndex);
+          "@AutoFactory method argument is null but is not marked @Nullable. Argument "
+              + argumentNumber
+              + " of "
+              + argumentCount);
     }
     return reference;
   }

factory/src/test/resources/expected/FactoryImplementingGenericInterfaceExtensionFactory.java

@@ -29,24 +29,26 @@ final class FactoryImplementingGenericInterfaceExtensionFactory
 
   @Inject
   FactoryImplementingGenericInterfaceExtensionFactory(Provider<String> sProvider) {
-    this.sProvider = checkNotNull(sProvider, 1);
+    this.sProvider = checkNotNull(sProvider, 1, 1);
   }
 
   FactoryImplementingGenericInterfaceExtension create(Integer i) {
     return new FactoryImplementingGenericInterfaceExtension(
-        checkNotNull(sProvider.get(), 1), checkNotNull(i, 2));
+        checkNotNull(sProvider.get(), 1, 2), checkNotNull(i, 2, 2));
   }
 
   @Override
   public FactoryImplementingGenericInterfaceExtension make(Integer arg) {
     return create(arg);
   }
 
-  private static <T> T checkNotNull(T reference, int argumentIndex) {
+  private static <T> T checkNotNull(T reference, int argumentNumber, int argumentCount) {
     if (reference == null) {
       throw new NullPointerException(
-          "@AutoFactory method argument is null but is not marked @Nullable. Argument index: "
-              + argumentIndex);
+          "@AutoFactory method argument is null but is not marked @Nullable. Argument "
+              + argumentNumber
+              + " of "
+              + argumentCount);
     }
     return reference;
   }