V2 false positive type error within is_resource conditional

[bshaffer]

Bug report

See the failing phpstan run

We are getting a type error within a union type because "resource" is not supported, even when the call is wrapped in if (!is_resource) { ... }.

This is a regression since v1

Code snippet that reproduces the problem

https://phpstan.org/r/73167be4-3cfc-43a8-8613-9041aa97a126

Expected output

The issue reported is incorrect

Did PHPStan help you today? Did it make you happy in any way?

Yes, it made me very happy 😊

[ondrejmirtes]

This is expected behaviour. is_resource returns false for closed resources, so even after this condition, the type can still be resource. It's better to ask for is_string: https://phpstan.org/r/55d2587b-6e66-45aa-b3b6-b5f454f303bb

[bshaffer]

I think this may be due to the fact that is_resource returns false for closed resources, and so is not sufficient for type safety. I don't know what the alternative here is, however, and I suggest a better error message if possible!

[bshaffer]

@ondrejmirtes if that's the case then why does it still throw the error for open-resource?

https://phpstan.org/r/8e923c21-dc2f-4ccc-af5e-ac29f78f0468

[herndlm]

Those are currently just aliases for resource basically. I wonder if it makes sense and is worth to add the open/closed state to the resource type and use it for cases like these 🤔

[bshaffer]

@herndlm that would certainly be helpful in my case - I can't just change the conditional for a patch release, so I'll have to roll phpstan back :-/

[herndlm]

I'll look into it and will include you in the PR if it works. Currently thinking about adding a isClosed trinary to ResourceType and making use of that in type narrowing 🤞

[herndlm]

fyi I did some digging and playing around with an implementation, but changing something as big as I was thinking seems too risky and not worth it. E.g. was thinking of a) adding additional type information to ResourceType or b) creating a dedicated ClosedResourceType (as I also saw it in psalm). It became clear pretty fast that there will be various issues with the type system because either a resource stays a super type of closed resource (= closed resource is still a resource) or not. with the later, overrides for type narrowing would be needed which again causes other issues, weirdly looking types or a closed resource type magically appearing after a !is_resource() check.

psalm is separating those 2 currently, which feels somewhat reasonable to me, but there are also issues reported there, see e.g. vimeo/psalm#5854 or vimeo/psalm#2763.

I think both solutions have issues and maybe it's best to quote @muglug from one of the above links

Given that resources are effectively on their way out I won't be fixing this, but you can work around it

I feel whatever I do here will break something :/

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.