[Test] Rename fixture and refactor to solve random error (#3677)

* [Test] Rename fixture to solve random error * [ci-review] Rector Rectify * [ci-review] Rector Rectify * also rename fixture on PropertyRenameFactoryTest * increment name * reduce fixture length * refactor * try give enough time to write to temporary file when FileSystem::write() overlapped * fix phpstan * retry write --------- Co-authored-by: GitHub Action <actions@github.com>
rectorphp · Apr 24, 2023 · 10c36b0 · 10c36b0
1 parent 07a0475
commit 10c36b0
Show file tree

Hide file tree

Showing 11 changed files with 54 additions and 25 deletions.
diff --git a/packages/Testing/PHPUnit/AbstractRectorTestCase.php b/packages/Testing/PHPUnit/AbstractRectorTestCase.php
@@ -125,6 +125,14 @@ protected function doTestFile(string $fixtureFilePath): void
         // write temp file
         FileSystem::write($inputFilePath, $inputFileContents);
 
+        if (! is_file($inputFilePath)) {
+            // give enough time to write process
+            sleep(3);
+
+            // write temp file
+            FileSystem::write($inputFilePath, $inputFileContents);
+        }
+
         $this->doTestFileMatchesExpectedContent($inputFilePath, $inputFileContents, $expectedFileContents, $fixtureFilePath);
     }
 

diff --git a/phpstan.neon b/phpstan.neon
@@ -806,3 +806,15 @@ parameters:
         - '#Access to an undefined property Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts#'
         - '#Access to an undefined property PhpParser\\Node\\Stmt\\ClassLike\|Rector\\Core\\Contract\\PhpParser\\Node\\StmtsAwareInterface\:\:\$stmts#'
 
+        -
+            message: '#Method "renameVariableInFunctionLike\(\)" returns bool type, so the name should start with is/has/was#'
+            path: rules/Naming/VariableRenamer.php
+
+        -
+            message: '#Method call return value that should be used, but is not#'
+            paths:
+                - rules/Naming/ParamRenamer/ParamRenamer.php
+                - rules/Naming/PropertyRenamer/PropertyPromotionRenamer.php
+                - rules/Naming/Rector/Assign/RenameVariableToMatchMethodCallReturnTypeRector.php
+                - rules/Naming/Rector/ClassMethod/RenameVariableToMatchNewTypeRector.php
+                - rules/Php80/Rector/Class_/ClassPropertyAssignToConstructorPromotionRector.php
diff --git a/...ToMatchTypeRector/Fixture/fixture.php.inc → ...ename_property_and_property_fetch.php.inc b/...ToMatchTypeRector/Fixture/fixture.php.inc → ...ename_property_and_property_fetch.php.inc
@@ -4,7 +4,7 @@ namespace Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Fixt
 
 use Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Source\EliteManager;
 
