Skip to content

Fix sprintf() inference for constant values with format-width in pattern #3474

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.

Sign up for GitHub

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 3 commits into from
Sep 26, 2024
Merged

Fix sprintf() inference for constant values with format-width in pattern #3474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6ad1137
Fix sprintf() inference for constant values with format-width in pattern
staabm Sep 23, 2024
f932dab
Update bug-7387.php
staabm Sep 23, 2024
f635b0e
more tests
staabm Sep 24, 2024
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
14 changes: 8 additions & 6 deletions src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,13 +84,13 @@ public function getTypeFromFunctionCall(
}

// 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] !== '') {
if (preg_match('/^%(?P<argnum>[0-9]*\$)?(?P<width>[0-9]*)\.?[0-9]*(?P<specifier>[sbdeEfFgGhHouxX])$/', $constantString->getValue(), $matches) === 1) {
if ($matches['argnum'] !== '') {
// invalid positional argument
if ($matches[1] === '0$') {
if ($matches['argnum'] === '0$') {
return null;
}
$checkArg = intval(substr($matches[1], 0, -1));
$checkArg = intval(substr($matches['argnum'], 0, -1));
} else {
$checkArg = 1;
}
Expand All @@ -103,11 +103,13 @@ public function getTypeFromFunctionCall(
// if the format string is just a placeholder and specified an argument
// of stringy type, then the return value will be of the same type
$checkArgType = $scope->getType($args[$checkArg]->value);
if ($matches[2] === 's'
if (
$matches['specifier'] === 's'
&& ($checkArgType->isConstantValue()->no() || $matches['width'] === '')
Comment on lines +107 to +108
Copy link
Contributor Author

@staabm staabm Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

the actual fix is here. everything else is refactoring/making code easier to read

&& ($checkArgType->isString()->yes() || $checkArgType->isInteger()->yes())
) {
$singlePlaceholderEarlyReturn = $checkArgType->toString();
} elseif ($matches[2] !== 's') {
} elseif ($matches['specifier'] !== 's') {
$singlePlaceholderEarlyReturn = new IntersectionType([
new StringType(),
new AccessoryNumericStringType(),
Expand Down
7 changes: 7 additions & 0 deletions tests/PHPStan/Analyser/nsrt/bug-7387.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ public function positionalArgs($mixed, int $i, float $f, string $s, int $posInt,
assertType('numeric-string', sprintf('%2$14s', $mixed, $intRange));
assertType('non-falsy-string&numeric-string', sprintf('%2$14s', $mixed, $nonZeroIntRange));

assertType("non-falsy-string", sprintf('%2$14s', $mixed, 1));
assertType("non-falsy-string", sprintf('%2$14s', $mixed, '1'));
assertType("non-falsy-string", sprintf('%2$14s', $mixed, 'abc'));
assertType("'1'", sprintf('%2$s', $mixed, 1));
assertType("'1'", sprintf('%2$s', $mixed, '1'));
assertType("'abc'", sprintf('%2$s', $mixed, 'abc'));

assertType('numeric-string', sprintf('%2$.14F', $mixed, $i));
assertType('numeric-string', sprintf('%2$.14F', $mixed, $f));
assertType('numeric-string', sprintf('%2$.14F', $mixed, $s));
Expand Down
Loading