Add schema info to diagnostics (#326)

* #310 Add schema info to diagnostics Signed-off-by: Yevhen Vydolob <[email protected]> * Show schema url if file has multiple schemas Signed-off-by: Yevhen Vydolob <[email protected]> * Fix k8s url Signed-off-by: Yevhen Vydolob <[email protected]> * Update test/schemaValidation.test.ts Co-authored-by: Josh Pinkney <[email protected]> * Remove commented code Signed-off-by: Yevhen Vydolob <[email protected]> * Set schema url and title to child schemas Signed-off-by: Yevhen Vydolob <[email protected]> Co-authored-by: Josh Pinkney <[email protected]>
redhat-developer · Oct 15, 2020 · c36aab6 · c36aab6
1 parent 9738eda
commit c36aab6
Show file tree

Hide file tree

Showing 9 changed files with 276 additions and 70 deletions.
diff --git a/src/languageservice/jsonSchema.ts b/src/languageservice/jsonSchema.ts
@@ -8,6 +8,7 @@ export interface JSONSchema {
   id?: string;
   $id?: string;
   $schema?: string;
+  url?: string;
   type?: string | string[];
   title?: string;
   // eslint-disable-next-line @typescript-eslint/no-explicit-any

diff --git a/src/languageservice/parser/jsonParser07.ts b/src/languageservice/parser/jsonParser07.ts
diff --git a/src/languageservice/services/yamlSchemaService.ts b/src/languageservice/services/yamlSchemaService.ts
@@ -162,6 +162,7 @@ export class YAMLSchemaService extends JSONSchemaService {
           );
         }
         merge(node, unresolvedSchema.schema, uri, linkPath);
+        node.url = uri;
         // eslint-disable-next-line @typescript-eslint/no-use-before-define
         return resolveRefs(node, unresolvedSchema.schema, uri, referencedHandle.dependencies);
       });
@@ -296,19 +297,21 @@ export class YAMLSchemaService extends JSONSchemaService {
       }
 
       if (schemas.length > 0) {
-        return super
-          .createCombinedSchema(resource, schemas)
-          .getResolvedSchema()
-          .then((schema) => {
-            if (
-              schema.schema &&
-              schema.schema.schemaSequence &&
-              schema.schema.schemaSequence[(<SingleYAMLDocument>doc).currentDocIndex]
-            ) {
-              return new ResolvedSchema(schema.schema.schemaSequence[(<SingleYAMLDocument>doc).currentDocIndex]);
-            }
-            return schema;
-          });
+        const schemaHandle = super.createCombinedSchema(resource, schemas);
+        return schemaHandle.getResolvedSchema().then((schema) => {
+          if (schema.schema) {
+            schema.schema.url = schemaHandle.url;
+          }
+
+          if (
+            schema.schema &&
+            schema.schema.schemaSequence &&
+            schema.schema.schemaSequence[(<SingleYAMLDocument>doc).currentDocIndex]
+          ) {
+            return new ResolvedSchema(schema.schema.schemaSequence[(<SingleYAMLDocument>doc).currentDocIndex]);
+          }
+          return schema;
+        });
       }
 
       return Promise.resolve(null);
@@ -389,6 +392,9 @@ export class YAMLSchemaService extends JSONSchemaService {
   private async resolveCustomSchema(schemaUri, doc): ResolvedSchema {
     const unresolvedSchema = await this.loadSchema(schemaUri);
     const schema = await this.resolveSchemaContent(unresolvedSchema, schemaUri, []);
+    if (schema.schema) {
+      schema.schema.url = schemaUri;
+    }
     if (schema.schema && schema.schema.schemaSequence && schema.schema.schemaSequence[doc.currentDocIndex]) {
       return new ResolvedSchema(schema.schema.schemaSequence[doc.currentDocIndex]);
     }
@@ -542,7 +548,7 @@ export class YAMLSchemaService extends JSONSchemaService {
           }
         );
       }
-
+      unresolvedJsonSchema.uri = schemaUri;
       return unresolvedJsonSchema;
     });
   }

diff --git a/src/languageservice/services/yamlValidation.ts b/src/languageservice/services/yamlValidation.ts
@@ -5,13 +5,15 @@
  *--------------------------------------------------------------------------------------------*/
 'use strict';
 
