Skip to content

Commit

Permalink
Merge pull request redhat-developer#176 from redhat-developer/validat…
Browse files Browse the repository at this point in the history 
…ion-enum-fix

Fix for validation with multiple enums
  • Loading branch information
@JPinkney
JPinkney authored Aug 19, 2019
2 parents 00db0de + a1ce7da commit e999719
Show file tree
Hide file tree
Showing 4 changed files with 159 additions and 29 deletions.
4 changes: 2 additions & 2 deletions src/languageservice/parser/jsonParser04.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -926,7 +926,7 @@ export class ValidationResult {
if (propertyValidationResult.enumValueMatch || !this.hasProblems() && propertyValidationResult.propertiesMatches) {
this.propertiesValueMatches++;
}
if (propertyValidationResult.enumValueMatch && propertyValidationResult.enumValues && propertyValidationResult.enumValues.length === 1) {
if (propertyValidationResult.enumValueMatch && propertyValidationResult.enumValues) {
this.primaryValueMatches++;
}
}
Expand All @@ -950,7 +950,7 @@ export class ValidationResult {

public compareKubernetes(other: ValidationResult): number {
const hasProblems = this.hasProblems();
if (this.propertiesMatches !== other.propertiesMatches){
if (this.propertiesMatches !== other.propertiesMatches) {
return this.propertiesMatches - other.propertiesMatches;
}
if (this.enumValueMatch !== other.enumValueMatch) {
Expand Down
47 changes: 29 additions & 18 deletions src/languageservice/parser/jsonParser07.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,13 +211,17 @@ export class ValidationResult {
// tslint:disable-next-line: no-any
public enumValues: any[];

constructor() {
constructor(isKubernetes: boolean) {
this.problems = [];
this.propertiesMatches = 0;
this.propertiesValueMatches = 0;
this.primaryValueMatches = 0;
this.enumValueMatch = false;
this.enumValues = null;
if (isKubernetes) {
this.enumValues = [];
} else {
this.enumValues = null;
}
}

public hasProblems(): boolean {
Expand All @@ -239,7 +243,7 @@ export class ValidationResult {
this.enumValues = this.enumValues.concat(validationResult.enumValues);
for (const error of this.problems) {
if (error.code === ErrorCode.EnumValueMismatch) {
error.message = localize('enumWarning', 'Value is not accepted. Valid values: {0}.', this.enumValues.map(v => JSON.stringify(v)).join(', '));
error.message = localize('enumWarning', 'Value is not accepted. Valid values: {0}.', [...new Set(this.enumValues)].map(v => JSON.stringify(v)).join(', '));
}
}
}
Expand All @@ -251,7 +255,7 @@ export class ValidationResult {
if (propertyValidationResult.enumValueMatch || !propertyValidationResult.hasProblems() && propertyValidationResult.propertiesMatches) {
this.propertiesValueMatches++;
}
if (propertyValidationResult.enumValueMatch && propertyValidationResult.enumValues && propertyValidationResult.enumValues.length === 1) {
if (propertyValidationResult.enumValueMatch && propertyValidationResult.enumValues) {
this.primaryValueMatches++;
}
}
Expand Down Expand Up @@ -344,7 +348,7 @@ export class JSONDocument {

public validate(textDocument: TextDocument, schema: JSONSchema): Diagnostic[] {
if (this.root && schema) {
const validationResult = new ValidationResult();
const validationResult = new ValidationResult(this.isKubernetes);
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));
Expand All @@ -357,7 +361,7 @@ 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, this.isKubernetes);
validate(this.root, schema, new ValidationResult(this.isKubernetes), matchingSchemas, this.isKubernetes);
}
return matchingSchemas.schemas;
}
Expand Down Expand Up @@ -420,7 +424,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
}
const notSchema = asSchema(schema.not);
if (notSchema) {
const subValidationResult = new ValidationResult();
const subValidationResult = new ValidationResult(isKubernetes);
const subMatchingSchemas = matchingSchemas.newSub();
validate(node, notSchema, subValidationResult, subMatchingSchemas, isKubernetes);
if (!subValidationResult.hasProblems()) {
Expand All @@ -443,7 +447,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
let bestMatch: { schema: JSONSchema; validationResult: ValidationResult; matchingSchemas: ISchemaCollector; } = null;
for (const subSchemaRef of alternatives) {
const subSchema = asSchema(subSchemaRef);
const subValidationResult = new ValidationResult();
const subValidationResult = new ValidationResult(isKubernetes);
const subMatchingSchemas = matchingSchemas.newSub();
validate(node, subSchema, subValidationResult, subMatchingSchemas, isKubernetes);
if (!subValidationResult.hasProblems()) {
Expand Down Expand Up @@ -481,7 +485,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
}

const testBranch = (schema: JSONSchemaRef) => {
const subValidationResult = new ValidationResult();
const subValidationResult = new ValidationResult(isKubernetes);
const subMatchingSchemas = matchingSchemas.newSub();

validate(node, asSchema(schema), subValidationResult, subMatchingSchemas, isKubernetes);
Expand All @@ -494,7 +498,7 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio

const testCondition = (ifSchema: JSONSchemaRef, thenSchema?: JSONSchemaRef, elseSchema?: JSONSchemaRef) => {
const subSchema = asSchema(ifSchema);
const subValidationResult = new ValidationResult();
const subValidationResult = new ValidationResult(isKubernetes);
const subMatchingSchemas = matchingSchemas.newSub();

validate(node, subSchema, subValidationResult, subMatchingSchemas, isKubernetes);
Expand Down Expand Up @@ -707,22 +711,24 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
for (let index = 0; index < subSchemas.length; index++) {
const subSchemaRef = subSchemas[index];
const subSchema = asSchema(subSchemaRef);
const itemValidationResult = new ValidationResult();
const itemValidationResult = new ValidationResult(isKubernetes);
const item = node.items[index];
if (item) {
validate(item, subSchema, itemValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(itemValidationResult);
validationResult.mergeEnumValues(itemValidationResult);
} else if (node.items.length >= subSchemas.length) {
validationResult.propertiesValueMatches++;
}
}
if (node.items.length > subSchemas.length) {
if (typeof schema.additionalItems === 'object') {
for (let i = subSchemas.length; i < node.items.length; i++) {
const itemValidationResult = new ValidationResult();
const itemValidationResult = new ValidationResult(isKubernetes);
// tslint:disable-next-line: no-any
validate(node.items[i], <any>schema.additionalItems, itemValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(itemValidationResult);
validationResult.mergeEnumValues(itemValidationResult);
}
} else if (schema.additionalItems === false) {
validationResult.problems.push({
Expand All @@ -736,17 +742,18 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
const itemSchema = asSchema(schema.items);
if (itemSchema) {
for (const item of node.items) {
const itemValidationResult = new ValidationResult();
const itemValidationResult = new ValidationResult(isKubernetes);
validate(item, itemSchema, itemValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(itemValidationResult);
validationResult.mergeEnumValues(itemValidationResult);
}
}
}

const containsSchema = asSchema(schema.contains);
if (containsSchema) {
const doesContain = node.items.some(item => {
const itemValidationResult = new ValidationResult();
const itemValidationResult = new ValidationResult(isKubernetes);
validate(item, containsSchema, itemValidationResult, NoOpSchemaCollector.instance, isKubernetes);
return !itemValidationResult.hasProblems();
});
Expand Down Expand Up @@ -841,9 +848,10 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
validationResult.propertiesValueMatches++;
}
} else {
const propertyValidationResult = new ValidationResult();
const propertyValidationResult = new ValidationResult(isKubernetes);
validate(child, propertySchema, propertyValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(propertyValidationResult);
validationResult.mergeEnumValues(propertyValidationResult);
}
}

Expand Down Expand Up @@ -872,9 +880,10 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
validationResult.propertiesValueMatches++;
}
} else {
const propertyValidationResult = new ValidationResult();
const propertyValidationResult = new ValidationResult(isKubernetes);
validate(child, propertySchema, propertyValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(propertyValidationResult);
validationResult.mergeEnumValues(propertyValidationResult);
}
}
}
Expand All @@ -886,10 +895,11 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
for (const propertyName of unprocessedProperties) {
const child = seenKeys[propertyName];
if (child) {
const propertyValidationResult = new ValidationResult();
const propertyValidationResult = new ValidationResult(isKubernetes);
// tslint:disable-next-line: no-any
validate(child, <any>schema.additionalProperties, propertyValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(propertyValidationResult);
validationResult.mergeEnumValues(propertyValidationResult);
}
}
} else if (schema.additionalProperties === false) {
Expand Down Expand Up @@ -956,9 +966,10 @@ function validate(node: ASTNode, schema: JSONSchema, validationResult: Validatio
} else {
const propertySchema = asSchema(propertyDep);
if (propertySchema) {
const propertyValidationResult = new ValidationResult();
const propertyValidationResult = new ValidationResult(isKubernetes);
validate(node, propertySchema, propertyValidationResult, matchingSchemas, isKubernetes);
validationResult.mergePropertyMatch(propertyValidationResult);
validationResult.mergeEnumValues(propertyValidationResult);
}
}
}
Expand Down
76 changes: 67 additions & 9 deletions test/autoCompletion.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,18 @@ languageService.configure(languageSettings);

suite('Auto Completion Tests', () => {

describe('yamlCompletion with bowerrc', function () {
function setup(content: string) {
return TextDocument.create('file://~/Desktop/vscode-k8s/test.yaml', 'yaml', 0, content);
}

describe('doComplete', function () {
function parseSetup(content: string, position) {
const testTextDocument = setup(content);
return languageService.doComplete(testTextDocument, testTextDocument.positionAt(position), false);
}

function setup(content: string) {
return TextDocument.create('file://~/Desktop/vscode-k8s/test.yaml', 'yaml', 0, content);
}
describe('yamlCompletion with bowerrc', function () {

function parseSetup(content: string, position) {
const testTextDocument = setup(content);
return languageService.doComplete(testTextDocument, testTextDocument.positionAt(position), false);
}
describe('doComplete', function () {

it('Autocomplete on root node without word', done => {
const content = '';
Expand Down Expand Up @@ -145,5 +145,63 @@ suite('Auto Completion Tests', () => {
}).then(done, done);
});
});

describe('Autocompletion using custom provided schema', function () {
it('Test that properties that have multiple enums get auto completed properly', done => {
const schema = {
'definitions': {
'ImageBuild': {
'type': 'object',
'properties': {
'kind': {
'type': 'string',
'enum': [
'ImageBuild',
'ImageBuilder'
]
}
}
},
'ImageStream': {
'type': 'object',
'properties': {
'kind': {
'type': 'string',
'enum': [
'ImageStream',
'ImageStreamBuilder'
]
}
}
}
},
'oneOf': [
{
'$ref': '#/definitions/ImageBuild'
},
{
'$ref': '#/definitions/ImageStream'
}
]
};
languageService.configure({
schemas: [{
uri: 'file://test.yaml',
fileMatch: ['*.yaml', '*.yml'],
schema
}],
completion: true
});
const content = 'kind: ';
const validator = parseSetup(content, 6);
validator.then(function (result) {
assert.equal(result.items.length, 4);
assert.equal(result.items[0].label, 'ImageBuild');
assert.equal(result.items[1].label, 'ImageBuilder');
assert.equal(result.items[2].label, 'ImageStream');
assert.equal(result.items[3].label, 'ImageStreamBuilder');
}).then(done, done);
});
});
});
});
61 changes: 61 additions & 0 deletions test/schemaValidation.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -332,5 +332,66 @@ suite('Validation Tests', () => {
});

});

describe('Test with custom schemas', function () {
function parseSetup(content: string) {
const testTextDocument = setupTextDocument(content);
return languageService.doValidation(testTextDocument, true);
}

it('Test that properties that match multiple enums get validated properly', done => {
const schema = {
'definitions': {
'ImageStreamImport': {
'type': 'object',
'properties': {
'kind': {
'type': 'string',
'enum': [
'ImageStreamImport'
]
}
}
},
'ImageStreamLayers': {
'type': 'object',
'properties': {
'kind': {
'type': 'string',
'enum': [
'ImageStreamLayers'
]
}
}
}
},
'oneOf': [
{
'$ref': '#/definitions/ImageStreamImport'
},
{
'$ref': '#/definitions/ImageStreamLayers'
}
]
};
languageService.configure({
schemas: [{
uri: 'file://test.yaml',
fileMatch: ['*.yaml', '*.yml'],
schema
}],
validate: true,
isKubernetes: true
});
const content = 'kind: ';
const validator = parseSetup(content);
validator.then(function (result) {
assert.equal(result.length, 2);
// tslint:disable-next-line:quotemark
assert.equal(result[1].message, `Value is not accepted. Valid values: "ImageStreamImport", "ImageStreamLayers".`);
}).then(done, done);
});

});
});
});

0 comments on commit e999719

Please sign in to comment.