Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: default value with special chars with anyOf #963

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

fix: default value with special chars with anyOf #963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fae39cd
fix: default value with special chars with anyOf
p-spacek May 2, 2024
26ec17a
chore: accept null,0,emptyString in default or const property
p-spacek May 2, 2024
10fcfed
Merge branch 'main' into fix/default-value-with-special-chars-with-anyOf
p-spacek Oct 24, 2024
33ef09a
fix: special chars in property
p-spacek Oct 24, 2024
67b24b1
fix: special chars in object completion
p-spacek Oct 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 33 additions & 19 deletions src/languageservice/services/yamlCompletion.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -640,8 +640,9 @@ export class YamlCompletion {
completionItem.textEdit.newText = completionItem.insertText;
}
// remove $x or use {$x:value} in documentation
const mdText = insertText.replace(/\${[0-9]+[:|](.*)}/g, (s, arg) => arg).replace(/\$([0-9]+)/g, '');

let mdText = insertText.replace(/\${[0-9]+[:|](.*)}/g, (s, arg) => arg).replace(/\$([0-9]+)/g, '');
// unescape special chars for markdown, reverse operation to getInsertTextForPlainText
mdText = getOriginalTextFromEscaped(mdText);
const originalDocumentation = completionItem.documentation ? [completionItem.documentation, '', '----', ''] : [];
completionItem.documentation = {
kind: MarkupKind.Markdown,
Expand Down Expand Up @@ -1095,6 +1096,7 @@ export class YamlCompletion {

Object.keys(schema.properties).forEach((key: string) => {
const propertySchema = schema.properties[key] as JSONSchema;
const keyEscaped = getInsertTextForPlainText(key);
let type = Array.isArray(propertySchema.type) ? propertySchema.type[0] : propertySchema.type;
if (!type) {
if (propertySchema.anyOf) {
Expand All @@ -1114,14 +1116,14 @@ export class YamlCompletion {
case 'number':
case 'integer':
case 'anyOf': {
let value = propertySchema.default || propertySchema.const;
if (value) {
if (type === 'string') {
let value = propertySchema.default === undefined ? propertySchema.const : propertySchema.default;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes condition to allow null,0,emptyString as a default or const
the same logic is correctly used in other places

if (isDefined(value)) {
if (type === 'string' || typeof value === 'string') {
value = convertToStringValue(value);
}
insertText += `${indent}${key}: \${${insertIndex++}:${value}}\n`;
insertText += `${indent}${keyEscaped}: \${${insertIndex++}:${value}}\n`;
} else {
insertText += `${indent}${key}: $${insertIndex++}\n`;
insertText += `${indent}${keyEscaped}: $${insertIndex++}\n`;
}
break;
}
Expand All @@ -1138,7 +1140,7 @@ export class YamlCompletion {
arrayTemplate = arrayInsertLines.join('\n');
}
insertIndex = arrayInsertResult.insertIndex;
insertText += `${indent}${key}:\n${indent}${this.indentation}- ${arrayTemplate}\n`;
insertText += `${indent}${keyEscaped}:\n${indent}${this.indentation}- ${arrayTemplate}\n`;
}
break;
case 'object':
Expand All @@ -1150,7 +1152,7 @@ export class YamlCompletion {
insertIndex++
);
insertIndex = objectInsertResult.insertIndex;
insertText += `${indent}${key}:\n${objectInsertResult.insertText}\n`;
insertText += `${indent}${keyEscaped}:\n${objectInsertResult.insertText}\n`;
}
break;
}
Expand All @@ -1165,7 +1167,7 @@ export class YamlCompletion {
}: \${${insertIndex++}:${propertySchema.default}}\n`;
break;
case 'string':
insertText += `${indent}${key}: \${${insertIndex++}:${convertToStringValue(propertySchema.default)}}\n`;
insertText += `${indent}${keyEscaped}: \${${insertIndex++}:${convertToStringValue(propertySchema.default)}}\n`;
break;
case 'array':
case 'object':
Expand Down Expand Up @@ -1230,7 +1232,7 @@ export class YamlCompletion {
case 'string': {
let snippetValue = JSON.stringify(value);
snippetValue = snippetValue.substr(1, snippetValue.length - 2); // remove quotes
snippetValue = this.getInsertTextForPlainText(snippetValue); // escape \ and }
snippetValue = getInsertTextForPlainText(snippetValue); // escape \ and }
if (type === 'string') {
snippetValue = convertToStringValue(snippetValue);
}
Expand All @@ -1243,10 +1245,6 @@ export class YamlCompletion {
return this.getInsertTextForValue(value, separatorAfter, type);
}

private getInsertTextForPlainText(text: string): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved outside of the class to be able to use it from global functions

return text.replace(/[\\$}]/g, '\\$&'); // escape $, \ and }
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
private getInsertTextForValue(value: any, separatorAfter: string, type: string | string[]): string {
if (value === null) {
Expand All @@ -1259,13 +1257,13 @@ export class YamlCompletion {
}
case 'number':
case 'boolean':
return this.getInsertTextForPlainText(value + separatorAfter);
return getInsertTextForPlainText(value + separatorAfter);
}
type = Array.isArray(type) ? type[0] : type;
if (type === 'string') {
value = convertToStringValue(value);
}
return this.getInsertTextForPlainText(value + separatorAfter);
return getInsertTextForPlainText(value + separatorAfter);
}

private getInsertTemplateForValue(
Expand All @@ -1290,14 +1288,14 @@ export class YamlCompletion {
if (typeof element === 'object') {
valueTemplate = `${this.getInsertTemplateForValue(element, indent + this.indentation, navOrder, separatorAfter)}`;
} else {
valueTemplate = ` \${${navOrder.index++}:${this.getInsertTextForPlainText(element + separatorAfter)}}\n`;
valueTemplate = ` \${${navOrder.index++}:${getInsertTextForPlainText(element + separatorAfter)}}\n`;
}
insertText += `${valueTemplate}`;
}
}
return insertText;
}
return this.getInsertTextForPlainText(value + separatorAfter);
return getInsertTextForPlainText(value + separatorAfter);
}

private addSchemaValueCompletions(
Expand Down Expand Up @@ -1669,6 +1667,20 @@ export class YamlCompletion {
}
}

/**
* escape $, \ and }
*/
function getInsertTextForPlainText(text: string): string {
return text.replace(/(\\?)([\\$}])/g, (match, escapeChar, specialChar) => {
// If it's already escaped (has a backslash before it), return it as is
return escapeChar ? match : `\\${specialChar}`;
});
}

function getOriginalTextFromEscaped(text: string): string {
return text.replace(/\\([\\$}])/g, '$1');
}

const isNumberExp = /^\d+$/;
function convertToStringValue(param: unknown): string {
let value: string;
Expand All @@ -1681,6 +1693,8 @@ function convertToStringValue(param: unknown): string {
return value;
}

value = getInsertTextForPlainText(value); // escape $, \ and }

if (value === 'true' || value === 'false' || value === 'null' || isNumberExp.test(value)) {
return `"${value}"`;
}
Expand Down
91 changes: 91 additions & 0 deletions test/autoCompletion.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,27 @@ describe('Auto Completion Tests', () => {
expect(result.items[0].insertText).equal('validation:\n \\"null\\": ${1:false}');
});
});
it('Autocomplete key object with special chars', async () => {
schemaProvider.addSchema(SCHEMA_ID, {
type: 'object',
properties: {
$validation: {
type: 'object',
additionalProperties: false,
properties: {
$prop$1: {
type: 'string',
default: '$value$1',
},
},
},
},
});
const content = ''; // len: 0
const result = await parseSetup(content, 0);
expect(result.items.length).equal(1);
expect(result.items[0].insertText).equals('\\$validation:\n \\$prop\\$1: ${1:\\$value\\$1}');
});

it('Autocomplete on boolean value (with value content)', (done) => {
schemaProvider.addSchema(SCHEMA_ID, {
Expand Down Expand Up @@ -931,6 +952,49 @@ describe('Auto Completion Tests', () => {
);
});

it('Autocompletion should escape $ in defaultValue in anyOf', async () => {
schemaProvider.addSchema(SCHEMA_ID, {
type: 'object',
properties: {
car: {
type: 'object',
required: ['engine'],
properties: {
engine: {
anyOf: [
{
type: 'object',
},
{
type: 'string',
},
],
default: 'type$1234',
},
},
},
},
});
const content = '';
const completion = await parseSetup(content, 0);
expect(completion.items.map((i) => i.insertText)).to.deep.equal(['car:\n engine: ${1:type\\$1234}']);
});

it('Autocompletion should escape $ in property', async () => {
schemaProvider.addSchema(SCHEMA_ID, {
type: 'object',
properties: {
$prop$1: {
type: 'string',
},
},
required: ['$prop$1'],
});
const content = '';
const completion = await parseSetup(content, 0);
expect(completion.items.map((i) => i.insertText)).includes('\\$prop\\$1: ');
});

it('Autocompletion should escape colon when indicating map', async () => {
schemaProvider.addSchema(SCHEMA_ID, {
type: 'object',
Expand Down Expand Up @@ -3125,6 +3189,33 @@ describe('Auto Completion Tests', () => {

expect(result.items.map((i) => i.label)).to.have.members(['fruit', 'vegetable']);
});
it('Should escape insert text with special chars but do not escape it in documenation', async () => {
const schema = {
properties: {
$prop1: {
properties: {
$prop2: {
type: 'string',
},
},
required: ['$prop2'],
},
},
required: ['$prop1'],
};
schemaProvider.addSchema(SCHEMA_ID, schema);
const content = '';
const result = await parseSetup(content, content.length);

expect(
result.items.map((i) => ({ inserText: i.insertText, documentation: (i.documentation as MarkupContent).value }))
).to.deep.equal([
{
inserText: '\\$prop1:\n \\$prop2: ',
documentation: '```yaml\n$prop1:\n $prop2: \n```',
},
]);
});
});
it('Should function when settings are undefined', async () => {
languageService.configure({ completion: true });
Expand Down
2 changes: 1 addition & 1 deletion test/schemaValidation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1289,7 +1289,7 @@ obj:
4,
18,
DiagnosticSeverity.Error,
'yaml-schema: Package',
'yaml-schema: Composer Package',
'https://raw.githubusercontent.com/composer/composer/master/res/composer-schema.json'
)
);
Expand Down
Loading