diff --git a/packages/quicktype-core/src/input/JSONSchemaInput.ts b/packages/quicktype-core/src/input/JSONSchemaInput.ts index 66b1c4104b..ec2e0ddd54 100644 --- a/packages/quicktype-core/src/input/JSONSchemaInput.ts +++ b/packages/quicktype-core/src/input/JSONSchemaInput.ts @@ -30,7 +30,10 @@ import { patternAttributeProducer, } from "../attributes/Constraints.js"; import { defaultValueAttributeProducer } from "../attributes/DefaultValue.js"; -import { descriptionAttributeProducer } from "../attributes/Description.js"; +import { + descriptionAttributeProducer, + propertyDescriptionsTypeAttributeKind, +} from "../attributes/Description.js"; import { enumValuesAttributeProducer } from "../attributes/EnumValues.js"; import { StringTypes } from "../attributes/StringTypes.js"; import { @@ -819,6 +822,40 @@ async function addTypesInSchema( typeForCanonicalRef.set(loc.canonicalRef, t); } + async function descriptionForReferencedSchema( + schema: JSONSchema, + loc: Location, + ): Promise { + if ( + typeof schema !== "object" || + typeof schema.description === "string" + ) { + return undefined; + } + + const visited = new Set(); + for (;;) { + if (typeof schema !== "object" || typeof schema.$ref !== "string") { + return undefined; + } + + const key = `${loc.canonicalRef.toString()}|${schema.$ref}`; + if (visited.has(key)) return undefined; + visited.add(key); + + [schema, loc] = await resolver.resolveVirtualRef( + loc, + Ref.parse(schema.$ref), + ); + if ( + typeof schema === "object" && + typeof schema.description === "string" + ) { + return schema.description; + } + } + } + async function makeObject( loc: Location, attributes: TypeAttributes, @@ -832,19 +869,48 @@ async function addTypesInSchema( const propertiesMap = mapSortBy(mapFromObject(properties), (_, k) => sortKey(k), ); + const referencedPropertyDescriptions = new Map< + string, + ReadonlySet + >(); const props = await mapMapSync( propertiesMap, async (propSchema, propName) => { const propLoc = loc.push("properties", propName); + const checkedSchema = checkJSONSchema( + propSchema, + propLoc.canonicalRef, + ); const t = await toType( - checkJSONSchema(propSchema, propLoc.canonicalRef), + checkedSchema, propLoc, makeNamesTypeAttributes(propName, true), ); + const description = await descriptionForReferencedSchema( + checkedSchema, + propLoc, + ); + if (description !== undefined) { + referencedPropertyDescriptions.set( + propName, + new Set([description]), + ); + } + const isOptional = !required.has(propName); return typeBuilder.makeClassProperty(t, isOptional); }, ); + if (referencedPropertyDescriptions.size > 0) { + attributes = combineTypeAttributes( + "union", + attributes, + propertyDescriptionsTypeAttributeKind.makeAttributes( + referencedPropertyDescriptions, + ), + ); + } + let additionalPropertiesType: TypeRef | undefined; if ( additionalProperties === undefined || diff --git a/packages/quicktype-core/src/language/JSONSchema/JSONSchemaRenderer.ts b/packages/quicktype-core/src/language/JSONSchema/JSONSchemaRenderer.ts index 227a46da75..b35a64eabd 100644 --- a/packages/quicktype-core/src/language/JSONSchema/JSONSchemaRenderer.ts +++ b/packages/quicktype-core/src/language/JSONSchema/JSONSchemaRenderer.ts @@ -128,11 +128,12 @@ 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), - ); + const propertyDescription = this.descriptionForClassProperty( + o, + name, + ); + if (propertyDescription !== undefined) { + addDescriptionToSchema(prop, propertyDescription); } props[name] = prop; diff --git a/test/inputs/schema/description-ref.1.fail.no-defaults.json b/test/inputs/schema/description-ref.1.fail.no-defaults.json new file mode 100644 index 0000000000..3809185c06 --- /dev/null +++ b/test/inputs/schema/description-ref.1.fail.no-defaults.json @@ -0,0 +1,3 @@ +{ + "name": "Ada" +} diff --git a/test/inputs/schema/description-ref.1.json b/test/inputs/schema/description-ref.1.json new file mode 100644 index 0000000000..36a1b77272 --- /dev/null +++ b/test/inputs/schema/description-ref.1.json @@ -0,0 +1,4 @@ +{ + "name": "Ada", + "surname": "Lovelace" +} diff --git a/test/inputs/schema/description-ref.schema b/test/inputs/schema/description-ref.schema new file mode 100644 index 0000000000..b1029806b8 --- /dev/null +++ b/test/inputs/schema/description-ref.schema @@ -0,0 +1,25 @@ +{ + "$defs": { + "name": { + "type": "string", + "description": "name description" + }, + "surname": { + "type": "string", + "description": "surname description" + } + }, + "type": "object", + "required": [ + "name", + "surname" + ], + "properties": { + "name": { + "$ref": "#/$defs/name" + }, + "surname": { + "$ref": "#/$defs/surname" + } + } +} diff --git a/test/languages.ts b/test/languages.ts index 120f3170b0..3ca3dad134 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-ref.schema", // The default-value fail sample also relies on required-property // enforcement, which cJSON does not do. "default-value.schema", @@ -1002,6 +1003,9 @@ export const SwiftLanguage: Language = { // This works on macOS, but on Linux one of the failure test cases doesn't fail ...skipsUntypedUnions, "required.schema", + // Same missing-required detection gap as required.schema: the + // no-defaults fail sample does not fail on Linux. + "description-ref.schema", // The default-value fail sample also relies on required-property enforcement. "default-value.schema", "multi-type-enum.schema", @@ -1918,6 +1922,9 @@ export const HaskellLanguage: Language = { "keyword-unions.schema", "optional-any.schema", "required.schema", + // Same as required.schema: the Maybe-encoding driver prints "null" + // and exits 0, so this no-defaults fail sample can't be detected. + "description-ref.schema", // The default-value fail sample also relies on required-property enforcement. "default-value.schema", "required-non-properties.schema", @@ -2087,6 +2094,8 @@ export const TypeScriptZodLanguage: Language = { "optional-any.schema", "recursive-union-flattening.schema", "required.schema", + // Same missing-required detection gap as required.schema. + "description-ref.schema", // The default-value fail sample also relies on required-property enforcement. "default-value.schema", "required-non-properties.schema", @@ -2206,6 +2215,8 @@ export const TypeScriptEffectSchemaLanguage: Language = { "keyword-unions.schema", "optional-any.schema", "required.schema", + // Same missing-required detection gap as required.schema. + "description-ref.schema", // The default-value fail sample also relies on required-property enforcement. "default-value.schema", "required-non-properties.schema", @@ -2273,6 +2284,7 @@ export const ElixirLanguage: Language = { // Struct keys cannot be enforced at runtime in Elixir and their values will just be set to null. "strict-optional.schema", "required.schema", + "description-ref.schema", // The default-value fail sample also relies on required-property enforcement. "default-value.schema", "boolean-subschema.schema", diff --git a/test/unit/schema-description-ref.test.ts b/test/unit/schema-description-ref.test.ts new file mode 100644 index 0000000000..557a36f25f --- /dev/null +++ b/test/unit/schema-description-ref.test.ts @@ -0,0 +1,55 @@ +import { + InputData, + JSONSchemaInput, + quicktype, +} from "../../packages/quicktype-core/src/index.js"; +import { describe, expect, test } from "vitest"; + +const schema = { + $defs: { + name: { + type: "string", + description: "name description", + }, + surname: { + type: "string", + description: "surname description", + }, + }, + type: "object", + properties: { + name: { $ref: "#/$defs/name" }, + surname: { $ref: "#/$defs/surname" }, + }, +}; + +interface RenderedSchema { + definitions: Record< + string, + { properties: Record } + >; +} + +async function generateSchema(): Promise { + const schemaInput = new JSONSchemaInput(undefined); + await schemaInput.addSource({ + name: "Input", + schema: JSON.stringify(schema), + }); + + const inputData = new InputData(); + inputData.addInput(schemaInput); + + const result = await quicktype({ inputData, lang: "schema" }); + return JSON.parse(result.lines.join("\n")) as RenderedSchema; +} + +describe("JSON Schema descriptions through $ref (issue #1582)", () => { + test("descriptions from distinct definitions do not merge", async () => { + const output = await generateSchema(); + const properties = output.definitions.Input.properties; + + expect(properties.name.description).toBe("name description"); + expect(properties.surname.description).toBe("surname description"); + }); +});