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

psalm insists that printf() needs at least two arguments #9987

Closed
pilif opened this issue Jul 3, 2023 · 4 comments · Fixed by #9993 or #10088
Closed

psalm insists that printf() needs at least two arguments #9987

pilif opened this issue Jul 3, 2023 · 4 comments · Fixed by #9993 or #10088

Comments

@pilif
Copy link
Contributor

pilif commented Jul 3, 2023

https://psalm.dev/r/fbda7f627d

this is totally valid code. Now granted, echo could have been used, but it's totally valid.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/fbda7f627d
<?php

printf("a");
Psalm output (using commit 8d1876a):

ERROR: TooFewArguments - 3:1 - Too few arguments for printf, expecting at least 2 arguments

@kkmuffme
Copy link
Contributor

kkmuffme commented Jul 3, 2023

Yeah that was kind of the point tbh, but it is just a side-effect of sprintf being analyzed (since sprintf without 2 args makes no sense, just save the sprintf call)

I'll fix this quickly

kkmuffme added a commit to kkmuffme/psalm that referenced this issue Jul 3, 2023
@kkmuffme
Copy link
Contributor

kkmuffme commented Jul 6, 2023

@orklah sorry to reopen this, but I think this error makes sense actually.

Practically, using printf is fine, however this usually indicates a bug/something was forgotten when refactoring the code. If you really use printf with only 1 arg, you should rather use print() (or echo) instead of printf.

Could you revert the merged #9993 ?

@orklah
Copy link
Collaborator

orklah commented Jul 6, 2023

Well, in principle I'd disagree (evidence A: an issue has been open days after release on that topic)

However, it seems master still emits an issue for that snippet saying that there is no placeholder in the first arg. So we're kinda in a middle ground

It would be better if it had a specific issue to be help ignore this issue for people that uses printf that way but the current issue is explicit enough to help find a potential issue

kkmuffme added a commit to kkmuffme/psalm that referenced this issue Aug 4, 2023
Fix vimeo#10021
Fix vimeo#9987 again now for all cases (specifically vimeo#9987 (comment))

Use RedundantFunctionCall instead of InvalidArgument, where it's technically valid.
Report TooManyArguments when a format without placeholders is used
Report an error for splat with vprintf/vsprintf
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