Skip to content

Commit

Permalink
feat: multiple schema distinction in validation (#410)
Browse files Browse the repository at this point in the history
* feat: add multiple schema distinction in validation

* chore: multiple schema distinction
 - add new end line
 - modify warning message consts

Co-authored-by: Petr Spacek <[email protected]>
  • Loading branch information
p-spacek and Petr Spacek committed Feb 23, 2021
1 parent f0c5594 commit ce66564
Show file tree
Hide file tree
Showing 13 changed files with 371 additions and 22 deletions.
1 change: 1 addition & 0 deletions src/languageservice/jsonSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export interface JSONSchema {
multipleOf?: number;
required?: string[];
$ref?: string;
_$ref?: string;
anyOf?: JSONSchemaRef[];
allOf?: JSONSchemaRef[];
oneOf?: JSONSchemaRef[];
Expand Down
105 changes: 95 additions & 10 deletions src/languageservice/parser/jsonParser07.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import * as Json from 'jsonc-parser';
import { JSONSchema, JSONSchemaRef } from '../jsonSchema';
import { isNumber, equals, isString, isDefined, isBoolean } from '../utils/objects';
import { getSchemaTypeName } from '../utils/schemaUtils';
import {
ASTNode,
ObjectASTNode,
Expand All @@ -23,6 +24,7 @@ import { URI } from 'vscode-uri';
import { DiagnosticSeverity, Range } from 'vscode-languageserver-types';
import { TextDocument } from 'vscode-languageserver-textdocument';
import { Diagnostic } from 'vscode-languageserver';
import { isArrayEqual } from '../utils/arrUtils';

const localize = nls.loadMessageBundle();

Expand Down Expand Up @@ -55,18 +57,32 @@ const formats = {
};

export const YAML_SOURCE = 'YAML';
const YAML_SCHEMA_PREFIX = 'yaml-schema: ';

export enum ProblemType {
missingRequiredPropWarning = 'missingRequiredPropWarning',
typeMismatchWarning = 'typeMismatchWarning',
constWarning = 'constWarning',
}

const ProblemTypeMessages: Record<ProblemType, string> = {
[ProblemType.missingRequiredPropWarning]: 'Missing property "{0}".',
[ProblemType.typeMismatchWarning]: 'Incorrect type. Expected "{0}".',
[ProblemType.constWarning]: 'Value must be {0}.',
};
export interface IProblem {
location: IRange;
severity: DiagnosticSeverity;
code?: ErrorCode;
message: string;
source?: string;
schemaUri?: string;
problemType?: ProblemType;
problemArgs?: string[];
schemaUri?: string[];
}

interface DiagnosticExt extends Diagnostic {
schemaUri?: string;
schemaUri?: string[];
}

export abstract class ASTNodeImpl {
Expand Down Expand Up @@ -345,6 +361,35 @@ export class ValidationResult {
}
}

/**
* Merge multiple warnings with same problemType together
* @param subValidationResult another possible result
*/
public mergeWarningGeneric(subValidationResult: ValidationResult, problemTypesToMerge: ProblemType[]): void {
if (this.problems?.length) {
for (const problemType of problemTypesToMerge) {
const bestResults = this.problems.filter((p) => p.problemType === problemType);
for (const bestResult of bestResults) {
const mergingResult = subValidationResult.problems?.find(
(p) =>
p.problemType === problemType &&
bestResult.location.offset === p.location.offset &&
(problemType !== ProblemType.missingRequiredPropWarning || isArrayEqual(p.problemArgs, bestResult.problemArgs)) // missingProp is merged only with same problemArg
);
if (mergingResult) {
if (mergingResult.problemArgs.length) {
mergingResult.problemArgs
.filter((p) => !bestResult.problemArgs.includes(p))
.forEach((p) => bestResult.problemArgs.push(p));
bestResult.message = getWarningMessage(bestResult.problemType, bestResult.problemArgs);
}
this.mergeSources(mergingResult, bestResult);
}
}
}
}
}

public mergePropertyMatch(propertyValidationResult: ValidationResult): void {
this.merge(propertyValidationResult);
this.propertiesMatches++;
Expand All @@ -359,6 +404,16 @@ export class ValidationResult {
}
}

private mergeSources(mergingResult: IProblem, bestResult: IProblem): void {
const mergingSource = mergingResult.source.replace(YAML_SCHEMA_PREFIX, '');
if (!bestResult.source.includes(mergingSource)) {
bestResult.source = bestResult.source + ' | ' + mergingSource;
}
if (!bestResult.schemaUri.includes(mergingResult.schemaUri[0])) {
bestResult.schemaUri = bestResult.schemaUri.concat(mergingResult.schemaUri);
}
}

public compareGeneric(other: ValidationResult): number {
const hasProblems = this.hasProblems();
if (hasProblems !== other.hasProblems()) {
Expand Down Expand Up @@ -537,12 +592,16 @@ function validate(
}
} else if (schema.type) {
if (!matchesType(schema.type)) {
//get more specific name than just object
const schemaType = schema.type === 'object' ? getSchemaTypeName(schema) : schema.type;
validationResult.problems.push({
location: { offset: node.offset, length: node.length },
severity: DiagnosticSeverity.Warning,
message: schema.errorMessage || localize('typeMismatchWarning', 'Incorrect type. Expected "{0}".', schema.type),
message: schema.errorMessage || getWarningMessage(ProblemType.typeMismatchWarning, [schemaType]),
source: getSchemaSource(schema, originalSchema),
schemaUri: getSchemaUri(schema, originalSchema),
problemType: ProblemType.typeMismatchWarning,
problemArgs: [schemaType],
});
}
}
Expand Down Expand Up @@ -704,9 +763,11 @@ function validate(
location: { offset: node.offset, length: node.length },
severity: DiagnosticSeverity.Warning,
code: ErrorCode.EnumValueMismatch,
message: schema.errorMessage || localize('constWarning', 'Value must be {0}.', JSON.stringify(schema.const)),
problemType: ProblemType.constWarning,
message: schema.errorMessage || getWarningMessage(ProblemType.constWarning, [JSON.stringify(schema.const)]),
source: getSchemaSource(schema, originalSchema),
schemaUri: getSchemaUri(schema, originalSchema),
problemArgs: [JSON.stringify(schema.const)],
});
validationResult.enumValueMatch = false;
} else {
Expand Down Expand Up @@ -1050,9 +1111,11 @@ function validate(
validationResult.problems.push({
location: location,
severity: DiagnosticSeverity.Warning,
message: localize('MissingRequiredPropWarning', 'Missing property "{0}".', propertyName),
message: getWarningMessage(ProblemType.missingRequiredPropWarning, [propertyName]),
source: getSchemaSource(schema, originalSchema),
schemaUri: getSchemaUri(schema, originalSchema),
problemArgs: [propertyName],
problemType: ProblemType.missingRequiredPropWarning,
});
}
}
Expand Down Expand Up @@ -1277,8 +1340,21 @@ function validate(
}

//genericComparison tries to find the best matching schema using a generic comparison
// eslint-disable-next-line @typescript-eslint/no-explicit-any
function genericComparison(maxOneMatch, subValidationResult, bestMatch, subSchema, subMatchingSchemas): any {
function genericComparison(
maxOneMatch,
subValidationResult: ValidationResult,
bestMatch: {
schema: JSONSchema;
validationResult: ValidationResult;
matchingSchemas: ISchemaCollector;
},
subSchema,
subMatchingSchemas
): {
schema: JSONSchema;
validationResult: ValidationResult;
matchingSchemas: ISchemaCollector;
} {
if (!maxOneMatch && !subValidationResult.hasProblems() && !bestMatch.validationResult.hasProblems()) {
// no errors, both are equally good matches
bestMatch.matchingSchemas.merge(subMatchingSchemas);
Expand All @@ -1297,6 +1373,11 @@ function validate(
// there's already a best matching but we are as good
bestMatch.matchingSchemas.merge(subMatchingSchemas);
bestMatch.validationResult.mergeEnumValues(subValidationResult);
bestMatch.validationResult.mergeWarningGeneric(subValidationResult, [
ProblemType.missingRequiredPropWarning,
ProblemType.typeMismatchWarning,
ProblemType.constWarning,
]);
}
}
return bestMatch;
Expand All @@ -1321,14 +1402,18 @@ function getSchemaSource(schema: JSONSchema, originalSchema: JSONSchema): string
}
}
if (label) {
return `yaml-schema: ${label}`;
return `${YAML_SCHEMA_PREFIX}${label}`;
}
}

return YAML_SOURCE;
}

function getSchemaUri(schema: JSONSchema, originalSchema: JSONSchema): string | undefined {
function getSchemaUri(schema: JSONSchema, originalSchema: JSONSchema): string[] {
const uriString = schema.url ?? originalSchema.url;
return uriString;
return uriString ? [uriString] : [];
}

function getWarningMessage(problemType: ProblemType, args: string[]): string {
return localize(problemType, ProblemTypeMessages[problemType], args.join(' | '));
}
17 changes: 10 additions & 7 deletions src/languageservice/services/yamlCodeActions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,11 @@
import { TextDocument } from 'vscode-languageserver-textdocument';
import { ClientCapabilities, CodeAction, CodeActionParams, Command, Connection, Diagnostic } from 'vscode-languageserver';
import { YamlCommands } from '../../commands';
import * as path from 'path';
import { CommandExecutor } from '../../languageserver/commandExecutor';

interface YamlDiagnosticData {
schemaUri: string;
schemaUri: string[];
}
export class YamlCodeActions {
constructor(commandExecutor: CommandExecutor, connection: Connection, private readonly clientCapabilities: ClientCapabilities) {
Expand Down Expand Up @@ -47,18 +48,20 @@ export class YamlCodeActions {
}
const schemaUriToDiagnostic = new Map<string, Diagnostic[]>();
for (const diagnostic of diagnostics) {
const schemaUri = (diagnostic.data as YamlDiagnosticData)?.schemaUri;
if (schemaUri && (schemaUri.startsWith('file') || schemaUri.startsWith('https'))) {
if (!schemaUriToDiagnostic.has(schemaUri)) {
schemaUriToDiagnostic.set(schemaUri, []);
const schemaUri = (diagnostic.data as YamlDiagnosticData)?.schemaUri || [];
for (const schemaUriStr of schemaUri) {
if (schemaUriStr && (schemaUriStr.startsWith('file') || schemaUriStr.startsWith('https'))) {
if (!schemaUriToDiagnostic.has(schemaUriStr)) {
schemaUriToDiagnostic.set(schemaUriStr, []);
}
schemaUriToDiagnostic.get(schemaUriStr).push(diagnostic);
}
schemaUriToDiagnostic.get(schemaUri).push(diagnostic);
}
}
const result = [];
for (const schemaUri of schemaUriToDiagnostic.keys()) {
const action = CodeAction.create(
'Jump to schema location',
`Jump to schema location (${path.basename(schemaUri)})`,
Command.create('JumpToSchema', YamlCommands.JUMP_TO_SCHEMA, schemaUri)
);
action.diagnostics = schemaUriToDiagnostic.get(schemaUri);
Expand Down
2 changes: 2 additions & 0 deletions src/languageservice/services/yamlSchemaService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,8 @@ export class YAMLSchemaService extends JSONSchemaService {
while (next.$ref) {
const ref = next.$ref;
const segments = ref.split('#', 2);
//return back removed $ref. We lost info about referenced type without it.
next._$ref = next.$ref;
delete next.$ref;
if (segments[0].length > 0) {
openPromises.push(resolveExternalLink(next, segments[0], segments[1], parentSchemaURL, parentSchemaDependencies));
Expand Down
14 changes: 14 additions & 0 deletions src/languageservice/utils/arrUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,3 +72,17 @@ export function filterInvalidCustomTags(customTags: string[]): string[] {
return false;
});
}
export function isArrayEqual(fst: Array<unknown>, snd: Array<unknown>): boolean {
if (!snd) {
return false;
}
if (snd.length !== fst.length) {
return false;
}
for (let index = fst.length - 1; index >= 0; index--) {
if (fst[index] !== snd[index]) {
return false;
}
}
return true;
}
37 changes: 37 additions & 0 deletions src/languageservice/utils/schemaUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import { JSONSchema } from '../jsonSchema';

export function getSchemaTypeName(schema: JSONSchema): string {
if (schema.$id) {
const type = getSchemaRefTypeTitle(schema.$id);
return type;
}
if (schema.$ref || schema._$ref) {
const type = getSchemaRefTypeTitle(schema.$ref || schema._$ref);
return type;
}
const typeStr = schema.title || (Array.isArray(schema.type) ? schema.type.join(' | ') : schema.type); //object
return typeStr;
}

/**
* Get type name from reference url
* @param $ref reference to the same file OR to the another component OR to the section in another component:
* `schema-name.schema.json` -> schema-name
* `custom-scheme://shared-schema.json#/definitions/SomeType` -> SomeType
* `custom-scheme://schema-name.schema.json` -> schema-name
* `shared-schema.schema.json#/definitions/SomeType` -> SomeType
* `file:///Users/user/Documents/project/schemas/schema-name.schema.json` -> schema-name
* `#/definitions/SomeType` -> SomeType
* `#/definitions/io.k8s.api.apps.v1.DaemonSetSpec` => io.k8s.api.apps.v1.DaemonSetSpec
* `file:///default_schema_id.yaml` => default_schema_id.yaml
* test: https://regex101.com/r/ZpuXxk/1
*/
export function getSchemaRefTypeTitle($ref: string): string {
const match = $ref.match(/^(?:.*\/)?(.*?)(?:\.schema\.json)?$/);
let type = !!match && match[1];
if (!type) {
type = 'typeNotFound';
console.error(`$ref (${$ref}) not parsed properly`);
}
return type;
}
79 changes: 79 additions & 0 deletions test/fixtures/testMultipleSimilarSchema.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
{
"sharedSchema": {
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"type1": {
"properties": {
"objA": {
"type": "object"
},
"propA": {
"type": "string"
},
"constA": {
"type": "string",
"const": "constForType1"
}
},
"required": [
"objA",
"propA",
"constA"
],
"type": "object"
}
}
},
"schema": {
"$schema": "http://json-schema.org/draft-07/schema#",
"definitions": {
"type2": {
"properties": {
"obj2": {
"type": "object"
}
},
"required": [
"obj2"
],
"type": "object"
},
"type3": {
"properties": {
"objA": {
"type": "object"
},
"propA": {
"type": "string"
},
"constA": {
"type": "string",
"const": "constForType3"
}
},
"required": [
"objA",
"propA",
"constA"
],
"type": "object"
}
},
"properties": {
"test_anyOf_objects": {
"anyOf": [
{
"$ref": "sharedSchema.json#/definitions/type1"
},
{
"$ref": "#/definitions/type2"
},
{
"$ref": "#/definitions/type3"
}
]
}
},
"type": "object"
}
}
Loading

0 comments on commit ce66564

Please sign in to comment.