Skip to content

Commit 0d4a5fe

Browse files
committed
Enable validating private properties of parent classes
All validators related to object properties only consider the properties of the object being validated, not those of its parent object. That's because PHP reflection only gets the properties visible to the current class (public or protected). The problem with that is that when there's a private property in the parent class, we're completely unaware of it. This commit will modify those rules by retrieving properties from the parent class, ensuring we capture all properties that require validation.
1 parent 16c2a75 commit 0d4a5fe

File tree

7 files changed

+109
-34
lines changed

7 files changed

+109
-34
lines changed

library/Rules/Attributes.php

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111

1212
use Attribute;
1313
use ReflectionAttribute;
14+
use ReflectionClass;
1415
use ReflectionObject;
16+
use ReflectionProperty;
1517
use Respect\Validation\Id;
1618
use Respect\Validation\Result;
1719
use Respect\Validation\Rule;
@@ -28,32 +30,65 @@ public function evaluate(mixed $input): Result
2830
return $objectType->withId($id);
2931
}
3032

31-
$rules = [];
3233
$reflection = new ReflectionObject($input);
33-
foreach ($reflection->getAttributes(Rule::class, ReflectionAttribute::IS_INSTANCEOF) as $attribute) {
34-
$rules[] = $attribute->newInstance();
34+
$rules = [...$this->getClassRules($reflection), ...$this->getPropertyRules($reflection)];
35+
if ($rules === []) {
36+
return (new AlwaysValid())->evaluate($input)->withId($id);
37+
}
38+
39+
return (new Reducer(...$rules))->evaluate($input)->withId($id);
40+
}
41+
42+
/** @return array<Rule> */
43+
private function getClassRules(ReflectionObject $reflection): array
44+
{
45+
$rules = [];
46+
while ($reflection instanceof ReflectionClass) {
47+
foreach ($reflection->getAttributes(Rule::class, ReflectionAttribute::IS_INSTANCEOF) as $attribute) {
48+
$rules[] = $attribute->newInstance();
49+
}
50+
51+
$reflection = $reflection->getParentClass();
3552
}
3653

37-
foreach ($reflection->getProperties() as $property) {
38-
$childrenRules = [];
54+
return $rules;
55+
}
56+
57+
/** @return array<Rule> */
58+
private function getPropertyRules(ReflectionObject $reflection): array
59+
{
60+
$rules = [];
61+
foreach ($this->getProperties($reflection) as $propertyName => $property) {
62+
$propertyRules = [];
3963
foreach ($property->getAttributes(Rule::class, ReflectionAttribute::IS_INSTANCEOF) as $attribute) {
40-
$childrenRules[] = $attribute->newInstance();
64+
$propertyRules[] = $attribute->newInstance();
4165
}
4266

43-
if ($childrenRules === []) {
67+
if ($propertyRules === []) {
4468
continue;
4569
}
4670

4771
$allowsNull = $property->getType()?->allowsNull() ?? false;
4872

49-
$childRule = new Reducer(...$childrenRules);
50-
$rules[] = new Property($property->getName(), $allowsNull ? new NullOr($childRule) : $childRule);
73+
$childRule = new Reducer(...$propertyRules);
74+
$rules[] = new Property($propertyName, $allowsNull ? new NullOr($childRule) : $childRule);
5175
}
5276

53-
if ($rules === []) {
54-
return (new AlwaysValid())->evaluate($input)->withId($id);
77+
return $rules;
78+
}
79+
80+
/** @return array<ReflectionProperty> */
81+
private function getProperties(ReflectionObject $reflection): array
82+
{
83+
$properties = [];
84+
while ($reflection instanceof ReflectionClass) {
85+
foreach ($reflection->getProperties() as $property) {
86+
$properties[$property->name] = $property;
87+
}
88+
89+
$reflection = $reflection->getParentClass();
5590
}
5691

57-
return (new Reducer(...$rules))->evaluate($input)->withId($id);
92+
return $properties;
5893
}
5994
}

library/Rules/Property.php

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
namespace Respect\Validation\Rules;
1111

1212
use Attribute;
13+
use ReflectionClass;
1314
use ReflectionObject;
1415
use Respect\Validation\Path;
1516
use Respect\Validation\Result;
@@ -34,15 +35,23 @@ public function evaluate(mixed $input): Result
3435
}
3536

3637
return $this->rule
37-
->evaluate($this->extractPropertyValue($input, $this->propertyName))
38+
->evaluate($this->getPropertyValue($input, $this->propertyName))
3839
->withPath(new Path($this->propertyName));
3940
}
4041

