Last modified: 2014-05-06 15:24:49 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 32607 - User::newFromName should return null instead of false on invalid input
User::newFromName should return null instead of false on invalid input
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
All All
: Lowest enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
Depends on:
Blocks: code_quality
  Show dependency treegraph
Reported: 2011-11-23 15:58 UTC by Antoine "hashar" Musso (WMF)
Modified: 2014-05-06 15:24 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Description Antoine "hashar" Musso (WMF) 2011-11-23 15:58:02 UTC
The User class propose several factory methods. Most of them will always return a User object but two of them would not:

 User::newFromName()  can return false

 User::newFromConfirmationCode() can return null

I would prefer having factories return null if no object have been made. This way we will avoid fatal errors when passing the result to a function explicitly expecting a User object. Example

function delete( User $user ) {
 // do something
delete( User::newFromName( '' ) );


That code will throw a catchable fatal error since the delete function expect either a User object or the null value.
Comment 1 Antoine "hashar" Musso (WMF) 2011-11-23 16:02:35 UTC
So basically, User::newFromName() should return null :-)
Comment 2 Brion Vibber 2011-11-23 16:46:00 UTC
Updated summary to say so :)
Comment 3 Daniel Friesen 2011-11-24 04:22:03 UTC
We'll need to do a quick scan for code that may use === false explicitly, but if we don't find any of that, then I see no problem with this change.
Comment 4 Alex Monk 2012-02-23 18:03:33 UTC
Lines 192 and 193 in maintenance/importImages.php:
	$wgUser = User::newFromName( $real_user );
	if ( $wgUser === false ) {
Comment 5 Antoine "hashar" Musso (WMF) 2014-05-06 15:24:49 UTC
This has too many possible back compatibility issues and there is no good way to deprecate the old values.  So lets keep the code dirty.  Maybe one day we will refactor the User class entirely. Meanwhile there is no point in keeping this bug around.

Note You need to log in before you can comment on or make changes to this bug.