Skip to content

Commit 80ef8ff

Browse files
felixfbeckerfelixfbecker
authored
fix(indexing): properly resolve self, static and parent keywords (#532)
Previously we would dump static, self and parent as literal FQNs into the index.
1 parent b1a1875 commit 80ef8ff

21 files changed

+173
-109
lines changed

‎.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ install:
2323
- composer install --prefer-dist --no-interaction
2424
script:
2525
- vendor/bin/phpcs -n
26-
- vendor/bin/phpunit --coverage-clover=coverage.xml
26+
- vendor/bin/phpunit --coverage-clover=coverage.xml --colors=always
2727
- bash <(curl -s https://codecov.io/bash)
2828

2929
jobs:

‎.vscode/launch.json

Lines changed: 25 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,27 @@
11
{
2-
"version": "0.2.0",
3-
"configurations": [
4-
{
5-
"name": "Listen for XDebug",
6-
"type": "php",
7-
"request": "launch",
8-
"port": 9000
9-
},
10-
{
11-
"name": "Launch currently open script",
12-
"type": "php",
13-
"request": "launch",
14-
"program": "${file}",
15-
"cwd": "${fileDirname}",
16-
"port": 9000
17-
},
18-
{
19-
"name": "PHPUnit",
20-
"type": "php",
21-
"request": "launch",
22-
"program": "${workspaceRoot}/vendor/phpunit/phpunit/phpunit",
23-
"cwd": "${workspaceRoot}",
24-
"args": [
25-
// "--filter", "CompletionTest"
26-
]
27-
}
28-
]
2+
"version": "0.2.0",
3+
"configurations": [
4+
{
5+
"name": "PHPUnit",
6+
"type": "php",
7+
"request": "launch",
8+
"program": "${workspaceRoot}/vendor/phpunit/phpunit/phpunit",
9+
// "args": ["--filter", "testDefinitionForSelfKeyword"],
10+
"cwd": "${workspaceRoot}"
11+
},
12+
{
13+
"name": "Listen for XDebug",
14+
"type": "php",
15+
"request": "launch",
16+
"port": 9000
17+
},
18+
{
19+
"name": "Launch currently open script",
20+
"type": "php",
21+
"request": "launch",
22+
"program": "${file}",
23+
"cwd": "${fileDirname}",
24+
"port": 9000
25+
}
26+
]
2927
}

‎src/DefinitionResolver.php

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -264,20 +264,46 @@ public function resolveReferenceNodeToDefinition(Node $node)
264264
// Other references are references to a global symbol that have an FQN
265265
// Find out the FQN
266266
$fqn = $this->resolveReferenceNodeToFqn($node);
267-
if ($fqn === null) {
267+
if (!$fqn) {
268268
return null;
269269
}
270+
271+
if ($fqn === 'self' || $fqn === 'static') {
272+
// Resolve self and static keywords to the containing class
273+
// (This is not 100% correct for static but better than nothing)
274+
$classNode = $node->getFirstAncestor(Node\Statement\ClassDeclaration::class);
275+
if (!$classNode) {
276+
return;
277+
}
278+
$fqn = (string)$classNode->getNamespacedName();
279+
if (!$fqn) {
280+
return;
281+
}
282+
} else if ($fqn === 'parent') {
283+
// Resolve parent keyword to the base class FQN
284+
$classNode = $node->getFirstAncestor(Node\Statement\ClassDeclaration::class);
285+
if (!$classNode || !$classNode->classBaseClause || !$classNode->classBaseClause->baseClass) {
286+
return;
287+
}
288+
$fqn = (string)$classNode->classBaseClause->baseClass->getResolvedName();
289+
if (!$fqn) {
290+
return;
291+
}
292+
}
293+
270294
// If the node is a function or constant, it could be namespaced, but PHP falls back to global
271295
// http://php.net/manual/en/language.namespaces.fallback.php
272296
// TODO - verify that this is not a method
273297
$globalFallback = ParserHelpers\isConstantFetch($node) || $parent instanceof Node\Expression\CallExpression;
298+
274299
// Return the Definition object from the index index
275300
return $this->index->getDefinition($fqn, $globalFallback);
276301
}
277302

278303
/**
279304
* Given any node, returns the FQN of the symbol that is referenced
280305
* Returns null if the FQN could not be resolved or the reference node references a variable
306+
* May also return "static", "self" or "parent"
281307
*
282308
* @param Node $node
283309
* @return string|null

‎src/TreeAnalyzer.php

Lines changed: 62 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,9 @@ private function collectDefinitionsAndReferences(Node $node)
140140
$this->definitionNodes[$fqn] = $node;
141141
$this->definitions[$fqn] = $this->definitionResolver->createDefinitionFromNode($node, $fqn);
142142
} else {
143+
143144
$parent = $node->parent;
144-
if (!(
145+
if (
145146
(
146147
// $node->parent instanceof Node\Expression\ScopedPropertyAccessExpression ||
147148
($node instanceof Node\Expression\ScopedPropertyAccessExpression ||
@@ -150,41 +151,68 @@ private function collectDefinitionsAndReferences(Node $node)
150151
$node->parent instanceof Node\Expression\CallExpression ||
151152
$node->memberName instanceof PhpParser\Token
152153
))
153-
|| ($parent instanceof Node\Statement\NamespaceDefinition && $parent->name !== null && $parent->name->getStart() === $node->getStart()))
154+
|| ($parent instanceof Node\Statement\NamespaceDefinition && $parent->name !== null && $parent->name->getStart() === $node->getStart())
154155
) {
155-
$fqn = $this->definitionResolver->resolveReferenceNodeToFqn($node);
156-
if ($fqn !== null) {
157-
$this->addReference($fqn, $node);
158-
159-
if (
160-
$node instanceof Node\QualifiedName
161-
&& ($node->isQualifiedName() || $node->parent instanceof Node\NamespaceUseClause)
162-
&& !($parent instanceof Node\Statement\NamespaceDefinition && $parent->name->getStart() === $node->getStart()
163-
)
164-
) {
165-
// Add references for each referenced namespace
166-
$ns = $fqn;
167-
while (($pos = strrpos($ns, '\\')) !== false) {
168-
$ns = substr($ns, 0, $pos);
169-
$this->addReference($ns, $node);
170-
}
171-
}
156+
return;
157+
}
172158

173-
// Namespaced constant access and function calls also need to register a reference
174-
// to the global version because PHP falls back to global at runtime
175-
// http://php.net/manual/en/language.namespaces.fallback.php
176-
if (ParserHelpers\isConstantFetch($node) ||
177-
($parent instanceof Node\Expression\CallExpression
178-
&& !(
179-
$node instanceof Node\Expression\ScopedPropertyAccessExpression ||
180-
$node instanceof Node\Expression\MemberAccessExpression
181-
))) {
182-
$parts = explode('\\', $fqn);
183-
if (count($parts) > 1) {
184-
$globalFqn = end($parts);
185-
$this->addReference($globalFqn, $node);
186-
}
187-
}
159+
$fqn = $this->definitionResolver->resolveReferenceNodeToFqn($node);
160+
if (!$fqn) {
161+
return;
162+
}
163+
164+
if ($fqn === 'self' || $fqn === 'static') {
165+
// Resolve self and static keywords to the containing class
166+
// (This is not 100% correct for static but better than nothing)
167+
$classNode = $node->getFirstAncestor(Node\Statement\ClassDeclaration::class);
168+
if (!$classNode) {
169+
return;
170+
}
171+
$fqn = (string)$classNode->getNamespacedName();
172+
if (!$fqn) {
173+
return;
174+
}
175+
} else if ($fqn === 'parent') {
176+
// Resolve parent keyword to the base class FQN
177+
$classNode = $node->getFirstAncestor(Node\Statement\ClassDeclaration::class);
178+
if (!$classNode || !$classNode->classBaseClause || !$classNode->classBaseClause->baseClass) {
179+
return;
180+
}
181+
$fqn = (string)$classNode->classBaseClause->baseClass->getResolvedName();
182+
if (!$fqn) {
183+
return;
184+
}
185+
}
186+
187+
$this->addReference($fqn, $node);
188+
189+
if (
190+
$node instanceof Node\QualifiedName
191+
&& ($node->isQualifiedName() || $node->parent instanceof Node\NamespaceUseClause)
192+
&& !($parent instanceof Node\Statement\NamespaceDefinition && $parent->name->getStart() === $node->getStart()
193+
)
194+
) {
195+
// Add references for each referenced namespace
196+
$ns = $fqn;
197+
while (($pos = strrpos($ns, '\\')) !== false) {
198+
$ns = substr($ns, 0, $pos);
199+
$this->addReference($ns, $node);
200+
}
201+
}
202+
203+
// Namespaced constant access and function calls also need to register a reference
204+
// to the global version because PHP falls back to global at runtime
205+
// http://php.net/manual/en/language.namespaces.fallback.php
206+
if (ParserHelpers\isConstantFetch($node) ||
207+
($parent instanceof Node\Expression\CallExpression
208+
&& !(
209+
$node instanceof Node\Expression\ScopedPropertyAccessExpression ||
210+
$node instanceof Node\Expression\MemberAccessExpression
211+
))) {
212+
$parts = explode('\\', $fqn);
213+
if (count($parts) > 1) {
214+
$globalFqn = end($parts);
215+
$this->addReference($globalFqn, $node);
188216
}
189217
}
190218
}

‎tests/Server/ServerTestCase.php

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -122,15 +122,16 @@ public function setUp()
122122
0 => new Location($referencesUri, new Range(new Position(29, 5), new Position(29, 15)))
123123
],
124124
'TestNamespace\\TestClass' => [
125-
0 => new Location($symbolsUri , new Range(new Position(99, 25), new Position(99, 34))), // class ChildClass extends TestClass {}
126-
1 => new Location($referencesUri, new Range(new Position( 4, 11), new Position( 4, 20))), // $obj = new TestClass();
127-
2 => new Location($referencesUri, new Range(new Position( 7, 0), new Position( 7, 9))), // TestClass::staticTestMethod();
128-
3 => new Location($referencesUri, new Range(new Position( 8, 5), new Position( 8, 14))), // echo TestClass::$staticTestProperty;
129-
4 => new Location($referencesUri, new Range(new Position( 9, 5), new Position( 9, 14))), // TestClass::TEST_CLASS_CONST;
130-
5 => new Location($referencesUri, new Range(new Position(21, 18), new Position(21, 27))), // function whatever(TestClass $param)
131-
6 => new Location($referencesUri, new Range(new Position(21, 37), new Position(21, 46))), // function whatever(TestClass $param): TestClass
132-
7 => new Location($referencesUri, new Range(new Position(39, 0), new Position(39, 9))), // TestClass::$staticTestProperty[123]->testProperty;
133-
8 => new Location($useUri, new Range(new Position( 4, 4), new Position( 4, 27))), // use TestNamespace\TestClass;
125+
0 => new Location($symbolsUri, new Range(new Position(48, 13), new Position(48, 17))), // echo self::TEST_CLASS_CONST;
126+
1 => new Location($symbolsUri , new Range(new Position(99, 25), new Position(99, 34))), // class ChildClass extends TestClass {}
127+
2 => new Location($referencesUri, new Range(new Position( 4, 11), new Position( 4, 20))), // $obj = new TestClass();
128+
3 => new Location($referencesUri, new Range(new Position( 7, 0), new Position( 7, 9))), // TestClass::staticTestMethod();
129+
4 => new Location($referencesUri, new Range(new Position( 8, 5), new Position( 8, 14))), // echo TestClass::$staticTestProperty;
130+
5 => new Location($referencesUri, new Range(new Position( 9, 5), new Position( 9, 14))), // TestClass::TEST_CLASS_CONST;
131+
6 => new Location($referencesUri, new Range(new Position(21, 18), new Position(21, 27))), // function whatever(TestClass $param)
132+
7 => new Location($referencesUri, new Range(new Position(21, 37), new Position(21, 46))), // function whatever(TestClass $param): TestClass
133+
8 => new Location($referencesUri, new Range(new Position(39, 0), new Position(39, 9))), // TestClass::$staticTestProperty[123]->testProperty;
134+
9 => new Location($useUri, new Range(new Position( 4, 4), new Position( 4, 27))), // use TestNamespace\TestClass;
134135
],
135136
'TestNamespace\\TestChild' => [
136137
0 => new Location($referencesUri, new Range(new Position(42, 5), new Position(42, 25))), // echo $child->testProperty;
@@ -176,14 +177,15 @@ public function setUp()
176177
1 => new Location($globalReferencesUri, new Range(new Position(29, 5), new Position(29, 15)))
177178
],
178179
'TestClass' => [
179-
0 => new Location($globalSymbolsUri, new Range(new Position(99, 25), new Position(99, 34))), // class ChildClass extends TestClass {}
180-
1 => new Location($globalReferencesUri, new Range(new Position( 4, 11), new Position( 4, 20))), // $obj = new TestClass();
181-
2 => new Location($globalReferencesUri, new Range(new Position( 7, 0), new Position( 7, 9))), // TestClass::staticTestMethod();
182-
3 => new Location($globalReferencesUri, new Range(new Position( 8, 5), new Position( 8, 14))), // echo TestClass::$staticTestProperty;
183-
4 => new Location($globalReferencesUri, new Range(new Position( 9, 5), new Position( 9, 14))), // TestClass::TEST_CLASS_CONST;
184-
5 => new Location($globalReferencesUri, new Range(new Position(21, 18), new Position(21, 27))), // function whatever(TestClass $param)
185-
6 => new Location($globalReferencesUri, new Range(new Position(21, 37), new Position(21, 46))), // function whatever(TestClass $param): TestClass
186-
7 => new Location($globalReferencesUri, new Range(new Position(39, 0), new Position(39, 9))), // TestClass::$staticTestProperty[123]->testProperty;
180+
0 => new Location($globalSymbolsUri, new Range(new Position(48, 13), new Position(48, 17))), // echo self::TEST_CLASS_CONST;
181+
1 => new Location($globalSymbolsUri, new Range(new Position(99, 25), new Position(99, 34))), // class ChildClass extends TestClass {}
182+
2 => new Location($globalReferencesUri, new Range(new Position( 4, 11), new Position( 4, 20))), // $obj = new TestClass();
183+
3 => new Location($globalReferencesUri, new Range(new Position( 7, 0), new Position( 7, 9))), // TestClass::staticTestMethod();
184+
4 => new Location($globalReferencesUri, new Range(new Position( 8, 5), new Position( 8, 14))), // echo TestClass::$staticTestProperty;
185+
5 => new Location($globalReferencesUri, new Range(new Position( 9, 5), new Position( 9, 14))), // TestClass::TEST_CLASS_CONST;
186+
6 => new Location($globalReferencesUri, new Range(new Position(21, 18), new Position(21, 27))), // function whatever(TestClass $param)
187+
7 => new Location($globalReferencesUri, new Range(new Position(21, 37), new Position(21, 46))), // function whatever(TestClass $param): TestClass
188+
8 => new Location($globalReferencesUri, new Range(new Position(39, 0), new Position(39, 9))), // TestClass::$staticTestProperty[123]->testProperty;
187189
],
188190
'TestChild' => [
189191
0 => new Location($globalReferencesUri, new Range(new Position(42, 5), new Position(42, 25))), // echo $child->testProperty;

‎tests/Server/TextDocument/Definition/GlobalTest.php

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,23 @@ public function testDefinitionEmptyResult()
2929
$this->assertEquals([], $result);
3030
}
3131

32+
public function testDefinitionForSelfKeyword()
33+
{
34+
// echo self::TEST_CLASS_CONST;
35+
// Get definition for self
36+
$reference = $this->getReferenceLocations('TestClass')[0];
37+
$result = $this->textDocument->definition(
38+
new TextDocumentIdentifier($reference->uri),
39+
$reference->range->start
40+
)->wait();
41+
$this->assertEquals($this->getDefinitionLocation('TestClass'), $result);
42+
}
43+
3244
public function testDefinitionForClassLike()
3345
{
3446
// $obj = new TestClass();
3547
// Get definition for TestClass
36-
$reference = $this->getReferenceLocations('TestClass')[0];
48+
$reference = $this->getReferenceLocations('TestClass')[1];
3749
$result = $this->textDocument->definition(
3850
new TextDocumentIdentifier($reference->uri),
3951
$reference->range->start
@@ -45,7 +57,7 @@ public function testDefinitionForClassOnStaticMethodCall()
4557
{
4658
// TestClass::staticTestMethod();
4759
// Get definition for TestClass
48-
$reference = $this->getReferenceLocations('TestClass')[1];
60+
$reference = $this->getReferenceLocations('TestClass')[2];
4961
$result = $this->textDocument->definition(
5062
new TextDocumentIdentifier($reference->uri),
5163
$reference->range->start
@@ -57,7 +69,7 @@ public function testDefinitionForClassOnStaticPropertyFetch()
5769
{
5870
// echo TestClass::$staticTestProperty;
5971
// Get definition for TestClass
60-
$reference = $this->getReferenceLocations('TestClass')[2];
72+
$reference = $this->getReferenceLocations('TestClass')[3];
6173
$result = $this->textDocument->definition(
6274
new TextDocumentIdentifier($reference->uri),
6375
$reference->range->start
@@ -69,7 +81,7 @@ public function testDefinitionForClassOnConstFetch()
6981
{
7082
// TestClass::TEST_CLASS_CONST;
7183
// Get definition for TestClass
72-
$reference = $this->getReferenceLocations('TestClass')[3];
84+
$reference = $this->getReferenceLocations('TestClass')[4];
7385
$result = $this->textDocument->definition(
7486
new TextDocumentIdentifier($reference->uri),
7587
$reference->range->start
@@ -213,7 +225,7 @@ public function testDefinitionForParamTypeHints()
213225
{
214226
// function whatever(TestClass $param) {
215227
// Get definition for TestClass
216-
$reference = $this->getReferenceLocations('TestClass')[4];
228+
$reference = $this->getReferenceLocations('TestClass')[5];
217229
$result = $this->textDocument->definition(
218230
new TextDocumentIdentifier($reference->uri),
219231
$reference->range->start
@@ -225,7 +237,7 @@ public function testDefinitionForReturnTypeHints()
225237
{
226238
// function whatever(TestClass $param): TestClass {
227239
// Get definition for TestClass
228-
$reference = $this->getReferenceLocations('TestClass')[5];
240+
$reference = $this->getReferenceLocations('TestClass')[6];
229241
$result = $this->textDocument->definition(
230242
new TextDocumentIdentifier($reference->uri),
231243
$reference->range->start

‎tests/Server/TextDocument/Definition/NamespacedTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public function testDefinitionForClassLikeUseStatement()
3434
{
3535
// use TestNamespace\TestClass;
3636
// Get definition for TestClass
37-
$reference = $this->getReferenceLocations('TestClass')[6];
37+
$reference = $this->getReferenceLocations('TestClass')[7];
3838
$result = $this->textDocument->definition(
3939
new TextDocumentIdentifier($reference->uri),
4040
$reference->range->start
@@ -46,7 +46,7 @@ public function testDefinitionForClassLikeGroupUseStatement()
4646
{
4747
// use TestNamespace\{TestTrait, TestInterface};
4848
// Get definition for TestInterface
49-
$reference = $this->getReferenceLocations('TestClass')[0];
49+
$reference = $this->getReferenceLocations('TestClass')[1];
5050
$result = $this->textDocument->definition(
5151
new TextDocumentIdentifier($reference->uri),
5252
$reference->range->start

‎tests/Server/TextDocument/HoverTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public function testHoverForClassLike()
1515
{
1616
// $obj = new TestClass();
1717
// Get hover for TestClass
18-
$reference = $this->getReferenceLocations('TestClass')[0];
18+
$reference = $this->getReferenceLocations('TestClass')[1];
1919
$result = $this->textDocument->hover(
2020
new TextDocumentIdentifier($reference->uri),
2121
$reference->range->start

0 commit comments

Comments
 (0)