[Breaking change request] RuneIterator.current and .rawIndex no longer returns null.

[lrhn]

Summary

The RuneIterator class has some members which return null in the state where the iterator has no "current" element.

Those are members which are not intended to be used in that state, and which has no right value to return. The Iterator.current getter inherited from Iterator has this issue in general, and in the Null Safe SDK, the Iterator.current getter is no longer required to return null when there is no current element.

What is changing:

The RuneIterator now returns the following values when it has no current rune (before calling moveNext at the beginning or after calling reset, or after moveNext has returned false):

• current: -1
• rawIndex: -1
• currentAsString: empty string.

Why is this changing?

Because the alternative would be to make the return type nullable.
For current, even that is not possible because Iterable<T>.current returns T, and overriding it with something returning T? is not allowed.
Making the other return values nullable is highly annoying for users who only use these getters in the state where they are intended to be used, and where they have a meaningful value to return. Those users would have to check for null on every access, even though they know it will succeed.

Expected impact

Little to none. The [RuneIterator] class is not heavily used, and when iterated using for/in or other normal iteration methods, the getters in question are not used at all.

If someone is using those getters, and they are depending on reading them when there is no current value and using the the null to detect that there is no value, they will now fail.

The impact is well worth the API improvement for Null Safe code.

[franklinyow]

cc @Hixie @matanlurey @dgrove @vsmenon for review and approval.

[Hixie]

LGTM with the same caveats as other similar changes (that we need to be ready to reconsider if the impact is found to be large).

[vsmenon]

lgtm

[matanlurey]

LGTM

[franklinyow]

Approved

[franklinyow]

Landed. Close as discussed in SCRUM

[natebosch]

The discussed alternative is a nullable type. Have we had a discussion about throwing instead?

It turns out that -1 happens to be easier to use for the code I had to migrate that was broken by this - but that's because this code is written in a really weird way and isn't using moveNext() the way we really should be. While trying to learn what this code was doing and fix it though, I was really surprised by the -1 - an exception with a clear message would be more actionable and the bugs less subtle.

dart-lang/test#1198