diff --git a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php index 1c389f9fb9a..1e1f0ad24b0 100644 --- a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php +++ b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php @@ -13,7 +13,9 @@ use PHPStan\Analyser\ExpressionResult; use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\ExprHandler; +use PHPStan\Analyser\ExprHandler\Helper\IllegalOffsetTypeHelper; use PHPStan\Analyser\ExprHandler\Helper\NullsafeShortCircuitingHelper; +use PHPStan\Analyser\InternalThrowPoint; use PHPStan\Analyser\MutatingScope; use PHPStan\Analyser\NodeScopeResolver; use PHPStan\Analyser\NoopNodeCallback; @@ -23,9 +25,11 @@ use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Node\Expr\TypeExpr; +use PHPStan\Php\PhpVersion; use PHPStan\Type\NeverType; use PHPStan\Type\ObjectType; use PHPStan\Type\Type; +use TypeError; use function array_merge; /** @@ -35,6 +39,10 @@ final class ArrayDimFetchHandler implements ExprHandler { + public function __construct(private PhpVersion $phpVersion) + { + } + public function supports(Expr $expr): bool { return $expr instanceof ArrayDimFetch; @@ -109,6 +117,16 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex )->getThrowPoints()); } + if ( + $this->phpVersion->throwsTypeErrorForIllegalOffsets() + // only array and string offset reads throw TypeError for illegal offsets, + // reads on null and other scalars emit a warning, ArrayAccess objects go through offsetGet() + && (!$varType->isArray()->no() || !$varType->isString()->no()) + && IllegalOffsetTypeHelper::mayOffsetThrowTypeError($scope->getType($expr->dim)) + ) { + $throwPoints[] = InternalThrowPoint::createExplicit($scope, new ObjectType(TypeError::class), $expr, false); + } + return new ExpressionResult( $scope, hasYield: $dimResult->hasYield() || $varResult->hasYield(), diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 540119391e7..72b6276b4fe 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -28,6 +28,7 @@ use PHPStan\Analyser\ExpressionResultStorage; use PHPStan\Analyser\ExpressionTypeHolder; use PHPStan\Analyser\ExprHandler; +use PHPStan\Analyser\ExprHandler\Helper\IllegalOffsetTypeHelper; use PHPStan\Analyser\ImpurePoint; use PHPStan\Analyser\InternalThrowPoint; use PHPStan\Analyser\MutatingScope; @@ -608,6 +609,24 @@ public function processAssignVar( } } + if ($this->phpVersion->throwsTypeErrorForIllegalOffsets()) { + foreach ($offsetTypes as [$offsetType, $offsetDimFetch]) { + // $arr[] = ... never throws for the offset + if ($offsetType === null) { + continue; + } + if (!IllegalOffsetTypeHelper::mayOffsetThrowTypeError($offsetType)) { + continue; + } + // writes to ArrayAccess objects go through offsetSet() whose throw points are modelled below + $containerType = $scope->getType($offsetDimFetch->var); + if ((new ObjectType(ArrayAccess::class))->isSuperTypeOf($containerType)->yes()) { + continue; + } + $throwPoints[] = InternalThrowPoint::createExplicit($scope, new ObjectType(TypeError::class), $offsetDimFetch, false); + } + } + $valueToWrite = $scope->getType($assignedExpr); $nativeValueToWrite = $scope->getNativeType($assignedExpr); $scopeBeforeAssignEval = $scope; diff --git a/src/Analyser/ExprHandler/Helper/IllegalOffsetTypeHelper.php b/src/Analyser/ExprHandler/Helper/IllegalOffsetTypeHelper.php new file mode 100644 index 00000000000..c84f0e65b88 --- /dev/null +++ b/src/Analyser/ExprHandler/Helper/IllegalOffsetTypeHelper.php @@ -0,0 +1,20 @@ +isArray()->no() || !$offsetType->isObject()->no(); + } + +} diff --git a/src/Php/PhpVersion.php b/src/Php/PhpVersion.php index 4ea71d7f8e9..3ad80e0a0cf 100644 --- a/src/Php/PhpVersion.php +++ b/src/Php/PhpVersion.php @@ -186,6 +186,12 @@ public function throwsValueErrorForInternalFunctions(): bool return $this->versionId >= 80000; } + // see https://wiki.php.net/rfc/engine_warnings - "Illegal offset type" family + public function throwsTypeErrorForIllegalOffsets(): bool + { + return $this->versionId >= 80000; + } + public function supportsHhPrintfSpecifier(): bool { return $this->versionId >= 80000; diff --git a/tests/PHPStan/Analyser/nsrt/throw-points/array-dim-fetch.php b/tests/PHPStan/Analyser/nsrt/throw-points/array-dim-fetch.php index 3a64f2c3a28..0d93e4c0293 100644 --- a/tests/PHPStan/Analyser/nsrt/throw-points/array-dim-fetch.php +++ b/tests/PHPStan/Analyser/nsrt/throw-points/array-dim-fetch.php @@ -4,18 +4,8 @@ use PHPStan\TrinaryLogic; use function PHPStan\Testing\assertVariableCertainty; -use function ThrowPoints\Helpers\doesntThrow; use function ThrowPoints\Helpers\maybeThrows; -function () { - try { - [][doesntThrow()]; - $foo = 1; - } finally { - assertVariableCertainty(TrinaryLogic::createYes(), $foo); - } -}; - function () { try { [][maybeThrows()]; diff --git a/tests/PHPStan/Analyser/nsrt/throw-points/assign-op.php b/tests/PHPStan/Analyser/nsrt/throw-points/assign-op.php index c419633fabf..7cf54b2bf9a 100644 --- a/tests/PHPStan/Analyser/nsrt/throw-points/assign-op.php +++ b/tests/PHPStan/Analyser/nsrt/throw-points/assign-op.php @@ -41,14 +41,6 @@ function () { } }; -function () { - try { - $foo[doesntThrow()] .= 0; - } finally { - assertVariableCertainty(TrinaryLogic::createYes(), $foo); - } -}; - function () { try { $foo[maybeThrows()] .= 0; diff --git a/tests/PHPStan/Analyser/nsrt/throw-points/assign.php b/tests/PHPStan/Analyser/nsrt/throw-points/assign.php index 13357533a43..055c05b9f1a 100644 --- a/tests/PHPStan/Analyser/nsrt/throw-points/assign.php +++ b/tests/PHPStan/Analyser/nsrt/throw-points/assign.php @@ -49,14 +49,6 @@ function () { } }; -function () { - try { - $foo[doesntThrow()] = 0; - } finally { - assertVariableCertainty(TrinaryLogic::createYes(), $foo); - } -}; - function () { try { $foo[maybeThrows()] = 0; @@ -153,14 +145,6 @@ function () { } }; -function () { - try { - [$foo[doesntThrow()]] = 1; - } finally { - assertVariableCertainty(TrinaryLogic::createYes(), $foo); - } -}; - function () { try { [$foo[maybeThrows()]] = 1; diff --git a/tests/PHPStan/Analyser/nsrt/throw-points/illegal-array-offset-php7.php b/tests/PHPStan/Analyser/nsrt/throw-points/illegal-array-offset-php7.php new file mode 100644 index 00000000000..10b7341aaaa --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/throw-points/illegal-array-offset-php7.php @@ -0,0 +1,43 @@ += 8.0 + +namespace ThrowPoints\IllegalArrayOffset; + +use PHPStan\TrinaryLogic; +use function PHPStan\Testing\assertVariableCertainty; +use function ThrowPoints\Helpers\doesntThrow; + +// doesntThrow() never throws by itself, but on PHP 8.0+ its mixed return value used +// as an array/string offset may be an array or an object, which throws TypeError. +// That TypeError is the only throw point here, so $foo may be left unassigned. + +function () { + try { + $foo[doesntThrow()] = 0; + } finally { + assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); + } +}; + +function () { + try { + [$foo[doesntThrow()]] = 1; + } finally { + assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); + } +}; + +function () { + try { + $foo[doesntThrow()] .= 0; + } finally { + assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); + } +}; + +function () { + try { + [][doesntThrow()]; + $foo = 1; + } finally { + assertVariableCertainty(TrinaryLogic::createMaybe(), $foo); + } +}; diff --git a/tests/PHPStan/Levels/data/arrayOffsetAccess-4.json b/tests/PHPStan/Levels/data/arrayOffsetAccess-4.json index 3f91016ab34..eba3b85399b 100644 --- a/tests/PHPStan/Levels/data/arrayOffsetAccess-4.json +++ b/tests/PHPStan/Levels/data/arrayOffsetAccess-4.json @@ -14,26 +14,6 @@ "line": 18, "ignorable": true }, - { - "message": "Expression \"$a[$objectOrInt]\" on a separate line does not do anything.", - "line": 19, - "ignorable": true - }, - { - "message": "Expression \"$a[$objectOrNull]\" on a separate line does not do anything.", - "line": 20, - "ignorable": true - }, - { - "message": "Expression \"$a[$explicitlyMixed]\" on a separate line does not do anything.", - "line": 21, - "ignorable": true - }, - { - "message": "Expression \"$a[$implicitlyMixed]\" on a separate line does not do anything.", - "line": 22, - "ignorable": true - }, { "message": "Expression \"$arrayOrObject[42]\" on a separate line does not do anything.", "line": 24, @@ -49,26 +29,6 @@ "line": 27, "ignorable": true }, - { - "message": "Expression \"$arrayOrObject[$objectOrInt]\" on a separate line does not do anything.", - "line": 28, - "ignorable": true - }, - { - "message": "Expression \"$arrayOrObject[$objectOrNull]\" on a separate line does not do anything.", - "line": 29, - "ignorable": true - }, - { - "message": "Expression \"$arrayOrObject[$explicitlyMixed]\" on a separate line does not do anything.", - "line": 30, - "ignorable": true - }, - { - "message": "Expression \"$arrayOrObject[$implicitlyMixed]\" on a separate line does not do anything.", - "line": 31, - "ignorable": true - }, { "message": "Expression \"$explicitlyMixed[42]\" on a separate line does not do anything.", "line": 33, @@ -84,26 +44,6 @@ "line": 36, "ignorable": true }, - { - "message": "Expression \"$explicitlyMixed[$objectOrInt]\" on a separate line does not do anything.", - "line": 37, - "ignorable": true - }, - { - "message": "Expression \"$explicitlyMixed[$objectOrNull]\" on a separate line does not do anything.", - "line": 38, - "ignorable": true - }, - { - "message": "Expression \"$explicitlyMixed[$explicitlyMixed]\" on a separate line does not do anything.", - "line": 39, - "ignorable": true - }, - { - "message": "Expression \"$explicitlyMixed[$implicitlyMixed]\" on a separate line does not do anything.", - "line": 40, - "ignorable": true - }, { "message": "Expression \"$implicitlyMixed[42]\" on a separate line does not do anything.", "line": 42, @@ -118,25 +58,5 @@ "message": "Expression \"$implicitlyMixed[$intOrNull]\" on a separate line does not do anything.", "line": 45, "ignorable": true - }, - { - "message": "Expression \"$implicitlyMixed[$objectOrInt]\" on a separate line does not do anything.", - "line": 46, - "ignorable": true - }, - { - "message": "Expression \"$implicitlyMixed[$objectOrNull]\" on a separate line does not do anything.", - "line": 47, - "ignorable": true - }, - { - "message": "Expression \"$implicitlyMixed[$explicitlyMixed]\" on a separate line does not do anything.", - "line": 48, - "ignorable": true - }, - { - "message": "Expression \"$implicitlyMixed[$implicitlyMixed]\" on a separate line does not do anything.", - "line": 49, - "ignorable": true } -] +] \ No newline at end of file diff --git a/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php b/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php index b72065feddf..daec1ae6353 100644 --- a/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php +++ b/tests/PHPStan/Rules/Exceptions/CatchWithUnthrownExceptionRuleTest.php @@ -705,6 +705,33 @@ public function testBug9146(): void ]); } + #[RequiresPhp('>= 8.0.0')] + public function testBug6970(): void + { + $this->analyse([__DIR__ . '/data/bug-6970.php'], [ + [ + 'Dead catch - TypeError is never thrown in the try block.', + 96, + ], + [ + 'Dead catch - TypeError is never thrown in the try block.', + 109, + ], + [ + 'Dead catch - TypeError is never thrown in the try block.', + 147, + ], + [ + 'Dead catch - TypeError is never thrown in the try block.', + 198, + ], + [ + 'Dead catch - TypeError is never thrown in the try block.', + 286, + ], + ]); + } + #[RequiresPhp('>= 8.3.0')] public function testPr5105(): void { diff --git a/tests/PHPStan/Rules/Exceptions/data/bug-6970.php b/tests/PHPStan/Rules/Exceptions/data/bug-6970.php new file mode 100644 index 00000000000..da85c26da2e --- /dev/null +++ b/tests/PHPStan/Rules/Exceptions/data/bug-6970.php @@ -0,0 +1,334 @@ += 8.0 + +namespace Bug6970; + +use ArrayAccess; +use Generator; +use TypeError; +use UnexpectedValueException; +use function count; + +/** + * @template K + * @template T + * @template L + * @template U + * + * @param iterable $stream + * @param callable(T, K): Generator $fn + * + * @return Generator + */ +function scollect(iterable $stream, callable $fn): Generator +{ + foreach ($stream as $key => $value) { + yield from $fn($value, $key); + } +} + +/** + * @param mixed[] $array + * + * @return mixed[] + */ +function collectWithKeys(array $array, callable $fn): array +{ + $stream = scollect($array, $fn); + + $values = []; + $counter = 0; + + foreach ($stream as $key => $value) { + try { + $values[$key] = $value; + } catch (TypeError) { + throw new UnexpectedValueException('The key yielded in the callable is not compatible with the type "array-key".'); + } + + ++$counter; + } + + if ($counter !== count($values)) { + throw new UnexpectedValueException( + 'Data loss occurred because of duplicated keys. Use `collect()` if you do not care about ' . + 'the yielded keys, or use `scollect()` if you need to support duplicated keys (as arrays cannot).', + ); + } + + return $values; +} + +class MoreCases +{ + + /** + * @param mixed[] $array + * @param mixed $key + */ + public function writeMixedKey(array $array, $key): void + { + try { + $array[$key] = 1; + } catch (TypeError) { // not dead + echo 'caught'; + } + } + + /** + * @param mixed[] $array + */ + public function writeObjectKey(array $array, \stdClass $key): void + { + try { + $array[$key] = 1; + } catch (TypeError) { // not dead + echo 'caught'; + } + } + + /** + * @param mixed[] $array + */ + public function writeIntKey(array $array, int $key): void + { + try { + $array[$key] = 1; + } catch (TypeError) { // error: Dead catch - TypeError is never thrown in the try block. + echo 'caught'; + } + } + + /** + * @param mixed[] $array + * @param mixed $value + */ + public function append(array $array, $value): void + { + try { + $array[] = $value; + } catch (TypeError) { // error: Dead catch - TypeError is never thrown in the try block. + echo 'caught'; + } + } + + /** + * @param mixed[] $array + * @param mixed $key + */ + public function assignOpMixedKey(array $array, $key): void + { + try { + $array[$key] .= 'x'; + } catch (TypeError) { // not dead + echo 'caught'; + } + } + + /** + * @param mixed[] $array + * @param mixed $key + */ + public function readMixedKey(array $array, $key): void + { + try { + $x = $array[$key]; + } catch (TypeError) { // not dead + echo 'caught'; + } + } + + /** + * @param mixed[] $array + */ + public function readIntKey(array $array, int $key): void + { + try { + $x = $array[$key]; + } catch (TypeError) { // error: Dead catch - TypeError is never thrown in the try block. + echo 'caught'; + } + } + + /** + * @param mixed[] $array + * @param mixed $key + */ + public function readCoalesceMixedKey(array $array, $key): void + { + try { + $x = $array[$key] ?? null; + } catch (TypeError) { // not dead + echo 'caught'; + } + } + + /** + * @param mixed[] $array + * @param mixed $key + */ + public function issetMixedKey(array $array, $key): void + { + try { + $x = isset($array[$key]); + } catch (TypeError) { // not dead + echo 'caught'; + } + } + + /** + * @param mixed[] $array + * @param mixed $key + */ + public function unsetMixedKey(array $array, $key): void + { + try { + unset($array[$key]); + } catch (TypeError) { // not dead + echo 'caught'; + } + } + + /** + * @param mixed[] $array + */ + public function unsetIntKey(array $array, int $key): void + { + try { + unset($array[$key]); + } catch (TypeError) { // error: Dead catch - TypeError is never thrown in the try block. + echo 'caught'; + } + } + + /** + * @param mixed $key + */ + public function stringOffsetWriteMixedKey(string $s, $key): void + { + try { + $s[$key] = 'x'; + } catch (TypeError) { // not dead + echo 'caught'; + } + } + + /** + * @param mixed $key + */ + public function stringOffsetReadMixedKey(string $s, $key): void + { + try { + $x = $s[$key]; + } catch (TypeError) { // not dead + echo 'caught'; + } + } + + /** + * @param mixed[] $array + * @param int|mixed[] $key + */ + public function writeArrayOrIntKey(array $array, $key): void + { + try { + $array[$key] = 1; + } catch (TypeError) { // not dead - key may be an array + echo 'caught'; + } + } + + /** + * @param mixed[] $array + * @param int|\stdClass $key + */ + public function writeObjectOrIntKey(array $array, $key): void + { + try { + $array[$key] = 1; + } catch (TypeError) { // not dead - key may be an object + echo 'caught'; + } + } + + /** + * @param mixed[]|int $arrayOrInt + * @param mixed $key + */ + public function readArrayOrIntVar($arrayOrInt, $key): void + { + try { + $x = $arrayOrInt[$key]; + } catch (TypeError) { // not dead - reading an array offset with an illegal key throws + echo 'caught'; + } + } + + /** + * @param string|int $stringOrInt + * @param mixed $key + */ + public function readStringOrIntVar($stringOrInt, $key): void + { + try { + $x = $stringOrInt[$key]; + } catch (TypeError) { // not dead - reading a string offset with an illegal key throws + echo 'caught'; + } + } + + /** + * @param mixed $key + */ + public function writeConcreteArrayAccessKey(ConcreteArrayAccess $container, $key): void + { + try { + $container[$key] = 1; + } catch (TypeError) { // error: dead - ArrayAccess offsetSet() handles the offset, so no TypeError from the illegal offset + echo 'caught'; + } + } + + /** + * @param mixed[]|ConcreteArrayAccess $container + * @param mixed $key + */ + public function writeMaybeArrayAccessKey($container, $key): void + { + try { + $container[$key] = 1; + } catch (TypeError) { // not dead - container may be a plain array, whose illegal offset throws + echo 'caught'; + } + } + +} + +/** + * @implements ArrayAccess + */ +final class ConcreteArrayAccess implements ArrayAccess +{ + + public function offsetExists($offset): bool + { + return true; + } + + public function offsetGet($offset): mixed + { + return null; + } + + /** + * @throws \RuntimeException + */ + public function offsetSet($offset, $value): void + { + throw new \RuntimeException(); + } + + public function offsetUnset($offset): void + { + } + +}