-import { Diagnostic, TextDocument } from 'vscode-languageserver-types';
+import { Diagnostic } from 'vscode-languageserver-types';
 import { PromiseConstructor, LanguageSettings } from '../yamlLanguageService';
 import { parse as parseYAML, YAMLDocument } from '../parser/yamlParser07';
 import { SingleYAMLDocument } from '../parser/yamlParser07';
 import { YAMLSchemaService } from './yamlSchemaService';
-import { JSONValidation } from 'vscode-json-languageservice/lib/umd/services/jsonValidation';
 import { YAMLDocDiagnostic } from '../utils/parseUtils';
+import { TextDocument } from 'vscode-languageserver';
+import { JSONValidation } from 'vscode-json-languageservice/lib/umd/services/jsonValidation';
+import { YAML_SOURCE } from '../parser/jsonParser07';
 
 /**
  * Convert a YAMLDocDiagnostic to a language server Diagnostic
@@ -24,11 +26,7 @@ export const yamlDiagToLSDiag = (yamlDiag: YAMLDocDiagnostic, textDocument: Text
     end: textDocument.positionAt(yamlDiag.location.end),
   };
 
-  return {
-    message: yamlDiag.message,
-    range,
-    severity: yamlDiag.severity,
-  };
+  return Diagnostic.create(range, yamlDiag.message, yamlDiag.severity, undefined, YAML_SOURCE);
 };
 
 export class YAMLValidation {
@@ -66,6 +64,7 @@ export class YAMLValidation {
       currentYAMLDoc.currentDocIndex = index;
 
       const validation = await this.jsonValidation.doValidation(textDocument, currentYAMLDoc);
+
       const syd = (currentYAMLDoc as unknown) as SingleYAMLDocument;
       if (syd.errors.length > 0) {
         // TODO: Get rid of these type assertions (shouldn't need them)
@@ -95,6 +94,10 @@ export class YAMLValidation {
         err = yamlDiagToLSDiag(err, textDocument);
       }
 
+      if (!err.source) {
+        err.source = YAML_SOURCE;
+      }
+
       const errSig = err.range.start.line + ' ' + err.range.start.character + ' ' + err.message;
       if (!foundSignatures.has(errSig)) {
         duplicateMessagesRemoved.push(err);

diff --git a/src/languageservice/utils/parseUtils.ts b/src/languageservice/utils/parseUtils.ts
@@ -16,6 +16,7 @@ export interface YAMLDocDiagnostic {
     end: number;
   };
   severity: 1 | 2;
+  source?: string;
 }
 
 /**

diff --git a/test/schema.test.ts b/test/schema.test.ts
@@ -4,10 +4,11 @@ import * as assert from 'assert';
 import * as parser from '../src/languageservice/parser/yamlParser07';
 import * as SchemaService from '../src/languageservice/services/yamlSchemaService';
 import * as JsonSchema from '../src/languageservice/jsonSchema';
-import url = require('url');
+import * as url from 'url';
 import { XHRResponse, xhr } from 'request-light';
 import { MODIFICATION_ACTIONS, SchemaDeletions } from '../src/languageservice/services/yamlSchemaService';
 import { KUBERNETES_SCHEMA_URL } from '../src/languageservice/utils/schemaUrls';
+import { expect } from 'chai';
 
 const requestServiceMock = function (uri: string): Promise<string> {
   return Promise.reject<string>(`Resource ${uri} not found.`);
@@ -60,6 +61,7 @@ suite('JSON Schema', () => {
           id: 'https://myschemastore/child',
           type: 'bool',
           description: 'Test description',
+          url: 'https://myschemastore/child',
         });
       })
       .then(
@@ -155,14 +157,17 @@ suite('JSON Schema', () => {
         assert.deepEqual(fs.schema.properties['p1'], {
           type: 'string',
           enum: ['object'],
+          url: 'https://myschemastore/main/schema2.json',
         });
         assert.deepEqual(fs.schema.properties['p2'], {
           type: 'string',
           enum: ['object'],
+          url: 'https://myschemastore/main/schema2.json',
         });
         assert.deepEqual(fs.schema.properties['p3'], {
           type: 'string',
           enum: ['object'],
+          url: 'https://myschemastore/main/schema2.json',
         });
       })
       .then(
@@ -325,6 +330,31 @@ suite('JSON Schema', () => {
       );
   });
 
+  test('Schema has url', async () => {
+    const service = new SchemaService.YAMLSchemaService(requestServiceMock, workspaceContext);
+    const id = 'https://myschemastore/test1';
+    const schema: JsonSchema.JSONSchema = {
+      type: 'object',
+      properties: {
+        child: {
+          type: 'object',
+          properties: {
+            grandchild: {
+              type: 'number',
+              description: 'Meaning of Life',
+            },
+          },
+        },
+      },
+    };
+
+    service.registerExternalSchema(id, ['*.json'], schema);
+
+    const result = await service.getSchemaForResource('test.json', undefined);
+
+    expect(result.schema.url).equal(id);
+  });
+
   test('Null Schema', function (testDone) {
     const service = new SchemaService.YAMLSchemaService(requestServiceMock, workspaceContext);
 

diff --git a/test/schemaValidation.test.ts b/test/schemaValidation.test.ts
@@ -2,7 +2,7 @@
  *  Copyright (c) Red Hat. All rights reserved.
  *  Licensed under the MIT License. See License.txt in the project root for license information.
  *--------------------------------------------------------------------------------------------*/