41-
private function extractPropertyValue(object $input, string $property): mixed
42+
private function getPropertyValue(object $object, string $propertyName): mixed
4243
{
43-
$reflectionObject = new ReflectionObject($input);
44-
$reflectionProperty = $reflectionObject->getProperty($property);
44+
$reflection = new ReflectionObject($object);
45+
while ($reflection instanceof ReflectionClass) {
46+
if ($reflection->hasProperty($propertyName)) {
47+
$property = $reflection->getProperty($propertyName);
4548

46-
return $reflectionProperty->isInitialized($input) ? $reflectionProperty->getValue($input) : null;
49+
return $property->isInitialized($object) ? $property->getValue($object) : null;
50+
}
51+
52+
$reflection = $reflection->getParentClass();
53+
}
54+
55+
return null;
4756
}
4857
}

library/Rules/PropertyExists.php

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
namespace Respect\Validation\Rules;
1111

1212
use Attribute;
13+
use ReflectionClass;
1314
use ReflectionObject;
1415
use Respect\Validation\Message\Template;
1516
use Respect\Validation\Path;
@@ -33,19 +34,24 @@ public function __construct(
3334
public function evaluate(mixed $input): Result
3435
{
3536
return new Result(
36-
$this->hasProperty($input),
37+
is_object($input) && $this->hasProperty($input),
3738
$input,
3839
$this,
3940
path: new Path($this->propertyName),
4041
);
4142
}
4243

43-
private function hasProperty(mixed $input): bool
44+
private function hasProperty(object $object): bool
4445
{
45-
if (!is_object($input)) {
46-
return false;
46+
$reflection = new ReflectionObject($object);
47+
while ($reflection instanceof ReflectionClass) {
48+
if ($reflection->hasProperty($this->propertyName)) {
49+
return true;
50+
}
51+
52+
$reflection = $reflection->getParentClass();
4753
}
4854

49-
return (new ReflectionObject($input))->hasProperty($this->propertyName);
55+
return false;
5056
}
5157
}

phpstan.neon.dist

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ parameters:
2121
path: tests/unit/Message/Translator/ArrayTranslatorTest.php
2222
- message: '/Access to an undefined property PHPUnit\\Framework\\TestCase/'
2323
path: tests/feature/Rules/SizeTest.php
24+
- message: '/Property Respect\\Validation\\Test\\Stubs\\.+::\$[a-zA-Z]+ is never read, only written./'
25+
path: tests/library/Stubs
2426
level: 8
2527
treatPhpDocTypesAsCertain: false
2628
paths:

tests/feature/Rules/AttributesTest.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -46,20 +46,20 @@
4646
fn(string $message, string $fullMessage, array $messages) => expect()
4747
->and($message)->toBe('`.name` must not be empty')
4848
->and($fullMessage)->toBe(<<<'FULL_MESSAGE'
49-
- `Respect\Validation\Test\Stubs\WithAttributes { +$name="" +$birthdate="not a date" +$email="not an email" +$phone ... }` must pass the rules
49+
- `Respect\Validation\Test\Stubs\WithAttributes { +$name="" +$birthdate="not a date" #$phone="not a phone number" + ... }` must pass the rules
5050
- `.name` must not be empty
5151
- `.birthdate` must pass all the rules
5252
- `.birthdate` must be a valid date in the format "2005-12-30"
5353
- For comparison with now, `.birthdate` must be a valid datetime
54-
- `.email` must be a valid email address or must be null
5554
- `.phone` must be a valid telephone number or must be null
55+
- `.email` must be a valid email address or must be null
5656
FULL_MESSAGE)
5757
->and($messages)->toBe([
58-
'__root__' => '`Respect\Validation\Test\Stubs\WithAttributes { +$name="" +$birthdate="not a date" +$email="not an email" +$phone ... }` must pass the rules',
58+
'__root__' => '`Respect\Validation\Test\Stubs\WithAttributes { +$name="" +$birthdate="not a date" #$phone="not a phone number" + ... }` must pass the rules',
5959
'name' => '`.name` must not be empty',
6060
'birthdate' => 'For comparison with now, `.birthdate` must be a valid datetime',
61-
'email' => '`.email` must be a valid email address or must be null',
6261
'phone' => '`.phone` must be a valid telephone number or must be null',
62+
'email' => '`.email` must be a valid email address or must be null',
6363
]),
6464
));
6565

