Page MenuHomePhabricator

Add sniff to prevent using config globals when Config is available
Closed, ResolvedPublic

Description

Seen in several newbie patches. Instead of

global $wgMyConfigGlobal;

they should use

$this->getConfig()->get( 'MyConfigGlobal' );

Quite similiar to ExtendClassUsageSniff.

Details

Related Gerrit Patches:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 30 2018, 10:46 AM
MGChecker added a subscriber: MGChecker.EditedAug 30 2018, 9:00 PM

Is this a recommendation that is valid for all config globals in all places? Is there any documentation about this?

This could be tricky, because the sniff has to decide between config globals and objects/non-config globals. That means the sniff needs the way to set globals in .phpcs.xml as input for use in extensions. The core list can be found on T159283 and should be hard coded.

Is this a recommendation that is valid for all config globals in all places? Is there any documentation about this?

ExtendClassUsageSniff is only acting on classes extends ContextSource, because there it is better to call getConfig to get modern code.

Change 481653 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/tools/codesniffer@master] Expand ExtendClassUsageSniff to check for config globals

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

Umherirrender triaged this task as Normal priority.

Change 481653 merged by jenkins-bot:
[mediawiki/tools/codesniffer@master] Expand ExtendClassUsageSniff to check for config globals

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

Umherirrender closed this task as Resolved.Jan 30 2019, 8:24 PM