Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ final class ChildOfParentWithPrivatePromotedProperty extends ParentControllerWit
$this->logger->log('level', 'value');
}
#[\Symfony\Contracts\Service\Attribute\Required]
public function autowire(\Psr\Log\LoggerInterface $logger): void
public function autowireChildOfParentWithPrivatePromotedProperty(\Psr\Log\LoggerInterface $logger): void
{
$this->logger = $logger;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ final class ParentConstructorMultipleServices extends ParentControllerWithPrivat
$this->logger->log('level', $this->translator->trans('value'));
}
#[\Symfony\Contracts\Service\Attribute\Required]
public function autowire(\Psr\Log\LoggerInterface $logger, \Symfony\Contracts\Translation\TranslatorInterface $translator): void
public function autowireParentConstructorMultipleServices(\Psr\Log\LoggerInterface $logger, \Symfony\Contracts\Translation\TranslatorInterface $translator): void
{
$this->logger = $logger;
$this->translator = $translator;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ final class ParentWithAutowireMethod extends ParentControllerWithAutowireMethod
$this->logger->log('level', 'value');
}
#[\Symfony\Contracts\Service\Attribute\Required]
public function autowireServices(\Psr\Log\LoggerInterface $logger): void
public function autowireParentWithAutowireMethod(\Psr\Log\LoggerInterface $logger): void
{
$this->logger = $logger;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php

declare(strict_types=1);

namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\FixtureTypeGuarded;

use Psr\Log\LoggerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Routing\Annotation\Route;

class NonGuardedController extends AbstractController
{
#[Route('/some-action', name: 'some_action')]
public function someAction(LoggerInterface $logger)
{
$logger->log('level', 'value');
}
}

?>
-----
<?php

declare(strict_types=1);

namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\FixtureTypeGuarded;

use Psr\Log\LoggerInterface;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Routing\Annotation\Route;

class NonGuardedController extends AbstractController
{
public function __construct(private readonly \Psr\Log\LoggerInterface $logger)
{
}
#[Route('/some-action', name: 'some_action')]
public function someAction()
{
$this->logger->log('level', 'value');
}
}

?>
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

declare(strict_types=1);

namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\FixtureTypeGuarded;

use Psr\Log\LoggerInterface;
use Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source\AbstractCustomController;
use Symfony\Component\Routing\Annotation\Route;

class SkipTypeGuardedController extends AbstractCustomController
{
#[Route('/some-action', name: 'some_action')]
public function someAction(LoggerInterface $logger)
{
$logger->log('level', 'value');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source;

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;

abstract class AbstractCustomController extends AbstractController
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector;

use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class TypeGuardedTest extends AbstractRectorTestCase
{
#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/FixtureTypeGuarded');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/type_guarded_rule.php';
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\Symfony\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector;
use Rector\Symfony\Tests\CodeQuality\Rector\Class_\ControllerMethodInjectionToConstructorRector\Source\AbstractCustomController;

return RectorConfig::configure()
->withRules([ControllerMethodInjectionToConstructorRector::class])
->withTypeGuardedClasses([AbstractCustomController::class]);
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,7 @@ final class ControllerMethodInjectionToConstructorRector extends AbstractRector
*/
private const array COMMON_ENTITY_CONTAINS_SUBNAMESPACES = ["\\Entity\\", "\\Document\\", "\\Model\\"];

private const string AUTOWIRE_METHOD_NAME = 'autowire';

/**
* Used when a parent class already defines autowire(), to avoid overriding it
*/
private const string FALLBACK_AUTOWIRE_METHOD_NAME = 'autowireServices';
private const string AUTOWIRE_METHOD_NAME_PREFIX = 'autowire';

public function __construct(
private readonly ControllerAnalyzer $controllerAnalyzer,
Expand All @@ -75,7 +70,7 @@ public function __construct(
public function getRuleDefinition(): RuleDefinition
{
return new RuleDefinition(
'Change Symfony controller method injection to direct constructor dependency, to separate params and services clearly. If a parent class has a constructor, use #[Required] autowire() method instead, to avoid repeating all parent params',
'Change Symfony controller method injection to direct constructor dependency, to separate params and services clearly. If a parent class has a constructor, use #[Required] autowire<ShortClassName>() method instead, to avoid repeating all parent params',
[
new CodeSample(
<<<'CODE_SAMPLE'
Expand Down Expand Up @@ -145,7 +140,7 @@ public function someAction()
}

#[Required]
public function autowire(SomeService $someService): void
public function autowireSomeController(SomeService $someService): void
{
$this->someService = $someService;
}
Expand Down Expand Up @@ -177,6 +172,11 @@ public function refactor(Node $node): ?Node
return null;
}

// removing action params would break child classes of user-guarded classes
if ($this->parentClassMethodTypeOverrideGuard->isTypeGuardedClass($node)) {
return null;
}

$propertyMetadatas = [];

$constructParamVariables = [];
Expand Down Expand Up @@ -495,20 +495,14 @@ private function addRequiredAutowireClassMethod(Class_ $class, array $propertyMe
}
}

/**
* Suffix with the short class name, to keep the method unique in case of inheritance
*/
private function resolveAutowireMethodName(Class_ $class): string
{
$classReflection = $this->reflectionResolver->resolveClassReflection($class);
if (! $classReflection instanceof ClassReflection) {
return self::AUTOWIRE_METHOD_NAME;
}

foreach ($classReflection->getParents() as $parentClassReflection) {
if ($parentClassReflection->hasNativeMethod(self::AUTOWIRE_METHOD_NAME)) {
return self::FALLBACK_AUTOWIRE_METHOD_NAME;
}
}
$shortClassName = $class->name instanceof Identifier ? $class->name->toString() : '';

return self::AUTOWIRE_METHOD_NAME;
return self::AUTOWIRE_METHOD_NAME_PREFIX . ucfirst($shortClassName);
}

private function hasParentConstructor(Class_ $class): bool
Expand Down
Loading