Skip to content

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)
@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.

@staabm staabm force-pushed the vprintf branch 2 times, most recently from ecf35bc to c0ac13c Compare June 10, 2024 09:33
@staabm staabm force-pushed the vprintf branch 2 times, most recently from 323ad98 to eb1e62d Compare June 19, 2024 10:17
@staabm staabm force-pushed the vprintf branch 2 times, most recently from 6085104 to d14b403 Compare July 15, 2024 05:01
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
@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

3 participants