Skip to content

Find references to property if virtual #672

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 1 commit into from
Jun 6, 2025
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
17 changes: 17 additions & 0 deletions src/phpDocumentor/Reflection/NodeVisitor/FindingVisitor.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?php

declare(strict_types=1);

namespace phpDocumentor\Reflection\NodeVisitor;

use PhpParser\NodeVisitor\FirstFindingVisitor as BaseFindingVisitor;

final class FindingVisitor extends BaseFindingVisitor
{
public function __construct(callable $filterCallback)
{
parent::__construct($filterCallback);

$this->foundNode = null;
}
}
73 changes: 69 additions & 4 deletions src/phpDocumentor/Reflection/Php/Factory/PropertyBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use phpDocumentor\Reflection\DocBlockFactoryInterface;
use phpDocumentor\Reflection\Fqsen;
use phpDocumentor\Reflection\Location;
use phpDocumentor\Reflection\NodeVisitor\FindingVisitor;
use phpDocumentor\Reflection\Php\AsymmetricVisibility;
use phpDocumentor\Reflection\Php\Factory\Reducer\Reducer;
use phpDocumentor\Reflection\Php\Property as PropertyElement;
Expand All @@ -15,16 +16,21 @@
use phpDocumentor\Reflection\Php\Visibility;
use PhpParser\Comment\Doc;
use PhpParser\Modifiers;
use PhpParser\Node;
use PhpParser\Node\ComplexType;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\PropertyFetch;
use PhpParser\Node\Expr\Variable;
use PhpParser\Node\Identifier;
use PhpParser\Node\Name;
use PhpParser\Node\Param;
use PhpParser\Node\PropertyHook as PropertyHookNode;
use PhpParser\NodeTraverser;
use PhpParser\PrettyPrinter\Standard as PrettyPrinter;

use function array_filter;
use function array_map;
use function count;
use function method_exists;

/**
Expand Down Expand Up @@ -141,6 +147,14 @@ public function hooks(array $hooks): self

public function build(ContextStack $context): PropertyElement
{
$hooks = array_filter(array_map(
fn (PropertyHookNode $hook) => $this->buildHook($hook, $context, $this->visibility),
$this->hooks,
));

// Check if this is a virtual property by examining all hooks
$isVirtual = $this->isVirtualProperty($this->hooks, $this->fqsen->getName());

return new PropertyElement(
$this->fqsen,
$this->visibility,
Expand All @@ -151,10 +165,8 @@ public function build(ContextStack $context): PropertyElement
$this->endLocation,
(new Type())->fromPhpParser($this->type),
$this->readOnly,
array_filter(array_map(
fn (PropertyHookNode $hook) => $this->buildHook($hook, $context, $this->visibility),
$this->hooks,
)),
$hooks,
$isVirtual,
);
}

Expand Down Expand Up @@ -264,6 +276,59 @@ private function buildHook(PropertyHookNode $hook, ContextStack $context, Visibi
return $result;
}

/**
* Detects if a property is virtual by checking if any of its hooks reference the property itself.
*
* A virtual property is one where no defined hook references the property itself.
* For example, in the 'get' hook, it doesn't use $this->propertyName.
*
* @param PropertyHookNode[] $hooks The property hooks to check
* @param string $propertyName The name of the property
*
* @return bool True if the property is virtual, false otherwise
*/
private function isVirtualProperty(array $hooks, string $propertyName): bool
{
if (empty($hooks)) {
return false;
}

foreach ($hooks as $hook) {
$stmts = $hook->getStmts();

if ($stmts === null || count($stmts) === 0) {
continue;
}

$finder = new FindingVisitor(
static function (Node $node) use ($propertyName) {
// Check if the node is a property fetch that references the property
return $node instanceof PropertyFetch && $node->name instanceof Identifier &&
$node->name->toString() === $propertyName &&
$node->var instanceof Variable &&
$node->var->name === 'this';
},
);

$traverser = new NodeTraverser($finder);
$traverser->traverse($stmts);

if ($finder->getFoundNode() !== null) {
return false;
}
}

return true;
}

