From d34a79f715e4f8484b397171ce90346fee3c0ed4 Mon Sep 17 00:00:00 2001 From: Jeremy Fiel <32110157+jeremyfiel@users.noreply.github.com> Date: Fri, 22 May 2026 18:26:44 +0000 Subject: [PATCH] fix(rules): handle bare required schemas in parent-context composition --- .changeset/nasty-peas-agree.md | 6 + ...no-required-schema-properties-undefined.md | 80 ++++ ...quired-schema-properties-undefined.test.ts | 417 ++++++++---------- ...no-required-schema-properties-undefined.ts | 51 ++- 4 files changed, 309 insertions(+), 245 deletions(-) create mode 100644 .changeset/nasty-peas-agree.md diff --git a/.changeset/nasty-peas-agree.md b/.changeset/nasty-peas-agree.md new file mode 100644 index 0000000000..1b5de8eddb --- /dev/null +++ b/.changeset/nasty-peas-agree.md @@ -0,0 +1,6 @@ +--- +'@redocly/openapi-core': patch +'@redocly/cli': patch +--- + +Updated `no-required-schema-properties-undefined` rule to cover additional edge cases. diff --git a/docs/@v2/rules/common/no-required-schema-properties-undefined.md b/docs/@v2/rules/common/no-required-schema-properties-undefined.md index 3ca3bffe9a..fe64b7cbb8 100644 --- a/docs/@v2/rules/common/no-required-schema-properties-undefined.md +++ b/docs/@v2/rules/common/no-required-schema-properties-undefined.md @@ -128,6 +128,86 @@ schemas: example: doggie ``` +The rule accepts bare `required` constraints on property sub-schemas when the property's type is defined in a parent `allOf` sibling. This is a valid JSON Schema pattern for adding presence constraints on top of a referenced base type: + +```yaml +schemas: + PersonBase: + type: object + properties: + personName: + type: object + properties: + givenName: + type: string + familyName: + type: string + Person: + type: object + allOf: + - $ref: '#/components/schemas/PersonBase' + properties: + personName: + required: + - givenName + - familyName + required: + - personName +``` + +The rule also accepts `oneOf` and `anyOf` branches used as pure constraint fragments, where each branch contains only a `required` keyword and the property's type is defined in a parent `allOf` sibling: + +```yaml +schemas: + PersonBase: + type: object + properties: + communication: + type: object + properties: + landlines: + type: array + mobiles: + type: array + emails: + type: array + Person: + type: object + allOf: + - $ref: '#/components/schemas/PersonBase' + properties: + communication: + oneOf: + - required: + - landlines + - required: + - mobiles + - required: + - emails + required: + - communication +``` + +Misspellings in bare `required` lists are still caught. If a required key does not exist in the property's type definition resolved through the parent `allOf` sibling, the rule reports an error: + +```yaml +schemas: + Person: + type: object + allOf: + - $ref: '#/components/schemas/PersonBase' + properties: + personName: + required: + - giveName # misspelling of givenName + required: + - personName +``` + +```bash +Required property 'giveName' is undefined. +``` + ## Related rules - [no-schema-type-mismatch](./no-schema-type-mismatch.md) diff --git a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts index a560e94732..e8899cfec7 100644 --- a/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts +++ b/packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts @@ -46,7 +46,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'test' is not defined.", + "message": "Required property 'test' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -60,7 +60,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'test2' is not defined.", + "message": "Required property 'test2' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -110,7 +110,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'test' is not defined.", + "message": "Required property 'test' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -234,7 +234,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'foo' is not defined.", + "message": "Required property 'foo' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -290,7 +290,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'surname' is not defined.", + "message": "Required property 'surname' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -433,7 +433,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'test' is not defined.", + "message": "Required property 'test' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -542,7 +542,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'name' is not defined.", + "message": "Required property 'name' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -552,6 +552,46 @@ describe('no-required-schema-properties-undefined', () => { `); }); + it('should not report when bare required constraints are defined via anyOf branches in parent context', async () => { + const document = parseYamlToDocument( + outdent` + openapi: 3.0.0 + components: + schemas: + PersonBase: + type: object + properties: + communication: + type: object + properties: + landlines: + type: array + mobiles: + type: array + Person: + type: object + allOf: + - $ref: '#/components/schemas/PersonBase' + properties: + communication: + anyOf: + - required: + - landlines + - required: + - mobiles + `, + 'foobar.yaml' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); + }); + it('should NOT report if one or more of the required properties are defined when used in schema with anyOf keyword', async () => { const document = parseYamlToDocument( outdent` @@ -655,7 +695,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'name' is not defined.", + "message": "Required property 'name' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -669,7 +709,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'huntingSkill' is not defined.", + "message": "Required property 'huntingSkill' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -805,7 +845,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'missing-required-prop' is not defined.", + "message": "Required property 'missing-required-prop' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -851,7 +891,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'missingRequired' is not defined.", + "message": "Required property 'missingRequired' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -900,7 +940,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'missing-required' is not defined.", + "message": "Required property 'missing-required' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -910,17 +950,33 @@ describe('no-required-schema-properties-undefined', () => { `); }); - it('should not crash on unresolved $ref', async () => { + it('should not report bare required when property is defined in parent allOf sibling', async () => { const document = parseYamlToDocument( outdent` openapi: 3.0.0 components: schemas: - Object: + PersonBase: + type: object + properties: + personName: + type: object + properties: + givenName: + type: string + familyName: + type: string + Person: + type: object allOf: - - $ref: '#/components/schemas/NotExists' + - $ref: '#/components/schemas/PersonBase' + properties: + personName: + required: + - givenName + - familyName required: - - name + - personName `, 'foobar.yaml' ); @@ -931,90 +987,43 @@ describe('no-required-schema-properties-undefined', () => { config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), }); - expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` - [ - { - "location": [ - { - "pointer": "#/components/schemas/Object/required/0", - "reportOnKey": false, - "source": "foobar.yaml", - }, - ], - "message": "Required property 'name' is not defined.", - "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", - "ruleId": "no-required-schema-properties-undefined", - "severity": "error", - "suggest": [], - }, - ] - `); - }); - - it('should not report a required property defined as a sibling of $ref', async () => { - const document = parseYamlToDocument( - outdent` - openapi: 3.1.0 - info: - title: Test - version: 1.0.0 - paths: {} - components: - schemas: - BadRequest: - $ref: '#/components/schemas/Base' - properties: - status: - type: integer - required: - - status - - title - Base: - type: object - properties: - title: - type: string - Usage: - $ref: '#/components/schemas/BadRequest' - `, - 'foobar.yaml' - ); - - const results = await lintDocument({ - externalRefResolver: new BaseResolver(), - document, - config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), - }); - expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); - it('should not report a required property defined further down a composed $ref chain', async () => { + it('should not report bare required in oneOf branches when property is defined in parent allOf sibling', async () => { const document = parseYamlToDocument( outdent` - openapi: 3.1.0 - info: - title: Test - version: 1.0.0 - paths: {} - components: - schemas: - Usage: - $ref: '#/components/schemas/Middle' - Middle: - $ref: '#/components/schemas/Mid2' - title: Middle - required: - - deepProp - Mid2: - $ref: '#/components/schemas/Leaf' - title: Mid2 - Leaf: - type: object - properties: - deepProp: - type: string - `, + openapi: 3.0.0 + components: + schemas: + PersonBase: + type: object + properties: + communication: + type: object + properties: + landlines: + type: array + mobiles: + type: array + emails: + type: array + Person: + type: object + allOf: + - $ref: '#/components/schemas/PersonBase' + properties: + communication: + oneOf: + - required: + - landlines + - required: + - mobiles + - required: + - emails + required: + - communication + `, 'foobar.yaml' ); @@ -1027,33 +1036,40 @@ describe('no-required-schema-properties-undefined', () => { expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); - it('should not report a required property defined in an allOf sibling of a referenced composed $ref', async () => { + it('should not report bare required in anyOf branches when property is defined in parent allOf sibling', async () => { const document = parseYamlToDocument( outdent` - openapi: 3.1.0 - info: - title: Test - version: 1.0.0 - paths: {} - components: - schemas: - Usage: - $ref: '#/components/schemas/Composed' - Composed: - $ref: '#/components/schemas/Base' - allOf: - - type: object - properties: - extra: - type: string - required: - - extra - Base: - type: object - properties: - title: - type: string - `, + openapi: 3.0.0 + components: + schemas: + PersonBase: + type: object + properties: + communication: + type: object + properties: + landlines: + type: array + mobiles: + type: array + emails: + type: array + Person: + type: object + allOf: + - $ref: '#/components/schemas/PersonBase' + properties: + communication: + anyOf: + - required: + - landlines + - required: + - mobiles + - required: + - emails + required: + - communication + `, 'foobar.yaml' ); @@ -1066,30 +1082,33 @@ describe('no-required-schema-properties-undefined', () => { expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); }); - it('should report an undefined required property written next to an inline $ref', async () => { + it('should report misspelled required property even when parent allOf sibling defines the property', async () => { const document = parseYamlToDocument( outdent` - openapi: 3.1.0 - info: - title: Test - version: 1.0.0 - paths: {} - components: - schemas: - Payload: - type: object - properties: - data: - $ref: '#/components/schemas/Base' - required: - - name - - missing - Base: - type: object - properties: - name: - type: string - `, + openapi: 3.0.0 + components: + schemas: + PersonBase: + type: object + properties: + personName: + type: object + properties: + givenName: + type: string + familyName: + type: string + Person: + type: object + allOf: + - $ref: '#/components/schemas/PersonBase' + properties: + personName: + required: + - giveName + required: + - personName + `, 'foobar.yaml' ); @@ -1104,12 +1123,12 @@ describe('no-required-schema-properties-undefined', () => { { "location": [ { - "pointer": "#/components/schemas/Payload/properties/data/required/1", + "pointer": "#/components/schemas/Person/properties/personName/required/0", "reportOnKey": false, "source": "foobar.yaml", }, ], - "message": "Required property 'missing' is not defined.", + "message": "Required property 'giveName' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", @@ -1119,104 +1138,18 @@ describe('no-required-schema-properties-undefined', () => { `); }); - it('should ignore a non-array required on a composed parameter $ref', async () => { - const document = parseYamlToDocument( - outdent` - openapi: 3.1.0 - info: - title: Test - version: 1.0.0 - paths: - /demo: - get: - parameters: - - $ref: '#/components/parameters/Alias' - responses: {} - components: - parameters: - Alias: - $ref: '#/components/parameters/Base' - required: true - Base: - name: q - in: query - schema: - type: string - `, - 'foobar.yaml' - ); - - const results = await lintDocument({ - externalRefResolver: new BaseResolver(), - document, - config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), - }); - - expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); - }); - - it('should find a property on a composed hop even when the chain end was already visited', async () => { - const document = parseYamlToDocument( - outdent` - openapi: 3.1.0 - info: - title: Test - version: 1.0.0 - paths: {} - components: - schemas: - Cat: - allOf: - - $ref: '#/components/schemas/Leaf' - - $ref: '#/components/schemas/Composed' - required: - - special - Composed: - $ref: '#/components/schemas/Leaf' - properties: - special: - type: string - Leaf: - type: object - properties: - name: - type: string - `, - 'foobar.yaml' - ); - - const results = await lintDocument({ - externalRefResolver: new BaseResolver(), - document, - config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), - }); - - expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`); - }); - - it('should still report a genuinely undefined property in a composed $ref chain', async () => { + it('should not crash on unresolved $ref', async () => { const document = parseYamlToDocument( outdent` - openapi: 3.1.0 - info: - title: Test - version: 1.0.0 - paths: {} - components: - schemas: - Usage: - $ref: '#/components/schemas/Middle' - Middle: - $ref: '#/components/schemas/Leaf' - title: Middle - required: - - missingProp - Leaf: - type: object - properties: - deepProp: - type: string - `, + openapi: 3.0.0 + components: + schemas: + Object: + allOf: + - $ref: '#/components/schemas/NotExists' + required: + - name + `, 'foobar.yaml' ); @@ -1231,12 +1164,12 @@ describe('no-required-schema-properties-undefined', () => { { "location": [ { - "pointer": "#/components/schemas/Middle/required/0", + "pointer": "#/components/schemas/Object/required/0", "reportOnKey": false, "source": "foobar.yaml", }, ], - "message": "Required property 'missingProp' is not defined.", + "message": "Required property 'name' is undefined.", "reference": "https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined", "ruleId": "no-required-schema-properties-undefined", "severity": "error", diff --git a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts index 9dde6ee842..d3fc1d1db6 100644 --- a/packages/core/src/rules/common/no-required-schema-properties-undefined.ts +++ b/packages/core/src/rules/common/no-required-schema-properties-undefined.ts @@ -1,5 +1,6 @@ import { type Location } from '../../ref-utils.js'; import type { OasRef } from '../../typings/openapi.js'; +import { getOwn } from '../../utils/get-own.js'; import { isNotEmptyArray } from '../../utils/is-not-empty-array.js'; import { isPlainObject } from '../../utils/is-plain-object.js'; import type { Async2Rule, Async3Rule, Arazzo1Rule, Oas2Rule, Oas3Rule } from '../../visitors.js'; @@ -80,14 +81,58 @@ export const NoRequiredSchemaPropertiesUndefined: }; const compositionRoot = findCompositionRoot(parents.length - 2, currentSchema); + const propertyContainerSchema = compositionRoot ?? currentSchema; + + const hasPropertyInParentContext = ( + propertyName: string, + targetSchema: AnySchema + ): boolean => { + for (let i = parents.length - 2; i >= 0; i--) { + const ancestor = parents[i]; + const props = ancestor.properties as Record | undefined; + if (!props) continue; + + const propertyKey = (Object.keys(props) as string[]).find((key) => { + const schema = getOwn(props, key) as AnySchema; + if (schema === targetSchema) return true; + return resolveSchema(schema, ctx).schema === targetSchema; + }); + if (!propertyKey) continue; + + const checkSiblings = (siblings: AnySchema[] | undefined): boolean => + !!siblings?.some((sibling) => { + const { schema: siblingSchema, location } = resolveSchema(sibling, ctx); + if (!siblingSchema?.properties) return false; + const propertyDef = getOwn( + siblingSchema.properties as Record, + propertyKey + ) as AnySchema | undefined; + return ( + propertyDef !== undefined && + schemaHasProperty(propertyDef, propertyName, ctx, new Set(), location) + ); + }); + + if ( + checkSiblings(ancestor.allOf) || + checkSiblings(ancestor.anyOf) || + checkSiblings(ancestor.oneOf) + ) { + return true; + } + } + + return false; + }; for (const [i, requiredProperty] of currentSchema.required.entries()) { if ( - !schemaHasProperty(currentSchema, requiredProperty, ctx) && - !schemaHasProperty(compositionRoot, requiredProperty, ctx) + !schemaHasProperty(currentSchema, requiredProperty, ctx, new Set(), ctx.location) && + !schemaHasProperty(compositionRoot, requiredProperty, ctx, new Set(), ctx.location) && + !hasPropertyInParentContext(requiredProperty, propertyContainerSchema) ) { ctx.report({ - message: `Required property '${requiredProperty}' is not defined.`, + message: `Required property '${requiredProperty}' is undefined.`, location: ctx.location.child(['required', i]), reference: 'https://redocly.com/docs/cli/rules/common/no-required-schema-properties-undefined',