TASK: Fix bug for method with redirect (#3218)

* TASK: Fix bug for method with redirect Resolves: #3038 * TASK: Downgrade github action * [ci-review] Rector Rectify * [ci-review] Rector Rectify Co-authored-by: GitHub Action <action@github.com>
sabbelasichon · Nov 20, 2022 · 98208a1 · 98208a1
1 parent 37661f0
commit 98208a1
Show file tree

Hide file tree

Showing 19 changed files with 123 additions and 63 deletions.
diff --git a/.github/workflows/rector.yaml b/.github/workflows/rector.yaml
@@ -57,7 +57,7 @@ jobs:
             -
                 # commit only to core contributors who have repository access
                 if: github.event.pull_request.head.repo.full_name == github.repository
-                uses: EndBug/add-and-commit@v9
+                uses: EndBug/add-and-commit@v7.5.0
                 with:
                     # The arguments for the `git add` command (see the paragraph below for more info)
                     add: .

diff --git a/src/FileProcessor/Resources/Icons/IconsFileProcessor.php b/src/FileProcessor/Resources/Icons/IconsFileProcessor.php
@@ -32,7 +32,7 @@ final class IconsFileProcessor implements FileProcessorInterface
      * @var IconRectorInterface[]
      * @readonly
      */
-    private array $iconsRector;
+    private array $iconsRector = [];
 
     /**
      * @readonly

diff --git a/src/FileProcessor/TypoScript/Conditions/GlobalStringConditionMatcher.php b/src/FileProcessor/TypoScript/Conditions/GlobalStringConditionMatcher.php
@@ -34,10 +34,10 @@ public function change(string $condition): ?string
                 $matches
             );
 
-            $type = trim((string) $matches['type']);
-            $property = trim((string) $matches['property']);
-            $operator = trim((string) $matches['operator']);
-            $value = trim((string) $matches['value']);
+            $type = trim($matches['type']);
+            $property = trim($matches['property']);
+            $operator = trim($matches['operator']);
+            $value = trim($matches['value']);
 
             switch ($type) {
                 case 'ENV':

diff --git a/src/FileProcessor/TypoScript/Conditions/GlobalVarConditionMatcher.php b/src/FileProcessor/TypoScript/Conditions/GlobalVarConditionMatcher.php
@@ -41,10 +41,10 @@ public function change(string $condition): ?string
                 continue;
             }
 
-            $type = isset($matches['type']) ? trim((string) $matches['type']) : '';
-            $property = isset($matches['property']) ? trim((string) $matches['property']) : '';
-            $operator = isset($matches['operator']) ? trim((string) $matches['operator']) : '';
-            $value = isset($matches[self::VALUE]) ? trim((string) $matches[self::VALUE]) : '';
+            $type = isset($matches['type']) ? trim($matches['type']) : '';
+            $property = isset($matches['property']) ? trim($matches['property']) : '';
+            $operator = isset($matches['operator']) ? trim($matches['operator']) : '';
+            $value = isset($matches[self::VALUE]) ? trim($matches[self::VALUE]) : '';
 
             $key = sprintf('%s.%s.%s', $type, $property, $operator);
 
@@ -106,7 +106,7 @@ public function change(string $condition): ?string
                     $condition = $valueMatches['condition'];
                 }
 
-                $newConditions[] = sprintf('%s in [%s]', trim((string) $condition), trim(implode(',', $values)));
+                $newConditions[] = sprintf('%s in [%s]', trim($condition), trim(implode(',', $values)));
             } else {
                 $newConditions[] = implode(' || ', $conditions[$key]);
             }

diff --git a/src/FileProcessor/TypoScript/Conditions/PageConditionMatcher.php b/src/FileProcessor/TypoScript/Conditions/PageConditionMatcher.php
@@ -29,7 +29,7 @@ public function change(string $condition): ?string
             return $condition;
         }
 
-        return sprintf('page["%s"] == "%s"', trim((string) $matches[1]), trim((string) $matches[2]));
+        return sprintf('page["%s"] == "%s"', trim($matches[1]), trim($matches[2]));
     }
 
     public function shouldApply(string $condition): bool

diff --git a/src/FileProcessor/TypoScript/Conditions/TimeConditionMatcher.php b/src/FileProcessor/TypoScript/Conditions/TimeConditionMatcher.php
@@ -70,7 +70,7 @@ public function change(string $condition): ?string
                 'date("%s") %s %s',
                 trim(self::TIME_MAPPING[$time]),
                 trim(self::OPERATOR_MAPPING[$operator]),
-                trim((string) $value)
+                trim($value)
             );
         }
 

diff --git a/...ileProcessor/TypoScript/Rector/v9/v4/OldConditionToExpressionLanguageTypoScriptRector.php b/...ileProcessor/TypoScript/Rector/v9/v4/OldConditionToExpressionLanguageTypoScriptRector.php
@@ -65,7 +65,7 @@ public function enterNode(Statement $statement): void
 
         foreach ($conditions as $condition) {
             foreach ($this->conditionMatchers as $conditionMatcher) {
-                $condition = trim((string) $condition);
+                $condition = trim($condition);
                 if (! $conditionMatcher->shouldApply($condition)) {
                     continue;
                 }

diff --git a/src/FileProcessor/TypoScript/TypoScriptFileProcessor.php b/src/FileProcessor/TypoScript/TypoScriptFileProcessor.php
@@ -100,13 +100,13 @@ final class TypoScriptFileProcessor implements ConfigurableProcessorInterface
      * @var TypoScriptRectorInterface[]
      * @readonly
      */
-    private array $typoScriptRectors;
+    private array $typoScriptRectors = [];
 
     /**
      * @var TypoScriptPostRectorInterface[]
      * @readonly
      */
-    private array $typoScriptPostRectors;
+    private array $typoScriptPostRectors = [];
 
     private ParameterProvider $parameterProvider;
 

diff --git a/src/NodeAnalyzer/ExtbaseControllerRedirectAnalyzer.php b/src/NodeAnalyzer/ExtbaseControllerRedirectAnalyzer.php
@@ -0,0 +1,55 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Ssch\TYPO3Rector\NodeAnalyzer;
+
+use PhpParser\Node;
+use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Stmt\ClassMethod;
+use PHPStan\Type\ObjectType;
+use Rector\Core\PhpParser\Node\BetterNodeFinder;
+use Rector\NodeNameResolver\NodeNameResolver;
+use Rector\NodeTypeResolver\NodeTypeResolver;
+
+final class ExtbaseControllerRedirectAnalyzer
+{
+    private NodeTypeResolver $nodeTypeResolver;
+
+    private BetterNodeFinder $betterNodeFinder;
+
+    private NodeNameResolver $nodeNameResolver;
+
+    public function __construct(
+        NodeTypeResolver $nodeTypeResolver,
+        BetterNodeFinder $betterNodeFinder,
+        NodeNameResolver $nodeNameResolver
+    ) {
+        $this->nodeTypeResolver = $nodeTypeResolver;
+        $this->betterNodeFinder = $betterNodeFinder;
+        $this->nodeNameResolver = $nodeNameResolver;
+    }
+
+    /**
+     * @param string[] $redirectMethods
+     */
+    public function hasRedirectCall(ClassMethod $classMethod, array $redirectMethods): bool
+    {
+        return (bool) $this->betterNodeFinder->find((array) $classMethod->stmts, function (Node $node) use (
+            $redirectMethods
+        ): bool {
+            if (! $node instanceof MethodCall) {
+                return false;
+            }
+
+            if (! $this->nodeTypeResolver->isMethodStaticCallOrClassMethodObjectType(
+                $node,
+                new ObjectType('TYPO3\CMS\Extbase\Mvc\Controller\ActionController')
+            )) {
+                return false;
+            }
+
+            return $this->nodeNameResolver->isNames($node->name, $redirectMethods);
+        });
+    }
+}
diff --git a/src/Rector/General/ConvertImplicitVariablesToExplicitGlobalsRector.php b/src/Rector/General/ConvertImplicitVariablesToExplicitGlobalsRector.php
@@ -90,10 +90,6 @@ private function shouldSkip(Variable $node): bool
             return false;
         }
 
