Skip to content

Commit 12c1aee

Browse files
zonuexezonuexe
committed
Add stringable access check to AccessPropertiesCheck
1 parent 5ab9acc commit 12c1aee

File tree

5 files changed

+184
-3
lines changed

5 files changed

+184
-3
lines changed

‎src/Rules/Properties/AccessPropertiesCheck.php

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ public function __construct(
4040
private bool $reportMagicProperties,
4141
#[AutowiredParameter]
4242
private bool $checkDynamicProperties,
43+
#[AutowiredParameter(ref: '%featureToggles.checkNonStringableDynamicAccess%')]
44+
private bool $checkNonStringableDynamicAccess,
4345
)
4446
{
4547
}
@@ -49,13 +51,30 @@ public function __construct(
4951
*/
5052
public function check(PropertyFetch $node, Scope $scope, bool $write): array
5153
{
54+
$errors = [];
5255
if ($node->name instanceof Identifier) {
5356
$names = [$node->name->name];
5457
} else {
5558
$names = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $scope->getType($node->name)->getConstantStrings());
59+
60+
if (!$write && $this->checkNonStringableDynamicAccess) {
61+
$nameTypeResult = $this->ruleLevelHelper->findTypeToCheck(
62+
$scope,
63+
$node->name,
64+
'',
65+
static fn (Type $type) => $type->toString()->isString()->yes(),
66+
);
67+
$nameType = $nameTypeResult->getType();
68+
if ($nameType instanceof ErrorType || $nameType->toString() instanceof ErrorType || !$nameType->toString()->isString()->yes()) {
69+
$originalNameType = $scope->getType($node->name);
70+
$className = $scope->getType($node->var)->describe(VerbosityLevel::typeOnly());
71+
$errors[] = RuleErrorBuilder::message(sprintf('Property name for %s must be a string, but %s was given.', $className, $originalNameType->describe(VerbosityLevel::precise())))
72+
->identifier('property.nameNotString')
73+
->build();
74+
}
75+
}
5676
}
5777

58-
$errors = [];
5978
foreach ($names as $name) {
6079
$errors = array_merge($errors, $this->processSingleProperty($scope, $node, $name, $write));
6180
}

‎tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ protected function getRule(): Rule
1919
{
2020
$reflectionProvider = self::createReflectionProvider();
2121
return new AccessPropertiesInAssignRule(
22-
new AccessPropertiesCheck($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new PhpVersion(PHP_VERSION_ID), true, true),
22+
new AccessPropertiesCheck($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new PhpVersion(PHP_VERSION_ID), true, true, true),
2323
);
2424
}
2525

@@ -96,6 +96,21 @@ public function testBug4492(): void
9696
$this->analyse([__DIR__ . '/data/bug-4492.php'], []);
9797
}
9898

99+
public function testDynamicStringableAccess(): void
100+
{
101+
// All warnings are reported by the AccessPropertiesRule.
102+
// The AccessPropertiesInAssignRule does not report any warnings.
103+
$this->analyse([__DIR__ . '/data/dynamic-stringable-access.php'], []);
104+
}
105+
106+
#[RequiresPhp('>= 8.0')]
107+
public function testDynamicStringableNullsafeAccess(): void
108+
{
109+
// All warnings are reported by the AccessPropertiesRule.
110+
// The AccessPropertiesInAssignRule does not report any warnings.
111+
$this->analyse([__DIR__ . '/data/dynamic-stringable-nullsafe-access.php'], []);
112+
}
113+
99114
public function testObjectShapes(): void
100115
{
101116
$tipText = 'Learn more: <fg=cyan>https://phpstan.org/blog/solving-phpstan-access-to-undefined-property</>';

‎tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class AccessPropertiesRuleTest extends RuleTestCase
2626
protected function getRule(): Rule
2727
{
2828
$reflectionProvider = self::createReflectionProvider();
29-
return new AccessPropertiesRule(new AccessPropertiesCheck($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, $this->checkThisOnly, $this->checkUnionTypes, false, false, false, true), new PhpVersion(PHP_VERSION_ID), true, $this->checkDynamicProperties));
29+
return new AccessPropertiesRule(new AccessPropertiesCheck($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, $this->checkThisOnly, $this->checkUnionTypes, false, false, false, true), new PhpVersion(PHP_VERSION_ID), true, $this->checkDynamicProperties, true));
3030
}
3131

3232
public function testAccessProperties(): void
@@ -973,6 +973,84 @@ public function testBug8629(): void
973973
$this->analyse([__DIR__ . '/data/bug-8629.php'], []);
974974
}
975975

