Page MenuHomePhabricator

Clarify deprecation of method overrides in the stable interface policy
Closed, ResolvedPublicSpike

Description

The SIP has detailed instructions for deprecating calls to a method, but no guidance regarding deprecating method overrides. This task aims to fill that gap.
Prior art: T193613: RFC: Establish stable interface policy for PHP code, T255803: RFC: Amendment to the Stable interface policy, June 2020.

The issue

Deprecating method overrides is different from method calls. The dependency is inverted:

ownercaller
method deprecationthis classother code
override deprecationother (subclass)this (superclass)

*this: deprecating class

In the case of normal method deprecation the method is defined in the deprecating class and the caller can be any code.
In the case of method override deprecation the method is defined by an unknown subclass and the caller of the (virtual) method is usually the deprecating (super)class (but can be any code).

Solution

T267080: Helper for deprecating method overrides (Patch 638202)

Method override deprecation requires a different approach:

  • The inherited method is not necessarily called, thus the deprecation code cannot be in the method.
  • An overridden method is not required to alter the program state differently from the inherited method, therefore whether a method is overridden cannot be determined with standard program flow.
  • Reflection (metaprogramming) is used to determine the presence of an override.

Motivating example

T266735: Fix or archive skin using deprecated Skin::setupSkinUserCss() method (Patch 638203)

Documentation

This ticket is about formalizing the rules of deprecating method overrides in the SIP.

  • The inherited method might be called by the override, therefore it cannot be removed.
  • The inherited method is not necessarily called, thus the deprecation code cannot be in the method.
  • The deprecation code should be in an initialization function: constructors might be run before the page headers are sent, generating the deprecation warning too early.

The deprecation code is (details in patch 638202):

use DeprecateOverridesHelper;
...
$this->deprecateMethodOverride( 'deprecatedMethod', '1.36', __CLASS__ );

Question

In what form should this be presented in the SIP?

Event Timeline

Demian triaged this task as Medium priority.Nov 3 2020, 4:41 AM
Demian moved this task from Incoming to In progress on the User-Demian board.
Demian changed the subtype of this task from "Task" to "Spike".

[Resetting assignee due to inactive user account]

Thank you for the suggestion. I propose to add the following wording to the policy:

When hard deprecating code that is stable to override,

  • a deprecation warning MUST be triggered in case the method is overridden by a subclass.
  • the method MUST still be called if it is overridden.

See the draft at https://www.mediawiki.org/wiki/User:DKinzler_(WMF)/Stable_interface_policy#Stable_to_override

@daniel since this is now without an owner, I propose you decide on the wording yourself and close it out.

This will be resolved when T268326 is adopted.

daniel claimed this task.