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/.vscode/settings.json b/.vscode/settings.json index 38150bde..d85d6805 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -3,5 +3,16 @@ "search.exclude": { "**/validation": true, "**/tests/Validation/cases": true + }, + "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 new file mode 100644 index 00000000..ebfaeaf2 --- /dev/null +++ b/fixtures/completion/child_class_visibility.php @@ -0,0 +1,50 @@ + + } +} + +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/fixtures/global_symbols.php b/fixtures/global_symbols.php index 25b928b1..bb69d003 100644 --- a/fixtures/global_symbols.php +++ b/fixtures/global_symbols.php @@ -114,6 +114,6 @@ class UnusedClass public $unusedProperty; public function unusedMethod() - { + { } } diff --git a/fixtures/visibility_class.php b/fixtures/visibility_class.php new file mode 100644 index 00000000..6900a8df --- /dev/null +++ b/fixtures/visibility_class.php @@ -0,0 +1,77 @@ +testProperty = $testParameter; + } + + private function privateTestMethod() + { + return $this->privateProperty; + } + + protected function protectedTestMethod() + { + return $this->protectedProperty; + } +} \ No newline at end of file diff --git a/src/CompletionProvider.php b/src/CompletionProvider.php index e7dfcf1a..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 ( @@ -247,7 +246,7 @@ public function provideCompletion( $fqns = FqnUtilities\getFqnsFromType( $this->definitionResolver->resolveExpressionNodeToType($node->dereferencableExpression) ); - + $isInMethodDeclaration = null !== $node->getFirstAncestor(\Microsoft\PhpParser\Node\MethodDeclaration::class); // 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 @@ -255,7 +254,10 @@ public function provideCompletion( $prefixLen = strlen($prefix); // Collect fqn definitions foreach ($this->index->getChildDefinitionsForFqn($parentFqn) as $fqn => $def) { - if (substr($fqn, 0, $prefixLen) === $prefix && $def->isMember) { + if (substr($fqn, 0, $prefixLen) === $prefix && + $def->isMember && + $def->isVisible($prefix, $fqns[0] . '->', $isInMethodDeclaration) + ) { $list->items[] = CompletionItemFactory::fromDefinition($def); } } @@ -278,7 +280,6 @@ public function provideCompletion( $fqns = FqnUtilities\getFqnsFromType( $classType = $this->definitionResolver->resolveExpressionNodeToType($scoped->scopeResolutionQualifier) ); - // The FQNs of the symbol and its parents (eg the implemented interfaces) foreach ($this->expandParentFqns($fqns) as $parentFqn) { // Append :: operator to only get static members of all parents diff --git a/src/Definition.php b/src/Definition.php index d81594db..7742546e 100644 --- a/src/Definition.php +++ b/src/Definition.php @@ -133,4 +133,35 @@ 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, bool $isInMethodDeclaration): bool + { + if ($isInMethodDeclaration) { + 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); + } } diff --git a/tests/Server/TextDocument/CompletionWithVisibilityTest.php b/tests/Server/TextDocument/CompletionWithVisibilityTest.php new file mode 100644 index 00000000..e595d2a0 --- /dev/null +++ b/tests/Server/TextDocument/CompletionWithVisibilityTest.php @@ -0,0 +1,197 @@ + + */ +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 . '/visibility_class.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('/visibility_class.php', 64, 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;