-import { configureLanguageService, SCHEMA_ID, setupSchemaIDTextDocument } from './utils/testHelper';
+import { configureLanguageService, SCHEMA_ID, setupSchemaIDTextDocument, setupTextDocument } from './utils/testHelper';
 import { createExpectedError } from './utils/verifyError';
 import { ServiceSetup } from './utils/serviceSetup';
 import {
@@ -14,9 +14,12 @@ import {
   ColonMissingError,
   BlockMappingEntryError,
   DuplicateKeyError,
+  propertyIsNotAllowed,
 } from './utils/errorMessages';
 import * as assert from 'assert';
-import { Diagnostic } from 'vscode-languageserver';
+import { Diagnostic, DiagnosticSeverity, TextDocument } from 'vscode-languageserver';
+import { expect } from 'chai';
+import { KUBERNETES_SCHEMA_URL } from '../src/languageservice/utils/schemaUrls';
 
 const languageSettingsSetup = new ServiceSetup().withValidate().withCustomTags(['!Test', '!Ref sequence']);
 const languageService = configureLanguageService(languageSettingsSetup.languageSettings);
@@ -101,7 +104,10 @@ suite('Validation Tests', () => {
       validator
         .then(function (result) {
           assert.equal(result.length, 1);
-          assert.deepEqual(result[0], createExpectedError(BooleanTypeError, 0, 11, 0, 15));
+          assert.deepEqual(
+            result[0],
+            createExpectedError(BooleanTypeError, 0, 11, 0, 15, DiagnosticSeverity.Warning, `yaml-schema: file:///${SCHEMA_ID}`)
+          );
         })
         .then(done, done);
     });
@@ -120,7 +126,10 @@ suite('Validation Tests', () => {
       validator
         .then(function (result) {
           assert.equal(result.length, 1);
-          assert.deepEqual(result[0], createExpectedError(StringTypeError, 0, 5, 0, 10));
+          assert.deepEqual(
+            result[0],
+            createExpectedError(StringTypeError, 0, 5, 0, 10, DiagnosticSeverity.Warning, `yaml-schema: file:///${SCHEMA_ID}`)
+          );
         })
         .then(done, done);
     });
@@ -200,7 +209,10 @@ suite('Validation Tests', () => {
       validator
         .then(function (result) {
           assert.equal(result.length, 1);
-          assert.deepEqual(result[0], createExpectedError(BooleanTypeError, 0, 11, 0, 16));
+          assert.deepEqual(
+            result[0],
+            createExpectedError(BooleanTypeError, 0, 11, 0, 16, DiagnosticSeverity.Warning, `yaml-schema: file:///${SCHEMA_ID}`)
+          );
         })
         .then(done, done);
     });
@@ -219,7 +231,10 @@ suite('Validation Tests', () => {
       validator
         .then(function (result) {
           assert.equal(result.length, 1);
-          assert.deepEqual(result[0], createExpectedError(StringTypeError, 0, 5, 0, 7));
+          assert.deepEqual(
+            result[0],
+            createExpectedError(StringTypeError, 0, 5, 0, 7, DiagnosticSeverity.Warning, `yaml-schema: file:///${SCHEMA_ID}`)
+          );
         })
         .then(done, done);
     });
@@ -258,7 +273,10 @@ suite('Validation Tests', () => {
       validator
         .then(function (result) {
           assert.equal(result.length, 1);
-          assert.deepEqual(result[0], createExpectedError(StringTypeError, 0, 5, 0, 11));
+          assert.deepEqual(
+            result[0],
+            createExpectedError(StringTypeError, 0, 5, 0, 11, DiagnosticSeverity.Warning, `yaml-schema: file:///${SCHEMA_ID}`)
+          );
         })
         .then(done, done);
     });
@@ -365,7 +383,10 @@ suite('Validation Tests', () => {
       validator
         .then(function (result) {
           assert.equal(result.length, 1);
-          assert.deepEqual(result[0], createExpectedError(ObjectTypeError, 0, 9, 0, 13));
+          assert.deepEqual(
+            result[0],
+            createExpectedError(ObjectTypeError, 0, 9, 0, 13, DiagnosticSeverity.Warning, `yaml-schema: file:///${SCHEMA_ID}`)
+          );
         })
         .then(done, done);
     });
@@ -407,7 +428,10 @@ suite('Validation Tests', () => {
       validator
         .then(function (result) {
           assert.equal(result.length, 1);
-          assert.deepEqual(result[0], createExpectedError(ArrayTypeError, 0, 11, 0, 15));
+          assert.deepEqual(
+            result[0],
+            createExpectedError(ArrayTypeError, 0, 11, 0, 15, DiagnosticSeverity.Warning, `yaml-schema: file:///${SCHEMA_ID}`)
+          );
         })
         .then(done, done);
     });
@@ -637,7 +661,10 @@ suite('Validation Tests', () => {
       validator
         .then(function (result) {
           assert.equal(result.length, 1);
-          assert.deepEqual(result[0], createExpectedError(StringTypeError, 0, 4, 0, 4));
+          assert.deepEqual(
+            result[0],
+            createExpectedError(StringTypeError, 0, 4, 0, 4, DiagnosticSeverity.Warning, `yaml-schema: file:///${SCHEMA_ID}`)
+          );
         })
         .then(done, done);
     });
@@ -815,4 +842,79 @@ suite('Validation Tests', () => {
         .then(done, done);
     });
   });