976+
public function testDynamicStringableAccess(): void
977+
{
978+
$this->checkThisOnly = false;
979+
$this->checkUnionTypes = false;
980+
$this->checkDynamicProperties = false;
981+
$this->analyse([__DIR__ . '/data/dynamic-stringable-access.php'], [
982+
// DynamicStringableAccess\Foo::testProperties()
983+
[
984+
'Property name for $this(DynamicStringableAccess\Foo) must be a string, but $this(DynamicStringableAccess\Foo) was given.',
985+
13,
986+
],
987+
[
988+
'Property name for DynamicStringableAccess\Foo must be a string, but $this(DynamicStringableAccess\Foo) was given.',
989+
14,
990+
],
991+
[
992+
'Property name for $this(DynamicStringableAccess\Foo) must be a string, but $this(DynamicStringableAccess\Foo) was given.',
993+
15,
994+
],
995+
[
996+
'Property name for $this(DynamicStringableAccess\Foo) must be a string, but $this(DynamicStringableAccess\Foo) was given.',
997+
16,
998+
],
999+
[
1000+
'Property name for $this(DynamicStringableAccess\Foo) must be a string, but object was given.',
1001+
17,
1002+
],
1003+
[
1004+
'Property name for $this(DynamicStringableAccess\Foo) must be a string, but array was given.',
1005+
18,
1006+
],
1007+
// DynamicStringableAccess\Foo::testPropertyAssignments()
1008+
[
1009+
'Property name for $this(DynamicStringableAccess\Foo) must be a string, but $this(DynamicStringableAccess\Foo) was given.',
1010+
30,
1011+
],
1012+
[
1013+
'Property name for DynamicStringableAccess\Foo must be a string, but $this(DynamicStringableAccess\Foo) was given.',
1014+
31,
1015+
],
1016+
[
1017+
'Property name for $this(DynamicStringableAccess\Foo) must be a string, but object was given.',
1018+
32,
1019+
],
1020+
]);
1021+
}
1022+
1023+
#[RequiresPhp('>= 8.0')]
1024+
public function testDynamicStringableNullsafeAccess(): void
1025+
{
1026+
$this->checkThisOnly = false;
1027+
$this->checkUnionTypes = false;
1028+
$this->checkDynamicProperties = false;
1029+
$this->analyse([__DIR__ . '/data/dynamic-stringable-nullsafe-access.php'], [
1030+
// DynamicStringableNullsafeAccess\Foo::testNullsafePropertyFetch()
1031+
[
1032+
'Property name for $this(DynamicStringableNullsafeAccess\Foo) must be a string, but $this(DynamicStringableNullsafeAccess\Foo) was given.',
1033+
13,
1034+
],
1035+
[
1036+
'Property name for DynamicStringableNullsafeAccess\Foo must be a string, but $this(DynamicStringableNullsafeAccess\Foo) was given.',
1037+
14,
1038+
],
1039+
[
1040+
'Property name for $this(DynamicStringableNullsafeAccess\Foo) must be a string, but $this(DynamicStringableNullsafeAccess\Foo) was given.',
1041+
15,
1042+
],
1043+
[
1044+
'Property name for $this(DynamicStringableNullsafeAccess\Foo) must be a string, but $this(DynamicStringableNullsafeAccess\Foo) was given.',
1045+
16,
1046+
],
1047+
[
1048+
'Property name for $this(DynamicStringableNullsafeAccess\Foo) must be a string, but object was given.',
1049+
17,
1050+
],
1051+
]);
1052+
}
1053+
9761054
#[RequiresPhp('>= 8.0')]
9771055
public function testBug9694(): void
9781056
{

‎tests/PHPStan/Rules/Properties/data/dynamic-stringable-access.php

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace DynamicStringableAccess;
4+
5+
use Stringable;
6+
7+
final class Foo
8+
{
9+
private self $var;
10+
11+
public function testProperties(string $name, Stringable $stringable, object $object, array $array): void
12+
{
13+
echo $this->{$this}->name;
14+
echo $this->var->$this;
15+
echo $this->$this->$name;
16+
echo $this->$this->name;
17+
echo $this->$object;
18+
echo $this->$array;
19+
20+
echo $this->$name; // valid
21+
echo $this->$stringable; // valid
22+
echo $this->{1111}; // valid
23+
echo $this->{true}; // valid
24+
echo $this->{false}; // valid
25+
echo $this->{null}; // valid
26+
}
27+
28+
public function testPropertyAssignments(string $name, Stringable $stringable, object $object): void
29+
{
30+
$this->{$this} = $name;
31+
$this->var->{$this} = $name;
32+
$this->$object = $name;
33+
34+
$this->$name = $name; // valid
35+
$this->$stringable = $name; // valid
36+
$this->{1111} = $name; // valid
37+
$this->{true} = $name; // valid
38+
$this->{false} = $name; // valid
39+
$this->{null} = $name; // valid
40+
}
41+
42+
}

‎tests/PHPStan/Rules/Properties/data/dynamic-stringable-nullsafe-access.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php declare(strict_types = 1); // lint >= 8.0
2+
3+
namespace DynamicStringableNullsafeAccess;
4+
5+
use Stringable;
6+
7+
final class Foo
8+
{
9+
private self $var;
10+
11+
public function testNullsafePropertyFetch(string $name, Stringable $stringable, object $object): void
12+
{
13+
echo $this?->{$this}?->name;
14+
echo $this?->var?->$this;
15+
echo $this?->$this?->$name;
16+
echo $this?->$this?->name;
17+
echo $this?->$object;
18+
19+
echo $this?->$name; // valid
20+
echo $this?->$stringable; // valid
21+
echo $this?->{1111}; // valid
22+
echo $this?->{true}; // valid
23+
echo $this?->{false}; // valid
24+
echo $this?->{null}; // valid
25+
}
26+
27+
}

0 commit comments

Comments
 (0)