Skip to content

Commit ced28cb

Browse files
hakrehakre
committed
Guard *scanf() return type extension by counter
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 dd63663 ("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.
1 parent f8d7b8a commit ced28cb

5 files changed

Lines changed: 137 additions & 29 deletions

File tree

‎src/Type/Php/SscanfFunctionDynamicReturnTypeExtension.php‎

Lines changed: 66 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
use PhpParser\Node\Expr\FuncCall;
66
use PHPStan\Analyser\Scope;
77
use PHPStan\DependencyInjection\AutowiredService;
8+
use PHPStan\Php\PhpVersion;
89
use PHPStan\Reflection\FunctionReflection;
10+
use PHPStan\Rules\Functions\PrintfHelper;
911
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
1012
use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
1113
use PHPStan\Type\Constant\ConstantArrayTypeBuilder;
@@ -15,6 +17,8 @@
1517
use PHPStan\Type\FloatType;
1618
use PHPStan\Type\IntegerType;
1719
use PHPStan\Type\IntersectionType;
20+
use PHPStan\Type\NeverType;
21+
use PHPStan\Type\NullType;
1822
use PHPStan\Type\StringType;
1923
use PHPStan\Type\Type;
2024
use PHPStan\Type\TypeCombinator;
@@ -26,6 +30,13 @@
2630
final class SscanfFunctionDynamicReturnTypeExtension implements DynamicFunctionReturnTypeExtension
2731
{
2832

33+
public function __construct(
34+
private PrintfHelper $printfHelper,
35+
private PhpVersion $phpVersion,
36+
)
37+
{
38+
}
39+
2940
public function isFunctionSupported(FunctionReflection $functionReflection): bool
3041
{
3142
return in_array($functionReflection->getName(), ['sscanf', 'fscanf'], true);
@@ -48,44 +59,70 @@ public function getTypeFromFunctionCall(
4859
return null;
4960
}
5061

51-
if (preg_match_all('/%(\d*)(\[[^\]]+\]|[cdeEfosux]{1})/', $formatType->getValue(), $matches) > 0) {
52-
$arrayBuilder = ConstantArrayTypeBuilder::createEmpty();
53-
54-
for ($i = 0; $i < count($matches[0]); $i++) {
55-
$length = $matches[1][$i];
56-
$specifier = $matches[2][$i];
57-
58-
$type = new StringType();
59-
if ($length !== '') {
60-
if (((int) $length) > 1) {
61-
$type = new IntersectionType([
62-
$type,
63-
new AccessoryNonFalsyStringType(),
64-
]);
65-
} else {
66-
$type = new IntersectionType([
67-
$type,
62+
$formatValue = $formatType->getValue();
63+
$placeholderCount = $this->printfHelper->getScanfPlaceholdersCount($formatValue);
64+
if ($placeholderCount === null) {
65+
return $this->phpVersion->throwsValueErrorForInternalFunctions() ? new NeverType() : new NullType();
66+
}
67+
68+
if ($placeholderCount === 0) {
69+
return TypeCombinator::addNull(
70+
ConstantArrayTypeBuilder::createEmpty()->getArray(),
71+
);
72+
}
73+
74+
if (preg_match_all('/%(\d*)(\[[^\]]+\]|[cdeEfosux]{1})/', $formatValue, $matches) !== $placeholderCount) {
75+
$safeBuilder = ConstantArrayTypeBuilder::createEmpty();
76+
for ($i = 0; $i < $placeholderCount; ++$i) {
77+
$safeBuilder->setOffsetValueType(
78+
new ConstantIntegerType($i),
79+
TypeCombinator::union(
80+
new FloatType(),
81+
new IntegerType(),
82+
new IntersectionType([
83+
new StringType(),
6884
new AccessoryNonEmptyStringType(),
69-
]);
70-
}
71-
}
85+
]),
86+
new NullType(),
87+
),
88+
);
89+
}
90+
return TypeCombinator::addNull($safeBuilder->getArray());
91+
}
7292

73-
if (in_array($specifier, ['d', 'o', 'u', 'x'], true)) {
74-
$type = new IntegerType();
93+
$arrayBuilder = ConstantArrayTypeBuilder::createEmpty();
94+
for ($i = 0; $i < count($matches[0]); $i++) {
95+
$length = $matches[1][$i];
96+
$specifier = $matches[2][$i];
97+
98+
$type = new StringType();
99+
if ($length !== '') {
100+
if (((int) $length) > 1) {
101+
$type = new IntersectionType([
102+
$type,
103+
new AccessoryNonFalsyStringType(),
104+
]);
105+
} else {
106+
$type = new IntersectionType([
107+
$type,
108+
new AccessoryNonEmptyStringType(),
109+
]);
75110
}
111+
}
76112

77-
if (in_array($specifier, ['e', 'E', 'f'], true)) {
78-
$type = new FloatType();
79-
}
113+
if (in_array($specifier, ['d', 'o', 'u', 'x'], true)) {
114+
$type = new IntegerType();
115+
}
80116

81-
$type = TypeCombinator::addNull($type);
82-
$arrayBuilder->setOffsetValueType(new ConstantIntegerType($i), $type);
117+
if (in_array($specifier, ['e', 'E', 'f'], true)) {
118+
$type = new FloatType();
83119
}
84120

85-
return TypeCombinator::addNull($arrayBuilder->getArray());
121+
$type = TypeCombinator::addNull($type);
122+
$arrayBuilder->setOffsetValueType(new ConstantIntegerType($i), $type);
86123
}
87124

88-
return null;
125+
return TypeCombinator::addNull($arrayBuilder->getArray());
89126
}
90127

91128
}

‎tests/PHPStan/Analyser/NodeScopeResolverTest.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,12 @@ private static function findTestFiles(): iterable
284284
yield __DIR__ . '/../Rules/Variables/data/bug-14124.php';
285285
yield __DIR__ . '/../Rules/Variables/data/bug-14124b.php';
286286
yield __DIR__ . '/../Rules/Arrays/data/bug-14308.php';
287+
288+
if (PHP_VERSION_ID < 80000) {
289+
yield __DIR__ . '/data/sscanf-php74.php';
290+
} else {
291+
yield __DIR__ . '/data/sscanf-php80.php';
292+
}
287293
}
288294

289295
/**

‎tests/PHPStan/Analyser/data/sscanf-php74.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
namespace SscanfPHP74;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
function sscanfInvalidFormatMixingPositionalWithSequential(string $s) {
8+
assertType('null', sscanf($s, '%1$s %s'));
9+
}

‎tests/PHPStan/Analyser/data/sscanf-php80.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
<?php
2+
3+
namespace SscanfPHP80;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
function sscanfInvalidFormatMixingPositionalWithSequential(string $s) {
8+
assertType('*NEVER*', sscanf($s, '%1$s %s'));
9+
}

‎tests/PHPStan/Analyser/nsrt/bug-14597.php‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
<?php
2+
3+
namespace Bug14597;
4+
5+
use function PHPStan\Testing\assertType;
6+
7+
function sscanfNulTerminator(string $s) {
8+
// NUL byte terminates sscanf format string – placeholders after \0 are ignored
9+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%d\0%d"));
10+
assertType('array{float|int|non-empty-string|null, float|int|non-empty-string|null}|null', sscanf($s, "%d %s\0%d"));
11+
assertType('array{}|null', sscanf($s, "\0%d%s"));
12+
}
13+
14+
function fscanfNulTerminator($r) {
15+
// Same for fscanf
16+
assertType('array{float|int|non-empty-string|null}|null', fscanf($r, "%d\0%d"));
17+
assertType('array{float|int|non-empty-string|null, float|int|non-empty-string|null}|null', fscanf($r, "%d %s\0%d"));
18+
assertType('array{}|null', fscanf($r, "\0%d%s"));
19+
}
20+
21+
function sscanfEdgeCases(string $s) {
22+
// Empty format string – no placeholders
23+
assertType('array{}|null', sscanf($s, ""));
24+
25+
// %n – counts characters consumed, returns integer
26+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%n"));
27+
28+
// %% – literal percent, not a placeholder
29+
assertType('array{}|null', sscanf($s, "%%"));
30+
31+
// %i – integer with base detection
32+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%i"));
33+
34+
// %X – uppercase hex, same as %x
35+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%X"));
36+
37+
// %D – uppercase alias for %d
38+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%D"));
39+
40+
// Size modifiers (l, L, h) – consumed by ValidateFormat, no effect on PHP type
41+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%ld"));
42+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%lf"));
43+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%Lf"));
44+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%hd"));
45+
assertType('array{float|int|non-empty-string|null}|null', sscanf($s, "%lu"));
46+
assertType('array{float|int|non-empty-string|null, float|int|non-empty-string|null, float|int|non-empty-string|null}|null', sscanf($s, "%ld %lf %s"));
47+
}

0 commit comments

Comments
 (0)