Add stringable-object pseudo type

[rvanvelzen]

This implements the stringable-object type that Psalm also supports. A notable difference is that on PHP 8 it is simply an alias for Stringable.

On top of this type a new rule is also implemented that guard against possibly-invalid object-to-string casts. This only really applies to the simple object type, for which the stringable-ness was not checked yet.

[ondrejmirtes]

Do you see how we could get rid of StringAlwaysAcceptingObjectWithToStringType and these hacks?

phpstan-src/src/Reflection/SignatureMap/NativeFunctionReflectionProvider.php

Lines 109 to 142 in 7e9ab72

	if (
	$parameterSignature->getName() === 'values'
	&& (
	$lowerCasedFunctionName === 'printf'
	|| $lowerCasedFunctionName === 'sprintf'
	)
	) {
	$type = new UnionType([
	new StringAlwaysAcceptingObjectWithToStringType(),
	new IntegerType(),
	new FloatType(),
	new NullType(),
	new BooleanType(),
	]);
	}
	if (
	$parameterSignature->getName() === 'fields'
	&& $lowerCasedFunctionName === 'fputcsv'
	) {
	$type = new ArrayType(
	new UnionType([
	new StringType(),
	new IntegerType(),
	]),
	new UnionType([
	new StringAlwaysAcceptingObjectWithToStringType(),
	new IntegerType(),
	new FloatType(),
	new NullType(),
	new BooleanType(),
	]),
	);
	}

[rvanvelzen]

Do you see how we could get rid of StringAlwaysAcceptingObjectWithToStringType and these hacks?

Yep, working on it :)

[rvanvelzen]

This fails (at least locally) because PHPStan is using the Symfony polyfill's definition of Stringable instead of the built-in :/

[ondrejmirtes]

Yeah, this isn't great, additionally because of the polyfill PHPStan isn't going to tell us about a nonexistent Stringable pre-PHP 8.0: https://phpstan.org/r/9e44f422-93e6-4923-9911-fe6f75940403

The solution is in this class: https://github.com/phpstan/phpstan-src/blob/1.9.x/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php

We'd have to debug the ways Stringable is discovered before PHP 8 and ideally write a new source locator that would wrap an existing one so that it's not discovered by mistake in a polyfill. It's going to be different in phpstan-src (where the polyfill is known through composer.json) and it's going to be different when PHPStan is installed and used as PHAR (where the polyfill is probably known through AutoloadSourceLocator but also maybe AutoloadFunctionsSourceLocator).

Please note that there's already PhpVersionBlacklistSourceLocator that does this job in some cases.

But at the same time, we can't break projects that use the same polyfill and thus can rely on Stringable already existing before PHP 8.

The previous sentence also means we can't break analysis of phpstan-src itself 🤦‍♂️

So what I'd like to see:

1. We need some integration tests around this. Which means to add them here https://github.com/phpstan/phpstan/blob/1.9.x/.github/workflows/other-tests.yml.
   a) Test that runs on PHP 7.4 and references Stringable - expect an error (run with a baseline file and the result should be green - means the error is present)
   b) Test that runs on PHP 8.0 and references Stringable - should be green without a baseline
   c) Test that runs on PHP 7.4, references Stringable, and has polyfills - should be green without a baseline
   d) Test that runs on PHP 8.0, references Stringable, and has polyfills - should be green without a baseline
2. Add a new source locator wrapper for AutoloadSourceLocator so that polyfills from PHPStan's PHAR itself are skipped. Another namespace to skip would be Hoa which is unprefixed in the PHAR because it's an old and messy library.
3. Now we have green tests from 1) but we haven't solved our problem in phpstan-src 🤦‍♂️ But that solution doesn't have to be that clean. We can probably come up with some toggle that's used in https://github.com/phpstan/phpstan-src/blob/1.9.x/src/Testing/TestCase.neon and that makes polyfills being skipped in BetterReflectionSourceLocatorFactory even in phpstan-src.

[VincentLanglet]

Wouldn't it better to write

Suggested change

	if ($objectType->hasMethod('__toString')->maybe()) {
	if ($this->reportMaybe && $objectType->hasMethod('__toString')->maybe()) {

And add the rule on every level ?

This way if someone is level x but use

reportMaybe: true

it will be reported.

And on the opposite side, using

reportMaybe: false

disable the rule.

[ondrejmirtes]

The rule is on level 7. reportMaybes will never be false.

[VincentLanglet]

The rule is on level 7. reportMaybes will never be false.

You can override the value in the config.

I already have/had project with config

parameters:
    level: 6
    reportMaybe: true

or

parameters:
    level: 7
    reportMaybe: false

This is useful when bumping level one by one.
It's a smaller step than bumping from 6 to 7 which is doing

parameters:
	checkUnionTypes: true
	reportMaybes: true
	checkListType: true

I can do one PR 6 + checkList, then later another PR with 6 + checklist and reportMaybe then a last PR with. 6 + everything, and then change the config to the level 7.

I feel like it's easier to opt-in, opt-out the rules when they are config dependent rather than level dependent. But that's a small difference.

[ondrejmirtes]

You're relying on internal config parameters that aren't documented on phpstan.org. This isn't a supported code path.

[VincentLanglet]

You're relying on internal config parameters that aren't documented on phpstan.org. This isn't a supported code path.

Oh, I never thought there was internal config parameter. I see, I understand then.

[VincentLanglet]

But then, the following example https://phpstan.org/r/658bbf1b-b6d6-4b8e-a844-449c42cb75ed
should be reported as an error on a lower level than the level 7

On PHP level 8 the check

$objectType->hasMethod('__toString')->no()

should be true. (I don't know if it does currently)

So it would make sens to have this rule on a lower level with the following check

if (
    $objectType->hasMethod('__toString')->no()
    || $this->reportMaybe && $objectType->hasMethod('__toString')->maybe()
) {

no ?