Fixes Ion 1.1 symbols for opcodes E1-E3

[popematt]

Issue #, if available:

None

Description of changes:

Prior to this change, Op codes E1 - E3 were being handled incorrectly because of several problems:

• Ion 1.0 and Ion 1.1 symbol values with SIDs were both being treated as if they were encoded as a (big endian, Ion 1.0) UInts.
• IonReaderContinuableCoreBinary.readFixedUInt_1_1() would start reading at peekIndex and stop at valueMarker.endIndex, but peekindex was not necessarily being set.
• The IonTypeID for Opcode E3 has a length of -1, and when reading the next value, IonCursorBinary was blindly setting valueMarker.endIndex to one less than the start index.

Changes:

• symbolValueId() now branches based on the Ion version. If it is Ion 1.1, it reads FixedUInt rather than UInt.
• IonReaderContinuableCoreBinary.readFixedUInt_1_1() now accepts a start and end index.
• If an Ion 1.1 IonTypeID doesn't have a length prefix (i.e. variableLength == false), and it has length == -1, (and it's not delimited), then slowReadValueHeader() and calculateEndIndex_1_1() now assume it to be a FlexInt or FlexUInt, and the length is calculated by looking at the continuation bits of the value.
• Adds Ion 1.1 SID symbol value test cases to IonReaderContinuableCoreBinaryTest.

Why didn't we notice this sooner? Lack of proper unit testing, and maybe a bit of (bad) luck. Most of our test suite has symbol tables with less than 256 symbols, which means that (a) for the SIDs we were testing, there was no practical difference between big and little endian since they were only one byte long, and (b) we were not exercising opcodes E2 and E3 in any meaningful way.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[tgregg]

Why didn't we notice this sooner?

We just hadn't implemented the feature yet, then we forgot about it. That explains the lack of tests. Everything was just following the existing 1.0 code path.

[tgregg]

It looks like spotless is complaining about SeekableReaderTest in this build. I changed that in #842 on master, and applied our spotless rules to it. Why is it causing a failure on this branch? Spotless always compares against changes from master?

[tgregg]

Approved, noting the ktlint failure.

[popematt]

Spotless always compares against changes from master?

Yes. It does. If I can figure it out, I will change it to use the commit from master that is the closest ancestor of the current commit—that way divergence between master and a feature branch won't cause sudden failures like this.