diff --git a/packages/langium/src/lsp/completion/completion-provider.ts b/packages/langium/src/lsp/completion/completion-provider.ts index 898c09d50..4124ab509 100644 --- a/packages/langium/src/lsp/completion/completion-provider.ts +++ b/packages/langium/src/lsp/completion/completion-provider.ts @@ -23,7 +23,7 @@ import { CompletionItemKind, CompletionList, Position } from 'vscode-languageser import * as ast from '../../grammar/generated/ast.js'; import { getExplicitRuleType } from '../../grammar/internal-grammar-util.js'; import { assignMandatoryAstProperties, getContainerOfType } from '../../utils/ast-util.js'; -import { findDeclarationNodeAtOffset, findLeafNodeAtOffset } from '../../utils/cst-util.js'; +import { findDeclarationNodeAtOffset, findLeafNodeBeforeOffset } from '../../utils/cst-util.js'; import { getEntryRule } from '../../utils/grammar-util.js'; import { stream } from '../../utils/stream.js'; import { findFirstFeatures, findNextFeatures } from './follow-element-computation.js'; @@ -237,7 +237,7 @@ export class DefaultCompletionProvider implements CompletionProvider { const dataTypeRuleOffsets = this.findDataTypeRuleStart(cst, offset); if (dataTypeRuleOffsets) { const [ruleStart, ruleEnd] = dataTypeRuleOffsets; - const parentNode = findLeafNodeAtOffset(cst, ruleStart)?.astNode; + const parentNode = findLeafNodeBeforeOffset(cst, ruleStart)?.astNode; yield { ...partialContext, node: parentNode, @@ -248,9 +248,14 @@ export class DefaultCompletionProvider implements CompletionProvider { } // For all other purposes, it's enough to jump to the start of the current/previous token const { nextTokenStart, nextTokenEnd, previousTokenStart, previousTokenEnd } = this.backtrackToAnyToken(text, offset); - let astNode: AstNode | undefined; + let astNodeOffset = nextTokenStart; + if (offset <= nextTokenStart && previousTokenStart !== undefined) { + // This check indicates that the cursor is still before the next token, so we should use the previous AST node (if it exists) + astNodeOffset = previousTokenStart; + } + const astNode = findLeafNodeBeforeOffset(cst, astNodeOffset)?.astNode; + let performNextCompletion = true; if (previousTokenStart !== undefined && previousTokenEnd !== undefined && previousTokenEnd === offset) { - astNode = findLeafNodeAtOffset(cst, previousTokenStart)?.astNode; // This context aims to complete the current feature yield { ...partialContext, @@ -259,18 +264,26 @@ export class DefaultCompletionProvider implements CompletionProvider { tokenEndOffset: previousTokenEnd, features: this.findFeaturesAt(textDocument, previousTokenStart), }; - // This context aims to complete the immediate next feature (if one exists at the current cursor position) - // It uses the previous AST node for that. - yield { - ...partialContext, - node: astNode, - tokenOffset: previousTokenEnd, - tokenEndOffset: previousTokenEnd, - features: this.findFeaturesAt(textDocument, previousTokenEnd), - }; + // The completion after the current token should be prevented in case we find out that the current token definitely isn't completed yet + // This is usually the case when the current token ends on a letter. + performNextCompletion = this.performNextTokenCompletion( + document, + text.substring(previousTokenStart, previousTokenEnd), + previousTokenStart, + previousTokenEnd + ); + if (performNextCompletion) { + // This context aims to complete the immediate next feature (if one exists at the current cursor position) + // It uses the previous cst start/offset for that. + yield { + ...partialContext, + node: astNode, + tokenOffset: previousTokenEnd, + tokenEndOffset: previousTokenEnd, + features: this.findFeaturesAt(textDocument, previousTokenEnd), + }; + } } - astNode = findLeafNodeAtOffset(cst, nextTokenStart)?.astNode - ?? (previousTokenStart === undefined ? undefined : findLeafNodeAtOffset(cst, previousTokenStart)?.astNode); if (!astNode) { const parserRule = getEntryRule(this.grammar); @@ -284,8 +297,8 @@ export class DefaultCompletionProvider implements CompletionProvider { tokenEndOffset: nextTokenEnd, features: findFirstFeatures(parserRule.definition) }; - } else { - // This context aims to complete the next feature, using the next ast node + } else if (performNextCompletion) { + // This context aims to complete the next feature, using the next cst start/end yield { ...partialContext, node: astNode, @@ -296,6 +309,14 @@ export class DefaultCompletionProvider implements CompletionProvider { } } + protected performNextTokenCompletion(document: LangiumDocument, text: string, _offset: number, _end: number): boolean { + // This regex returns false if the text ends with a letter. + // We don't want to complete new text immediately after a keyword, ID etc. + // We only care about the last character in the text, so we use $ here. + // The \P{L} used here is a Unicode category that matches any character that is not a letter + return /\P{L}$/u.test(text); + } + protected findDataTypeRuleStart(cst: CstNode, offset: number): [number, number] | undefined { let containerNode: CstNode | undefined = findDeclarationNodeAtOffset(cst, offset, this.grammarConfig.nameRegexp); // Identify whether the element was parsed as part of a data type rule @@ -454,7 +475,7 @@ export class DefaultCompletionProvider implements CompletionProvider { protected filterKeyword(context: CompletionContext, keyword: ast.Keyword): boolean { // Filter out keywords that do not contain any word character - return keyword.value.match(/[\w]/) !== null; + return /\p{L}/u.test(keyword.value); } protected fillCompletionItem(context: CompletionContext, item: CompletionValueItem): CompletionItem | undefined { diff --git a/packages/langium/src/utils/cst-util.ts b/packages/langium/src/utils/cst-util.ts index e4a8b8529..094e346c2 100644 --- a/packages/langium/src/utils/cst-util.ts +++ b/packages/langium/src/utils/cst-util.ts @@ -155,30 +155,77 @@ export function isCommentNode(cstNode: CstNode, commentNames: string[]): boolean return isLeafCstNode(cstNode) && commentNames.includes(cstNode.tokenType.name); } +/** + * Finds the leaf CST node at the specified 0-based string offset. + * Note that the given offset will be within the range of the returned leaf node. + * + * If the offset does not point to a CST node (but just white space), this method will return `undefined`. + * + * @param node The CST node to search through. + * @param offset The specified offset. + * @returns The CST node at the specified offset. + */ export function findLeafNodeAtOffset(node: CstNode, offset: number): LeafCstNode | undefined { if (isLeafCstNode(node)) { return node; } else if (isCompositeCstNode(node)) { - let firstChild = 0; - let lastChild = node.content.length - 1; - while (firstChild < lastChild) { - const middleChild = Math.floor((firstChild + lastChild) / 2); - const n = node.content[middleChild]; - if (n.offset > offset) { - lastChild = middleChild - 1; - } else if (n.end <= offset) { - firstChild = middleChild + 1; - } else { - return findLeafNodeAtOffset(n, offset); - } + const searchResult = binarySearch(node, offset, false); + if (searchResult) { + return findLeafNodeAtOffset(searchResult, offset); } - if (firstChild === lastChild) { - return findLeafNodeAtOffset(node.content[firstChild], offset); + } + return undefined; +} + +/** + * Finds the leaf CST node at the specified 0-based string offset. + * If no CST node exists at the specified position, it will return the leaf node before it. + * + * If there is no leaf node before the specified offset, this method will return `undefined`. + * + * @param node The CST node to search through. + * @param offset The specified offset. + * @returns The CST node closest to the specified offset. + */ +export function findLeafNodeBeforeOffset(node: CstNode, offset: number): LeafCstNode | undefined { + if (isLeafCstNode(node)) { + return node; + } else if (isCompositeCstNode(node)) { + const searchResult = binarySearch(node, offset, true); + if (searchResult) { + return findLeafNodeBeforeOffset(searchResult, offset); } } return undefined; } +function binarySearch(node: CompositeCstNode, offset: number, closest: boolean): CstNode | undefined { + let left = 0; + let right = node.content.length - 1; + let closestNode: CstNode | undefined = undefined; + + while (left <= right) { + const middle = Math.floor((left + right) / 2); + const middleNode = node.content[middle]; + + if (middleNode.offset <= offset && middleNode.end > offset) { + // Found an exact match + return middleNode; + } + + if (middleNode.end <= offset) { + // Update the closest node (less than offset) and move to the right half + closestNode = closest ? middleNode : undefined; + left = middle + 1; + } else { + // Move to the left half + right = middle - 1; + } + } + + return closestNode; +} + export function getPreviousNode(node: CstNode, hidden = true): CstNode | undefined { while (node.container) { const parent = node.container; diff --git a/packages/langium/test/lsp/completion-provider.test.ts b/packages/langium/test/lsp/completion-provider.test.ts index ce63b99db..c58497045 100644 --- a/packages/langium/test/lsp/completion-provider.test.ts +++ b/packages/langium/test/lsp/completion-provider.test.ts @@ -75,6 +75,25 @@ describe('Langium completion provider', () => { ] }); }); + + test('Should not complete next feature directly after ID', async () => { + const model = ` + grammar g<|> + A: name<|>="A"; + B: "B";`; + // The completion provider used to propose any referencable element here + // It should not propose anything + await completion({ + text: model, + index: 0, + expectedItems: [] + }); + await completion({ + text: model, + index: 1, + expectedItems: [] + }); + }); }); describe('Completion within alternatives', () => { diff --git a/packages/langium/test/lsp/goto-definition.test.ts b/packages/langium/test/lsp/goto-definition.test.ts index dbae2ca4d..165a1f3af 100644 --- a/packages/langium/test/lsp/goto-definition.test.ts +++ b/packages/langium/test/lsp/goto-definition.test.ts @@ -30,7 +30,7 @@ interface A { } X retu<|>rns A: - <|>na<|>me<|>=ID; + <|>na<|>me<|>=ID; <|> `.trim(); const grammarServices = createLangiumGrammarServices(EmptyFileSystem).grammar; @@ -94,7 +94,7 @@ describe('Definition Provider', () => { }); }); - describe('Should not find anything on keywords', () => { + describe('Should not find anything on certain cst nodes', () => { test('Should find nothing on `terminal` keyword', async () => { await gotoDefinition({ @@ -111,5 +111,13 @@ describe('Definition Provider', () => { rangeIndex: [] }); }); + + test('Should find nothing white space', async () => { + await gotoDefinition({ + text, + index: 9, + rangeIndex: [] + }); + }); }); }); diff --git a/packages/langium/test/lsp/hover.test.ts b/packages/langium/test/lsp/hover.test.ts index d6af6939a..a15369e57 100644 --- a/packages/langium/test/lsp/hover.test.ts +++ b/packages/langium/test/lsp/hover.test.ts @@ -17,7 +17,7 @@ const text = ` * Hi I am a grammar JSDoc comment */ // Another single line comment - grammar <|>g + grammar <|>g <|> /** * Hi I am Rule 'X' */ @@ -33,6 +33,14 @@ const hover = expectHover(grammarServices); describe('Hover', () => { + test('Hovering over whitespace should not provide a hover', async () => { + await hover({ + text, + index: 1, + hover: undefined + }); + }); + test('Hovering over the root node should also provide the documentation', async () => { await hover({ text, @@ -44,7 +52,7 @@ describe('Hover', () => { test('Hovering over X definition shows the comment hovering', async () => { await hover({ text, - index: 1, + index: 2, hover: "Hi I am Rule 'X'" }); }); @@ -52,7 +60,7 @@ describe('Hover', () => { test('Hovering over X definition shows the comment hovering', async () => { await hover({ text, - index: 3, + index: 4, hover: "Hi I am Rule 'X'" }); }); @@ -60,7 +68,7 @@ describe('Hover', () => { test('Hovering over Y renders the link as a vscode uri link', async () => { await hover({ text, - index: 2, + index: 3, hover: /Hi I reference Rule \[`X`\]\(file:\/\/\/\w*\.langium#L14%2C3\)/ }); }); diff --git a/packages/langium/test/utils/cst-utils.test.ts b/packages/langium/test/utils/cst-utils.test.ts index c01d4e302..5d8794e55 100644 --- a/packages/langium/test/utils/cst-utils.test.ts +++ b/packages/langium/test/utils/cst-utils.test.ts @@ -4,55 +4,55 @@ * terms of the MIT License, which is available in the project root. ******************************************************************************/ -import type { Grammar } from 'langium'; +import type { Grammar, LeafCstNode } from 'langium'; import { describe, expect, test } from 'vitest'; -import { createLangiumGrammarServices, findLeafNodeAtOffset, EmptyFileSystem } from 'langium'; +import { createLangiumGrammarServices, findLeafNodeAtOffset, EmptyFileSystem, findLeafNodeBeforeOffset, expandToString } from 'langium'; import { parseHelper } from 'langium/test'; const services = createLangiumGrammarServices(EmptyFileSystem); const parser = parseHelper(services.grammar); -describe('CST Utils', () => { - - test('Find Leaf Node at Offset: Main: value=<|>AB;', async () => { - const text = ` - grammar test - Main: value=AB; - terminal fragment Frag: 'B'; - terminal AB: 'A' Frag; - `; - - const grammar = await parser(text); - const offset = grammar.textDocument.getText().indexOf('AB'); - const leafnode = findLeafNodeAtOffset(grammar.parseResult.value.$cstNode!, offset!); - expect(leafnode!.text).toBe('AB'); +describe('findLeafNode', () => { + + for (const findLeafNode of [ + findLeafNodeAtOffset, + findLeafNodeBeforeOffset + ]) { + test(`Find "AB" using ${findLeafNode.name} at Main: value=<|>AB;`, async () => { + const leafnode = await getLeafNode(findLeafNode, 0); + expect(leafnode?.text).toBe('AB'); + }); + + test(`Find "AB" using ${findLeafNode.name} at Main: value=A<|>B;`, async () => { + const leafnode = await getLeafNode(findLeafNode, 1); + expect(leafnode?.text).toBe('AB'); + }); + + test(`Find ";" using ${findLeafNode.name} at Main: value=AB<|>;`, async () => { + const leafnode = await getLeafNode(findLeafNode, 2); + expect(leafnode?.text).toBe(';'); + }); + } + + test('Find no leaf Node at offset: Main: value=AB <|> ;', async () => { + const leafnode = await getLeafNode(findLeafNodeAtOffset, 2, 3); + expect(leafnode).toBeUndefined(); }); - test('Find Leaf Node at Offset: Main: value=A<|>B;', async () => { - const text = ` - grammar test - Main: value=AB; - terminal fragment Frag: 'B'; - terminal AB: 'A' Frag; - `; - - const grammar = await parser(text); - const offset = grammar.textDocument.getText().indexOf('AB') + 1; - const leafnode = findLeafNodeAtOffset(grammar.parseResult.value.$cstNode!, offset!); - expect(leafnode!.text).toBe('AB'); + test('Find "AB" before offset: Main: value=AB <|> ;', async () => { + const leafnode = await getLeafNode(findLeafNodeBeforeOffset, 2, 3); + expect(leafnode).toBeDefined(); + expect(leafnode?.text).toBe('AB'); }); - test('Find Leaf Node at Offset: Main: value=AB<|>;', async () => { - const text = ` - grammar test - Main: value=AB; - terminal fragment Frag: 'B'; - terminal AB: 'A' Frag; + async function getLeafNode(findLeafNode: typeof findLeafNodeAtOffset, index: number, spaces?: number): Promise { + const text = expandToString` + Main: value=AB${spaces ? ' '.repeat(spaces) : ''}; + terminal AB: 'A'; `; - const grammar = await parser(text); - const offset = grammar.textDocument.getText().indexOf('AB') + 2; - const leafnode = findLeafNodeAtOffset(grammar.parseResult.value.$cstNode!, offset!); - expect(leafnode!.text).toBe(';'); - }); + const offset = text.indexOf('AB') + index; + const leafnode = findLeafNode(grammar.parseResult.value.$cstNode!, offset!); + return leafnode; + } });