Hi
On 6/5/24 10:16, Benjamin Außenhofer wrote:
Tim will finalize the implementation PR now and work on its merge in the
upcoming days.
During review Ilija pointed out that adding the internal
ZEND_ACC_DEPRECATED flag was skipped for abstract methods and interface
methods, questioning the decision. The decision dates back to Benjamin's
initial PoC implementation and I've had a short discussion with him
about this, with the result that it's probably best to ask the list, as
it was not specified in the RFC.
Currently adding the #[\Deprecated] attribute on an abstract method or
interface method does not do anything special. It only applies the
attribute, which then can be accessed via Reflection.
It would be easy to change the implementation to also apply the
ZEND_ACC_DEPRECATED flag. This would have the following consequences:
1. `ReflectionMethod::isDeprecated()` will return `true`.
2. No deprecation error will be emitted. Neither when implementing the
method in a child class, nor when calling the method of the child class.
(1) is probably the obviously correct behavior. For (2) the reasoning is
as follows:
- The implementation relies on the pre-existing behavior of
ZEND_ACC_DEPRECATED, which does not emit a deprecation error for
overridden methods / implemented abstract or interface methods.
- If the deprecation error would be emitted when implementing the
method, then the error would not be actionable. Child classes are still
forced to implement the abstract method / interface method, thus there
is no way to change the code to not emit the error.
- It is not obviously correct to inherit the deprecation when overriding
a non-abstract method of a parent class, just like attributes themselves
are not inherited [1]. Abstract methods and interface methods should
keep consistency with non-abstract methods. The reasoning why it's not
obviously correct to inherit the deprecation is that due to not
inheriting the Attribute, a custom deprecation message would not be applied.
It goes without saying that IDEs can opt to suggest adding the
#[\Deprecated] attribute if the overridden method is deprecated itself
by means of the existing code diagnosis functionality, making the
attribute on abstract methods / interface methods useful by that means.
We plan to adjust the implementation to also apply to abstract and
interface methods with the semantics outlined in (1) and (2). Does
anyone have an opinion about this / does anyone disagree with that decision?
Best regards
Tim Düsterhus
[1] https://3v4l.org/GIZRj
