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/@v1/rules/oas/no-required-schema-properties-undefined.md b/docs/@v1/rules/oas/no-required-schema-properties-undefined.md index 90a4f3ed6e..4f2f5199e2 100644 --- a/docs/@v1/rules/oas/no-required-schema-properties-undefined.md +++ b/docs/@v1/rules/oas/no-required-schema-properties-undefined.md @@ -98,6 +98,86 @@ schemas: - Name ``` +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` 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-invalid-schema-examples](./no-invalid-schema-examples.md) 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..04abf81f4e 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` 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 04167ac42d..bf909a7d26 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.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -59,7 +59,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'test2' is not defined.", + "message": "Required property 'test2' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -108,7 +108,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'test' is not defined.", + "message": "Required property 'test' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -231,7 +231,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'foo' is not defined.", + "message": "Required property 'foo' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -286,7 +286,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'surname' is not defined.", + "message": "Required property 'surname' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -428,7 +428,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'test' is not defined.", + "message": "Required property 'test' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -536,7 +536,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'name' is not defined.", + "message": "Required property 'name' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -648,7 +648,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'name' is not defined.", + "message": "Required property 'name' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -661,7 +661,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'huntingSkill' is not defined.", + "message": "Required property 'huntingSkill' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -796,7 +796,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.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -841,7 +841,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'missingRequired' is not defined.", + "message": "Required property 'missingRequired' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -889,7 +889,194 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'missing-required' is not defined.", + "message": "Required property 'missing-required' is undefined.", + "ruleId": "no-required-schema-properties-undefined", + "severity": "error", + "suggest": [], + }, + ] + `); + }); + + 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: + 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 + `, + '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 bare required in oneOf branches when property is defined in parent allOf sibling', 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 + emails: + type: array + Person: + type: object + allOf: + - $ref: '#/components/schemas/PersonBase' + properties: + communication: + oneOf: + - required: + - landlines + - required: + - mobiles + - required: + - emails + required: + - communication + `, + '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 bare required in anyOf branches when property is defined in parent allOf sibling', 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 + emails: + type: array + Person: + type: object + allOf: + - $ref: '#/components/schemas/PersonBase' + properties: + communication: + anyOf: + - required: + - landlines + - required: + - mobiles + - required: + - emails + required: + - communication + `, + '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 report misspelled required property even when parent allOf sibling defines the property', async () => { + const document = parseYamlToDocument( + outdent` + 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' + ); + + const results = await lintDocument({ + externalRefResolver: new BaseResolver(), + document, + config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }), + }); + + expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(` + [ + { + "location": [ + { + "pointer": "#/components/schemas/Person/properties/personName/required/0", + "reportOnKey": false, + "source": "foobar.yaml", + }, + ], + "message": "Required property 'giveName' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], @@ -929,7 +1116,7 @@ describe('no-required-schema-properties-undefined', () => { "source": "foobar.yaml", }, ], - "message": "Required property 'name' is not defined.", + "message": "Required property 'name' is undefined.", "ruleId": "no-required-schema-properties-undefined", "severity": "error", "suggest": [], 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 c8751b2622..082d7e24f0 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 @@ -81,13 +81,50 @@ export const NoRequiredSchemaPropertiesUndefined: const compositionRoot = findCompositionRoot(parents.length - 2, 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 propKey = (Object.keys(props) as string[]).find((k) => { + const val = getOwn(props, k) as AnySchema; + if (val === targetSchema) return true; + return resolveSchema(val, ctx).schema === targetSchema; + }); + if (!propKey) continue; + + const checkSiblings = (siblings: AnySchema[] | undefined): boolean => + !!siblings?.some((sibling) => { + const { schema: s, location } = resolveSchema(sibling, ctx); + if (!s?.properties) return false; + const propDef = getOwn(s.properties as Record, propKey); + if (propDef === undefined) return false; + return hasProperty(propDef as AnySchema, propertyName, 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 ( !hasProperty(currentSchema, requiredProperty, new Set()) && - !hasProperty(compositionRoot, requiredProperty, new Set()) + !hasProperty(compositionRoot, requiredProperty, new Set()) && + !hasPropertyInParentContext(requiredProperty, compositionRoot ?? currentSchema) ) { ctx.report({ - message: `Required property '${requiredProperty}' is not defined.`, + message: `Required property '${requiredProperty}' is undefined.`, location: ctx.location.child(['required', i]), }); }