Skip to content

Commit d91b327

Browse files
jaapiojaapio
authored
Merge pull request #672 from phpDocumentor/feature/virtual-detection
Find references to property if virtual
2 parents 6d99fb8 + a6fff8f commit d91b327

File tree

5 files changed

+194
-4
lines changed

5 files changed

+194
-4
lines changed

‎src/phpDocumentor/Reflection/NodeVisitor/FindingVisitor.php‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace phpDocumentor\Reflection\NodeVisitor;
6+
7+
use PhpParser\NodeVisitor\FirstFindingVisitor as BaseFindingVisitor;
8+
9+
final class FindingVisitor extends BaseFindingVisitor
10+
{
11+
public function __construct(callable $filterCallback)
12+
{
13+
parent::__construct($filterCallback);
14+
15+
$this->foundNode = null;
16+
}
17+
}

‎src/phpDocumentor/Reflection/Php/Factory/PropertyBuilder.php‎

Lines changed: 69 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use phpDocumentor\Reflection\DocBlockFactoryInterface;
88
use phpDocumentor\Reflection\Fqsen;
99
use phpDocumentor\Reflection\Location;
10+
use phpDocumentor\Reflection\NodeVisitor\FindingVisitor;
1011
use phpDocumentor\Reflection\Php\AsymmetricVisibility;
1112
use phpDocumentor\Reflection\Php\Factory\Reducer\Reducer;
1213
use phpDocumentor\Reflection\Php\Property as PropertyElement;
@@ -15,16 +16,21 @@
1516
use phpDocumentor\Reflection\Php\Visibility;
1617
use PhpParser\Comment\Doc;
1718
use PhpParser\Modifiers;
19+
use PhpParser\Node;
1820
use PhpParser\Node\ComplexType;
1921
use PhpParser\Node\Expr;
22+
use PhpParser\Node\Expr\PropertyFetch;
23+
use PhpParser\Node\Expr\Variable;
2024
use PhpParser\Node\Identifier;
2125
use PhpParser\Node\Name;
2226
use PhpParser\Node\Param;
2327
use PhpParser\Node\PropertyHook as PropertyHookNode;
28+
use PhpParser\NodeTraverser;
2429
use PhpParser\PrettyPrinter\Standard as PrettyPrinter;
2530

2631
use function array_filter;
2732
use function array_map;
33+
use function count;
2834
use function method_exists;
2935

3036
/**
@@ -141,6 +147,14 @@ public function hooks(array $hooks): self
141147

142148
public function build(ContextStack $context): PropertyElement
143149
{
150+
$hooks = array_filter(array_map(
151+
fn (PropertyHookNode $hook) => $this->buildHook($hook, $context, $this->visibility),
152+
$this->hooks,
153+
));
154+
155+
// Check if this is a virtual property by examining all hooks
156+
$isVirtual = $this->isVirtualProperty($this->hooks, $this->fqsen->getName());
157+
144158
return new PropertyElement(
145159
$this->fqsen,
146160
$this->visibility,
@@ -151,10 +165,8 @@ public function build(ContextStack $context): PropertyElement
151165
$this->endLocation,
152166
(new Type())->fromPhpParser($this->type),
153167
$this->readOnly,
154-
array_filter(array_map(
155-
fn (PropertyHookNode $hook) => $this->buildHook($hook, $context, $this->visibility),
156-
$this->hooks,
157-
)),
168+
$hooks,
169+
$isVirtual,
158170
);
159171
}
160172

@@ -264,6 +276,59 @@ private function buildHook(PropertyHookNode $hook, ContextStack $context, Visibi
264276
return $result;
265277
}
266278

279+
/**
280+
* Detects if a property is virtual by checking if any of its hooks reference the property itself.
281+
*
282+
* A virtual property is one where no defined hook references the property itself.
283+
* For example, in the 'get' hook, it doesn't use $this->propertyName.
284+
*
285+
* @param PropertyHookNode[] $hooks The property hooks to check
286+
* @param string $propertyName The name of the property
287+
*
288+
* @return bool True if the property is virtual, false otherwise
289+
*/
290+
private function isVirtualProperty(array $hooks, string $propertyName): bool
291+
{
292+
if (empty($hooks)) {
293+
return false;
294+
}
295+
296+
foreach ($hooks as $hook) {
297+
$stmts = $hook->getStmts();
298+
299+
if ($stmts === null || count($stmts) === 0) {
300+
continue;
301+
}
302+
303+
$finder = new FindingVisitor(
304+
static function (Node $node) use ($propertyName) {
305+
// Check if the node is a property fetch that references the property
306+
return $node instanceof PropertyFetch && $node->name instanceof Identifier &&
307+
$node->name->toString() === $propertyName &&
308+
$node->var instanceof Variable &&
309+
$node->var->name === 'this';
310+
},
311+
);
312+
313+
$traverser = new NodeTraverser($finder);
314+
$traverser->traverse($stmts);
315+
316+
if ($finder->getFoundNode() !== null) {
317+
return false;
318+
}
319+
}
320+
321+
return true;
322+
}
323+
324+
/**
325+
* Builds the hook visibility based on the hook name and property visibility.
326+
*
327+
* @param string $hookName The name of the hook ('get' or 'set')
328+
* @param Visibility $propertyVisibility The visibility of the property
329+
*
330+
* @return Visibility The appropriate visibility for the hook
331+
*/
267332
private function buildHookVisibility(string $hookName, Visibility $propertyVisibility): Visibility
268333
{
269334
if ($propertyVisibility instanceof AsymmetricVisibility === false) {

‎src/phpDocumentor/Reflection/Php/Property.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public function __construct(
5555
private readonly Type|null $type = null,
5656
private readonly bool $readOnly = false,
5757
private readonly array $hooks = [],
58+
private readonly bool $virtual = false,
5859
) {
5960
$this->visibility = $visibility ?: new Visibility('public');
6061
$this->location = $location ?: new Location(-1);
@@ -154,4 +155,14 @@ public function getHooks(): array
154155
{
155156
return $this->hooks;
156157
}
158+
159+
/**
160+
* Returns true when this property is virtual (not explicitly backed).
161+
*
162+
* A virtual property is one where no defined hook references the property itself.
163+
*/
164+
public function isVirtual(): bool
165+
{
166+
return $this->virtual;
167+
}
157168
}

‎tests/integration/PropertyHookTest.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ public function testPropertyHookWithDocblocks(): void
3333
$class = $project->getFiles()[$file]->getClasses()['\PropertyHook'];
3434
$hooks = $class->getProperties()['\PropertyHook::$example']->getHooks();
3535

36+
$this->assertTrue($class->getProperties()['\PropertyHook::$example']->isVirtual());
3637
$this->assertCount(2, $hooks);
3738
$this->assertEquals('get', $hooks[0]->getName());
3839
$this->assertEquals(new Visibility(Visibility::PUBLIC_), $hooks[0]->getVisibility());
@@ -73,6 +74,7 @@ public function testPropertyHookAsymmetric(): void
7374
),
7475
$class->getProperties()['\PropertyHook::$example']->getVisibility()
7576
);
77+
$this->assertTrue($class->getProperties()['\PropertyHook::$example']->isVirtual());
7678
$this->assertCount(2, $hooks);
7779
$this->assertEquals('get', $hooks[0]->getName());
7880
$this->assertEquals(new Visibility(Visibility::PUBLIC_), $hooks[0]->getVisibility());
@@ -91,4 +93,39 @@ public function testPropertyHookAsymmetric(): void
9193
),
9294
), $hooks[1]->getArguments()[0]);
9395
}
96+
97+
public function testVirtualProperty(): void
98+
{
99+
$file = __DIR__ . '/data/PHP84/PropertyHookVirtual.php';
100+
$projectFactory = ProjectFactory::createInstance();
101+
$project = $projectFactory->create('My project', [new LocalFile($file)]);
102+
103+
$class = $project->getFiles()[$file]->getClasses()['\PropertyHookVirtual'];
104+
105+
// Test get-only virtual property
106+
$fullNameProperty = $class->getProperties()['\PropertyHookVirtual::$fullName'];
107+
$this->assertTrue($fullNameProperty->isVirtual(), 'Property with getter that doesn\'t reference itself should be virtual');
108+
$this->assertCount(1, $fullNameProperty->getHooks());
109+
$this->assertEquals('get', $fullNameProperty->getHooks()[0]->getName());
110+
111+
// Test set-only virtual property
112+
$compositeNameProperty = $class->getProperties()['\PropertyHookVirtual::$compositeName'];
113+
$this->assertTrue($compositeNameProperty->isVirtual(), 'Property with setter that doesn\'t reference itself should be virtual');
114+
$this->assertCount(1, $compositeNameProperty->getHooks());
115+
$this->assertEquals('set', $compositeNameProperty->getHooks()[0]->getName());
116+
117+
// Test property with both get and set hooks that doesn't reference itself
118+
$completeFullNameProperty = $class->getProperties()['\PropertyHookVirtual::$completeFullName'];
119+
$this->assertTrue($completeFullNameProperty->isVirtual(), 'Property with getter and setter that don\'t reference itself should be virtual');
120+
$this->assertCount(2, $completeFullNameProperty->getHooks());
121+
122+
$nonVirtualPropertyWithoutHooks = $class->getProperties()['\PropertyHookVirtual::$firstName'];
123+
$this->assertFalse($nonVirtualPropertyWithoutHooks->isVirtual(), 'Property without hooks should not be virtual');
124+
$this->assertCount(0, $nonVirtualPropertyWithoutHooks->getHooks());
125+
126+
// Test non-virtual property that references itself
127+
$nonVirtualNameProperty = $class->getProperties()['\PropertyHookVirtual::$nonVirtualName'];
128+
$this->assertFalse($nonVirtualNameProperty->isVirtual(), 'Property with hooks that reference itself should not be virtual');
129+
$this->assertCount(2, $nonVirtualNameProperty->getHooks());
130+
}
94131
}

