Skip to content

Commit

Permalink
Merge pull request #757 from redhat-developer/fix-enum-validation
Browse files Browse the repository at this point in the history
Fix enum validation
  • Loading branch information
msivasubramaniaan authored Aug 2, 2022
2 parents 1f1d47d + 2b73a11 commit bb0d33f
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 10 deletions.
27 changes: 24 additions & 3 deletions src/languageservice/parser/jsonParser07.ts
Original file line number Diff line number Diff line change
Expand Up @@ -580,12 +580,32 @@ export class JSONDocument {
return null;
}

public getMatchingSchemas(schema: JSONSchema, focusOffset = -1, exclude: ASTNode = null): IApplicableSchema[] {
/**
* This method returns the list of applicable schemas
*
* currently used @param didCallFromAutoComplete flag to differentiate the method call, when it is from auto complete
* then user still types something and skip the validation for timebeing untill completed.
* On https://github.com/redhat-developer/yaml-language-server/pull/719 the auto completes need to populate the list of enum string which matches to the enum
* and on https://github.com/redhat-developer/vscode-yaml/issues/803 the validation should throw the error based on the enum string.
*
* @param schema schema
* @param focusOffset offsetValue
* @param exclude excluded Node
* @param didCallFromAutoComplete true if method called from AutoComplete
* @returns array of applicable schemas
*/
public getMatchingSchemas(
schema: JSONSchema,
focusOffset = -1,
exclude: ASTNode = null,
didCallFromAutoComplete?: boolean
): IApplicableSchema[] {
const matchingSchemas = new SchemaCollector(focusOffset, exclude);
if (this.root && schema) {
validate(this.root, schema, schema, new ValidationResult(this.isKubernetes), matchingSchemas, {
isKubernetes: this.isKubernetes,
disableAdditionalProperties: this.disableAdditionalProperties,
callFromAutoComplete: didCallFromAutoComplete,
});
}
return matchingSchemas.schemas;
Expand All @@ -594,6 +614,7 @@ export class JSONDocument {
interface Options {
isKubernetes: boolean;
disableAdditionalProperties: boolean;
callFromAutoComplete?: boolean;
}
function validate(
node: ASTNode,
Expand All @@ -604,7 +625,7 @@ function validate(
options: Options
// eslint-disable-next-line @typescript-eslint/no-explicit-any
): any {
const { isKubernetes } = options;
const { isKubernetes, callFromAutoComplete } = options;
if (!node) {
return;
}
Expand Down Expand Up @@ -797,7 +818,7 @@ function validate(
const val = getNodeValue(node);
let enumValueMatch = false;
for (const e of schema.enum) {
if (equals(val, e) || (isString(val) && isString(e) && val && e.startsWith(val))) {
if (equals(val, e) || (callFromAutoComplete && isString(val) && isString(e) && val && e.startsWith(val))) {
enumValueMatch = true;
break;
}
Expand Down
26 changes: 19 additions & 7 deletions src/languageservice/services/yamlCompletion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export class YamlCompletion {
this.parentSkeletonSelectedFirst = languageSettings.parentSkeletonSelectedFirst;
}

async doComplete(document: TextDocument, position: Position, isKubernetes = false): Promise<CompletionList> {
async doComplete(document: TextDocument, position: Position, isKubernetes = false, doComplete = true): Promise<CompletionList> {
const result = CompletionList.create([], false);
if (!this.completionEnabled) {
return result;
Expand Down Expand Up @@ -489,7 +489,17 @@ export class YamlCompletion {
}
}

this.addPropertyCompletions(schema, currentDoc, node, originalNode, '', collector, textBuffer, overwriteRange);
this.addPropertyCompletions(
schema,
currentDoc,
node,
originalNode,
'',
collector,
textBuffer,
overwriteRange,
doComplete
);

if (!schema && currentWord.length > 0 && text.charAt(offset - currentWord.length - 1) !== '"') {
collector.add({
Expand All @@ -503,7 +513,7 @@ export class YamlCompletion {

// proposals for values
const types: { [type: string]: boolean } = {};
this.getValueCompletions(schema, currentDoc, node, offset, document, collector, types);
this.getValueCompletions(schema, currentDoc, node, offset, document, collector, types, doComplete);
} catch (err) {
this.telemetry.sendError('yaml.completion.error', { error: convertErrorToTelemetryMsg(err) });
}
Expand Down Expand Up @@ -621,9 +631,10 @@ export class YamlCompletion {
separatorAfter: string,
collector: CompletionsCollector,
textBuffer: TextBuffer,
overwriteRange: Range
overwriteRange: Range,
doComplete: boolean
): void {
const matchingSchemas = doc.getMatchingSchemas(schema.schema);
const matchingSchemas = doc.getMatchingSchemas(schema.schema, -1, null, doComplete);
const existingKey = textBuffer.getText(overwriteRange);
const lineContent = textBuffer.getLineContent(overwriteRange.start.line);
const hasOnlyWhitespace = lineContent.trim().length === 0;
Expand Down Expand Up @@ -814,7 +825,8 @@ export class YamlCompletion {
offset: number,
document: TextDocument,
collector: CompletionsCollector,
types: { [type: string]: boolean }
types: { [type: string]: boolean },
doComplete: boolean
): void {
let parentKey: string = null;

Expand All @@ -838,7 +850,7 @@ export class YamlCompletion {

if (node && (parentKey !== null || isSeq(node))) {
const separatorAfter = '';
const matchingSchemas = doc.getMatchingSchemas(schema.schema);
const matchingSchemas = doc.getMatchingSchemas(schema.schema, -1, null, doComplete);
for (const s of matchingSchemas) {
if (s.node.internalNode === node && !s.inverted && s.schema) {
if (s.schema.items) {
Expand Down
35 changes: 35 additions & 0 deletions test/schemaValidation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1732,5 +1732,40 @@ obj:
expect(result.length).to.eq(3);
expect(telemetry.messages).to.be.empty;
});

it('Enum Validation with invalid data', async () => {
languageService.addSchema(SCHEMA_ID, {
definitions: {
rule: {
description: 'A rule',
type: 'object',
properties: {
kind: {
description: 'The kind of rule',
type: 'string',
enum: ['tested'],
},
},
required: ['kind'],
additionalProperties: false,
},
},
properties: {
rules: {
description: 'Rule list',
type: 'array',
items: {
$ref: '#/definitions/rule',
},
minProperties: 1,
additionalProperties: false,
},
},
});
const content = 'rules:\n - kind: test';
const result = await parseSetup(content);
expect(result.length).to.eq(1);
expect(result[0].message).to.eq('Value is not accepted. Valid values: "tested".');
});
});
});

0 comments on commit bb0d33f

Please sign in to comment.