diff --git a/src/Rules/Properties/AccessPropertiesCheck.php b/src/Rules/Properties/AccessPropertiesCheck.php index a92c8ba5dd..09453458d3 100644 --- a/src/Rules/Properties/AccessPropertiesCheck.php +++ b/src/Rules/Properties/AccessPropertiesCheck.php @@ -40,6 +40,8 @@ public function __construct( private bool $reportMagicProperties, #[AutowiredParameter] private bool $checkDynamicProperties, + #[AutowiredParameter(ref: '%featureToggles.checkNonStringableDynamicAccess%')] + private bool $checkNonStringableDynamicAccess, ) { } @@ -49,13 +51,30 @@ public function __construct( */ public function check(PropertyFetch $node, Scope $scope, bool $write): array { + $errors = []; if ($node->name instanceof Identifier) { $names = [$node->name->name]; } else { $names = array_map(static fn (ConstantStringType $type): string => $type->getValue(), $scope->getType($node->name)->getConstantStrings()); + + if (!$write && $this->checkNonStringableDynamicAccess) { + $nameTypeResult = $this->ruleLevelHelper->findTypeToCheck( + $scope, + $node->name, + '', + static fn (Type $type) => $type->toString()->isString()->yes(), + ); + $nameType = $nameTypeResult->getType(); + if ($nameType instanceof ErrorType || $nameType->toString() instanceof ErrorType || !$nameType->toString()->isString()->yes()) { + $originalNameType = $scope->getType($node->name); + $className = $scope->getType($node->var)->describe(VerbosityLevel::typeOnly()); + $errors[] = RuleErrorBuilder::message(sprintf('Property name for %s must be a string, but %s was given.', $className, $originalNameType->describe(VerbosityLevel::precise()))) + ->identifier('property.nameNotString') + ->build(); + } + } } - $errors = []; foreach ($names as $name) { $errors = array_merge($errors, $this->processSingleProperty($scope, $node, $name, $write)); } diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php index c89cf2867b..73ea69e5fd 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesInAssignRuleTest.php @@ -19,7 +19,7 @@ protected function getRule(): Rule { $reflectionProvider = self::createReflectionProvider(); return new AccessPropertiesInAssignRule( - new AccessPropertiesCheck($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new PhpVersion(PHP_VERSION_ID), true, true), + new AccessPropertiesCheck($reflectionProvider, new RuleLevelHelper($reflectionProvider, true, false, true, false, false, false, true), new PhpVersion(PHP_VERSION_ID), true, true, true), ); } @@ -96,6 +96,21 @@ public function testBug4492(): void $this->analyse([__DIR__ . '/data/bug-4492.php'], []); } + public function testDynamicStringableAccess(): void + { + // All warnings are reported by the AccessPropertiesRule. + // The AccessPropertiesInAssignRule does not report any warnings. + $this->analyse([__DIR__ . '/data/dynamic-stringable-access.php'], []); + } + + #[RequiresPhp('>= 8.0')] + public function testDynamicStringableNullsafeAccess(): void + { + // All warnings are reported by the AccessPropertiesRule. + // The AccessPropertiesInAssignRule does not report any warnings. + $this->analyse([__DIR__ . '/data/dynamic-stringable-nullsafe-access.php'], []); + } + public function testObjectShapes(): void { $tipText = 'Learn more: https://phpstan.org/blog/solving-phpstan-access-to-undefined-property'; diff --git a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php index 5d3697767d..39c4948c24 100644 --- a/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php +++ b/tests/PHPStan/Rules/Properties/AccessPropertiesRuleTest.php @@ -26,7 +26,7 @@ class AccessPropertiesRuleTest extends RuleTestCase protected function getRule(): Rule { $reflectionProvider = self::createReflectionProvider(); - 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)); + 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)); } public function testAccessProperties(): void @@ -973,6 +973,84 @@ public function testBug8629(): void $this->analyse([__DIR__ . '/data/bug-8629.php'], []); } + public function testDynamicStringableAccess(): void + { + $this->checkThisOnly = false; + $this->checkUnionTypes = false; + $this->checkDynamicProperties = false; + $this->analyse([__DIR__ . '/data/dynamic-stringable-access.php'], [ + // DynamicStringableAccess\Foo::testProperties() + [ + 'Property name for $this(DynamicStringableAccess\Foo) must be a string, but $this(DynamicStringableAccess\Foo) was given.', + 13, + ], + [ + 'Property name for DynamicStringableAccess\Foo must be a string, but $this(DynamicStringableAccess\Foo) was given.', + 14, + ], + [ + 'Property name for $this(DynamicStringableAccess\Foo) must be a string, but $this(DynamicStringableAccess\Foo) was given.', + 15, + ], + [ + 'Property name for $this(DynamicStringableAccess\Foo) must be a string, but $this(DynamicStringableAccess\Foo) was given.', + 16, + ], + [ + 'Property name for $this(DynamicStringableAccess\Foo) must be a string, but object was given.', + 17, + ], + [ + 'Property name for $this(DynamicStringableAccess\Foo) must be a string, but array was given.', + 18, + ], + // DynamicStringableAccess\Foo::testPropertyAssignments() + [ + 'Property name for $this(DynamicStringableAccess\Foo) must be a string, but $this(DynamicStringableAccess\Foo) was given.', + 30, + ], + [ + 'Property name for DynamicStringableAccess\Foo must be a string, but $this(DynamicStringableAccess\Foo) was given.', + 31, + ], + [ + 'Property name for $this(DynamicStringableAccess\Foo) must be a string, but object was given.', + 32, + ], + ]); + } + + #[RequiresPhp('>= 8.0')] + public function testDynamicStringableNullsafeAccess(): void + { + $this->checkThisOnly = false; + $this->checkUnionTypes = false; + $this->checkDynamicProperties = false; + $this->analyse([__DIR__ . '/data/dynamic-stringable-nullsafe-access.php'], [ + // DynamicStringableNullsafeAccess\Foo::testNullsafePropertyFetch() + [ + 'Property name for $this(DynamicStringableNullsafeAccess\Foo) must be a string, but $this(DynamicStringableNullsafeAccess\Foo) was given.', + 13, + ], + [ + 'Property name for DynamicStringableNullsafeAccess\Foo must be a string, but $this(DynamicStringableNullsafeAccess\Foo) was given.', + 14, + ], + [ + 'Property name for $this(DynamicStringableNullsafeAccess\Foo) must be a string, but $this(DynamicStringableNullsafeAccess\Foo) was given.', + 15, + ], + [ + 'Property name for $this(DynamicStringableNullsafeAccess\Foo) must be a string, but $this(DynamicStringableNullsafeAccess\Foo) was given.', + 16, + ], + [ + 'Property name for $this(DynamicStringableNullsafeAccess\Foo) must be a string, but object was given.', + 17, + ], + ]); + } + #[RequiresPhp('>= 8.0')] public function testBug9694(): void { diff --git a/tests/PHPStan/Rules/Properties/data/dynamic-stringable-access.php b/tests/PHPStan/Rules/Properties/data/dynamic-stringable-access.php new file mode 100644 index 0000000000..ed31e5ed12 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/dynamic-stringable-access.php @@ -0,0 +1,42 @@ +{$this}->name; + echo $this->var->$this; + echo $this->$this->$name; + echo $this->$this->name; + echo $this->$object; + echo $this->$array; + + echo $this->$name; // valid + echo $this->$stringable; // valid + echo $this->{1111}; // valid + echo $this->{true}; // valid + echo $this->{false}; // valid + echo $this->{null}; // valid + } + + public function testPropertyAssignments(string $name, Stringable $stringable, object $object): void + { + $this->{$this} = $name; + $this->var->{$this} = $name; + $this->$object = $name; + + $this->$name = $name; // valid + $this->$stringable = $name; // valid + $this->{1111} = $name; // valid + $this->{true} = $name; // valid + $this->{false} = $name; // valid + $this->{null} = $name; // valid + } + +} diff --git a/tests/PHPStan/Rules/Properties/data/dynamic-stringable-nullsafe-access.php b/tests/PHPStan/Rules/Properties/data/dynamic-stringable-nullsafe-access.php new file mode 100644 index 0000000000..3977592c31 --- /dev/null +++ b/tests/PHPStan/Rules/Properties/data/dynamic-stringable-nullsafe-access.php @@ -0,0 +1,27 @@ += 8.0 + +namespace DynamicStringableNullsafeAccess; + +use Stringable; + +final class Foo +{ + private self $var; + + public function testNullsafePropertyFetch(string $name, Stringable $stringable, object $object): void + { + echo $this?->{$this}?->name; + echo $this?->var?->$this; + echo $this?->$this?->$name; + echo $this?->$this?->name; + echo $this?->$object; + + echo $this?->$name; // valid + echo $this?->$stringable; // valid + echo $this?->{1111}; // valid + echo $this?->{true}; // valid + echo $this?->{false}; // valid + echo $this?->{null}; // valid + } + +}