‎tests/integration/data/PHP84/PropertyHookVirtual.php‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
class PropertyHookVirtual
6+
{
7+
/**
8+
* A virtual property that composes a full name from first and last name
9+
*/
10+
public string $fullName {
11+
// This is a virtual property with a getter
12+
// It doesn't reference $this->fullName
13+
get {
14+
return $this->firstName . ' ' . $this->lastName;
15+
}
16+
}
17+
18+
/**
19+
* A virtual property that decomposes a full name into first and last name
20+
*/
21+
public string $compositeName {
22+
// This is a virtual property with a setter
23+
// It doesn't reference $this->compositeName
24+
set(string $value) {
25+
[$this->firstName, $this->lastName] = explode(' ', $value, 2);
26+
}
27+
}
28+
29+
/**
30+
* A virtual property with both getter and setter
31+
*/
32+
public string $completeFullName {
33+
// Getter doesn't reference $this->completeFullName
34+
get {
35+
return $this->firstName . ' ' . $this->lastName;
36+
}
37+
// Setter doesn't reference $this->completeFullName
38+
set(string $value) {
39+
[$this->firstName, $this->lastName] = explode(' ', $value, 2);
40+
}
41+
}
42+
43+
/**
44+
* A non-virtual property that references itself in its hook
45+
*/
46+
public string $nonVirtualName {
47+
get {
48+
return $this->nonVirtualName ?? $this->firstName;
49+
}
50+
set(string $value) {
51+
$this->nonVirtualName = $value;
52+
}
53+
}
54+
55+
public function __construct(
56+
private string $firstName = 'John',
57+
private string $lastName = 'Doe'
58+
) {
59+
}
60+
}

0 commit comments

Comments
 (0)