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

Trait methods with Override attribute with existing parent methods throw error after #14480 was fixed #14493

Closed
spaze opened this issue Jun 7, 2024 · 3 comments

Comments

@spaze
Copy link

spaze commented Jun 7, 2024

Description

I have a trait method that looks like this

trait Traita
{
    #[Override]
    public function methodWithDefaultImplementation(): int {}
}

which is used in a class

final class Childa extends Parenta
{
    use Traita;
}

where Parenta looks like

abstract class Parenta
{
    public function methodWithDefaultImplementation(): int {}
}

(all in the repo here https://github.com/spaze/php-override-test)

and this has started throwing errors () on the patched PHP 8.3.8 after the fix dstogov@955d717 for #14480 has been applied:

PHP Fatal error:  Childa::methodWithDefaultImplementation() has #[\Override] attribute, but no matching parent method exists

see https://github.com/spaze/php-override-test/actions/runs/9409995626/job/25920912091

I'm running tests with shivammathur/setup-php and they have patched 8.3.8 with the mentioned patch see shivammathur/setup-php#849 (comment)

When run with unpatched deb.sury.org package, there's no error (but the code from #14480 throws errors as expected without the patch).

PHP Version

8.3.8 + #14480.patch

Operating System

No response

@spaze
Copy link
Author

spaze commented Jun 7, 2024

This was caused by a improperly applied patch shivammathur/setup-php#849 (comment) closing this, sorry for the noise.

@spaze spaze closed this as not planned Won't fix, can't repro, duplicate, stale Jun 7, 2024
@dstogov
Copy link
Member

dstogov commented Jun 7, 2024

@spaze thanks for testing this.
I was afraid I might break something related, but I can't reproduce this failure.
The override.php works fine, without any errors (PHP-8.3 branch).
May be you applied the patch from PHP-8.2 to PHP-8.3? (patch for PHP-8.3 is a bit different).

@iluuu1994 could you please also verify the test

@spaze thank you in any case. It's better to double check the false alarm, than pass a new regression into a release.

@spaze
Copy link
Author

spaze commented Jun 7, 2024

@dstogov Thanks and sorry for the false alarm. I had the bug written for the shivammathur/setup-php repo and at the very last second decided to file it here. Again, sorry, my bad and lesson learned 😊

After @shivammathur has re-applied the fix my code started working again. Thank you both, nice work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants