Page MenuHomePhabricator

MessageCache::getParserOptions should be private
Closed, ResolvedPublic

Description

The method[1] currently has no scope defined. It uses $wgUser, and will eventually need to have its signature changed to require a user parameter. Until then, it should be made private.

https://www.mediawiki.org/wiki/Manual:Coding_conventions/PHP#Visibility says

Mark new code with proper visibility modifiers, including public if appropriate, but do not add visibility to existing code without first checking, testing and refactoring as required. It's generally a good idea to avoid visibility changes unless you're making changes to the function which would break old uses of it anyway.

Looking through every known use of getParserOptions[2] shows no calls outside of the class.

Since the plan is to make breaking changes (require a user parameter), I suggest it be made private now to avoid the introduction of any uses that will then need to be changed (i.e. to ensure that it only remains used within the class)

[1] https://gerrit.wikimedia.org/g/mediawiki/core/+/a50432f78c6986389fb75e20fc9ce1edf1d37015/includes/cache/MessageCache.php#202
[2] https://codesearch.wmflabs.org/search/?q=getParserOptions&i=nope&files=&repos=

Event Timeline

DannyS712 triaged this task as Medium priority.Mar 8 2020, 7:58 AM
DannyS712 created this task.
DannyS712 moved this task from Unsorted to Next on the User-DannyS712 board.

Tagging CPT to review the proposal

What section of the release notes should this be? Breaking changes in 1.35 or Other changes in 1.35?

Change 579457 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/core@master] Make MessageCache::getParserOptions private

https://gerrit.wikimedia.org/r/579457

Put as a breaking change.

Change 579457 merged by jenkins-bot:
[mediawiki/core@master] Make MessageCache::getParserOptions private

https://gerrit.wikimedia.org/r/579457

DannyS712 closed this task as Resolved.Mar 13 2020, 5:33 AM