Skip to content

Fix incoming call hierarchy ranges #11455

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bschnurr wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix incoming call hierarchy ranges #11455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/pyright-internal/src/languageService/callHierarchyProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,12 @@ class FindIncomingCallTreeWalker extends ParseTreeWalker {
};
} else {
const functionRange = convertOffsetsToRange(
executionNode.start,
executionNode.start + executionNode.length,
this._parseResults.tokenizerOutput.lines
);

const functionSelectionRange = convertOffsetsToRange(
executionNode.d.name.start,
executionNode.d.name.start + executionNode.d.name.length,
this._parseResults.tokenizerOutput.lines
Expand All @@ -599,7 +605,7 @@ class FindIncomingCallTreeWalker extends ParseTreeWalker {
kind: SymbolKind.Function,
uri: convertUriToLspUriString(this._program.fileSystem, this._fileUri),
range: functionRange,
selectionRange: functionRange,
selectionRange: functionSelectionRange,
};
}

Expand Down
4 changes: 4 additions & 0 deletions packages/pyright-internal/src/languageService/referencesProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,10 @@ export class ReferencesProvider {
return this.getDeclarationForNode(program, fileUri, node, reporter, useCase, token);
}

if (node.nodeType === ParseNodeType.Function) {
Copy link
Copy Markdown
Collaborator

@rchiodo rchiodo May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot generated:
Broader behavioral scope than documented. getDeclarationForPosition is also called by renameProvider.ts and reportReferences. This change means Rename and Find All References now resolve from the def keyword position (previously returned undefined). This is likely a net improvement, but worth noting in the PR description and considering additional test coverage for rename-from-def scenarios.

[verified]

return this.getDeclarationForNode(program, fileUri, node.d.name, reporter, useCase, token);
}

// For other node types, there are no references to be found.
return undefined;
}
Expand Down
16 changes: 10 additions & 6 deletions ...internal/src/tests/fourslash/showcallhierarchy.incomingCalls.aliasedFunction.fourslash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
////
//// def callByName():
//// func()
//// def [|callByAlias|]():
//// /*marker2*/foobar()
//// def [|/*callByAliasRange*/callByAlias|](): /*marker2*/foobar()

// @filename: consume2.py
//// from declare import func as foobar
Expand All @@ -20,10 +19,15 @@
//// func()

{
const ranges = helper.getRanges();
const itemList = ranges.map((range) => {
return { filePath: range.fileName, range: helper.convertPositionRange(range), name: 'callByAlias' };
});
const callByAliasRange = helper.getPositionRange('callByAliasRange');
const itemList = [
{
filePath: helper.getMappedFilePath('consume.py'),
range: helper.expandPositionRange(callByAliasRange, 4, 12),
selectionRange: callByAliasRange,
name: 'callByAlias',
},
];

helper.verifyShowCallHierarchyGetIncomingCalls({
marker1: {
Expand Down
31 changes: 17 additions & 14 deletions ...yright-internal/src/tests/fourslash/showcallhierarchy.incomingCalls.function.fourslash.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,35 @@
/// <reference path="typings/fourslash.d.ts" />

// @filename: declare.py
//// def /*marker1*/func():
//// /*marker3*/def /*marker1*/func():
//// return 1

// @filename: consume.py
//// from declare import func
//// from declare import /*marker2*/func as foobar
////
//// def [|callByName|]():
//// /*marker3*/func()
//// def callByAlias():
//// foobar()
//// def [|/*callByNameSelection*/callByName|](): /*marker2*/func()

// @filename: consume2.py
//// from declare import func
////
//// def [|callByName2|]():
//// func()
//// def [|/*callByName2Selection*/callByName2|](): func()

{
const ranges = helper.getRanges();
const references = ranges.map((range) => {
return { path: range.fileName, range: helper.convertPositionRange(range) };
});
const callByNameSelectionRange = helper.getPositionRange('callByNameSelection');
const callByName2SelectionRange = helper.getPositionRange('callByName2Selection');
const itemList = [
{ filePath: references[0].path, range: references[0].range, name: 'callByName' },
{ filePath: references[1].path, range: references[1].range, name: 'callByName2' },
{
filePath: helper.getMappedFilePath('consume.py'),
Copy link
Copy Markdown
Collaborator

@rchiodo rchiodo May 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot generated:
Magic numbers in expandPositionRange are fragile. The offsets (4, 10) encode "def " (4 chars) and "(): func()" (10 chars). Any future edit to the test Python (renaming, adding type annotations) silently breaks these without a compile error. Consider using a named range [|...|] wrapping the full function definition for the range assertion. This matches the robustness guidance in the test instructions.

[verified]

range: helper.expandPositionRange(callByNameSelectionRange, 4, 10),
selectionRange: callByNameSelectionRange,
name: 'callByName',
},
{
filePath: helper.getMappedFilePath('consume2.py'),
range: helper.expandPositionRange(callByName2SelectionRange, 4, 10),
selectionRange: callByName2SelectionRange,
name: 'callByName2',
},
];

helper.verifyShowCallHierarchyGetIncomingCalls({
Expand Down
1 change: 1 addition & 0 deletions packages/pyright-internal/src/tests/fourslash/typings/fourslash.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ declare namespace _ {
filePath: string;
name: string;
range: PositionRange;
selectionRange?: PositionRange;
}

interface TextRange {
Expand Down
9 changes: 9 additions & 0 deletions packages/pyright-internal/src/tests/harness/fourslash/testState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1272,6 +1272,9 @@ export class TestState {
const expectedFilePath = map[name].items.map((x) => x.filePath);
const expectedRange = map[name].items.map((x) => x.range);
const expectedName = map[name].items.map((x) => x.name);
const expectedSelectionRange = map[name].items
.map((x) => x.selectionRange)
.filter((x): x is PositionRange => x !== undefined);

const position = this.convertOffsetToPosition(fileName, marker.position);
const actual = new CallHierarchyProvider(
Expand All @@ -1289,6 +1292,12 @@ export class TestState {
for (const a of actual) {
assert.strictEqual(expectedRange?.filter((e) => this._deepEqual(a.from.range, e)).length, 1);
assert.strictEqual(expectedName?.filter((e) => this._deepEqual(a.from.name, e)).length, 1);
if (expectedSelectionRange.length > 0) {
assert.strictEqual(
expectedSelectionRange.filter((e) => this._deepEqual(a.from.selectionRange, e)).length,
1
);
}
assert.ok(
expectedFilePath?.filter((e) =>
this._deepEqual(a.from.uri, Uri.file(e, this.serviceProvider).toString())
Expand Down
Loading