+
+  describe('Schema with title', () => {
+    it('validator uses schema title instead of url', async () => {
+      languageService.addSchema(SCHEMA_ID, {
+        type: 'object',
+        title: 'Schema Super title',
+        properties: {
+          analytics: {
+            type: 'string',
+          },
+        },
+      });
+      const content = 'analytics: 1';
+      const result = await parseSetup(content);
+      expect(result[0]).deep.equal(
+        createExpectedError(StringTypeError, 0, 11, 0, 12, DiagnosticSeverity.Warning, 'yaml-schema: Schema Super title')
+      );
+    });
+  });
+
+  describe('Multiple schema for single file', () => {
+    it('should add proper source to diagnostic', async () => {
+      const languageSettingsSetup = new ServiceSetup()
+        .withValidate()
+        .withSchemaFileMatch({ uri: KUBERNETES_SCHEMA_URL, fileMatch: ['test.yaml'] })
+        .withSchemaFileMatch({ uri: 'https://json.schemastore.org/composer', fileMatch: ['test.yaml'] });
+      const languageService = configureLanguageService(languageSettingsSetup.languageSettings);
+
+      const content = `
+      abandoned: v1
+      archive:
+        exclude:
+          asd: asd`;
+      const testTextDocument = setupTextDocument(content);
+      const result = await languageService.doValidation(testTextDocument, true);
+      expect(result[0]).deep.equal(
+        createExpectedError(
+          ArrayTypeError,
+          4,
+          10,
+          4,
+          18,
+          DiagnosticSeverity.Warning,
+          'yaml-schema: https://json.schemastore.org/composer'
+        )
+      );
+    });
+
+    it('should add proper source to diagnostic in case of drone', async () => {
+      const languageSettingsSetup = new ServiceSetup()
+        .withValidate()
+        .withSchemaFileMatch({ uri: KUBERNETES_SCHEMA_URL, fileMatch: ['.drone.yml'] })
+        .withSchemaFileMatch({ uri: 'https://json.schemastore.org/drone', fileMatch: ['.drone.yml'] });
+      const languageService = configureLanguageService(languageSettingsSetup.languageSettings);
+
+      const content = `
+      apiVersion: v1
+      kind: Deployment
+      `;
+
+      const testTextDocument = TextDocument.create('file://~/Desktop/vscode-yaml/.drone.yml', 'yaml', 0, content);
+      const result = await languageService.doValidation(testTextDocument, true);
+      expect(result[5]).deep.equal(
+        createExpectedError(
+          propertyIsNotAllowed('apiVersion'),
+          1,
+          6,
+          1,
+          16,
+          DiagnosticSeverity.Warning,
+          'yaml-schema: Drone CI configuration file'
+        )
+      );
+    });
+  });
 });