How do you handle PHP 8 resources-which-got-converted-to-objects?

[Seldaek]

For example this:

https://phpstan.org/r/6463971d-0d60-4183-b577-d486bf602e63

On PHP 8, socket_create returns a Socket. Should one type Socket|resource|false? I need compatibility with PHP 7 and 8.

[hrach]

I was trying to convice Ondrej to provide php-version-aware ignore comments to allow this but it seems that Ondrej's preferred way is including proper configs with ignored errors. No-go for me.

[ondrejmirtes]

Hi,
yeah, Socket|resource|false is most precise if you need to support both PHP 7 and 8.

Currently the best way to run PHPStan on such codebases is to run it twice, for both PHP 7 and PHP 8. You'll have to run it with different config files as some code is marked as invalid for PHP 7 (for example the Socket class doesn't exist there), and some code is not necessary on PHP 8, like checking for === false for functions that no longer return false.

Unfortunately that's currently the best way to do it, but not ideal. PHPStan currently supports only a single PHP version to consider for a codebase, not a range, as I explained here at length: #4060 (comment)

At least you can reuse the common parts of configuration files with includes: https://phpstan.org/config-reference#multiple-files

I also promised that if there's enough requests for this, I'll make the considered PHP versions a range, so that, but it will require some rewriting of PHPStan core.

[Seldaek]

Ok thanks, I plan to only run phpstan on PHP 8 going forward, it's maybe less correct strictly speaking but IMO good enough and saves some time dealing with conditional errors etc.

[dktapps]

My preferred solution (which avoids masking false positives):

phpstan.php7.neon

includes:
	- phpstan.shared.neon

parameters:
	typeAliases:
		PhpSocket: resource

phpstan.php8.neon:

includes:
	- phpstan.shared.neon

parameters:
	typeAliases:
		PhpSocket: '\Socket'

and then annotate using @phpstan-var PhpSocket.

[ondrejmirtes]

That's a hack I can't recommend.

[dktapps]

Well, it seems to me there's no non-hack solution to this problem. At least this one doesn't require storing reams of ignoreErrors for different versions.

[AJenbo]

@ondrejmirtes would it be possible to make PHPStan ignore content inside blocks guarded by version checks?

/** @var mixed */
$socket = socket_create(AF_INET, SOCK_DGRAM, SOL_UDP);
if (version_compare(PHP_VERSION, '8.0.0', '<') {
    if (!is_resource($socket)) return;
} else {
    if (!$socket instanceof \Socket) return;
}
socket_accept($socket);

That way we can have each run use the same config and just have a simple version matrix in the CI.
This also avoid developers having to be aware of what version and config they should use locally, not to mention the IDE integrations.

[AJenbo]

The only other option I can think of is to add support for version in the phpstan.neon file

		-
			message: "#^Call to function is_bool\\(\\) with string will always evaluate to false\\.$#"
			count: 1
			php: "~8.0.0"
			path: ../src/Type/Php/SubstrDynamicReturnTypeExtension.php

[ondrejmirtes]

What I do in PHPStan is this: https://github.com/phpstan/phpstan-src/blob/1.11.x/build/ignore-by-php-version.neon.php

You can reference a file like this from your phpstan.neon.

[AJenbo]

Hm I wasn't aware that it was possible to include dynamic scripts as configs, that does sort of cover my last suggestion. Though I would still prefer a system where I don't have to balance the config and code carefully against each other and can rather have code that follows the general rules and limtes imposed (I think PHPStan does a really god job her in general).

[staabm]

@ondrejmirtes I wonder whether it would be possible/useful to narrow PHPStan internal PhpVersion in blocks like

if (version_compare(PHP_VERSION, '8.0.0', '<') {
    // xx
}

so extensions and other stuff depending on PhpVersion would work on this more precise version.
I guess one problem would be, that PHPStan signatures and stubs would need to change while analysis.

[ondrejmirtes]

We'd have to have PhpVersion on Scope instead of an injected service.

We probably wouldn't get rid of PhpVersion as a DI service everywhere, but rules should definitely access it on Scope.