Skip to content

Commit 8856aa1

Browse files
committed
refactor(rules): two use cases not handled by current implementation
1 parent 8becb8a commit 8856aa1

3 files changed

Lines changed: 185 additions & 1 deletion

File tree

.changeset/nasty-peas-agree.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@redocly/openapi-core': patch
3+
'@redocly/cli': patch
4+
---
5+
6+
Updated `no-required-schema-properties-undefined` rule to cover additional edge cases.

packages/core/src/rules/common/__tests__/no-required-schema-properties-undefined.test.ts

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,147 @@ describe('no-required-schema-properties-undefined', () => {
898898
`);
899899
});
900900

901+
it('should not report bare required when property is defined in parent allOf sibling', async () => {
902+
const document = parseYamlToDocument(
903+
outdent`
904+
openapi: 3.0.0
905+
components:
906+
schemas:
907+
PersonBase:
908+
type: object
909+
properties:
910+
personName:
911+
type: object
912+
properties:
913+
givenName:
914+
type: string
915+
familyName:
916+
type: string
917+
Person:
918+
type: object
919+
allOf:
920+
- $ref: '#/components/schemas/PersonBase'
921+
properties:
922+
personName:
923+
required:
924+
- givenName
925+
- familyName
926+
required:
927+
- personName
928+
`,
929+
'foobar.yaml'
930+
);
931+
932+
const results = await lintDocument({
933+
externalRefResolver: new BaseResolver(),
934+
document,
935+
config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }),
936+
});
937+
938+
expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`);
939+
});
940+
941+
it('should not report bare required in oneOf branches when property is defined in parent allOf sibling', async () => {
942+
const document = parseYamlToDocument(
943+
outdent`
944+
openapi: 3.0.0
945+
components:
946+
schemas:
947+
PersonBase:
948+
type: object
949+
properties:
950+
communication:
951+
type: object
952+
properties:
953+
landlines:
954+
type: array
955+
mobiles:
956+
type: array
957+
emails:
958+
type: array
959+
Person:
960+
type: object
961+
allOf:
962+
- $ref: '#/components/schemas/PersonBase'
963+
properties:
964+
communication:
965+
oneOf:
966+
- required:
967+
- landlines
968+
- required:
969+
- mobiles
970+
- required:
971+
- emails
972+
required:
973+
- communication
974+
`,
975+
'foobar.yaml'
976+
);
977+
978+
const results = await lintDocument({
979+
externalRefResolver: new BaseResolver(),
980+
document,
981+
config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }),
982+
});
983+
984+
expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`[]`);
985+
});
986+
987+
it('should report misspelled required property even when parent allOf sibling defines the property', async () => {
988+
const document = parseYamlToDocument(
989+
outdent`
990+
openapi: 3.0.0
991+
components:
992+
schemas:
993+
PersonBase:
994+
type: object
995+
properties:
996+
personName:
997+
type: object
998+
properties:
999+
givenName:
1000+
type: string
1001+
familyName:
1002+
type: string
1003+
Person:
1004+
type: object
1005+
allOf:
1006+
- $ref: '#/components/schemas/PersonBase'
1007+
properties:
1008+
personName:
1009+
required:
1010+
- giveName
1011+
required:
1012+
- personName
1013+
`,
1014+
'foobar.yaml'
1015+
);
1016+
1017+
const results = await lintDocument({
1018+
externalRefResolver: new BaseResolver(),
1019+
document,
1020+
config: await createConfig({ rules: { 'no-required-schema-properties-undefined': 'error' } }),
1021+
});
1022+
1023+
expect(replaceSourceWithRef(results)).toMatchInlineSnapshot(`
1024+
[
1025+
{
1026+
"location": [
1027+
{
1028+
"pointer": "#/components/schemas/Person/properties/personName/required/0",
1029+
"reportOnKey": false,
1030+
"source": "foobar.yaml",
1031+
},
1032+
],
1033+
"message": "Required property 'giveName' is not defined.",
1034+
"ruleId": "no-required-schema-properties-undefined",
1035+
"severity": "error",
1036+
"suggest": [],
1037+
},
1038+
]
1039+
`);
1040+
});
1041+
9011042
it('should not crash on unresolved $ref', async () => {
9021043
const document = parseYamlToDocument(
9031044
outdent`

packages/core/src/rules/common/no-required-schema-properties-undefined.ts

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,47 @@ export const NoRequiredSchemaPropertiesUndefined:
8181

8282
const compositionRoot = findCompositionRoot(parents.length - 2, currentSchema);
8383

84+
const hasPropertyInParentContext = (
85+
propertyName: string,
86+
targetSchema: AnySchema
87+
): boolean => {
88+
for (let i = parents.length - 2; i >= 0; i--) {
89+
const ancestor = parents[i];
90+
const props = ancestor.properties as Record<string, AnySchema> | undefined;
91+
if (!props) continue;
92+
93+
const propKey = (Object.keys(props) as string[]).find((k) => {
94+
const val = getOwn(props, k) as AnySchema;
95+
if (val === targetSchema) return true;
96+
return resolveSchema(val, ctx).schema === targetSchema;
97+
});
98+
if (!propKey) continue;
99+
100+
const checkSiblings = (siblings: AnySchema[] | undefined): boolean =>
101+
!!siblings?.some((sibling) => {
102+
const { schema: s, location } = resolveSchema(sibling, ctx);
103+
if (!s?.properties) return false;
104+
const propDef = getOwn(s.properties as Record<string, AnySchema>, propKey);
105+
if (propDef === undefined) return false;
106+
return hasProperty(propDef as AnySchema, propertyName, new Set(), location);
107+
});
108+
109+
if (
110+
checkSiblings(ancestor.allOf) ||
111+
checkSiblings(ancestor.anyOf) ||
112+
checkSiblings(ancestor.oneOf)
113+
) {
114+
return true;
115+
}
116+
}
117+
return false;
118+
};
119+
84120
for (const [i, requiredProperty] of currentSchema.required.entries()) {
85121
if (
86122
!hasProperty(currentSchema, requiredProperty, new Set()) &&
87-
!hasProperty(compositionRoot, requiredProperty, new Set())
123+
!hasProperty(compositionRoot, requiredProperty, new Set()) &&
124+
!hasPropertyInParentContext(requiredProperty, compositionRoot ?? currentSchema)
88125
) {
89126
ctx.report({
90127
message: `Required property '${requiredProperty}' is not defined.`,

0 commit comments

Comments
 (0)