False positive #[Override] required for __toString() about psalm HOT 7 CLOSED

I think you could argue this either way. Adding the #[Override] attribute is valid in that it doesn't cause a PHP runtime error.

I think the behavior I'd shoot for is:

• If Test explicitly implements Stringable, then MissingOverrideAttribute is triggered.
• If Test implicitly implements Stringable, as in the reproducer, then no MissingOverrideAttribute is triggered.

Sound reasonable?

from psalm.

PS: kia ora 🥝 :-D

from psalm.

I assume you also mean transitively-extended types? eg

abstract class A implements Stringable
{
}

class B extends A
{
    public function __toString(): string
    {
        return '';
    }
}

I think we should still issue MissingOverrideAttribute in this case, yes?

If so, then I think Stringable is currently the only case in the language where an interface can be implemented implicitly.

from psalm.

It's not reproducible at psalm-dev,

You just need to set one of the checkboxes on the settings screen: https://psalm.dev/r/07b8ed2a92

from psalm.

I don't think Stringable deserves any special treatment at all, unless the language requires it. All exceptions is a burden to maintain by the core developers team.

My suggestion: ONLY mark a method as MissingOverrideAttribute IF AND ONLY IF there is an EXPLICITLY extended type that declares the method with the same name.

from psalm.

I assume you also mean transitively-extended types? eg

Yep, as long as a "is a" relationship established via any native type system mechanism (be it implements, extends, directly or transitively).

I think we should still issue MissingOverrideAttribute in this case, yes?

Totally

from psalm.

I found these snippets:

<?php
declare(strict_types=1);

namespace Zerkms;

final readonly class Test
{
    public function __toString(): string
    {
        return '';
    }
}

Psalm output (using commit ef3b018):

ERROR: MissingOverrideAttribute - 8:5 - Method __toString should have the "Override" attribute

from psalm.