diff --git a/packages/quicktype-core/src/language/Python/JSONPythonRenderer.ts b/packages/quicktype-core/src/language/Python/JSONPythonRenderer.ts index 20d5fc5aa8..98b44f4db1 100644 --- a/packages/quicktype-core/src/language/Python/JSONPythonRenderer.ts +++ b/packages/quicktype-core/src/language/Python/JSONPythonRenderer.ts @@ -22,7 +22,7 @@ import { UnionMemberMatchTransformer, transformationForType, } from "../../Transformers.js"; -import type { ClassType, Type } from "../../Type/index.js"; +import type { ClassType, Type, UnionType } from "../../Type/index.js"; import { matchType } from "../../Type/TypeUtils.js"; import { PythonRenderer } from "./PythonRenderer.js"; @@ -785,6 +785,21 @@ export class JSONPythonRenderer extends PythonRenderer { return panic(`Transformer ${xfer.kind} is not supported`); } + private unionMembers(unionType: UnionType): Type[] { + // from_float also accepts integers, so from_int must be tried first. + const members = Array.from(unionType.members); + const integerIndex = members.findIndex((m) => m.kind === "integer"); + const doubleIndex = members.findIndex((m) => m.kind === "double"); + + if (doubleIndex >= 0 && integerIndex > doubleIndex) { + const integerType = defined(members[integerIndex]); + members.splice(integerIndex, 1); + members.splice(doubleIndex, 0, integerType); + } + + return members; + } + // Returns the code to deserialize `value` as type `t`. If `t` has // an associated transformer, the code for that transformer is // returned. @@ -837,7 +852,7 @@ export class JSONPythonRenderer extends PythonRenderer { }), (unionType) => { // FIXME: handle via transformers - const deserializers = Array.from(unionType.members).map( + const deserializers = this.unionMembers(unionType).map( (m) => makeLambda(this.deserializer(identity, m)).source, ); return compose(value, (v) => [ @@ -929,7 +944,7 @@ export class JSONPythonRenderer extends PythonRenderer { ")", ]), (unionType) => { - const serializers = Array.from(unionType.members).map( + const serializers = this.unionMembers(unionType).map( (m) => makeLambda(this.serializer(identity, m)).source, ); return compose(value, (v) => [ diff --git a/test/inputs/schema/integer-before-number.1.json b/test/inputs/schema/integer-before-number.1.json new file mode 100644 index 0000000000..7ed6ff82de --- /dev/null +++ b/test/inputs/schema/integer-before-number.1.json @@ -0,0 +1 @@ +5 diff --git a/test/inputs/schema/integer-before-number.2.json b/test/inputs/schema/integer-before-number.2.json new file mode 100644 index 0000000000..9ad974f610 --- /dev/null +++ b/test/inputs/schema/integer-before-number.2.json @@ -0,0 +1 @@ +5.5 diff --git a/test/inputs/schema/integer-before-number.3.fail.union.json b/test/inputs/schema/integer-before-number.3.fail.union.json new file mode 100644 index 0000000000..c10b1ae65f --- /dev/null +++ b/test/inputs/schema/integer-before-number.3.fail.union.json @@ -0,0 +1 @@ +"not a number" diff --git a/test/inputs/schema/integer-before-number.schema b/test/inputs/schema/integer-before-number.schema new file mode 100644 index 0000000000..5d8aa8914c --- /dev/null +++ b/test/inputs/schema/integer-before-number.schema @@ -0,0 +1,5 @@ +{ + "$schema": "http://json-schema.org/draft-06/schema#", + "description": "An integer or non-integer number", + "type": ["integer", "number"] +} diff --git a/test/languages.ts b/test/languages.ts index 70fa741ebf..5fb848dff2 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -121,6 +121,7 @@ export const CSharpLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "top-level-enum.schema", // The code we generate for top-level enums is incompatible with the driver ], // The default framework is SystemTextJson; this fixture deliberately @@ -167,6 +168,7 @@ export const CSharpLanguageSystemTextJson: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "top-level-enum.schema", // The code we generate for top-level enums is incompatible with the driver // The following skips are pre-existing System.Text.Json renderer issues, // found when first enabling the schema fixture for this language: @@ -218,7 +220,10 @@ export const JavaLanguage: Language = { "nst-test-suite.json", ], skipMiscJSON: false, - skipSchema: ["keyword-unions.schema"], // generates classes with names that are case-insensitively equal + skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. + "keyword-unions.schema", // generates classes with names that are case-insensitively equal + ], rendererOptions: {}, // The default is array-type=list; this keeps the T[] code path // covered. @@ -229,6 +234,7 @@ export const JavaLanguage: Language = { export const JavaLanguageWithLegacyDateTime: Language = { ...JavaLanguage, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. ...JavaLanguage.skipSchema, "date-time.schema", // Expects less strict serialization. ], @@ -327,7 +333,7 @@ export const RustLanguage: Language = { output: "module_under_test.rs", topLevel: "TopLevel", skipJSON: [], - skipSchema: [], + skipSchema: ["integer-before-number.schema"], // Python-specific union-order regression. skipMiscJSON: false, rendererOptions: {}, quickTestRendererOptions: [ @@ -377,6 +383,7 @@ export const CrystalLanguage: Language = { "e8b04.json", ], skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // Crystal does not handle enum mapping ...skipsEnumValueValidation, // Crystal does not support top-level primitives @@ -473,6 +480,7 @@ export const RubyLanguage: Language = { "kitchen-sink.json", ], skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // We don't generate a convenience method for top-level enums "top-level-enum.schema", ], @@ -514,6 +522,7 @@ export const GoLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // can't differenciate empty array and nothing for optional empty array // (omitempty). "postman-collection.schema", @@ -590,6 +599,7 @@ export const CJSONLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. /* Member names are different when generating with schema */ "vega-lite.schema", /* Enum as TopLevel is not supported */ @@ -650,7 +660,7 @@ export const CJSONDefaultLanguage: Language = { topLevel: "TopLevel", includeJSON: ["nbl-stats.json"], skipMiscJSON: true, - skipSchema: [], + skipSchema: ["integer-before-number.schema"], // Python-specific union-order regression. rendererOptions: {}, quickTestRendererOptions: [], sourceFiles: ["src/language/CJSON/index.ts"], @@ -674,7 +684,7 @@ export const CJSONMultiHeaderLanguage: Language = { topLevel: "TopLevel", includeJSON: ["nbl-stats.json"], skipMiscJSON: true, - skipSchema: [], + skipSchema: ["integer-before-number.schema"], // Python-specific union-order regression. rendererOptions: { "source-style": "multi-source" }, quickTestRendererOptions: [], sourceFiles: ["src/language/CJSON/index.ts"], @@ -698,7 +708,7 @@ export const CJSONMultiSplitLanguage: Language = { topLevel: "TopLevel", includeJSON: ["nbl-stats.json"], skipMiscJSON: true, - skipSchema: [], + skipSchema: ["integer-before-number.schema"], // Python-specific union-order regression. rendererOptions: { "source-style": "multi-source", "header-only": "false", @@ -752,6 +762,7 @@ export const CPlusPlusLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // uses too much memory "keyword-unions.schema", // The generated deserializer accepts non-object values when all class properties are optional. @@ -838,6 +849,7 @@ export const ElmLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "union-list.schema", // recursion "list.schema", // recursion "ref-remote.schema", // recursion @@ -912,6 +924,7 @@ export const SwiftLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // The code we generate for top-level enums is incompatible with the driver "top-level-enum.schema", // This works on macOS, but on Linux one of the failure test cases doesn't fail @@ -977,7 +990,7 @@ export const ObjectiveCLanguage: Language = { "combinations4.json", ], skipMiscJSON: false, - skipSchema: [], + skipSchema: ["integer-before-number.schema"], // Python-specific union-order regression. rendererOptions: { functions: "true" }, quickTestRendererOptions: [], sourceFiles: ["src/language/Objective-C/index.ts"], @@ -1016,6 +1029,7 @@ export const TypeScriptLanguage: Language = { skipJSON: [], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "keyword-unions.schema", // can't handle "constructor" property // Pre-existing failures (this fixture is not in CI yet, and these // fail with unmodified master too): objects with both declared @@ -1058,7 +1072,10 @@ export const JavaScriptLanguage: Language = { topLevel: "TopLevel", skipJSON: [], skipMiscJSON: false, - skipSchema: ["keyword-unions.schema"], // can't handle "constructor" property + skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. + "keyword-unions.schema", // can't handle "constructor" property + ], rendererOptions: {}, quickTestRendererOptions: [ { "runtime-typecheck": "false" }, @@ -1089,7 +1106,7 @@ export const JavaScriptPropTypesLanguage: Language = { "spotify-album.json", // renderer does not support recursion "76ae1.json", // renderer does not support recursion ], - skipSchema: [], + skipSchema: ["integer-before-number.schema"], // Python-specific union-order regression. skipMiscJSON: false, rendererOptions: { "module-system": "es6" }, quickTestRendererOptions: [{ converters: "top-level" }], @@ -1111,6 +1128,7 @@ export const FlowLanguage: Language = { skipJSON: [], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "keyword-unions.schema", // can't handle "constructor" property ], rendererOptions: { "explicit-unions": "yes" }, @@ -1167,6 +1185,7 @@ export const Scala3Language: Language = { "bug2521.json", ], skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // Same raw-identifier limitation as in skipJSON: a property // named "_" and classes shadowing None/Option don't compile. "keyword-unions.schema", @@ -1217,6 +1236,7 @@ export const Scala3UpickleLanguage: Language = { "bug2521.json", ], skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // Same raw-identifier limitation as in skipJSON: a property // named "_" and classes shadowing None/Option don't compile. "keyword-unions.schema", @@ -1287,7 +1307,7 @@ I havea no idea how to encode these tests correctly. "php-mixed-union.json", "nst-test-suite.json", ], - skipSchema: [], + skipSchema: ["integer-before-number.schema"], // Python-specific union-order regression. skipMiscJSON: false, rendererOptions: { "just-types": "true" }, quickTestRendererOptions: [], @@ -1349,6 +1369,7 @@ export const KotlinLanguage: Language = { "bug427.json", ], skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // Very weird - the types are correct, but it can (de)serialize the string, // which is not represented in the types (implicit-class-array-union); // class-map-union: KlaxonException: Couldn't find a suitable constructor for class UnionValue to initialize with {} @@ -1440,6 +1461,7 @@ export const KotlinJacksonLanguage: Language = { "bug427.json", ], skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // Very weird - the types are correct, but it can (de)serialize the string, // which is not represented in the types (implicit-class-array-union); // class-map-union: KlaxonException: Couldn't find a suitable constructor for class UnionValue to initialize with {} @@ -1575,6 +1597,7 @@ export const KotlinXLanguage: Language = { "identifiers.json", ], skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // Unions render as sealed classes without serializer wiring, so // deserialization fails at runtime (documented TODO in // KotlinXRenderer.ts). @@ -1644,6 +1667,7 @@ export const DartLanguage: Language = { "keywords.json", ], skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "enum-with-null.schema", // Deliberately NOT ...skipsEnumValueValidation: Dart runs // optional-enum.schema as its own regression test (see PR #2720), @@ -1712,6 +1736,7 @@ export const PikeLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "top-level-enum.schema", // output generated properly, but not a class "keyword-unions.schema", // seems like a problem with deserializing // no implicit cast int <-> float in Pike @@ -1798,6 +1823,7 @@ export const HaskellLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "any.schema", ...skipsUntypedUnions, // The test driver encodes the Maybe result, so a failed decode prints @@ -1854,6 +1880,7 @@ export const PHPLanguage: Language = { ], skipMiscJSON: true, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // PHP class names are case-insensitive, but the namer dedups // case-sensitively, so this declares classes that collide (same // reason Java and Python skip it). @@ -1963,6 +1990,7 @@ export const TypeScriptZodLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "any.schema", ...skipsUntypedUnions, "direct-union.schema", @@ -2084,6 +2112,7 @@ export const TypeScriptEffectSchemaLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. "any.schema", ...skipsUntypedUnions, "direct-union.schema", @@ -2144,6 +2173,7 @@ export const ElixirLanguage: Language = { ], skipMiscJSON: false, skipSchema: [ + "integer-before-number.schema", // Python-specific union-order regression. // The error occurs because a guard clause that references TopLevel is compiled before TopLevel itself. To fix this, put // TopLevel before Bar, but this doesn't address the actual problem if for example a pattern match to Bar was in TopLevel. "mutually-recursive.schema", diff --git a/test/unit/python-union-order.test.ts b/test/unit/python-union-order.test.ts new file mode 100644 index 0000000000..9487c2ff72 --- /dev/null +++ b/test/unit/python-union-order.test.ts @@ -0,0 +1,26 @@ +import { + InputData, + JSONSchemaInput, + quicktype, +} from "../../packages/quicktype-core/src/index.js"; +import { expect, test } from "vitest"; + +const schema = JSON.stringify({ + type: ["integer", "number"], +}); + +// Schema fixtures use JSON.parse, which unifies JSON integers and floats as +// JavaScript numbers (5 and 5.0 compare identically), so assert source order. +test("Python tries integer before number in unions", async () => { + const schemaInput = new JSONSchemaInput(undefined); + await schemaInput.addSource({ name: "TopLevel", schema }); + + const inputData = new InputData(); + inputData.addInput(schemaInput); + + const result = await quicktype({ inputData, lang: "python" }); + const output = result.lines.join("\n"); + + expect(output).toContain("from_union([from_int, from_float]"); + expect(output).toContain("from_union([from_int, to_float]"); +});