diff --git a/config/v12/typo3-120.php b/config/v12/typo3-120.php index 2011f2c40..d6ef5092c 100644 --- a/config/v12/typo3-120.php +++ b/config/v12/typo3-120.php @@ -8,6 +8,7 @@ use Ssch\TYPO3Rector\CodeQuality\General\RenameClassMapAliasRector; use Ssch\TYPO3Rector\TYPO312\v0\AddMethodToWidgetInterfaceClassesRector; use Ssch\TYPO3Rector\TYPO312\v0\ChangeExtbaseValidatorsRector; +use Ssch\TYPO3Rector\TYPO312\v0\ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector; use Ssch\TYPO3Rector\TYPO312\v0\ImplementSiteLanguageAwareInterfaceRector; use Ssch\TYPO3Rector\TYPO312\v0\MigrateBackendModuleRegistrationRector; use Ssch\TYPO3Rector\TYPO312\v0\MigrateContentObjectRendererGetTypoLinkUrlRector; @@ -171,4 +172,5 @@ $rectorConfig->rule(MigrateFetchToFetchAssociativeRector::class); $rectorConfig->rule(MigrateBackendModuleRegistrationRector::class); $rectorConfig->rule(MigrateContentObjectRendererGetTypoLinkUrlRector::class); + $rectorConfig->rule(ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector::class); }; diff --git a/rules/TYPO312/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector.php b/rules/TYPO312/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector.php new file mode 100644 index 000000000..5efd682a6 --- /dev/null +++ b/rules/TYPO312/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector.php @@ -0,0 +1,134 @@ +> + */ + public function getNodeTypes(): array + { + return [ClassMethod::class]; + } + + public function getRuleDefinition(): RuleDefinition + { + return new RuleDefinition('Extbase controller actions with redirects must return ResponseInterface', [ + new CodeSample( + <<<'CODE_SAMPLE' +use TYPO3\CMS\Extbase\Mvc\Controller\ActionController; + +class MyController extends ActionController +{ + public function someAction() + { + $this->redirect('foo', 'bar'); + } +} +CODE_SAMPLE + , + <<<'CODE_SAMPLE' +use Psr\Http\Message\ResponseInterface; +use TYPO3\CMS\Extbase\Mvc\Controller\ActionController; + +class MyController extends ActionController +{ + public function someAction(): ResponseInterface + { + return $this->redirect('foo', 'bar'); + } +} +CODE_SAMPLE + ), + ]); + } + + /** + * @param ClassMethod $node + */ + public function refactor(Node $node): ?Node + { + if (! $this->nodeTypeResolver->isMethodStaticCallOrClassMethodObjectType( + $node, + new ObjectType('TYPO3\CMS\Extbase\Mvc\Controller\ActionController') + )) { + return null; + } + + $hasChanged = false; + + $this->traverseNodesWithCallable($node, function (Node $node) use (&$hasChanged) { + if (! $node instanceof Expression) { + return null; + } + + $methodCall = $node->expr; + if (! $methodCall instanceof MethodCall) { + return null; + } + + if (! $this->isNames($methodCall->name, ['redirect', 'redirectToUri'])) { + return null; + } + + $returnMethodCall = new Return_($methodCall); + $hasChanged = true; + return $returnMethodCall; + }); + + if (! $hasChanged) { + return null; + } + + $this->changeActionMethodReturnTypeIfPossible($node); + + return $node; + } + + private function changeActionMethodReturnTypeIfPossible(ClassMethod $actionMethodNode): void + { + if ($actionMethodNode->returnType instanceof Identifier + && $actionMethodNode->returnType->name !== null + && $actionMethodNode->returnType->name === 'void' + ) { + $actionMethodNode->returnType = null; + } + + $comments = $actionMethodNode->getComments(); + $comments = array_map( + static fn (Comment $comment) => new Comment(str_replace( + '@return void', + '@return \Psr\Http\Message\ResponseInterface', + $comment->getText() + )), + $comments + ); + $actionMethodNode->setAttribute(AttributeKey::COMMENTS, $comments); + + // Add returnType only if it is the only statement, otherwise it is not reliable + if (is_countable($actionMethodNode->stmts) && count((array) $actionMethodNode->stmts) === 1) { + $actionMethodNode->returnType = new FullyQualified('Psr\Http\Message\ResponseInterface'); + } + } +} diff --git a/tests/Rector/v12/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRectorTest.php b/tests/Rector/v12/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRectorTest.php new file mode 100644 index 000000000..fe55d7b4e --- /dev/null +++ b/tests/Rector/v12/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRectorTest.php @@ -0,0 +1,31 @@ +doTestFile($filePath); + } + + /** + * @return \Iterator> + */ + public static function provideData(): \Iterator + { + return self::yieldFilesFromDirectory(__DIR__ . '/Fixture'); + } + + public function provideConfigFilePath(): string + { + return __DIR__ . '/config/configured_rule.php'; + } +} diff --git a/tests/Rector/v12/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector/Fixture/extbase_controller_actions_return_response_interface.php.inc b/tests/Rector/v12/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector/Fixture/extbase_controller_actions_return_response_interface.php.inc new file mode 100644 index 000000000..abf5b66f6 --- /dev/null +++ b/tests/Rector/v12/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector/Fixture/extbase_controller_actions_return_response_interface.php.inc @@ -0,0 +1,79 @@ +redirect('list'); + } + + public function imageDeleteAction() + { + $this->redirectToUri('foo'); + } + + /** + * @return void + */ + public function imageDeleteActionWithReturnVoid(): void + { + $this->redirectToUri('foo'); + } + + public function alreadyMigratedHandleExpiredRegistrationsAction(): ResponseInterface + { + return $this->redirect('list'); + } + + public function alreadyMigratedImageDeleteAction(): ResponseInterface + { + return $this->redirectToUri('foo'); + } +} + +?> +----- +redirect('list'); + } + + public function imageDeleteAction(): ResponseInterface + { + return $this->redirectToUri('foo'); + } + + /** + * @return \Psr\Http\Message\ResponseInterface + */ + public function imageDeleteActionWithReturnVoid(): ResponseInterface + { + return $this->redirectToUri('foo'); + } + + public function alreadyMigratedHandleExpiredRegistrationsAction(): ResponseInterface + { + return $this->redirect('list'); + } + + public function alreadyMigratedImageDeleteAction(): ResponseInterface + { + return $this->redirectToUri('foo'); + } +} + +?> diff --git a/tests/Rector/v12/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector/config/configured_rule.php b/tests/Rector/v12/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector/config/configured_rule.php new file mode 100644 index 000000000..27959197a --- /dev/null +++ b/tests/Rector/v12/v0/ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector/config/configured_rule.php @@ -0,0 +1,11 @@ +import(__DIR__ . '/../../../../../../config/config_test.php'); + $rectorConfig->rule(ExtbaseActionsWithRedirectMustReturnResponseInterfaceRector::class); +};