Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add test for phpstan issue 7396 #1442

Draft
wants to merge 4 commits into
base: 2.0.x
Choose a base branch
from
+65 −0
Draft

Add test for phpstan issue 7396 #1442

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
856040e
Add test for phpstan issue 7396
mvorisek Jun 18, 2022
2f5a3ba
TEST
mvorisek Jun 18, 2022
253aa2e
fix test
mvorisek Jun 18, 2022
d90bc0a
Revert "TEST"
mvorisek Jun 18, 2022
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
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/DynamicReturnTypeExtensionTypeInferenceTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public function dataAsserts(): iterable
yield from $this->gatherAssertTypes(__DIR__ . '/data/dynamic-method-return-getsingle-conditional.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7344.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7391b.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-7385.php');
}

/**
Expand Down
36 changes: 36 additions & 0 deletions tests/PHPStan/Analyser/data/TestDynamicReturnTypeExtensions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@
use PhpParser\Node\Expr\New_;
use PhpParser\Node\Expr\StaticCall;
use PHPStan\Analyser\Scope;
use PHPStan\Analyser\SpecifiedTypes;
use PHPStan\Analyser\TypeSpecifier;
use PHPStan\Analyser\TypeSpecifierAwareExtension;
use PHPStan\Analyser\TypeSpecifierContext;
use PHPStan\Reflection\Dummy\ChangedTypeMethodReflection;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Reflection\ParametersAcceptorSelector;
Expand All @@ -18,6 +22,7 @@
use PHPStan\Type\DynamicStaticMethodReturnTypeExtension;
use PHPStan\Type\IntegerType;
use PHPStan\Type\IntersectionType;
use PHPStan\Type\MethodTypeSpecifyingExtension;
use PHPStan\Type\NeverType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\ObjectWithoutClassType;
Expand Down Expand Up @@ -263,3 +268,34 @@ public function getTypeFromStaticMethodCall(
return $scope->getType(new New_($methodCall->class));
}
}

class Bug7385MethodTypeSpecifyingExtension implements TypeSpecifierAwareExtension, MethodTypeSpecifyingExtension
{
public function getClass(): string
{
return \Bug7385\Model::class;
}

public function isMethodSupported(MethodReflection $methodReflection, MethodCall $methodCall = null, TypeSpecifierContext $context = null): bool
{
return $methodReflection->getName() === 'assertHasIface';
}

/** @var TypeSpecifier */
protected $typeSpecifier;

public function setTypeSpecifier(TypeSpecifier $typeSpecifier): void
{
$this->typeSpecifier = $typeSpecifier;
}

public function specifyTypes(MethodReflection $methodReflection, MethodCall $methodCall, Scope $scope, TypeSpecifierContext $context): SpecifiedTypes
{
$type = TypeCombinator::intersect(
$scope->getType($methodCall->var),
new ObjectType(\Bug7385\Iface::class)
);

return $this->typeSpecifier->create($methodCall->var, $type, TypeSpecifierContext::createNull());
}
}
24 changes: 24 additions & 0 deletions tests/PHPStan/Analyser/data/bug-7385.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
<?php declare(strict_types = 1);

namespace Bug7385;

use function PHPStan\Testing\assertType;

class Model
{
public function assertHasIface(): void
{
throw new \Error('For static analysis only, $this type is narrowed by MethodTypeSpecifyingExtension');
}
}

interface Iface
{
}

function () {
$m = random_int(0, 1) === 0 ? new Model() : new class() extends Model implements Iface {};
assertType('Bug7385\Model', $m);
$m->assertHasIface();
assertType('Bug7385\Iface&Bug7385\Model', $m);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ondrejmirtes this assertion is not passing, do you know why?

  • Bug7385MethodTypeSpecifyingExtension::specifyTypes() is correctly reached and $type is (the expected) Bug7385\Iface&Bug7385\Model
  • I am not completely sure what TypeSpecifierContext::createXxx() is used for or if I should use TypeSpecifierContext::createTrue(), but I tried it.
  • I also tried to to set $overwrite param to true in $this->typeSpecifier->create() factory method, but it seems the $m type is still Bug7385\Model as if the MethodTypeSpecifyingExtension result is completely ignored

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ondrejmirtes can you please take a look on this issue?

Copy link
Member

Choose a reason for hiding this comment

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

There's 343 open bug reports (https://github.com/phpstan/phpstan/labels/bug) and 323 feature requests (https://github.com/phpstan/phpstan/labels/feature-request). Your issue isn't anything urgent to warrant immediate attention - it's not anything special and doesn't stand out from other 666 open issues.

This PR doesn't even have an associated issue - I closed #7385 because there wasn't anything about type-specifying extensions in the description.

There might even be an open issue about "type-specifying extension doesn't allow me to change the type of the object the method is called on", I'm not sure.

Anyway, if you're already this far, you can take one more step and debug where it actually gets lost, and fix it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, there is phpstan/phpstan#7396 issue I opened later after a short discussion about an implicit support in phpstan/phpstan#7385

};
4 changes: 4 additions & 0 deletions tests/PHPStan/Analyser/dynamic-return-type.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,7 @@ services:
class: PHPStan\Tests\Bug7391BDynamicStaticMethodReturnTypeExtension
tags:
- phpstan.broker.dynamicStaticMethodReturnTypeExtension
-
class: PHPStan\Tests\Bug7385MethodTypeSpecifyingExtension
tags:
- phpstan.typeSpecifier.methodTypeSpecifyingExtension
Loading