@@ -68,13 +68,13 @@
6868
fn(string $message, string $fullMessage, array $messages) => expect()
6969
->and($message)->toBe('`.email` must be defined')
7070
->and($fullMessage)->toBe(<<<'FULL_MESSAGE'
71-
- `Respect\Validation\Test\Stubs\WithAttributes { +$name="John Doe" +$birthdate="2024-06-23" +$email=null +$phone=n ... }` must pass at least one of the rules
71+
- `Respect\Validation\Test\Stubs\WithAttributes { +$name="John Doe" +$birthdate="2024-06-23" #$phone=null +$address ... }` must pass at least one of the rules
7272
- `.email` must be defined
7373
- `.phone` must be defined
7474
FULL_MESSAGE)
7575
->and($messages)->toBe([
7676
'anyOf' => [
77-
'__root__' => '`Respect\Validation\Test\Stubs\WithAttributes { +$name="John Doe" +$birthdate="2024-06-23" +$email=null +$phone=n ... }` must pass at least one of the rules',
77+
'__root__' => '`Respect\Validation\Test\Stubs\WithAttributes { +$name="John Doe" +$birthdate="2024-06-23" #$phone=null +$address ... }` must pass at least one of the rules',
7878
'email' => '`.email` must be defined',
7979
'phone' => '`.phone` must be defined',
8080
],
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php
2+
3+
/*
4+
* Copyright (c) Alexandre Gomes Gaigalas <[email protected]>
5+
* SPDX-License-Identifier: MIT
6+
*/
7+
8+
declare(strict_types=1);
9+
10+
namespace Respect\Validation\Test\Stubs;
11+
12+
use Respect\Validation\Rules as Rule;
13+
14+
abstract class ParentWithAttributes
15+
{
16+
public function __construct(
17+
#[Rule\Email]
18+
private string|null $email = null,
19+
#[Rule\Phone]
20+
protected string|null $phone = null,
21+
public string|null $address = null,
22+
) {
23+
}
24+
}

tests/library/Stubs/WithAttributes.php

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,19 +15,18 @@
1515
new Rule\Property('email', new Rule\NotUndef()),
1616
new Rule\Property('phone', new Rule\NotUndef()),
1717
)]
18-
final class WithAttributes
18+
final class WithAttributes extends ParentWithAttributes
1919
{
2020
public function __construct(
2121
#[Rule\NotEmpty]
2222
public string $name,
2323
#[Rule\Date('Y-m-d')]
2424
#[Rule\DateTimeDiff('years', new Rule\LessThanOrEqual(25))]
2525
public string $birthdate,
26-
#[Rule\Email]
27-
public string|null $email = null,
28-
#[Rule\Phone]
29-
public string|null $phone = null,
30-
public string|null $address = null,
26+
string|null $email = null,
27+
string|null $phone = null,
28+
string|null $address = null,
3129
) {
30+
parent::__construct($email, $phone, $address);
3231
}
3332
}

0 commit comments

Comments
 (0)