Page MenuHomePhabricator

Add sniff to detect usage of deprecated $wgUser
Closed, DeclinedPublic

Description

Tim said we can phase out $wgUser in favor of RequestContext::getMain()->getUser()

https://www.facebook.com/brionv/posts/10151391376376852

We could use a PHP CodeSniffer sniff to report $wgUser should be replaced.

Details

Reference
bz48963

Event Timeline

bzimport raised the priority of this task from to Low.Nov 22 2014, 1:42 AM
bzimport added a project: MediaWiki-General.
bzimport set Reference to bz48963.
bzimport added a subscriber: Unknown Object (MLST).
hashar created this task.May 30 2013, 10:17 AM

Don't we already have a deprecation system in general? (both for methods and for globals). How is phpcs relevant?

With the proper configuration, accessing deprecated methods or globals, a PHP error is emitted.

hashar added a comment.Jun 1 2013, 8:31 PM

And what is the "proper" configuration to react whenever $wgUser is referenced / accessed ?

(In reply to comment #2)

And what is the "proper" configuration to react whenever $wgUser is
referenced / accessed ?

The proper solution is to assert that no php errors/notices are emitted during our running of the installer, php unit tests and http requests.

Marking as wontfix and suggest further discussion be at bug 48002.

Timo that is not covering everything. We want to get rid of $wgUser and the easiest is to detect whether it is used in the code, a sniff is a trivial thing to add in.

Related URL: https://gerrit.wikimedia.org/r/69090 (Gerrit Change Ic3d5f082afefe761a24ab0822859a9ef88c61e02)

Created attachment 12564
FB preview

(In reply to comment #0)

https://www.facebook.com/brionv/posts/10151391376376852

Attaching for posterity's sake.

Attached:

Umherirrender notified on the change:

There is a DeprecatedGlobals.php to get deprecation warnings on a global (used for $wgArticle), but $wgUser is not deprecated at the moment, so this is only for a hint to the developer and should not be a blocking sniff.

So I guess that needs to be done directly in core instead of phpcs as Timo pointed out. Moving bugs under MediaWiki product.

Change 69090 abandoned by Hashar:
Implement DeprecatedGlobalSniff for $wgUser

Reason:
I do not knew about DeprecatedGlobals.php seems it would get the job done. I have moved the bug report https://bugzilla.wikimedia.org/show_bug.cgi?id=48963 under MediaWiki product.

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

hashar added a comment.May 6 2014, 3:50 PM

There is no patch pending. Reseting bug status.

What is this DeprecatedGlobals.php file?

The following patch is probably rather relevant.
https://gerrit.wikimedia.org/r/#/c/153399/

hashar removed a subscriber: hashar.Mar 14 2015, 10:12 PM
Krinkle renamed this task from mark $wgUser as deprecated using DeprecatedGlobals.php to Detect usage of deprecated $wgUser in phpcs.Apr 1 2015, 4:02 AM
Krinkle updated the task description. (Show Details)
Krinkle edited projects, added MediaWiki-Codesniffer; removed MediaWiki-General.
Krinkle set Security to None.
Krinkle removed subscribers: Krinkle, Unknown Object (MLST).
polybuildr renamed this task from Detect usage of deprecated $wgUser in phpcs to Add sniff to detect usage of deprecated $wgUser.Jun 3 2015, 9:16 PM
polybuildr moved this task from Untriaged to Accepted rule changes on the MediaWiki-Codesniffer board.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 11 2015, 8:57 AM

I am interested in working on this. Should the PHP CodeSniffer sniff for $wgUser and warn to use RequestContext::getMain()->getUser() instead? Also, should it be automatically replaceable using phpcbf?

Aashaka added a subscriber: hashar.Mar 15 2016, 3:11 AM

@Addshore, I think that @hashar's patch with changes to the description and a few other changes is what I had in mind. May I amend @hashar's patch, or submit a new one? I am relatively new here, and would love some guidance.

@Aashaka my patch on https://gerrit.wikimedia.org/r/#/c/69090/ is the wrong approach. Reviewer hinted at DeprecatedGlobals.php.

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptJul 13 2016, 6:25 PM

We need the wgUser config for static functions as you can't do $this on static functions.

Reedy added a subscriber: Reedy.Jan 5 2017, 11:04 PM

We need the wgUser config for static functions as you can't do $this on static functions.

You can use RequestContext::getMain()->getUser() directly if it's a static function

Oh, thanks.

So, what's the status here? Adding wgUser is trivial, do we want to do it?

hashar closed this task as Declined.Apr 30 2019, 1:11 AM

I guess that will be achieved by actually deprecating $wgUser: T159299