-        if ($this->filesFinder->isExtTables($this->file->getFilePath())) {
-            return false;
-        }
-
-        return true;
+        return ! $this->filesFinder->isExtTables($this->file->getFilePath());
     }
 }
diff --git a/src/Rector/v11/v0/ExtbaseControllerActionsMustReturnResponseInterfaceRector.php b/src/Rector/v11/v0/ExtbaseControllerActionsMustReturnResponseInterfaceRector.php
@@ -17,6 +17,7 @@
 use PHPStan\Type\ObjectType;
 use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
 use Rector\Core\Rector\AbstractRector;
+use Ssch\TYPO3Rector\NodeAnalyzer\ExtbaseControllerRedirectAnalyzer;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 use Webmozart\Assert\Assert;
@@ -30,6 +31,7 @@ final class ExtbaseControllerActionsMustReturnResponseInterfaceRector extends Ab
 {
     /**
      * @api
+     * @var string
      */
     public const REDIRECT_METHODS = 'redirect_methods';
 
@@ -48,6 +50,13 @@ final class ExtbaseControllerActionsMustReturnResponseInterfaceRector extends Ab
      */
     private array $redirectMethods = ['redirect', 'redirectToUri'];
 
+    private ExtbaseControllerRedirectAnalyzer $extbaseControllerRedirectAnalyzer;
+
+    public function __construct(ExtbaseControllerRedirectAnalyzer $extbaseControllerRedirectAnalyzer)
+    {
+        $this->extbaseControllerRedirectAnalyzer = $extbaseControllerRedirectAnalyzer;
+    }
+
     /**
      * @return array<class-string<Node>>
      */
@@ -196,7 +205,7 @@ private function shouldSkip(ClassMethod $classMethod): bool
             return true;
         }
 
