r42548 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r42547‎ | r42548 | r42549 >
Date:14:04, 25 October 2008
Author:tstarling
Status:old
Tags:
Comment:
Removed all instances of empty() where error suppression was not intended. Replaced with conversion to boolean, count() or empty string check as appropriate. Fixes a number of bugs due to incorrect conversion to boolean: suppressed edit summaries containing '0', ignored titles called '0', searches for '0' ignored, etc.
Modified paths:
  • /trunk/phase3/includes/api/ApiBase.php (modified) (history)
  • /trunk/phase3/includes/api/ApiBlock.php (modified) (history)
  • /trunk/phase3/includes/api/ApiDelete.php (modified) (history)
  • /trunk/phase3/includes/api/ApiEditPage.php (modified) (history)
  • /trunk/phase3/includes/api/ApiFormatXml.php (modified) (history)
  • /trunk/phase3/includes/api/ApiMain.php (modified) (history)
  • /trunk/phase3/includes/api/ApiPageSet.php (modified) (history)
  • /trunk/phase3/includes/api/ApiPatrol.php (modified) (history)
  • /trunk/phase3/includes/api/ApiProtect.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQuery.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryBacklinks.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryCategoryInfo.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryDuplicateFiles.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryExtLinksUsage.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryInfo.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryLogEvents.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryRecentChanges.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryRevisions.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQuerySearch.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryUserContributions.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryUsers.php (modified) (history)
  • /trunk/phase3/includes/api/ApiQueryWatchlist.php (modified) (history)
  • /trunk/phase3/includes/api/ApiResult.php (modified) (history)
  • /trunk/phase3/includes/api/ApiRollback.php (modified) (history)
  • /trunk/phase3/includes/api/ApiUnblock.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/api/ApiProtect.php
——@@ -46,7 +46,7 @@
4747 $this->dieUsageMsg(array('missingparam', 'title'));
4848 if(!isset($params['token']))
4949 $this->dieUsageMsg(array('missingparam', 'token'));
50 - if(!isset($params['protections']) || empty($params['protections']))
 50+ if(empty($params['protections']))
5151 $this->dieUsageMsg(array('missingparam', 'protections'));
5252
5353 if(!$wgUser->matchEditToken($params['token']))
——@@ -57,7 +57,7 @@
5858 $this->dieUsageMsg(array('invalidtitle', $params['title']));
5959
6060 $errors = $titleObj->getUserPermissionsErrors('protect', $wgUser);
61 - if(!empty($errors))
 61+ if($errors)
6262 // We don't care about multiple errors, just report one of them
6363 $this->dieUsageMsg(current($errors));
6464
Index: trunk/phase3/includes/api/ApiEditPage.php
——@@ -68,7 +68,7 @@
6969 $errors = $titleObj->getUserPermissionsErrors('edit', $wgUser);
7070 if(!$titleObj->exists())
7171 $errors = array_merge($errors, $titleObj->getUserPermissionsErrors('create', $wgUser));
72 - if(!empty($errors))
 72+ if(count($errors))
7373 $this->dieUsageMsg($errors[0]);
7474
7575 $articleObj = new Article($titleObj);
——@@ -145,7 +145,7 @@
146146 $r = array();
147147 if(!wfRunHooks('APIEditBeforeSave', array(&$ep, $ep->textbox1, &$r)))
148148 {
149 - if(!empty($r))
 149+ if(count($r))
150150 {
151151 $r['result'] = "Failure";
152152 $this->getResult()->addValue(null, $this->getModuleName(), $r);
Index: trunk/phase3/includes/api/ApiQueryUserContributions.php
——@@ -227,7 +227,7 @@
228228 $vals['top'] = '';
229229 }
230230
231 - if ($this->fld_comment && !empty ($row->rev_comment))
 231+ if ($this->fld_comment && isset( $row->rev_comment ) )
232232 $vals['comment'] = $row->rev_comment;
233233
234234 return $vals;
Index: trunk/phase3/includes/api/ApiQueryUsers.php
——@@ -68,7 +68,7 @@
6969 else
7070 $goodNames[] = $n;
7171 }
72 - if(empty($goodNames))
 72+ if(!count($goodNames))
