Page MenuHomePhabricator

Remove use of `@access`, `@protected` and `@private` on public methods in MediaWiki
Open, LowPublic

Description

Author: ezyang

Description:
I'm not sure how widespread the problem is, but certain classes misuse the
docblock's @access declaration. That is, if the access restrictions were to
translate straight to actual language level declarations in PHP5, you'd break a
lot of functionality. For now, I just know that HTMLForm.php couldn't possibly
be a class of only private methods (and nothing else). That would be quite
useless. I think they're supposed to be protected, but public would be even nicer.

A related bug would also ask for certain interfaces that were originally private
to be made public for extensions to use.

Assigned as blocker to Brion's "Our docs are teh suck" bug and added to the code
quality tracking bug.


Code search: https://codesearch.wmflabs.org/core/?q=%40(access%7Cprotected%7Cprivate)&i=nope&files=&repos=

Details

Reference
bz5176

Event Timeline

bzimport raised the priority of this task from to Low.Nov 21 2014, 9:09 PM
bzimport set Reference to bz5176.
bzimport added a subscriber: Unknown Object (MLST).

robchur wrote:

*** Bug 8000 has been marked as a duplicate of this bug. ***

Krinkle set Security to None.
Krinkle removed a subscriber: Unknown Object (MLST).
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 23 2015, 10:06 AM

This appears to be more of a general development practice/pattern request. Probably could address this concern (and possibly already is) as part of coding standards. We can then decide if refactoring to meet those standards is warranted through various ares of the codebase.

Krinkle renamed this task from Make access declarations private protected and public really mean it to Remove use of `@access`, `@protected` and `@private` on public methods in MediaWiki.Jun 15 2018, 3:43 PM
Krinkle updated the task description. (Show Details)

Agreed on deprecating @access. There are a few places where things are supposed to be private/protected, but we've made them public for whatever reason, but annotated the intent with @private. As long as that continues to be an exceptional case, I think allowing those tags is alright.

Squiz.Scope.MethodScope.Missing checks for an explicit visibility on methods.