From 4b91dd31840131bec685b065e9a0c2f6814ae1e8 Mon Sep 17 00:00:00 2001 From: Petr Spacek Date: Tue, 28 May 2024 13:59:46 +0200 Subject: [PATCH 1/5] feat: unify string insert text for array and property (#934) --- src/languageservice/services/yamlCompletion.ts | 2 +- test/autoCompletion.test.ts | 2 +- test/autoCompletionFix.test.ts | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/languageservice/services/yamlCompletion.ts b/src/languageservice/services/yamlCompletion.ts index 99e061f0b..0ef71baba 100644 --- a/src/languageservice/services/yamlCompletion.ts +++ b/src/languageservice/services/yamlCompletion.ts @@ -1205,7 +1205,7 @@ export class YamlCompletion { insertText = `\${${insertIndex++}:0}`; break; case 'string': - insertText = `\${${insertIndex++}:""}`; + insertText = `\${${insertIndex++}}`; break; case 'object': { diff --git a/test/autoCompletion.test.ts b/test/autoCompletion.test.ts index 78b14996d..dd5abe0c4 100644 --- a/test/autoCompletion.test.ts +++ b/test/autoCompletion.test.ts @@ -1047,7 +1047,7 @@ describe('Auto Completion Tests', () => { const completion = parseSetup(content, content.lastIndexOf('Ba') + 2); // pos: 3+2 completion .then(function (result) { - assert.strictEqual('fooBar:\n - ${1:""}', result.items[0].insertText); + assert.strictEqual('fooBar:\n - ${1}', result.items[0].insertText); }) .then(done, done); }); diff --git a/test/autoCompletionFix.test.ts b/test/autoCompletionFix.test.ts index 81053315b..f81c68aa7 100644 --- a/test/autoCompletionFix.test.ts +++ b/test/autoCompletionFix.test.ts @@ -482,7 +482,7 @@ objB: expect(completion.items.length).equal(1); expect(completion.items[0]).to.be.deep.equal( - createExpectedCompletion('objectWithArray', 'objectWithArray:\n - ${1:""}', 1, 4, 1, 4, 10, 2, { + createExpectedCompletion('objectWithArray', 'objectWithArray:\n - ${1}', 1, 4, 1, 4, 10, 2, { documentation: '', }) ); From e5dcce044e27dd6b6da54107f5eff092649ca002 Mon Sep 17 00:00:00 2001 From: Petr Spacek Date: Tue, 28 May 2024 14:07:20 +0200 Subject: [PATCH 2/5] fix: snippets in additionalProperties (#951) Co-authored-by: Muthurajan Sivasubramanian <93245779+msivasubramaniaan@users.noreply.github.com> --- .../services/yamlCompletion.ts | 3 ++- test/autoCompletionFix.test.ts | 24 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/src/languageservice/services/yamlCompletion.ts b/src/languageservice/services/yamlCompletion.ts index 0ef71baba..be21a8bb7 100644 --- a/src/languageservice/services/yamlCompletion.ts +++ b/src/languageservice/services/yamlCompletion.ts @@ -925,7 +925,8 @@ export class YamlCompletion { if (propertySchema) { this.addSchemaValueCompletions(propertySchema, separatorAfter, collector, types, 'value'); } - } else if (s.schema.additionalProperties) { + } + if (s.schema.additionalProperties) { this.addSchemaValueCompletions(s.schema.additionalProperties, separatorAfter, collector, types, 'value'); } } diff --git a/test/autoCompletionFix.test.ts b/test/autoCompletionFix.test.ts index f81c68aa7..c720ce76d 100644 --- a/test/autoCompletionFix.test.ts +++ b/test/autoCompletionFix.test.ts @@ -1183,6 +1183,30 @@ objB: expect(completion.items[0].insertText).to.be.equal('test1'); }); + it('should suggest defaultSnippets from additionalProperties', async () => { + const schema: JSONSchema = { + type: 'object', + properties: { + id: { + type: 'string', + }, + }, + additionalProperties: { + anyOf: [ + { + type: 'string', + defaultSnippets: [{ label: 'snippet', body: 'snippetBody' }], + }, + ], + }, + }; + schemaProvider.addSchema(SCHEMA_ID, schema); + const content = 'value: |\n|'; + const completion = await parseCaret(content); + + expect(completion.items.map((i) => i.insertText)).to.be.deep.equal(['snippetBody']); + }); + describe('should suggest prop of the object (based on not completed prop name)', () => { const schema: JSONSchema = { definitions: { From 0e17579f36cc56b37d6d65b6438e64937b0f2390 Mon Sep 17 00:00:00 2001 From: Tony <68118705+Legend-Master@users.noreply.github.com> Date: Tue, 28 May 2024 20:13:42 +0800 Subject: [PATCH 3/5] Improve some special cases for selection ranges (#939) * Fix some typings * Improve a few special cases * Add tests --------- Co-authored-by: Muthurajan Sivasubramanian <93245779+msivasubramaniaan@users.noreply.github.com> --- .../services/yamlSelectionRanges.ts | 115 ++++++++++++------ src/languageservice/yamlLanguageService.ts | 2 +- test/yamlSelectionRanges.test.ts | 82 ++++++++++++- tsconfig.json | 2 +- 4 files changed, 155 insertions(+), 46 deletions(-) diff --git a/src/languageservice/services/yamlSelectionRanges.ts b/src/languageservice/services/yamlSelectionRanges.ts index b361ba8d7..f60dcc91a 100644 --- a/src/languageservice/services/yamlSelectionRanges.ts +++ b/src/languageservice/services/yamlSelectionRanges.ts @@ -3,81 +3,72 @@ import { yamlDocumentsCache } from '../parser/yaml-documents'; import { TextDocument } from 'vscode-languageserver-textdocument'; import { ASTNode } from 'vscode-json-languageservice'; -export function getSelectionRanges(document: TextDocument, positions: Position[]): SelectionRange[] | undefined { - if (!document) { - return; - } +export function getSelectionRanges(document: TextDocument, positions: Position[]): SelectionRange[] { const doc = yamlDocumentsCache.getYamlDocument(document); return positions.map((position) => { const ranges = getRanges(position); - let current: SelectionRange; + let current: SelectionRange | undefined; for (const range of ranges) { current = SelectionRange.create(range, current); } - if (!current) { - current = SelectionRange.create({ - start: position, - end: position, - }); - } - return current; + return current ?? SelectionRange.create({ start: position, end: position }); }); function getRanges(position: Position): Range[] { const offset = document.offsetAt(position); const result: Range[] = []; for (const ymlDoc of doc.documents) { - let currentNode: ASTNode; - let firstNodeOffset: number; - let isFirstNode = true; + let currentNode: ASTNode | undefined; + let overrideStartOffset: number | undefined; ymlDoc.visit((node) => { const endOffset = node.offset + node.length; // Skip if end offset doesn't even reach cursor position if (endOffset < offset) { return true; } - let startOffset = node.offset; - // Recheck start offset with the trimmed one in case of this - // key: - // - value - // ↑ - if (startOffset > offset) { - const nodePosition = document.positionAt(startOffset); - if (nodePosition.line !== position.line) { - return true; - } - const lineBeginning = { line: nodePosition.line, character: 0 }; - const text = document.getText({ - start: lineBeginning, - end: nodePosition, - }); - if (text.trim().length !== 0) { + // Skip if we're ending at new line + // times: + // - second: 1 + // millisecond: 10 + // | - second: 2 + // ↑ millisecond: 0 + // (| is actually part of { second: 1, millisecond: 10 }) + // \r\n doesn't matter here + if (getTextFromOffsets(endOffset - 1, endOffset) === '\n') { + if (endOffset - 1 < offset) { return true; } - startOffset = document.offsetAt(lineBeginning); - if (startOffset > offset) { + } + + let startOffset = node.offset; + if (startOffset > offset) { + // Recheck start offset for some special cases + const newOffset = getStartOffsetForSpecialCases(node, position); + if (!newOffset || newOffset > offset) { return true; } + startOffset = newOffset; } + // Allow equal for children to override if (!currentNode || startOffset >= currentNode.offset) { currentNode = node; - firstNodeOffset = startOffset; + overrideStartOffset = startOffset; } return true; }); while (currentNode) { - const startOffset = isFirstNode ? firstNodeOffset : currentNode.offset; + const startOffset = overrideStartOffset ?? currentNode.offset; const endOffset = currentNode.offset + currentNode.length; const range = { start: document.positionAt(startOffset), end: document.positionAt(endOffset), }; const text = document.getText(range); - const trimmedText = text.trimEnd(); - const trimmedLength = text.length - trimmedText.length; - if (trimmedLength > 0) { - range.end = document.positionAt(endOffset - trimmedLength); + const trimmedText = trimEndNewLine(text); + const trimmedEndOffset = startOffset + trimmedText.length; + if (trimmedEndOffset >= offset) { + range.end = document.positionAt(trimmedEndOffset); } // Add a jump between '' "" {} [] const isSurroundedBy = (startCharacter: string, endCharacter?: string): boolean => { @@ -95,7 +86,7 @@ export function getSelectionRanges(document: TextDocument, positions: Position[] } result.push(range); currentNode = currentNode.parent; - isFirstNode = false; + overrideStartOffset = undefined; } // A position can't be in multiple documents if (result.length > 0) { @@ -104,4 +95,48 @@ export function getSelectionRanges(document: TextDocument, positions: Position[] } return result.reverse(); } + + function getStartOffsetForSpecialCases(node: ASTNode, position: Position): number | undefined { + const nodeStartPosition = document.positionAt(node.offset); + if (nodeStartPosition.line !== position.line) { + return; + } + + if (node.parent?.type === 'array') { + // array: + // - value + // ↑ + if (getTextFromOffsets(node.offset - 2, node.offset) === '- ') { + return node.offset - 2; + } + } + + if (node.type === 'array' || node.type === 'object') { + // array: + // - value + // ↑ + const lineBeginning = { line: nodeStartPosition.line, character: 0 }; + const text = document.getText({ start: lineBeginning, end: nodeStartPosition }); + if (text.trim().length === 0) { + return document.offsetAt(lineBeginning); + } + } + } + + function getTextFromOffsets(startOffset: number, endOffset: number): string { + return document.getText({ + start: document.positionAt(startOffset), + end: document.positionAt(endOffset), + }); + } +} + +function trimEndNewLine(str: string): string { + if (str.endsWith('\r\n')) { + return str.substring(0, str.length - 2); + } + if (str.endsWith('\n')) { + return str.substring(0, str.length - 1); + } + return str; } diff --git a/src/languageservice/yamlLanguageService.ts b/src/languageservice/yamlLanguageService.ts index 539371d8b..877fde067 100644 --- a/src/languageservice/yamlLanguageService.ts +++ b/src/languageservice/yamlLanguageService.ts @@ -175,7 +175,7 @@ export interface LanguageService { deleteSchemaContent: (schemaDeletions: SchemaDeletions) => void; deleteSchemasWhole: (schemaDeletions: SchemaDeletionsAll) => void; getFoldingRanges: (document: TextDocument, context: FoldingRangesContext) => FoldingRange[] | null; - getSelectionRanges: (document: TextDocument, positions: Position[]) => SelectionRange[] | undefined; + getSelectionRanges: (document: TextDocument, positions: Position[]) => SelectionRange[]; getCodeAction: (document: TextDocument, params: CodeActionParams) => CodeAction[] | undefined; getCodeLens: (document: TextDocument) => PromiseLike | CodeLens[] | undefined; resolveCodeLens: (param: CodeLens) => PromiseLike | CodeLens; diff --git a/test/yamlSelectionRanges.test.ts b/test/yamlSelectionRanges.test.ts index 3ac78ab41..93fd67e78 100644 --- a/test/yamlSelectionRanges.test.ts +++ b/test/yamlSelectionRanges.test.ts @@ -12,14 +12,16 @@ function isRangesEqual(range1: Range, range2: Range): boolean { ); } -function expectSelections(selectionRange: SelectionRange, ranges: Range[]): void { +function expectSelections(selectionRange: SelectionRange | undefined, ranges: Range[]): void { for (const range of ranges) { - expect(selectionRange.range).eql(range); + expect(selectionRange?.range).eql(range); + // Deduplicate ranges - while (selectionRange.parent && isRangesEqual(selectionRange.range, selectionRange.parent.range)) { + while (selectionRange?.parent && isRangesEqual(selectionRange.range, selectionRange.parent.range)) { selectionRange = selectionRange.parent; } - selectionRange = selectionRange.parent; + + selectionRange = selectionRange?.parent; } } @@ -62,6 +64,20 @@ key: { start: { line: 1, character: 0 }, end: { line: 3, character: 8 } }, ]); + positions = [ + { + line: 3, + character: 3, + }, + ]; + ranges = getSelectionRanges(document, positions); + expect(ranges.length).equal(positions.length); + expectSelections(ranges[0], [ + { start: { line: 3, character: 2 }, end: { line: 3, character: 8 } }, + { start: { line: 2, character: 2 }, end: { line: 3, character: 8 } }, + { start: { line: 1, character: 0 }, end: { line: 3, character: 8 } }, + ]); + positions = [ { line: 2, @@ -76,6 +92,64 @@ key: ]); }); + it('selection ranges for array of objects', () => { + const yaml = ` +times: + - second: 1 + millisecond: 10 + - second: 2 + millisecond: 0 + `; + let positions: Position[] = [ + { + line: 4, + character: 0, + }, + ]; + const document = setupTextDocument(yaml); + let ranges = getSelectionRanges(document, positions); + expect(ranges.length).equal(positions.length); + expectSelections(ranges[0], [ + { start: { line: 2, character: 2 }, end: { line: 5, character: 18 } }, + { start: { line: 1, character: 0 }, end: { line: 5, character: 18 } }, + ]); + + positions = [ + { + line: 5, + character: 2, + }, + ]; + ranges = getSelectionRanges(document, positions); + expect(ranges.length).equal(positions.length); + expectSelections(ranges[0], [ + { start: { line: 4, character: 4 }, end: { line: 5, character: 18 } }, + { start: { line: 2, character: 2 }, end: { line: 5, character: 18 } }, + { start: { line: 1, character: 0 }, end: { line: 5, character: 18 } }, + ]); + }); + + it('selection ranges for trailing spaces', () => { + const yaml = ` +key: + - 1 + - 2 \t + `; + const positions: Position[] = [ + { + line: 2, + character: 9, + }, + ]; + const document = setupTextDocument(yaml); + const ranges = getSelectionRanges(document, positions); + expect(ranges.length).equal(positions.length); + expectSelections(ranges[0], [ + { start: { line: 2, character: 2 }, end: { line: 3, character: 9 } }, + { start: { line: 1, character: 0 }, end: { line: 3, character: 9 } }, + ]); + }); + it('selection ranges jump for "" \'\'', () => { const yaml = ` - "word" diff --git a/tsconfig.json b/tsconfig.json index 5294b2662..fa23d249a 100755 --- a/tsconfig.json +++ b/tsconfig.json @@ -9,7 +9,7 @@ "outDir": "./out/server", "sourceMap": true, "target": "es2020", - "allowSyntheticDefaultImports": true, + "allowSyntheticDefaultImports": true, "skipLibCheck": true }, "include": [ "src", "test" ], From 0871dc12bb1d092bd08831dbe1f2179c41f55342 Mon Sep 17 00:00:00 2001 From: David Thompson Date: Tue, 28 May 2024 08:03:39 -0500 Subject: [PATCH 4/5] Add null check for `customTags` setting (#955) Fixes #807 Signed-off-by: David Thompson Co-authored-by: Muthurajan Sivasubramanian <93245779+msivasubramaniaan@users.noreply.github.com> --- src/languageservice/services/yamlCompletion.ts | 2 +- test/autoCompletion.test.ts | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/languageservice/services/yamlCompletion.ts b/src/languageservice/services/yamlCompletion.ts index be21a8bb7..513a23d63 100644 --- a/src/languageservice/services/yamlCompletion.ts +++ b/src/languageservice/services/yamlCompletion.ts @@ -293,7 +293,7 @@ export class YamlCompletion { proposed, }; - if (this.customTags.length > 0) { + if (this.customTags && this.customTags.length > 0) { this.getCustomTagValueCompletions(collector); } diff --git a/test/autoCompletion.test.ts b/test/autoCompletion.test.ts index dd5abe0c4..eba8fa6d0 100644 --- a/test/autoCompletion.test.ts +++ b/test/autoCompletion.test.ts @@ -3126,5 +3126,10 @@ describe('Auto Completion Tests', () => { expect(result.items.map((i) => i.label)).to.have.members(['fruit', 'vegetable']); }); }); + it('Should function when settings are undefined', async () => { + languageService.configure({ completion: true }); + const content = ''; + await parseSetup(content, 0); + }); }); }); From 7203630540f35c88731bcf03ac41ca90b5919125 Mon Sep 17 00:00:00 2001 From: Gustav Eikaas <46537983+GustavEikaas@users.noreply.github.com> Date: Wed, 29 May 2024 10:02:40 +0200 Subject: [PATCH 5/5] fix: crash when url is undefined (#954) * fix: crash when url is undefined * add another null check * style: prettier fix --------- Co-authored-by: Muthurajan Sivasubramanian <93245779+msivasubramaniaan@users.noreply.github.com> --- src/languageserver/handlers/settingsHandlers.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/languageserver/handlers/settingsHandlers.ts b/src/languageserver/handlers/settingsHandlers.ts index 95ddba773..e2351ea7a 100644 --- a/src/languageserver/handlers/settingsHandlers.ts +++ b/src/languageserver/handlers/settingsHandlers.ts @@ -81,7 +81,7 @@ export class SettingsHandler { if (settings.yaml.schemaStore) { this.yamlSettings.schemaStoreEnabled = settings.yaml.schemaStore.enable; - if (settings.yaml.schemaStore.url.length !== 0) { + if (settings.yaml.schemaStore.url?.length !== 0) { this.yamlSettings.schemaStoreUrl = settings.yaml.schemaStore.url; } } @@ -180,7 +180,7 @@ export class SettingsHandler { private async setSchemaStoreSettingsIfNotSet(): Promise { const schemaStoreIsSet = this.yamlSettings.schemaStoreSettings.length !== 0; let schemaStoreUrl = ''; - if (this.yamlSettings.schemaStoreUrl.length !== 0) { + if (this.yamlSettings.schemaStoreUrl?.length !== 0) { schemaStoreUrl = this.yamlSettings.schemaStoreUrl; } else { schemaStoreUrl = JSON_SCHEMASTORE_URL;