Repository: amzn/ion-js (ts)

Issue:
`IonReader.bigIntValue()` does not always return JSBI values when reading Ion binary
In the process of testing my own code, I noticed that [`IonReader.bigIntValue()`](https://amzn.github.io/ion-js/api/interfaces/_ionreader_.reader.html#bigintvalue) was sometimes returning `number` values, contrary to its contract of only returning JSBI instances or null. Further testing revealed that this only occurred when reading binary data. A minimal failing test case can be found in my fork of this repository here: https://github.com/YourFin/ion-js/commit/03a85a3854a9f5d8457c41e7d011a0fc5e332037

Some spelunking revealed that this is likely a regression introduced by https://github.com/amzn/ion-js/commit/3554f29b438fbc7021cf3760a46775257514be1d, as following the call stack down reveals that `.bigIntValue()` returns [the `any`-typed `_curr` value](https://github.com/amzn/ion-js/blob/6f3ef1670557c410ca8e627696079ca6264d7e90/src/IonParserBinaryRaw.ts#L133) on the underlying `ParserBinaryRaw`. Prior to https://github.com/amzn/ion-js/commit/3554f29b438fbc7021cf3760a46775257514be1d, when `_curr` was updated by [`_readIntegerMagnitude`](https://github.com/amzn/ion-js/commit/3554f29b438fbc7021cf3760a46775257514be1d#diff-efaccdda66ff99d413445232e7630c33a3487359b088b2f95a41a9bb37c198eeR784) (via [`IonParserBinaryRaw.bigIntValue`](https://github.com/amzn/ion-js/blob/6f3ef1670557c410ca8e627696079ca6264d7e90/src/IonParserBinaryRaw.ts#L474) -> [`IonParserBinaryRaw.load_value`](https://github.com/amzn/ion-js/blob/6f3ef1670557c410ca8e627696079ca6264d7e90/src/IonParserBinaryRaw.ts#L805)) it could only be set to JSBI values, but following the commit it could be set to JSBI or `number` values.

I have [what appears to be a fix for this issue](https://github.com/YourFin/ion-js/commit/82c9a437fed8aee63c7d025a17e70552efdeb97e) committed to my fork of this repository; let me know (slack nuckolp@) if you'd like me to cut that as a pr.

Pull request:
Fix #709 Ion binary reader.bigIntValue returning `number`s
*Issue #, if available:*

#709

*Description of changes:*

Fixes a bug where `IonReader.bigIntValue()` would return `number` values when reading binary ion data. See #709 for more details

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.


Patch:
diff --git a/src/IonParserBinaryRaw.ts b/src/IonParserBinaryRaw.ts
index 5269344..40d06c2 100644
--- a/src/IonParserBinaryRaw.ts
+++ b/src/IonParserBinaryRaw.ts
@@ -472,6 +472,10 @@ export class ParserBinaryRaw {
           return null;
         }
         this.load_value();
+        if (!(this._curr instanceof JSBI)) {
+          const num = this._curr!;
+          return JSBI.BigInt(num);
+        }
         return this._curr!;
       default:
         throw new Error(


Test patch:
diff --git a/test/IonReaderConsistency.ts b/test/IonReaderConsistency.ts
index af918eb..c8d91e8 100644
--- a/test/IonReaderConsistency.ts
+++ b/test/IonReaderConsistency.ts
@@ -2,18 +2,19 @@ import {assert} from "chai";
 import * as ion from "../src/Ion";
 import {IonTypes} from "../src/IonTypes";
 import {IonType} from "../src/Ion";
+import JSBI from "jsbi";
 
 describe('IonReaderConsistency', () => {
     let writerTypes = [
-        {name: 'Binary', instance: ion.makeBinaryWriter()},
-        {name: 'Text',   instance: ion.makeTextWriter()},
-        {name: 'Pretty', instance: ion.makePrettyWriter()},
+        {name: 'Binary', mkInstance: () => ion.makeBinaryWriter()},
+        {name: 'Text',   mkInstance: () => ion.makeTextWriter()},
+        {name: 'Pretty', mkInstance: () => ion.makePrettyWriter()},
     ];
 
     // regression test for https://github.com/amzn/ion-js/issues/514
     writerTypes.forEach(writerType => {
-        let writer = writerType.instance;
         it('Reads annotations correctly from structs created by a ' + writerType.name + ' writer', () => {
+            let writer = writerType.mkInstance();
             // writes the following values with the provided writer, sums the 'id' ints
             // within structs annotated with 'foo', and verifies the sum is 26:
             //
@@ -83,6 +84,16 @@ describe('IonReaderConsistency', () => {
 
             assert.equal(sum, 26);
         });
+
+        it('Reads big-ints created by a ' + writerType.name + ' writer', () => {
+            let writer = writerType.mkInstance();
+            writer.writeInt(1);
+            writer.close();
+            let reader = ion.makeReader(writer.getBytes());
+            reader.next();
+            let result = reader.bigIntValue()!;
+            assert.isTrue(JSBI.equal(result, JSBI.BigInt('1')))
+        });
     });
 });