[TypeDeclaration] Skip test methods with exception in ReturnNeverType…

…Rector, [php74] Skip conditinal assign in RestoreDefaultNullToNullableTypePropertyRector as most likely desired to assign or fail (#6430) * [TypeDeclaration] Skip test methods with exception in ReturnNeverTypeRector * [php74] Skip conditinal assign in RestoreDefaultNullToNullableTypePropertyRector as most likely desired to assign or fail
rectorphp · Nov 13, 2024 · 3ee5b72 · 3ee5b72
1 parent db1a84c
commit 3ee5b72
Show file tree

Hide file tree

Showing 9 changed files with 116 additions and 36 deletions.
diff --git a/...RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_assigned_in_construct.php.inc b/...RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_assigned_in_construct.php.inc
@@ -2,7 +2,7 @@
 
 namespace Rector\Tests\Php74\Rector\Property\RestoreDefaultNullToNullableTypePropertyRector\Fixture;
 
-class SkipAssignedInConstruct
+final class SkipAssignedInConstruct
 {
     public ?string $name;
 

diff --git a/...ctor/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_checked.php.inc b/...ctor/Property/RestoreDefaultNullToNullableTypePropertyRector/Fixture/skip_checked.php.inc
@@ -0,0 +1,19 @@
+<?php
+
+namespace Rector\Tests\Php74\Rector\Property\RestoreDefaultNullToNullableTypePropertyRector\Fixture;
+
+final class SkipChecked
+{
+    private ?string $name;
+
+    public function __construct(array $items)
+    {
+        foreach ($items as $item) {
+            $this->name = $item;
+        }
+
+        if (! isset($this->item)) {
+            throw new \InvalidArgumentException();
+        }
+    }
+}
diff --git a/...TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector/Fixture/skip_test_method.php.inc b/...TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector/Fixture/skip_test_method.php.inc
@@ -0,0 +1,16 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnNeverTypeRector\Fixture;
+
+use Exception;
+use PHPUnit\Framework\TestCase;
+
+final class SkipTestMethod extends TestCase
+{
+    public function testSomething(): void
+    {
+        $this->expectException(Exception::class);
+
+        throw new \InvalidArgumentException();
+    }
+}
diff --git a/rules/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector.php b/rules/Php74/Rector/Property/RestoreDefaultNullToNullableTypePropertyRector.php
@@ -71,7 +71,7 @@ public function refactor(Node $node): ?Node
         $hasChanged = false;
 
         foreach ($node->getProperties() as $property) {
-            if ($this->shouldSkip($property, $node)) {
+            if ($this->shouldSkipProperty($property, $node)) {
                 continue;
             }
 
@@ -93,7 +93,7 @@ public function provideMinPhpVersion(): int
         return PhpVersionFeature::TYPED_PROPERTIES;
     }
 
-    private function shouldSkip(Property $property, Class_ $class): bool
+    private function shouldSkipProperty(Property $property, Class_ $class): bool
     {
         if ($property->type === null) {
             return true;
@@ -103,8 +103,7 @@ private function shouldSkip(Property $property, Class_ $class): bool
             return true;
         }
 
-        $onlyProperty = $property->props[0];
-        if ($onlyProperty->default instanceof Expr) {
+        if ($property->props[0]->default instanceof Expr) {
             return true;
         }
 
@@ -118,7 +117,7 @@ private function shouldSkip(Property $property, Class_ $class): bool
 
         // is variable assigned in constructor
         $propertyName = $this->getName($property);
-        return $this->constructorAssignDetector->isPropertyAssigned($class, $propertyName);
+        return $this->constructorAssignDetector->isPropertyAssignedConditionally($class, $propertyName);
     }
 
     private function isReadonlyProperty(Property $property): bool

diff --git a/rules/Php80/Rector/FunctionLike/MixedTypeRector.php b/rules/Php80/Rector/FunctionLike/MixedTypeRector.php
@@ -94,11 +94,8 @@ public function refactor(Node $node): ?Node
 
         $this->refactorParamTypes($node, $phpDocInfo);
         $hasChanged = $this->paramTagRemover->removeParamTagsIfUseless($phpDocInfo, $node, new MixedType());
-        if ($this->hasChanged) {
-            return $node;
-        }
 
-        if ($hasChanged) {
+        if ($this->hasChanged || $hasChanged) {
             return $node;
         }
 

diff --git a/rules/TypeDeclaration/AlreadyAssignDetector/ConstructorAssignDetector.php b/rules/TypeDeclaration/AlreadyAssignDetector/ConstructorAssignDetector.php
@@ -10,6 +10,7 @@
 use PhpParser\Node\Expr\PropertyFetch;
 use PhpParser\Node\Identifier;
 use PhpParser\Node\Stmt;
+use PhpParser\Node\Stmt\Class_;
 use PhpParser\Node\Stmt\ClassLike;
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Else_;
@@ -19,6 +20,8 @@
 use PhpParser\NodeTraverser;
 use PHPStan\Type\ObjectType;
 use Rector\NodeAnalyzer\PropertyFetchAnalyzer;
+use Rector\NodeDecorator\StatementDepthAttributeDecorator;
+use Rector\NodeTypeResolver\Node\AttributeKey;
 use Rector\NodeTypeResolver\NodeTypeResolver;
 use Rector\PhpDocParser\NodeTraverser\SimpleCallableNodeTraverser;
 use Rector\PhpParser\Comparing\NodeComparator;
@@ -28,11 +31,6 @@
 
 final readonly class ConstructorAssignDetector
 {
-    /**
-     * @var string
-     */
-    private const IS_FIRST_LEVEL_STATEMENT = 'first_level_stmt';
-
     public function __construct(
         private NodeTypeResolver $nodeTypeResolver,
         private PropertyAssignMatcher $propertyAssignMatcher,
@@ -43,7 +41,12 @@ public function __construct(
     ) {
     }
 
-    public function isPropertyAssigned(ClassLike $classLike, string $propertyName): bool
+    public function isPropertyAssignedConditionally(Class_ $class, string $propertyName): bool
+    {
+        return $this->isPropertyAssigned($class, $propertyName, true);
+    }
+
+    public function isPropertyAssigned(ClassLike $classLike, string $propertyName, bool $allowConditional = false): bool
     {
         $initializeClassMethods = $this->matchInitializeClassMethod($classLike);
         if ($initializeClassMethods === []) {
@@ -52,12 +55,12 @@ public function isPropertyAssigned(ClassLike $classLike, string $propertyName):
 
         $isAssignedInConstructor = false;
 
-        $this->decorateFirstLevelStatementAttribute($initializeClassMethods);
+        StatementDepthAttributeDecorator::decorateClassMethods($initializeClassMethods);
 
         foreach ($initializeClassMethods as $initializeClassMethod) {
             $this->simpleCallableNodeTraverser->traverseNodesWithCallable((array) $initializeClassMethod->stmts, function (
                 Node $node
-            ) use ($propertyName, &$isAssignedInConstructor): ?int {
+            ) use ($propertyName, &$isAssignedInConstructor, $allowConditional): ?int {
                 if ($this->isIfElseAssign($node, $propertyName)) {
                     $isAssignedInConstructor = true;
                     return NodeTraverser::STOP_TRAVERSAL;
@@ -77,10 +80,15 @@ public function isPropertyAssigned(ClassLike $classLike, string $propertyName):
                     return NodeTraverser::STOP_TRAVERSAL;
                 }
 
-                $isFirstLevelStatement = $assign->getAttribute(self::IS_FIRST_LEVEL_STATEMENT);
+                $isFirstLevelStatement = $assign->getAttribute(AttributeKey::IS_FIRST_LEVEL_STATEMENT);
 
                 // cannot be nested
                 if ($isFirstLevelStatement !== true) {
+                    if ($allowConditional) {
+                        $isAssignedInConstructor = true;
+                        return NodeTraverser::STOP_TRAVERSAL;
+                    }
+
                     return null;
                 }
 
@@ -139,22 +147,6 @@ private function matchAssignExprToPropertyName(Node $node, string $propertyName)
         return $this->propertyAssignMatcher->matchPropertyAssignExpr($node, $propertyName);
     }
 
-    /**
-     * @param ClassMethod[] $classMethods
-     */
-    private function decorateFirstLevelStatementAttribute(array $classMethods): void
-    {
-        foreach ($classMethods as $classMethod) {
-            foreach ((array) $classMethod->stmts as $methodStmt) {
-                $methodStmt->setAttribute(self::IS_FIRST_LEVEL_STATEMENT, true);
-
-                if ($methodStmt instanceof Expression) {
-                    $methodStmt->expr->setAttribute(self::IS_FIRST_LEVEL_STATEMENT, true);
-                }
-            }
-        }
-    }
-
     /**
      * @return ClassMethod[]
      */

diff --git a/rules/TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector.php b/rules/TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector.php
@@ -8,6 +8,7 @@
 use PhpParser\Node\Stmt\ClassMethod;
 use PhpParser\Node\Stmt\Function_;
 use Rector\PHPStan\ScopeFetcher;
+use Rector\PHPUnit\NodeAnalyzer\TestsNodeAnalyzer;
 use Rector\Rector\AbstractRector;
 use Rector\TypeDeclaration\NodeManipulator\AddNeverReturnType;
 use Rector\ValueObject\PhpVersionFeature;
@@ -21,7 +22,8 @@
 final class ReturnNeverTypeRector extends AbstractRector implements MinPhpVersionInterface
 {
     public function __construct(
-        private readonly AddNeverReturnType $addNeverReturnType
+        private readonly AddNeverReturnType $addNeverReturnType,
+        private readonly TestsNodeAnalyzer $testsNodeAnalyzer
     ) {
     }
 
@@ -67,11 +69,33 @@ public function getNodeTypes(): array
     public function refactor(Node $node): ?Node
     {
         $scope = ScopeFetcher::fetch($node);
+
+        if ($this->isTestClassMethodWithFilledReturnType($node)) {
+            return null;
+        }
+
         return $this->addNeverReturnType->add($node, $scope);
     }
 
     public function provideMinPhpVersion(): int
     {
         return PhpVersionFeature::NEVER_TYPE;
     }
+
+    private function isTestClassMethodWithFilledReturnType(ClassMethod|Function_ $callLike): bool
+    {
+        if (! $callLike instanceof ClassMethod) {
+            return false;
+        }
+
+        if (! $callLike->isPublic()) {
+            return false;
+        }
+
+        if (! $this->testsNodeAnalyzer->isInTestClass($callLike)) {
+            return false;
+        }
+
+        return $callLike->returnType instanceof Node;
+    }
 }
diff --git a/src/NodeDecorator/StatementDepthAttributeDecorator.php b/src/NodeDecorator/StatementDepthAttributeDecorator.php
@@ -0,0 +1,28 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\NodeDecorator;
+
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
+use Rector\NodeTypeResolver\Node\AttributeKey;
+
+final class StatementDepthAttributeDecorator
+{
+    /**
+     * @param ClassMethod[] $classMethods
+     */
+    public static function decorateClassMethods(array $classMethods): void
+    {
+        foreach ($classMethods as $classMethod) {
+            foreach ((array) $classMethod->stmts as $methodStmt) {
+                $methodStmt->setAttribute(AttributeKey::IS_FIRST_LEVEL_STATEMENT, true);
+
+                if ($methodStmt instanceof Expression) {
+                    $methodStmt->expr->setAttribute(AttributeKey::IS_FIRST_LEVEL_STATEMENT, true);
+                }
+            }
+        }
+    }
+}
diff --git a/src/NodeTypeResolver/Node/AttributeKey.php b/src/NodeTypeResolver/Node/AttributeKey.php
@@ -285,4 +285,9 @@ final class AttributeKey
      * @var string
      */
     public const ATTRIBUTE_COMMENT = 'attribute_comment';
+
+    /**
+     * @var string
+     */
+    public const IS_FIRST_LEVEL_STATEMENT = 'first_level_stmt';
 }