r60744 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r60743‎ | r60744 | r60745 >
Date:19:59, 6 January 2010
Author:reedy
Status:ok (Comments)
Tags:
Comment:
Attempt at normalistion of comparison styles - empty string on left and right hand side normalised to it being on the rhs

Before this change, there were (? being regex 0 or 1)

"" ===? 1
'' ===? 24
"" !==? 8
'' !==? 32

== "" 14
== '' 344
!= "" 9
!== "" 4
!= '' 151
!== '' 85


Rhs was the much more common, and the preferred style by many developers.. (Was a similar discussion in #mediawiki recently.. After that lolbugreport i think)

Where there is a string (non empty) on the lhs, and variable/method call on the rhs still need normalising
Modified paths:
  • /trunk/phase3/includes/ChangesList.php (modified) (history)
  • /trunk/phase3/includes/EditPage.php (modified) (history)
  • /trunk/phase3/includes/Export.php (modified) (history)
  • /trunk/phase3/includes/GlobalFunctions.php (modified) (history)
  • /trunk/phase3/includes/Linker.php (modified) (history)
  • /trunk/phase3/includes/LogPage.php (modified) (history)
  • /trunk/phase3/includes/OutputPage.php (modified) (history)
  • /trunk/phase3/includes/ProtectionForm.php (modified) (history)
  • /trunk/phase3/includes/Skin.php (modified) (history)
  • /trunk/phase3/includes/SkinTemplate.php (modified) (history)
  • /trunk/phase3/includes/Title.php (modified) (history)
  • /trunk/phase3/includes/Wiki.php (modified) (history)
  • /trunk/phase3/includes/db/Database.php (modified) (history)
  • /trunk/phase3/includes/db/DatabaseOracle.php (modified) (history)
  • /trunk/phase3/includes/parser/Parser.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialEmailuser.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialIpblocklist.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialLockdb.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialProtectedpages.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialProtectedtitles.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialSearch.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUnlockdb.php (modified) (history)
  • /trunk/phase3/includes/specials/SpecialUserlogin.php (modified) (history)
  • /trunk/phase3/skins/CologneBlue.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/Skin.php
——@@ -1127,7 +1127,7 @@
11281128 global $wgOut;
11291129
11301130 $sub = $wgOut->getSubtitle();
1131 - if ( '' == $sub ) {
 1131+ if ( $sub == '' ) {
11321132 global $wgExtraSubtitle;
11331133 $sub = wfMsgExt( 'tagline', 'parsemag' ) . $wgExtraSubtitle;
11341134 }
——@@ -1530,7 +1530,7 @@
15311531 }
15321532
15331533 function logoText( $align = '' ) {
1534 - if ( '' != $align ) {
 1534+ if ( $align != '' ) {
15351535 $a = " align='{$align}'";
15361536 } else {
15371537 $a = '';
——@@ -1839,7 +1839,7 @@
18401840 $url = $nt->escapeFullURL();
18411841 $text = $wgContLang->getLanguageName( $nt->getInterwiki() );
18421842
1843 - if ( '' == $text ) { $text = $l; }
 1843+ if ( $text == '' ) { $text = $l; }
18441844 $style = $this->getExternalLinkAttributes();
18451845 $s .= "<a href=\"{$url}\"{$style}>{$text}</a>";
18461846 }
Index: trunk/phase3/includes/ChangesList.php
——@@ -262,7 +262,7 @@
263263 # Make date header if necessary
264264 $date = $wgLang->date( $rc_timestamp, true, true );
265265 if( $date != $this->lastdate ) {
266 - if( '' != $this->lastdate ) {
 266+ if( $this->lastdate != '' ) {
267267 $s .= "</ul>\n";
268268 }
269269 $s .= Xml::element( 'h4', null, $date ) . "\n<ul class=\"special\">";
Index: trunk/phase3/includes/ProtectionForm.php
——@@ -191,7 +191,7 @@
192192
193193 list( $cascadeSources, /* $restrictions */ ) = $this->mTitle->getCascadeProtectionSources();
194194
195 - if ( "" != $err ) {
 195+ if ( $err != "" ) {
196196 $wgOut->setSubtitle( wfMsgHtml( 'formerror' ) );
197197 $wgOut->addHTML( "<p class='error'>{$err}</p>\n" );
198198 }
Index: trunk/phase3/includes/Title.php
——@@ -542,7 +542,7 @@
543543 public function getNsText() {
544544 global $wgContLang;
545545
546 - if ( '' != $this->mInterwiki ) {
 546+ if ( $this->mInterwiki != '' ) {
547547 // This probably shouldn't even happen. ohh man, oh yuck.
548548 // But for interwiki transclusion it sometimes does.
549549 // Shit. Shit shit shit.
——@@ -654,7 +654,7 @@
655655 */
656656 public function getFullText() {
657657 $text = $this->getPrefixedText();
658 - if( '' != $this->mFragment ) {
 658+ if( $this->mFragment != '' ) {
659659 $text .= '#' . $this->mFragment;
660660 }
661661 return $text;
——@@ -738,7 +738,7 @@
739739 $baseUrl = $interwiki->getURL( );
740740
741741 $namespace = wfUrlencode( $this->getNsText() );
742 - if ( '' != $namespace ) {
 742+ if ( $namespace != '' ) {
743743 # Can this actually happen? Interwikis shouldn't be parsed.
744744 # Yes! It can in interwiki transclusion. But... it probably shouldn't.
745745 $namespace .= ':';
——@@ -910,7 +910,7 @@
911911 * interwiki link
912912 */
913913 public function getEditURL() {
914 - if ( '' != $this->mInterwiki ) { return ''; }
 914+ if ( $this->mInterwiki != '' ) { return ''; }
915915 $s = $this->getLocalURL( 'action=edit' );
916916
917917 return $s;
——@@ -929,7 +929,7 @@
930930 * Is this Title interwiki?
931931 * @return \type{\bool}
932932 */
933 - public function isExternal() { return ( '' != $this->mInterwiki ); }
 933+ public function isExternal() { return ( $this->mInterwiki != '' ); }
934934
935935 /**
936936 * Is this page "semi-protected" - the *only* protection is autoconfirm?
——@@ -1305,7 +1305,7 @@
13061306 if( $right == 'sysop' ) {
13071307 $right = 'protect';
13081308 }
1309 - if( '' != $right && !$user->isAllowed( $right ) ) {
 1309+ if( $right != '' && !$user->isAllowed( $right ) ) {
13101310 // Users with 'editprotected' permission can edit protected pages
13111311 if( $action=='edit' && $user->isAllowed( 'editprotected' ) ) {
13121312 // Users with 'editprotected' permission cannot edit protected pages
——@@ -1337,7 +1337,7 @@
13381338 if( $cascadingSources > 0 && isset($restrictions[$action]) ) {
13391339 foreach( $restrictions[$action] as $right ) {
13401340 $right = ( $right == 'sysop' ) ? 'protect' : $right;
1341 - if( '' != $right && !$user->isAllowed( $right ) ) {
 1341+ if( $right != '' && !$user->isAllowed( $right ) ) {
13421342 $pages = '';
13431343 foreach( $cascadingSources as $page )
13441344 $pages .= '* [[:' . $page->getPrefixedText() . "]]\n";
——@@ -2196,7 +2196,7 @@
21972197 */
21982198 /* private */ function prefix( $name ) {
21992199 $p = '';
2200 - if ( '' != $this->mInterwiki ) {
 2200+ if ( $this->mInterwiki != '' ) {
22012201 $p = $this->mInterwiki . ':';
22022202 }
22032203 if ( 0 != $this->mNamespace ) {
——@@ -2274,7 +2274,7 @@
22752275 $dbkey = preg_replace( '/[ _\xA0\x{1680}\x{180E}\x{2000}-\x{200A}\x{2028}\x{2029}\x{202F}\x{205F}\x{3000}]+/u', '_', $dbkey );
22762276 $dbkey = trim( $dbkey, '_' );
22772277
2278 - if ( '' == $dbkey ) {
 2278+ if ( $dbkey == '' ) {
22792279 return false;
22802280 }
22812281
——@@ -2349,7 +2349,7 @@
23502350
23512351 # We already know that some pages won't be in the database!
23522352 #
2353 - if ( '' != $this->mInterwiki || NS_SPECIAL == $this->mNamespace ) {
 2353+ if ( $this->mInterwiki != '' || NS_SPECIAL == $this->mNamespace ) {
23542354 $this->mArticleID = 0;
23552355 }
23562356 $fragment = strstr( $dbkey, '#' );
——@@ -2661,9 +2661,9 @@
26622662 if ( strlen( $nt->getDBkey() ) < 1 ) {
26632663 $errors[] = array('articleexists');
26642664 }
2665 - if ( ( '' == $this->getDBkey() ) ||
 2665+ if ( ( $this->getDBkey() == '' ) ||
26662666 ( !$oldid ) ||
2667 - ( '' == $nt->getDBkey() ) ) {
 2667+ ( $nt->getDBkey() == '' ) ) {
26682668 $errors[] = array('badarticleerror');
26692669 }
26702670
Index: trunk/phase3/includes/parser/Parser.php
——@@ -537,7 +537,7 @@
538538 $taglist = implode( '|', $elements );
539539 $start = "/<($taglist)(\\s+[^>]*?|\\s*?)(\/?" . ">)|<(!--)/i";
540540
541 - while ( '' != $text ) {
 541+ while ( $text != '' ) {
542542 $p = preg_split( $start, $text, 2, PREG_SPLIT_DELIM_CAPTURE );
543543 $stripped .= $p[0];
544544 if( count( $p ) < 5 ) {
——@@ -1683,7 +1683,7 @@
16841684 wfProfileOut( __METHOD__."-might_be_img" );
16851685 }
16861686
1687 - $wasblank = ( '' == $text );
 1687+ $wasblank = ( $text == '' );
16881688 if ( $wasblank ) $text = $link;
16891689
16901690 # Link not escaped by : , create the various objects
——@@ -1871,7 +1871,7 @@
18721872 */
18731873 /* private */ function closeParagraph() {
18741874 $result = '';
1875 - if ( '' != $this->mLastSection ) {
 1875+ if ( $this->mLastSection != '' ) {
18761876 $result = '</' . $this->mLastSection . ">\n";
18771877 }
18781878 $this->mInPre = false;
——@@ -2081,7 +2081,7 @@
20822082 $t = substr( $t, 1 );
20832083 } else {
20842084 // paragraph
2085 - if ( '' == trim($t) ) {
 2085+ if ( trim($t) == '' ) {
20862086 if ( $paragraphStack ) {
20872087 $output .= $paragraphStack.'<br />';
20882088 $paragraphStack = false;
——@@ -2121,7 +2121,7 @@
21222122 $output .= $this->closeList( $prefix2[$prefixLength-1] );
21232123 --$prefixLength;
21242124 }
2125 - if ( '' != $this->mLastSection ) {
 2125+ if ( $this->mLastSection != '' ) {
21262126 $output .= '</' . $this->mLastSection . '>';
21272127 $this->mLastSection = '';
21282128 }
——@@ -3989,7 +3989,7 @@
39903990 $m = array();
39913991 if ( preg_match( "/^($nc+:|)$tc+?( \\($tc+\\))$/", $t, $m ) ) {
39923992 $text = preg_replace( $p2, "[[$m[1]\\1$m[2]|\\1]]", $text );
3993 - } elseif ( preg_match( "/^($nc+:|)$tc+?(, $tc+|)$/", $t, $m ) && '' != "$m[1]$m[2]" ) {
 3993+ } elseif ( preg_match( "/^($nc+:|)$tc+?(, $tc+|)$/", $t, $m ) && "$m[1]$m[2]" != '' ) {
39943994 $text = preg_replace( $p2, "[[$m[1]\\1$m[2]|\\1]]", $text );
39953995 } else {
39963996 # if there's no context, don't bother duplicating the title
Index: trunk/phase3/includes/Linker.php
——@@ -350,7 +350,7 @@
351351 * despite $query not being used.
352352 */
353353 function makeSelfLinkObj( $nt, $text = '', $query = '', $trail = '', $prefix = '' ) {
354 - if ( '' == $text ) {
 354+ if ( $text == '' ) {
355355 $text = htmlspecialchars( $nt->getPrefixedText() );
356356 }
357357 list( $inside, $trail ) = Linker::splitTrail( $trail );
——@@ -388,7 +388,7 @@
389389 * via Parser::maybeMakeExternalImage().
390390 */
391391 function makeExternalImage( $url, $alt = '' ) {
392 - if ( '' == $alt ) {
 392+ if ( $alt == '' ) {
393393 $alt = $this->fnamePart( $url );
394394 }
395395 $img = '';
——@@ -541,7 +541,7 @@
542542
543543 $s = $thumb->toHtml( $params );
544544 }
545 - if ( '' != $fp['align'] ) {
 545+ if ( $fp['align'] != '' ) {
546546 $s = "<div class=\"float{$fp['align']}\">{$s}</div>";
547547 }
548548 return str_replace("\n", ' ',$prefix.$s.$postfix);
——@@ -748,7 +748,7 @@
749749 function specialLink( $name, $key = '' ) {
750750 global $wgContLang;
751751
752 - if ( '' == $key ) { $key = strtolower( $name ); }
 752+ if ( $key == '' ) { $key = strtolower( $name ); }
753753 $pn = $wgContLang->ucfirst( $name );
754754 return $this->makeKnownLink( $wgContLang->specialPage( $pn ),
755755 wfMsg( $key ) );
——@@ -1068,7 +1068,7 @@
10691069 }
10701070
10711071 # Handle link renaming [[foo|text]] will show link as "text"
1072 - if( "" != $match[3] ) {
 1072+ if( $match[3] != "" ) {
10731073 $text = $match[3];
10741074 } else {
10751075 $text = $match[1];
——@@ -1153,7 +1153,7 @@
11541154 }
11551155
11561156 $ret = $contextTitle->getPrefixedText(). '/' . trim($noslash) . $suffix;
1157 - if( '' === $text ) {
 1157+ if( $text === '' ) {
11581158 $text = $target . $suffix;
11591159 } # this might be changed for ugliness reasons
11601160 } else {
——@@ -1171,7 +1171,7 @@
11721172 # / at the end means don't show full path
11731173 if( substr( $nodotdot, -1, 1 ) === '/' ) {
11741174 $nodotdot = substr( $nodotdot, 0, -1 );
1175 - if( '' === $text ) {
 1175+ if( $text === '' ) {
11761176 $text = $nodotdot . $suffix;
11771177 }
11781178 }
——@@ -1422,7 +1422,7 @@
14231423 $regex = $wgContLang->linkTrail();
14241424 }
14251425 $inside = '';
1426 - if ( '' != $trail ) {
 1426+ if ( $trail != '' ) {
14271427 $m = array();
14281428 if ( preg_match( $regex, $trail, $m ) ) {
14291429 $inside = $m[1];
Index: trunk/phase3/includes/db/Database.php
——@@ -2223,7 +2223,7 @@
22242224 }
22252225 }
22262226
2227 - if ( '' != $cmd ) { $cmd .= ' '; }
 2227+ if ( $cmd != '' ) { $cmd .= ' '; }
22282228 $cmd .= "$line\n";
22292229
22302230 if ( $done ) {
Index: trunk/phase3/includes/db/DatabaseOracle.php
——@@ -955,7 +955,7 @@
956956 }
957957 }
958958
959 - if ( '' != $cmd ) {
 959+ if ( $cmd != '' ) {
960960 $cmd .= ' ';
961961 }
962962 $cmd .= "$line\n";
Index: trunk/phase3/includes/LogPage.php
——@@ -104,7 +104,7 @@
105105 */
106106 public function getRcComment() {
107107 $rcComment = $this->actionText;
108 - if( '' != $this->comment ) {
 108+ if( $this->comment != '' ) {
109109 if ($rcComment == '')
110110 $rcComment = $this->comment;
111111 else
Index: trunk/phase3/includes/Export.php
——@@ -449,7 +449,7 @@
450450 if( $row->page_is_redirect ) {
451451 $out .= ' ' . Xml::element( 'redirect', array() ) . "\n";
452452 }
453 - if( '' != $row->page_restrictions ) {
 453+ if( $row->page_restrictions != '' ) {
454454 $out .= ' ' . Xml::element( 'restrictions', array(),
455455 strval( $row->page_restrictions ) ) . "\n";
456456 }
Index: trunk/phase3/includes/EditPage.php
——@@ -934,7 +934,7 @@
935935 }
936936
937937 # Don't save a new article if it's blank.
938 - if ( '' == $this->textbox1 ) {
 938+ if ( $this->textbox1 == '' ) {
939939 wfProfileOut( __METHOD__ );
940940 return self::AS_BLANK_ARTICLE;
941941 }
Index: trunk/phase3/includes/OutputPage.php
——@@ -1004,7 +1004,7 @@
10051005 return;
10061006 }
10071007 wfProfileIn( __METHOD__ );
1008 - if ( '' != $this->mRedirect ) {
 1008+ if ( $this->mRedirect != '' ) {
10091009 # Standards require redirect URLs to be absolute
10101010 $this->mRedirect = wfExpandUrl( $this->mRedirect );
10111011 if( $this->mRedirectCode == '301' || $this->mRedirectCode == '303' ) {
——@@ -1569,7 +1569,7 @@
15701570 $returntoquery = $wgRequest->getText( 'returntoquery' );
15711571 }
15721572
1573 - if ( '' === $returnto ) {
 1573+ if ( $returnto === '' ) {
15741574 $returnto = Title::newMainPage();
15751575 }
15761576
——@@ -1607,7 +1607,7 @@
16081608 $ret .= "<?xml version=\"1.0\" encoding=\"$wgOutputEncoding\" ?" . ">\n";
16091609 }
16101610
1611 - if ( '' == $this->getHTMLTitle() ) {
 1611+ if ( $this->getHTMLTitle() == '' ) {
16121612 $this->setHTMLTitle( wfMsg( 'pagetitle', $this->getPageTitle() ));
16131613 }
16141614
Index: trunk/phase3/includes/Wiki.php
——@@ -113,7 +113,7 @@
114114 if( $curid = $wgRequest->getInt( 'curid' ) ) {
115115 # URLs like this are generated by RC, because rc_title isn't always accurate
116116 $ret = Title::newFromID( $curid );
117 - } elseif( '' == $title && 'delete' != $action ) {
 117+ } elseif( $title == '' && 'delete' != $action ) {
118118 $ret = Title::newMainPage();
119119 } else {
120120 $ret = Title::newFromURL( $title );
Index: trunk/phase3/includes/GlobalFunctions.php
——@@ -337,7 +337,7 @@
338338 array_map( array( $wgOut, 'debug' ), $cache );
339339 $cache = array();
340340 }
341 - if ( '' != $wgDebugLogFile && !$wgProfileOnly ) {
 341+ if ( $wgDebugLogFile != '' && !$wgProfileOnly ) {
342342 # Strip unprintables; they can switch terminal modes when binary data
343343 # gets dumped, which is pretty annoying.
344344 $text = preg_replace( '![\x00-\x08\x0b\x0c\x0e-\x1f]!', ' ', $text );
——@@ -480,7 +480,7 @@
481481 $log = sprintf( "%s\t%04.3f\t%s\n",
482482 gmdate( 'YmdHis' ), $elapsed,
483483 urldecode( $wgRequest->getRequestURL() . $forward ) );
484 - if ( '' != $wgDebugLogFile && ( $wgRequest->getVal('action') != 'raw' || $wgDebugRawPage ) ) {
 484+ if ( $wgDebugLogFile != '' && ( $wgRequest->getVal('action') != 'raw' || $wgDebugRawPage ) ) {
485485 wfErrorLog( $log . $prof, $wgDebugLogFile );
486486 }
487487 }
——@@ -497,7 +497,7 @@
498498 if ( !is_null( $wgReadOnly ) ) {
499499 return (bool)$wgReadOnly;
500500 }
501 - if ( '' == $wgReadOnlyFile ) {
 501+ if ( $wgReadOnlyFile == '' ) {
502502 return false;
503503 }
504504 // Set $wgReadOnly for faster access next time
——@@ -1285,8 +1285,8 @@
12861286
12871287 $cgi = '';
12881288 foreach ( $array1 as $key => $value ) {
1289 - if ( '' !== $value ) {
1290 - if ( '' != $cgi ) {
 1289+ if ( $value != '' ) {
 1290+ if ( $cgi != '' ) {
12911291 $cgi .= '&';
12921292 }
12931293 if ( is_array( $value ) ) {
Index: trunk/phase3/includes/SkinTemplate.php
——@@ -744,10 +744,10 @@
745745 $urlArgs = 'action=edit&section=new';
746746 $preloadMsg = wfMsgForContent( 'talk-addsection-preload' );
747747 $editintroMsg = wfMsgForContent( 'talk-addsection-editintro' );
748 - if( '' != $preloadMsg ) {
 748+ if( $preloadMsg != '' ) {
749749 $urlArgs .= '&preload=' . urlencode( $preloadMsg );
750750 }
751 - if( '' != $editintroMsg ) {
 751+ if( $editintroMsg != '' ) {
752752 $urlArgs .= '&editintro=' . urlencode( $editintroMsg );
753753 }
754754 $content_actions['addsection'] = array(
Index: trunk/phase3/includes/specials/SpecialUserlogin.php
——@@ -122,7 +122,7 @@
123123 function addNewAccountMailPassword() {
124124 global $wgOut;
125125
126 - if ('' == $this->mEmail) {
 126+ if ( $this->mEmail == '' ) {
127127 $this->mainLoginForm( wfMsg( 'noemail', htmlspecialchars( $this->mName ) ) );
128128 return;
129129 }
——@@ -234,7 +234,7 @@
235235 // cation server before they create an account (otherwise, they can
236236 // create a local account and login as any domain user). We only need
237237 // to check this for domains that aren't local.
238 - if( 'local' != $this->mDomain && '' != $this->mDomain ) {
 238+ if( 'local' != $this->mDomain && $this->mDomain != '' ) {
239239 if( !$wgAuth->canCreateAccounts() && ( !$wgAuth->userExists( $this->mName ) || !$wgAuth->authenticate( $this->mName, $this->mPassword ) ) ) {
240240 $this->mainLoginForm( wfMsg( 'wrongpassword' ) );
241241 return false;
——@@ -394,7 +394,7 @@
395395 */
396396 public function authenticateUserData() {
397397 global $wgUser, $wgAuth;
398 - if ( '' == $this->mName ) {
 398+ if ( $this->mName == '' ) {
399399 return self::NO_NAME;
400400 }
401401
——@@ -492,7 +492,7 @@
493493 // faces etc will probably just fail cleanly here.
494494 $retval = self::RESET_PASS;
495495 } else {
496 - $retval = '' == $this->mPassword ? self::EMPTY_PASS : self::WRONG_PASS;
 496+ $retval = ($this->mPassword == '') ? self::EMPTY_PASS : self::WRONG_PASS;
497497 }
498498 } else {
499499 $wgAuth->updateUser( $u );
——@@ -670,7 +670,7 @@
671671 return;
672672 }
673673
674 - if ( '' == $this->mName ) {
 674+ if ( $this->mName == '' ) {
675675 $this->mainLoginForm( wfMsg( 'noname' ) );
676676 return;
677677 }
——@@ -714,7 +714,7 @@
715715 function mailPasswordInternal( $u, $throttle = true, $emailTitle = 'passwordremindertitle', $emailText = 'passwordremindertext' ) {
716716 global $wgServer, $wgScript, $wgUser, $wgNewPasswordExpiry;
717717
718 - if ( '' == $u->getEmail() ) {
 718+ if ( $u->getEmail() == '' ) {
719719 return new WikiError( wfMsg( 'noemail', $u->getName() ) );
720720 }
721721 $ip = wfGetIP();
——@@ -868,7 +868,7 @@
869869 }
870870 }
871871
872 - if ( '' == $this->mName ) {
 872+ if ( $this->mName == '' ) {
873873 if ( $wgUser->isLoggedIn() ) {
874874 $this->mName = $wgUser->getName();
875875 } else {
Index: trunk/phase3/includes/specials/SpecialLockdb.php
——@@ -53,7 +53,7 @@
5454 $wgOut->setPagetitle( wfMsg( 'lockdb' ) );
5555 $wgOut->addWikiMsg( 'lockdbtext' );
5656
57 - if ( "" != $err ) {
 57+ if ( $err != "" ) {
5858 $wgOut->setSubtitle( wfMsg( 'formerror' ) );
5959 $wgOut->addHTML( '<p class="error">' . htmlspecialchars( $err ) . "</p>\n" );
6060 }
Index: trunk/phase3/includes/specials/SpecialIpblocklist.php
——@@ -95,7 +95,7 @@
9696 $titleObj = SpecialPage::getTitleFor( "Ipblocklist" );
9797 $action = $titleObj->getLocalURL( "action=submit" );
9898
99 - if ( "" != $err ) {
 99+ if ( $err != "" ) {
100100 $wgOut->setSubtitle( wfMsg( "formerror" ) );
101101 $wgOut->addWikiText( Xml::tags( 'span', array( 'class' => 'error' ), $err ) . "\n" );
102102 }
——@@ -236,7 +236,7 @@
237237 global $wgOut, $wgUser;
238238
239239 $wgOut->setPagetitle( wfMsg( "ipblocklist" ) );
240 - if ( "" != $msg ) {
 240+ if ( $msg != "" ) {
241241 $wgOut->setSubtitle( $msg );
242242 }
243243
Index: trunk/phase3/includes/specials/SpecialProtectedtitles.php
——@@ -16,7 +16,7 @@
1717 function showList( $msg = '' ) {
1818 global $wgOut, $wgRequest;
1919
20 - if ( "" != $msg ) {
 20+ if ( $msg != "" ) {
2121 $wgOut->setSubtitle( $msg );
2222 }
2323
Index: trunk/phase3/includes/specials/SpecialEmailuser.php
——@@ -262,7 +262,7 @@
263263
264264 }
265265 static function validateEmailTarget ( $target ) {
266 - if ( "" == $target ) {
 266+ if ( $target == "" ) {
267267 wfDebug( "Target is empty.\n" );
268268 return "notarget";
269269 }
Index: trunk/phase3/includes/specials/SpecialUnlockdb.php
——@@ -45,7 +45,7 @@
4646 $wgOut->setPagetitle( wfMsg( "unlockdb" ) );
4747 $wgOut->addWikiMsg( "unlockdbtext" );
4848
49 - if ( "" != $err ) {
 49+ if ( $err != "" ) {
5050 $wgOut->setSubtitle( wfMsg( "formerror" ) );
5151 $wgOut->addHTML( '<p class="error">' . htmlspecialchars( $err ) . "</p>\n" );
5252 }
Index: trunk/phase3/includes/specials/SpecialSearch.php
——@@ -219,7 +219,7 @@
220220 }
221221
222222 $filePrefix = $wgContLang->getFormattedNsText(NS_FILE).':';
223 - if( '' === trim( $term ) || $filePrefix === trim( $term ) ) {
 223+ if( trim( $term ) === '' || $filePrefix === trim( $term ) ) {
224224 $wgOut->addHTML( $this->searchFocus() );
225225 $wgOut->addHTML( $this->formHeader($term, 0, 0));
226226 if( $this->searchAdvanced ) {
Index: trunk/phase3/includes/specials/SpecialProtectedpages.php
——@@ -16,7 +16,7 @@
1717 public function showList( $msg = '' ) {
1818 global $wgOut, $wgRequest;
1919
20 - if( "" != $msg ) {
 20+ if( $msg != "" ) {
2121 $wgOut->setSubtitle( $msg );
2222 }
2323
Index: trunk/phase3/skins/CologneBlue.php
——@@ -228,13 +228,13 @@
229229 }
230230 if ( $wgUser->isAllowed( 'delete' ) ) {
231231 $dtp = $this->deleteThisPage();
232 - if ( '' != $dtp ) {
 232+ if ( $dtp != '' ) {
233233 $s .= $sep . $dtp;
234234 }
235235 }
236236 if ( $wgUser->isAllowed( 'protect' ) ) {
237237 $ptp = $this->protectThisPage();
238 - if ( '' != $ptp ) {
 238+ if ( $ptp != '' ) {
239239 $s .= $sep . $ptp;
240240 }
241241 }
——@@ -341,7 +341,7 @@
342342 $search = $wgRequest->getText( 'search' );
343343 $action = $this->escapeSearchLink();
344344 $s = "<form id=\"searchform{$this->searchboxes}\" method=\"get\" class=\"inline\" action=\"$action\">";
345 - if( '' != $label ) {
 345+ if( $label != '' ) {
346346 $s .= "{$label}: ";
347347 }
348348

Follow-up revisions

RevisionCommit summaryAuthorDate
r60748Minor followup to r60744, fix for comment 2reedy21:27, 6 January 2010
r61175Follow-up to r60744: one more const != $var <--> $var != const switchmaxsem18:25, 17 January 2010

Comments

#Comment by Bryan (talk | contribs)   20:17, 6 January 2010

Well, if you have a very short and a very long condition, the "<short> !== <long>" is more readable than the other way round.

#Comment by P.Copp (talk | contribs)   21:15, 6 January 2010 
--- trunk/phase3/includes/GlobalFunctions.php 2010-01-06 19:51:29 UTC (rev 60743)
+++ trunk/phase3/includes/GlobalFunctions.php 2010-01-06 19:59:42 UTC (rev 60744)
[...]
@@ -1285,8 +1285,8 @@
 
  $cgi = '';
  foreach ( $array1 as $key => $value ) {
- if ( '' !== $value ) {
- if ( '' != $cgi ) {
+ if ( $value != '' ) {
+ if ( $cgi != '' ) {
[...]

This one breaks query parameters set to 0, e.g. the "show bots"-link on RecentChanges.

#Comment by Reedy (talk | contribs)   21:29, 6 January 2010

Cheers, fixed in r60748

#Comment by MarkAHershberger (talk | contribs)   00:14, 7 January 2010

I missed the discussion on IRC, but this seems very dangerous since people put constants on the LHS to avoid boneheaded bugs like 

if ( $var =  ) {


#Comment by Reedy (talk | contribs)   00:42, 7 January 2010

Sure, but people should really realise that you need the ==

Either way, the usages like this were certainly the minority

#Comment by MarkAHershberger (talk | contribs)   01:13, 7 January 2010

"should recognize" isn't as foolproof as "won't work".

Yes, people "should recognize" that 

if ( $var = "" ) {

is incorrect. But typos happen. However, if you're in the habit of writing with the variable on the RHS, then the following won't work: 

if ( "" = $var ) {

PHP sees a syntax error ("unexpected =") and you know right away that you've made a typo.

#Comment by Reedy (talk | contribs)   16:37, 7 January 2010

[16:32:28] <Reedy> flyingparchment: you don't like  != someVariable do you? [16:32:38] <flyingparchment> no [16:32:39] <Reedy> you prefer someVaribale != [16:32:51] <flyingparchment> no one in their right mind likes 0 != var

Status & tagging log