Report invalid printf placeholder#4478
Conversation
e13d386 to
5603955
Compare
|
This pull request has been marked as ready for review. |
| if ($count === null) { | ||
| return [ | ||
| RuleErrorBuilder::message(sprintf( | ||
| 'Call to %s contains an invalid placeholder.', |
There was a problem hiding this comment.
Would be nice to know what the invalid placeholder is and its position. Otherwise it'd be hard to debug with long format strings as %c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%c%.
There was a problem hiding this comment.
I thought about this and I was wondering if we should iterate and add more details with acceptsReasonsTip later because:
-
This would require some weird signature for getPrintfPlaceholdersCount (Returning the count + the invalid placeholders/positions)
-
This would require to handle error message for
- Union string
%c%|%c%c%=> What would be the message ? 3rd position ? 5th position ? - String with multiple wrong placeholder like
%%% %%%=> Should we report all the invalid ones ?
- Union string
=> maybe you already have an idea about the format of the message @ondrejmirtes ?
|
Also maybe this belongs to bleeding edge only? |
I would have consider this kinda as a "bugfix". I can put this behind bleeding edge, but that would mean adding an extra parameters to |
5603955 to
294b418
Compare
294b418 to
c8b5089
Compare
| RuleErrorBuilder::message(sprintf( | ||
| 'Call to %s contains an invalid placeholder.', | ||
| $name, | ||
| ))->identifier(sprintf('argument.%s', $name))->build(), |
There was a problem hiding this comment.
What's this identifier going to be?
There was a problem hiding this comment.
I copied the existing one
The identifier will be argument.vprintf or argument.vsprintf.
| RuleErrorBuilder::message(sprintf( | ||
| 'Call to %s contains an invalid placeholder.', | ||
| $name, | ||
| ))->identifier(sprintf('argument.%s', $name))->build(), |
There was a problem hiding this comment.
What's this identifier going to be?
There was a problem hiding this comment.
I copied the existing one
Identifier will be argument.printf and similar
|
Thank you! |
Closes phpstan/phpstan#1889
Closes phpstan/phpstan#8547 (cc @staabm)