Merge pull request #2938 from glideapps/fix/2931-int64-range-overflow

fix(core): infer whole numbers outside the target's integer range as double

Author: schani

packages/quicktype-core/src/TargetLanguage.ts

@@ -15,6 +15,7 @@ import type { Type } from "./Type/Type.js";
 import type { StringTypeMapping } from "./Type/TypeBuilderUtils.js";
 import type { TypeGraph } from "./Type/TypeGraph.js";
 import type { Comment } from "./support/Comments.js";
+import { INT64_RANGE, type IntegerRange } from "./support/IntegerRange.js";
 import { defined } from "./support/Support.js";
 import type { LanguageName, RendererOptions } from "./types.js";
 
@@ -121,4 +122,22 @@ export abstract class TargetLanguage<
     public get dateTimeRecognizer(): DateTimeRecognizer {
         return new DefaultDateTimeRecognizer();
     }
+
+    /**
+     * The inclusive range of whole numbers in input JSON that quicktype
+     * infers as the language's integer type.  Whole numbers outside the
+     * range are inferred as `double` instead, because the integer type
+     * could not round-trip them (issue #2931).  `null` means the
+     * language's integers are arbitrary-precision.
+     *
+     * Languages whose integer width depends on a renderer option (like
+     * cJSON's `integer-size`) override this and inspect
+     * `rendererOptions`, which are the same untyped option values that
+     * `makeRenderer` receives.
+     */
+    public getSupportedIntegerRange(
+        _rendererOptions: Record<string, unknown> = {},
+    ): IntegerRange | null {
+        return INT64_RANGE;
+    }
 }

packages/quicktype-core/src/index.ts

@@ -16,6 +16,15 @@ export {
     type InferenceFlagName,
 } from "./Inference.js";
 export { CompressedJSON, type Value } from "./input/CompressedJSON.js";
