Fix interface implementer in AddOverrideAttributeToOverriddenMethodsR…

…ector, add override only in case of overiding parent method with contents (#6447)

rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/Fixture/apply_attribute_to_override_method_from_trait.php.inc

@@ -8,10 +8,6 @@ class ApplyAttributeToOverrideMethodFromTrait
 {
     use ExampleFromTrait;
 
-    public function foo()
-    {
-    }
-
     public function bar()
     {
     }
@@ -30,10 +26,6 @@ class ApplyAttributeToOverrideMethodFromTrait
     use ExampleFromTrait;
 
     #[\Override]
-    public function foo()
-    {
-    }
-
     public function bar()
     {
     }

rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/Fixture/skip_abstract_trait_method.php.inc

@@ -0,0 +1,14 @@
+<?php
+
+namespace Rector\Tests\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector\Fixture;
+
+use Rector\Tests\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector\Source\ExampleFromTrait;
+
+final class SkipAbstractTraitMethod
+{
+    use ExampleFromTrait;
+
+    public function foo()
+    {
+    }
+}

rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/Fixture/skip_execute_override.php.inc

@@ -0,0 +1,17 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector\Fixture;
+
+use Symfony\Component\Console\Command\Command;
+use Symfony\Component\Console\Input\InputInterface;
+use Symfony\Component\Console\Output\OutputInterface;
+
+final class SkipExecuteOverride extends Command
+{
+    protected function execute(InputInterface $input, OutputInterface $output)
+    {
+        $result = 100;
+    }
+}

rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/Fixture/skip_on_interface.php.inc

@@ -0,0 +1,11 @@
+<?php
+
+namespace Rector\Tests\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector\Fixture;
+
+final class SkipOnInterface implements \Stringable
+{
+    public function __toString(): string
+    {
+        return 'implements only';
+    }
+}

rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/Fixture/skip_on_parent_return_null.php.inc

@@ -0,0 +1,16 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector\Fixture;
+
+use PhpParser\Node;
+use PhpParser\NodeVisitorAbstract;
+
+final class SkipOnParentReturnNull extends NodeVisitorAbstract
+{
+    public function enterNode(Node $node)
+    {
+        return 1;
+    }
+}

rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/Fixture/skip_setup_override.php.inc

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Rector\Tests\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector\Fixture;
+
+use PHPUnit\Framework\TestCase;
+
+final class SkipSetupOverride extends TestCase
+{
+    protected function setUp(): void
+    {
+        $result = 1000;
+    }
+}

rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/Source/ExampleFromTrait.php

@@ -6,7 +6,8 @@ trait ExampleFromTrait
 {
     public abstract function foo();
 
-    public function bar() {
-
+    public function bar()
+    {
+        $value = 'non empty';
     }
 }

rules-tests/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector/Source/ExampleParentClass.php

@@ -6,9 +6,11 @@ class ExampleParentClass
 {
     public function foo()
     {
+        $value = 'non empty';
     }
 
     private function bar()
     {
+        $value = 'non empty';
     }
 }

rules/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector.php

@@ -7,30 +7,47 @@
 use PhpParser\Node;
 use PhpParser\Node\Attribute;
 use PhpParser\Node\AttributeGroup;
+use PhpParser\Node\Expr;
 use PhpParser\Node\Name\FullyQualified;
+use PhpParser\Node\Stmt;
 use PhpParser\Node\Stmt\Class_;
+use PhpParser\Node\Stmt\ClassLike;
+use PhpParser\Node\Stmt\ClassMethod;
+use PhpParser\Node\Stmt\Return_;
+use PhpParser\Node\Stmt\Throw_;
 use PHPStan\Reflection\ClassReflection;
 use PHPStan\Reflection\ReflectionProvider;
 use Rector\NodeAnalyzer\ClassAnalyzer;
 use Rector\Php80\NodeAnalyzer\PhpAttributeAnalyzer;
+use Rector\PhpParser\AstResolver;
+use Rector\PhpParser\Node\Value\ValueResolver;
 use Rector\Rector\AbstractRector;
+use Rector\ValueObject\MethodName;
 use Rector\ValueObject\PhpVersionFeature;
 use Rector\VersionBonding\Contract\MinPhpVersionInterface;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
 use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
 
 /**
  * @see https://wiki.php.net/rfc/marking_overriden_methods
+ *
  * @see \Rector\Tests\Php83\Rector\ClassMethod\AddOverrideAttributeToOverriddenMethodsRector\AddOverrideAttributeToOverriddenMethodsRectorTest
  */
 final class AddOverrideAttributeToOverriddenMethodsRector extends AbstractRector implements MinPhpVersionInterface
 {
+    /**
+     * @var string
+     */
+    private const OVERRIDE_CLASS = 'Override';
+
     private bool $hasChanged = false;
 
     public function __construct(
         private readonly ReflectionProvider $reflectionProvider,
         private readonly ClassAnalyzer $classAnalyzer,
         private readonly PhpAttributeAnalyzer $phpAttributeAnalyzer,
+        private readonly AstResolver $astResolver,
+        private readonly ValueResolver $valueResolver,
     ) {
     }
 
@@ -48,7 +65,7 @@ public function foo()
     }
 }
 
-class ChildClass extends ParentClass
+final class ChildClass extends ParentClass
 {
     public function foo()
     {
@@ -64,7 +81,7 @@ public function foo()
     }
 }
 
-class ChildClass extends ParentClass
+final class ChildClass extends ParentClass
 {
     #[\Override]
     public function foo()
@@ -102,13 +119,15 @@ public function refactor(Node $node): ?Node
         }
 
         $classReflection = $this->reflectionProvider->getClass($className);
-        $parentClassReflections = [
-            ...$classReflection->getParents(),
-            ...$classReflection->getInterfaces(),
-            ...$classReflection->getTraits(),
-        ];
 
-        $this->processAddOverrideAttribute($node, $parentClassReflections);
+        $parentClassReflections = array_merge($classReflection->getParents(), $classReflection->getTraits());
+        if ($parentClassReflections === []) {
+            return null;
+        }
+
+        foreach ($node->getMethods() as $classMethod) {
+            $this->processAddOverrideAttribute($classMethod, $parentClassReflections);
+        }
 
         if (! $this->hasChanged) {
             return null;
@@ -125,47 +144,92 @@ public function provideMinPhpVersion(): int
     /**
      * @param ClassReflection[] $parentClassReflections
      */
-    private function processAddOverrideAttribute(Class_ $class, array $parentClassReflections): void
+    private function processAddOverrideAttribute(ClassMethod $classMethod, array $parentClassReflections): void
     {
-        if ($parentClassReflections === []) {
+        if ($this->shouldSkipClassMethod($classMethod)) {
             return;
         }
 
-        foreach ($class->getMethods() as $classMethod) {
-            if ($classMethod->name->toString() === '__construct') {
+        /** @var string $classMethodName */
+        $classMethodName = $this->getName($classMethod->name);
+
+        // Private methods should be ignored
+        foreach ($parentClassReflections as $parentClassReflection) {
+            if (! $parentClassReflection->hasNativeMethod($classMethod->name->toString())) {
+                continue;
+            }
+
+            // ignore if it is a private method on the parent
+            if (! $parentClassReflection->hasNativeMethod($classMethodName)) {
                 continue;
             }
 
-            if ($classMethod->isPrivate()) {
+            $parentMethod = $parentClassReflection->getNativeMethod($classMethodName);
+            if ($parentMethod->isPrivate()) {
                 continue;
             }
 
-            // ignore if it already uses the attribute
-            if ($this->phpAttributeAnalyzer->hasPhpAttribute($classMethod, 'Override')) {
+            if ($this->shouldSkipParentClassMethod($parentClassReflection, $classMethod)) {
                 continue;
             }
 
-            // Private methods should be ignored
-            foreach ($parentClassReflections as $parentClassReflection) {
-                if (! $parentClassReflection->hasNativeMethod($classMethod->name->toString())) {
-                    continue;
-                }
+            $classMethod->attrGroups[] = new AttributeGroup([new Attribute(new FullyQualified(self::OVERRIDE_CLASS))]);
+            $this->hasChanged = true;
+            return;
+        }
+    }
+
+    private function shouldSkipClassMethod(ClassMethod $classMethod): bool
+    {
+        if ($this->isName($classMethod->name, MethodName::CONSTRUCT)) {
+            return true;
+        }
+
+        if ($classMethod->isPrivate()) {
+            return true;
+        }
+
+        // ignore if it already uses the attribute
+        return $this->phpAttributeAnalyzer->hasPhpAttribute($classMethod, self::OVERRIDE_CLASS);
+    }
+
+    private function shouldSkipParentClassMethod(ClassReflection $parentClassReflection, ClassMethod $classMethod): bool
+    {
+        // parse parent method, if it has some contents or not
+        $parentClass = $this->astResolver->resolveClassFromClassReflection($parentClassReflection);
+        if (! $parentClass instanceof ClassLike) {
+            return true;
+        }
+
+        $parentClassMethod = $parentClass->getMethod($classMethod->name->toString());
+        if (! $parentClassMethod instanceof ClassMethod) {
+            return true;
+        }
 
-                // ignore if it is a private method on the parent
-                $parentMethod = $parentClassReflection->getNativeMethod($classMethod->name->toString());
-                if ($parentMethod->isPrivate()) {
-                    continue;
-                }
+        if ($parentClassMethod->isAbstract()) {
+            return true;
+        }
 
-                if ($parentClassReflection->isTrait() && ! $parentMethod->isAbstract()) {
-                    continue;
-                }
+        // has any stmts?
+        if ($parentClassMethod->stmts === null || $parentClassMethod->stmts === []) {
+            return true;
+        }
 
-                $classMethod->attrGroups[] = new AttributeGroup([new Attribute(new FullyQualified('Override'))]);
-                $this->hasChanged = true;
+        if (count($parentClassMethod->stmts) === 1) {
+            /** @var Stmt $soleStmt */
+            $soleStmt = $parentClassMethod->stmts[0];
+            // most likely, return null; is interface to be designed to override
+            if ($soleStmt instanceof Return_ && $soleStmt->expr instanceof Expr && $this->valueResolver->isNull(
+                $soleStmt->expr
+            )) {
+                return true;
+            }
 
-                continue 2;
+            if ($soleStmt instanceof Throw_) {
+                return true;
             }
         }
+
+        return false;
     }
 }