From 5835bd314b7f2eae775501d9545681e148b63c54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Fri, 9 Nov 2018 21:35:05 +0100 Subject: [PATCH 01/13] [PR] Intellisense autocomplete also suggests private or protected method and properties Checking the visibility and the context from where the node is: inside a method declaration or after an instantiation --- src/CompletionProvider.php | 6 ++++-- src/Definition.php | 29 +++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 44cf5b8e..2dba3735 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -246,8 +246,10 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // Collect all definitions that match any of the prefixes foreach ($this->index->getDefinitions() as $fqn => $def) { foreach ($prefixes as $prefix) { - if (substr($fqn, 0, strlen($prefix)) === $prefix && $def->isMember) { - $list->items[] = CompletionItemFactory::fromDefinition($def); + if (substr($fqn, 0, strlen($prefix)) === $prefix && + $def->isMember && + $def->isVisible($prefix, $prefixes[0], $node)) { + $list->items[] = CompletionItem::fromDefinition($def); } } } diff --git a/src/Definition.php b/src/Definition.php index 9ea27f9c..4b6239db 100644 --- a/src/Definition.php +++ b/src/Definition.php @@ -133,4 +133,33 @@ public function getAncestorDefinitions(ReadableIndex $index, bool $includeSelf = } } } + + /** + * Checks the definition's visibility. + * @return bool + */ + public function isVisible(string $match, string $caller, \Microsoft\PhpParser\Node $node): bool + { + $ancestor = $node->getFirstAncestor(\Microsoft\PhpParser\Node\MethodDeclaration::class); + + if ($ancestor) { + if ($match !== $caller && $this->isPrivate()) { + return false; + } + } else if ($this->isProtected() || $this->isPrivate()) { + return false; + } + + return true; + } + + private function isPrivate(): bool + { + return 'private' === substr($this->declarationLine, 0, 7); + } + + private function isProtected(): bool + { + return 'protected' === substr($this->declarationLine, 0, 9); + } } From a54fcf9d14f1584e553c49e3742ce56e8cea0d68 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sat, 10 Nov 2018 13:14:51 +0100 Subject: [PATCH 02/13] Adding netbeans config to gitignore --- .gitignore | 1 + src/CompletionProvider.php | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitignore b/.gitignore index 6476dbe1..a7c04f14 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ composer.lock stubs *.ast node_modules/ +/nbproject/ \ No newline at end of file diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 2dba3735..fa6d54f3 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -236,7 +236,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon $fqns = FqnUtilities\getFqnsFromType( $this->definitionResolver->resolveExpressionNodeToType($node->dereferencableExpression) ); - + $start = microtime(true); // Add the object access operator to only get members of all parents $prefixes = []; foreach ($this->expandParentFqns($fqns) as $prefix) { @@ -253,7 +253,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon } } } - + echo microtime(true) - $start; } elseif ( ($scoped = $node->parent) instanceof Node\Expression\ScopedPropertyAccessExpression || ($scoped = $node) instanceof Node\Expression\ScopedPropertyAccessExpression From d1bdcbd173ac2dd83b9429a4cd5ef82f7b0d255a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sat, 10 Nov 2018 13:40:55 +0100 Subject: [PATCH 03/13] Resolving node ancestor in the CompletionProvider, so it's resolved outside the loop --- src/CompletionProvider.php | 5 +++-- src/Definition.php | 12 +++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index fa6d54f3..4a87f1d6 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -237,6 +237,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon $this->definitionResolver->resolveExpressionNodeToType($node->dereferencableExpression) ); $start = microtime(true); + $isInMethodDeclaration = null !== $node->getFirstAncestor(\Microsoft\PhpParser\Node\MethodDeclaration::class); // Add the object access operator to only get members of all parents $prefixes = []; foreach ($this->expandParentFqns($fqns) as $prefix) { @@ -248,8 +249,8 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon foreach ($prefixes as $prefix) { if (substr($fqn, 0, strlen($prefix)) === $prefix && $def->isMember && - $def->isVisible($prefix, $prefixes[0], $node)) { - $list->items[] = CompletionItem::fromDefinition($def); + $def->isVisible($prefix, $prefixes[0], $isInMethodDeclaration)) { + $list->items[] = CompletionItemFactory::fromDefinition($def); } } } diff --git a/src/Definition.php b/src/Definition.php index 4b6239db..39636bb4 100644 --- a/src/Definition.php +++ b/src/Definition.php @@ -136,13 +136,15 @@ public function getAncestorDefinitions(ReadableIndex $index, bool $includeSelf = /** * Checks the definition's visibility. + * @param string $match Owner of the FQNS + * @param string $caller Descendant of the FQNS owner + * @param bool $isInMethodDeclaration checking if the call is from inside a + * method * @return bool */ - public function isVisible(string $match, string $caller, \Microsoft\PhpParser\Node $node): bool - { - $ancestor = $node->getFirstAncestor(\Microsoft\PhpParser\Node\MethodDeclaration::class); - - if ($ancestor) { + public function isVisible(string $match, string $caller, bool $isInMethodDeclaration): bool + { + if ($isInMethodDeclaration) { if ($match !== $caller && $this->isPrivate()) { return false; } From b808e3c365a5226cf6b7cd32a2512d9431f0ba01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sat, 10 Nov 2018 13:46:40 +0100 Subject: [PATCH 04/13] Update .gitignore Ignoting Netbeans conf --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 6476dbe1..a7c04f14 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ composer.lock stubs *.ast node_modules/ +/nbproject/ \ No newline at end of file From 88bb2eaf177edf9dfab245cf1c663a22fd494019 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sat, 10 Nov 2018 13:52:16 +0100 Subject: [PATCH 05/13] Minor changes --- src/CompletionProvider.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 4a87f1d6..2461031b 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -236,7 +236,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon $fqns = FqnUtilities\getFqnsFromType( $this->definitionResolver->resolveExpressionNodeToType($node->dereferencableExpression) ); - $start = microtime(true); + $isInMethodDeclaration = null !== $node->getFirstAncestor(\Microsoft\PhpParser\Node\MethodDeclaration::class); // Add the object access operator to only get members of all parents $prefixes = []; @@ -254,7 +254,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon } } } - echo microtime(true) - $start; + } elseif ( ($scoped = $node->parent) instanceof Node\Expression\ScopedPropertyAccessExpression || ($scoped = $node) instanceof Node\Expression\ScopedPropertyAccessExpression From df898ec633eb4a2a4fd95ac49c13699993d5e846 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sat, 10 Nov 2018 14:22:55 +0100 Subject: [PATCH 06/13] Merging with master --- src/CompletionProvider.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index 2461031b..ff640f0b 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -272,7 +272,7 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon $fqns = FqnUtilities\getFqnsFromType( $classType = $this->definitionResolver->resolveExpressionNodeToType($scoped->scopeResolutionQualifier) ); - + $isInMethodDeclaration = null !== $node->getFirstAncestor(\Microsoft\PhpParser\Node\MethodDeclaration::class); // Append :: operator to only get static members of all parents $prefixes = []; foreach ($this->expandParentFqns($fqns) as $prefix) { @@ -282,7 +282,9 @@ public function provideCompletion(PhpDocument $doc, Position $pos, CompletionCon // Collect all definitions that match any of the prefixes foreach ($this->index->getDefinitions() as $fqn => $def) { foreach ($prefixes as $prefix) { - if (substr(strtolower($fqn), 0, strlen($prefix)) === strtolower($prefix) && $def->isMember) { + if (substr(strtolower($fqn), 0, strlen($prefix)) === strtolower($prefix) && + $def->isMember && + $def->isVisible($prefix, $prefixes[0], $isInMethodDeclaration)) { $list->items[] = CompletionItemFactory::fromDefinition($def); } } From e71e4e0ff04f149f204f231835228c86a80a5ae9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sat, 10 Nov 2018 17:44:05 +0100 Subject: [PATCH 07/13] Trimmed Trailing Whitespace --- .vscode/settings.json | 3 ++- fixtures/completion/public_only.php | 15 +++++++++++++++ src/CompletionProvider.php | 4 ++-- src/Definition.php | 8 ++++---- 4 files changed, 23 insertions(+), 7 deletions(-) create mode 100644 fixtures/completion/public_only.php diff --git a/.vscode/settings.json b/.vscode/settings.json index 38150bde..63af6ee2 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,5 +3,6 @@ "search.exclude": { "**/validation": true, "**/tests/Validation/cases": true - } + }, + "files.trimTrailingWhitespace": true, } diff --git a/fixtures/completion/public_only.php b/fixtures/completion/public_only.php new file mode 100644 index 00000000..aadb96c9 --- /dev/null +++ b/fixtures/completion/public_only.php @@ -0,0 +1,15 @@ +index->getDefinitions() as $fqn => $def) { foreach ($prefixes as $prefix) { - if (substr($fqn, 0, strlen($prefix)) === $prefix && + if (substr($fqn, 0, strlen($prefix)) === $prefix && $def->isMember && $def->isVisible($prefix, $prefixes[0], $isInMethodDeclaration)) { $list->items[] = CompletionItemFactory::fromDefinition($def); } } } - + } elseif ( ($scoped = $node->parent) instanceof Node\Expression\ScopedPropertyAccessExpression || ($scoped = $node) instanceof Node\Expression\ScopedPropertyAccessExpression diff --git a/src/Definition.php b/src/Definition.php index 39636bb4..42329fea 100644 --- a/src/Definition.php +++ b/src/Definition.php @@ -133,17 +133,17 @@ public function getAncestorDefinitions(ReadableIndex $index, bool $includeSelf = } } } - + /** * Checks the definition's visibility. * @param string $match Owner of the FQNS * @param string $caller Descendant of the FQNS owner - * @param bool $isInMethodDeclaration checking if the call is from inside a + * @param bool $isInMethodDeclaration checking if the call is from inside a * method * @return bool */ public function isVisible(string $match, string $caller, bool $isInMethodDeclaration): bool - { + { if ($isInMethodDeclaration) { if ($match !== $caller && $this->isPrivate()) { return false; @@ -151,7 +151,7 @@ public function isVisible(string $match, string $caller, bool $isInMethodDeclara } else if ($this->isProtected() || $this->isPrivate()) { return false; } - + return true; } From 73ab1520a37a92f789142c55f1cd0b6d8d1c692e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sat, 10 Nov 2018 23:57:14 +0100 Subject: [PATCH 08/13] Visibility Test and Changing line ending from CRLF -> LF Added CompletionWithVisibilityTest.php - checks visibility of a property or method call (only public) - checks visibility of a Class (can see all methods and properties) - checks visibility of a child class (public and protected) --- .vscode/settings.json | 1 + .../completion/child_class_visibility.php | 9 + fixtures/completion/public_only.php | 15 -- fixtures/global_symbols.php | 26 ++- .../CompletionWithVisibilityTest.php | 178 ++++++++++++++++++ tests/Validation/ValidationTest.php | 14 +- 6 files changed, 220 insertions(+), 23 deletions(-) create mode 100644 fixtures/completion/child_class_visibility.php delete mode 100644 fixtures/completion/public_only.php create mode 100644 tests/Server/TextDocument/CompletionWithVisibilityTest.php diff --git a/.vscode/settings.json b/.vscode/settings.json index 63af6ee2..4897ad83 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -5,4 +5,5 @@ "**/tests/Validation/cases": true }, "files.trimTrailingWhitespace": true, + "files.eol": "\n", } diff --git a/fixtures/completion/child_class_visibility.php b/fixtures/completion/child_class_visibility.php new file mode 100644 index 00000000..44a177dc --- /dev/null +++ b/fixtures/completion/child_class_visibility.php @@ -0,0 +1,9 @@ + + } +} \ No newline at end of file diff --git a/fixtures/completion/public_only.php b/fixtures/completion/public_only.php deleted file mode 100644 index aadb96c9..00000000 --- a/fixtures/completion/public_only.php +++ /dev/null @@ -1,15 +0,0 @@ -testProperty = $testParameter; } + + private function privateTestMethod() + { + return $this->privateProperty; + } + + protected function protectedTestMethod() + { + return $this->protectedProperty; + } } trait TestTrait @@ -114,6 +138,6 @@ class UnusedClass public $unusedProperty; public function unusedMethod() - { + { } } diff --git a/tests/Server/TextDocument/CompletionWithVisibilityTest.php b/tests/Server/TextDocument/CompletionWithVisibilityTest.php new file mode 100644 index 00000000..63151d6b --- /dev/null +++ b/tests/Server/TextDocument/CompletionWithVisibilityTest.php @@ -0,0 +1,178 @@ + + */ +class CompletionWithVisibilityTest extends TestCase +{ + + /** + * @var TextDocument + */ + private $textDocument; + + /** + * @var PhpDocumentLoader + */ + private $loader; + + /** + * + * @var string + */ + private $fixturesPath; + + public function __construct($name = null, array $data = array(), $dataName = '') + { + parent::__construct($name, $data, $dataName); + $this->fixturesPath = __DIR__ . '/../../../fixtures'; + } + + public function setUp() + { + $client = new LanguageClient(new MockProtocolStream, new MockProtocolStream); + $projectIndex = new ProjectIndex(new Index, new DependenciesIndex); + $definitionResolver = new DefinitionResolver($projectIndex); + $contentRetriever = new FileSystemContentRetriever; + $this->loader = new PhpDocumentLoader($contentRetriever, $projectIndex, $definitionResolver); + $this->loader->load(pathToUri($this->fixturesPath . '/global_symbols.php'))->wait(); + $this->loader->load(pathToUri($this->fixturesPath . '/symbols.php'))->wait(); + $this->textDocument = new TextDocument($this->loader, $definitionResolver, $client, $projectIndex); + } + + /** + * Can access only to public properties and methods + */ + public function testVisibilityFromCall() + { + $items = $this->getCompletion('/completion/property.php', 3, 6); + // doesn't contain any of these properties and methods + $this->assertCompletionsListSubsetNotContains(new CompletionList([ + new CompletionItem( + 'privateProperty', CompletionItemKind::PROPERTY, '\TestClass', + 'Reprehenderit magna velit mollit ipsum do.' + ), + new CompletionItem( + 'privateTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + ), + new CompletionItem( + 'protectedProperty', CompletionItemKind::PROPERTY, '\TestClass', + 'Reprehenderit magna velit mollit ipsum do.' + ), + new CompletionItem( + 'protectedTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + ) + ], true), $items); + } + + /** + * From a Child class only public and protected properties and methods are + * visible + * + */ + public function testVisibilityInsideADescendantClassMethod() + { + $items = $this->getCompletion('/completion/child_class_visibility.php', 6, 16); + // doesn't contain any of these properties and methods + $this->assertCompletionsListSubsetNotContains(new CompletionList([ + new CompletionItem( + 'privateProperty', CompletionItemKind::PROPERTY, '\TestClass', + 'Reprehenderit magna velit mollit ipsum do.' + ), + new CompletionItem( + 'privateTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + ) + ], true), $items); + } + + public function testVisibilityInsideClassMethod() + { + $items = $this->getCompletion('/global_symbols.php', 73, 15); + // can see all properties and methods + $this->assertCompletionsListSubset(new CompletionList([ + new CompletionItem( + 'privateProperty', CompletionItemKind::PROPERTY, '\TestClass', + 'Reprehenderit magna velit mollit ipsum do.' + ), + new CompletionItem( + 'privateTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + ), + new CompletionItem( + 'protectedProperty', CompletionItemKind::PROPERTY, '\TestClass', + 'Reprehenderit magna velit mollit ipsum do.' + ), + new CompletionItem( + 'protectedTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + ), + new CompletionItem( + 'testProperty', + CompletionItemKind::PROPERTY, + '\TestClass', // Type of the property + 'Reprehenderit magna velit mollit ipsum do.' + ), + new CompletionItem( + 'testMethod', + CompletionItemKind::METHOD, + '\TestClass', // Return type of the method + 'Non culpa nostrud mollit esse sunt laboris in irure ullamco cupidatat amet.' + ) + ], true), $items); + + } + + /** + * + * @param string $fixtureFile + * @param int $line + * @param int $char + * @return CompletionList + */ + private function getCompletion(string $fixtureFile, int $line, int $char) + { + $completionUri = pathToUri($this->fixturesPath . $fixtureFile); + $this->loader->open($completionUri, file_get_contents($completionUri)); + return $this->textDocument->completion( + new TextDocumentIdentifier($completionUri), new Position($line, $char) + )->wait(); + } + + private function assertCompletionsListSubset(CompletionList $subsetList, CompletionList $list) + { + foreach ($subsetList->items as $expectedItem) { + $this->assertContains($expectedItem, $list->items, null, null, false); + } + + $this->assertEquals($subsetList->isIncomplete, $list->isIncomplete); + } + + private function assertCompletionsListSubsetNotContains(CompletionList $subsetList, CompletionList $list) + { + foreach ($subsetList->items as $expectedItem) { + $this->assertNotContains($expectedItem, $list->items, null, null, false); + } + $this->assertEquals($subsetList->isIncomplete, $list->isIncomplete); + } +} diff --git a/tests/Validation/ValidationTest.php b/tests/Validation/ValidationTest.php index 2c3efafd..62b27eb6 100644 --- a/tests/Validation/ValidationTest.php +++ b/tests/Validation/ValidationTest.php @@ -1,5 +1,4 @@ getSize() < 100000) { $testProviderArray[] = [$file->getPathname()]; } @@ -57,17 +57,17 @@ public function testDefinitionsAndReferences($testCaseFile) $outputFile = getExpectedValuesFile($testCaseFile); if (!file_exists($outputFile)) { - file_put_contents($outputFile, json_encode($actualValues, JSON_PRETTY_PRINT|JSON_UNESCAPED_SLASHES)); + file_put_contents($outputFile, json_encode($actualValues, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); } - $expectedValues = (array)json_decode(file_get_contents($outputFile)); + $expectedValues = (array) json_decode(file_get_contents($outputFile)); try { $this->assertEquals($expectedValues['definitions'], $actualValues['definitions']); - $this->assertEquals((array)$expectedValues['references'], (array)$actualValues['references'], sprintf('references match in "%s"', $outputFile)); + $this->assertEquals((array) $expectedValues['references'], (array) $actualValues['references'], sprintf('references match in "%s"', $outputFile)); } catch (\Throwable $e) { $outputFile = getExpectedValuesFile($testCaseFile); - file_put_contents($outputFile, json_encode($actualValues, JSON_PRETTY_PRINT|JSON_UNESCAPED_SLASHES)); + file_put_contents($outputFile, json_encode($actualValues, JSON_PRETTY_PRINT | JSON_UNESCAPED_SLASHES)); throw $e; } @@ -135,7 +135,7 @@ private function getTestValuesFromDefs($definitions): array } elseif ($propertyName === 'extends') { $definition->$propertyName = $definition->$propertyName ?? []; } elseif ($propertyName === 'type' && $definition->type !== null) { - $defsForAssert[$fqn]['type__tostring'] = (string)$definition->type; + $defsForAssert[$fqn]['type__tostring'] = (string) $definition->type; } $defsForAssert[$fqn][$propertyName] = $definition->$propertyName; From fd9d326c7adcf01beccad0b49727e5e71b9fdee4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sun, 11 Nov 2018 00:19:19 +0100 Subject: [PATCH 09/13] Fixing phpcs errors --- .../CompletionWithVisibilityTest.php | 62 ++++++++++++------- 1 file changed, 41 insertions(+), 21 deletions(-) diff --git a/tests/Server/TextDocument/CompletionWithVisibilityTest.php b/tests/Server/TextDocument/CompletionWithVisibilityTest.php index 63151d6b..84ff4760 100644 --- a/tests/Server/TextDocument/CompletionWithVisibilityTest.php +++ b/tests/Server/TextDocument/CompletionWithVisibilityTest.php @@ -72,18 +72,26 @@ public function testVisibilityFromCall() // doesn't contain any of these properties and methods $this->assertCompletionsListSubsetNotContains(new CompletionList([ new CompletionItem( - 'privateProperty', CompletionItemKind::PROPERTY, '\TestClass', + 'privateProperty', + CompletionItemKind::PROPERTY, + '\TestClass', 'Reprehenderit magna velit mollit ipsum do.' - ), + ), new CompletionItem( - 'privateTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + 'privateTestMethod', + CompletionItemKind::METHOD, + 'mixed' // Return type of the method ), - new CompletionItem( - 'protectedProperty', CompletionItemKind::PROPERTY, '\TestClass', + new CompletionItem( + 'protectedProperty', + CompletionItemKind::PROPERTY, + '\TestClass', 'Reprehenderit magna velit mollit ipsum do.' - ), + ), new CompletionItem( - 'protectedTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + 'protectedTestMethod', + CompletionItemKind::METHOD, + 'mixed' // Return type of the method ) ], true), $items); } @@ -99,33 +107,45 @@ public function testVisibilityInsideADescendantClassMethod() // doesn't contain any of these properties and methods $this->assertCompletionsListSubsetNotContains(new CompletionList([ new CompletionItem( - 'privateProperty', CompletionItemKind::PROPERTY, '\TestClass', + 'privateProperty', + CompletionItemKind::PROPERTY, + '\TestClass', 'Reprehenderit magna velit mollit ipsum do.' - ), + ), new CompletionItem( - 'privateTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + 'privateTestMethod', + CompletionItemKind::METHOD, + 'mixed' // Return type of the method ) ], true), $items); } - + public function testVisibilityInsideClassMethod() { $items = $this->getCompletion('/global_symbols.php', 73, 15); // can see all properties and methods $this->assertCompletionsListSubset(new CompletionList([ new CompletionItem( - 'privateProperty', CompletionItemKind::PROPERTY, '\TestClass', + 'privateProperty', + CompletionItemKind::PROPERTY, + '\TestClass', 'Reprehenderit magna velit mollit ipsum do.' - ), + ), new CompletionItem( - 'privateTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + 'privateTestMethod', + CompletionItemKind::METHOD, + 'mixed' // Return type of the method ), - new CompletionItem( - 'protectedProperty', CompletionItemKind::PROPERTY, '\TestClass', + new CompletionItem( + 'protectedProperty', + CompletionItemKind::PROPERTY, + '\TestClass', 'Reprehenderit magna velit mollit ipsum do.' - ), + ), new CompletionItem( - 'protectedTestMethod', CompletionItemKind::METHOD, 'mixed' // Return type of the method + 'protectedTestMethod', + CompletionItemKind::METHOD, + 'mixed' // Return type of the method ), new CompletionItem( 'testProperty', @@ -140,7 +160,6 @@ public function testVisibilityInsideClassMethod() 'Non culpa nostrud mollit esse sunt laboris in irure ullamco cupidatat amet.' ) ], true), $items); - } /** @@ -155,8 +174,9 @@ private function getCompletion(string $fixtureFile, int $line, int $char) $completionUri = pathToUri($this->fixturesPath . $fixtureFile); $this->loader->open($completionUri, file_get_contents($completionUri)); return $this->textDocument->completion( - new TextDocumentIdentifier($completionUri), new Position($line, $char) - )->wait(); + new TextDocumentIdentifier($completionUri), + new Position($line, $char) + )->wait(); } private function assertCompletionsListSubset(CompletionList $subsetList, CompletionList $list) From 8d4fe1ef9f7f720e30a128ef8b70e46ece7845a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sun, 11 Nov 2018 00:33:17 +0100 Subject: [PATCH 10/13] CompletionTest fixed --- tests/Server/TextDocument/CompletionTest.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index c64c8e67..d845429c 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/tests/Server/TextDocument/CompletionTest.php @@ -827,12 +827,23 @@ public function testThisWithPrefix() '\TestClass', // Type of the property 'Reprehenderit magna velit mollit ipsum do.' ), + new CompletionItem( + 'protectedProperty', + CompletionItemKind::PROPERTY, + '\TestClass', // Type of the property + 'Reprehenderit magna velit mollit ipsum do.' + ), new CompletionItem( 'testMethod', CompletionItemKind::METHOD, '\TestClass', // Return type of the method 'Non culpa nostrud mollit esse sunt laboris in irure ullamco cupidatat amet.' ), + new CompletionItem( + 'protectedTestMethod', + CompletionItemKind::METHOD, + 'mixed' // Return type of the method + ), new CompletionItem( 'foo', CompletionItemKind::PROPERTY, From 9a75adfe0775254038969b75e1d6cad03ad1f765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sun, 11 Nov 2018 00:56:07 +0100 Subject: [PATCH 11/13] Decided to move visibility files apart Other tests were failing because I modifed the global_symbols.php file --- fixtures/global_symbols.php | 24 ------ fixtures/visibility_class.php | 76 +++++++++++++++++++ tests/Server/TextDocument/CompletionTest.php | 11 --- .../CompletionWithVisibilityTest.php | 5 +- 4 files changed, 78 insertions(+), 38 deletions(-) create mode 100644 fixtures/visibility_class.php diff --git a/fixtures/global_symbols.php b/fixtures/global_symbols.php index 7f81df71..bb69d003 100644 --- a/fixtures/global_symbols.php +++ b/fixtures/global_symbols.php @@ -41,20 +41,6 @@ class TestClass implements TestInterface */ public $testProperty; - /** - * Reprehenderit magna velit mollit ipsum do. - * - * @var TestClass - */ - private $privateProperty; - - /** - * Reprehenderit magna velit mollit ipsum do. - * - * @var TestClass - */ - protected $protectedProperty; - /** * Do magna consequat veniam minim proident eiusmod incididunt aute proident. */ @@ -73,16 +59,6 @@ public function testMethod($testParameter): TestInterface { $this->testProperty = $testParameter; } - - private function privateTestMethod() - { - return $this->privateProperty; - } - - protected function protectedTestMethod() - { - return $this->protectedProperty; - } } trait TestTrait diff --git a/fixtures/visibility_class.php b/fixtures/visibility_class.php new file mode 100644 index 00000000..ece34621 --- /dev/null +++ b/fixtures/visibility_class.php @@ -0,0 +1,76 @@ +testProperty = $testParameter; + } + + private function privateTestMethod() + { + return $this->privateProperty; + } + + protected function protectedTestMethod() + { + return $this->protectedProperty; + } +} \ No newline at end of file diff --git a/tests/Server/TextDocument/CompletionTest.php b/tests/Server/TextDocument/CompletionTest.php index d845429c..c64c8e67 100644 --- a/tests/Server/TextDocument/CompletionTest.php +++ b/tests/Server/TextDocument/CompletionTest.php @@ -827,23 +827,12 @@ public function testThisWithPrefix() '\TestClass', // Type of the property 'Reprehenderit magna velit mollit ipsum do.' ), - new CompletionItem( - 'protectedProperty', - CompletionItemKind::PROPERTY, - '\TestClass', // Type of the property - 'Reprehenderit magna velit mollit ipsum do.' - ), new CompletionItem( 'testMethod', CompletionItemKind::METHOD, '\TestClass', // Return type of the method 'Non culpa nostrud mollit esse sunt laboris in irure ullamco cupidatat amet.' ), - new CompletionItem( - 'protectedTestMethod', - CompletionItemKind::METHOD, - 'mixed' // Return type of the method - ), new CompletionItem( 'foo', CompletionItemKind::PROPERTY, diff --git a/tests/Server/TextDocument/CompletionWithVisibilityTest.php b/tests/Server/TextDocument/CompletionWithVisibilityTest.php index 84ff4760..e595d2a0 100644 --- a/tests/Server/TextDocument/CompletionWithVisibilityTest.php +++ b/tests/Server/TextDocument/CompletionWithVisibilityTest.php @@ -58,8 +58,7 @@ public function setUp() $definitionResolver = new DefinitionResolver($projectIndex); $contentRetriever = new FileSystemContentRetriever; $this->loader = new PhpDocumentLoader($contentRetriever, $projectIndex, $definitionResolver); - $this->loader->load(pathToUri($this->fixturesPath . '/global_symbols.php'))->wait(); - $this->loader->load(pathToUri($this->fixturesPath . '/symbols.php'))->wait(); + $this->loader->load(pathToUri($this->fixturesPath . '/visibility_class.php'))->wait(); $this->textDocument = new TextDocument($this->loader, $definitionResolver, $client, $projectIndex); } @@ -122,7 +121,7 @@ public function testVisibilityInsideADescendantClassMethod() public function testVisibilityInsideClassMethod() { - $items = $this->getCompletion('/global_symbols.php', 73, 15); + $items = $this->getCompletion('/visibility_class.php', 64, 15); // can see all properties and methods $this->assertCompletionsListSubset(new CompletionList([ new CompletionItem( From fabe20b1267b455ef373cc7e7b30e9cb73d646b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20No=C3=A9=20Gonz=C3=A1lez?= Date: Sun, 11 Nov 2018 01:01:51 +0100 Subject: [PATCH 12/13] EOL --- fixtures/visibility_class.php | 1 + 1 file changed, 1 insertion(+) diff --git a/fixtures/visibility_class.php b/fixtures/visibility_class.php index ece34621..6900a8df 100644 --- a/fixtures/visibility_class.php +++ b/fixtures/visibility_class.php @@ -1,6 +1,7 @@ Date: Tue, 13 Nov 2018 21:49:02 +0100 Subject: [PATCH 13/13] [FIX] Checked changes and tested again --- .vscode/settings.json | 9 ++++ .../completion/child_class_visibility.php | 53 ++++++++++++++++--- src/CompletionProvider.php | 22 ++++---- 3 files changed, 66 insertions(+), 18 deletions(-) diff --git a/.vscode/settings.json b/.vscode/settings.json index 4897ad83..d85d6805 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -6,4 +6,13 @@ }, "files.trimTrailingWhitespace": true, "files.eol": "\n", + + "files.exclude": { + "**/.git": true, + "**/.svn": true, + "**/.hg": true, + "**/CVS": true, + "**/.DS_Store": true, + "**/tests": false + } } diff --git a/fixtures/completion/child_class_visibility.php b/fixtures/completion/child_class_visibility.php index 44a177dc..ebfaeaf2 100644 --- a/fixtures/completion/child_class_visibility.php +++ b/fixtures/completion/child_class_visibility.php @@ -1,9 +1,50 @@ - } -} \ No newline at end of file + public function canSeeMethod() + { + $this-> + } +} + +class Foo extends Bar +{ + + public function getRandom() + { + $this->c; + return random_bytes(25); + } +} + +class Bar +{ + + private $test; + protected $seeme; + + public function __construct() + { + $this->test = 'Basic'; + } + + public function getTest() + { + return $this->test; + } + + private function cantSee() + { + + } + + protected function canSee($arg) + { + # code... + } +} + +$foo = new Foo(); +$foo->getRandom(); diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index f8f85bfd..fc76e4e0 100644 --- a/src/CompletionProvider.php +++ b/src/CompletionProvider.php @@ -159,7 +159,6 @@ public function provideCompletion( ): CompletionList { // This can be made much more performant if the tree follows specific invariants. $node = $doc->getNodeAtPosition($pos); - // Get the node at the position under the cursor $offset = $node === null ? -1 : $pos->toOffset($node->getFileContents()); if ( @@ -248,18 +247,17 @@ public function provideCompletion( $this->definitionResolver->resolveExpressionNodeToType($node->dereferencableExpression) ); $isInMethodDeclaration = null !== $node->getFirstAncestor(\Microsoft\PhpParser\Node\MethodDeclaration::class); - // Add the object access operator to only get members of all parents - $prefixes = []; - foreach ($this->expandParentFqns($fqns) as $prefix) { - $prefixes[] = $prefix . '->'; - } - - // Collect all definitions that match any of the prefixes - foreach ($this->index->getDefinitions() as $fqn => $def) { - foreach ($prefixes as $prefix) { - if (substr($fqn, 0, strlen($prefix)) === $prefix && + // The FQNs of the symbol and its parents (eg the implemented interfaces) + foreach ($this->expandParentFqns($fqns) as $parentFqn) { + // Add the object access operator to only get members of all parents + $prefix = $parentFqn . '->'; + $prefixLen = strlen($prefix); + // Collect fqn definitions + foreach ($this->index->getChildDefinitionsForFqn($parentFqn) as $fqn => $def) { + if (substr($fqn, 0, $prefixLen) === $prefix && $def->isMember && - $def->isVisible($prefix, $prefixes[0], $isInMethodDeclaration)) { + $def->isVisible($prefix, $fqns[0] . '->', $isInMethodDeclaration) + ) { $list->items[] = CompletionItemFactory::fromDefinition($def); } }