Skip to content
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

Don't suggest private methods #682

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ composer.lock
stubs
*.ast
node_modules/
/nbproject/
11 changes: 11 additions & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
50 changes: 50 additions & 0 deletions fixtures/completion/child_class_visibility.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?php

class ThisChildClass extends TestClass
{

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();
2 changes: 1 addition & 1 deletion fixtures/global_symbols.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,6 @@ class UnusedClass
public $unusedProperty;

public function unusedMethod()
{
{
}
}
77 changes: 77 additions & 0 deletions fixtures/visibility_class.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
<?php

/**
* Pariatur ut laborum tempor voluptate consequat ea deserunt.
*
* Deserunt enim minim sunt sint ea nisi. Deserunt excepteur tempor id nostrud
* laboris commodo ad commodo velit mollit qui non officia id. Nulla duis veniam
* veniam officia deserunt et non dolore mollit ea quis eiusmod sit non. Occaecat
* consequat sunt culpa exercitation pariatur id reprehenderit nisi incididunt Lorem
* sint. Officia culpa pariatur laborum nostrud cupidatat consequat mollit.
*/
class TestClass
{
/**
* Anim labore veniam consectetur laboris minim quis aute aute esse nulla ad.
*
* @var int
*/
const TEST_CLASS_CONST = 123;

/**
* Lorem excepteur officia sit anim velit veniam enim.
*
* @var TestClass[]
*/
public static $staticTestProperty;

/**
* Reprehenderit magna velit mollit ipsum do.
*
* @var TestClass
*/
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.
*/
public static function staticTestMethod()
{
echo self::TEST_CLASS_CONST;
}

/**
* Non culpa nostrud mollit esse sunt laboris in irure ullamco cupidatat amet.
*
* @param TestClass $testParameter Lorem sunt velit incididunt mollit
* @return TestClass
*/
public function testMethod($testParameter): TestInterface
{
$this->testProperty = $testParameter;
}

private function privateTestMethod()
{
return $this->privateProperty;
}

protected function protectedTestMethod()
{
return $this->protectedProperty;
}
}
9 changes: 5 additions & 4 deletions src/CompletionProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -247,15 +246,18 @@ 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
$prefix = $parentFqn . '->';
$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);
}
}
Expand All @@ -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
Expand Down
31 changes: 31 additions & 0 deletions src/Definition.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Loading