Skip to content

Commit

Permalink
Improve completion AST node accuracy (#1267)
Browse files Browse the repository at this point in the history
  • Loading branch information
@msujew
msujew authored Nov 8, 2023
1 parent 2915ff4 commit bc6dabe
Show file tree
Hide file tree
Showing 6 changed files with 180 additions and 77 deletions.
57 changes: 39 additions & 18 deletions packages/langium/src/lsp/completion/completion-provider.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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);
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
75 changes: 61 additions & 14 deletions packages/langium/src/utils/cst-util.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
19 changes: 19 additions & 0 deletions packages/langium/test/lsp/completion-provider.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
12 changes: 10 additions & 2 deletions packages/langium/test/lsp/goto-definition.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ interface A {
}
X retu<|>rns A:
<|>na<|>me<|>=ID;
<|>na<|>me<|>=ID; <|>
`.trim();

const grammarServices = createLangiumGrammarServices(EmptyFileSystem).grammar;
Expand Down Expand Up @@ -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({
Expand All @@ -111,5 +111,13 @@ describe('Definition Provider', () => {
rangeIndex: []
});
});

test('Should find nothing white space', async () => {
await gotoDefinition({
text,
index: 9,
rangeIndex: []
});
});
});
});
16 changes: 12 additions & 4 deletions packages/langium/test/lsp/hover.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
*/
Expand All @@ -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,
Expand All @@ -44,23 +52,23 @@ 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'"
});
});

test('Hovering over X definition shows the comment hovering', async () => {
await hover({
text,
index: 3,
index: 4,
hover: "Hi I am Rule 'X'"
});
});

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\)/
});
});
Expand Down
Loading

0 comments on commit bc6dabe

Please sign in to comment.