Skip void without final keyword in ReturnNeverTypeRector as could be …

…implemented with more precise children (#5379)

* Skip void without final keyword in ReturnNeverTypeRector as could be implemented with more precise children

* simplify

* tidy up

* tidy up

* [ci-review] Rector Rectify

---------

Co-authored-by: GitHub Action <actions@github.com>

rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector/Fixture/has_children_return_never.php.inc

@@ -34,4 +34,4 @@ class HasChildrenReturnNeverChild extends HasChildrenReturnNever {
     }
 }
 
-?>
+?>

rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector/Fixture/require_existing_final_void.php.inc

@@ -0,0 +1,31 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnNeverTypeRector\Fixture;
+
+use Rector\Core\Exception\ShouldNotHappenException;
+
+final class RequireExistingFinalVoid
+{
+    public function run(): void
+    {
+        throw new ShouldNotHappenException('implement by child');
+    }
+}
+
+?>
+-----
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnNeverTypeRector\Fixture;
+
+use Rector\Core\Exception\ShouldNotHappenException;
+
+final class RequireExistingFinalVoid
+{
+    public function run(): never
+    {
+        throw new ShouldNotHappenException('implement by child');
+    }
+}
+
+?>

rules-tests/TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector/Fixture/skip_existing_void.php.inc

@@ -0,0 +1,13 @@
+<?php
+
+namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnNeverTypeRector\Fixture;
+
+use Rector\Core\Exception\ShouldNotHappenException;
+
+class SkipExistingVoid
+{
+    public function run(): void
+    {
+        throw new ShouldNotHappenException('implement by child');
+    }
+}

rules/CodeQuality/Rector/Expression/TernaryFalseExpressionToIfRector.php

@@ -77,7 +77,10 @@ public function refactorWithScope(Node $node, Scope $scope): ?Node
             return null;
         }
 
