Allow only bool|float|int|string for sprintf()

[simPod]

Allow only bool|float|int|string for sprintf()

[simPod]

is this doable? Should I continue with fixing tests?

[Kharhamel]

Hello, sorry for the late reply, i didn't get notified for some reason.
What are your trying to do? Restrict the type for ...$args? I don't know if it is doable, it depends how phpstan or psalm consider types on destructured arguments.

[simPod]

Yes exactly. Because now it allows me to pass whatever and static analysis cannot catch it. I get notified on runtime and that's already late.

Eg. passing an object without __toString() into Safe\sprintf() passes phpstan checks because $args are typed as mixed.

[Kharhamel]

About your failed pipeline, i think the reason the test fails is that it cannot find your new sprintf().

The sprintf() used by the test is fetch by 'require_once DIR . '/../../generated/strings.php';' which doesn't match anymore since your new sprintf lives in 'lib/special_cases.php', so you need to edit the require_once

[simPod]

Thanks. Ok, so it sounds viable and I can proceed with this?

[Kharhamel]

Honestly, no idea. You should ask the guys from psalm or phpstan to know if this is possible.

From the point of view of safe, this is actually pretty tricky to test because the php language itself wont throw errors if you don't respect docblocks.
You need to test than phpstan create an error, which means you have to run phpstan in the test and check its result. I think it is already done somewhere but I don't remember, maybe @moufmouf does.

[Kharhamel]

To be fair, if you are sure than phpstan and psalm will work with your new param then you don't really need to test it. You can just edit the require to make the test succesfull again and be done with it

[Kharhamel]

Another things you can try, which may be easier and cleaner, is to edit CustomPhpStanFunctionMap.php instead.

We actually use the stubs from phpstan to generate the typehints and the docblock of our generated functions. The file itself allow us to overwrite phpstan stubs.

So you simply add a new line in CustomPhpStanFunctionMap for sprintf with your new type bool|float|int|string. It is cleaner because you don't have to put sprintf in the special_case file. The only possible issue is, I don't know if you can write '...args' in the stubs. But I think you should try this way instead

[simPod]

Yea I tested it so that's np. I was rather wondering about the way I solved it here, did not find easier way to fix the typehints then to redefine the function.

edit CustomPhpStanFunctionMap.php instead.

this sounds the best so far, thanks! will try

[Kharhamel]

Hello @simPod Did you manage to make it work?

[simPod]

@Kharhamel yes, I'm still into it. Are you asking to check whether this is stale or need to move it forward?

[Kharhamel]

No just checking if it is stale don't worry

[Kharhamel]

Hello @simPod, are you still on it?

[simPod]

I needed to resolve these PRs but unfortunately there's no activity.

php/phd#24
php/doc-en#55
salathe/phpdoc-base#2
php/doc-en#57

[Kharhamel]

If you are blocked by the doc not being updated, you can overload the typings in this file: CustomPhpStanFunctionMap.php
See my previous comment:

Another things you can try, which may be easier and cleaner, is to edit CustomPhpStanFunctionMap.php instead.

We actually use the stubs from phpstan to generate the typehints and the docblock of our generated functions. The file itself allow us to overwrite phpstan stubs.

So you simply add a new line in CustomPhpStanFunctionMap for sprintf with your new type bool|float|int|string. It is cleaner because you don't have to put sprintf in the special_case file. The only possible issue is, I don't know if you can write '...args' in the stubs. But I think you should try this way instead

[Kharhamel]

@simPod I am closing this PR since it is too old. If you still want to implement this feature, you better start over to not have conflicts.