From 0055a09368d60a07f2e708ae55bc1d9d6b4857d7 Mon Sep 17 00:00:00 2001 From: Krzysztof Kowalczyk Date: Mon, 18 Dec 2023 13:47:21 +0100 Subject: [PATCH] feat(ls): add path template linting rule (#3538) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This linting rule that validates if the template expressions defined within the path template are all present in the appropriate Parameter Objects. Refs #3517 Co-authored-by: Vladimír Gorej --- package-lock.json | 12 +- packages/apidom-ls/package.json | 2 +- .../services/validation/linter-functions.ts | 59 +++++- .../oas/path-template-all-defined-2-0.yaml | 104 +++++++++++ .../oas/path-template-all-defined-3-0.yaml | 105 +++++++++++ .../oas/path-template-all-defined-3-1.yaml | 105 +++++++++++ packages/apidom-ls/test/validate.ts | 171 ++++++++++++++++++ 7 files changed, 549 insertions(+), 9 deletions(-) create mode 100644 packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-2-0.yaml create mode 100644 packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-0.yaml create mode 100644 packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-1.yaml diff --git a/package-lock.json b/package-lock.json index 14d90038d..727fa1123 100644 --- a/package-lock.json +++ b/package-lock.json @@ -29375,11 +29375,12 @@ } }, "node_modules/openapi-path-templating": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/openapi-path-templating/-/openapi-path-templating-1.2.0.tgz", - "integrity": "sha512-m1f9ws9Zn/1CjaBPxECzLmtaTyFwO79eL53pgHUmjCBJQShtVo3vhM0yGOyPF3dxsQ0FYrX+zfflkvaw4+PlNg==", + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/openapi-path-templating/-/openapi-path-templating-1.3.0.tgz", + "integrity": "sha512-nXA8Y5jvZc8mSxDNgy8ov4BQayshc2G6ZDThMOtghbQS5iOUq1lukOTVzvnFMcyl7fithDgvyuA+V8O0W/CASw==", "dependencies": { - "apg-lite": "^1.0.2" + "apg-lite": "^1.0.2", + "prettier": "^3.1.1" }, "engines": { "node": ">=14.16" @@ -31764,7 +31765,6 @@ "version": "3.1.1", "resolved": "https://registry.npmjs.org/prettier/-/prettier-3.1.1.tgz", "integrity": "sha512-22UbSzg8luF4UuZtzgiUOfcGM8s4tjBv6dJRT7j275NXsy2jb4aJa4NNveul5x4eqlF1wuhuR2RElK71RvmVaw==", - "dev": true, "bin": { "prettier": "bin/prettier.cjs" }, @@ -38062,7 +38062,7 @@ "@swagger-api/apidom-parser-adapter-yaml-1-2": "^0.88.0", "@swagger-api/apidom-reference": "^0.88.0", "@types/ramda": "~0.29.6", - "openapi-path-templating": "^1.2.0", + "openapi-path-templating": "^1.3.0", "ramda": "~0.29.1", "ramda-adjunct": "^4.1.1", "vscode-languageserver-protocol": "^3.17.2", diff --git a/packages/apidom-ls/package.json b/packages/apidom-ls/package.json index a767dbc10..e25a2aa30 100644 --- a/packages/apidom-ls/package.json +++ b/packages/apidom-ls/package.json @@ -116,7 +116,7 @@ "@swagger-api/apidom-parser-adapter-yaml-1-2": "^0.88.0", "@swagger-api/apidom-reference": "^0.88.0", "@types/ramda": "~0.29.6", - "openapi-path-templating": "^1.2.0", + "openapi-path-templating": "^1.3.0", "ramda": "~0.29.1", "ramda-adjunct": "^4.1.1", "vscode-languageserver-protocol": "^3.17.2", diff --git a/packages/apidom-ls/src/services/validation/linter-functions.ts b/packages/apidom-ls/src/services/validation/linter-functions.ts index c595ff234..b453b8ac6 100644 --- a/packages/apidom-ls/src/services/validation/linter-functions.ts +++ b/packages/apidom-ls/src/services/validation/linter-functions.ts @@ -8,9 +8,10 @@ import { toValue, ArraySlice, ObjectElement, + isArrayElement, } from '@swagger-api/apidom-core'; import { CompletionItem } from 'vscode-languageserver-types'; -import { test } from 'openapi-path-templating'; +import { test, resolve } from 'openapi-path-templating'; // eslint-disable-next-line import/no-cycle import { @@ -1009,8 +1010,62 @@ export const standardLinterfunctions: FunctionItem[] = [ functionName: 'apilintOpenAPIPathTemplateValid', function: (element: Element) => { if (isStringElement(element)) { - return true; + const pathItemElement = (element.parent as MemberElement).value as ObjectElement; + + if (pathItemElement.length === 0) { + return true; + } + + let oneOfParametersIsReferenceObject = false; + const parameterElements: Element[] = []; + const isParameterElement = (el: Element): boolean => el.element === 'parameter'; + const isReferenceElement = (el: Element): boolean => el.element === 'reference'; + + const pathItemParameterElements = pathItemElement.get('parameters'); + if (isArrayElement(pathItemParameterElements)) { + pathItemParameterElements.forEach((parameter) => { + if (isReferenceElement(parameter) && !oneOfParametersIsReferenceObject) { + oneOfParametersIsReferenceObject = true; + } + if (isParameterElement(parameter)) { + parameterElements.push(parameter); + } + }); + } + + pathItemElement.forEach((el) => { + if (el.element === 'operation') { + const operationParameterElements = (el as ObjectElement).get('parameters'); + if (isArrayElement(operationParameterElements)) { + operationParameterElements.forEach((parameter) => { + if (isReferenceElement(parameter) && !oneOfParametersIsReferenceObject) { + oneOfParametersIsReferenceObject = true; + } + if (isParameterElement(parameter)) { + parameterElements.push(parameter); + } + }); + } + } + }); + + const allowedLocation = ['path', 'query']; + const pathTemplateResolveParams: { [key: string]: 'placeholder' } = {}; + + parameterElements.forEach((parameter) => { + if (allowedLocation.includes(toValue((parameter as ObjectElement).get('in')))) { + pathTemplateResolveParams[toValue((parameter as ObjectElement).get('name'))] = + 'placeholder'; + } + }); + + const pathTemplate = toValue(element); + const resolvedPathTemplate = resolve(pathTemplate, pathTemplateResolveParams); + const includesTemplateExpression = test(resolvedPathTemplate, { strict: true }); + + return !includesTemplateExpression || oneOfParametersIsReferenceObject; } + return true; }, }, diff --git a/packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-2-0.yaml b/packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-2-0.yaml new file mode 100644 index 000000000..45ddea638 --- /dev/null +++ b/packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-2-0.yaml @@ -0,0 +1,104 @@ +swagger: '2.0' +info: + title: Foo + version: 0.1.0 +parameters: + test_id: + name: test_id + in: path + required: true + schema: + type: string + format: uuid + title: Test Id +paths: + /foo/bar/{baz}/test/{foo_id}/baz/{bar_id}: + delete: + summary: Delete foo bar test baz + operationId: deleteFooBarTestBaz + parameters: + - name: foo_id + in: path + required: true + schema: + type: string + format: uuid + title: Foo Id + - name: bar_id + in: path + required: true + schema: + type: string + format: uuid + title: Bar Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + /test/{foo_id}/{bar_id}: + get: + summary: Get test foo bar + operationId: getTestFooBar + parameters: + - name: foo_id + in: path + required: true + schema: + type: string + format: uuid + title: Foo Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + /reference/{test_id}/{baz_id}: + get: + summary: Get test baz + operationId: getReferenceTestBaz + parameters: + - $ref: '#/parameters/test_id' + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + /just_parameters_object/{x_id}/{y_id}: + parameters: + - name: x_id + in: path + required: true + schema: + type: string + format: uuid + title: X Id + /both_parameters_and_operations_object/{a_id}/{b_id}/{c_id}: + get: + summary: Get both parameters and operations object a b c + operationId: getBothParametersAndOperationsObjectABC + parameters: + - name: b_id + in: path + required: true + schema: + type: string + format: uuid + title: B Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + parameters: + - name: a_id + in: path + required: true + schema: + type: string + format: uuid + title: A Id diff --git a/packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-0.yaml b/packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-0.yaml new file mode 100644 index 000000000..cb1fc8c37 --- /dev/null +++ b/packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-0.yaml @@ -0,0 +1,105 @@ +openapi: 3.0.0 +info: + title: Foo + version: 0.1.0 +components: + parameters: + test_id: + name: test_id + in: path + required: true + schema: + type: string + format: uuid + title: Test Id +paths: + /foo/bar/{baz}/test/{foo_id}/baz/{bar_id}: + delete: + summary: Delete foo bar test baz + operationId: deleteFooBarTestBaz + parameters: + - name: foo_id + in: path + required: true + schema: + type: string + format: uuid + title: Foo Id + - name: bar_id + in: path + required: true + schema: + type: string + format: uuid + title: Bar Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + /test/{foo_id}/{bar_id}: + get: + summary: Get test foo bar + operationId: getTestFooBar + parameters: + - name: foo_id + in: path + required: true + schema: + type: string + format: uuid + title: Foo Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + /reference/{test_id}/{baz_id}: + get: + summary: Get test baz + operationId: getReferenceTestBaz + parameters: + - $ref: '#/components/parameters/test_id' + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + /just_parameters_object/{x_id}/{y_id}: + parameters: + - name: x_id + in: path + required: true + schema: + type: string + format: uuid + title: X Id + /both_parameters_and_operations_object/{a_id}/{b_id}/{c_id}: + get: + summary: Get both parameters and operations object a b c + operationId: getBothParametersAndOperationsObjectABC + parameters: + - name: b_id + in: path + required: true + schema: + type: string + format: uuid + title: B Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + parameters: + - name: a_id + in: path + required: true + schema: + type: string + format: uuid + title: A Id diff --git a/packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-1.yaml b/packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-1.yaml new file mode 100644 index 000000000..be7d8d82b --- /dev/null +++ b/packages/apidom-ls/test/fixtures/validation/oas/path-template-all-defined-3-1.yaml @@ -0,0 +1,105 @@ +openapi: 3.1.0 +info: + title: Foo + version: 0.1.0 +components: + parameters: + test_id: + name: test_id + in: path + required: true + schema: + type: string + format: uuid + title: Test Id +paths: + /foo/bar/{baz}/test/{foo_id}/baz/{bar_id}: + delete: + summary: Delete foo bar test baz + operationId: deleteFooBarTestBaz + parameters: + - name: foo_id + in: path + required: true + schema: + type: string + format: uuid + title: Foo Id + - name: bar_id + in: path + required: true + schema: + type: string + format: uuid + title: Bar Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + /test/{foo_id}/{bar_id}: + get: + summary: Get test foo bar + operationId: getTestFooBar + parameters: + - name: foo_id + in: path + required: true + schema: + type: string + format: uuid + title: Foo Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + /reference/{test_id}/{baz_id}: + get: + summary: Get test baz + operationId: getReferenceTestBaz + parameters: + - $ref: '#/components/parameters/test_id' + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + /just_parameters_object/{x_id}/{y_id}: + parameters: + - name: x_id + in: path + required: true + schema: + type: string + format: uuid + title: X Id + /both_parameters_and_operations_object/{a_id}/{b_id}/{c_id}: + get: + summary: Get both parameters and operations object a b c + operationId: getBothParametersAndOperationsObjectABC + parameters: + - name: b_id + in: path + required: true + schema: + type: string + format: uuid + title: B Id + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + parameters: + - name: a_id + in: path + required: true + schema: + type: string + format: uuid + title: A Id diff --git a/packages/apidom-ls/test/validate.ts b/packages/apidom-ls/test/validate.ts index 5446403be..8e4aa6658 100644 --- a/packages/apidom-ls/test/validate.ts +++ b/packages/apidom-ls/test/validate.ts @@ -3378,4 +3378,175 @@ describe('apidom-ls-validate', function () { languageService.terminate(); }); + + it('oas 2.0 / yaml - every path template should be defined', async function () { + const validationContext: ValidationContext = { + comments: DiagnosticSeverity.Error, + maxNumberOfProblems: 100, + relatedInformation: false, + }; + + const spec = fs + .readFileSync( + path.join(__dirname, 'fixtures', 'validation', 'oas', 'path-template-all-defined-2-0.yaml'), + ) + .toString(); + const doc: TextDocument = TextDocument.create( + 'foo://bar/path-template-all-defined-2-0.yaml', + 'yaml', + 0, + spec, + ); + + const languageService: LanguageService = getLanguageService(contextNoSchema); + + const result = await languageService.doValidation(doc, validationContext); + const expected: Diagnostic[] = [ + { + range: { start: { line: 14, character: 2 }, end: { line: 14, character: 43 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + { + range: { start: { line: 39, character: 2 }, end: { line: 39, character: 25 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + { + range: { start: { line: 69, character: 2 }, end: { line: 69, character: 39 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + { + range: { start: { line: 78, character: 2 }, end: { line: 78, character: 61 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + ]; + assert.deepEqual(result, expected as Diagnostic[]); + + languageService.terminate(); + }); + + it('oas 3.0 / yaml - every path template should be defined', async function () { + const validationContext: ValidationContext = { + comments: DiagnosticSeverity.Error, + maxNumberOfProblems: 100, + relatedInformation: false, + }; + + const spec = fs + .readFileSync( + path.join(__dirname, 'fixtures', 'validation', 'oas', 'path-template-all-defined-3-0.yaml'), + ) + .toString(); + const doc: TextDocument = TextDocument.create( + 'foo://bar/path-template-all-defined-3-0.yaml', + 'yaml', + 0, + spec, + ); + + const languageService: LanguageService = getLanguageService(contextNoSchema); + + const result = await languageService.doValidation(doc, validationContext); + const expected: Diagnostic[] = [ + { + range: { start: { line: 15, character: 2 }, end: { line: 15, character: 43 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + { + range: { start: { line: 40, character: 2 }, end: { line: 40, character: 25 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + { + range: { start: { line: 70, character: 2 }, end: { line: 70, character: 39 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + { + range: { start: { line: 79, character: 2 }, end: { line: 79, character: 61 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + ]; + assert.deepEqual(result, expected as Diagnostic[]); + + languageService.terminate(); + }); + + it('oas 3.1 / yaml - every path template should be defined', async function () { + const validationContext: ValidationContext = { + comments: DiagnosticSeverity.Error, + maxNumberOfProblems: 100, + relatedInformation: false, + }; + + const spec = fs + .readFileSync( + path.join(__dirname, 'fixtures', 'validation', 'oas', 'path-template-all-defined-3-1.yaml'), + ) + .toString(); + const doc: TextDocument = TextDocument.create( + 'foo://bar/path-template-all-defined-3-1.yaml', + 'yaml', + 0, + spec, + ); + + const languageService: LanguageService = getLanguageService(contextNoSchema); + + const result = await languageService.doValidation(doc, validationContext); + const expected: Diagnostic[] = [ + { + range: { start: { line: 15, character: 2 }, end: { line: 15, character: 43 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + { + range: { start: { line: 40, character: 2 }, end: { line: 40, character: 25 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + { + range: { start: { line: 70, character: 2 }, end: { line: 70, character: 39 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + { + range: { start: { line: 79, character: 2 }, end: { line: 79, character: 61 } }, + message: 'path template expressions is not matched with Parameter Object(s)', + severity: 1, + code: 3040101, + source: 'apilint', + }, + ]; + assert.deepEqual(result, expected as Diagnostic[]); + + languageService.terminate(); + }); });