Skip to content

Commit 3b2667e

Browse files
PeterJCLawDonJayamanne
PeterJCLaw
authored and
DonJayamanne
committed
Ensure navigation to definitons follows imports and is transparent to decoration (DonJayamanne#1712)
* Add baseline function definition navigation tests This provides a starting point for more interesting tests. * Add tests for current cases Specifically this covers some basic cases as well as the reproductions of DonJayamanne#1638 and #1033 plus the variant of #1033 which always worked. * Make navigation to definitions follow imports This fixes DonJayamanne#1638 and simplifies the code (to what I believe that @davidhalter had in mind in microsoft#1033 (comment)). This change means that all of the test cases recently added to 'navigation.tests.ts' now pass, meaning that navigtion to the definition of functions works through imports and goes to the original function, even when that function is decorated. * Add news entry for PR * Improve framing of this
1 parent aa578fc commit 3b2667e

6 files changed

Lines changed: 175 additions & 15 deletions

File tree

‎news/2 Fixes/1638.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Ensure navigation to definitons follows imports and is transparent to decoration ([#1638](https://github.com/Microsoft/vscode-python/issues/1638); thanks [Peter Law](https://github.com/PeterJCLaw))

‎pythonFiles/completion.py‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -570,21 +570,7 @@ def _process_request(self, request):
570570
sys_path=sys.path, environment=self.environment)
571571

572572
if lookup == 'definitions':
573-
defs = []
574-
try:
575-
defs = self._get_definitionsx(script.goto_definitions(follow_imports=False), request['id'])
576-
except:
577-
pass
578-
try:
579-
if len(defs) == 0:
580-
defs = self._get_definitionsx(script.goto_definitions(), request['id'])
581-
except:
582-
pass
583-
try:
584-
if len(defs) == 0:
585-
defs = self._get_definitionsx(script.goto_assignments(), request['id'])
586-
except:
587-
pass
573+
defs = self._get_definitionsx(script.goto_assignments(follow_imports=True), request['id'])
588574
return json.dumps({'id': request['id'], 'results': defs})
589575
if lookup == 'tooltip':
590576
if jediPreview:

‎src/test/definitions/navigation.test.ts‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
// Licensed under the MIT License.
2+
3+
'use strict';
4+
import * as assert from 'assert';
5+
import * as path from 'path';
6+
import * as vscode from 'vscode';
7+
import { closeActiveWindows, initialize, initializeTest } from '../initialize';
8+
9+
const decoratorsPath = path.join(__dirname, '..', '..', '..', 'src', 'test', 'pythonFiles', 'definition', 'navigation');
10+
const fileDefinitions = path.join(decoratorsPath, 'definitions.py');
11+
const fileUsages = path.join(decoratorsPath, 'usages.py');
12+
13+
// tslint:disable-next-line:max-func-body-length
14+
suite('Definition Navigation', () => {
15+
suiteSetup(initialize);
16+
setup(initializeTest);
17+
suiteTeardown(closeActiveWindows);
18+
teardown(closeActiveWindows);
19+
20+
const assertFile = (expectedLocation: string, location: vscode.Uri) => {
21+
const relLocation = vscode.workspace.asRelativePath(location);
22+
const expectedRelLocation = vscode.workspace.asRelativePath(expectedLocation);
23+
assert.equal(expectedRelLocation, relLocation, 'Position is in wrong file');
24+
};
25+
26+
const formatPosition = (position: vscode.Position) => {
27+
return `${position.line},${position.character}`;
28+
};
29+
30+
const assertRange = (expectedRange: vscode.Range, range: vscode.Range) => {
31+
assert.equal(formatPosition(expectedRange.start), formatPosition(range.start), 'Start position is incorrect');
32+
assert.equal(formatPosition(expectedRange.end), formatPosition(range.end), 'End position is incorrect');
33+
};
34+
35+
const buildTest = (startFile: string, startPosition: vscode.Position, expectedFile: string, expectedRange: vscode.Range) => {
36+
return async () => {
37+
const textDocument = await vscode.workspace.openTextDocument(startFile);
38+
await vscode.window.showTextDocument(textDocument);
39+
assert(vscode.window.activeTextEditor, 'No active editor');
40+
41+
const locations = await vscode.commands.executeCommand<vscode.Location[]>('vscode.executeDefinitionProvider', textDocument.uri, startPosition);
42+
assert.equal(1, locations!.length, 'Wrong number of results');
43+
44+
const def = locations![0];
45+
assertFile(expectedFile, def.uri);
46+
assertRange(expectedRange, def.range!);
47+
};
48+
};
49+
50+
test('From own definition', buildTest(
51+
fileDefinitions,
52+
new vscode.Position(2, 6),
53+
fileDefinitions,
54+
new vscode.Range(2, 0, 11, 17)
55+
));
56+
57+
test('Nested function', buildTest(
58+
fileDefinitions,
59+
new vscode.Position(11, 16),
60+
fileDefinitions,
61+
new vscode.Range(6, 4, 10, 16)
62+
));
63+
64+
test('Decorator usage', buildTest(
65+
fileDefinitions,
66+
new vscode.Position(13, 1),
67+
fileDefinitions,
68+
new vscode.Range(2, 0, 11, 17)
69+
));
70+
71+
test('Function decorated by stdlib', buildTest(
72+
fileDefinitions,
73+
new vscode.Position(29, 6),
74+
fileDefinitions,
75+
new vscode.Range(21, 0, 27, 17)
76+
));
77+
78+
test('Function decorated by local decorator', buildTest(
79+
fileDefinitions,
80+
new vscode.Position(30, 6),
81+
fileDefinitions,
82+
new vscode.Range(14, 0, 18, 7)
83+
));
84+
85+
test('Module imported decorator usage', buildTest(
86+
fileUsages,
87+
new vscode.Position(3, 15),
88+
fileDefinitions,
89+
new vscode.Range(2, 0, 11, 17)
90+
));
91+
92+
test('Module imported function decorated by stdlib', buildTest(
93+
fileUsages,
94+
new vscode.Position(11, 19),
95+
fileDefinitions,
96+
new vscode.Range(21, 0, 27, 17)
97+
));
98+
99+
test('Module imported function decorated by local decorator', buildTest(
100+
fileUsages,
101+
new vscode.Position(12, 19),
102+
fileDefinitions,
103+
new vscode.Range(14, 0, 18, 7)
104+
));
105+
106+
test('Specifically imported decorator usage', buildTest(
107+
fileUsages,
108+
new vscode.Position(7, 1),
109+
fileDefinitions,
110+
new vscode.Range(2, 0, 11, 17)
111+
));
112+
113+
test('Specifically imported function decorated by stdlib', buildTest(
114+
fileUsages,
115+
new vscode.Position(14, 6),
116+
fileDefinitions,
117+
new vscode.Range(21, 0, 27, 17)
118+
));
119+
120+
test('Specifically imported function decorated by local decorator', buildTest(
121+
fileUsages,
122+
new vscode.Position(15, 6),
123+
fileDefinitions,
124+
new vscode.Range(14, 0, 18, 7)
125+
));
126+
});

‎src/test/pythonFiles/definition/navigation/__init__.py‎

Whitespace-only changes.

‎src/test/pythonFiles/definition/navigation/definitions.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
from contextlib import contextmanager
2+
3+
def my_decorator(fn):
4+
"""
5+
This is my decorator.
6+
"""
7+
def wrapper(*args, **kwargs):
8+
"""
9+
This is the wrapper.
10+
"""
11+
return 42
12+
return wrapper
13+
14+
@my_decorator
15+
def thing(arg):
16+
"""
17+
Thing which is decorated.
18+
"""
19+
pass
20+
21+
@contextmanager
22+
def my_context_manager():
23+
"""
24+
This is my context manager.
25+
"""
26+
print("before")
27+
yield
28+
print("after")
29+
30+
with my_context_manager():
31+
thing(19)

‎src/test/pythonFiles/definition/navigation/usages.py‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import definitions
2+
from .definitions import my_context_manager, my_decorator, thing
3+
4+
@definitions.my_decorator
5+
def one():
6+
pass
7+
8+
@my_decorator
9+
def two():
10+
pass
11+
12+
with definitions.my_context_manager():
13+
definitions.thing(19)
14+
15+
with my_context_manager():
16+
thing(19)

0 commit comments

Comments
 (0)