From ced28cb403ec12a1032dac531392f404eaf9c8a4 Mon Sep 17 00:00:00 2001 From: "Hans Krentel (hakre)" Date: Mon, 11 May 2026 14:35:41 +0200 Subject: [PATCH] Guard `*scanf()` return type extension by counter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the recently fixed `PrintfHelper::getScanfPlaceholdersCount()` to guard the return‑type extension against format‑induced imprecisions. The counter now guards the extension’s independent regex‑based counting, syncing it with the `PrintfParametersRule` for the first time. Invalid formats (uncountable) now return a precise error type (`NeverType`/`NullType`) instead of a false `array|null`. Valid formats (countable) that the old regex would miscount or ignore are now handled by a counter‑sized safe skeleton. The regex is now an optional precision layer, not the foundation for structural correctness. The counter overrides the regex wherever they disagree. This is the same approach as in dd63663420 ("Fix counting `*scanf()` format string placeholders (#5594)", 2026-05-10) that eliminated the count regression, now applied to the return‑type logic. Removes the old bottom‑of‑method `return null` as a natural consequence. For example, an invalid format (mixing positional `%n$` with sequential `%`) now correctly returns `null` on 7.4. Gegenprobe: the counter doesn’t guess – it asks PHP itself. --- ...canfFunctionDynamicReturnTypeExtension.php | 95 +++++++++++++------ .../Analyser/NodeScopeResolverTest.php | 6 ++ tests/PHPStan/Analyser/data/sscanf-php74.php | 9 ++ tests/PHPStan/Analyser/data/sscanf-php80.php | 9 ++ tests/PHPStan/Analyser/nsrt/bug-14597.php | 47 +++++++++ 5 files changed, 137 insertions(+), 29 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/sscanf-php74.php create mode 100644 tests/PHPStan/Analyser/data/sscanf-php80.php create mode 100644 tests/PHPStan/Analyser/nsrt/bug-14597.php diff --git a/src/Type/Php/SscanfFunctionDynamicReturnTypeExtension.php b/src/Type/Php/SscanfFunctionDynamicReturnTypeExtension.php index de22ba0a461..6e34ff36edb 100644 --- a/src/Type/Php/SscanfFunctionDynamicReturnTypeExtension.php +++ b/src/Type/Php/SscanfFunctionDynamicReturnTypeExtension.php @@ -5,7 +5,9 @@ use PhpParser\Node\Expr\FuncCall; use PHPStan\Analyser\Scope; use PHPStan\DependencyInjection\AutowiredService; +use PHPStan\Php\PhpVersion; use PHPStan\Reflection\FunctionReflection; +use PHPStan\Rules\Functions\PrintfHelper; use PHPStan\Type\Accessory\AccessoryNonEmptyStringType; use PHPStan\Type\Accessory\AccessoryNonFalsyStringType; use PHPStan\Type\Constant\ConstantArrayTypeBuilder; @@ -15,6 +17,8 @@ use PHPStan\Type\FloatType; use PHPStan\Type\IntegerType; use PHPStan\Type\IntersectionType; +use PHPStan\Type\NeverType; +use PHPStan\Type\NullType; use PHPStan\Type\StringType; use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; @@ -26,6 +30,13 @@ final class SscanfFunctionDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension { + public function __construct( + private PrintfHelper $printfHelper, + private PhpVersion $phpVersion, + ) + { + } + public function isFunctionSupported(FunctionReflection $functionReflection): bool { return in_array($functionReflection->getName(), ['sscanf', 'fscanf'], true); @@ -48,44 +59,70 @@ public function getTypeFromFunctionCall( return null; } - if (preg_match_all('/%(\d*)(\[[^\]]+\]|[cdeEfosux]{1})/', $formatType->getValue(), $matches) > 0) { - $arrayBuilder = ConstantArrayTypeBuilder::createEmpty(); - - for ($i = 0; $i < count($matches[0]); $i++) { - $length = $matches[1][$i]; - $specifier = $matches[2][$i]; - - $type = new StringType(); - if ($length !== '') { - if (((int) $length) > 1) { - $type = new IntersectionType([ - $type, - new AccessoryNonFalsyStringType(), - ]); - } else { - $type = new IntersectionType([ - $type, + $formatValue = $formatType->getValue(); + $placeholderCount = $this->printfHelper->getScanfPlaceholdersCount($formatValue); + if ($placeholderCount === null) { + return $this->phpVersion->throwsValueErrorForInternalFunctions() ? new NeverType() : new NullType(); + } + + if ($placeholderCount === 0) { + return TypeCombinator::addNull( + ConstantArrayTypeBuilder::createEmpty()->getArray(), + ); + } + + if (preg_match_all('/%(\d*)(\[[^\]]+\]|[cdeEfosux]{1})/', $formatValue, $matches) !== $placeholderCount) { + $safeBuilder = ConstantArrayTypeBuilder::createEmpty(); + for ($i = 0; $i < $placeholderCount; ++$i) { + $safeBuilder->setOffsetValueType( + new ConstantIntegerType($i), + TypeCombinator::union( + new FloatType(), + new IntegerType(), + new IntersectionType([ + new StringType(), new AccessoryNonEmptyStringType(), - ]); - } - } + ]), + new NullType(), + ), + ); + } + return TypeCombinator::addNull($safeBuilder->getArray()); + } - if (in_array($specifier, ['d', 'o', 'u', 'x'], true)) { - $type = new IntegerType(); + $arrayBuilder = ConstantArrayTypeBuilder::createEmpty(); + for ($i = 0; $i < count($matches[0]); $i++) { + $length = $matches[1][$i]; + $specifier = $matches[2][$i]; + + $type = new StringType(); + if ($length !== '') { + if (((int) $length) > 1) { + $type = new IntersectionType([ + $type, + new AccessoryNonFalsyStringType(), + ]); + } else { + $type = new IntersectionType([ + $type, + new AccessoryNonEmptyStringType(), + ]); } + } - if (in_array($specifier, ['e', 'E', 'f'], true)) { - $type = new FloatType(); - } + if (in_array($specifier, ['d', 'o', 'u', 'x'], true)) { + $type = new IntegerType(); + } - $type = TypeCombinator::addNull($type); - $arrayBuilder->setOffsetValueType(new ConstantIntegerType($i), $type); + if (in_array($specifier, ['e', 'E', 'f'], true)) { + $type = new FloatType(); } - return TypeCombinator::addNull($arrayBuilder->getArray()); + $type = TypeCombinator::addNull($type); + $arrayBuilder->setOffsetValueType(new ConstantIntegerType($i), $type); } - return null; + return TypeCombinator::addNull($arrayBuilder->getArray()); } } diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index a8903a13df5..ced33a0ee59 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -284,6 +284,12 @@ private static function findTestFiles(): iterable yield __DIR__ . '/../Rules/Variables/data/bug-14124.php'; yield __DIR__ . '/../Rules/Variables/data/bug-14124b.php'; yield __DIR__ . '/../Rules/Arrays/data/bug-14308.php'; + + if (PHP_VERSION_ID < 80000) { + yield __DIR__ . '/data/sscanf-php74.php'; + } else { + yield __DIR__ . '/data/sscanf-php80.php'; + } } /** diff --git a/tests/PHPStan/Analyser/data/sscanf-php74.php b/tests/PHPStan/Analyser/data/sscanf-php74.php new file mode 100644 index 00000000000..19b89bc0bb4 --- /dev/null +++ b/tests/PHPStan/Analyser/data/sscanf-php74.php @@ -0,0 +1,9 @@ +