-        if ($this->hasRedirectCall($classMethod)) {
+        if ($this->extbaseControllerRedirectAnalyzer->hasRedirectCall($classMethod, $this->redirectMethods)) {
             return true;
         }
 
@@ -219,24 +228,6 @@ private function findReturns(ClassMethod $classMethod): array
         return $this->betterNodeFinder->findInstanceOf((array) $classMethod->stmts, Return_::class);
     }
 
-    private function hasRedirectCall(ClassMethod $classMethod): bool
-    {
-        return (bool) $this->betterNodeFinder->find((array) $classMethod->stmts, function (Node $node): bool {
-            if (! $node instanceof MethodCall) {
-                return false;
-            }
-
-            if (! $this->nodeTypeResolver->isMethodStaticCallOrClassMethodObjectType(
-                $node,
-                new ObjectType('TYPO3\CMS\Extbase\Mvc\Controller\ActionController')
-            )) {
-                return false;
-            }
-
-            return $this->isNames($node->name, $this->redirectMethods);
-        });
-    }
-
     private function lastStatementIsExitCall(ClassMethod $classMethod): bool
     {
         if (null === $classMethod->stmts) {

diff --git a/src/Rector/v11/v3/typo3/UseNormalizedParamsToGetRequestUrlRector.php b/src/Rector/v11/v3/typo3/UseNormalizedParamsToGetRequestUrlRector.php
@@ -6,6 +6,7 @@
 
 use PhpParser\Node;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Identifier;
 use PHPStan\Type\ObjectType;
 use Rector\Core\Rector\AbstractRector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -34,7 +35,7 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        $node->name = new Node\Identifier('getAttribute');
+        $node->name = new Identifier('getAttribute');
         $node->args = $this->nodeFactory->createArgs(['normalizedParams']);
 
         return $this->nodeFactory->createMethodCall($node, 'getRequestUrl');

diff --git a/src/Rector/v11/v5/SubstituteBackendTemplateViewWithModuleTemplateRector.php b/src/Rector/v11/v5/SubstituteBackendTemplateViewWithModuleTemplateRector.php
@@ -21,6 +21,7 @@
 use Rector\Core\Rector\AbstractRector;
 use Rector\PostRector\Collector\NodesToAddCollector;
 use Rector\PostRector\ValueObject\PropertyMetadata;
+use Ssch\TYPO3Rector\NodeAnalyzer\ExtbaseControllerRedirectAnalyzer;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
@@ -55,12 +56,16 @@ final class SubstituteBackendTemplateViewWithModuleTemplateRector extends Abstra
      */
     private ClassDependencyManipulator $classDependencyManipulator;
 
+    private ExtbaseControllerRedirectAnalyzer $extbaseControllerRedirectAnalyzer;
+
     public function __construct(
         ClassDependencyManipulator $classDependencyManipulator,
-        NodesToAddCollector $nodesToAddCollector
+        NodesToAddCollector $nodesToAddCollector,
+        ExtbaseControllerRedirectAnalyzer $extbaseControllerRedirectAnalyzer
     ) {
         $this->classDependencyManipulator = $classDependencyManipulator;
         $this->nodesToAddCollector = $nodesToAddCollector;
+        $this->extbaseControllerRedirectAnalyzer = $extbaseControllerRedirectAnalyzer;
     }
 
     /**
@@ -87,6 +92,13 @@ public function refactor(Node $node): ?Node
         $classMethods = $node->getMethods();
 
         foreach ($classMethods as $classMethod) {
+            if ($this->extbaseControllerRedirectAnalyzer->hasRedirectCall(
+                $classMethod,
+                ['redirect', 'redirectToUri']
+            )) {
+                continue;
+            }
+
             $this->substituteModuleTemplateMethodCalls($classMethod);
             $this->callSetContentAndGetContent($classMethod);
         }

diff --git a/src/Rector/v12/v0/typo3/ImplementSiteLanguageAwareInterfaceRector.php b/src/Rector/v12/v0/typo3/ImplementSiteLanguageAwareInterfaceRector.php
@@ -6,9 +6,12 @@
 
 use PhpParser\Builder\Property;
 use PhpParser\Node;
+use PhpParser\Node\Expr\Variable;
 use PhpParser\Node\Name\FullyQualified;
+use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Return_;
 use PHPStan\Analyser\Scope;
 use PHPStan\Reflection\ClassReflection;
@@ -90,14 +93,14 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
                 [
                     'flags' => Class_::MODIFIER_PUBLIC,
                     'stmts' => [
-                        new Node\Stmt\Expression(
+                        new Expression(
                             $this->nodeFactory->createPropertyAssignmentWithExpr(
                                 'siteLanguage',
-                                new Node\Expr\Variable('siteLanguage')
+                                new Variable('siteLanguage')
                             )
                         ),
                     ],
-                    'params' => [new Node\Param(new Node\Expr\Variable('siteLanguage'), null, $siteLanguageName)],
+                    'params' => [new Param(new Variable('siteLanguage'), null, $siteLanguageName)],
                 ]
             );
             $node->stmts[] = $setterOfSiteLanguage;

diff --git a/src/Rector/v12/v0/typo3/RegisterExtbaseTypeConvertersAsServicesRector.php b/src/Rector/v12/v0/typo3/RegisterExtbaseTypeConvertersAsServicesRector.php
@@ -7,6 +7,7 @@
 use PhpParser\Node;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Return_;
 use PhpParser\NodeTraverser;
 use PHPStan\Reflection\ReflectionProvider;
 use PHPStan\Type\ObjectType;
@@ -97,7 +98,7 @@ public function refactor(Node $node): ?Node
             new SmartFileInfo($this->file->getFilePath())
         );
 
-        if (null === $extEmConf) {
+        if (! $extEmConf instanceof SmartFileInfo) {
             return null;
         }
 
@@ -154,7 +155,7 @@ public function getRuleDefinition(): RuleDefinition
      *
      * @return array<string, string|int>
      */
-    protected function collectServiceTags(array $classStatements): array
+    private function collectServiceTags(array $classStatements): array
     {
         $collectServiceTags = [
             'name' => 'extbase.type_converter',
@@ -170,7 +171,7 @@ protected function collectServiceTags(array $classStatements): array
             }
 
             /** @var Node\Stmt\Return_[] $returns */
-            $returns = $this->betterNodeFinder->findInstanceOf($node->stmts, Node\Stmt\Return_::class);
+            $returns = $this->betterNodeFinder->findInstanceOf($node->stmts, Return_::class);
 
             $value = null;
             foreach ($returns as $return) {
@@ -218,8 +219,6 @@ private function shouldSkip(StaticCall $node): bool
      */
     private function getYamlConfiguration(string $existingServicesYamlFilePath): array
     {
-        $yamlConfiguration = [];
-
         if (file_exists($existingServicesYamlFilePath)) {
             return Yaml::parse((string) file_get_contents($existingServicesYamlFilePath));
         }
@@ -231,7 +230,7 @@ private function getYamlConfiguration(string $existingServicesYamlFilePath): arr
             }
         }
 
-        return $yamlConfiguration;
+        return [];
     }
 
     private function shouldSkipNodeOfTypeConverter(Node $node): bool
@@ -240,14 +239,10 @@ private function shouldSkipNodeOfTypeConverter(Node $node): bool
             return true;
         }
 
-        if (! $this->nodeNameResolver->isNames(
+        return ! $this->nodeNameResolver->isNames(
             $node->name,
             ['getSupportedSourceTypes', 'getSupportedTargetType', 'getPriority']
-        )) {
-            return true;
-        }
-
-        return false;
+        );
     }
 
     /**

diff --git a/src/Rector/v12/v0/typo3/UseConfigArrayForTSFEPropertiesRector.php b/src/Rector/v12/v0/typo3/UseConfigArrayForTSFEPropertiesRector.php
@@ -95,6 +95,7 @@ private function shouldSkip(PropertyFetch $node): bool
         )) {
             return false;
         }
+
         return ! $this->isObjectType(
             $node->var,
             new ObjectType('TYPO3\CMS\Frontend\Controller\TypoScriptFrontendController')