Skip to content

Fix variable type from method return value, add tests #393

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.

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

Merged
merged 4 commits into from
Jun 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 11 additions & 0 deletions fixtures/completion/method_return_type.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

class FooClass {
public function foo(): FooClass {
return $this;
}
}

$fc = new FooClass();
$foo = $fc->foo();
$foo->
12 changes: 12 additions & 0 deletions fixtures/completion/static_method_return_type.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<?php

class FooClass {
public static function staticFoo(): FooClass {
return new FooClass();
}

public function bar() { }
}

$foo = FooClass::staticFoo();
$foo->
10 changes: 10 additions & 0 deletions src/DefinitionResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -623,6 +623,16 @@ public function resolveExpressionNodeToType($expr)
}
}

// MEMBER CALL EXPRESSION/SCOPED PROPERTY CALL EXPRESSION
// The type of the member/scoped property call expression is the type of the method, so resolve the
// type of the callable expression.
if ($expr instanceof Node\Expression\CallExpression && (
$expr->callableExpression instanceof Node\Expression\MemberAccessExpression ||
$expr->callableExpression instanceof Node\Expression\ScopedPropertyAccessExpression)
) {
return $this->resolveExpressionNodeToType($expr->callableExpression);
}

// MEMBER ACCESS EXPRESSION
if ($expr instanceof Node\Expression\MemberAccessExpression) {
if ($expr->memberName instanceof Node\Expression) {
Expand Down
4 changes: 2 additions & 2 deletions tests/LanguageServerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public function testIndexingWithDirectFileAccess()
if ($msg->body->method === 'window/logMessage' && $promise->state === Promise::PENDING) {
if ($msg->body->params->type === MessageType::ERROR) {
$promise->reject(new Exception($msg->body->params->message));
} else if (strpos($msg->body->params->message, 'All 27 PHP files parsed') !== false) {
} else if (preg_match('/All \d+ PHP files parsed/', $msg->body->params->message)) {
$promise->fulfill();
}
}
Expand Down Expand Up @@ -103,7 +103,7 @@ public function testIndexingWithFilesAndContentRequests()
if ($promise->state === Promise::PENDING) {
$promise->reject(new Exception($msg->body->params->message));
}
} else if (strpos($msg->body->params->message, 'All 27 PHP files parsed') !== false) {
} else if (preg_match('/All \d+ PHP files parsed/', $msg->body->params->message)) {
$promise->fulfill();
}
}
Expand Down
44 changes: 44 additions & 0 deletions tests/Server/TextDocument/CompletionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,50 @@ public function testBarePhp()
], true), $items);
}

public function testMethodReturnType()
{
$completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/method_return_type.php');
$this->loader->open($completionUri, file_get_contents($completionUri));
$items = $this->textDocument->completion(
new TextDocumentIdentifier($completionUri),
new Position(10, 6)
)->wait();
$this->assertCompletionsListSubset(new CompletionList([
new CompletionItem(
'foo',
CompletionItemKind::METHOD,
'\FooClass',
null,
null,
null,
null,
null
)
], true), $items);
}

public function testStaticMethodReturnType()
{
$completionUri = pathToUri(__DIR__ . '/../../../fixtures/completion/static_method_return_type.php');
$this->loader->open($completionUri, file_get_contents($completionUri));
$items = $this->textDocument->completion(
new TextDocumentIdentifier($completionUri),
new Position(11, 6)
)->wait();
$this->assertCompletionsListSubset(new CompletionList([
new CompletionItem(
'bar',
CompletionItemKind::METHOD,
'mixed',
null,
null,
null,
null,
null
)
], true), $items);
}