-class SomeClass
+class RenamePropertyAndPropertyFetch
 {
     /**
      * @var EliteManager
@@ -25,7 +25,7 @@ namespace Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Fixt
 
 use Rector\Tests\Naming\Rector\Class_\RenamePropertyToMatchTypeRector\Source\EliteManager;
 
-class SomeClass
+class RenamePropertyAndPropertyFetch
 {
     /**
      * @var EliteManager

diff --git a/...er_than_return_type_for_iterables.php.inc → ...me_over_return_type_for_iterables.php.inc b/...er_than_return_type_for_iterables.php.inc → ...me_over_return_type_for_iterables.php.inc
@@ -4,7 +4,7 @@ namespace Rector\Tests\Naming\Rector\Foreach_\RenameForeachValueVariableToMatchM
 
 use Rector\Tests\Naming\Rector\Foreach_\RenameForeachValueVariableToMatchMethodCallReturnTypeRector\Source\DummyCollection;
 
-final class ForeachPreferMethodNameRatherThanReturnTypeForIterables
+final class ForeachPreferMethodNameOverReturnTypeForIterables
 {
     private DummyCollection $books;
 
@@ -34,7 +34,7 @@ namespace Rector\Tests\Naming\Rector\Foreach_\RenameForeachValueVariableToMatchM
 
 use Rector\Tests\Naming\Rector\Foreach_\RenameForeachValueVariableToMatchMethodCallReturnTypeRector\Source\DummyCollection;
 
-final class ForeachPreferMethodNameRatherThanReturnTypeForIterables
+final class ForeachPreferMethodNameOverReturnTypeForIterables
 {
     private DummyCollection $books;
 

diff --git a/...meFactory/Fixture/skip_some_class.php.inc → ...name_property_and_property_fetch2.php.inc b/...meFactory/Fixture/skip_some_class.php.inc → ...name_property_and_property_fetch2.php.inc
@@ -6,7 +6,7 @@ namespace Rector\Tests\Naming\ValueObjectFactory\PropertyRenameFactory\Fixture;
 
 use Rector\Tests\Naming\ValueObjectFactory\PropertyRenameFactory\Source\EliteManager;
 
-final class SkipSomeClass
+final class RenamePropertyAndPropertyFetch2
 {
     /**
      * @var EliteManager

diff --git a/rules-tests/Naming/ValueObjectFactory/PropertyRenameFactory/PropertyRenameFactoryTest.php b/rules-tests/Naming/ValueObjectFactory/PropertyRenameFactory/PropertyRenameFactoryTest.php
@@ -65,6 +65,6 @@ public function test(string $filePathWithProperty, string $expectedName, string
 
     public static function provideData(): Iterator
     {
-        yield [__DIR__ . '/Fixture/skip_some_class.php.inc', 'eliteManager', 'eventManager'];
+        yield [__DIR__ . '/Fixture/rename_property_and_property_fetch2.php.inc', 'eliteManager', 'eventManager'];
     }
 }
diff --git a/rules/Naming/Rector/Foreach_/RenameForeachValueVariableToMatchMethodCallReturnTypeRector.php b/rules/Naming/Rector/Foreach_/RenameForeachValueVariableToMatchMethodCallReturnTypeRector.php
@@ -98,9 +98,18 @@ public function refactor(Node $node): ?Node
             return null;
         }
 
-        $this->renameVariable($variableAndCallForeach, $expectedName);
+        $hasRenamed = $this->variableRenamer->renameVariableInFunctionLike(
+            $variableAndCallForeach->getFunctionLike(),
+            $variableAndCallForeach->getVariableName(),
+            $expectedName,
+            null
+        );
+
+        if ($hasRenamed) {
+            return $node;
+        }
 
-        return $node;
+        return null;
     }
 
     private function shouldSkip(VariableAndCallForeach $variableAndCallForeach, string $expectedName): bool
@@ -120,14 +129,4 @@ private function shouldSkip(VariableAndCallForeach $variableAndCallForeach, stri
             $variableAndCallForeach->getVariable()
         );
     }
-
-    private function renameVariable(VariableAndCallForeach $variableAndCallForeach, string $expectedName): void
-    {
-        $this->variableRenamer->renameVariableInFunctionLike(
-            $variableAndCallForeach->getFunctionLike(),
-            $variableAndCallForeach->getVariableName(),
-            $expectedName,
-            null
-        );
-    }
 }
diff --git a/rules/Naming/VariableRenamer.php b/rules/Naming/VariableRenamer.php
@@ -36,16 +36,18 @@ public function renameVariableInFunctionLike(
         string $oldName,
         string $expectedName,
         ?Assign $assign = null
-    ): void {
+    ): bool {
         $isRenamingActive = false;
 
         if (! $assign instanceof Assign) {
             $isRenamingActive = true;
         }
 
+        $hasRenamed = false;
+
         $this->simpleCallableNodeTraverser->traverseNodesWithCallable(
             (array) $functionLike->getStmts(),
-            function (Node $node) use ($oldName, $expectedName, $assign, &$isRenamingActive): ?Variable {
+            function (Node $node) use ($oldName, $expectedName, $assign, &$isRenamingActive, &$hasRenamed): ?Variable {
                 if ($assign instanceof Assign && $node === $assign) {
                     $isRenamingActive = true;
                     return null;
@@ -71,9 +73,16 @@ function (Node $node) use ($oldName, $expectedName, $assign, &$isRenamingActive)
                     return null;
                 }
 
-                return $this->renameVariableIfMatchesName($node, $oldName, $expectedName);
+                $variable = $this->renameVariableIfMatchesName($node, $oldName, $expectedName);
+                if ($variable instanceof Variable) {
+                    $hasRenamed = true;
+                }
+
+                return $variable;
             }
         );
+
+        return $hasRenamed;
     }
 
     private function isParamInParentFunction(Variable $variable): bool

diff --git a/src/FileSystem/PhpFilesFinder.php b/src/FileSystem/PhpFilesFinder.php
@@ -5,8 +5,6 @@
 namespace Rector\Core\FileSystem;
 
 use Rector\Caching\UnchangedFilesFilter;
-use Rector\Core\Util\StringUtils;
-use Rector\Core\ValueObject\StaticNonPhpFileSuffixes;
 
 final class PhpFilesFinder
 {

diff --git a/src/NodeManipulator/ClassMethodAssignManipulator.php b/src/NodeManipulator/ClassMethodAssignManipulator.php
@@ -217,7 +217,11 @@ private function collectReferenceVariableNames(ClassMethod $classMethod): array
 
         foreach ($variables as $variable) {
             $variableName = $this->nodeNameResolver->getName($variable);
-            if ($variableName === 'this' || $variableName === null) {
+            if ($variableName === 'this') {
+                continue;
+            }
+
+            if ($variableName === null) {
                 continue;
             }
 

diff --git a/src/NonPhpFile/NonPhpFileProcessor.php b/src/NonPhpFile/NonPhpFileProcessor.php
@@ -12,7 +12,6 @@
 use Rector\Core\ValueObject\Configuration;
 use Rector\Core\ValueObject\Error\SystemError;
 use Rector\Core\ValueObject\Reporting\FileDiff;
-use Rector\Core\ValueObject\StaticNonPhpFileSuffixes;
 use Rector\Parallel\ValueObject\Bridge;
 use Symfony\Component\Filesystem\Filesystem;