Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
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
5 changes: 5 additions & 0 deletions extension.neon
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,11 @@ services:
factory: PHPStan\Type\Symfony\EnvelopeReturnTypeExtension
tags: [phpstan.broker.dynamicMethodReturnTypeExtension]

# BrowserKitAssertionTrait::getClient() return type
-
class: PHPStan\Type\Symfony\BrowserKitAssertionTraitReturnTypeExtension
tags: [phpstan.broker.expressionTypeResolverExtension]

# Messenger HandleTrait::handle() return type
-
class: PHPStan\Type\Symfony\MessengerHandleTraitReturnTypeExtension
Expand Down
68 changes: 68 additions & 0 deletions src/Type/Symfony/BrowserKitAssertionTraitReturnTypeExtension.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<?php declare(strict_types = 1);

namespace PHPStan\Type\Symfony;

use PhpParser\Node\Expr;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Identifier;
use PHPStan\Analyser\Scope;
use PHPStan\Type\ExpressionTypeResolverExtension;
use PHPStan\Type\NullType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use PHPStan\Type\UnionType;
use function count;

final class BrowserKitAssertionTraitReturnTypeExtension implements ExpressionTypeResolverExtension
{

private const TRAIT_NAME = 'Symfony\Bundle\FrameworkBundle\Test\BrowserKitAssertionsTrait';
private const TRAIT_METHOD_NAME = 'getClient';

public function getType(Expr $expr, Scope $scope): ?Type
{
if ($this->isSupported($expr, $scope)) {
$args = $expr->getArgs();
if (count($args) > 0) {
return TypeCombinator::intersect(
$scope->getType($args[0]->value),
new UnionType([
new ObjectType('Symfony\Component\BrowserKit\AbstractBrowser'),
new NullType(),
]),
);
}

return new ObjectType('Symfony\Component\BrowserKit\AbstractBrowser');
}

return null;
}

/**
* @phpstan-assert-if-true =MethodCall $expr
*/
private function isSupported(Expr $expr, Scope $scope): bool
{
if (!($expr instanceof MethodCall) || !($expr->name instanceof Identifier) || $expr->name->name !== self::TRAIT_METHOD_NAME) {
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand the logic. You're interested in the method name and you're interested where we're calling it from.

But you're not interested on what we're calling the method. This extension might be influencing a total coincidence that's nothing to do with BrowserKitAssertionsTrait::getClient().

We're in a class, the class has getClient method. And the method is declared in BrowserKitAssertionsTrait trait. But the method we're still calling might be unrelated.

My hunch is telling me you're no checking $scope->getMethodReflection($scope->getType($expr->var), $expr->name->name) but you should be.

Also, method names are case-insensitive, so toLowerString() needs to happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied the logic from the extension you gave me as example

private function isSupported(Expr $expr, Scope $scope): bool
{
if (!($expr instanceof MethodCall) || !($expr->name instanceof Identifier) || $expr->name->name !== self::TRAIT_METHOD_NAME) {
return false;
}
if (!$scope->isInClass()) {
return false;
}
$reflectionClass = $scope->getClassReflection()->getNativeReflection();
if (!$reflectionClass->hasMethod(self::TRAIT_METHOD_NAME)) {
return false;
}
$methodReflection = $reflectionClass->getMethod(self::TRAIT_METHOD_NAME);
$declaringClassReflection = $methodReflection->getBetterReflection()->getDeclaringClass();
return $declaringClassReflection->getName() === self::TRAIT_NAME;
}

So I assume it should be improve on the other extension too.

return false;
}

if (!$scope->isInClass()) {
return false;
}

$reflectionClass = $scope->getClassReflection()->getNativeReflection();

if (!$reflectionClass->hasMethod(self::TRAIT_METHOD_NAME)) {
return false;
}

$methodReflection = $reflectionClass->getMethod(self::TRAIT_METHOD_NAME);
$declaringClassReflection = $methodReflection->getBetterReflection()->getDeclaringClass();

return $declaringClassReflection->getName() === self::TRAIT_NAME;
}

}
1 change: 1 addition & 0 deletions tests/Type/Symfony/ExtensionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class ExtensionTest extends TypeInferenceTestCase
/** @return mixed[] */
public function dataFileAsserts(): iterable
{
yield from $this->gatherAssertTypes(__DIR__ . '/data/browserkit_assertion_trait.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/messenger_handle_trait.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/envelope_all.php');
yield from $this->gatherAssertTypes(__DIR__ . '/data/header_bag_get.php');
Expand Down
23 changes: 23 additions & 0 deletions tests/Type/Symfony/data/browserkit_assertion_trait.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?php declare(strict_types = 1);

namespace BrowserKitAssertionTrait;

use Symfony\Bundle\FrameworkBundle\Test\BrowserKitAssertionsTrait;
use Symfony\Component\BrowserKit\AbstractBrowser;
use function PHPStan\Testing\assertType;

class Foo {
use BrowserKitAssertionsTrait;

/**
* @param mixed $mixed
*/
public function test(AbstractBrowser $browser, ?AbstractBrowser $nullableBrowser, $mixed)
{
assertType('Symfony\Component\BrowserKit\AbstractBrowser', $this->getClient());
assertType('null', $this->getClient(null));
assertType('Symfony\Component\BrowserKit\AbstractBrowser', $this->getClient($browser));
assertType('Symfony\Component\BrowserKit\AbstractBrowser|null', $this->getClient($nullableBrowser));
assertType('Symfony\Component\BrowserKit\AbstractBrowser|null', $this->getClient($mixed));
}
}