Skip to content

Commit

Permalink
Fixes that allow kubernetes to receive the correct validation events
Browse files Browse the repository at this point in the history
  • Loading branch information
@JPinkney
JPinkney committed Jul 15, 2019
1 parent 6b7ef57 commit 04f5833
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 43 deletions.
115 changes: 79 additions & 36 deletions src/languageservice/parser/jsonParser07.ts
View file
Expand Up @@ -208,7 +208,7 @@ export class ValidationResult {
public propertiesValueMatches: number;
public primaryValueMatches: number;
public enumValueMatch: boolean;
// tslint:disable-next-line: no-any
// tslint:disable-next-line: no-any
public enumValues: any[];

constructor() {
Expand Down Expand Up @@ -256,20 +256,40 @@ export class ValidationResult {
}
}

public compare(other: ValidationResult): number {
public compareGeneric(other: ValidationResult): number {
const hasProblems = this.hasProblems();
if (hasProblems !== other.hasProblems()) {
return hasProblems ? -1 : 1;
}
if (this.enumValueMatch !== other.enumValueMatch) {
return other.enumValueMatch ? -1 : 1;
}
if (this.propertiesValueMatches !== other.propertiesValueMatches) {
return this.propertiesValueMatches - other.propertiesValueMatches;
}
if (this.primaryValueMatches !== other.primaryValueMatches) {
return this.primaryValueMatches - other.primaryValueMatches;
}
return this.propertiesMatches - other.propertiesMatches;
}

public compareKubernetes(other: ValidationResult): number {
const hasProblems = this.hasProblems();
if (this.propertiesMatches !== other.propertiesMatches){
return this.propertiesMatches - other.propertiesMatches;
}
if (this.enumValueMatch !== other.enumValueMatch) {
return other.enumValueMatch ? -1 : 1;
}
if (this.primaryValueMatches !== other.primaryValueMatches) {
return this.primaryValueMatches - other.primaryValueMatches;
}
if (this.propertiesValueMatches !== other.propertiesValueMatches) {
return this.propertiesValueMatches - other.propertiesValueMatches;
}
if (hasProblems !== other.hasProblems()) {
return hasProblems ? -1 : 1;
}
return this.propertiesMatches - other.propertiesMatches;
}

Expand All @@ -294,6 +314,8 @@ export function contains(node: ASTNode, offset: number, includeRightBound = fals

export class JSONDocument {

public isKubernetes: boolean;

constructor(public readonly root: ASTNode, public readonly syntaxErrors: Diagnostic[] = [], public readonly comments: Range[] = []) {
}

Expand Down Expand Up @@ -323,7 +345,7 @@ export class JSONDocument {
public validate(textDocument: TextDocument, schema: JSONSchema): Diagnostic[] {
if (this.root && schema) {
const validationResult = new ValidationResult();
validate(this.root, schema, validationResult, NoOpSchemaCollector.instance);
validate(this.root, schema, validationResult, NoOpSchemaCollector.instance, this.isKubernetes);
return validationResult.problems.map(p => {
const range = Range.create(textDocument.positionAt(p.location.offset), textDocument.positionAt(p.location.offset + p.location.length));
return Diagnostic.create(range, p.message, p.severity, p.code);
Expand All @@ -335,13 +357,13 @@ export class JSONDocument {
public getMatchingSchemas(schema: JSONSchema, focusOffset: number = -1, exclude: ASTNode = null): IApplicableSchema[] {
const matchingSchemas = new SchemaCollector(focusOffset, exclude);
if (this.root && schema) {
validate(this.root, schema, new ValidationResult(), matchingSchemas);
validate(this.root, schema, new ValidationResult(), matchingSchemas, this.isKubernetes);
}
return matchingSchemas.schemas;
}
}

function validate(node: ASTNode, schema: JSONSchema, validationResult: ValidationResult, matchingSchemas: ISchemaCollector) {
function validate(node: ASTNode, schema: JSONSchema, validationResult: ValidationResult, matchingSchemas: ISchemaCollector, isKubernetes: boolean) {

if (!node || !matchingSchemas.include(node)) {
return;
Expand All @@ -361,7 +383,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
_validateNumberNode(node, schema, validationResult, matchingSchemas);
break;
case 'property':
return validate(node.valueNode, schema, validationResult, matchingSchemas);
return validate(node.valueNode, schema, validationResult, matchingSchemas, isKubernetes);
}
_validateNode();

Expand Down Expand Up @@ -393,14 +415,14 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
}
if (Array.isArray(schema.allOf)) {
for (const subSchemaRef of schema.allOf) {
validate(node, asSchema(subSchemaRef), validationResult, matchingSchemas);
validate(node, asSchema(subSchemaRef), validationResult, matchingSchemas, isKubernetes);
}
}
const notSchema = asSchema(schema.not);
if (notSchema) {
const subValidationResult = new ValidationResult();
const subMatchingSchemas = matchingSchemas.newSub();
validate(node, notSchema, subValidationResult, subMatchingSchemas);
validate(node, notSchema, subValidationResult, subMatchingSchemas, isKubernetes);
if (!subValidationResult.hasProblems()) {
validationResult.problems.push({
location: { offset: node.offset, length: node.length },
Expand All @@ -423,29 +445,16 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
const subSchema = asSchema(subSchemaRef);
const subValidationResult = new ValidationResult();
const subMatchingSchemas = matchingSchemas.newSub();
validate(node, subSchema, subValidationResult, subMatchingSchemas);
validate(node, subSchema, subValidationResult, subMatchingSchemas, isKubernetes);
if (!subValidationResult.hasProblems()) {
matches.push(subSchema);
}
if (!bestMatch) {
bestMatch = { schema: subSchema, validationResult: subValidationResult, matchingSchemas: subMatchingSchemas };
} else if (isKubernetes) {
bestMatch = alternativeComparison(subValidationResult, bestMatch, subSchema, subMatchingSchemas);
} else {
if (!maxOneMatch && !subValidationResult.hasProblems() && !bestMatch.validationResult.hasProblems()) {
// no errors, both are equally good matches
bestMatch.matchingSchemas.merge(subMatchingSchemas);
bestMatch.validationResult.propertiesMatches += subValidationResult.propertiesMatches;
bestMatch.validationResult.propertiesValueMatches += subValidationResult.propertiesValueMatches;
} else {
const compareResult = subValidationResult.compare(bestMatch.validationResult);
if (compareResult > 0) {
// our node is the best matching so far
bestMatch = { schema: subSchema, validationResult: subValidationResult, matchingSchemas: subMatchingSchemas };
} else if (compareResult === 0) {
// there's already a best matching but we are as good
bestMatch.matchingSchemas.merge(subMatchingSchemas);
bestMatch.validationResult.mergeEnumValues(subValidationResult);
}
}
bestMatch = genericComparison(maxOneMatch, subValidationResult, bestMatch, subSchema, subMatchingSchemas);
}
}

Expand Down Expand Up @@ -475,7 +484,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
const subValidationResult = new ValidationResult();
const subMatchingSchemas = matchingSchemas.newSub();

validate(node, asSchema(schema), subValidationResult, subMatchingSchemas);
validate(node, asSchema(schema), subValidationResult, subMatchingSchemas, isKubernetes);

validationResult.merge(subValidationResult);
validationResult.propertiesMatches += subValidationResult.propertiesMatches;
Expand All @@ -488,7 +497,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
const subValidationResult = new ValidationResult();
const subMatchingSchemas = matchingSchemas.newSub();

validate(node, subSchema, subValidationResult, subMatchingSchemas);
validate(node, subSchema, subValidationResult, subMatchingSchemas, isKubernetes);
matchingSchemas.merge(subMatchingSchemas);

if (!subValidationResult.hasProblems()) {
Expand Down Expand Up @@ -701,7 +710,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
const itemValidationResult = new ValidationResult();
const item = node.items[index];
if (item) {
validate(item, subSchema, itemValidationResult, matchingSchemas);
validate(item, subSchema, itemValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(itemValidationResult);
} else if (node.items.length >= subSchemas.length) {
validationResult.propertiesValueMatches++;
Expand All @@ -712,7 +721,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
for (let i = subSchemas.length; i < node.items.length; i++) {
const itemValidationResult = new ValidationResult();
// tslint:disable-next-line: no-any
validate(node.items[i], <any>schema.additionalItems, itemValidationResult, matchingSchemas);
validate(node.items[i], <any>schema.additionalItems, itemValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(itemValidationResult);
}
} else if (schema.additionalItems === false) {
Expand All @@ -728,7 +737,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
if (itemSchema) {
for (const item of node.items) {
const itemValidationResult = new ValidationResult();
validate(item, itemSchema, itemValidationResult, matchingSchemas);
validate(item, itemSchema, itemValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(itemValidationResult);
}
}
Expand All @@ -738,7 +747,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
if (containsSchema) {
const doesContain = node.items.some(item => {
const itemValidationResult = new ValidationResult();
validate(item, containsSchema, itemValidationResult, NoOpSchemaCollector.instance);
validate(item, containsSchema, itemValidationResult, NoOpSchemaCollector.instance, isKubernetes);
return !itemValidationResult.hasProblems();
});

Expand Down Expand Up @@ -833,7 +842,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
}
} else {
const propertyValidationResult = new ValidationResult();
validate(child, propertySchema, propertyValidationResult, matchingSchemas);
validate(child, propertySchema, propertyValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(propertyValidationResult);
}
}
Expand Down Expand Up @@ -864,7 +873,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
}
} else {
const propertyValidationResult = new ValidationResult();
validate(child, propertySchema, propertyValidationResult, matchingSchemas);
validate(child, propertySchema, propertyValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(propertyValidationResult);
}
}
Expand All @@ -879,7 +888,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
if (child) {
const propertyValidationResult = new ValidationResult();
// tslint:disable-next-line: no-any
validate(child, <any>schema.additionalProperties, propertyValidationResult, matchingSchemas);
validate(child, <any>schema.additionalProperties, propertyValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(propertyValidationResult);
}
}
Expand Down Expand Up @@ -948,7 +957,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
const propertySchema = asSchema(propertyDep);
if (propertySchema) {
const propertyValidationResult = new ValidationResult();
validate(node, propertySchema, propertyValidationResult, matchingSchemas);
validate(node, propertySchema, propertyValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(propertyValidationResult);
}
}
Expand All @@ -961,10 +970,44 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
for (const f of node.properties) {
const key = f.keyNode;
if (key) {
validate(key, propertyNames, validationResult, NoOpSchemaCollector.instance);
validate(key, propertyNames, validationResult, NoOpSchemaCollector.instance, isKubernetes);
}
}
}
}

//Alternative comparison is specifically used by the kubernetes/openshift schema but may lead to better results then genericComparison depending on the schema
function alternativeComparison(subValidationResult, bestMatch, subSchema, subMatchingSchemas){
const compareResult = subValidationResult.compareKubernetes(bestMatch.validationResult);
if (compareResult > 0) {
// our node is the best matching so far
bestMatch = { schema: subSchema, validationResult: subValidationResult, matchingSchemas: subMatchingSchemas };
} else if (compareResult === 0) {
// there's already a best matching but we are as good
bestMatch.matchingSchemas.merge(subMatchingSchemas);
bestMatch.validationResult.mergeEnumValues(subValidationResult);
}
return bestMatch;
}

//genericComparison tries to find the best matching schema using a generic comparison
function genericComparison(maxOneMatch, subValidationResult, bestMatch, subSchema, subMatchingSchemas){
if (!maxOneMatch && !subValidationResult.hasProblems() && !bestMatch.validationResult.hasProblems()) {
// no errors, both are equally good matches
bestMatch.matchingSchemas.merge(subMatchingSchemas);
bestMatch.validationResult.propertiesMatches += subValidationResult.propertiesMatches;
bestMatch.validationResult.propertiesValueMatches += subValidationResult.propertiesValueMatches;
} else {
const compareResult = subValidationResult.compareGeneric(bestMatch.validationResult);
if (compareResult > 0) {
// our node is the best matching so far
bestMatch = { schema: subSchema, validationResult: subValidationResult, matchingSchemas: subMatchingSchemas };
} else if (compareResult === 0) {
// there's already a best matching but we are as good
bestMatch.matchingSchemas.merge(subMatchingSchemas);
bestMatch.validationResult.mergeEnumValues(subValidationResult);
}
}
return bestMatch;
}
}
16 changes: 9 additions & 7 deletions src/languageservice/parser/yamlParser07.ts
View file
Expand Up @@ -24,13 +24,15 @@ export class SingleYAMLDocument extends JSONDocument {
public root;
public errors;
public warnings;
public isKubernetes: boolean;

constructor(lines: number[]) {
constructor(lines: number[], kubernetes = false) {
super(null, []);
this.lines = lines;
this.root = null;
this.errors = [];
this.warnings = [];
this.isKubernetes = kubernetes;
}

public getSchemas(schema, doc, node) {
Expand Down Expand Up @@ -174,8 +176,8 @@ function convertError(e: Yaml.YAMLException) {
};
}

function createJSONDocument(yamlDoc: Yaml.YAMLNode, startPositions: number[], text: string) {
const _doc = new SingleYAMLDocument(startPositions);
function createJSONDocument(yamlDoc: Yaml.YAMLNode, startPositions: number[], text: string, isKubernetes: boolean) {
const _doc = new SingleYAMLDocument(startPositions, isKubernetes);
_doc.root = recursivelyBuildAst(null, yamlDoc);

if (!_doc.root) {
Expand Down Expand Up @@ -207,19 +209,19 @@ function createJSONDocument(yamlDoc: Yaml.YAMLNode, startPositions: number[], te
}

export class YAMLDocument {
public documents: JSONDocument[];
public documents: SingleYAMLDocument[];
private errors;
private warnings;

constructor(documents: JSONDocument[]) {
constructor(documents: SingleYAMLDocument[]) {
this.documents = documents;
this.errors = [];
this.warnings = [];
}

}

export function parse(text: string, customTags = []): YAMLDocument {
export function parse(text: string, customTags = [], isKubernetes = false): YAMLDocument {

const startPositions = getLineStartPositions(text);
// This is documented to return a YAMLNode even though the
Expand Down Expand Up @@ -260,5 +262,5 @@ export function parse(text: string, customTags = []): YAMLDocument {

Yaml.loadAll(text, doc => yamlDocs.push(doc), additionalOptions);

return new YAMLDocument(yamlDocs.map(doc => createJSONDocument(doc, startPositions, text)));
return new YAMLDocument(yamlDocs.map(doc => createJSONDocument(doc, startPositions, text, isKubernetes)));
}
1 change: 1 addition & 0 deletions src/languageservice/services/yamlValidation.ts
View file
Expand Up @@ -41,6 +41,7 @@ export class YAMLValidation {
const yamlDocument: YAMLDocument = parseYAML(textDocument.getText(), this.customTags);
const validationResult: Diagnostic[] = [];
for (const currentYAMLDoc of yamlDocument.documents) {
currentYAMLDoc.isKubernetes = isKubernetes;
const validation = await this.jsonLanguageService.doValidation(textDocument, currentYAMLDoc);
const syd = currentYAMLDoc as unknown as SingleYAMLDocument;
if (syd.errors.length > 0) {
Expand Down
20 changes: 20 additions & 0 deletions test/schema.test.ts
View file
Expand Up @@ -300,4 +300,24 @@ suite('JSON Schema', () => {
testDone(error);
});
});

test('Schema with non uri registers correctly', function (testDone) {
const service = new SchemaService.JSONSchemaService(requestServiceMock, workspaceContext);
const non_uri = 'non_uri';
service.registerExternalSchema(non_uri, ['*.yml', '*.yaml'], {
'properties': {
'test_node': {
'description': 'my test_node description',
'enum': [
'test 1',
'test 2'
]
}
}
});
service.getResolvedSchema(non_uri).then(schema => {
assert.notEqual(schema, undefined);
testDone();
});
});
});

0 comments on commit 04f5833

Please sign in to comment.