Skip to content

More precise sprintf() format-arg based return type #3192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixes previous too specific types

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;
}
}
Loading