Skip to content

Commit

Permalink
More precise sprintf() format arg-based return type
Browse files Browse the repository at this point in the history
  • Loading branch information
staabm authored Jul 19, 2024
1 parent 8a1f098 commit 9bd84cf
Show file tree
Hide file tree
Showing 6 changed files with 157 additions and 11 deletions.
88 changes: 82 additions & 6 deletions src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,16 @@
use PHPStan\Type\Accessory\AccessoryNonEmptyStringType;
use PHPStan\Type\Accessory\AccessoryNonFalsyStringType;
use PHPStan\Type\Accessory\AccessoryNumericStringType;
use PHPStan\Type\Constant\ConstantIntegerType;
use PHPStan\Type\Constant\ConstantStringType;
use PHPStan\Type\DynamicFunctionReturnTypeExtension;
use PHPStan\Type\IntegerRangeType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\StringType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use Throwable;
use function array_fill;
use function array_key_exists;
use function array_shift;
use function count;
Expand Down Expand Up @@ -55,13 +58,32 @@ public function getTypeFromFunctionCall(

$formatType = $scope->getType($args[0]->value);
$formatStrings = $formatType->getConstantStrings();
if (count($formatStrings) === 0) {
return null;
}

$singlePlaceholderEarlyReturn = null;
$allPatternsNonEmpty = count($formatStrings) !== 0;
$allPatternsNonFalsy = count($formatStrings) !== 0;
foreach ($formatStrings as $constantString) {
// The printf format is %[argnum$][flags][width][.precision]
$constantParts = $this->getFormatConstantParts(
$constantString->getValue(),
$functionReflection,
$functionCall,
$scope,
);
if ($constantParts !== null) {
if ($constantParts->isNonFalsyString()->yes()) { // phpcs:ignore Generic.CodeAnalysis.EmptyStatement.DetectedIf
// keep all bool flags as is
} elseif ($constantParts->isNonEmptyString()->yes()) {
$allPatternsNonFalsy = false;
} else {
$allPatternsNonEmpty = false;
$allPatternsNonFalsy = false;
}
} else {
$allPatternsNonEmpty = false;
$allPatternsNonFalsy = false;
}

// The printf format is %[argnum$][flags][width][.precision]specifier.
if (preg_match('/^%([0-9]*\$)?[0-9]*\.?[0-9]*([sbdeEfFgGhHouxX])$/', $constantString->getValue(), $matches) === 1) {
if ($matches[1] !== '') {
// invalid positional argument
Expand Down Expand Up @@ -103,12 +125,23 @@ public function getTypeFromFunctionCall(
return $singlePlaceholderEarlyReturn;
}

if ($formatType->isNonFalsyString()->yes()) {
$isNonEmpty = $allPatternsNonEmpty;
if (
count($formatStrings) === 0
&& $functionReflection->getName() === 'sprintf'
&& count($args) === 2
&& $formatType->isNonEmptyString()->yes()
&& $scope->getType($args[1]->value)->isNonEmptyString()->yes()
) {
$isNonEmpty = true;
}

if ($allPatternsNonFalsy) {
$returnType = new IntersectionType([
new StringType(),
new AccessoryNonFalsyStringType(),
]);
} elseif ($formatType->isNonEmptyString()->yes()) {
} elseif ($isNonEmpty) {
$returnType = new IntersectionType([
new StringType(),
new AccessoryNonEmptyStringType(),
Expand All @@ -120,6 +153,49 @@ public function getTypeFromFunctionCall(
return $returnType;
}

/**
* Detect constant strings in the format which neither depend on placeholders nor on given value arguments.
*/
private function getFormatConstantParts(
string $format,
FunctionReflection $functionReflection,
FuncCall $functionCall,
Scope $scope,
): ?ConstantStringType
{
$args = $functionCall->getArgs();
if ($functionReflection->getName() === 'sprintf') {
$valuesCount = count($args) - 1;
} elseif (
$functionReflection->getName() === 'vsprintf'
&& count($args) >= 2
) {
$arraySize = $scope->getType($args[1]->value)->getArraySize();
if (!($arraySize instanceof ConstantIntegerType)) {
return null;
}

$valuesCount = $arraySize->getValue();
} else {
return null;
}

if ($valuesCount <= 0) {
return null;
}
$dummyValues = array_fill(0, $valuesCount, '');

try {
$formatted = @vsprintf($format, $dummyValues);
if ($formatted === false) { // @phpstan-ignore identical.alwaysFalse (PHP7.2 compat)
return null;
}
return new ConstantStringType($formatted);
} catch (Throwable) {
return null;
}
}

/**
* @param Arg[] $args
*/
Expand Down
2 changes: 1 addition & 1 deletion tests/PHPStan/Analyser/nsrt/bug-7387.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function vsprintf(array $array)
assertType('numeric-string', vsprintf("%4d", explode('-', '1988-8-1')));
assertType('numeric-string', vsprintf("%4d", $array));
assertType('numeric-string', vsprintf("%4d", ['123']));
assertType('non-falsy-string', vsprintf("%s", ['123']));
assertType('string', vsprintf("%s", ['123'])); // could be '123'
// too many arguments.. php silently allows it
assertType('numeric-string', vsprintf("%4d", ['123', '456']));
}
Expand Down
32 changes: 31 additions & 1 deletion tests/PHPStan/Analyser/nsrt/non-empty-string.php
Original file line number Diff line number Diff line change
Expand Up @@ -303,9 +303,10 @@ class MoreNonEmptyStringFunctions

/**
* @param non-empty-string $nonEmpty
* @param non-falsy-string $nonFalsy
* @param '1'|'2'|'5'|'10' $constUnion
*/
public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUnion)
public function doFoo(string $s, string $nonEmpty, string $nonFalsy, int $i, bool $bool, $constUnion)
{
assertType('string', addslashes($s));
assertType('non-empty-string', addslashes($nonEmpty));
Expand Down Expand Up @@ -350,9 +351,20 @@ public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUni

assertType('string', sprintf($s));
assertType('string', sprintf($nonEmpty));
assertType('string', sprintf($s, $nonEmpty));
assertType('string', sprintf($nonEmpty, $s));
assertType('string', sprintf($s, $nonFalsy));
assertType('string', sprintf($nonFalsy, $s));
assertType('non-empty-string', sprintf($nonEmpty, $nonEmpty));
assertType('non-empty-string', sprintf($nonEmpty, $nonFalsy));
assertType('non-empty-string', sprintf($nonFalsy, $nonEmpty));
assertType('string', vsprintf($s, []));
assertType('string', vsprintf($nonEmpty, []));

assertType('non-empty-string', sprintf("%s0%s", $s, $s));
assertType('non-empty-string', sprintf("%s0%s%s%s%s", $s, $s, $s, $s, $s));
assertType('non-empty-string', sprintf("%s0%s%s%s%s%s", $s, $s, $s, $s, $s, $s));

assertType('0', strlen(''));
assertType('5', strlen('hallo'));
assertType('int<0, 1>', strlen($bool));
Expand All @@ -374,4 +386,22 @@ public function doFoo(string $s, string $nonEmpty, int $i, bool $bool, $constUni
assertType('string', str_repeat($s, $i));
}

function multiplesPrintfFormats(string $s) {
$maybeNonEmpty = '%s';
$maybeNonFalsy = '%s';
$nonEmpty = '%s0';
$nonFalsy = '%sAA';

if (rand(0,1)) {
$maybeNonEmpty = '%s0';
$maybeNonFalsy = '%sAA';
$nonEmpty = '0%s';
$nonFalsy = 'AA%s';
}

assertType('string', sprintf($maybeNonEmpty, $s));
assertType('string', sprintf($maybeNonFalsy, $s));
assertType('non-empty-string', sprintf($nonEmpty, $s));
assertType('non-falsy-string', sprintf($nonFalsy, $s));
}
}
16 changes: 13 additions & 3 deletions tests/PHPStan/Analyser/nsrt/non-falsy-string.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,22 @@ function stringFunctions(string $s, $nonFalsey, $arrayOfNonFalsey, $nonEmptyArra
assertType('string', sprintf($nonFalsey));
assertType("'foo'", sprintf('foo'));
assertType("string", sprintf(...$arr));
assertType("non-falsy-string", sprintf('%s', ...$arr)); // should be 'string'
assertType("string", sprintf('%s', ...$arr));

// empty array only works as long as no placeholder in the pattern
assertType('string', vsprintf($nonFalsey, []));
assertType('string', vsprintf($nonFalsey, []));
assertType("non-falsy-string", vsprintf('foo', [])); // should be 'foo'
assertType("non-falsy-string", vsprintf('%s', ...$arr)); // should be 'string'
assertType("string", vsprintf('foo', []));

assertType("string", vsprintf('%s', ...$arr));
assertType("string", vsprintf(...$arr));
assertType('non-falsy-string', vsprintf('%sAA%s', [$s, $s]));
assertType('non-falsy-string', vsprintf('%d%d', [$s, $s])); // could be non-falsy-string&numeric-string

assertType('non-falsy-string', sprintf("%sAA%s", $s, $s));
assertType('non-falsy-string', sprintf("%d%d", $s, $s)); // could be non-falsy-string&numeric-string
assertType('non-falsy-string', sprintf("%sAA%s%s%s%s", $s, $s, $s, $s, $s));
assertType('non-falsy-string', sprintf("%sAA%s%s%s%s%s", $s, $s, $s, $s, $s, $s));

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1052,4 +1052,10 @@ public function testBug10697(): void
$this->analyse([__DIR__ . '/data/bug-10697.php'], []);
}

public function testBug10493(): void
{
$this->checkAlwaysTrueStrictComparison = true;
$this->analyse([__DIR__ . '/data/bug-10493.php'], []);
}

}
24 changes: 24 additions & 0 deletions tests/PHPStan/Rules/Comparison/data/bug-10493.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php // lint >= 8.1

namespace Bug10493;

class Foo
{
public function __construct(
private readonly ?string $old,
private readonly ?string $new,
)
{
}

public function foo(): ?string
{
$return = sprintf('%s%s', $this->old, $this->new);

if ($return === '') {
return null;
}

return $return;
}
}

0 comments on commit 9bd84cf

Please sign in to comment.