Skip to content

Commit ed35166

Browse files
committed
Add stringable access check to AccessPropertiesCheck
1 parent 5ab9acc commit ed35166

File tree

4 files changed

+108
-3
lines changed

4 files changed

+108
-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: 8 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,13 @@ public function testBug4492(): void
9696
$this->analyse([__DIR__ . '/data/bug-4492.php'], []);
9797
}
9898

99+
public function testBug9475(): void
100+
{
101+
// All warnings are reported by the AccessPropertiesRule.
102+
// The AccessPropertiesInAssignRule does not report any warnings.
103+
$this->analyse([__DIR__ . '/data/bug-9475.php'], []);
104+
}
105+
99106
public function testObjectShapes(): void
100107
{
101108
$tipText = 'Learn more: <fg=cyan>https://phpstan.org/blog/solving-phpstan-access-to-undefined-property</>';

tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php

Lines changed: 46 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,51 @@ public function testBug8629(): void
973973
$this->analyse([__DIR__ . '/data/bug-8629.php'], []);
974974
}
975975

976+
public function testBug9475(): void
977+
{
978+
$this->checkThisOnly = false;
979+
$this->checkUnionTypes = false;
980+
$this->checkDynamicProperties = false;
981+
$this->analyse([__DIR__ . '/data/bug-9475.php'], [
982+
[
983+
'Property name for $this(Bug9475\Properties) must be a string, but $this(Bug9475\Properties) was given.',
984+
13,
985+
],
986+
[
987+
'Property name for Bug9475\Properties must be a string, but $this(Bug9475\Properties) was given.',
988+
14,
989+
],
990+
[
991+
'Property name for $this(Bug9475\Properties) must be a string, but $this(Bug9475\Properties) was given.',
992+
15,
993+
],
994+
[
995+
'Property name for $this(Bug9475\Properties) must be a string, but $this(Bug9475\Properties) was given.',
996+
16,
997+
],
998+
[
999+
'Property name for $this(Bug9475\Properties) must be a string, but object was given.',
1000+
17,
1001+
],
1002+
[
1003+
'Property name for $this(Bug9475\Properties) must be a string, but array was given.',
1004+
18,
1005+
],
1006+
[
1007+
'Property name for $this(Bug9475\Properties) must be a string, but $this(Bug9475\Properties) was given.',
1008+
26,
1009+
],
1010+
[
1011+
'Property name for Bug9475\Properties must be a string, but $this(Bug9475\Properties) was given.',
1012+
27,
1013+
],
1014+
[
1015+
'Property name for $this(Bug9475\Properties) must be a string, but object was given.',
1016+
28,
1017+
],
1018+
]);
1019+
}
1020+
9761021
#[RequiresPhp('>= 8.0')]
9771022
public function testBug9694(): void
9781023
{
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace Bug9475;
4+
5+
use Stringable;
6+
7+
final class Properties
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+
}
23+
24+
public function testPropertyAssignments(string $name, Stringable $stringable, object $object): void
25+
{
26+
$this->{$this} = $name;
27+
$this->var->{$this} = $name;
28+
$this->$object = $name;
29+
30+
$this->$name = $name; // valid
31+
$this->$stringable = $name; // valid
32+
}
33+
34+
}

0 commit comments

Comments
 (0)