Skip to content
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

vprintf/vsprintf require a non-empty-array argument #3126

Merged
merged 14 commits into from
Jul 19, 2024

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Jun 4, 2024

these functions throw a ValueError when called with an empty array, see

@staabm
Copy link
Contributor Author

staabm commented Jun 4, 2024

should SprintfFunctionDynamicReturnTypeExtension return a ErrorType in these cases?

@ondrejmirtes
Copy link
Member

Your example is misleading: https://3v4l.org/Mvl72

It'd make much more sense to create a rule similar to PrintfParametersRule.

@clxmstaab clxmstaab force-pushed the vprintf branch 2 times, most recently from 08e8aa1 to faf6099 Compare June 4, 2024 16:31
Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

I don't see tests for general arrays

  1. Zero placeholders + array (it's okay)
  2. One or more placeholders + array (not okay)
  3. One or more placeholders + non-empty-array (okay)

conf/config.level0.neon Outdated Show resolved Hide resolved
src/Rules/Functions/PrintfArrayParametersRule.php Outdated Show resolved Hide resolved
src/Rules/Functions/PrintfArrayParametersRule.php Outdated Show resolved Hide resolved
@staabm staabm marked this pull request as ready for review June 5, 2024 09:44
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

conf/config.level0.neon Outdated Show resolved Hide resolved
src/Rules/Functions/PrintfArrayParametersRule.php Outdated Show resolved Hide resolved
src/Rules/Functions/PrintfArrayParametersRule.php Outdated Show resolved Hide resolved
src/Rules/Functions/PrintfArrayParametersRule.php Outdated Show resolved Hide resolved
src/Rules/Functions/PrintfParametersRule.php Outdated Show resolved Hide resolved
}

if ($placeHoldersCount instanceof IntegerRangeType
&& $formatArgsCount instanceof IntegerRangeType
Copy link
Member

Choose a reason for hiding this comment

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

What about handling $placeHoldersCount as IntegerRangeType + $formarArgsCount as ConstantIntegerType?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

if I don't oversee anything, this IF statement here will cover the remaining cases.

I simplified it further.

@staabm staabm marked this pull request as draft July 17, 2024 15:23
@staabm staabm marked this pull request as ready for review July 17, 2024 15:35
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes ondrejmirtes merged commit e35eae4 into phpstan:1.11.x Jul 19, 2024
447 of 454 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

@staabm staabm deleted the vprintf branch July 19, 2024 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants