Update JSON path after promoteNameToValue

[tanvir-ux]

Fixes #1768.

MapTypeAdapterFactory reads object-encoded keys through promoteNameToValue, which turned a property name into a value without recording it in pathNames. After the key was consumed, getPath() stayed at $., so a later parse error (for example an unterminated string value) omitted the property name.

JsonReader now records the promoted name once it is actually read, and JsonTreeReader.promoteNameToValue writes the name into the path immediately. Duplicate-key messages therefore include $.key as well.

I already have a Google CLA on file (auto#2123).

[eamonnmcmanus]

Thanks for doing this! Just one small request.

I believe @Marcono1234 is familiar with the issues here and may want to weigh in.

[eamonnmcmanus]

I think this would be slightly easier to read like this:

String s1 = reader.nextString();
assertThat(s1).isEqualTo("name");
assertThat(reader.getPreviousPath()).isEqualTo("$.name");

The idea being to avoid calling methods with side-effects inside assertThat. As written the difference between what's happening in the first assertion and in the second is not obvious.

[tanvir-ux]

Split the assertions as requested so nextString() is no longer called inside assertThat.

[tanvir-ux]

Thanks a lot, @eamonnmcmanus 🙂

[Marcono1234]

I believe @Marcono1234 is familiar with the issues here and may want to weigh in.

Thanks a lot for this pull request! Looks good to me and I think you covered all corner cases.

I am wondering though if the recordPromotedName calls for case PEEKED_LONG are redundant; have created #3117 for that. @tanvir-ux, please let me know what you think, and whether I am overlooking something.

(Sorry for this late comment.)

[tanvir-ux]

Thanks @Marcono1234 — you're not overlooking anything.

promoteNameToValue only ever leaves a name token (string forms), so PEEKED_LONG can't happen while promotedName is set. The Long.toString(peekedLong) calls were dead for that path and just added cost. #3117 looks right to me (and the "1" / nextLong coverage is a nice addition).