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

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T34607, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 32607 - User::newFromName should return null instead of false on invalid input
User::newFromName should return null instead of false on invalid input
Status: RESOLVED WONTFIX
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.20.x
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: ---


Attachments

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

<?php
function delete( User $user ) {
 // do something
}
delete( User::newFromName( '127.0.0.1' ) );

?>

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.


Navigation
Links