Skip to content

Commit 04f1759

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 04f1759

2 files changed

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

0 commit comments

Comments
 (0)