private function assertCompletionsListSubset(CompletionList $subsetList, CompletionList $list)
{
foreach ($subsetList->items as $expectedItem) {
Expand Down
3 changes: 1 addition & 2 deletions tests/Validation/ValidationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,7 @@ private function getTestValuesFromDefs($definitions): array
} elseif ($propertyName === 'extends') {
$definition->$propertyName = $definition->$propertyName ?? [];
} elseif ($propertyName === 'type' && $definition->type !== null) {
// Class info is not captured by json_encode. It's important for 'type'.
$defsForAssert[$fqn]['type__class'] = get_class($definition->type);
$defsForAssert[$fqn]['type__tostring'] = (string)$definition->type;
}

$defsForAssert[$fqn][$propertyName] = $definition->$propertyName;
Expand Down
6 changes: 3 additions & 3 deletions tests/Validation/cases/WithReturnTypehints.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
},
"containerName": "Fixtures\\Prophecy\\WithReturnTypehints"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Object_",
"type__tostring": "\\self",
"type": {},
"declarationLine": "public function getSelf(): self {",
"documentation": null
Expand All @@ -88,7 +88,7 @@
},
"containerName": "Fixtures\\Prophecy\\WithReturnTypehints"
},
"type__class": "phpDocumentor\\Reflection\\Types\\String_",
"type__tostring": "string",
"type": {},
"declarationLine": "public function getName(): string {",
"documentation": null
Expand All @@ -107,7 +107,7 @@
},
"containerName": "Fixtures\\Prophecy\\WithReturnTypehints"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Object_",
"type__tostring": "\\parent",
"type": {},
"declarationLine": "public function getParent(): parent {",
"documentation": null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
},
"containerName": "A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Array_",
"type__tostring": "string[]",
"type": {},
"declarationLine": "protected $foo;",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/classDefinition1.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
},
"containerName": "TestNamespace\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Integer",
"type__tostring": "int",
"type": {},
"declarationLine": "public $a;",
"documentation": null
Expand Down
4 changes: 2 additions & 2 deletions tests/Validation/cases/classProperty1.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
},
"containerName": "TestNamespace\\TestClass"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public $testProperty;",
"documentation": null
Expand All @@ -77,7 +77,7 @@
},
"containerName": "TestNamespace\\TestClass"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public function testMethod($testParameter)",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/constants.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
},
"containerName": "MyNamespace\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public static function suite()",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/constants2.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
},
"containerName": "MyNamespace\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public static function suite()",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/constants3.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
},
"containerName": "MyNamespace\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public static function suite()",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/constants4.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
},
"containerName": "MyNamespace\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public function suite()",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/constants5.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
},
"containerName": "MyNamespace\\Mbstring"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Object_",
"type__tostring": "\\MyNamespace\\PHP_INT_MAX",
"type": {},
"declarationLine": "const MB_CASE_FOLD = PHP_INT_MAX;",
"documentation": null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
},
"containerName": "A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "function b ($a = MY_CONSTANT);",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/magicConsts.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
},
"containerName": "A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Array_",
"type__tostring": "\\__CLASS__[]",
"type": {},
"declarationLine": "private static $deprecationsTriggered;",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/memberAccess1.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
},
"containerName": "MyNamespace\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "static function a() {",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/memberAccess2.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
},
"containerName": "MyNamespace\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "static function a() {",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/memberAccess3.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
},
"containerName": "MyNamespace\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public static function getInitializer(ClassLoader $loader)",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/memberAccess4.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
},
"containerName": "MyNamespace\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public function testRequest()",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/memberAccess5.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
},
"containerName": "MyNamespace\\ParseErrorsTest"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public function setUp()",
"documentation": null
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/memberCall1.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
},
"containerName": "MyNamespace\\ParseErrorsTest"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "public function setAccount(AccountInterface $account)",
"documentation": null
Expand Down
7 changes: 7 additions & 0 deletions tests/Validation/cases/methodReturnType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

class FooClass {
public function foo(): FooClass {
return $this;
}
}
46 changes: 46 additions & 0 deletions tests/Validation/cases/methodReturnType.php.expected.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
{
"references": {
"FooClass": [
"./methodReturnType.php"
]
},
"definitions": {
"FooClass": {
"fqn": "FooClass",
"extends": [],
"isGlobal": true,
"isStatic": false,
"canBeInstantiated": true,
"symbolInformation": {
"name": "FooClass",
"kind": 5,
"location": {
"uri": "./methodReturnType.php"
},
"containerName": ""
},
"type": null,
"declarationLine": "class FooClass {",
"documentation": null
},
"FooClass->foo()": {
"fqn": "FooClass->foo()",
"extends": [],
"isGlobal": false,
"isStatic": false,
"canBeInstantiated": false,
"symbolInformation": {
"name": "foo",
"kind": 6,
"location": {
"uri": "./methodReturnType.php"
},
"containerName": "FooClass"
},
"type__tostring": "\\FooClass",
"type": {},
"declarationLine": "public function foo(): FooClass {",
"documentation": null
}
}
}
4 changes: 2 additions & 2 deletions tests/Validation/cases/multipleNamespaces.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@
},
"containerName": "MyNamespace1\\B"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "function b() {",
"documentation": null
Expand Down Expand Up @@ -121,7 +121,7 @@
},
"containerName": "MyNamespace2\\A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "function a () {",
"documentation": null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
},
"containerName": "Foo"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Object_",
"type__tostring": "\\Iterator",
"type": {},
"declarationLine": "public function fn()",
"documentation": "Foo"
Expand Down
2 changes: 1 addition & 1 deletion tests/Validation/cases/nameToken.php.expected.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
},
"containerName": "A"
},
"type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
"type__tostring": "mixed",
"type": {},
"declarationLine": "function b() {",
"documentation": null
Expand Down
Loading