[#31247] - [RFC] Handling new method arguments in PHP 8
- Closed
- 25 Mar 2021
- Medium
- Build: staging
- # 31247
Problem identified
Before PHP 8, incompatible method signatures only produced a warning (PHP 7) or strict standards notice (PHP 5). We did not consider this an issue, as long as newly added arguments were optional. But in PHP 8 this produces a fatal error in child classes. We need a decision on how this should be handled going forward.
joomla-cms-bot
- | Labels |
Added:
?
?
|
||
| Title |
|
||||||
| Labels |
Added:
?
|
||
Do we actually have that in core? Should be fixed on all places.
Do we actually have that in core?
I am not sure, I tried to find a tool to check this but phpcs hasn't implemented this check yet.
Do we actually have that in core?
Agreed, I think it is a bad coding practice to change the method signature
Perhaps https://phpstan.org or https://psalm.dev can help here out?
My question is not about having the issue in our codebase (we do have one instance in FoF, see PR #30922) but about policy for handling this in the future. Something like #15826 would be a total B/C break in PHP 8 because any extending class overriding the method would break. So what would we do now if we needed to add more arguments? Introduce a new method?
Some clarification on what falls under B/C promise and what doesn't would be welcome. For 5.0 maybe we should consider making classes final where it makes sense.
As long as this kind of changes are done for a new major release, they should be fine.
I would make classes only final when they are not part of the public API. This is actually the case on many of them in libraries/src as we want to use interfaces wherever possible for example the factories. This requires a change in the BC policy that not all in libraries is API.
| Status | New | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-03-25 17:42:30 |
| Closed_By | ⇒ | SharkyKZ |
Can you add an example?
Incompatible method signatures would already be that optional parameters are missing?