Fix variable type from method return value, add tests. Fixes felixfbe…

…cker#386
roblourens · Jun 10, 2017 · 99908b5 · 99908b5
1 parent 42d0c7b
commit 99908b5
Show file tree

Hide file tree

Showing 52 changed files with 262 additions and 59 deletions.
diff --git a/fixtures/completion/method_return_type.php b/fixtures/completion/method_return_type.php
@@ -0,0 +1,11 @@
+<?php
+
+class FooClass {
+    public function foo(): FooClass {
+        return $this;
+    }
+}
+
+$fc = new FooClass();
+$foo = $fc->foo();
+$foo->
diff --git a/fixtures/completion/static_method_return_type.php b/fixtures/completion/static_method_return_type.php
@@ -0,0 +1,12 @@
+<?php
+
+class FooClass {
+    public static function staticFoo(): FooClass {
+        return new FooClass();
+    }
+
+    public function bar() { }
+}
+
+$foo = FooClass::staticFoo();
+$foo->
diff --git a/src/DefinitionResolver.php b/src/DefinitionResolver.php
@@ -623,6 +623,16 @@ public function resolveExpressionNodeToType($expr)
             }
         }
 
+        // MEMBER CALL EXPRESSION
+        //   The type of the member call expression is the type of the method, so resolve the
+        //   type of the member access 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) {

diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php
@@ -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) {

diff --git a/tests/Validation/ValidationTest.php b/tests/Validation/ValidationTest.php
@@ -137,8 +137,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;

diff --git a/tests/Validation/cases/WithReturnTypehints.php.expected.json b/tests/Validation/cases/WithReturnTypehints.php.expected.json
@@ -63,7 +63,7 @@
                 },
                 "containerName": "Fixtures\\Prophecy\\WithReturnTypehints"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Object_",
+            "type__tostring": "\\self",
             "type": {},
             "declarationLine": "public function getSelf(): self {",
             "documentation": null
@@ -82,7 +82,7 @@
                 },
                 "containerName": "Fixtures\\Prophecy\\WithReturnTypehints"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\String_",
+            "type__tostring": "string",
             "type": {},
             "declarationLine": "public function getName(): string {",
             "documentation": null
@@ -101,7 +101,7 @@
                 },
                 "containerName": "Fixtures\\Prophecy\\WithReturnTypehints"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Object_",
+            "type__tostring": "\\parent",
             "type": {},
             "declarationLine": "public function getParent(): parent {",
             "documentation": null

diff --git a/tests/Validation/cases/arrayValueShouldBeBoolean.php.expected.json b/tests/Validation/cases/arrayValueShouldBeBoolean.php.expected.json
@@ -33,7 +33,7 @@
                 },
                 "containerName": "A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Array_",
+            "type__tostring": "string[]",
             "type": {},
             "declarationLine": "protected $foo;",
             "documentation": null

diff --git a/tests/Validation/cases/classDefinition1.php.expected.json b/tests/Validation/cases/classDefinition1.php.expected.json
@@ -58,7 +58,7 @@
                 },
                 "containerName": "TestNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Integer",
+            "type__tostring": "int",
             "type": {},
             "declarationLine": "public $a;",
             "documentation": null

diff --git a/tests/Validation/cases/classProperty1.php.expected.json b/tests/Validation/cases/classProperty1.php.expected.json
@@ -58,7 +58,7 @@
                 },
                 "containerName": "TestNamespace\\TestClass"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "public $testProperty;",
             "documentation": null
@@ -77,7 +77,7 @@
                 },
                 "containerName": "TestNamespace\\TestClass"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "public function testMethod($testParameter)",
             "documentation": null

diff --git a/tests/Validation/cases/constants.php.expected.json b/tests/Validation/cases/constants.php.expected.json
@@ -58,7 +58,7 @@
                 },
                 "containerName": "MyNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "public static function suite()",
             "documentation": null

diff --git a/tests/Validation/cases/constants2.php.expected.json b/tests/Validation/cases/constants2.php.expected.json
@@ -58,7 +58,7 @@
                 },
                 "containerName": "MyNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "public static function suite()",
             "documentation": null

diff --git a/tests/Validation/cases/constants3.php.expected.json b/tests/Validation/cases/constants3.php.expected.json
@@ -58,7 +58,7 @@
                 },
                 "containerName": "MyNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "public static function suite()",
             "documentation": null

diff --git a/tests/Validation/cases/constants4.php.expected.json b/tests/Validation/cases/constants4.php.expected.json
@@ -58,7 +58,7 @@
                 },
                 "containerName": "MyNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "public function suite()",
             "documentation": null

