Skip to content

Commit 4bd3eca

Browse files
hakrehakre
committed
Fix counting *scanf() format string placeholders
Update PrintfHelper.php to make public function getScanfPlaceholdersCount(string $format): ?int` return the sscanf() vetted number of placeholders that return/assign conversions. Addresses long-standing regressions reported originally in [phpstan-10260]. References: - [phpstan-10260] - phpstan/phpstan#10260 (comment) - [phpstan-src-5591] - [phpstan-14567] - https://3v4l.org/WR85Q [phpstan-10260]: phpstan/phpstan#10260 [phpstan-src-5591]: #5591 [phpstan-14567]: phpstan/phpstan#14567
1 parent 5d1ad1e commit 4bd3eca

2 files changed

Lines changed: 184 additions & 10 deletions

File tree

‎src/Rules/Functions/PrintfHelper.php‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,14 @@
55
use Nette\Utils\Strings;
66
use PHPStan\DependencyInjection\AutowiredService;
77
use PHPStan\Php\PhpVersion;
8+
use ValueError;
89
use function array_filter;
910
use function array_keys;
1011
use function count;
1112
use function in_array;
1213
use function max;
1314
use function sprintf;
15+
use function sscanf;
1416
use function strlen;
1517
use const PREG_SET_ORDER;
1618

@@ -26,24 +28,36 @@ public function __construct(private PhpVersion $phpVersion)
2628

2729
public function getPrintfPlaceholdersCount(string $format): ?int
2830
{
29-
return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format, false);
31+
return $this->getPlaceholdersCount(self::PRINTF_SPECIFIER_PATTERN, $format);
3032
}
3133

3234
/** @phpstan-return array<int, non-empty-list<PrintfPlaceholder>> parameter index => placeholders */
3335
public function getPrintfPlaceholders(string $format): ?array
3436
{
35-
return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format, false);
37+
return $this->parsePlaceholders(self::PRINTF_SPECIFIER_PATTERN, $format);
3638
}
3739

3840
public function getScanfPlaceholdersCount(string $format): ?int
3941
{
40-
return $this->getPlaceholdersCount('(?<specifier>[cdDeEfinosuxX%s]|\[[^\]]+\])', $format, true);
42+
if ($this->phpVersion->throwsValueErrorForInternalFunctions()) {
43+
try {
44+
$result = sscanf('', '%*n' . $format);
45+
} catch (ValueError) {
46+
return null;
47+
}
48+
} else {
49+
$result = @sscanf('', '%*n' . $format);
50+
}
51+
if ($result === null) {
52+
return null;
53+
}
54+
return count($result);
4155
}
4256

4357
/**
4458
* @phpstan-return array<int, non-empty-list<PrintfPlaceholder>>|null parameter index => placeholders
4559
*/
46-
private function parsePlaceholders(string $specifiersPattern, string $format, bool $isScanf): ?array
60+
private function parsePlaceholders(string $specifiersPattern, string $format): ?array
4761
{
4862
$addSpecifier = '';
4963
if ($this->phpVersion->supportsHhPrintfSpecifier()) {
@@ -72,10 +86,6 @@ private function parsePlaceholders(string $specifiersPattern, string $format, bo
7286
$showValueSuffix = false;
7387

7488
if (isset($placeholder['width']) && $placeholder['width'] !== '') {
75-
if ($isScanf) {
76-
// In scanf, * means assignment suppression - skip this placeholder entirely
77-
continue;
78-
}
7989
$parsedPlaceholders[] = new PrintfPlaceholder(
8090
sprintf('"%s" (width)', $placeholder[0]),
8191
$parameterIdx++,
@@ -136,9 +146,9 @@ private function getAcceptingTypeBySpecifier(string $specifier): string
136146
return 'mixed';
137147
}
138148

139-
private function getPlaceholdersCount(string $specifiersPattern, string $format, bool $isScanf): ?int
149+
private function getPlaceholdersCount(string $specifiersPattern, string $format): ?int
140150
{
141-
$placeholdersMap = $this->parsePlaceholders($specifiersPattern, $format, $isScanf);
151+
$placeholdersMap = $this->parsePlaceholders($specifiersPattern, $format);
142152
if ($placeholdersMap === null) {
143153
return null;
144154
}

‎tests/PHPStan/Rules/Functions/PrintfHelperTest.php‎

Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Rules\Functions;
4+
5+
use Generator;
6+
use IteratorAggregate;
7+
use Override;
8+
use PHPStan\Php\PhpVersion;
9+
use PHPStan\Testing\PHPStanTestCase;
10+
use PHPUnit\Framework\Attributes\CoversMethod;
11+
use PHPUnit\Framework\Attributes\DataProvider;
12+
use PHPUnit\Framework\Attributes\RequiresPhp;
13+
use Throwable;
14+
use ValueError;
15+
use function max;
16+
use function min;
17+
use function range;
18+
use function sprintf;
19+
use function sscanf;
20+
use function strlen;
21+
use const PHP_INT_MAX;
22+
use const PHP_VERSION_ID;
23+
24+
#[CoversMethod(PrintfHelper::class, 'getScanfPlaceholdersCount')]
25+
class PrintfHelperTest extends PHPStanTestCase
26+
{
27+
28+
private PrintfHelper $printf;
29+
30+
#[Override]
31+
protected function setUp(): void
32+
{
33+
$this->printf = $this->getPhpVersionIdAwareHelper(PHP_VERSION_ID);
34+
}
35+
36+
#[RequiresPhp('< 8.0.0')]
37+
public function testReturnsNullForInvalidPatternOnLegacyPhpVersion(): void
38+
{
39+
$this->assertNull($this->printf->getScanfPlaceholdersCount('%a'));
40+
}
41+
42+
#[RequiresPhp('>= 8.0.0')]
43+
public function testReturnsNullForInvalidPatternOnPhp8(): void
44+
{
45+
$this->assertNull($this->printf->getScanfPlaceholdersCount('%a'));
46+
}
47+
48+
#[RequiresPhp('>= 8.0.0')]
49+
#[DataProvider('dataLegacyVersionIds')]
50+
public function testLegacyVersionStillThrowsValueErrorOnPhp8(int $versionId): void
51+
{
52+
$helper = $this->getPhpVersionIdAwareHelper($versionId);
53+
$this->expectException(ValueError::class);
54+
$this->expectExceptionMessage('Bad scan conversion character "a"');
55+
$helper->getScanfPlaceholdersCount('%a');
56+
$this->fail('check your phpunit');
57+
}
58+
59+
private function getPhpVersionIdAwareHelper(int $versionId): PrintfHelper
60+
{
61+
return new PrintfHelper($this->getPhpVersion($versionId));
62+
}
63+
64+
private function getPhpVersion(int $versionId): PhpVersion
65+
{
66+
return new PhpVersion($versionId);
67+
}
68+
69+
public static function dataLegacyVersionIds(): Generator
70+
{
71+
static $line;
72+
$line ??= new class () implements IteratorAggregate {
73+
74+
/** @var array<int, array<int, int>> */
75+
private readonly array $base;
76+
77+
/** @var array<int, int> */
78+
private readonly array $major;
79+
80+
/** @var array<int, int> */
81+
private readonly array $release;
82+
83+
/**
84+
* @return Generator<int,array{int, int, int}, void, void>
85+
*/
86+
#[Override]
87+
public function getIterator(): Generator
88+
{
89+
foreach ($this->major as $major) {
90+
$minors = $this->base[$major];
91+
foreach ($minors as $minor) {
92+
foreach ($this->release as $release) {
93+
yield [$major, $minor, $release];
94+
}
95+
}
96+
}
97+
}
98+
99+
public function __construct()
100+
{
101+
$this->major = range(4, 9);
102+
$this->release = range(0, 48);
103+
/**
104+
* @var array<int, array<int, int>>
105+
*/
106+
$base = [
107+
4 => [3, 4],
108+
5 => range(0, 6),
109+
6 => [],
110+
7 => range(0, 4),
111+
];
112+
foreach ($this->major as $major) {
113+
if (isset($base[$major])) {
114+
continue;
115+
}
116+
117+
// 8: 0...48, 9: 0...36, 10: 0...24, 11...: 0...12
118+
$base[$major] = range(0, max(1, -1 * ($major + -12)) * 12);
119+
}
120+
$this->base = $base;
121+
}
122+
123+
};
124+
125+
$errored = 0;
126+
$skipped = 0;
127+
$matched = 0;
128+
$expectSkipped = 4214;
129+
$expectMatched = 686;
130+
131+
foreach ($line as [$major, $minor, $release]) {
132+
$triplet = [max(1, $major), max(0, min(99, $minor)), max(0, min(99, $release))];
133+
$strval = sprintf('%d%02d%02d', ...$triplet);
134+
$ret = sscanf($strval, '%d%n', $intval, $len);
135+
self::assertSame(2, $ret, 'check your base');
136+
self::assertIsInt($intval, 'check %d specifier php manual and sscanf sources');
137+
self::assertSame(strlen($strval), $len, 'check string: ' . $strval . ' which is of len ' . strlen($strval));
138+
self::assertNotSame(PHP_INT_MAX, $intval, 'IntMax ceiling hit');
139+
self::assertGreaterThan(0, $intval, 'UnsignedInt floor hit');
140+
$self ??= new PrintfHelperTest(__METHOD__);
141+
try {
142+
$version = $self->getPhpVersion($intval);
143+
} catch (Throwable) {
144+
$errored++;
145+
continue;
146+
}
147+
$throws = $version->throwsValueErrorForInternalFunctions();
148+
if ($throws) {
149+
$skipped++;
150+
continue;
151+
}
152+
$matched++;
153+
$i ??= 0;
154+
$i++;
155+
$case = sprintf('case #%02d php %d.%d.%d [version-id %s]', $i, $major, $minor, $release, $strval);
156+
yield $case => [$intval];
157+
}
158+
159+
self::assertSame(0, $errored);
160+
self::assertSame($expectSkipped, $skipped);
161+
self::assertSame($expectMatched, $matched);
162+
}
163+
164+
}

0 commit comments

Comments
 (0)