7373 return $retval;
7474
7575 $db = $this->getDb();
Index: trunk/phase3/includes/api/ApiBlock.php
——@@ -88,7 +88,7 @@
8989
9090 $userID = $expiry = null;
9191 $retval = $form->doBlock($userID, $expiry);
92 - if(!empty($retval))
 92+ if(count($retval))
9393 // We don't care about multiple errors, just report one of them
9494 $this->dieUsageMsg($retval);
9595
Index: trunk/phase3/includes/api/ApiQueryRecentChanges.php
——@@ -100,7 +100,7 @@
101101 $this->addWhereRange('rc_timestamp', $dir, $start, $end);
102102 $this->addWhereFld('rc_namespace', $namespace);
103103 $this->addWhereFld('rc_deleted', 0);
104 - if(!empty($titles))
 104+ if($titles)
105105 {
106106 $lb = new LinkBatch;
107107 foreach($titles as $t)
——@@ -256,7 +256,7 @@
257257 private function extractRowInfo($row) {
258258 /* If page was moved somewhere, get the title of the move target. */
259259 $movedToTitle = false;
260 - if (!empty($row->rc_moved_to_title))
 260+ if (isset($row->rc_moved_to_title) && $row->rc_moved_to_title !== '')
261261 $movedToTitle = Title :: makeTitle($row->rc_moved_to_ns, $row->rc_moved_to_title);
262262
263263 /* Determine the title of the page that has been changed. */
——@@ -320,7 +320,7 @@
321321 $vals['timestamp'] = wfTimestamp(TS_ISO_8601, $row->rc_timestamp);
322322
323323 /* Add edit summary / log summary. */
324 - if ($this->fld_comment && !empty ($row->rc_comment)) {
 324+ if ($this->fld_comment && isset($row->rc_comment)) {
325325 $vals['comment'] = $row->rc_comment;
326326 }
327327
Index: trunk/phase3/includes/api/ApiQueryExtLinksUsage.php
——@@ -54,7 +54,7 @@
5555
5656 // Find the right prefix
5757 global $wgUrlProtocols;
58 - if(!is_null($protocol) && !empty($protocol) && !in_array($protocol, $wgUrlProtocols))
 58+ if($protocol && !in_array($protocol, $wgUrlProtocols))
5959 {
6060 foreach ($wgUrlProtocols as $p) {
6161 if( substr( $p, 0, strlen( $protocol ) ) === $protocol ) {
Index: trunk/phase3/includes/api/ApiQueryInfo.php
——@@ -197,7 +197,7 @@
198198 $pageLength = $pageSet->getCustomField('page_len');
199199
200200 $db = $this->getDB();
201 - if ($fld_protection && !empty($titles)) {
 201+ if ($fld_protection && count($titles)) {
202202 $this->addTables('page_restrictions');
203203 $this->addFields(array('pr_page', 'pr_type', 'pr_level', 'pr_expiry', 'pr_cascade'));
204204 $this->addWhereFld('pr_page', array_keys($titles));
——@@ -273,7 +273,7 @@
274274 }
275275
276276 // We don't need to check for pt stuff if there are no nonexistent titles
277 - if($fld_protection && !empty($missing))
 277+ if($fld_protection && count($missing))
278278 {
279279 $this->resetQueryParams();
280280 // Construct a custom WHERE clause that matches all titles in $missing
——@@ -367,7 +367,7 @@
368368 else if($fld_talkid)
369369 $talktitles[] = $t->getTalkPage();
370370 }
371 - if(!empty($talktitles) || !empty($subjecttitles))
 371+ if(count($talktitles) || count($subjecttitles))
372372 {
373373 // Construct a custom WHERE clause that matches
374374 // all titles in $talktitles and $subjecttitles
Index: trunk/phase3/includes/api/ApiMain.php
——@@ -590,7 +590,7 @@
591591
592592 public static function makeHelpMsgHeader($module, $paramName) {
593593 $modulePrefix = $module->getModulePrefix();
594 - if (!empty($modulePrefix))
 594+ if (strval($modulePrefix) !== '')
595595 $modulePrefix = "($modulePrefix) ";
596596
597597 return "* $paramName={$module->getModuleName()} $modulePrefix*";
Index: trunk/phase3/includes/api/ApiQueryBacklinks.php
——@@ -211,7 +211,7 @@
212212 }
213213 $db->freeResult($res);
214214
215 - if($this->redirect && !empty($this->redirTitles))
 215+ if($this->redirect && count($this->redirTitles))
216216 {
217217 $this->resetQueryParams();
218218 $this->prepareSecondQuery($resultPageSet);
Index: trunk/phase3/includes/api/ApiQuerySearch.php
——@@ -54,7 +54,7 @@
5555 $limit = $params['limit'];
5656 $query = $params['search'];
5757 $what = $params['what'];
58 - if (is_null($query) || empty($query))
 58+ if (strval($query) === '')
5959 $this->dieUsage("empty search string is not allowed", 'param-search');
6060
6161 $search = SearchEngine::create();
Index: trunk/phase3/includes/api/ApiUnblock.php
——@@ -69,7 +69,7 @@
7070 $user = $params['user'];
7171 $reason = (is_null($params['reason']) ? '' : $params['reason']);
7272 $retval = IPUnblockForm::doUnblock($id, $user, $reason, $range);
73 - if(!empty($retval))
 73+ if($retval)
7474 $this->dieUsageMsg($retval);
7575
7676 $res['id'] = $id;
Index: trunk/phase3/includes/api/ApiDelete.php
——@@ -73,15 +73,15 @@
7474
7575 $reason = (isset($params['reason']) ? $params['reason'] : NULL);
7676 if ($titleObj->getNamespace() == NS_IMAGE) {
77 - $retval = self::deletefile($params['token'], $titleObj, $params['oldimage'], $reason, false);
78 - if(!empty($retval))
 77+ $retval = self::deleteFile($params['token'], $titleObj, $params['oldimage'], $reason, false);
 78+ if(count($retval))
7979 // We don't care about multiple errors, just report one of them
8080 $this->dieUsageMsg(current($retval));
8181 } else {
8282 $articleObj = new Article($titleObj);
8383 $retval = self::delete($articleObj, $params['token'], $reason);
8484
85 - if(!empty($retval))
 85+ if(count($retval))
8686 // We don't care about multiple errors, just report one of them
8787 $this->dieUsageMsg(current($retval));
8888
Index: trunk/phase3/includes/api/ApiBase.php
——@@ -572,7 +572,7 @@
573573 if (is_array($allowedValues)) {
574574 # Check for unknown values
575575 $unknown = array_diff($valuesList, $allowedValues);
576 - if(!empty($unknown))
 576+ if(count($unknown))
577577 {
578578 if($allowMultiple)
579579 {
Index: trunk/phase3/includes/api/ApiResult.php
——@@ -100,7 +100,7 @@
101101 }
102102 elseif (is_array($arr[$name]) && is_array($value)) {
103103 $merged = array_intersect_key($arr[$name], $value);
104 - if (empty ($merged))
 104+ if (!count($merged))
105105 $arr[$name] += $value;
106106 else
107107 ApiBase :: dieDebug(__METHOD__, "Attempting to merge element $name");
——@@ -180,7 +180,7 @@
181181 }
182182 }
183183
184 - if (empty($name))
 184+ if (!$name)
185185 $data[] = $value; // Add list element
186186 else
187187 ApiResult :: setElement($data, $name, $value); // Add named element
——@@ -201,7 +201,7 @@
202202 $argc = func_num_args();
203203
204204 if ($argc > 2) {
205 - for ($i = 1; !empty($isec) && $i < $argc; $i++) {
 205+ for ($i = 1; $isec && $i < $argc; $i++) {
206206 $arr = func_get_arg($i);
207207
208208 foreach (array_keys($isec) as $key) {
Index: trunk/phase3/includes/api/ApiPageSet.php
——@@ -368,7 +368,7 @@
369369 }
370370
371371 private function initFromPageIds($pageids) {
372 - if(empty($pageids))
 372+ if(!count($pageids))
373373 return;
374374
375375 $pageids = array_map('intval', $pageids); // paranoia
——@@ -440,7 +440,7 @@
441441 else
442442 {
443443 // The remaining pageids do not exist
444 - if(empty($this->mMissingPageIDs))
 444+ if(!$this->mMissingPageIDs)
445445 $this->mMissingPageIDs = array_keys($remaining);
446446 else
447447 $this->mMissingPageIDs = array_merge($this->mMissingPageIDs, array_keys($remaining));
——@@ -450,7 +450,7 @@
451451
452452 private function initFromRevIDs($revids) {
453453
454 - if(empty($revids))
 454+ if(!count($revids))
455455 return;
456456
457457 $db = $this->getDB();
——@@ -488,7 +488,7 @@
489489
490490 // Repeat until all redirects have been resolved
491491 // The infinite loop is prevented by keeping all known pages in $this->mAllPages
492 - while (!empty ($this->mPendingRedirectIDs)) {
 492+ while ($this->mPendingRedirectIDs) {
493493
494494 // Resolve redirects by querying the pagelinks table, and repeat the process
495495 // Create a new linkBatch object for the next pass
——@@ -537,7 +537,7 @@
538538 $this->mRedirectTitles[$from] = $to;
539539 }
540540 $db->freeResult($res);
541 - if(!empty($this->mPendingRedirectIDs))
 541+ if($this->mPendingRedirectIDs)
542542 {
543543 # We found pages that aren't in the redirect table
544544 # Add them
——@@ -580,7 +580,7 @@
581581 continue; // There's nothing else we can do
582582 }
583583 $iw = $titleObj->getInterwiki();
584 - if (!empty($iw)) {
 584+ if (strval($iw) !== '') {
585585 // This title is an interwiki link.
586586 $this->mInterwikiTitles[$titleObj->getPrefixedText()] = $iw;
587587 } else {
Index: trunk/phase3/includes/api/ApiQueryLogEvents.php
——@@ -213,7 +213,7 @@
214214 if ($this->fld_timestamp) {
215215 $vals['timestamp'] = wfTimestamp(TS_ISO_8601, $row->log_timestamp);
216216 }
217 - if ($this->fld_comment && !empty ($row->log_comment)) {
 217+ if ($this->fld_comment && isset($row->log_comment)) {
218218 $vals['comment'] = $row->log_comment;
219219 }
220220
Index: trunk/phase3/includes/api/ApiQueryCategoryInfo.php
——@@ -41,9 +41,10 @@
4242
4343 public function execute() {
4444 $alltitles = $this->getPageSet()->getAllTitlesByNamespace();
 45+ if ( empty( $alltitles[NS_CATEGORY] ) ) {
 46+ return;
 47+ }
4548 $categories = $alltitles[NS_CATEGORY];
46 - if(empty($categories))
47 - return;
4849
4950 $titles = $this->getPageSet()->getGoodTitles() +
5051 $this->getPageSet()->getMissingTitles();
Index: trunk/phase3/includes/api/ApiQueryDuplicateFiles.php
——@@ -50,9 +50,10 @@
5151 private function run($resultPageSet = null) {
5252 $params = $this->extractRequestParams();
5353 $namespaces = $this->getPageSet()->getAllTitlesByNamespace();
 54+ if ( empty( $namespaces[NS_IMAGE] ) ) {
 55+ return;
 56+ }
5457 $images = $namespaces[NS_IMAGE];
55 - if(empty($images))
56 - return;
5758
5859 $this->addTables('image', 'i1');
5960 $this->addTables('image', 'i2');
Index: trunk/phase3/includes/api/ApiQueryWatchlist.php
——@@ -237,7 +237,7 @@
238238 $vals['newlen'] = intval($row->rc_new_len);
239239 }
240240
241 - if ($this->fld_comment && !empty ($row->rc_comment))
 241+ if ($this->fld_comment && isset( $row->rc_comment ))
242242 $vals['comment'] = $row->rc_comment;
243243
244244 return $vals;
Index: trunk/phase3/includes/api/ApiRollback.php
——@@ -66,7 +66,7 @@
6767 $details = null;
6868 $retval = $articleObj->doRollback($username, $summary, $params['token'], $params['markbot'], $details);
6969
70 - if(!empty($retval))
 70+ if($retval)
7171 // We don't care about multiple errors, just report one of them
7272 $this->dieUsageMsg(current($retval));
7373
Index: trunk/phase3/includes/api/ApiPatrol.php
——@@ -57,7 +57,7 @@
5858 $this->dieUsageMsg(array('nosuchrcid', $params['rcid']));
5959 $retval = RecentChange::markPatrolled($params['rcid']);
6060
61 - if(!empty($retval))
 61+ if($retval)
6262 $this->dieUsageMsg(current($retval));
6363
6464 $result = array('rcid' => $rc->getAttribute('rc_id'));
Index: trunk/phase3/includes/api/ApiQuery.php
——@@ -256,7 +256,7 @@
257257 );
258258 }
259259
260 - if (!empty ($normValues)) {
 260+ if (count($normValues)) {
261261 $result->setIndexedTagName($normValues, 'n');
262262 $result->addValue('query', 'normalized', $normValues);
263263 }
——@@ -270,7 +270,7 @@
271271 );
272272 }
273273
274 - if (!empty ($intrwValues)) {
 274+ if (count($intrwValues)) {
275275 $result->setIndexedTagName($intrwValues, 'i');
276276 $result->addValue('query', 'interwiki', $intrwValues);
277277 }
——@@ -284,7 +284,7 @@
285285 );
286286 }
287287
288 - if (!empty ($redirValues)) {
 288+ if (count($redirValues)) {
289289 $result->setIndexedTagName($redirValues, 'r');
290290 $result->addValue('query', 'redirects', $redirValues);
291291 }
——@@ -293,7 +293,7 @@
294294 // Missing revision elements
295295 //
296296 $missingRevIDs = $pageSet->getMissingRevisionIDs();
297 - if (!empty ($missingRevIDs)) {
 297+ if (count($missingRevIDs)) {
298298 $revids = array ();
299299 foreach ($missingRevIDs as $revid) {
300300 $revids[$revid] = array (
——@@ -335,7 +335,7 @@
336336 $pages[$pageid] = $vals;
337337 }
338338
339 - if (!empty ($pages)) {
 339+ if (count($pages)) {
340340
341341 if ($this->params['indexpageids']) {
342342 $pageIDs = array_keys($pages);
Index: trunk/phase3/includes/api/ApiQueryRevisions.php
——@@ -288,7 +288,7 @@
289289
290290 if ($this->fld_comment) {
291291 $comment = $revision->getComment();
292 - if (!empty($comment))
 292+ if (strval($comment) !== '')
293293 $vals['comment'] = $comment;
294294 }
295295
Index: trunk/phase3/includes/api/ApiFormatXml.php
——@@ -114,15 +114,15 @@
115115 }
116116 }
117117
118 - if (is_null($subElemIndName) && !empty ($indElements))
 118+ if (is_null($subElemIndName) && count($indElements))
119119 ApiBase :: dieDebug(__METHOD__, "($elemName, ...) has integer keys without _element value. Use ApiResult::setIndexedTagName().");
120120
121 - if (!empty ($subElements) && !empty ($indElements) && !is_null($subElemContent))
 121+ if (count($subElements) && count($indElements) && !is_null($subElemContent))
122122 ApiBase :: dieDebug(__METHOD__, "($elemName, ...) has content and subelements");
123123
124124 if (!is_null($subElemContent)) {
125125 $this->printText($indstr . wfElement($elemName, $elemValue, $subElemContent));
126 - } elseif (empty ($indElements) && empty ($subElements)) {
 126+ } elseif (!count($indElements) && !count($subElements)) {
127127 $this->printText($indstr . wfElement($elemName, $elemValue));
128128 } else {
129129 $this->printText($indstr . wfElement($elemName, $elemValue, null));

Status & tagging log