+export {
+    INT8_RANGE,
+    INT16_RANGE,
+    INT32_RANGE,
+    INT64_RANGE,
+    type IntegerRange,
+    JS_SAFE_INTEGER_RANGE,
+    integerStringInRange,
+} from "./support/IntegerRange.js";
 export {
     type Input,
     InputData,

packages/quicktype-core/src/input/CompressedJSON.ts

@@ -2,6 +2,11 @@ import { addHashCode, hashCodeInit, hashString } from "collection-utils";
 
 import { inferTransformedStringTypeKindForString } from "../attributes/StringTypes.js";
 import type { DateTimeRecognizer } from "../DateTime.js";
+import {
+    INT64_RANGE,
+    type IntegerRange,
+    integerStringInRange,
+} from "../support/IntegerRange.js";
 import { assert, defined, panic } from "../support/Support.js";
 import {
     type TransformedStringTypeKind,
@@ -66,13 +71,50 @@ export abstract class CompressedJSON<T> {
 
     private readonly _arrays: Value[][] = [];
 
+    /**
+     * `supportedIntegerRange` is the range of whole numbers in the input
+     * that get inferred as `integer`; whole numbers outside it are inferred
+     * as `double`, because the target language's integer type could not
+     * round-trip them.  `null` means the target's integers are
+     * arbitrary-precision.  See `TargetLanguage.getSupportedIntegerRange`.
+     */
     public constructor(
         public readonly dateTimeRecognizer: DateTimeRecognizer,
         public readonly handleRefs: boolean,
+        public readonly supportedIntegerRange: IntegerRange | null = INT64_RANGE,
     ) {}
 
     public abstract parse(input: T): Promise<Value>;
 
+    /**
+     * Whether a whole number in the input, given as the decimal string of
+     * its JSON literal, fits `supportedIntegerRange`.  Works on the digit
+     * string because such literals can exceed what a JavaScript number can
+     * represent exactly.
+     */
+    protected integerStringFits(integerString: string): boolean {
+        const range = this.supportedIntegerRange;
+        if (range === null) return true;
+        return integerStringInRange(integerString, range);
+    }
+
+    /**
+     * Whether a number that `JSON.parse` produced should be inferred as
+     * `double`.  The original literal is gone at this point, but for whole
+     * numbers below 1e21, `toFixed(0)` gives the exact decimal value of the
+     * double, and it errs on the right side at range boundaries: a literal
+     * like 9223372036854775807 (INT64_MAX) parses to the double
+     * 9223372036854775808, which is correctly outside the int64 range.  At
+     * 1e21 doubles are far beyond any fixed-size integer type and `toFixed`
+     * switches to exponential notation, so those are doubles outright.
+     */
+    protected parsedNumberIsDouble(n: number): boolean {
+        if (n !== Math.floor(n)) return true;
+        if (this.supportedIntegerRange === null) return false;
+        if (Math.abs(n) >= 1e21) return true;
+        return !this.integerStringFits(n.toFixed(0));
+    }
+
     public parseSync(_input: T): Value {
         return panic("parseSync not implemented in CompressedJSON");
     }
@@ -340,11 +382,7 @@ export class CompressedJSONFromString extends CompressedJSON<string> {
         } else if (typeof json === "string") {
             this.commitString(json);
         } else if (typeof json === "number") {
-            const isDouble =
-                json !== Math.floor(json) ||
-                json < Number.MIN_SAFE_INTEGER ||
-                json > Number.MAX_SAFE_INTEGER;
-            this.commitNumber(isDouble);
+            this.commitNumber(this.parsedNumberIsDouble(json));
         } else if (Array.isArray(json)) {
             this.pushArrayContext();
             for (const v of json) {

packages/quicktype-core/src/input/Inputs.ts

@@ -206,6 +206,7 @@ export function jsonInputForTargetLanguage(
     targetLanguage: LanguageName | TargetLanguage,
     languages?: TargetLanguage[],
     handleJSONRefs = false,
+    rendererOptions: Record<string, unknown> = {},
 ): JSONInput<string> {
     if (typeof targetLanguage === "string") {
         targetLanguage = defined(languageNamed(targetLanguage, languages));
@@ -214,6 +215,7 @@ export function jsonInputForTargetLanguage(
     const compressedJSON = new CompressedJSONFromString(
         targetLanguage.dateTimeRecognizer,
         handleJSONRefs,
+        targetLanguage.getSupportedIntegerRange(rendererOptions),
     );
     return new JSONInput(compressedJSON);
 }

packages/quicktype-core/src/language/CJSON/language.ts

@@ -27,6 +27,13 @@ import {
     StringOption,
     getOptionValues,
 } from "../../RendererOptions/index.js";
+import {
+    INT8_RANGE,
+    INT16_RANGE,
+    INT32_RANGE,
+    INT64_RANGE,
+    type IntegerRange,
+} from "../../support/IntegerRange.js";
 import { TargetLanguage } from "../../TargetLanguage.js";
 import type { LanguageName, RendererOptions } from "../../types.js";
 
@@ -142,6 +149,28 @@ export class CJSONTargetLanguage extends TargetLanguage<
         return true;
     }
 
+    /**
+     * Return the range of whole numbers the generated integer type can
+     * represent, which depends on the `integer-size` renderer option
+     * (int64_t by default)
+     * @param rendererOptions: untyped renderer option values
+     * @return the range of the configured integer type
+     */
+    public getSupportedIntegerRange(
+        rendererOptions: Record<string, unknown> = {},
+    ): IntegerRange | null {
+        switch (cJSONOptions.typeIntegerSize.getValue(rendererOptions)) {
+            case "int8_t":
+                return INT8_RANGE;
+            case "int16_t":
+                return INT16_RANGE;
+            case "int32_t":
+                return INT32_RANGE;
+            default:
+                return INT64_RANGE;
+        }
+    }
+
     /**
      * Indicate if language support optional class properties
      * @return true

packages/quicktype-core/src/language/Crystal/language.ts

@@ -1,4 +1,5 @@
 import type { RenderContext } from "../../Renderer.js";
+import { INT32_RANGE, type IntegerRange } from "../../support/IntegerRange.js";
 import { TargetLanguage } from "../../TargetLanguage.js";
 
 import { CrystalRenderer } from "./CrystalRenderer.js";
@@ -16,6 +17,11 @@ export class CrystalTargetLanguage extends TargetLanguage<
         super(crystalLanguageConfig);
     }
 
+    // The Crystal renderer emits `Int32` for inferred integers.
+    public getSupportedIntegerRange(): IntegerRange | null {
+        return INT32_RANGE;
+    }
+
     protected makeRenderer(renderContext: RenderContext): CrystalRenderer {
         return new CrystalRenderer(this, renderContext);
     }

packages/quicktype-core/src/language/Elixir/language.ts

@@ -4,6 +4,7 @@ import {
     StringOption,
     getOptionValues,
 } from "../../RendererOptions/index.js";
+import type { IntegerRange } from "../../support/IntegerRange.js";
 import { TargetLanguage } from "../../TargetLanguage.js";
 import type { LanguageName, RendererOptions } from "../../types.js";
 
@@ -28,6 +29,11 @@ export const elixirLanguageConfig = {
 export class ElixirTargetLanguage extends TargetLanguage<
     typeof elixirLanguageConfig
 > {
+    // Elixir's integers are arbitrary-precision.
+    public getSupportedIntegerRange(): IntegerRange | null {
+        return null;
+    }
+
     public constructor() {
         super(elixirLanguageConfig);
     }

packages/quicktype-core/src/language/Elm/language.ts

@@ -5,6 +5,10 @@ import {
     StringOption,
     getOptionValues,
 } from "../../RendererOptions/index.js";
+import {
+    type IntegerRange,
+    JS_SAFE_INTEGER_RANGE,
+} from "../../support/IntegerRange.js";
 import { TargetLanguage } from "../../TargetLanguage.js";
 import type { LanguageName, RendererOptions } from "../../types.js";
 
@@ -55,6 +59,12 @@ export class ElmTargetLanguage extends TargetLanguage<
         return true;
     }
 
+    // Elm compiles to JavaScript, where `Int` is an IEEE-754 double at
+    // runtime, so integers are only exact within the JS safe range.
+    public getSupportedIntegerRange(): IntegerRange | null {
+        return JS_SAFE_INTEGER_RANGE;
+    }
+
     protected makeRenderer<Lang extends LanguageName = "elm">(
         renderContext: RenderContext,
         untypedOptionValues: RendererOptions<Lang>,

packages/quicktype-core/src/language/JSONSchema/language.ts

@@ -1,4 +1,5 @@
 import type { RenderContext } from "../../Renderer.js";
+import type { IntegerRange } from "../../support/IntegerRange.js";
 import { TargetLanguage } from "../../TargetLanguage.js";
 import {
     type StringTypeMapping,
@@ -17,6 +18,11 @@ export const JSONSchemaLanguageConfig = {
 export class JSONSchemaTargetLanguage extends TargetLanguage<
     typeof JSONSchemaLanguageConfig
 > {
+    // JSON Schema's `integer` type is unbounded.
+    public getSupportedIntegerRange(): IntegerRange | null {
+        return null;
+    }
+
     public constructor() {
         super(JSONSchemaLanguageConfig);
     }

packages/quicktype-core/src/language/JavaScript/language.ts

@@ -6,6 +6,10 @@ import {
 } from "../../RendererOptions/index.js";
 import { AcronymStyleOptions, acronymOption } from "../../support/Acronyms.js";
 import { convertersOption } from "../../support/Converters.js";
+import {
+    JS_SAFE_INTEGER_RANGE,
+    type IntegerRange,
+} from "../../support/IntegerRange.js";
 import { TargetLanguage } from "../../TargetLanguage.js";
 import type {
     PrimitiveStringTypeKind,
@@ -51,6 +55,10 @@ export const javaScriptLanguageConfig = {
 export class JavaScriptTargetLanguage extends TargetLanguage<
     typeof javaScriptLanguageConfig
 > {
+    public getSupportedIntegerRange(): IntegerRange | null {
+        return JS_SAFE_INTEGER_RANGE;
+    }
+
     public constructor() {
         super(javaScriptLanguageConfig);
     }