From 75ce359871adb824aab7d7215af0a2c9a381741b Mon Sep 17 00:00:00 2001 From: frantuma Date: Tue, 18 Jul 2023 08:58:11 +0200 Subject: [PATCH] fix(ls): fix validation for indirected local references Closes swagger-api/swagger-editor/issues/3626 --- package-lock.json | 4 -- .../apidom-ls/src/apidom-language-types.ts | 1 + .../services/completion/completion-service.ts | 2 +- .../services/validation/validation-service.ts | 15 ++--- packages/apidom-ls/src/utils/utils.ts | 31 ++++++++-- .../validation/oas/issue-editor-3626.yaml | 57 +++++++++++++++++++ packages/apidom-ls/test/validate.ts | 44 ++++++++++++++ 7 files changed, 136 insertions(+), 18 deletions(-) create mode 100644 packages/apidom-ls/test/fixtures/validation/oas/issue-editor-3626.yaml diff --git a/package-lock.json b/package-lock.json index fff3534c53..ee2eee5484 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12863,10 +12863,6 @@ "dev": true, "license": "ISC" }, - "node_modules/fsevents": { - "dev": true, - "optional": true - }, "node_modules/function-bind": { "version": "1.1.1", "license": "MIT" diff --git a/packages/apidom-ls/src/apidom-language-types.ts b/packages/apidom-ls/src/apidom-language-types.ts index 957d117c60..f24e17fff1 100644 --- a/packages/apidom-ls/src/apidom-language-types.ts +++ b/packages/apidom-ls/src/apidom-language-types.ts @@ -56,6 +56,7 @@ export enum Format { export type Pointer = { node: Element; ref: string; + isRef: boolean; }; export enum MergeStrategy { diff --git a/packages/apidom-ls/src/services/completion/completion-service.ts b/packages/apidom-ls/src/services/completion/completion-service.ts index 71f6b8b7e3..d100d74a45 100644 --- a/packages/apidom-ls/src/services/completion/completion-service.ts +++ b/packages/apidom-ls/src/services/completion/completion-service.ts @@ -894,7 +894,7 @@ export class DefaultCompletionService implements CompletionService { refElementType && refElementType.length > 0 ? refElementType : node.parent?.parent?.element; if (!nodeElement) return result; - const pointers = localReferencePointers(doc, nodeElement); + const pointers = localReferencePointers(doc, nodeElement, false); // build completion item let i = 97; for (const p of pointers) { diff --git a/packages/apidom-ls/src/services/validation/validation-service.ts b/packages/apidom-ls/src/services/validation/validation-service.ts index d77f1ec20b..086044bb19 100644 --- a/packages/apidom-ls/src/services/validation/validation-service.ts +++ b/packages/apidom-ls/src/services/validation/validation-service.ts @@ -208,7 +208,6 @@ export class DefaultValidationService implements ValidationService { } if (!result) return diagnostics; const { api } = result; - if (api === undefined) return diagnostics; const specVersion = getSpecVersion(api); @@ -224,7 +223,7 @@ export class DefaultValidationService implements ValidationService { if (refValueElement.toValue().startsWith('#')) { let pointers = pointersMap[referencedElement]; if (!pointers) { - pointers = localReferencePointers(doc, referencedElement); + pointers = localReferencePointers(doc, referencedElement, true); pointersMap[referencedElement] = pointers; } if (!pointers.some((p) => p.ref === refValueElement.toValue())) { @@ -250,11 +249,13 @@ export class DefaultValidationService implements ValidationService { } as LinterMetaData; for (const p of pointers) { // @ts-ignore - diagnostic.data.quickFix.push({ - message: `update to ${p.ref}`, - action: 'updateValue', - functionParams: [p.ref], - }); + if (refValueElement !== p.ref && !p.isRef) { + diagnostic.data.quickFix.push({ + message: `update to ${p.ref}`, + action: 'updateValue', + functionParams: [p.ref], + }); + } } // @ts-ignore this.quickFixesMap[code] = diagnostic.data.quickFix; diff --git a/packages/apidom-ls/src/utils/utils.ts b/packages/apidom-ls/src/utils/utils.ts index 40e43b13ec..e59bde2281 100644 --- a/packages/apidom-ls/src/utils/utils.ts +++ b/packages/apidom-ls/src/utils/utils.ts @@ -186,10 +186,19 @@ export function buildJsonPointer(path: string[]): string { return `#/${path.join('/')}`; } -export function localReferencePointers(doc: Element, nodeElement: string): Pointer[] { +interface FoundNode { + element: Element; + isRef: boolean; +} + +export function localReferencePointers( + doc: Element, + nodeElement: string, + includeRefs: boolean, +): Pointer[] { const pointers: Pointer[] = []; // traverse all doc to find nodes of the same type which are not a ref - const foundNodes: Element[] = []; + const foundNodes: FoundNode[] = []; let nodePath: string[] = []; function buildPointer(traverseNode: Element): void { if (!traverseNode) return; @@ -201,7 +210,13 @@ export function localReferencePointers(doc: Element, nodeElement: string): Point // TODO check for reference-element class or type instead function findRefNodes(traversedNode: Element): void { - if (traversedNode.element === nodeElement) { + if (includeRefs) { + const isRef = + nodeElement === traversedNode.getMetaProperty('referenced-element', '').toValue(); + if (isRef || traversedNode.element === nodeElement) { + foundNodes.push({ element: traversedNode, isRef }); + } + } else if (traversedNode.element === nodeElement) { if ( !( isObject(traversedNode) && @@ -209,15 +224,19 @@ export function localReferencePointers(doc: Element, nodeElement: string): Point traversedNode.get('$ref').toValue().length > 0 ) ) { - foundNodes.push(traversedNode); + foundNodes.push({ element: traversedNode, isRef: false }); } } } traverse(findRefNodes, doc); for (const foundNode of foundNodes) { nodePath = []; - buildPointer(foundNode); - pointers.push({ node: foundNode, ref: buildJsonPointer(nodePath) }); + buildPointer(foundNode.element); + pointers.push({ + node: foundNode.element, + ref: buildJsonPointer(nodePath), + isRef: foundNode.isRef, + }); } // TODO better sorting, NS plugin.. pointers.sort((a, b) => (a.ref.split('/').length > b.ref.split('/').length ? 1 : -1)); diff --git a/packages/apidom-ls/test/fixtures/validation/oas/issue-editor-3626.yaml b/packages/apidom-ls/test/fixtures/validation/oas/issue-editor-3626.yaml new file mode 100644 index 0000000000..8475ab09b7 --- /dev/null +++ b/packages/apidom-ls/test/fixtures/validation/oas/issue-editor-3626.yaml @@ -0,0 +1,57 @@ +openapi: 3.1.0 +info: + title: deref + version: 1.0.0 +servers: + - description: local + url: http://localhost:8082/ +paths: + /a: + get: + operationId: aget + parameters: + - $ref: '#/components/parameters/userId' + post: + operationId: apost + /b: + get: + operationId: bget + parameters: + - $ref: '#/components/parameters/userId' + post: + operationId: bpost + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/foo' +components: + schemas: + foo: + type: string + bar: + $id: http://localhost:8082/ + type: string + parameters: + userId: + $ref: '#/components/parameters/indirection1' + description: override + userIdBad: + $ref: '#/components/parameters/userIdRefBad' + description: override + indirection1: + $ref: '#/components/parameters/indirection2' + summary: indirect summary + indirection2: + $ref: '#/components/parameters/userIdRef' + summary: indirect summary + userIdRef: + name: userId + in: query + description: ID of the user + required: true + schema: + type: string + externalRef: + $ref: ./ex.json#/externalParameter + description: another ref diff --git a/packages/apidom-ls/test/validate.ts b/packages/apidom-ls/test/validate.ts index 0593b33883..4bbe3c0fd1 100644 --- a/packages/apidom-ls/test/validate.ts +++ b/packages/apidom-ls/test/validate.ts @@ -2997,4 +2997,48 @@ describe('apidom-ls-validate', function () { languageService.terminate(); }); + + it('oas / yaml - test editor issue 3626 / inidrect ref', async function () { + const validationContext: ValidationContext = { + comments: DiagnosticSeverity.Error, + maxNumberOfProblems: 100, + relatedInformation: false, + }; + + const spec = fs + .readFileSync(path.join(__dirname, 'fixtures', 'validation', 'oas', 'issue-editor-3626.yaml')) + .toString(); + const doc: TextDocument = TextDocument.create( + 'foo://bar/issue-editor-3626.yaml', + 'yaml', + 0, + spec, + ); + + const languageService: LanguageService = getLanguageService(contextNoSchema); + + const result = await languageService.doValidation(doc, validationContext); + result[0].code = 'test'; + const expected: Diagnostic[] = [ + { + range: { start: { line: 39, character: 12 }, end: { line: 39, character: 50 } }, + message: 'local reference not found', + severity: 1, + code: 'test', + source: 'apilint', + data: { + quickFix: [ + { + message: 'update to #/components/parameters/userIdRef', + action: 'updateValue', + functionParams: ['#/components/parameters/userIdRef'], + }, + ], + }, + }, + ]; + assert.deepEqual(result, expected as Diagnostic[]); + + languageService.terminate(); + }); });