AddOverrideAttributeToOverriddenMethodsRector is not compatible with PHPStan's checkMissingOverrideMethodAttribute rule

[ruudk]

Bug Report

First of all, thanks for this amazing project ❤

For a while now I noticed that Rector did not add Override attributes anymore, while PHPStan complained.

Today I looked into it and it turns out it's related to this change:

• Fix interface implementer in AddOverrideAttributeToOverriddenMethodsRector, add override only in case of overiding parent method with contents rector-src#6447

It states the following:

The Override interface makes sense only overriden parent class method (or trait one), so we know it's change is on purpose. It doesn't make sense to add override for interface, as all interface methods would be marked :)

I don't agree with this. I think the reason for using the Override attribute is exactly this. To indicate that you implement/override a method coming from a parent (class or interface). Whenever that method is removed there, it should error on the implementation, indicating that the thing you thought you were doing no longer has any impact.

https://wiki.php.net/rfc/marking_overriden_methods

This rule conflicts with PHPStan's checkMissingOverrideMethodAttribute rule.

For example, I'm currently working on a Symfony Console Command:

<?php

declare(strict_types=1);

namespace Dev\DevTools\Command;

use Symfony\Component\Console\Attribute\AsCommand;
use Symfony\Component\Console\Command\Command;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

#[AsCommand('my-command')]
final class Mycommand extends Command
{
    protected function configure() : void
    {
        $this->addArgument('arg', InputArgument::REQUIRED);
    }

    public function execute(InputInterface $input, OutputInterface $output) : int
    {
        return self::SUCCESS;
    }
}

And AddOverrideAttributeToOverriddenMethodsRector does not add the #[Override] attributes to the configure and execute methods.

This produces the following PHPStan errors:

Method Dev\DevTools\Command\RemoveUnusedTranslationsCommand::configure() overrides method Symfony\Component\Console\Command\Command::configure() but is missing the #[\Override] attribute.

Method Dev\DevTools\Command\RemoveUnusedTranslationsCommand::execute() overrides method Symfony\Component\Console\Command\Command::execute() but is missing the #[\Override] attribute.

When I comment this out:
https://github.com/rectorphp/rector-src/blob/217026caf877c60eb7a7fd61b5b16d9a642f1662/rules/Php83/Rector/ClassMethod/AddOverrideAttributeToOverriddenMethodsRector.php#L225-L243

Rector does successfully add the Override attributes and the errors go away.

/cc @TomasVotruba @samsonasik

[TomasVotruba]

Thanks for the report 👍

We're looking into this and decided to make rule configurable, so it gives you the option to take more strict use, while keeping code clean for those who want to use it only for overriden behavior (non-empty methods).

[ruudk]

Thanks!

I think there is another issue with the Rector: it does not add #[Override] attributes when implementing a method from an interface, since it only looks for class parents (extends).

Can be fixed with:

-        $parentClassReflections = $classReflection->getParents();
+        $parentClassReflections = [...$classReflection->getParents(), ...$classReflection->getImmediateInterfaces()];

[samsonasik]

The original behavior handle that :), I need to recollect what removed/added to avoid regression, PRs to trace:

• Fix interface implementer in AddOverrideAttributeToOverriddenMethodsRector, add override only in case of overiding parent method with contents rector-src#6447
• [Php83] Skip add #[\Override] on non-empty method from trait on AddOverrideAttributeToOverriddenMethodsRector rector-src#6471
• [Php83] Allow indirect override abstract method on Grand Child on AddOverrideAttributeToOverriddenMethodsRector rector-src#6512
• [Php83] Remove get traits class as ancestors collection on AddOverrideAttributeToOverriddenMethodsRector rector-src#6513

[ruudk]

Also seems that __toString is not picked up when using Stringable.