diff --git a/tests/Validation/cases/constants5.php.expected.json b/tests/Validation/cases/constants5.php.expected.json
@@ -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

diff --git a/tests/Validation/cases/constantsInFunctionParamDefault.php.expected.json b/tests/Validation/cases/constantsInFunctionParamDefault.php.expected.json
@@ -37,7 +37,7 @@
                 },
                 "containerName": "A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "function b ($a = MY_CONSTANT);",
             "documentation": null

diff --git a/tests/Validation/cases/magicConsts.php.expected.json b/tests/Validation/cases/magicConsts.php.expected.json
@@ -33,7 +33,7 @@
                 },
                 "containerName": "A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Array_",
+            "type__tostring": "\\__CLASS__[]",
             "type": {},
             "declarationLine": "private static $deprecationsTriggered;",
             "documentation": null

diff --git a/tests/Validation/cases/memberAccess1.php.expected.json b/tests/Validation/cases/memberAccess1.php.expected.json
@@ -58,7 +58,7 @@
                 },
                 "containerName": "MyNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "static function a() {",
             "documentation": null

diff --git a/tests/Validation/cases/memberAccess2.php.expected.json b/tests/Validation/cases/memberAccess2.php.expected.json
@@ -58,7 +58,7 @@
                 },
                 "containerName": "MyNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "static function a() {",
             "documentation": null

diff --git a/tests/Validation/cases/memberAccess3.php.expected.json b/tests/Validation/cases/memberAccess3.php.expected.json
@@ -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

diff --git a/tests/Validation/cases/memberAccess4.php.expected.json b/tests/Validation/cases/memberAccess4.php.expected.json
@@ -64,7 +64,7 @@
                 },
                 "containerName": "MyNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "public function testRequest()",
             "documentation": null

diff --git a/tests/Validation/cases/memberAccess5.php.expected.json b/tests/Validation/cases/memberAccess5.php.expected.json
@@ -51,7 +51,7 @@
                 },
                 "containerName": "MyNamespace\\ParseErrorsTest"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "public function setUp()",
             "documentation": null

diff --git a/tests/Validation/cases/memberCall1.php.expected.json b/tests/Validation/cases/memberCall1.php.expected.json
@@ -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

diff --git a/tests/Validation/cases/methodReturnType.php b/tests/Validation/cases/methodReturnType.php
@@ -0,0 +1,7 @@
+<?php
+
+class FooClass {
+    public function foo(): FooClass {
+        return $this;
+    }
+}
diff --git a/tests/Validation/cases/methodReturnType.php.expected.json b/tests/Validation/cases/methodReturnType.php.expected.json
@@ -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
+        }
+    }
+}
diff --git a/tests/Validation/cases/multipleNamespaces.php.expected.json b/tests/Validation/cases/multipleNamespaces.php.expected.json
@@ -64,7 +64,7 @@
                 },
                 "containerName": "MyNamespace1\\B"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "function b() {",
             "documentation": null
@@ -121,7 +121,7 @@
                 },
                 "containerName": "MyNamespace2\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "function a () {",
             "documentation": null

diff --git a/tests/Validation/cases/multiplePreceedingComments.php.expected.json b/tests/Validation/cases/multiplePreceedingComments.php.expected.json
@@ -33,7 +33,7 @@
                 },
                 "containerName": "Foo"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Object_",
+            "type__tostring": "\\Iterator",
             "type": {},
             "declarationLine": "public function fn()",
             "documentation": "Foo"

diff --git a/tests/Validation/cases/nameToken.php.expected.json b/tests/Validation/cases/nameToken.php.expected.json
@@ -33,7 +33,7 @@
                 },
                 "containerName": "A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "function b() {",
             "documentation": null

diff --git a/tests/Validation/cases/objectCreation.php.expected.json b/tests/Validation/cases/objectCreation.php.expected.json
@@ -55,7 +55,7 @@
                 },
                 "containerName": "MyNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "function a () {",
             "documentation": null

diff --git a/tests/Validation/cases/objectCreation2.php.expected.json b/tests/Validation/cases/objectCreation2.php.expected.json
@@ -76,7 +76,7 @@
                 },
                 "containerName": "MyNamespace\\A"
             },
-            "type__class": "phpDocumentor\\Reflection\\Types\\Mixed",
+            "type__tostring": "mixed",
             "type": {},
             "declarationLine": "function a () {",
             "documentation": null