From cde08f5b0ddc1fab0ad8c4ceb5b961e2a39210b2 Mon Sep 17 00:00:00 2001 From: JC Sackett Date: Thu, 28 Jul 2022 17:33:10 -0400 Subject: [PATCH 1/3] feat: allows arrays with oneOf/anyOf/allOf definitions to pass lint --- .../__tests__/property-rules.test.ts | 201 ++++++++++++++++++ .../rest/2022-05-25/property-rules.ts | 22 +- 2 files changed, 217 insertions(+), 6 deletions(-) diff --git a/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts b/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts index c20810dd..56a96efc 100644 --- a/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts +++ b/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts @@ -867,6 +867,207 @@ describe("body properties", () => { }); }); + describe("array properties", () => { + test("fails if items have no type information", () => { + const ruleRunner = new RuleRunner([propertyRules]); + const afterSpec: OpenAPIV3.Document = { + ...baseOpenAPI, + paths: { + "/example": { + get: { + responses: { + "200": { + description: "", + content: { + "application/json": { + schema: { + type: "object", + properties: { + things: { + type: "array", + items: {}, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + const ruleInputs = { + ...TestHelpers.createRuleInputs(afterSpec, afterSpec), + context, + }; + const results = ruleRunner.runRulesWithFacts(ruleInputs); + expect(results.every((result) => result.passed)).toBe(false); + expect(results.filter((result) => !result.passed)).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + error: "type was not found array items", + }), + ]), + ); + }); + test("succeeds if items have type", () => { + const ruleRunner = new RuleRunner([propertyRules]); + const afterSpec: OpenAPIV3.Document = { + ...baseOpenAPI, + paths: { + "/example": { + get: { + responses: { + "200": { + description: "", + content: { + "application/json": { + schema: { + type: "object", + properties: { + things: { + type: "array", + items: { + type: string, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + const ruleInputs = { + ...TestHelpers.createRuleInputs(afterSpec, afterSpec), + context, + }; + const results = ruleRunner.runRulesWithFacts(ruleInputs); + expect(results.every((result) => result.passed)).toBe(true); + }); + + test("succeeds if items have oneOf schema", () => { + const ruleRunner = new RuleRunner([propertyRules]); + const afterSpec: OpenAPIV3.Document = { + ...baseOpenAPI, + paths: { + "/example": { + get: { + responses: { + "200": { + description: "", + content: { + "application/json": { + schema: { + type: "object", + properties: { + things: { + type: "array", + items: { + oneOf: [], + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + const ruleInputs = { + ...TestHelpers.createRuleInputs(afterSpec, afterSpec), + context, + }; + const results = ruleRunner.runRulesWithFacts(ruleInputs); + expect(results.every((result) => result.passed)).toBe(true); + }); + + test("succeeds if items have allOf schema", () => { + const ruleRunner = new RuleRunner([propertyRules]); + const afterSpec: OpenAPIV3.Document = { + ...baseOpenAPI, + paths: { + "/example": { + get: { + responses: { + "200": { + description: "", + content: { + "application/json": { + schema: { + type: "object", + properties: { + things: { + type: "array", + items: { + allOf: [], + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + const ruleInputs = { + ...TestHelpers.createRuleInputs(afterSpec, afterSpec), + context, + }; + const results = ruleRunner.runRulesWithFacts(ruleInputs); + expect(results.every((result) => result.passed)).toBe(true); + }); + + test("succeeds if items have anyOf schema", () => { + const ruleRunner = new RuleRunner([propertyRules]); + const afterSpec: OpenAPIV3.Document = { + ...baseOpenAPI, + paths: { + "/example": { + get: { + responses: { + "200": { + description: "", + content: { + "application/json": { + schema: { + type: "object", + properties: { + things: { + type: "array", + items: { + anyOf: [], + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + const ruleInputs = { + ...TestHelpers.createRuleInputs(afterSpec, afterSpec), + context, + }; + const results = ruleRunner.runRulesWithFacts(ruleInputs); + expect(results.every((result) => result.passed)).toBe(true); + }); + }); + describe("breaking changes", () => { test("fails if a property is removed", () => { const ruleRunner = new RuleRunner([propertyRules]); diff --git a/src/rulesets/rest/2022-05-25/property-rules.ts b/src/rulesets/rest/2022-05-25/property-rules.ts index bbc915a7..d5a56a60 100644 --- a/src/rulesets/rest/2022-05-25/property-rules.ts +++ b/src/rulesets/rest/2022-05-25/property-rules.ts @@ -209,9 +209,14 @@ const arrayWithItemsInRequest = new RequestRule({ (property) => { if (property.raw.type === "array") { if (!property.raw.items || !("type" in property.raw.items)) { - throw new RuleError({ - message: "type was not found array items", - }); + const oneOf = property.raw.items["oneOf"]; + const allOf = property.raw.items["allOf"]; + const anyOf = property.raw.items["anyOf"]; + if (!oneOf && !allOf && !anyOf) { + throw new RuleError({ + message: "type was not found array items", + }); + } } } }, @@ -227,9 +232,14 @@ const arrayWithItemsInResponse = new ResponseBodyRule({ (property) => { if (property.raw.type === "array") { if (!property.raw.items || !("type" in property.raw.items)) { - throw new RuleError({ - message: "type was not found array items", - }); + const oneOf = property.raw.items["oneOf"]; + const allOf = property.raw.items["allOf"]; + const anyOf = property.raw.items["anyOf"]; + if (!oneOf && !allOf && !anyOf) { + throw new RuleError({ + message: "type was not found array items", + }); + } } } }, From 625b9382d727fd3dd481d0d3bc4e20fa1a57f8eb Mon Sep 17 00:00:00 2001 From: JC Sackett Date: Fri, 29 Jul 2022 14:13:10 -0400 Subject: [PATCH 2/3] fix: updates type in tests to pass tsc build --- src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts b/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts index 56a96efc..a4dd1539 100644 --- a/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts +++ b/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts @@ -929,7 +929,7 @@ describe("body properties", () => { things: { type: "array", items: { - type: string, + type: "string", }, }, }, From 96675cf89fffe04fd36efe09fbd37b13ded4a9aa Mon Sep 17 00:00:00 2001 From: JC Sackett Date: Fri, 29 Jul 2022 14:13:38 -0400 Subject: [PATCH 3/3] feat: moves items validation into helper function --- .../__tests__/property-rules.test.ts | 52 +++++++++++++++++-- .../rest/2022-05-25/property-rules.ts | 27 ++++------ src/rulesets/rest/2022-05-25/utils.ts | 24 +++++++++ 3 files changed, 82 insertions(+), 21 deletions(-) diff --git a/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts b/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts index a4dd1539..642a2a7d 100644 --- a/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts +++ b/src/rulesets/rest/2022-05-25/__tests__/property-rules.test.ts @@ -968,7 +968,7 @@ describe("body properties", () => { things: { type: "array", items: { - oneOf: [], + oneOf: [{ type: "string" }], }, }, }, @@ -1007,7 +1007,7 @@ describe("body properties", () => { things: { type: "array", items: { - allOf: [], + allOf: [{ type: "string" }], }, }, }, @@ -1046,7 +1046,7 @@ describe("body properties", () => { things: { type: "array", items: { - anyOf: [], + anyOf: [{ type: "string" }], }, }, }, @@ -1066,6 +1066,52 @@ describe("body properties", () => { const results = ruleRunner.runRulesWithFacts(ruleInputs); expect(results.every((result) => result.passed)).toBe(true); }); + + test("fails if composite in items has no type", () => { + const ruleRunner = new RuleRunner([propertyRules]); + const afterSpec: OpenAPIV3.Document = { + ...baseOpenAPI, + paths: { + "/example": { + get: { + responses: { + "200": { + description: "", + content: { + "application/json": { + schema: { + type: "object", + properties: { + things: { + type: "array", + items: { + anyOf: [], + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }, + }; + const ruleInputs = { + ...TestHelpers.createRuleInputs(afterSpec, afterSpec), + context, + }; + const results = ruleRunner.runRulesWithFacts(ruleInputs); + expect(results.every((result) => result.passed)).toBe(false); + expect(results.filter((result) => !result.passed)).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + error: "type was not found array items", + }), + ]), + ); + }); }); describe("breaking changes", () => { diff --git a/src/rulesets/rest/2022-05-25/property-rules.ts b/src/rulesets/rest/2022-05-25/property-rules.ts index d5a56a60..b0e936d3 100644 --- a/src/rulesets/rest/2022-05-25/property-rules.ts +++ b/src/rulesets/rest/2022-05-25/property-rules.ts @@ -10,6 +10,7 @@ import { isBreakingChangeAllowed, isCompiledOperationSunsetAllowed, isResourceMetaProperty, + isFullyTypedArray, specIsRemoved, } from "./utils"; @@ -208,15 +209,10 @@ const arrayWithItemsInRequest = new RequestRule({ "have type for array items", (property) => { if (property.raw.type === "array") { - if (!property.raw.items || !("type" in property.raw.items)) { - const oneOf = property.raw.items["oneOf"]; - const allOf = property.raw.items["allOf"]; - const anyOf = property.raw.items["anyOf"]; - if (!oneOf && !allOf && !anyOf) { - throw new RuleError({ - message: "type was not found array items", - }); - } + if (!isFullyTypedArray(property.raw)) { + throw new RuleError({ + message: "type was not found array items", + }); } } }, @@ -231,15 +227,10 @@ const arrayWithItemsInResponse = new ResponseBodyRule({ "have type for array items", (property) => { if (property.raw.type === "array") { - if (!property.raw.items || !("type" in property.raw.items)) { - const oneOf = property.raw.items["oneOf"]; - const allOf = property.raw.items["allOf"]; - const anyOf = property.raw.items["anyOf"]; - if (!oneOf && !allOf && !anyOf) { - throw new RuleError({ - message: "type was not found array items", - }); - } + if (!isFullyTypedArray(property.raw)) { + throw new RuleError({ + message: "type was not found array items", + }); } } }, diff --git a/src/rulesets/rest/2022-05-25/utils.ts b/src/rulesets/rest/2022-05-25/utils.ts index 13cad1d1..4cb32a7d 100644 --- a/src/rulesets/rest/2022-05-25/utils.ts +++ b/src/rulesets/rest/2022-05-25/utils.ts @@ -1,4 +1,5 @@ import { Field, RuleContext } from "@useoptic/rulesets-base"; +import { OpenAPIV3 } from "@useoptic/openapi-utilities"; export const isOpenApiPath = (path: string) => /\/openapi/.test(path); export const isSingletonPath = (rulesContext: RuleContext) => @@ -65,3 +66,26 @@ export const isResourceMetaProperty = (property: Field): boolean => { export const specIsRemoved = (spec): boolean => { return spec.change === "removed"; }; + +export const isFullyTypedArray = ( + array: OpenAPIV3.ArraySchemaObject, +): boolean => { + if (!array.items || !("type" in array.items)) { + for (const key of ["oneOf", "allOf", "anyOf"]) { + const composite = array.items[key]; + if (composite) { + if (composite.length === 0) { + return false; + } + for (const obj of composite) { + if (!("type" in obj)) { + return false; + } + } + return true; + } + } + return false; + } + return true; +};