Skip to content

Commit

Permalink
Symfony: precise detection of `EventSubscriberInterface::getSubscribe…
Browse files Browse the repository at this point in the history
…dEvents` (#122)
  • Loading branch information
janedbal authored Dec 20, 2024
1 parent f006be7 commit 593829a
Show file tree
Hide file tree
Showing 2 changed files with 124 additions and 12 deletions.
118 changes: 109 additions & 9 deletions src/Provider/SymfonyUsageProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@

use Composer\InstalledVersions;
use PhpParser\Node;
use PhpParser\Node\Stmt\Return_;
use PHPStan\Analyser\Scope;
use PHPStan\BetterReflection\Reflector\Exception\IdentifierNotFound;
use PHPStan\Node\InClassNode;
use PHPStan\Reflection\ExtendedMethodReflection;
use PHPStan\Reflection\MethodReflection;
use PHPStan\Symfony\ServiceMapFactory;
use ReflectionAttribute;
use ReflectionClass;
Expand Down Expand Up @@ -41,10 +43,115 @@ public function __construct(

public function getUsages(Node $node, Scope $scope): array
{
if (!$this->enabled || !$node instanceof InClassNode) { // @phpstan-ignore phpstanApi.instanceofAssumption
if (!$this->enabled) {
return [];
}

$usages = [];

if ($node instanceof InClassNode) { // @phpstan-ignore phpstanApi.instanceofAssumption
$usages = [
...$usages,
...$this->getUsagesFromReflection($node),
];
}

if ($node instanceof Return_) {
$usages = [
...$usages,
...$this->getUsagesOfEventSubscriber($node, $scope),
];
}

return $usages;
}

/**
* @return list<ClassMethodUsage>
*/
private function getUsagesOfEventSubscriber(Return_ $node, Scope $scope): array
{
if ($node->expr === null) {
return [];
}

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

if (!$scope->getFunction() instanceof MethodReflection) {
return [];
}

if ($scope->getFunction()->getName() !== 'getSubscribedEvents') {
return [];
}

if (!$scope->getClassReflection()->implementsInterface('Symfony\Component\EventDispatcher\EventSubscriberInterface')) {
return [];
}

$className = $scope->getClassReflection()->getName();

$usages = [];

// phpcs:disable Squiz.PHP.CommentedOutCode.Found
foreach ($scope->getType($node->expr)->getConstantArrays() as $rootArray) {
foreach ($rootArray->getValuesArray()->getValueTypes() as $eventConfig) {
// ['eventName' => 'methodName']
foreach ($eventConfig->getConstantStrings() as $subscriberMethodString) {
$usages[] = new ClassMethodUsage(
null,
new ClassMethodRef(
$className,
$subscriberMethodString->getValue(),
true,
),
);
}

// ['eventName' => ['methodName', $priority]]
foreach ($eventConfig->getConstantArrays() as $subscriberMethodArray) {
foreach ($subscriberMethodArray->getFirstIterableValueType()->getConstantStrings() as $subscriberMethodString) {
$usages[] = new ClassMethodUsage(
null,
new ClassMethodRef(
$className,
$subscriberMethodString->getValue(),
true,
),
);
}
}

// ['eventName' => [['methodName', $priority], ['methodName', $priority]]]
foreach ($eventConfig->getConstantArrays() as $subscriberMethodArray) {
foreach ($subscriberMethodArray->getIterableValueType()->getConstantArrays() as $innerArray) {
foreach ($innerArray->getFirstIterableValueType()->getConstantStrings() as $subscriberMethodString) {
$usages[] = new ClassMethodUsage(
null,
new ClassMethodRef(
$className,
$subscriberMethodString->getValue(),
true,
),
);
}
}
}
}
}

// phpcs:enable // phpcs:disable Squiz.PHP.CommentedOutCode.Found

return $usages;
}

/**
* @return list<ClassMethodUsage>
*/
private function getUsagesFromReflection(InClassNode $node): array
{
$classReflection = $node->getClassReflection();
$nativeReflection = $classReflection->getNativeReflection();
$className = $classReflection->getName();
Expand All @@ -70,8 +177,7 @@ public function getUsages(Node $node, Scope $scope): array

protected function shouldMarkAsUsed(ReflectionMethod $method): bool
{
return $this->isEventSubscriberMethod($method)
|| $this->isBundleConstructor($method)
return $this->isBundleConstructor($method)
|| $this->isEventListenerMethodWithAsEventListenerAttribute($method)
|| $this->isAutowiredWithRequiredAttribute($method)
|| $this->isConstructorWithAsCommandAttribute($method)
Expand All @@ -93,12 +199,6 @@ protected function fillDicClasses(ServiceMapFactory $serviceMapFactory): void
}
}

protected function isEventSubscriberMethod(ReflectionMethod $method): bool
{
// this is simplification, we should deduce that from AST of getSubscribedEvents() method
return $method->getDeclaringClass()->implementsInterface('Symfony\Component\EventDispatcher\EventSubscriberInterface');
}

protected function isBundleConstructor(ReflectionMethod $method): bool
{
return $method->isConstructor() && $method->getDeclaringClass()->isSubclassOf('Symfony\Component\HttpKernel\Bundle\Bundle');
Expand Down
18 changes: 15 additions & 3 deletions tests/Rule/data/providers/symfony.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Symfony\Component\HttpKernel\Attribute\AsController;
use Symfony\Component\HttpKernel\Bundle\Bundle;
use Symfony\Component\HttpKernel\Event\FilterControllerEvent;
use Symfony\Component\HttpKernel\Event\GetResponseForExceptionEvent;
use Symfony\Component\Routing\Attribute\Route;
use Symfony\Contracts\Service\Attribute\Required;

Expand Down Expand Up @@ -47,18 +49,28 @@ public function __construct() {
class SomeSubscriber implements EventSubscriberInterface
{

public function onKernelRequest(): void
public function string(): void
{
}

public function onNonsense(): void
public function stringInArray(): void
{
}

public function stringInArrayArray(): void
{
}

public function onNonsense(): void // error: Unused Symfony\SomeSubscriber::onNonsense
{
}

public static function getSubscribedEvents(): array
{
return [
'kernel.request' => [['onKernelRequest', 0]],
'kernel.exception' => 'string',
'kernel.controller' => ['stringInArray', 1],
'kernel.request' => [['stringInArrayArray', 0]],
];
}

Expand Down

0 comments on commit 593829a

Please sign in to comment.