diff --git a/conf/bleedingEdge.neon b/conf/bleedingEdge.neon index 91201231116..306005bf3f2 100644 --- a/conf/bleedingEdge.neon +++ b/conf/bleedingEdge.neon @@ -32,3 +32,4 @@ parameters: staticVariablesFromUsages: true unusedVariable: true unusedParameters: true + unsetOffsetOnMaybeAccessible: true diff --git a/conf/config.neon b/conf/config.neon index 52860b21fe9..81e074b9d7f 100644 --- a/conf/config.neon +++ b/conf/config.neon @@ -64,6 +64,7 @@ parameters: staticVariablesFromUsages: false unusedVariable: false unusedParameters: false + unsetOffsetOnMaybeAccessible: false fileExtensions: - php checkAdvancedIsset: false diff --git a/conf/parametersSchema.neon b/conf/parametersSchema.neon index 3bae9593991..758005de9dd 100644 --- a/conf/parametersSchema.neon +++ b/conf/parametersSchema.neon @@ -61,6 +61,7 @@ parametersSchema: staticVariablesFromUsages: bool() unusedVariable: bool() unusedParameters: bool() + unsetOffsetOnMaybeAccessible: bool() ]) fileExtensions: listOf(string()) checkAdvancedIsset: bool() diff --git a/src/Rules/Variables/UnsetRule.php b/src/Rules/Variables/UnsetRule.php index d2893e17fe3..2472dd2f967 100644 --- a/src/Rules/Variables/UnsetRule.php +++ b/src/Rules/Variables/UnsetRule.php @@ -4,12 +4,15 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; +use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Php\PhpVersion; use PHPStan\Rules\IdentifierRuleError; use PHPStan\Rules\Properties\PropertyReflectionFinder; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; +use PHPStan\Type\Type; +use PHPStan\Type\TypeUtils; use PHPStan\Type\VerbosityLevel; use function is_string; use function sprintf; @@ -24,6 +27,10 @@ final class UnsetRule implements Rule public function __construct( private PropertyReflectionFinder $propertyReflectionFinder, private PhpVersion $phpVersion, + #[AutowiredParameter] + private bool $reportMaybes, + #[AutowiredParameter(ref: '%featureToggles.unsetOffsetOnMaybeAccessible%')] + private bool $unsetOffsetOnMaybeAccessible, ) { } @@ -127,7 +134,11 @@ private function canBeUnset(Node $node, Scope $scope): ?IdentifierRuleError $type = $scope->getType($node->var); $dimType = $scope->getType($node->dim); - if ($type->isOffsetAccessible()->no() || $type->hasOffsetValueType($dimType)->no()) { + if ( + $type->isOffsetAccessible()->no() + || $type->hasOffsetValueType($dimType)->no() + || $this->isOffsetMaybeNotUnsettable($type) + ) { return RuleErrorBuilder::message( sprintf( 'Cannot unset offset %s on %s.', @@ -146,4 +157,28 @@ private function canBeUnset(Node $node, Scope $scope): ?IdentifierRuleError return null; } + /** + * unset() of an offset deprecates on false and throws an Error on true, an int, a float or a string, + * so a union like array|false, array|int or array|string can fail at runtime. + * A type whose offset access is not even legal (an object without ArrayAccess) is reported by NonexistentOffsetInArrayDimFetchRule. + */ + private function isOffsetMaybeNotUnsettable(Type $type): bool + { + if (!$this->unsetOffsetOnMaybeAccessible || !$this->reportMaybes) { + return false; + } + + if (!$type->isOffsetAccessLegal()->yes()) { + return false; + } + + foreach (TypeUtils::flattenTypes($type) as $innerType) { + if ($innerType->isScalar()->yes()) { + return true; + } + } + + return false; + } + } diff --git a/tests/PHPStan/Rules/Variables/UnsetRuleTest.php b/tests/PHPStan/Rules/Variables/UnsetRuleTest.php index 73515e60d5f..a43b03aee3b 100644 --- a/tests/PHPStan/Rules/Variables/UnsetRuleTest.php +++ b/tests/PHPStan/Rules/Variables/UnsetRuleTest.php @@ -16,12 +16,18 @@ class UnsetRuleTest extends RuleTestCase { + private bool $reportMaybes = false; + + private bool $unsetOffsetOnMaybeAccessible = false; + protected function getRule(): Rule { $container = self::getContainer(); return new UnsetRule( $container->getByType(PropertyReflectionFinder::class), $container->getByType(PhpVersion::class), + $this->reportMaybes, + $this->unsetOffsetOnMaybeAccessible, ); } @@ -229,4 +235,64 @@ public function testNullCoalesceAssignRightSideScope(): void $this->analyse([__DIR__ . '/data/null-coalesce-assign-right-side-scope.php'], []); } + public function testUnsetOffsetOnMaybeAccessible(): void + { + $this->reportMaybes = true; + $this->unsetOffsetOnMaybeAccessible = true; + $this->analyse([__DIR__ . '/data/unset-offset-maybe-accessible.php'], [ + [ + 'Cannot unset offset \'port\' on array{port?: int, path?: string}|false.', + 13, + ], + [ + 'Cannot unset offset \'port\' on array|int.', + 21, + ], + [ + 'Cannot unset offset \'port\' on array{scheme?: string, host?: string, port?: int<0, 65535>, user?: string, pass?: string, path?: string, query?: string, fragment?: string}|false.', + 56, + ], + [ + 'Cannot unset offset \'port\' on array|bool.', + 91, + ], + [ + 'Cannot unset offset \'port\' on array|true.', + 92, + ], + [ + 'Cannot unset offset \'port\' on array|float.', + 93, + ], + [ + 'Cannot unset offset \'port\' on array|false.', + 101, + ], + ]); + } + + public function testUnsetOffsetOnMaybeAccessibleWithoutReportMaybes(): void + { + $this->unsetOffsetOnMaybeAccessible = true; + $this->analyse([__DIR__ . '/data/unset-offset-maybe-accessible.php'], []); + } + + public function testUnsetOffsetOnMaybeAccessibleWithoutFeatureToggle(): void + { + $this->reportMaybes = true; + $this->analyse([__DIR__ . '/data/unset-offset-maybe-accessible.php'], []); + } + + public function testUnsetOffsetOnMaybeAccessibleTemplate(): void + { + $this->reportMaybes = true; + $this->unsetOffsetOnMaybeAccessible = true; + $this->analyse([__DIR__ . '/data/unset-offset-maybe-accessible-template.php'], [ + [ + 'Cannot unset offset \'port\' on array|(T of array|false).', + 11, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Variables/data/unset-offset-maybe-accessible-template.php b/tests/PHPStan/Rules/Variables/data/unset-offset-maybe-accessible-template.php new file mode 100644 index 00000000000..eef0d2a1e05 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/unset-offset-maybe-accessible-template.php @@ -0,0 +1,12 @@ +|false + * @param array|T $value + */ +function foo($value): void +{ + unset($value['port']); +} diff --git a/tests/PHPStan/Rules/Variables/data/unset-offset-maybe-accessible.php b/tests/PHPStan/Rules/Variables/data/unset-offset-maybe-accessible.php new file mode 100644 index 00000000000..f6020cfd9c1 --- /dev/null +++ b/tests/PHPStan/Rules/Variables/data/unset-offset-maybe-accessible.php @@ -0,0 +1,120 @@ += 8.0 + +namespace UnsetOffsetMaybeAccessible; + +class Foo +{ + + /** + * @param array{port?: int, path?: string}|false $parsed + */ + public function arrayOrFalse($parsed): void + { + unset($parsed['port']); + } + + /** + * @param array|int $value + */ + public function arrayOrInt($value): void + { + unset($value['port']); + } + + /** + * @param array|null $value + */ + public function arrayOrNull($value): void + { + unset($value['port']); + } + + /** + * @param array|\ArrayAccess $value + */ + public function arrayOrArrayAccess($value): void + { + unset($value['port']); + } + + /** + * @param array $value + */ + public function array($value): void + { + unset($value['port']); + } + + public function object(\stdClass $value): void + { + unset($value['port']); + } + + public function parseUrl(string $url): void + { + $parsed = parse_url($url); + unset($parsed['port']); + } + + public function explicitMixed(mixed $value): void + { + unset($value['port']); + } + + public function implicitMixed($value): void + { + unset($value['port']); + } + + public function mixedWithoutFalse(mixed $value): void + { + if ($value !== false) { + unset($value['port']); + } + } + + public function jsonDecodedAfterIsset(string $json): void + { + $body = json_decode($json, true); + if (isset($body['ttl'])) { + unset($body['ttl']); + } + } + + /** + * @param array|bool $bool + * @param array|true $true + * @param array|float $float + */ + public function scalars($bool, $true, $float): void + { + unset($bool['port']); + unset($true['port']); + unset($float['port']); + } + + /** + * @param array{x: array|false} $value + */ + public function nested(array $value): void + { + unset($value['x']['port']); + } + + /** + * @param array|\stdClass $value + */ + public function arrayOrObject($value): void + { + unset($value['port']); + } + + /** + * @param \stdClass|int $value + */ + public function objectOrInt($value): void + { + unset($value['port']); + } + +}