From 1138f045e2a27f9b069a3e512a04e6d49e04a596 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Sun, 1 Dec 2024 17:51:24 +0100 Subject: [PATCH] [DependencyInjection] Add GetBySymfonyStringToConstructorInjectionRector (#688) * register rule * conver getContainer() as well * covert this container property fetch as well * deprecate ContainerGetToConstructorInjectionRector * deprecate ContainerGetToConstructorInjectionRector, as handles same logic as GetToConstructorInjectionRector * mark * misc --- .../symfony/symfony-constructor-injection.php | 6 +- config/sets/symfony/symfony42.php | 13 +- .../Fixture/include_get_container.php.inc | 35 +++++ .../Fixture/cover_container_property.php.inc | 36 +++++ .../Fixture/command_validator.php.inc | 34 ++++ .../event_dispatcher_controller.php.inc | 34 ++++ ...tringToConstructorInjectionRectorTest.php} | 4 +- .../config/configured_rule.php | 10 ++ .../constant_as_service_regression.php.inc | 25 --- .../Fixture/first_class.php.inc | 38 ----- .../Fixture/my_command.php.inc | 39 ----- .../Fixture/skip_request_get.php.inc | 15 -- .../Fixture/skip_test_case.php.inc | 13 -- .../Fixture/skip_trait.php.inc | 11 -- .../some_translator_different_object.php.inc | 51 ------ .../Fixture/some_translator_exists.php.inc | 45 ------ .../Fixture/some_translator_exists2.php.inc | 51 ------ ...e_translator_exists_different_name.php.inc | 45 ------ .../Fixture/twig_extension_underscore.php.inc | 34 ---- .../Fixture/twig_interface_underscore.php.inc | 34 ---- .../Source/SomeTranslator.php | 9 -- .../Source/SpecificTwigExtension.php | 11 -- .../config.yaml | 7 - .../config/configured_rule.php | 11 -- .../xml/services.xml | 25 --- .../AutowireClassMethodFactory.php | 2 +- ...fonyStringToConstructorInjectionRector.php | 148 ++++++++++++++++++ .../Trait_/TraitGetByTypeToInjectRector.php | 3 +- .../ThisGetTypeMatcher.php | 56 +++++-- .../ContainerGetNameToTypeInTestsRector.php | 1 - ...ntainerGetToConstructorInjectionRector.php | 5 +- ...plateShortNotationToBundleSyntaxRector.php | 10 +- src/Enum/SymfonyClass.php | 20 +++ .../FrameworkBundle/Controller/Controller.php | 2 + 34 files changed, 391 insertions(+), 492 deletions(-) create mode 100644 rules-tests/DependencyInjection/Rector/Class_/CommandGetByTypeToConstructorInjectionRector/Fixture/include_get_container.php.inc create mode 100644 rules-tests/DependencyInjection/Rector/Class_/ControllerGetByTypeToConstructorInjectionRector/Fixture/cover_container_property.php.inc create mode 100644 rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/Fixture/command_validator.php.inc create mode 100644 rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/Fixture/event_dispatcher_controller.php.inc rename rules-tests/{Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/ContainerGetToConstructorInjectionRectorTest.php => DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/GetBySymfonyStringToConstructorInjectionRectorTest.php} (72%) create mode 100644 rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/config/configured_rule.php delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/constant_as_service_regression.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/first_class.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/my_command.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_request_get.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_test_case.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_trait.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_different_object.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists2.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists_different_name.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/twig_extension_underscore.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/twig_interface_underscore.php.inc delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Source/SomeTranslator.php delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Source/SpecificTwigExtension.php delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/config.yaml delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/config/configured_rule.php delete mode 100644 rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/xml/services.xml create mode 100644 rules/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector.php diff --git a/config/sets/symfony/symfony-constructor-injection.php b/config/sets/symfony/symfony-constructor-injection.php index d4980718..ac31ffc4 100644 --- a/config/sets/symfony/symfony-constructor-injection.php +++ b/config/sets/symfony/symfony-constructor-injection.php @@ -5,18 +5,20 @@ use Rector\Config\RectorConfig; use Rector\Symfony\DependencyInjection\Rector\Class_\CommandGetByTypeToConstructorInjectionRector; use Rector\Symfony\DependencyInjection\Rector\Class_\ControllerGetByTypeToConstructorInjectionRector; +use Rector\Symfony\DependencyInjection\Rector\Class_\GetBySymfonyStringToConstructorInjectionRector; +use Rector\Symfony\DependencyInjection\Rector\Trait_\TraitGetByTypeToInjectRector; use Rector\Symfony\Symfony28\Rector\MethodCall\GetToConstructorInjectionRector; use Rector\Symfony\Symfony34\Rector\Closure\ContainerGetNameToTypeInTestsRector; -use Rector\Symfony\Symfony42\Rector\MethodCall\ContainerGetToConstructorInjectionRector; return static function (RectorConfig $rectorConfig): void { $rectorConfig->rules([ // modern step-by-step narrow approach ControllerGetByTypeToConstructorInjectionRector::class, CommandGetByTypeToConstructorInjectionRector::class, + GetBySymfonyStringToConstructorInjectionRector::class, + TraitGetByTypeToInjectRector::class, // legacy rules that require container fetch - ContainerGetToConstructorInjectionRector::class, ContainerGetNameToTypeInTestsRector::class, GetToConstructorInjectionRector::class, ]); diff --git a/config/sets/symfony/symfony42.php b/config/sets/symfony/symfony42.php index 5c6e6f54..541301f0 100644 --- a/config/sets/symfony/symfony42.php +++ b/config/sets/symfony/symfony42.php @@ -15,7 +15,6 @@ use Rector\Renaming\Rector\MethodCall\RenameMethodRector; use Rector\Renaming\Rector\Name\RenameClassRector; use Rector\Renaming\ValueObject\MethodCallRename; -use Rector\Symfony\Symfony42\Rector\MethodCall\ContainerGetToConstructorInjectionRector; use Rector\Symfony\Symfony42\Rector\New_\RootNodeTreeBuilderRector; use Rector\Symfony\Symfony42\Rector\New_\StringToArrayArgumentProcessRector; use Rector\Transform\Rector\ClassMethod\WrapReturnRector; @@ -48,13 +47,11 @@ 'Symfony\Component\Translation\TranslatorInterface' => 'Symfony\Contracts\Translation\TranslatorInterface', ]); - # related to "Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand" deprecation, @see https://github.com/rectorphp/rector/issues/1629 - $rectorConfig->rule(ContainerGetToConstructorInjectionRector::class); - - # https://symfony.com/blog/new-in-symfony-4-2-important-deprecations - $rectorConfig->rule(StringToArrayArgumentProcessRector::class); - - $rectorConfig->rule(RootNodeTreeBuilderRector::class); + $rectorConfig->rules([ + # https://symfony.com/blog/new-in-symfony-4-2-important-deprecations + StringToArrayArgumentProcessRector::class, + RootNodeTreeBuilderRector::class, + ]); $rectorConfig->ruleWithConfiguration(ArgumentAdderRector::class, [ new ArgumentAdder( diff --git a/rules-tests/DependencyInjection/Rector/Class_/CommandGetByTypeToConstructorInjectionRector/Fixture/include_get_container.php.inc b/rules-tests/DependencyInjection/Rector/Class_/CommandGetByTypeToConstructorInjectionRector/Fixture/include_get_container.php.inc new file mode 100644 index 00000000..13cbf109 --- /dev/null +++ b/rules-tests/DependencyInjection/Rector/Class_/CommandGetByTypeToConstructorInjectionRector/Fixture/include_get_container.php.inc @@ -0,0 +1,35 @@ +getContainer()->get(SomeService::class); + } +} + +?> +----- +someService; + } +} + +?> diff --git a/rules-tests/DependencyInjection/Rector/Class_/ControllerGetByTypeToConstructorInjectionRector/Fixture/cover_container_property.php.inc b/rules-tests/DependencyInjection/Rector/Class_/ControllerGetByTypeToConstructorInjectionRector/Fixture/cover_container_property.php.inc new file mode 100644 index 00000000..0b57fc38 --- /dev/null +++ b/rules-tests/DependencyInjection/Rector/Class_/ControllerGetByTypeToConstructorInjectionRector/Fixture/cover_container_property.php.inc @@ -0,0 +1,36 @@ +container->get(SomeService::class); + } +} + +?> +----- +someService; + } +} + +?> diff --git a/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/Fixture/command_validator.php.inc b/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/Fixture/command_validator.php.inc new file mode 100644 index 00000000..e2b53e6d --- /dev/null +++ b/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/Fixture/command_validator.php.inc @@ -0,0 +1,34 @@ +get('validator'); + } +} + +?> +----- +validator; + } +} + +?> diff --git a/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/Fixture/event_dispatcher_controller.php.inc b/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/Fixture/event_dispatcher_controller.php.inc new file mode 100644 index 00000000..cbb790ab --- /dev/null +++ b/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/Fixture/event_dispatcher_controller.php.inc @@ -0,0 +1,34 @@ +get('event_dispatcher'); + } +} + +?> +----- +eventDispatcher; + } +} + +?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/ContainerGetToConstructorInjectionRectorTest.php b/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/GetBySymfonyStringToConstructorInjectionRectorTest.php similarity index 72% rename from rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/ContainerGetToConstructorInjectionRectorTest.php rename to rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/GetBySymfonyStringToConstructorInjectionRectorTest.php index 87b05c7b..5b22c570 100644 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/ContainerGetToConstructorInjectionRectorTest.php +++ b/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/GetBySymfonyStringToConstructorInjectionRectorTest.php @@ -2,13 +2,13 @@ declare(strict_types=1); -namespace Rector\Symfony\Tests\Symfony42\Rector\MethodCall\ContainerGetToConstructorInjectionRector; +namespace Rector\Symfony\Tests\DependencyInjection\Rector\Class_\GetBySymfonyStringToConstructorInjectionRector; use Iterator; use PHPUnit\Framework\Attributes\DataProvider; use Rector\Testing\PHPUnit\AbstractRectorTestCase; -final class ContainerGetToConstructorInjectionRectorTest extends AbstractRectorTestCase +final class GetBySymfonyStringToConstructorInjectionRectorTest extends AbstractRectorTestCase { #[DataProvider('provideData')] public function test(string $filePath): void diff --git a/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/config/configured_rule.php b/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/config/configured_rule.php new file mode 100644 index 00000000..4c78aad4 --- /dev/null +++ b/rules-tests/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector/config/configured_rule.php @@ -0,0 +1,10 @@ +rule(GetBySymfonyStringToConstructorInjectionRector::class); +}; diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/constant_as_service_regression.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/constant_as_service_regression.php.inc deleted file mode 100644 index a1d2392e..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/constant_as_service_regression.php.inc +++ /dev/null @@ -1,25 +0,0 @@ -get(G2::SVC_CONF); - return new static($configFactory); - } -} -?> ------ -get(G2::SVC_CONF); - return new static($configFactory); - } -} -?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/first_class.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/first_class.php.inc deleted file mode 100644 index 6ad50b66..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/first_class.php.inc +++ /dev/null @@ -1,38 +0,0 @@ -getContainer()->get('translator'); - - $someService = $this->getContainer()->get('translator')->translateSomething(); - } -} - -?> ------ -someTranslator; - - $someService = $this->someTranslator->translateSomething(); - } -} - -?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/my_command.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/my_command.php.inc deleted file mode 100644 index 5393c7b6..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/my_command.php.inc +++ /dev/null @@ -1,39 +0,0 @@ -getContainer()->get('some_service'); - } -} - -?> ------ -stdClass; - } -} - -?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_request_get.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_request_get.php.inc deleted file mode 100644 index be271453..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_request_get.php.inc +++ /dev/null @@ -1,15 +0,0 @@ -get((string) SomeTranslator::class); - } -} diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_test_case.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_test_case.php.inc deleted file mode 100644 index 61a1c7c1..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_test_case.php.inc +++ /dev/null @@ -1,13 +0,0 @@ -get('some'); - } -} diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_trait.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_trait.php.inc deleted file mode 100644 index 0d3d56a2..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/skip_trait.php.inc +++ /dev/null @@ -1,11 +0,0 @@ -getContainer()->get('some_service'); - } -} diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_different_object.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_different_object.php.inc deleted file mode 100644 index ebc5da59..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_different_object.php.inc +++ /dev/null @@ -1,51 +0,0 @@ -someTranslator = $someTranslator; - } - - protected function execute() - { - $someService = $this->getContainer()->get('translator'); - - $someService = $this->getContainer()->get('translator')->translateSomething(); - } -} - -?> ------ -someTranslator = $someTranslator; - } - - protected function execute() - { - $someService = $this->someTranslator2; - - $someService = $this->someTranslator2->translateSomething(); - } -} - -?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists.php.inc deleted file mode 100644 index 639ef412..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists.php.inc +++ /dev/null @@ -1,45 +0,0 @@ -getContainer()->get('translator'); - - $someService = $this->getContainer()->get('translator')->translateSomething(); - } -} - -?> ------ -someTranslator; - - $someService = $this->someTranslator->translateSomething(); - } -} - -?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists2.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists2.php.inc deleted file mode 100644 index eb3d1711..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists2.php.inc +++ /dev/null @@ -1,51 +0,0 @@ -someTranslator = $someTranslator; - } - - protected function execute() - { - $someService = $this->getContainer()->get('translator'); - - $someService = $this->getContainer()->get('translator')->translateSomething(); - } -} - -?> ------ -someTranslator = $someTranslator; - } - - protected function execute() - { - $someService = $this->someTranslator; - - $someService = $this->someTranslator->translateSomething(); - } -} - -?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists_different_name.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists_different_name.php.inc deleted file mode 100644 index 2be750c8..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/some_translator_exists_different_name.php.inc +++ /dev/null @@ -1,45 +0,0 @@ -getContainer()->get('translator'); - - $someService = $this->getContainer()->get('translator')->translateSomething(); - } -} - -?> ------ -this_is_a_translator; - - $someService = $this->this_is_a_translator->translateSomething(); - } -} - -?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/twig_extension_underscore.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/twig_extension_underscore.php.inc deleted file mode 100644 index b6196342..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/twig_extension_underscore.php.inc +++ /dev/null @@ -1,34 +0,0 @@ -get('twig_extension'); - } -} - -?> ------ -twigExtension; - } -} - -?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/twig_interface_underscore.php.inc b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/twig_interface_underscore.php.inc deleted file mode 100644 index 9092c158..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Fixture/twig_interface_underscore.php.inc +++ /dev/null @@ -1,34 +0,0 @@ -get('specific_twig_extension'); - } -} - -?> ------ -specificTwigExtension; - } -} - -?> diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Source/SomeTranslator.php b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Source/SomeTranslator.php deleted file mode 100644 index 472150be..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/Source/SomeTranslator.php +++ /dev/null @@ -1,9 +0,0 @@ -symfonyContainerXml(__DIR__ . '/../xml/services.xml'); - $rectorConfig->rule(ContainerGetToConstructorInjectionRector::class); -}; diff --git a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/xml/services.xml b/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/xml/services.xml deleted file mode 100644 index 9b8a1055..00000000 --- a/rules-tests/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector/xml/services.xml +++ /dev/null @@ -1,25 +0,0 @@ - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/rules/DependencyInjection/NodeFactory/AutowireClassMethodFactory.php b/rules/DependencyInjection/NodeFactory/AutowireClassMethodFactory.php index 2bda0e90..ffe545a9 100644 --- a/rules/DependencyInjection/NodeFactory/AutowireClassMethodFactory.php +++ b/rules/DependencyInjection/NodeFactory/AutowireClassMethodFactory.php @@ -20,7 +20,7 @@ use Rector\NodeNameResolver\NodeNameResolver; use Rector\PostRector\ValueObject\PropertyMetadata; -final class AutowireClassMethodFactory +final readonly class AutowireClassMethodFactory { public function __construct( private NodeNameResolver $nodeNameResolver diff --git a/rules/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector.php b/rules/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector.php new file mode 100644 index 00000000..bb9236cf --- /dev/null +++ b/rules/DependencyInjection/Rector/Class_/GetBySymfonyStringToConstructorInjectionRector.php @@ -0,0 +1,148 @@ + + */ + private const SYMFONY_NAME_TO_TYPE_MAP = [ + 'validator' => SymfonyClass::VALIDATOR_INTERFACE, + 'event_dispatcher' => SymfonyClass::EVENT_DISPATCHER_INTERFACE, + 'logger' => SymfonyClass::LOGGER_INTERFACE, + 'jms_serializer' => SymfonyClass::SERIALIZER_INTERFACE, + ]; + + public function __construct( + private readonly ClassDependencyManipulator $classDependencyManipulator, + private readonly ThisGetTypeMatcher $thisGetTypeMatcher, + private readonly PropertyNaming $propertyNaming, + ) { + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition( + 'Converts typical Symfony services like $this->get("validator") in commands/controllers to constructor injection (step 3/x)', + [ + new CodeSample( + <<<'CODE_SAMPLE' +use Symfony\Bundle\FrameworkBundle\Controller\Controller; + +final class SomeController extends Controller +{ + public function someMethod() + { + $someType = $this->get('validator'); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use Symfony\Bundle\FrameworkBundle\Controller\Controller; +use Symfony\Component\Validator\Validator\ValidatorInterface; + +final class SomeController extends Controller +{ + public function __construct(private ValidatorInterface $validator) + + public function someMethod() + { + $someType = $this->validator; + } +} +CODE_SAMPLE + ), + ] + ); + } + + /** + * @return array> + */ + public function getNodeTypes(): array + { + return [Class_::class]; + } + + /** + * @param Class_ $node + */ + public function refactor(Node $node): ?Node + { + if ($this->shouldSkipClass($node)) { + return null; + } + + $propertyMetadatas = []; + + $this->traverseNodesWithCallable($node, function (Node $node) use (&$propertyMetadatas): ?Node { + if (! $node instanceof MethodCall) { + return null; + } + + $serviceName = $this->thisGetTypeMatcher->matchString($node); + if (! is_string($serviceName)) { + return null; + } + + $serviceType = self::SYMFONY_NAME_TO_TYPE_MAP[$serviceName] ?? null; + if ($serviceType === null) { + return null; + } + + $propertyName = $this->propertyNaming->fqnToVariableName($serviceType); + $propertyMetadata = new PropertyMetadata($propertyName, new FullyQualifiedObjectType($serviceType)); + + $propertyMetadatas[] = $propertyMetadata; + return $this->nodeFactory->createPropertyFetch('this', $propertyMetadata->getName()); + }); + + if ($propertyMetadatas === []) { + return null; + } + + foreach ($propertyMetadatas as $propertyMetadata) { + $this->classDependencyManipulator->addConstructorDependency($node, $propertyMetadata); + } + + return $node; + } + + private function shouldSkipClass(Class_ $class): bool + { + $scope = ScopeFetcher::fetch($class); + + $classReflection = $scope->getClassReflection(); + if (! $classReflection instanceof ClassReflection) { + return true; + } + + if ($classReflection->isSubclassOf(SymfonyClass::CONTAINER_AWARE_COMMAND)) { + return false; + } + + return ! $classReflection->isSubclassOf(SymfonyClass::CONTROLLER); + } +} diff --git a/rules/DependencyInjection/Rector/Trait_/TraitGetByTypeToInjectRector.php b/rules/DependencyInjection/Rector/Trait_/TraitGetByTypeToInjectRector.php index b9cc74c5..3c4921c8 100644 --- a/rules/DependencyInjection/Rector/Trait_/TraitGetByTypeToInjectRector.php +++ b/rules/DependencyInjection/Rector/Trait_/TraitGetByTypeToInjectRector.php @@ -18,6 +18,7 @@ use Rector\PostRector\ValueObject\PropertyMetadata; use Rector\Rector\AbstractRector; use Rector\StaticTypeMapper\ValueObject\Type\FullyQualifiedObjectType; +use Rector\Symfony\DependencyInjection\NodeFactory\AutowireClassMethodFactory; use Rector\Symfony\DependencyInjection\ThisGetTypeMatcher; use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample; use Symplify\RuleDocGenerator\ValueObject\RuleDefinition; @@ -30,7 +31,7 @@ final class TraitGetByTypeToInjectRector extends AbstractRector public function __construct( private readonly PropertyNaming $propertyNaming, private readonly ThisGetTypeMatcher $thisGetTypeMatcher, - private readonly \Rector\Symfony\DependencyInjection\NodeFactory\AutowireClassMethodFactory $autowireClassMethodFactory, + private readonly AutowireClassMethodFactory $autowireClassMethodFactory, ) { } diff --git a/rules/DependencyInjection/ThisGetTypeMatcher.php b/rules/DependencyInjection/ThisGetTypeMatcher.php index eb813fa6..cd70bd98 100644 --- a/rules/DependencyInjection/ThisGetTypeMatcher.php +++ b/rules/DependencyInjection/ThisGetTypeMatcher.php @@ -4,8 +4,12 @@ namespace Rector\Symfony\DependencyInjection; +use PhpParser\Node\Expr; use PhpParser\Node\Expr\ClassConstFetch; use PhpParser\Node\Expr\MethodCall; +use PhpParser\Node\Expr\PropertyFetch; +use PhpParser\Node\Expr\Variable; +use PhpParser\Node\Scalar\String_; use Rector\NodeNameResolver\NodeNameResolver; final readonly class ThisGetTypeMatcher @@ -15,34 +19,68 @@ public function __construct( ) { } + public function matchString(MethodCall $methodCall): ?string + { + $getExpr = $this->matchGetExpr($methodCall); + if (! $getExpr instanceof String_) { + return null; + } + + return $getExpr->value; + } + public function match(MethodCall $methodCall): ?string { - if ($methodCall->isFirstClassCallable()) { + $getExpr = $this->matchGetExpr($methodCall); + if (! $getExpr instanceof ClassConstFetch) { return null; } - if (! $this->nodeNameResolver->isName($methodCall->name, 'get')) { + // must be class const fetch + if (! $this->nodeNameResolver->isName($getExpr->name, 'class')) { return null; } - if (! $this->nodeNameResolver->isName($methodCall->var, 'this')) { + return $this->nodeNameResolver->getName($getExpr->class); + } + + private function isValidContainerCall(MethodCall $methodCall): bool + { + if ($methodCall->var instanceof MethodCall && $this->nodeNameResolver->isName( + $methodCall->var->name, + 'getContainer' + )) { + return true; + } + + if ($methodCall->var instanceof Variable && $this->nodeNameResolver->isName($methodCall->var, 'this')) { + return true; + } + return $methodCall->var instanceof PropertyFetch && $this->nodeNameResolver->isName( + $methodCall->var->var, + 'this' + ) && $this->nodeNameResolver->isName($methodCall->var->name, 'container'); + } + + private function matchGetExpr(MethodCall $methodCall): ?Expr + { + if ($methodCall->isFirstClassCallable()) { return null; } - if (count($methodCall->getArgs()) !== 1) { + if (! $this->nodeNameResolver->isName($methodCall->name, 'get')) { return null; } - $firstArg = $methodCall->getArgs()[0]; - if (! $firstArg->value instanceof ClassConstFetch) { + if (! $this->isValidContainerCall($methodCall)) { return null; } - // must be class const fetch - if (! $this->nodeNameResolver->isName($firstArg->value->name, 'class')) { + if (count($methodCall->getArgs()) !== 1) { return null; } - return $this->nodeNameResolver->getName($firstArg->value->class); + $firstArg = $methodCall->getArgs()[0]; + return $firstArg->value; } } diff --git a/rules/Symfony34/Rector/Closure/ContainerGetNameToTypeInTestsRector.php b/rules/Symfony34/Rector/Closure/ContainerGetNameToTypeInTestsRector.php index 6b462f6d..d2f4976f 100644 --- a/rules/Symfony34/Rector/Closure/ContainerGetNameToTypeInTestsRector.php +++ b/rules/Symfony34/Rector/Closure/ContainerGetNameToTypeInTestsRector.php @@ -102,7 +102,6 @@ public function refactor(Node $node): ?Node } $classConstFetch = new ClassConstFetch(new FullyQualified($serviceType->getClassName()), 'class'); - $node->args[0] = new Arg($classConstFetch); return $node; diff --git a/rules/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector.php b/rules/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector.php index 614d7aa1..df14f03c 100644 --- a/rules/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector.php +++ b/rules/Symfony42/Rector/MethodCall/ContainerGetToConstructorInjectionRector.php @@ -9,6 +9,7 @@ use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Stmt\Class_; use PHPStan\Type\ObjectType; +use Rector\Configuration\Deprecation\Contract\DeprecatedInterface; use Rector\NodeManipulator\ClassDependencyManipulator; use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer; use Rector\PostRector\ValueObject\PropertyMetadata; @@ -21,9 +22,9 @@ /** * Ref: https://github.com/symfony/symfony/blob/master/UPGRADE-4.0.md#console * - * @see \Rector\Symfony\Tests\Symfony42\Rector\MethodCall\ContainerGetToConstructorInjectionRector\ContainerGetToConstructorInjectionRectorTest + * @deprecated This rule is deprecated as too vague and causing too many changes. Use more granular @see \Rector\Symfony\Set\SymfonySetList::SYMFONY_CONSTRUCTOR_INJECTION instead */ -final class ContainerGetToConstructorInjectionRector extends AbstractRector +final class ContainerGetToConstructorInjectionRector extends AbstractRector implements DeprecatedInterface { public function __construct( private readonly DependencyInjectionMethodCallAnalyzer $dependencyInjectionMethodCallAnalyzer, diff --git a/rules/Symfony43/Rector/MethodCall/ConvertRenderTemplateShortNotationToBundleSyntaxRector.php b/rules/Symfony43/Rector/MethodCall/ConvertRenderTemplateShortNotationToBundleSyntaxRector.php index dc9aa75e..21086ab0 100644 --- a/rules/Symfony43/Rector/MethodCall/ConvertRenderTemplateShortNotationToBundleSyntaxRector.php +++ b/rules/Symfony43/Rector/MethodCall/ConvertRenderTemplateShortNotationToBundleSyntaxRector.php @@ -37,7 +37,10 @@ public function getRuleDefinition(): RuleDefinition [ new CodeSample( <<<'CODE_SAMPLE' -class BaseController extends Controller { +use Symfony\Bundle\FrameworkBundle\Controller\Controller; + +class BaseController extends Controller +{ function indexAction() { $this->render('appBundle:Landing\Main:index.html.twig'); @@ -46,7 +49,10 @@ function indexAction() CODE_SAMPLE , <<<'CODE_SAMPLE' -class BaseController extends Controller { +use Symfony\Bundle\FrameworkBundle\Controller\Controller; + +class BaseController extends Controller +{ function indexAction() { $this->render('@app/Landing/Main/index.html.twig'); diff --git a/src/Enum/SymfonyClass.php b/src/Enum/SymfonyClass.php index 84d0b2bf..99cdc46a 100644 --- a/src/Enum/SymfonyClass.php +++ b/src/Enum/SymfonyClass.php @@ -25,4 +25,24 @@ final class SymfonyClass * @var string */ public const CONTAINER_AWARE_COMMAND = 'Symfony\Bundle\FrameworkBundle\Command\ContainerAwareCommand'; + + /** + * @var string + */ + public const EVENT_DISPATCHER_INTERFACE = 'Symfony\Contracts\EventDispatcher\EventDispatcherInterface'; + + /** + * @var string + */ + public const VALIDATOR_INTERFACE = 'Symfony\Component\Validator\Validator\ValidatorInterface'; + + /** + * @var string + */ + public const LOGGER_INTERFACE = 'Psr\Log\LoggerInterface'; + + /** + * @var string + */ + public const SERIALIZER_INTERFACE = 'JMS\Serializer\SerializerInterface'; } diff --git a/stubs/Symfony/Bundle/FrameworkBundle/Controller/Controller.php b/stubs/Symfony/Bundle/FrameworkBundle/Controller/Controller.php index 759bf43b..581bd786 100644 --- a/stubs/Symfony/Bundle/FrameworkBundle/Controller/Controller.php +++ b/stubs/Symfony/Bundle/FrameworkBundle/Controller/Controller.php @@ -13,6 +13,8 @@ abstract class Controller { + protected $container; + /** * @param string|AbstractType|FormInterface $formType */