Page MenuHomePhabricator

Where to place @deprecated on hook interfaces? On the interface or on the function?
Open, Needs TriagePublic


The EditPageBeforeEditToolbarHook interface was deprecated by adding @deprecated to the on-function [1].
The SkinTemplateTabActionHook interface was deprecated by adding @deprecated to the interface itself [2].

Different places could be shown up different in IDE and may overseen. Not sure if this is a issue or it is better to deprecate at both places?

Event Timeline

For IDEs and static analysis it doesn't matter much since the function is part of the interface, and it is the only function in that interface, so either way it should be inherited and understood for the common case.

I'd lean slightly toward putting it on the interface:

  • We put the @stable annotation on the interface as well (and it is required to be there by policy, not related to the method).
  • Putting it on the interface means there are more oppertunities in IDE and static analysis to warn against the hook. E.g. on the function, it would only apply to code invoking the hook (e.g. in core) and maybe in extension overrides where the method implementation is (if the analyzer inherits it by default, which it might not). But when on the interface, it will also apply directly in the file header where the interface is imported and "implemented" in the extensions' hook class definition.
  • The interface-level metadata is more prominent in API documentation. E.g. see mediawiki / SkinTemplateTabActionHook.

Having said that, these are all very minor things, which I'd say could be completely ignored if pretty much anything of value comes up in favour of the other way.

Agreed, the interface seems to make sense.

Of course, the place for hard-deprecation is but that's not an in-editor hint either.

Change 692380 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/core@master] Duplicate @deprecated annotation on interfaces

Change 692380 merged by jenkins-bot:

[mediawiki/core@master] Use @deprecated annotation on hook interfaces, not functions