-        if ($this->sideEffectNodeDetector->detect($ternary->else, $scope) || $this->sideEffectNodeDetector->detectCallExpr($ternary->else, $scope)) {
+        if ($this->sideEffectNodeDetector->detect(
+            $ternary->else,
+            $scope
+        ) || $this->sideEffectNodeDetector->detectCallExpr($ternary->else, $scope)) {
             return null;
         }
 

rules/Php72/NodeFactory/AnonymousFunctionFactory.php

@@ -257,7 +257,8 @@ private function createParams(PhpMethodReflection $phpMethodReflection, array $p
             $variable = new Variable($parameterReflection->getName());
             $defaultExpr = $this->resolveParamDefaultExpr($parameterReflection, $key, $classMethod);
             $type = $this->resolveParamType($parameterReflection);
-            $byRef = $parameterReflection->passedByReference()->yes();
+            $byRef = $parameterReflection->passedByReference()
+                ->yes();
 
             $params[] = new Param($variable, $defaultExpr, $type, $byRef);
         }

rules/TypeDeclaration/NodeAnalyzer/NeverFuncCallAnalyzer.php

@@ -0,0 +1,43 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\TypeDeclaration\NodeAnalyzer;
+
+use PhpParser\Node\Expr\Closure;
+use PhpParser\Node\Stmt;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Expression;
+use PhpParser\Node\Stmt\Function_;
+use PHPStan\Type\NeverType;
+use Rector\NodeTypeResolver\NodeTypeResolver;
+
+final class NeverFuncCallAnalyzer
+{
+    public function __construct(
+        private readonly NodeTypeResolver $nodeTypeResolver,
+    ) {
+    }
+
+    public function hasNeverFuncCall(ClassMethod | Closure | Function_ $functionLike): bool
+    {
+        $hasNeverType = false;
+
+        foreach ((array) $functionLike->stmts as $stmt) {
+            if ($stmt instanceof Expression) {
+                $stmt = $stmt->expr;
+            }
+
+            if ($stmt instanceof Stmt) {
+                continue;
+            }
+
+            $stmtType = $this->nodeTypeResolver->getNativeType($stmt);
+            if ($stmtType instanceof NeverType) {
+                $hasNeverType = true;
+            }
+        }
+
+        return $hasNeverType;
+    }
+}

rules/TypeDeclaration/Rector/ClassMethod/ReturnNeverTypeRector.php

@@ -8,18 +8,17 @@
 use PhpParser\Node\Expr\Closure;
 use PhpParser\Node\Expr\Yield_;
 use PhpParser\Node\Identifier;
-use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\ClassMethod;
-use PhpParser\Node\Stmt\Expression;
 use PhpParser\Node\Stmt\Function_;
 use PhpParser\Node\Stmt\Return_;
 use PhpParser\Node\Stmt\Throw_;
 use PHPStan\Analyser\Scope;
-use PHPStan\Type\NeverType;
+use PHPStan\Reflection\ClassReflection;
 use Rector\Core\PhpParser\Node\BetterNodeFinder;
 use Rector\Core\Rector\AbstractScopeAwareRector;
 use Rector\Core\ValueObject\PhpVersionFeature;
 use Rector\NodeNestingScope\ValueObject\ControlStructure;
+use Rector\TypeDeclaration\NodeAnalyzer\NeverFuncCallAnalyzer;
 use Rector\VendorLocker\NodeVendorLocker\ClassMethodReturnTypeOverrideGuard;
 use Rector\VersionBonding\Contract\MinPhpVersionInterface;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -35,6 +34,7 @@ final class ReturnNeverTypeRector extends AbstractScopeAwareRector implements Mi
     public function __construct(
         private readonly ClassMethodReturnTypeOverrideGuard $classMethodReturnTypeOverrideGuard,
         private readonly BetterNodeFinder $betterNodeFinder,
+        private readonly NeverFuncCallAnalyzer $neverFuncCallAnalyzer,
     ) {
     }
 
@@ -99,24 +99,11 @@ private function shouldSkip(ClassMethod | Function_ | Closure $node, Scope $scop
             return true;
         }
 
-        $hasReturn = $this->betterNodeFinder->hasInstancesOfInFunctionLikeScoped($node, Return_::class);
-        if ($hasReturn) {
+        if ($this->hasReturnOrYields($node)) {
             return true;
         }
 
-        $hasNotNeverNodes = $this->betterNodeFinder->hasInstancesOfInFunctionLikeScoped(
-            $node,
-            [Yield_::class, ...ControlStructure::CONDITIONAL_NODE_SCOPE_TYPES]
-        );
-
-        if ($hasNotNeverNodes) {
-            return true;
-        }
-
-        $hasNeverNodes = $this->betterNodeFinder->hasInstancesOfInFunctionLikeScoped($node, [Throw_::class]);
-        $hasNeverFuncCall = $this->hasNeverFuncCall($node);
-
-        if (! $hasNeverNodes && ! $hasNeverFuncCall) {
+        if (! $this->hasNeverNodesOrNeverFuncCalls($node)) {
             return true;
         }
 
@@ -131,28 +118,34 @@ private function shouldSkip(ClassMethod | Function_ | Closure $node, Scope $scop
             return false;
         }
 
+        // skip as most likely intentional
+        $classReflection = $scope->getClassReflection();
+        if ($classReflection instanceof ClassReflection && ! $classReflection->isFinalByKeyword() && $this->isName($node->returnType, 'void')) {
+            return true;
+        }
+
         return $this->isName($node->returnType, 'never');
     }
 
-    private function hasNeverFuncCall(ClassMethod | Closure | Function_ $functionLike): bool
+    private function hasReturnOrYields(ClassMethod|Function_|Closure $node): bool
     {
-        $hasNeverType = false;
-
-        foreach ((array) $functionLike->stmts as $stmt) {
-            if ($stmt instanceof Expression) {
-                $stmt = $stmt->expr;
-            }
+        if ($this->betterNodeFinder->hasInstancesOfInFunctionLikeScoped($node, Return_::class)) {
+            return true;
+        }
 
-            if ($stmt instanceof Stmt) {
-                continue;
-            }
+        return $this->betterNodeFinder->hasInstancesOfInFunctionLikeScoped(
+            $node,
+            [Yield_::class, ...ControlStructure::CONDITIONAL_NODE_SCOPE_TYPES]
+        );
+    }
 
-            $stmtType = $this->nodeTypeResolver->getNativeType($stmt);
-            if ($stmtType instanceof NeverType) {
-                $hasNeverType = true;
-            }
+    private function hasNeverNodesOrNeverFuncCalls(ClassMethod|Function_|Closure $node): bool
+    {
+        $hasNeverNodes = $this->betterNodeFinder->hasInstancesOfInFunctionLikeScoped($node, [Throw_::class]);
+        if ($hasNeverNodes) {
+            return true;
         }
 
-        return $hasNeverType;
+        return $this->neverFuncCallAnalyzer->hasNeverFuncCall($node);
     }
 }

src/PHPStan/NodeVisitor/UnreachableStatementNodeVisitor.php

@@ -48,7 +48,7 @@ public function enterNode(Node $node): ?Node
                 continue;
             }
 
-            if ($stmt->getAttribute(AttributeKey::IS_UNREACHABLE) === true)  {
+            if ($stmt->getAttribute(AttributeKey::IS_UNREACHABLE) === true) {
                 $isPassedUnreachableStmt = true;
                 continue;
             }