diff --git a/packages/quicktype-core/src/attributes/PropertyNames.ts b/packages/quicktype-core/src/attributes/PropertyNames.ts new file mode 100644 index 000000000..827bc68ae --- /dev/null +++ b/packages/quicktype-core/src/attributes/PropertyNames.ts @@ -0,0 +1,54 @@ +import { setIntersect, setUnion } from "collection-utils"; + +import type { Type, TypeKind } from "../Type/index.js"; + +import { TypeAttributeKind } from "./TypeAttributes.js"; + +/** + * The keys a JSON Schema `propertyNames` constraint allows, when it restricts + * them to a finite set of strings. Kept as plain data rather than a graph type + * so unrelated renderers never see a key enum they can't use. + */ +class PropertyNamesTypeAttributeKind extends TypeAttributeKind< + ReadonlySet +> { + public constructor() { + super("propertyNames"); + } + + public appliesToTypeKind(kind: TypeKind): boolean { + return kind === "object" || kind === "class" || kind === "map"; + } + + // A union of objects allows the union of their keys. + public combine(attrs: Array>): ReadonlySet { + return setUnion(...attrs); + } + + // Only keys every side allows survive; empty means a contradiction we + // don't try to express. + public intersect( + attrs: Array>, + ): ReadonlySet | undefined { + const cases = attrs.reduce(setIntersect); + return cases.size === 0 ? undefined : cases; + } + + // A schema-stated fact, not a weakening guess, so it stays as-is. + public makeInferred(cases: ReadonlySet): ReadonlySet { + return cases; + } + + public stringify(cases: ReadonlySet): string { + return `propertyNames: ${Array.from(cases).join(", ")}`; + } +} + +export const propertyNamesTypeAttributeKind: TypeAttributeKind< + ReadonlySet +> = new PropertyNamesTypeAttributeKind(); + +/** The keys `t` restricts its properties to, if it restricts them. */ +export function propertyNamesForType(t: Type): ReadonlySet | undefined { + return propertyNamesTypeAttributeKind.tryGetInAttributes(t.getAttributes()); +} diff --git a/packages/quicktype-core/src/input/JSONSchemaInput.ts b/packages/quicktype-core/src/input/JSONSchemaInput.ts index 38d451ffa..d6b37ae1f 100644 --- a/packages/quicktype-core/src/input/JSONSchemaInput.ts +++ b/packages/quicktype-core/src/input/JSONSchemaInput.ts @@ -32,6 +32,7 @@ import { import { defaultValueAttributeProducer } from "../attributes/DefaultValue.js"; import { descriptionAttributeProducer } from "../attributes/Description.js"; import { enumValuesAttributeProducer } from "../attributes/EnumValues.js"; +import { propertyNamesTypeAttributeKind } from "../attributes/PropertyNames.js"; import { StringTypes } from "../attributes/StringTypes.js"; import { type TypeAttributes, @@ -799,6 +800,23 @@ class Resolver { } } +/** The strings a schema restricts to, if it's a finite set. A mixed-type + * `enum` doesn't count, since the non-string cases can't be property names. */ +function finiteStringCases( + schema: JSONSchema, +): ReadonlySet | undefined { + if (typeof schema !== "object" || schema === null) return undefined; + if (typeof schema.const === "string") return new Set([schema.const]); + if (!Array.isArray(schema.enum)) return undefined; + + const cases = schema.enum.filter((c) => typeof c === "string"); + if (cases.length === 0 || cases.length !== schema.enum.length) { + return undefined; + } + + return new Set(cases); +} + async function addTypesInSchema( resolver: Resolver, typeBuilder: TypeBuilder, @@ -827,6 +845,7 @@ async function addTypesInSchema( additionalProperties: unknown, sortKey: (k: string) => number | string = (k: string): string => k.toLowerCase(), + propertyNames?: ReadonlySet, ): Promise { const required = new Set(requiredArray); const propertiesMap = mapSortBy(mapFromObject(properties), (_, k) => @@ -865,6 +884,29 @@ async function addTypesInSchema( ); } + // Required keys can't live in a map, so expand a finite key set into + // explicit properties (#1959). Required keys outside that set fall + // through to the check below, which rejects them. + const valueType = additionalPropertiesType; + const expandKeyCases = + propertyNames !== undefined && + props.size === 0 && + required.size > 0 && + valueType !== undefined; + if (expandKeyCases) { + const caseProps = mapSortBy( + mapFromIterable(propertyNames, (name) => + typeBuilder.makeClassProperty( + valueType, + !required.has(name), + ), + ), + (_, k) => sortKey(k), + ); + mapMergeInto(props, caseProps); + additionalPropertiesType = undefined; + } + const additionalRequired = setSubtract(required, props.keys()); if (additionalRequired.size > 0) { const t = additionalPropertiesType; @@ -882,8 +924,17 @@ async function addTypesInSchema( mapMergeInto(props, additionalProps); } + let objectAttributes = attributes; + if (propertyNames !== undefined && !expandKeyCases) { + objectAttributes = combineTypeAttributes( + "union", + attributes, + propertyNamesTypeAttributeKind.makeAttributes(propertyNames), + ); + } + return typeBuilder.getUniqueObjectType( - attributes, + objectAttributes, props, additionalPropertiesType, ); @@ -1112,6 +1163,20 @@ async function addTypesInSchema( return typeBuilder.getArrayType(arrayAttributes, itemType); } + /** The finite key set `propertyNames` allows, if any; anything else + * (`pattern`, `$ref`, ...) leaves the keys unrestricted. */ + function makePropertyNames(): ReadonlySet | undefined { + if (schema.propertyNames === undefined) return undefined; + + const propertyNamesLoc = loc.push("propertyNames"); + return finiteStringCases( + checkJSONSchema( + schema.propertyNames, + propertyNamesLoc.canonicalRef, + ), + ); + } + async function makeObjectType(): Promise { let required: string[]; if ( @@ -1181,6 +1246,7 @@ async function addTypesInSchema( required, additionalProperties, orderKey, + makePropertyNames(), ); } @@ -1280,6 +1346,7 @@ async function addTypesInSchema( schema.properties !== undefined || schema.additionalProperties !== undefined || schema.unevaluatedProperties !== undefined || + schema.propertyNames !== undefined || schema.items !== undefined || schema.prefixItems !== undefined || schema.required !== undefined || diff --git a/packages/quicktype-core/src/language/JavaScript/JavaScriptRenderer.ts b/packages/quicktype-core/src/language/JavaScript/JavaScriptRenderer.ts index 16f3e416b..3afd3e711 100644 --- a/packages/quicktype-core/src/language/JavaScript/JavaScriptRenderer.ts +++ b/packages/quicktype-core/src/language/JavaScript/JavaScriptRenderer.ts @@ -1,5 +1,6 @@ import { arrayIntercalate } from "collection-utils"; +import { propertyNamesForType } from "../../attributes/PropertyNames.js"; import { ConvenienceRenderer } from "../../ConvenienceRenderer.js"; import { type Name, type Namer, funPrefixNamer } from "../../Naming.js"; import type { RenderContext } from "../../Renderer.js"; @@ -21,6 +22,7 @@ import type { TargetLanguage } from "../../TargetLanguage.js"; import type { ClassProperty, ClassType, + MapType, ObjectType, Type, } from "../../Type/index.js"; @@ -127,7 +129,12 @@ export class JavaScriptRenderer extends ConvenienceRenderer { (_stringType) => '""', (arrayType) => ["a(", this.typeMapTypeFor(arrayType.items), ")"], (_classType) => panic("We handled this above"), - (mapType) => ["m(", this.typeMapTypeFor(mapType.values), ")"], + (mapType) => [ + "m(", + this.typeMapTypeFor(mapType.values), + ...this.typeMapKeysFor(mapType), + ")", + ], (_enumType) => panic("We handled this above"), (unionType) => { const children = Array.from(unionType.getChildren()).map( @@ -145,6 +152,16 @@ export class JavaScriptRenderer extends ConvenienceRenderer { ); } + /** Extra `m()` argument listing allowed keys, so the converter can reject + * others. Empty when the map's keys are unconstrained. */ + private typeMapKeysFor(mapType: MapType): Sourcelike[] { + const keys = propertyNamesForType(mapType); + if (keys === undefined) return []; + + const cases = Array.from(keys).map((c) => `"${utf16StringEscape(c)}"`); + return [", [", cases.join(", "), "]"]; + } + private typeMapTypeForProperty(p: ClassProperty): Sourcelike { const typeMap = this.typeMapTypeFor(p.type); if (!p.isOptional) { @@ -413,7 +430,7 @@ function transform(val${anyAnnotation}, typ${anyAnnotation}, getProps${anyAnnota return d; } - function transformObject(props${anyMapAnnotation}, additional${anyAnnotation}, val${anyAnnotation})${anyAnnotation} { + function transformObject(props${anyMapAnnotation}, additional${anyAnnotation}, val${anyAnnotation}, keys${anyAnnotation})${anyAnnotation} { if (val === null || typeof val !== "object" || Array.isArray(val)) { return invalidValue(l(ref || "object"), val, key, parent); } @@ -425,6 +442,9 @@ function transform(val${anyAnnotation}, typ${anyAnnotation}, getProps${anyAnnota }); Object.getOwnPropertyNames(val).forEach(key => { if (!Object.prototype.hasOwnProperty.call(props, key)) { + if (keys !== undefined && keys.indexOf(key) < 0) { + return invalidValue(l(keys.map((k${stringAnnotation}) => JSON.stringify(k)).join(" | ")), key, key, ref); + } result[key] = ${ this._jsOptions.runtimeTypecheckIgnoreUnknownProperties ? "val[key]" @@ -450,7 +470,7 @@ function transform(val${anyAnnotation}, typ${anyAnnotation}, getProps${anyAnnota if (typeof typ === "object") { return typ.hasOwnProperty("unionMembers") ? transformUnion(typ.unionMembers, val) : typ.hasOwnProperty("arrayItems") ? transformArray(typ.arrayItems, val) - : typ.hasOwnProperty("props") ? transformObject(getProps(typ), typ.additional, val) + : typ.hasOwnProperty("props") ? transformObject(getProps(typ), typ.additional, val, typ.keys) : invalidValue(typ, val, key, parent); } // Numbers can be parsed by Date but shouldn't be. @@ -482,9 +502,9 @@ function o(props${anyArrayAnnotation}, additional${anyAnnotation}) { return { props, additional }; } -function m(additional${anyAnnotation}) { +function m(additional${anyAnnotation}, keys${anyAnnotation} = undefined) { const props${anyArrayAnnotation} = []; - return { props, additional }; + return { props, additional, keys }; } function r(name${stringAnnotation}) { diff --git a/packages/quicktype-core/src/language/TypeScriptFlow/TypeScriptFlowBaseRenderer.ts b/packages/quicktype-core/src/language/TypeScriptFlow/TypeScriptFlowBaseRenderer.ts index 09c06ee80..f0282d89d 100644 --- a/packages/quicktype-core/src/language/TypeScriptFlow/TypeScriptFlowBaseRenderer.ts +++ b/packages/quicktype-core/src/language/TypeScriptFlow/TypeScriptFlowBaseRenderer.ts @@ -16,6 +16,7 @@ import { ArrayType, type ClassType, EnumType, + type MapType, ObjectType, type Type, UnionType, @@ -79,6 +80,16 @@ export abstract class TypeScriptFlowBaseRenderer extends JavaScriptRenderer { ); } + /** Overridden by TypeScript to spell out a `propertyNames` key constraint; + * Flow has no mapped types. */ + protected sourceForMapType(mapType: MapType): MultiWord { + return singleWord([ + "{ [key: string]: ", + this.sourceFor(mapType.values).source, + " }", + ]); + } + // Flow (pinned at flow-bin 0.66 in CI) has no tuple-rest syntax, so // the base implementation always renders plain array types; the // TypeScript renderer overrides this to spell out `minItems` @@ -126,12 +137,7 @@ export abstract class TypeScriptFlowBaseRenderer extends JavaScriptRenderer { (_stringType) => singleWord("string"), (arrayType) => this.sourceForArrayType(arrayType), (_classType) => panic("We handled this above"), - (mapType) => - singleWord([ - "{ [key: string]: ", - this.sourceFor(mapType.values).source, - " }", - ]), + (mapType) => this.sourceForMapType(mapType), (_enumType) => panic("We handled this above"), (unionType) => { if ( diff --git a/packages/quicktype-core/src/language/TypeScriptFlow/TypeScriptRenderer.ts b/packages/quicktype-core/src/language/TypeScriptFlow/TypeScriptRenderer.ts index b162b9384..84c49b771 100644 --- a/packages/quicktype-core/src/language/TypeScriptFlow/TypeScriptRenderer.ts +++ b/packages/quicktype-core/src/language/TypeScriptFlow/TypeScriptRenderer.ts @@ -1,4 +1,5 @@ import { minMaxItemsForType } from "../../attributes/Constraints.js"; +import { propertyNamesForType } from "../../attributes/PropertyNames.js"; import type { Name } from "../../Naming.js"; import { type MultiWord, @@ -8,7 +9,13 @@ import { singleWord, } from "../../Source.js"; import { camelCase, utf16StringEscape } from "../../support/Strings.js"; -import type { ArrayType, ClassType, EnumType, Type } from "../../Type/index.js"; +import type { + ArrayType, + ClassType, + EnumType, + MapType, + Type, +} from "../../Type/index.js"; import { isNamedType } from "../../Type/TypeUtils.js"; import type { JavaScriptTypeAnnotations } from "../JavaScript/index.js"; @@ -58,6 +65,26 @@ export class TypeScriptRenderer extends TypeScriptFlowBaseRenderer { return singleWord(source); } + // `propertyNames` restricts the allowed keys, not which are present, so + // this is `Partial>` rather than a bare `Record` (which + // would require every key) or an index signature (`error TS1337`). + protected sourceForMapType(mapType: MapType): MultiWord { + const keys = propertyNamesForType(mapType); + if (keys === undefined) { + return super.sourceForMapType(mapType); + } + + return singleWord([ + "Partial `"${utf16StringEscape(c)}"`) + .join(" | "), + ", ", + this.sourceFor(mapType.values).source, + ">>", + ]); + } + protected uncheckedParsedJson(t: Type, parsedJson: Sourcelike): Sourcelike { // With `raw-type any` and `prefer-unknown` the deserializer's // parameter is `unknown`, which can't be returned as the target diff --git a/test/inputs/schema/property-names.1.fail.property-names.json b/test/inputs/schema/property-names.1.fail.property-names.json new file mode 100644 index 000000000..4617a5127 --- /dev/null +++ b/test/inputs/schema/property-names.1.fail.property-names.json @@ -0,0 +1,6 @@ +{ + "icon": "flag", + "languages": { + "fr": { "name": "Français" } + } +} diff --git a/test/inputs/schema/property-names.1.json b/test/inputs/schema/property-names.1.json new file mode 100644 index 000000000..4bffad4b3 --- /dev/null +++ b/test/inputs/schema/property-names.1.json @@ -0,0 +1,7 @@ +{ + "icon": "flag", + "languages": { + "de": { "name": "Deutsch", "description": "German" }, + "en": { "name": "English" } + } +} diff --git a/test/inputs/schema/property-names.2.json b/test/inputs/schema/property-names.2.json new file mode 100644 index 000000000..3ac68474f --- /dev/null +++ b/test/inputs/schema/property-names.2.json @@ -0,0 +1,4 @@ +{ + "icon": "flag", + "languages": {} +} diff --git a/test/inputs/schema/property-names.schema b/test/inputs/schema/property-names.schema new file mode 100644 index 000000000..9511a3a2f --- /dev/null +++ b/test/inputs/schema/property-names.schema @@ -0,0 +1,25 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "type": "object", + "additionalProperties": false, + "required": ["icon", "languages"], + "properties": { + "icon": { "type": "string" }, + "languages": { + "type": "object", + "propertyNames": { + "type": "string", + "enum": ["de", "en"] + }, + "additionalProperties": { + "type": "object", + "additionalProperties": false, + "required": ["name"], + "properties": { + "name": { "type": "string" }, + "description": { "type": "string" } + } + } + } + } +} diff --git a/test/languages.ts b/test/languages.ts index 5e721fad1..34c13aed7 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -79,7 +79,8 @@ export type LanguageFeature = | "minmax" | "minmaxlength" | "minmaxitems" - | "pattern"; + | "pattern" + | "property-names"; export interface Language { name: LanguageName; @@ -1115,7 +1116,14 @@ export const TypeScriptLanguage: Language = { "e8b04.json", ], allowMissingNull: false, - features: ["enum", "union", "no-defaults", "strict-optional", "date-time"], + features: [ + "enum", + "union", + "no-defaults", + "strict-optional", + "date-time", + "property-names", + ], output: "TopLevel.ts", topLevel: "TopLevel", skipJSON: [], @@ -1159,7 +1167,14 @@ export const JavaScriptLanguage: Language = { diffViaSchema: false, skipDiffViaSchema: [], allowMissingNull: false, - features: ["enum", "union", "no-defaults", "strict-optional", "date-time"], + features: [ + "enum", + "union", + "no-defaults", + "strict-optional", + "date-time", + "property-names", + ], output: "TopLevel.js", topLevel: "TopLevel", skipJSON: [], @@ -1219,7 +1234,13 @@ export const FlowLanguage: Language = { diffViaSchema: false, skipDiffViaSchema: [], allowMissingNull: false, - features: ["enum", "union", "no-defaults", "strict-optional"], + features: [ + "enum", + "union", + "no-defaults", + "strict-optional", + "property-names", + ], output: "TopLevel.js", topLevel: "TopLevel", skipJSON: [], diff --git a/test/unit/property-names-schema.test.ts b/test/unit/property-names-schema.test.ts new file mode 100644 index 000000000..2d3023dd3 --- /dev/null +++ b/test/unit/property-names-schema.test.ts @@ -0,0 +1,178 @@ +// Regression test for #1959: `propertyNames` was ignored, so maps came out +// unconstrained. The fixture test `property-names.schema` covers the runtime +// converter; this covers the emitted type shape, which fixtures can't check. + +import { + FetchingJSONSchemaStore, + InputData, + JSONSchemaInput, + quicktype, +} from "quicktype-core"; +import { describe, expect, test } from "vitest"; + +const language = { + type: "object", + required: ["name"], + properties: { name: { type: "string" } }, +}; + +async function generate(schema: object, lang = "typescript"): Promise { + const schemaInput = new JSONSchemaInput(new FetchingJSONSchemaStore()); + await schemaInput.addSource({ + name: "Foo", + schema: JSON.stringify(schema), + }); + const inputData = new InputData(); + inputData.addInput(schemaInput); + + const result = await quicktype({ + inputData, + lang, + rendererOptions: { "just-types": "true" }, + }); + return result.lines.join("\n"); +} + +async function generateTypeScript(schema: object): Promise { + return await generate(schema); +} + +function schemaWithPropertyNames( + propertyNames: object, + languagesExtra: object = {}, +): object { + return { + $schema: "http://json-schema.org/draft-07/schema#", + type: "object", + required: ["languages"], + properties: { + languages: { + type: "object", + propertyNames, + additionalProperties: language, + ...languagesExtra, + }, + }, + }; +} + +describe("JSON Schema propertyNames (issue #1959)", () => { + test("an enum of key names constrains the map's keys", async () => { + const output = await generateTypeScript( + schemaWithPropertyNames({ type: "string", enum: ["de", "en"] }), + ); + + expect(output).toContain( + 'languages: Partial>;', + ); + }); + + test("the keys stay optional", async () => { + const output = await generateTypeScript( + schemaWithPropertyNames({ type: "string", enum: ["de", "en"] }), + ); + + // `{}` is still valid; a bare `Record<...>` would require every key. + expect(output).toContain("Partial { + const output = await generateTypeScript( + schemaWithPropertyNames({ type: "string", const: "en" }), + ); + + expect(output).toContain('languages: Partial>;'); + }); + + test("required keys become properties rather than a map", async () => { + const output = await generateTypeScript( + schemaWithPropertyNames( + { type: "string", enum: ["de", "en"] }, + { required: ["en"] }, + ), + ); + + expect(output).toContain("de?: Language;"); + expect(output).toContain("en: Language;"); + expect(output).not.toContain("[property: string]: Language;"); + }); + + test("required keys with no permitted values is still an error", async () => { + // `additionalProperties: false` permits no keys, so requiring one + // contradicts the schema. + await expect( + generateTypeScript( + schemaWithPropertyNames( + { type: "string", enum: ["de", "en"] }, + { required: ["en"], additionalProperties: false }, + ), + ), + ).rejects.toThrow(/required properties but forbidden additionalTypes/); + }); + + test("a non-finite constraint leaves the keys alone", async () => { + const output = await generateTypeScript( + schemaWithPropertyNames({ type: "string", pattern: "^[a-z]{2}$" }), + ); + + expect(output).toContain("languages: { [key: string]: Language };"); + }); + + test("a mixed-type enum leaves the keys alone", async () => { + const output = await generateTypeScript( + schemaWithPropertyNames({ enum: ["de", 3] }), + ); + + expect(output).toContain("languages: { [key: string]: Language };"); + }); + + test("a $ref'd constraint leaves the keys alone", async () => { + const output = await generateTypeScript({ + ...schemaWithPropertyNames({ $ref: "#/definitions/Lang" }), + definitions: { Lang: { type: "string", enum: ["de", "en"] } }, + }); + + // Known limitation: cases are read straight off the schema, and a + // $ref doesn't carry them. + expect(output).toContain("languages: { [key: string]: Language };"); + }); + + test("keys are escaped", async () => { + const output = await generateTypeScript( + schemaWithPropertyNames({ enum: ['a"b', "c-d"] }), + ); + + expect(output).toContain('Partial>'); + }); +}); + +// Only TypeScript spells the constraint out; other renderers must be +// unaffected since the attribute rides on a shared type graph. +describe("propertyNames leaves other languages alone", () => { + const schema = schemaWithPropertyNames({ + type: "string", + enum: ["de", "en"], + }); + + test("Flow keeps its index signature", async () => { + // Shares a base renderer with TypeScript, so easy to break by accident. + const output = await generate(schema, "flow"); + + expect(output).toContain("languages: { [key: string]: Language };"); + expect(output).not.toContain("Partial { + const output = await generate(schema, "python"); + + expect(output).toContain("languages: dict[str, Language]"); + expect(output).not.toContain("Literal"); + }); + + test("Go keeps its plain map", async () => { + const output = await generate(schema, "go"); + + expect(output).toContain("map[string]Language"); + }); +});