Skip to content

Commit 9bd84cf

Browse files
staabmstaabm
authored
More precise sprintf() format arg-based return type
1 parent 8a1f098 commit 9bd84cf

File tree

6 files changed

+157
-11
lines changed

6 files changed

+157
-11
lines changed

‎src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php

Lines changed: 82 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,16 @@
1111
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
1212
use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
1313
use PHPStan\Type\Accessory\AccessoryNumericStringType;
14+
use PHPStan\Type\Constant\ConstantIntegerType;
15+
use PHPStan\Type\Constant\ConstantStringType;
1416
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
1517
use PHPStan\Type\IntegerRangeType;
1618
use PHPStan\Type\IntersectionType;
1719
use PHPStan\Type\StringType;
1820
use PHPStan\Type\Type;
1921
use PHPStan\Type\TypeCombinator;
2022
use Throwable;
23+
use function array_fill;
2124
use function array_key_exists;
2225
use function array_shift;
2326
use function count;
@@ -55,13 +58,32 @@ public function getTypeFromFunctionCall(
5558

5659
$formatType = $scope->getType($args[0]->value);
5760
$formatStrings = $formatType->getConstantStrings();
58-
if (count($formatStrings) === 0) {
59-
return null;
60-
}
6161

6262
$singlePlaceholderEarlyReturn = null;
63+
$allPatternsNonEmpty = count($formatStrings) !== 0;
64+
$allPatternsNonFalsy = count($formatStrings) !== 0;
6365
foreach ($formatStrings as $constantString) {
64-
// The printf format is %[argnum$][flags][width][.precision]
66+
$constantParts = $this->getFormatConstantParts(
67+
$constantString->getValue(),
68+
$functionReflection,
69+
$functionCall,
70+
$scope,
71+
);
72+
if ($constantParts !== null) {
73+
if ($constantParts->isNonFalsyString()->yes()) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedIf
74+
// keep all bool flags as is
75+
} elseif ($constantParts->isNonEmptyString()->yes()) {
76+
$allPatternsNonFalsy = false;
77+
} else {
78+
$allPatternsNonEmpty = false;
79+
$allPatternsNonFalsy = false;
80+
}
81+
} else {
82+
$allPatternsNonEmpty = false;
83+
$allPatternsNonFalsy = false;
84+
}
85+
86+
// The printf format is %[argnum$][flags][width][.precision]specifier.
6587
if (preg_match('/^%([0-9]*\$)?[0-9]*\.?[0-9]*([sbdeEfFgGhHouxX])$/', $constantString->getValue(), $matches) === 1) {
6688
if ($matches[1] !== '') {
6789
// invalid positional argument
@@ -103,12 +125,23 @@ public function getTypeFromFunctionCall(
103125
return $singlePlaceholderEarlyReturn;
104126
}
105127

106-
if ($formatType->isNonFalsyString()->yes()) {
128+
$isNonEmpty = $allPatternsNonEmpty;
129+
if (
130+
count($formatStrings) === 0
131+
&& $functionReflection->getName() === 'sprintf'
132+
&& count($args) === 2
133+
&& $formatType->isNonEmptyString()->yes()
134+
&& $scope->getType($args[1]->value)->isNonEmptyString()->yes()
135+
) {
136+
$isNonEmpty = true;
137+
}
138+
139+
if ($allPatternsNonFalsy) {
107140
$returnType = new IntersectionType([
108141
new StringType(),
109142
new AccessoryNonFalsyStringType(),
110143
]);
111-
} elseif ($formatType->isNonEmptyString()->yes()) {
144+
} elseif ($isNonEmpty) {
112145
$returnType = new IntersectionType([
113146
new StringType(),
114147
new AccessoryNonEmptyStringType(),
@@ -120,6 +153,49 @@ public function getTypeFromFunctionCall(
120153
return $returnType;
121154
}
122155

156+
/**
157+
* Detect constant strings in the format which neither depend on placeholders nor on given value arguments.
158+
*/
159+
private function getFormatConstantParts(
160+
string $format,
161+
FunctionReflection $functionReflection,
162+
FuncCall $functionCall,
163+
Scope $scope,
164+
): ?ConstantStringType
165+
{
166+
$args = $functionCall->getArgs();
167+
if ($functionReflection->getName() === 'sprintf') {
168+
$valuesCount = count($args) - 1;
169+
} elseif (
170+
$functionReflection->getName() === 'vsprintf'
171+
&& count($args) >= 2
172+
) {
173+
$arraySize = $scope->getType($args[1]->value)->getArraySize();
174+
if (!($arraySize instanceof ConstantIntegerType)) {
175+
return null;
176+
}
177+
178+
$valuesCount = $arraySize->getValue();
179+
} else {
180+
return null;
181+
}
182+
183+
if ($valuesCount <= 0) {
184+
return null;
185+
}
186+
$dummyValues = array_fill(0, $valuesCount, '');
187+
188+
try {
189+
$formatted = @vsprintf($format, $dummyValues);
190+
if ($formatted === false) { // @phpstan-ignore identical.alwaysFalse (PHP7.2 compat)
191+
return null;
192+
}
193+
return new ConstantStringType($formatted);
194+
} catch (Throwable) {
195+
return null;
196+
}
197+
}
198+
123199
/**
124200
* @param Arg[] $args
125201
*/

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public function vsprintf(array $array)
8888
assertType('numeric-string', vsprintf("%4d", explode('-', '1988-8-1')));
8989
assertType('numeric-string', vsprintf("%4d", $array));
9090
assertType('numeric-string', vsprintf("%4d", ['123']));
91-
assertType('non-falsy-string', vsprintf("%s", ['123']));
91+
assertType('string', vsprintf("%s", ['123'])); // could be '123'
9292
// too many arguments.. php silently allows it
9393
assertType('numeric-string', vsprintf("%4d", ['123', '456']));
9494
}

‎tests/PHPStan/Analyser/nsrt/non-empty-string.php

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,9 +303,10 @@ class MoreNonEmptyStringFunctions
303303

304304
/**
305305
* @param non-empty-string $nonEmpty
306+
* @param non-falsy-string $nonFalsy
306307
* @param '1'|'2'|'5'|'10' $constUnion
307308
*/
308-
public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUnion)
309+
public function doFoo(string $s, string $nonEmpty, string $nonFalsy, int $i, bool $bool, $constUnion)
309310
{
310311
assertType('string', addslashes($s));
311312
assertType('non-empty-string', addslashes($nonEmpty));
@@ -350,9 +351,20 @@ public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUni
350351

351352
assertType('string', sprintf($s));
352353
assertType('string', sprintf($nonEmpty));
354+
assertType('string', sprintf($s, $nonEmpty));
355+
assertType('string', sprintf($nonEmpty, $s));
356+
assertType('string', sprintf($s, $nonFalsy));
357+
assertType('string', sprintf($nonFalsy, $s));
358+
assertType('non-empty-string', sprintf($nonEmpty, $nonEmpty));
359+
assertType('non-empty-string', sprintf($nonEmpty, $nonFalsy));
360+
assertType('non-empty-string', sprintf($nonFalsy, $nonEmpty));
353361
assertType('string', vsprintf($s, []));
354362
assertType('string', vsprintf($nonEmpty, []));
355363

364+
assertType('non-empty-string', sprintf("%s0%s", $s, $s));
365+
assertType('non-empty-string', sprintf("%s0%s%s%s%s", $s, $s, $s, $s, $s));
366+
assertType('non-empty-string', sprintf("%s0%s%s%s%s%s", $s, $s, $s, $s, $s, $s));
367+
356368
assertType('0', strlen(''));
357369
assertType('5', strlen('hallo'));
358370
assertType('int<0, 1>', strlen($bool));
@@ -374,4 +386,22 @@ public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUni
374386
assertType('string', str_repeat($s, $i));
375387
}
376388

389+
function multiplesPrintfFormats(string $s) {
390+
$maybeNonEmpty = '%s';
391+
$maybeNonFalsy = '%s';
392+
$nonEmpty = '%s0';
393+
$nonFalsy = '%sAA';
394+
395+
if (rand(0,1)) {
396+
$maybeNonEmpty = '%s0';
397+
$maybeNonFalsy = '%sAA';
398+
$nonEmpty = '0%s';
399+
$nonFalsy = 'AA%s';
400+
}
401+
402+
assertType('string', sprintf($maybeNonEmpty, $s));
403+
assertType('string', sprintf($maybeNonFalsy, $s));
404+
assertType('non-empty-string', sprintf($nonEmpty, $s));
405+
assertType('non-falsy-string', sprintf($nonFalsy, $s));
406+
}
377407
}

‎tests/PHPStan/Analyser/nsrt/non-falsy-string.php

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,22 @@ function stringFunctions(string $s, $nonFalsey, $arrayOfNonFalsey, $nonEmptyArra
107107
assertType('string', sprintf($nonFalsey));
108108
assertType("'foo'", sprintf('foo'));
109109
assertType("string", sprintf(...$arr));
110-
assertType("non-falsy-string", sprintf('%s', ...$arr)); // should be 'string'
110+
assertType("string", sprintf('%s', ...$arr));
111+
112+
// empty array only works as long as no placeholder in the pattern
111113
assertType('string', vsprintf($nonFalsey, []));
112114
assertType('string', vsprintf($nonFalsey, []));
113-
assertType("non-falsy-string", vsprintf('foo', [])); // should be 'foo'
114-
assertType("non-falsy-string", vsprintf('%s', ...$arr)); // should be 'string'
115+
assertType("string", vsprintf('foo', []));
116+
117+
assertType("string", vsprintf('%s', ...$arr));
115118
assertType("string", vsprintf(...$arr));
119+
assertType('non-falsy-string', vsprintf('%sAA%s', [$s, $s]));
120+
assertType('non-falsy-string', vsprintf('%d%d', [$s, $s])); // could be non-falsy-string&numeric-string
121+
122+
assertType('non-falsy-string', sprintf("%sAA%s", $s, $s));
123+
assertType('non-falsy-string', sprintf("%d%d", $s, $s)); // could be non-falsy-string&numeric-string
124+
assertType('non-falsy-string', sprintf("%sAA%s%s%s%s", $s, $s, $s, $s, $s));
125+
assertType('non-falsy-string', sprintf("%sAA%s%s%s%s%s", $s, $s, $s, $s, $s, $s));
116126

117127
assertType('int<1, max>', strlen($nonFalsey));
118128

‎tests/PHPStan/Rules/Comparison/StrictComparisonOfDifferentTypesRuleTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1052,4 +1052,10 @@ public function testBug10697(): void
10521052
$this->analyse([__DIR__ . '/data/bug-10697.php'], []);
10531053
}
10541054

1055+
public function testBug10493(): void
1056+
{
1057+
$this->checkAlwaysTrueStrictComparison = true;
1058+
$this->analyse([__DIR__ . '/data/bug-10493.php'], []);
1059+
}
1060+
10551061
}

‎tests/PHPStan/Rules/Comparison/data/bug-10493.php

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<?php // lint >= 8.1
2+
3+
namespace Bug10493;
4+
5+
class Foo
6+
{
7+
public function __construct(
8+
private readonly ?string $old,
9+
private readonly ?string $new,
10+
)
11+
{
12+
}
13+
14+
public function foo(): ?string
15+
{
16+
$return = sprintf('%s%s', $this->old, $this->new);
17+
18+
if ($return === '') {
19+
return null;
20+
}
21+
22+
return $return;
23+
}
24+
}

0 commit comments

Comments
 (0)