/**
* Builds the hook visibility based on the hook name and property visibility.
*
* @param string $hookName The name of the hook ('get' or 'set')
* @param Visibility $propertyVisibility The visibility of the property
*
* @return Visibility The appropriate visibility for the hook
*/
private function buildHookVisibility(string $hookName, Visibility $propertyVisibility): Visibility
{
if ($propertyVisibility instanceof AsymmetricVisibility === false) {
Expand Down
11 changes: 11 additions & 0 deletions src/phpDocumentor/Reflection/Php/Property.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ public function __construct(
private readonly Type|null $type = null,
private readonly bool $readOnly = false,
private readonly array $hooks = [],
private readonly bool $virtual = false,
) {
$this->visibility = $visibility ?: new Visibility('public');
$this->location = $location ?: new Location(-1);
Expand Down Expand Up @@ -154,4 +155,14 @@ public function getHooks(): array
{
return $this->hooks;
}

/**
* Returns true when this property is virtual (not explicitly backed).
*
* A virtual property is one where no defined hook references the property itself.
*/
public function isVirtual(): bool
{
return $this->virtual;
}
}
37 changes: 37 additions & 0 deletions tests/integration/PropertyHookTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ public function testPropertyHookWithDocblocks(): void
$class = $project->getFiles()[$file]->getClasses()['\PropertyHook'];
$hooks = $class->getProperties()['\PropertyHook::$example']->getHooks();

$this->assertTrue($class->getProperties()['\PropertyHook::$example']->isVirtual());
$this->assertCount(2, $hooks);
$this->assertEquals('get', $hooks[0]->getName());
$this->assertEquals(new Visibility(Visibility::PUBLIC_), $hooks[0]->getVisibility());
Expand Down Expand Up @@ -73,6 +74,7 @@ public function testPropertyHookAsymmetric(): void
),
$class->getProperties()['\PropertyHook::$example']->getVisibility()
);
$this->assertTrue($class->getProperties()['\PropertyHook::$example']->isVirtual());
$this->assertCount(2, $hooks);
$this->assertEquals('get', $hooks[0]->getName());
$this->assertEquals(new Visibility(Visibility::PUBLIC_), $hooks[0]->getVisibility());
Expand All @@ -91,4 +93,39 @@ public function testPropertyHookAsymmetric(): void
),
), $hooks[1]->getArguments()[0]);
}

public function testVirtualProperty(): void
{
$file = __DIR__ . '/data/PHP84/PropertyHookVirtual.php';
$projectFactory = ProjectFactory::createInstance();
$project = $projectFactory->create('My project', [new LocalFile($file)]);

$class = $project->getFiles()[$file]->getClasses()['\PropertyHookVirtual'];

// Test get-only virtual property
$fullNameProperty = $class->getProperties()['\PropertyHookVirtual::$fullName'];
$this->assertTrue($fullNameProperty->isVirtual(), 'Property with getter that doesn\'t reference itself should be virtual');
$this->assertCount(1, $fullNameProperty->getHooks());
$this->assertEquals('get', $fullNameProperty->getHooks()[0]->getName());

// Test set-only virtual property
$compositeNameProperty = $class->getProperties()['\PropertyHookVirtual::$compositeName'];
$this->assertTrue($compositeNameProperty->isVirtual(), 'Property with setter that doesn\'t reference itself should be virtual');
$this->assertCount(1, $compositeNameProperty->getHooks());
$this->assertEquals('set', $compositeNameProperty->getHooks()[0]->getName());

// Test property with both get and set hooks that doesn't reference itself
$completeFullNameProperty = $class->getProperties()['\PropertyHookVirtual::$completeFullName'];
$this->assertTrue($completeFullNameProperty->isVirtual(), 'Property with getter and setter that don\'t reference itself should be virtual');
$this->assertCount(2, $completeFullNameProperty->getHooks());

$nonVirtualPropertyWithoutHooks = $class->getProperties()['\PropertyHookVirtual::$firstName'];
$this->assertFalse($nonVirtualPropertyWithoutHooks->isVirtual(), 'Property without hooks should not be virtual');
$this->assertCount(0, $nonVirtualPropertyWithoutHooks->getHooks());

// Test non-virtual property that references itself
$nonVirtualNameProperty = $class->getProperties()['\PropertyHookVirtual::$nonVirtualName'];
$this->assertFalse($nonVirtualNameProperty->isVirtual(), 'Property with hooks that reference itself should not be virtual');
$this->assertCount(2, $nonVirtualNameProperty->getHooks());
}
}
60 changes: 60 additions & 0 deletions tests/integration/data/PHP84/PropertyHookVirtual.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
<?php

declare(strict_types=1);

class PropertyHookVirtual
{
/**
* A virtual property that composes a full name from first and last name
*/
public string $fullName {
// This is a virtual property with a getter
// It doesn't reference $this->fullName
get {
return $this->firstName . ' ' . $this->lastName;
}
}

/**
* A virtual property that decomposes a full name into first and last name
*/
public string $compositeName {
// This is a virtual property with a setter
// It doesn't reference $this->compositeName
set(string $value) {
[$this->firstName, $this->lastName] = explode(' ', $value, 2);
}
}

/**
* A virtual property with both getter and setter
*/
public string $completeFullName {
// Getter doesn't reference $this->completeFullName
get {
return $this->firstName . ' ' . $this->lastName;
}
// Setter doesn't reference $this->completeFullName
set(string $value) {
[$this->firstName, $this->lastName] = explode(' ', $value, 2);
}
}

/**
* A non-virtual property that references itself in its hook
*/
public string $nonVirtualName {
get {
return $this->nonVirtualName ?? $this->firstName;
}
set(string $value) {
$this->nonVirtualName = $value;
}
}

public function __construct(
private string $firstName = 'John',
private string $lastName = 'Doe'
) {
}
}