diff --git a/.github/workflows/test-pr.yaml b/.github/workflows/test-pr.yaml index 5550acf162..6f32211668 100644 --- a/.github/workflows/test-pr.yaml +++ b/.github/workflows/test-pr.yaml @@ -36,7 +36,7 @@ jobs: matrix: fixture: - typescript,typescript-zod,schema-typescript-zod,typescript-effect-schema - - javascript,schema-javascript + - javascript,schema-javascript,schema-description-unification - golang,schema-golang - cjson,schema-cjson - cjson-default,cjson-multi-header,cjson-multi-split diff --git a/packages/quicktype-core/src/language/JSONSchema/JSONSchemaRenderer.ts b/packages/quicktype-core/src/language/JSONSchema/JSONSchemaRenderer.ts index 227a46da75..0b1745fb57 100644 --- a/packages/quicktype-core/src/language/JSONSchema/JSONSchemaRenderer.ts +++ b/packages/quicktype-core/src/language/JSONSchema/JSONSchemaRenderer.ts @@ -128,12 +128,10 @@ export class JSONSchemaRenderer extends ConvenienceRenderer { const req: string[] = []; for (const [name, p] of o.getProperties()) { const prop = this.schemaForType(p.type); - if (prop.description === undefined) { - addDescriptionToSchema( - prop, - this.descriptionForClassProperty(o, name), - ); - } + addDescriptionToSchema( + prop, + this.descriptionForClassProperty(o, name), + ); props[name] = prop; if (!p.isOptional) { diff --git a/test/fixtures.ts b/test/fixtures.ts index 84616537e7..dda21ab781 100644 --- a/test/fixtures.ts +++ b/test/fixtures.ts @@ -929,6 +929,76 @@ class JSONSchemaFixture extends LanguageFixture { } } +// Regression fixture for #2801. It verifies schema-specific output that +// cannot be observed by compiling and running code generated from a JSON Schema. +class JSONSchemaToSchemaFixture extends Fixture { + readonly name = "schema-description-unification"; + + constructor() { + super(languages.TypeScriptLanguage); + } + + getSamples(sources: string[]): { priority: Sample[]; others: Sample[] } { + return samplesFromSources( + sources, + ["test/inputs/schema/description-unification.schema"], + [], + "schema", + ); + } + + async runWithSample( + sample: Sample, + index: number, + total: number, + ): Promise { + const cwd = this.getRunDirectory(); + const message = this.runMessageStart(sample, index, total, cwd, false); + const output = path.join(cwd, "schema.json"); + mkdirs(cwd); + + await quicktype({ + src: [sample.path], + srcLang: "schema", + lang: "schema", + topLevel: "DescriptionUnification", + out: output, + telemetry: "disable", + }); + + const schema = JSON.parse(fs.readFileSync(output, "utf8")) as { + definitions: Record< + string, + { properties: Record } + >; + }; + const expectedDescriptions = [ + ["Allow", "labels", "The labels to apply to the policy"], + [ + "Metadata", + "annotations", + "Additional annotations for the generated resource", + ], + ] as const; + + for (const [definition, property, expected] of expectedDescriptions) { + const actual = + schema.definitions[definition]?.properties[property] + ?.description; + if (actual !== expected) { + failWith("Schema property has the wrong description", { + definition, + property, + expected, + actual, + }); + } + } + + this.runMessageEnd(message, expectedDescriptions.length); + } +} + // `leadingComments` is a quicktype-core API option, so the CLI fixture path // cannot exercise it. class LeadingCommentsGoFixture extends JSONSchemaFixture { @@ -1785,6 +1855,7 @@ export const allFixtures: Fixture[] = [ new JSONFixture(languages.ElixirLanguage), new JSONSchemaJSONFixture(languages.CSharpLanguage), new JSONTypeScriptFixture(languages.CSharpLanguage), + new JSONSchemaToSchemaFixture(), // new JSONSchemaFixture(languages.CrystalLanguage), new JSONSchemaFixture(languages.JSONSchemaLanguage), new JSONSchemaFixture(languages.CSharpLanguage), diff --git a/test/inputs/schema/description-unification.1.fail.no-defaults.json b/test/inputs/schema/description-unification.1.fail.no-defaults.json new file mode 100644 index 0000000000..b3204df261 --- /dev/null +++ b/test/inputs/schema/description-unification.1.fail.no-defaults.json @@ -0,0 +1,7 @@ +{ + "allow": { + "labels": { + "env": "production" + } + } +} diff --git a/test/inputs/schema/description-unification.1.json b/test/inputs/schema/description-unification.1.json new file mode 100644 index 0000000000..c9991b0071 --- /dev/null +++ b/test/inputs/schema/description-unification.1.json @@ -0,0 +1,14 @@ +{ + "allow": { + "labels": { + "env": "production", + "team": "core" + } + }, + "metadata": { + "annotations": { + "note": "generated by quicktype", + "owner": "platform" + } + } +} diff --git a/test/inputs/schema/description-unification.schema b/test/inputs/schema/description-unification.schema new file mode 100644 index 0000000000..a3539dba1e --- /dev/null +++ b/test/inputs/schema/description-unification.schema @@ -0,0 +1,34 @@ +{ + "$schema": "http://json-schema.org/draft-07/schema#", + "title": "DescriptionUnification", + "type": "object", + "properties": { + "allow": { + "type": "object", + "properties": { + "labels": { + "description": "The labels to apply to the policy", + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "required": ["labels"] + }, + "metadata": { + "type": "object", + "properties": { + "annotations": { + "description": "Additional annotations for the generated resource", + "type": "object", + "additionalProperties": { + "type": "string" + } + } + }, + "required": ["annotations"] + } + }, + "required": ["allow", "metadata"] +} diff --git a/test/languages.ts b/test/languages.ts index 120f3170b0..e00e9336b5 100644 --- a/test/languages.ts +++ b/test/languages.ts @@ -678,6 +678,7 @@ export const CJSONLanguage: Language = { /* Required properties absent are not checked (for the current implementation, can be added later, should abord parsing and return NULL) */ "intersection.schema", "required.schema", + "description-unification.schema", // The default-value fail sample also relies on required-property // enforcement, which cJSON does not do. "default-value.schema", @@ -1918,6 +1919,7 @@ export const HaskellLanguage: Language = { "keyword-unions.schema", "optional-any.schema", "required.schema", + "description-unification.schema", // The default-value fail sample also relies on required-property enforcement. "default-value.schema", "required-non-properties.schema", @@ -2277,6 +2279,7 @@ export const ElixirLanguage: Language = { "default-value.schema", "boolean-subschema.schema", "intersection.schema", + "description-unification.schema", "optional-any.schema", // The test incorrectly succeeds due to the emitter being permissive for unions that contain only primitives. A future enhancement