Skip to content

Commit 8678202

Browse files
authored
Improve source range display on definition request (#2063)
1 parent c88f857 commit 8678202

File tree

4 files changed

+98
-16
lines changed

4 files changed

+98
-16
lines changed

packages/langium/src/lsp/completion/completion-provider.ts

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import type { MarkupContent } from 'vscode-languageserver';
2424
import { CompletionItemKind, CompletionList, Position } from 'vscode-languageserver';
2525
import * as ast from '../../languages/generated/ast.js';
2626
import { assignMandatoryProperties, getContainerOfType } from '../../utils/ast-utils.js';
27-
import { findDeclarationNodeAtOffset, findLeafNodeBeforeOffset } from '../../utils/cst-utils.js';
27+
import { findDeclarationNodeAtOffset, findLeafNodeBeforeOffset, getDatatypeNode } from '../../utils/cst-utils.js';
2828
import { getEntryRule, getExplicitRuleType } from '../../utils/grammar-utils.js';
2929
import { stream, type Stream } from '../../utils/stream.js';
3030
import { findFirstFeatures, findNextFeatures } from './follow-element-computation.js';
@@ -326,18 +326,14 @@ export class DefaultCompletionProvider implements CompletionProvider {
326326
}
327327

328328
protected findDataTypeRuleStart(cst: CstNode, offset: number): [number, number] | undefined {
329-
let containerNode: CstNode | undefined = findDeclarationNodeAtOffset(cst, offset, this.grammarConfig.nameRegexp);
329+
const containerNode = findDeclarationNodeAtOffset(cst, offset, this.grammarConfig.nameRegexp);
330+
if (!containerNode) {
331+
return undefined;
332+
}
330333
// Identify whether the element was parsed as part of a data type rule
331-
let isDataTypeNode = Boolean(getContainerOfType(containerNode?.grammarSource, ast.isParserRule)?.dataType);
332-
if (isDataTypeNode) {
333-
while (isDataTypeNode) {
334-
// Use the container to find the correct parent element
335-
containerNode = containerNode?.container;
336-
isDataTypeNode = Boolean(getContainerOfType(containerNode?.grammarSource, ast.isParserRule)?.dataType);
337-
}
338-
if (containerNode) {
339-
return [containerNode.offset, containerNode.end];
340-
}
334+
const fullNode = getDatatypeNode(containerNode);
335+
if (fullNode) {
336+
return [fullNode.offset, fullNode.end];
341337
}
342338
return undefined;
343339
}

packages/langium/src/lsp/definition-provider.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import type { MaybePromise } from '../utils/promise-utils.js';
1515
import type { LangiumDocument } from '../workspace/documents.js';
1616
import { LocationLink } from 'vscode-languageserver';
1717
import { getDocument } from '../utils/ast-utils.js';
18-
import { findDeclarationNodeAtOffset } from '../utils/cst-utils.js';
18+
import { findDeclarationNodeAtOffset, getDatatypeNode } from '../utils/cst-utils.js';
1919

2020
/**
2121
* Language-specific service for handling go to definition requests.
@@ -79,12 +79,17 @@ export class DefaultDefinitionProvider implements DefinitionProvider {
7979
}
8080

8181
protected findLinks(source: CstNode): GoToLink[] {
82+
const datatypeSourceNode = getDatatypeNode(source) ?? source;
8283
const targets = this.references.findDeclarationNodes(source);
8384
const links: GoToLink[] = [];
8485
for (const target of targets) {
8586
const targetDocument = getDocument(target.astNode);
8687
if (targets && targetDocument) {
87-
links.push({ source, target, targetDocument });
88+
links.push({
89+
source: datatypeSourceNode,
90+
target,
91+
targetDocument
92+
});
8893
}
8994
}
9095
return links;

packages/langium/src/utils/cst-utils.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,34 @@ import type { DocumentSegment } from '../workspace/documents.js';
1111
import type { Stream, TreeStream } from './stream.js';
1212
import { isCompositeCstNode, isLeafCstNode, isRootCstNode } from '../syntax-tree.js';
1313
import { TreeStreamImpl } from './stream.js';
14+
import { getContainerOfType } from './ast-utils.js';
15+
import { isParserRule } from '../languages/generated/ast.js';
16+
17+
/**
18+
* Attempts to find the CST node that belongs to the datatype element that contains the given CST node.
19+
*
20+
* @param cstNode The CST node for which to find the datatype node.
21+
* @returns The CST node corresponding to the datatype element, or the undefined if no such element exists.
22+
*/
23+
export function getDatatypeNode(cstNode: CstNode): CstNode | undefined {
24+
let current: CstNode | undefined = cstNode;
25+
let found = false;
26+
while (current) {
27+
const definingRule = getContainerOfType(current.grammarSource, isParserRule);
28+
if (definingRule && definingRule.dataType) {
29+
// Go up the chain. This element might be part of a larger datatype rule
30+
current = current.container;
31+
found = true;
32+
} else if (found) {
33+
// The last datatype node is the one we are looking for
34+
return current;
35+
} else {
36+
// We haven't found any datatype node yet and we've reached a non-datatype rule
37+
return undefined;
38+
}
39+
}
40+
return undefined;
41+
}
1442

1543
/**
1644
* Create a stream of all CST nodes that are directly and indirectly contained in the given root node,

packages/langium/test/lsp/goto-definition.test.ts

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,12 @@
44
* terms of the MIT License, which is available in the project root.
55
******************************************************************************/
66

7-
import { describe, test } from 'vitest';
8-
import { EmptyFileSystem } from 'langium';
7+
import { describe, expect, test } from 'vitest';
8+
import { EmptyFileSystem, URI } from 'langium';
99
import { createLangiumGrammarServices, createServicesForGrammar } from 'langium/grammar';
1010
import { expectGoToDefinition } from 'langium/test';
11+
import { expandToString } from 'langium/generate';
12+
import type { Range } from 'vscode-languageserver';
1113

1214
/**
1315
* Represents a grammar file
@@ -113,6 +115,57 @@ describe('Definition Provider', () => {
113115
});
114116
});
115117
});
118+
119+
test('Should highlight full datatype rule node', async () => {
120+
const grammar = `
121+
grammar Test
122+
entry Model: (elements+=Element)*;
123+
Element: Source | Target;
124+
Source: 'source' name=FQN;
125+
Target: 'target' ref=[Source];
126+
FQN returns string: ID ('.' ID)*;
127+
terminal ID: /\\w+/;
128+
hidden terminal WS: /\\s+/;
129+
`;
130+
const services = await createServicesForGrammar({ grammar });
131+
const text = expandToString`
132+
target a.b.c;
133+
source a.b.c;
134+
`;
135+
const workspace = services.shared.workspace;
136+
const document = workspace.LangiumDocumentFactory.fromString(text, URI.file('test.txt'));
137+
workspace.LangiumDocuments.addDocument(document);
138+
await workspace.DocumentBuilder.build([document]);
139+
const targetTextRange: Range = {
140+
start: document.textDocument.positionAt(text.indexOf('a.b.c')),
141+
end: document.textDocument.positionAt(text.indexOf('a.b.c') + 'a.b.c'.length)
142+
};
143+
const sourceTextRange: Range = {
144+
start: document.textDocument.positionAt(text.lastIndexOf('a.b.c')),
145+
end: document.textDocument.positionAt(text.lastIndexOf('a.b.c') + 'a.b.c'.length)
146+
};
147+
const provider = services.lsp.DefinitionProvider!;
148+
// Go to definition from target to source
149+
const defFromTarget = await provider.getDefinition(document, {
150+
textDocument: { uri: document.uri.toString() },
151+
position: targetTextRange.start,
152+
});
153+
expect(defFromTarget).toBeDefined();
154+
expect(defFromTarget).toHaveLength(1);
155+
const targetSourceRange = defFromTarget![0].originSelectionRange!;
156+
expect(targetSourceRange).toBeDefined();
157+
expect(targetSourceRange).toEqual(targetTextRange);
158+
// Go to definition from target to itself
159+
const defFromSource = await provider.getDefinition(document, {
160+
textDocument: { uri: document.uri.toString() },
161+
position: sourceTextRange.start,
162+
});
163+
expect(defFromSource).toBeDefined();
164+
expect(defFromSource).toHaveLength(1);
165+
const sourceRange = defFromSource![0].originSelectionRange!;
166+
expect(sourceRange).toBeDefined();
167+
expect(sourceRange).toEqual(sourceTextRange);
168+
});
116169
});
117170

118171
describe('Definition Provider with Infix Operators', async () => {

0 commit comments

Comments
 (0)