AutoFactory checkNotNull index in error message is 1-indexed, despite java being 0-indexed

[mhansen]

AutoFactory gives an error message

@AutoFactory method argument is null but is not marked @Nullable. Argument index 1

I wasn't sure if this meant the first or second element, as java is 0-indexed. IndexOutOfBoundsException is 0-indexed. But in AutoFactory this turns out to be 1-indexed.

It's a bit confusing.

The problem starts here where we start the loop at 1:

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

Line 162 in 75ef346

for (int argumentIndex = 1; parameters.hasNext(); argumentIndex++) {

:

      for (int argumentIndex = 1; parameters.hasNext(); argumentIndex++) {

Options:

1. Change to 0-indexed? Optionally, also make clear that it's 0-indexed.
2. Make clear in the error message you mean it's 1-indexed?
3. Put the argument name (text) in the error message? It's what AutoValue does, I think. Neatly sidesteps this issue and does not require users to count.

Thoughts?

[eamonnmcmanus]

I went back and looked at the change that introduced these index numbers, and found this from @ronshapiro in the review comments: "I didn't include the actual parameter name for Proguard obfuscation reasons." (cl/126078797 for those who have access.) I wonder if we could do something similar to AutoValue, which has a -A option you can use if you don't want identifiers in the generated code. We'd have to reverse the polarity here, though, since presumably there is lots of Android code that would suddenly get much bigger. Or it could be a new boolean element inside @AutoFactory.

At Google we have @AutoFactory methods with 80 parameters, so telling you that parameter 43 was null isn't all that helpful.

Anyway I think we could at least add text like "first argument is 1" to the message.

[eamonnmcmanus]

Probably just saying "Argument number 1" rather than "Argument index 1" would be enough for the rewording? We often index from 0, but we don't usually number things from 0.

[I'm trying not to be bothered by the wording of the rest of the message. The argument is null, but it's the parameter that is @Nullable.]

[mhansen]

I think number 1 sounds great! I think that is a clever way to avoid confusion

…

On Sat, 29 Apr 2023 at 10:09, Éamonn McManus ***@***.***> wrote: Probably just saying "Argument number 1" rather than "Argument index 1" would be enough for the rewording? We often index from 0, but we don't usually number things from 0. [I'm trying not to be bothered by the wording of the rest of the message. The argument is null, but it's the *parameter* that is @nullable.] — Reply to this email directly, view it on GitHub <#1495 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZYOLMLOOPA5JDMDTCENLXDRLZXANCNFSM6AAAAAAXOXGBT4> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[ronshapiro]

I think "number 1" is also probably a source of confusion, though possibly less. I vote for adding the start index/number is best. Or something like argument 1 of 7

[mhansen]

I think its a good idea to include the total even if it just helps users count back from the end sometimes. “1 of 7” sounds pretty clear to me

…

On Sun, 30 Apr 2023 at 13:29, Ron Shapiro ***@***.***> wrote: I think "number 1" is also probably a source of confusion, though possibly less. I vote for adding the start index/number is best. Or something like argument 1 of 7 — Reply to this email directly, view it on GitHub <#1495 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAZYOK5REBQXDIFNCHIZMLXDXMDBANCNFSM6AAAAAAXOXGBT4> . You are receiving this because you authored the thread.Message ID: ***@***.***>