Skip to content

Commit e8b28bd

Browse files
vijayupadyavijay upadya
vijayupadya
and
vijay upadya
authored
Update linkifier to navigate to correct method in qualified names (#1604)
* Handle function names correctly * Few optimizations * simplification * Make solution generic --------- Co-authored-by: vijay upadya <[email protected]>
1 parent 0444c85 commit e8b28bd

File tree

5 files changed

+238
-20
lines changed

5 files changed

+238
-20
lines changed

‎src/extension/linkify/test/vscode-node/findSymbol.test.ts‎

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,4 +82,147 @@ suite('Find symbol', () => {
8282
test('Should match on symbols with _', () => {
8383
assert.strictEqual(findBestSymbolByPath([docSymbol('_a_')], '_a_')?.name, '_a_');
8484
});
85+
86+
test('Should prefer rightmost symbol in flat symbols', () => {
87+
// When symbols are flat (SymbolInformation), prefer the rightmost match
88+
// This handles cases like `TextModel.undo()` where we want `undo`, not `TextModel`
89+
assert.strictEqual(
90+
findBestSymbolByPath([
91+
symbolInfo('TextModel'),
92+
symbolInfo('undo')
93+
], 'TextModel.undo()')?.name,
94+
'undo'
95+
);
96+
});
97+
98+
test('Should fall back to leftmost symbol if rightmost not found in flat symbols', () => {
99+
// If the rightmost part isn't found, fall back to leftmost matches
100+
assert.strictEqual(
101+
findBestSymbolByPath([
102+
symbolInfo('TextModel'),
103+
symbolInfo('someOtherMethod')
104+
], 'TextModel.undo()')?.name,
105+
'TextModel'
106+
);
107+
});
108+
109+
test('Should prefer hierarchical match over flat last part match', () => {
110+
// When both hierarchical and flat symbols exist, prefer the hierarchical match
111+
assert.strictEqual(
112+
findBestSymbolByPath([
113+
docSymbol('TextModel', docSymbol('undo')),
114+
symbolInfo('undo') // This is a different undo from a different class
115+
], 'TextModel.undo()')?.name,
116+
'undo'
117+
);
118+
});
119+
120+
test('Should handle deeply qualified names', () => {
121+
// Test multiple levels of qualification
122+
assert.strictEqual(
123+
findBestSymbolByPath([
124+
docSymbol('namespace', docSymbol('TextModel', docSymbol('undo')))
125+
], 'namespace.TextModel.undo()')?.name,
126+
'undo'
127+
);
128+
129+
// With flat symbols, prefer the rightmost part
130+
assert.strictEqual(
131+
findBestSymbolByPath([
132+
symbolInfo('namespace'),
133+
symbolInfo('TextModel'),
134+
symbolInfo('undo')
135+
], 'namespace.TextModel.undo()')?.name,
136+
'undo'
137+
);
138+
139+
// Middle part should be preferred over leftmost
140+
assert.strictEqual(
141+
findBestSymbolByPath([
142+
symbolInfo('namespace'),
143+
symbolInfo('TextModel')
144+
], 'namespace.TextModel.undo()')?.name,
145+
'TextModel'
146+
);
147+
});
148+
149+
test('Should handle mixed flat and hierarchical symbols', () => {
150+
// Some symbols are flat, some are nested
151+
assert.strictEqual(
152+
findBestSymbolByPath([
153+
symbolInfo('Model'),
154+
docSymbol('TextModel', docSymbol('undo')),
155+
symbolInfo('OtherClass')
156+
], 'TextModel.undo()')?.name,
157+
'undo'
158+
);
159+
});
160+
161+
test('Should handle Python-style naming conventions', () => {
162+
// Python uses underscores instead of camelCase
163+
assert.strictEqual(
164+
findBestSymbolByPath([
165+
docSymbol('MyClass', docSymbol('my_method'))
166+
], 'MyClass.my_method()')?.name,
167+
'my_method'
168+
);
169+
170+
// Python dunder methods
171+
assert.strictEqual(
172+
findBestSymbolByPath([
173+
docSymbol('MyClass', docSymbol('__init__'))
174+
], 'MyClass.__init__()')?.name,
175+
'__init__'
176+
);
177+
178+
// Python private methods
179+
assert.strictEqual(
180+
findBestSymbolByPath([
181+
docSymbol('MyClass', docSymbol('_private_method'))
182+
], 'MyClass._private_method()')?.name,
183+
'_private_method'
184+
);
185+
});
186+
187+
test('Should handle Python module qualified names', () => {
188+
// Python: module.Class.method
189+
assert.strictEqual(
190+
findBestSymbolByPath([
191+
docSymbol('my_module', docSymbol('MyClass', docSymbol('my_method')))
192+
], 'my_module.MyClass.my_method()')?.name,
193+
'my_method'
194+
);
195+
});
196+
197+
test('Should prefer rightmost match in flat symbols using position-based priority', () => {
198+
// When both class and method exist as flat symbols, prefer rightmost
199+
assert.strictEqual(
200+
findBestSymbolByPath([
201+
symbolInfo('TextModel'), // matchCount=1 (index 0)
202+
symbolInfo('undo') // matchCount=2 (index 1)
203+
], 'TextModel.undo()')?.name,
204+
'undo'
205+
);
206+
207+
// Reverse order - should still prefer undo due to higher matchCount
208+
assert.strictEqual(
209+
findBestSymbolByPath([
210+
symbolInfo('undo'), // matchCount=2 (index 1)
211+
symbolInfo('TextModel') // matchCount=1 (index 0)
212+
], 'TextModel.undo()')?.name,
213+
'undo'
214+
);
215+
216+
// Works for longer qualified names too
217+
// For 'a.b.c.d' => ['a', 'b', 'c', 'd']:
218+
// 'd' (index 3, matchCount=4) > 'c' (index 2, matchCount=3) > 'b' (index 1, matchCount=2) > 'a' (index 0, matchCount=1)
219+
assert.strictEqual(
220+
findBestSymbolByPath([
221+
symbolInfo('a'), // matchCount=1
222+
symbolInfo('b'), // matchCount=2
223+
symbolInfo('c'), // matchCount=3
224+
], 'a.b.c.d')?.name,
225+
'c' // Highest matchCount among available symbols
226+
);
227+
});
85228
});

‎src/extension/linkify/vscode-node/commands.ts‎

Lines changed: 51 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,7 @@ export function registerLinkCommands(
101101
// Command used when we have already resolved the link to a location.
102102
// This is currently used by the inline code linkifier for links such as `symbolName`
103103
vscode.commands.registerCommand(openSymbolFromReferencesCommand, async (...[_word, locations, requestId]: OpenSymbolFromReferencesCommandArgs) => {
104-
const dest = await resolveSymbolFromReferences(locations, CancellationToken.None);
104+
const dest = await resolveSymbolFromReferences(locations, undefined, CancellationToken.None);
105105

106106
/* __GDPR__
107107
"panel.action.openSymbolFromReferencesLink" : {
@@ -136,12 +136,32 @@ function toLocationLink(def: vscode.Location | vscode.LocationLink): vscode.Loca
136136
}
137137
}
138138

139-
export async function resolveSymbolFromReferences(locations: ReadonlyArray<{ uri: UriComponents; pos: vscode.Position }>, token: CancellationToken) {
139+
function findSymbolByName(symbols: Array<vscode.SymbolInformation | vscode.DocumentSymbol>, symbolName: string, maxDepth: number = 5): vscode.SymbolInformation | vscode.DocumentSymbol | undefined {
140+
for (const symbol of symbols) {
141+
if (symbol.name === symbolName) {
142+
return symbol;
143+
}
144+
// Check children if it's a DocumentSymbol and we haven't exceeded max depth
145+
if (maxDepth > 0 && 'children' in symbol && symbol.children) {
146+
const found = findSymbolByName(symbol.children, symbolName, maxDepth - 1);
147+
if (found) {
148+
return found;
149+
}
150+
}
151+
}
152+
return undefined;
153+
}
154+
155+
export async function resolveSymbolFromReferences(locations: ReadonlyArray<{ uri: UriComponents; pos: vscode.Position }>, symbolText: string | undefined, token: CancellationToken) {
140156
let dest: {
141157
type: 'definition' | 'firstOccurrence' | 'unresolved';
142158
loc: vscode.LocationLink;
143159
} | undefined;
144160

161+
// Extract the rightmost part from qualified symbol like "TextModel.undo()"
162+
const symbolParts = symbolText ? Array.from(symbolText.matchAll(/[#\w$][\w\d$]*/g), x => x[0]) : [];
163+
const targetSymbolName = symbolParts.length >= 2 ? symbolParts[symbolParts.length - 1] : undefined;
164+
145165
// TODO: These locations may no longer be valid if the user has edited the file since the references were found.
146166
for (const loc of locations) {
147167
try {
@@ -151,9 +171,37 @@ export async function resolveSymbolFromReferences(locations: ReadonlyArray<{ uri
151171
}
152172

153173
if (def) {
174+
const defLoc = toLocationLink(def);
175+
176+
// If we have a qualified name like "TextModel.undo()", try to find the specific symbol in the file
177+
if (targetSymbolName && symbolParts.length >= 2) {
178+
try {
179+
const symbols = await vscode.commands.executeCommand<Array<vscode.SymbolInformation | vscode.DocumentSymbol> | undefined>('vscode.executeDocumentSymbolProvider', defLoc.targetUri);
180+
if (symbols) {
181+
// Search for the target symbol in the document symbols
182+
const targetSymbol = findSymbolByName(symbols, targetSymbolName);
183+
if (targetSymbol) {
184+
let targetRange: vscode.Range;
185+
if ('selectionRange' in targetSymbol) {
186+
targetRange = targetSymbol.selectionRange;
187+
} else {
188+
targetRange = targetSymbol.location.range;
189+
}
190+
dest = {
191+
type: 'definition',
192+
loc: { targetUri: defLoc.targetUri, targetRange: targetRange, targetSelectionRange: targetRange },
193+
};
194+
break;
195+
}
196+
}
197+
} catch {
198+
// Failed to find symbol, fall through to use the first definition
199+
}
200+
}
201+
154202
dest = {
155203
type: 'definition',
156-
loc: toLocationLink(def),
204+
loc: defLoc,
157205
};
158206
break;
159207
}

‎src/extension/linkify/vscode-node/findSymbol.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,16 @@ function findBestSymbol(
4242
bestMatch = match;
4343
}
4444
} else { // Is a vscode.SymbolInformation
45-
if (symbol.name === symbolParts[0]) {
46-
bestMatch ??= { symbol, matchCount: 1 };
45+
// For flat symbol information, try to match against symbol parts
46+
// Prefer symbols that appear more to the right (higher index) in the qualified name
47+
// This prioritizes members over classes (e.g., in `TextModel.undo()`, prefer `undo`)
48+
const matchIndex = symbolParts.indexOf(symbol.name);
49+
if (matchIndex !== -1) {
50+
// Higher index = more to the right = higher priority
51+
const match = { symbol, matchCount: matchIndex + 1 };
52+
if (!bestMatch || match.matchCount > bestMatch.matchCount) {
53+
bestMatch = match;
54+
}
4755
}
4856
}
4957
}

‎src/extension/linkify/vscode-node/findWord.ts‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -220,23 +220,42 @@ export class ReferencesSymbolResolver {
220220

221221
// But then try breaking up inline code into symbol parts
222222
if (!wordMatches.length) {
223-
// Find the first symbol name before a non-symbol character
224-
// This will match `foo` in `this.foo(bar)`;
225-
const parts = codeText.split(/([#\w$][\w\d$]*)/g).map(x => x.trim()).filter(x => x.length);
226-
let primaryPart: string | undefined = undefined;
227-
for (const part of parts) {
228-
if (!/[#\w$][\w\d$]*/.test(part)) {
229-
break;
230-
}
231-
primaryPart = part;
232-
}
233-
234-
if (primaryPart && primaryPart !== codeText) {
235-
wordMatches = await this.instantiationService.invokeFunction(accessor => findWordInReferences(accessor, references, primaryPart, {
236-
// Always use stricter matching here as the parts can otherwise match on a lot of things
223+
// Extract all symbol parts from the code text
224+
// For example: `TextModel.undo()` -> ['TextModel', 'undo']
225+
const symbolParts = Array.from(codeText.matchAll(/[#\w$][\w\d$]*/g), x => x[0]);
226+
227+
if (symbolParts.length >= 2) {
228+
// For qualified names like `Class.method()`, search for both parts together
229+
// This helps disambiguate when there are multiple methods with the same name
230+
const firstPart = symbolParts[0];
231+
const lastPart = symbolParts[symbolParts.length - 1];
232+
233+
// First, try to find the class
234+
const classMatches = await this.instantiationService.invokeFunction(accessor => findWordInReferences(accessor, references, firstPart, {
237235
symbolMatchesOnly: true,
238236
maxResultCount: this.findWordOptions.maxResultCount,
239237
}, token));
238+
239+
// If we found the class, we'll rely on the click-time resolution to find the method
240+
if (classMatches.length) {
241+
wordMatches = classMatches;
242+
} else {
243+
// If no class found, try just the method name as fallback
244+
wordMatches = await this.instantiationService.invokeFunction(accessor => findWordInReferences(accessor, references, lastPart, {
245+
symbolMatchesOnly: true,
246+
maxResultCount: this.findWordOptions.maxResultCount,
247+
}, token));
248+
}
249+
} else if (symbolParts.length > 0) {
250+
// For single names like `undo`, try to find the method directly
251+
const lastPart = symbolParts[symbolParts.length - 1];
252+
253+
if (lastPart && lastPart !== codeText) {
254+
wordMatches = await this.instantiationService.invokeFunction(accessor => findWordInReferences(accessor, references, lastPart, {
255+
symbolMatchesOnly: true,
256+
maxResultCount: this.findWordOptions.maxResultCount,
257+
}, token));
258+
}
240259
}
241260
}
242261

‎src/extension/linkify/vscode-node/inlineCodeSymbolLinkifier.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ export class InlineCodeSymbolLinkifier implements IContributedLinkifier {
6060
};
6161

6262
out.push(new LinkifySymbolAnchor(info, async (token) => {
63-
const dest = await resolveSymbolFromReferences(loc.map(loc => ({ uri: loc.uri, pos: loc.range.start })), token);
63+
const dest = await resolveSymbolFromReferences(loc.map(loc => ({ uri: loc.uri, pos: loc.range.start })), symbolText, token);
6464
if (dest) {
6565
const selectionRange = dest.loc.targetSelectionRange ?? dest.loc.targetRange;
6666
info.location = new vscode.Location(dest.loc.targetUri, collapseRangeToStart(selectionRange));

0 commit comments

Comments
 (0)