Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MissingOverrideAttribute false positive with traits used by child classes #10982

Open
pilif opened this issue May 21, 2024 · 3 comments · May be fixed by #10989
Open

MissingOverrideAttribute false positive with traits used by child classes #10982

pilif opened this issue May 21, 2024 · 3 comments · May be fixed by #10989

Comments

@pilif
Copy link
Contributor

pilif commented May 21, 2024

https://psalm.dev/r/70ba038824

If you do what psalm says you're supposed to do and add the Override attribute to the trait method, you will create invalid code that PHP refuses to run (and psalm also flags as an error).

I think B should be totally free to import its own copy of the Foo trait.

Copy link

I found these snippets:

https://psalm.dev/r/70ba038824
<?php

trait Foo
{
    private function inTrait(): void { echo "foobar\n"; }
}

class A {
    use Foo;
    
    public function bar(): void {
        $this->inTrait();
    }        
}

class B extends A {    
    use Foo;

    function baz(): void
    {
        $this->inTrait();
    }
}

$b = new B();
$b->baz();
Psalm output (using commit 16b24bd):

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

@ghostwriter
Copy link

  • Private methods of a parent class do not satisfy #[\Override], because it's not part of the externally visible API.

@pilif
Copy link
Contributor Author

pilif commented May 23, 2024

indeed. Hence why psalm shouldn't ask me to add #[\Override] hence my bug report.

@ghostwriter ghostwriter linked a pull request May 24, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants