Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Add stringable-object pseudo type #1905

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

Draft
rvanvelzen wants to merge 5 commits into phpstan:1.9.x
base: 1.9.x
Choose a base branch
Loading
from rvanvelzen:stringable-object
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Specify Stringable from method_exists() as well
  • Loading branch information
rvanvelzen committed Oct 25, 2022
commit d407bc26e745518b99bdacb845d3a16b9c4e7ded
23 changes: 19 additions & 4 deletions src/Type/Php/MethodExistsTypeSpecifyingExtension.php
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@
use PHPStan\Type\IntersectionType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\ObjectWithoutClassType;
use PHPStan\Type\Type;
use PHPStan\Type\UnionType;
use function count;
use function strtolower;

class MethodExistsTypeSpecifyingExtension implements FunctionTypeSpecifyingExtension, TypeSpecifierAwareExtension
{
Expand Down Expand Up @@ -62,10 +64,7 @@ public function specifyTypes(
return $this->typeSpecifier->create(
$node->getArgs()[0]->value,
new UnionType([
new IntersectionType([
new ObjectWithoutClassType(),
new HasMethodType($methodNameType->getValue()),
]),
$this->resolveObjectMethodType($methodNameType->getValue()),
new ClassStringType(),
]),
$context,
Expand All @@ -74,4 +73,20 @@ public function specifyTypes(
);
}

private function resolveObjectMethodType(string $methodName): Type
{
if (strtolower($methodName) === '__tostring') {
$stringableType = new ObjectType('Stringable');
$classReflection = $stringableType->getClassReflection();
if ($classReflection !== null && $classReflection->isBuiltin()) {
return $stringableType;
}
}

return new IntersectionType([
new ObjectWithoutClassType(),
new HasMethodType($methodName),
]);
}

}
11 changes: 11 additions & 0 deletions tests/PHPStan/Analyser/data/stringable-object-php7.php
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,14 @@ function foo($object)
{
assertType('stringable-object', $object);
}

/**
* @param object $object
*/
function bar($object)
{
assertType('object', $object);
if (method_exists($object, '__toString')) {
assertType('stringable-object', $object);
}
}
11 changes: 11 additions & 0 deletions tests/PHPStan/Analyser/data/stringable-object-php8.php
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,14 @@ function foo($object)
{
assertType('Stringable', $object);
Copy link
Contributor Author

@rvanvelzen rvanvelzen Oct 24, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

@ondrejmirtes ondrejmirtes Oct 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

/**
* @param object $object
*/
function bar($object)
{
assertType('object', $object);
if (method_exists($object, '__toString')) {
assertType('Stringable', $object);
}
}
4 changes: 2 additions & 2 deletions tests/PHPStan/Type/TypeCombinatorTest.php
View file Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2646,7 +2646,7 @@ public function dataIntersect(): iterable
new HasMethodType('__toString'),
],
IntersectionType::class,
'object&hasMethod(__toString)',
'stringable-object',
],
[
[
Expand All @@ -2665,7 +2665,7 @@ public function dataIntersect(): iterable
new HasMethodType('__toString'),
],
IntersectionType::class,
'object&hasMethod(__toString)',
'stringable-object',
],
[
[
Expand Down

AltStyle によって変換されたページ (->オリジナル) /