Skip to content

support positional arguments in sprintf() constant format inference#1426

Merged
ondrejmirtes merged 4 commits intophpstan:1.7.xfrom
staabm:positional-sprintf
Jun 17, 2022
Merged

support positional arguments in sprintf() constant format inference#1426
ondrejmirtes merged 4 commits intophpstan:1.7.xfrom
staabm:positional-sprintf

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Jun 15, 2022

as requested in #1410 (comment)

with this PR we still don't support the whole glory of sprintf() - but I think we covered most use-cases as of this PR.

Copy link
Copy Markdown
Contributor Author

@staabm staabm Jun 15, 2022

Choose a reason for hiding this comment

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

3v4l seems to be down/not working atm.. I verified the test-script locally

@clxmstaab clxmstaab force-pushed the positional-sprintf branch from 6805787 to 5a19970 Compare June 15, 2022 15:46
@staabm staabm force-pushed the positional-sprintf branch from bd4537c to 21ffa2c Compare June 16, 2022 18:51
@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Jun 17, 2022

Build error seems unrelated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

%0$s isn't valid, please modify this part of the regex (and a test for this scenario)

%(?:[1-9][0-9]*\$)?

Thanks :)

Copy link
Copy Markdown
Contributor Author

@staabm staabm Jun 17, 2022

Choose a reason for hiding this comment

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

fixed. instead of changing the suggested regex, I left it as is and if-cased the 0, because otherwise a non-empty-string would have been returned by the followup logic, instead of the default-return-type

@staabm staabm force-pushed the positional-sprintf branch from 2cf716f to 57ae02c Compare June 17, 2022 12:44
@ondrejmirtes ondrejmirtes merged commit d6bd267 into phpstan:1.7.x Jun 17, 2022
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

@staabm staabm deleted the positional-sprintf branch June 17, 2022 13:07
@ondrejmirtes
Copy link
Copy Markdown
Member

FYI had to revert this: 82583cf

Because of:

Warning: Undefined array key 1 in /Users/ondrej/Development/phpstan/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php on line 49
PHP Warning:  Undefined array key 1 in /Users/ondrej/Development/phpstan/src/Type/Php/SprintfFunctionDynamicReturnTypeExtension.php on line 49

That shows up when running make tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants