Static code analysis housekeeping time... things that could be double-checked are...
authorNick Jenkins <nickj@users.mediawiki.org>
Tue, 21 Aug 2007 03:57:54 +0000 (03:57 +0000)
committerNick Jenkins <nickj@users.mediawiki.org>
Tue, 21 Aug 2007 03:57:54 +0000 (03:57 +0000)
if-if-else without curly braces [api/ApiQuerySiteinfo.php] --> adding
Unused global declaration:  $wgGroupPermissions --> removing
Unused global declaration:  $wgEmailConfirmToEdit (line 301) --> removing
Variable $id appears only once (line 1021)  --> removing
Variable $m was used before it was defined (line 805) --> defining.
Variable $retval was used before it was defined (line 2346) --> renaming to $result
Variable $rcid appears only once (line 244 of RecentChange.php)  --> using this instead of $change [Note: was left over from r24607 refactoring, revert if wrong please]
Unused global declaration:  $wgCommandLineMode (line 11) --> removing
Variable $k appears only once (line 132 of ImagePage.php) --> removing.
Variable $info appears only once (line 311 of ImagePage.php)  --> removing.
Unused global declaration:  $wgTitle (line 569 of ImagePage.php) -> removing.
Variable $handlerParams was used before it was defined (line 616 of Linker.php) --> resolved by Raymond in r24966
Variable $match was used before it was defined (line 1031 of Linker.php) --> defining.
Unused global declaration:  $wgEnotifWatchlist (line 253 of UserMailer.php) --> removing
Unused global declaration:  $wgShowUpdatedMarker (line 253 of UserMailer.php) --> removing
Variable $img appears only once (line 446 of SpecialUpload.php) --> added definition, defined as null, flagged with @todo  [Note: should $img be defined in this context, or is it intended to be null? And should the return value after the hook be checked in some way?]
Unused global declaration:  $wgEnableAPI (line 739 of SpecialUpload.php) --> removing.
Unused global declaration:  $wgNamespaceProtection (line 1030 of OutputPage.php) --> removing.
Unused global declaration:  $wgContLang (line 18 of SpecialWatchlist.php) --> removing.
Unused global declaration:  $wgRawHtml (line 269 of SpecialMovepage.php) --> removing.
The value of variable $page was never used (line 331 of SpecialUndelete.php) --> removing line, as $page gets redefined a few lines down.
Variable $synIndex appears only once (line 521 of MagicWord.php) --> commenting out.
Variable $case appears only once (line 539 of MagicWord.php) --> removing from foreach index key usage.
Variable $wgUser appears only once (line 1039 of Title.php) --> adding line to declare as a global, would be null otherwise.
Variable $m was used before it was defined (line 285 of Title.php) --> defining.
Variable $id appears only once (line 1150 of Title.php) --> removing from foreach index key usage.
Variable $subpage appears only once (line 1297 of Title.php) --> commenting out.
Variable $restrictions appears only once (line 1399 of Title.php) --> commenting out.
Variable $mime appears only once (line 210 of filerepo/OldLocalFile.php) --> removing.
Variable $deprefixedName appears only once (line 213 of filerepo/LocalFile.php) --> removing.
Variable $m appears only once (line 541 of filerepo/LocalFile.php) --> removing.
Variable $where appears only once (line 1245 of filerepo/LocalFile.php) --> removing.
Variable $info appears only once (line 1427 of filerepo/LocalFile.php) --> removing.
Variable $rel appears only once (line 138 of filerepo/RepoGroup.php) --> commenting out.
Variable $zone appears only once (line 138 of filerepo/RepoGroup.php) --> commenting out.
Variable $nbytes appears only once (line 208 of media/Generic.php) --> added a return line that uses $nbytes. [Note: I'm assuming that this was the intent]
Variable $offset appears only once (line 201 of SpecialListusers.php) --> removing.
Variable $limit appears only once (line 201 of SpecialListusers.php) --> removing.
Variable $groupTarget appears only once (line 203 of SpecialListusers.php) --> removing.
Unused global declaration:  $wgLang (line 74 of SpecialWantedpages.php) --> removing.
Variable $block appears only once (line 244 of SpecialProtectedpages.php) --> removing.
Variable $offset appears only once (line 281 of SpecialProtectedpages.php) --> removing.
Variable $limit appears only once (line 281 of SpecialProtectedpages.php) --> removing.
Unused global declaration:  $wgLang (line 30 of FileDeleteForm.php) --> removing.
Unused global declaration:  $wgServer (line 30 of FileDeleteForm.php) --> removing.

25 files changed:
includes/Article.php
includes/Database.php
includes/EditPage.php
includes/FileDeleteForm.php
includes/ImagePage.php
includes/Linker.php
includes/MagicWord.php
includes/OutputPage.php
includes/RecentChange.php
includes/SpecialListusers.php
includes/SpecialMovepage.php
includes/SpecialProtectedpages.php
includes/SpecialUndelete.php
includes/SpecialUpload.php
includes/SpecialUserlogin.php
includes/SpecialUserrights.php
includes/SpecialWantedpages.php
includes/SpecialWatchlist.php
includes/Title.php
includes/UserMailer.php
includes/api/ApiQuerySiteinfo.php
includes/filerepo/LocalFile.php
includes/filerepo/OldLocalFile.php
includes/filerepo/RepoGroup.php
includes/media/Generic.php

index cbf034b..32ce420 100644 (file)
@@ -802,6 +802,7 @@ class Article {
                                // Give hooks a chance to customise the output
                                if( wfRunHooks( 'ShowRawCssJs', array( $this->mContent, $this->mTitle, $wgOut ) ) ) {
                                        // Wrap the whole lot in a <pre> and don't parse
+                                       $m = array();
                                        preg_match( '!\.(css|js)$!u', $this->mTitle->getText(), $m );
                                        $wgOut->addHtml( "<pre class=\"mw-code mw-{$m[1]}\" dir=\"ltr\">\n" );
                                        $wgOut->addHtml( htmlspecialchars( $this->mContent ) );
@@ -2344,7 +2345,7 @@ class Article {
                                $wgOut->returnToMain( false, $this->mTitle );
                                break;
                        default:
-                               throw new MWException( __METHOD__ . ": Unknown return value `{$retval}`" );
+                               throw new MWException( __METHOD__ . ": Unknown return value `{$result}`" );
                }
 
        }
index e0969f2..7e5266f 100644 (file)
@@ -214,7 +214,7 @@ border=\"0\" ALT=\"Google\"></A>
 
                        $cache = new HTMLFileCache( $t );
                        if( $cache->isFileCached() ) {
-                               // FIXME: $msg is not defined on the next line.
+                               // @todo, FIXME: $msg is not defined on the next line.
                                $msg = '<p style="color: red"><b>'.$msg."<br />\n" .
                                        $cachederror . "</b></p>\n";
 
index a8db8bc..4cd776f 100644 (file)
@@ -298,7 +298,6 @@ class EditPage {
         */
        function edit() {
                global $wgOut, $wgUser, $wgRequest, $wgTitle;
-               global $wgEmailConfirmToEdit;
 
                if ( ! wfRunHooks( 'AlternateEdit', array( &$this ) ) )
                        return;
@@ -333,10 +332,11 @@ class EditPage {
 
                        if ($error[0] == 'readonlytext')
                        {
-                               if ($this->edit)
+                               if ($this->edit) {
                                        $this->formtype = 'preview';
-                               else if ($this->save || $this->preview || $this->diff)
+                               } elseif ($this->save || $this->preview || $this->diff) {
                                        $remove[] = $error;
+                               }
                        }
                }
                # array_diff returns elements in $permErrors that are not in $remove.
@@ -1018,9 +1018,10 @@ class EditPage {
                        if ( count($cascadeSources) > 0 ) {
                                # Explain, and list the titles responsible
                                $notice = wfMsgExt( 'cascadeprotectedwarning', array('parsemag'), count($cascadeSources) ) . "\n";
-                               foreach( $cascadeSources as $id => $page )
+                               foreach( $cascadeSources as $page ) {
                                        $notice .= '* [[:' . $page->getPrefixedText() . "]]\n";
                                }
+                       }
                        $wgOut->addWikiText( $notice );
                }
 
index fec993a..ad4e963 100644 (file)
@@ -29,7 +29,7 @@ class FileDeleteForm {
         * pending authentication, confirmation, etc.
         */
        public function execute() {
-               global $wgOut, $wgRequest, $wgUser, $wgLang, $wgServer;
+               global $wgOut, $wgRequest, $wgUser;
                $this->setHeaders();
 
                if( wfReadOnly() ) {
index 6443a16..f08a284 100644 (file)
@@ -129,7 +129,7 @@ class ImagePage extends Article {
                $r = wfMsg( 'metadata-help' ) . "\n\n";
                $r .= "{| id=mw_metadata class=mw_metadata\n";
                foreach ( $metadata as $type => $stuff ) {
-                       foreach ( $stuff as $k => $v ) {
+                       foreach ( $stuff as $v ) {
                                $class = Sanitizer::escapeId( $v['id'] );
                                if( $type == 'collapsed' ) {
                                        $class .= ' collapsable';
@@ -308,7 +308,6 @@ class ImagePage extends Article {
 
                        if ($showLink) {
                                $filename = wfEscapeWikiText( $this->img->getName() );
-                               $info = wfMsg( 'file-info', $sk->formatSize( $this->img->getSize() ), $mime );
 
                                global $wgContLang;
                                $dirmark = $wgContLang->getDirMark();
@@ -566,7 +565,7 @@ class ImageHistoryList {
        }
 
        public function imageHistoryLine( $iscur, $timestamp, $img, $user, $usertext, $size, $description, $dims ) {
-               global $wgUser, $wgLang, $wgTitle, $wgContLang;
+               global $wgUser, $wgLang, $wgContLang;
                $local = $this->img->isLocal();
                $row = '';
 
index d321e72..bede54b 100644 (file)
@@ -1028,6 +1028,7 @@ class Linker {
                $medians = '(?:' . preg_quote( Namespace::getCanonicalName( NS_MEDIA ), '/' ) . '|';
                $medians .= preg_quote( $wgContLang->getNsText( NS_MEDIA ), '/' ) . '):';
 
+               $match = array();
                while(preg_match('/\[\[:?(.*?)(\|(.*?))*\]\](.*)$/',$comment,$match)) {
                        # Handle link renaming [[foo|text]] will show link as "text"
                        if( "" != $match[3] ) {
index 9e8d025..f7a9400 100644 (file)
@@ -518,7 +518,7 @@ class MagicWordArray {
                                // continue;
                                throw new MWException( __METHOD__ . ': bad parameter name' );
                        }
-                       list( $synIndex, $magicName ) = $parts;
+                       list( /* $synIndex */, $magicName ) = $parts;
                        $paramValue = next( $m );
                        return array( $magicName, $paramValue );
                }
@@ -536,7 +536,7 @@ class MagicWordArray {
        public function matchVariableStartToEnd( $text ) {
                global $wgContLang;
                $regexes = $this->getVariableStartToEndRegex();
-               foreach ( $regexes as $case => $regex ) {
+               foreach ( $regexes as $regex ) {
                        if ( $regex !== '' ) {
                                $m = false;
                                if ( preg_match( $regex, $text, $m ) ) {
index 0dd4c3e..ae68f63 100644 (file)
@@ -1027,7 +1027,6 @@ class OutputPage {
                                        $this->addWikiText( wfMsgExt( 'cascadeprotected', 'parsemag', $count ) . "\n{$titles}" );
                        } elseif( !$wgTitle->isProtected( 'edit' ) && $wgTitle->isNamespaceProtected() ) {
                                // Namespace protection
-                               global $wgNamespaceProtection;
                                $ns = $wgTitle->getNamespace() == NS_MAIN
                                        ? wfMsg( 'nstab-main' )
                                        : $wgTitle->getNsText();
index c1bf4e6..79f32d0 100644 (file)
@@ -251,7 +251,7 @@ class RecentChange
                                'rc_patrolled' => 1
                        ),
                        array(
-                               'rc_id' => $change
+                               'rc_id' => $rcid
                        ),
                        __METHOD__
                );
index d6d4a34..460d425 100644 (file)
@@ -198,10 +198,6 @@ class UsersPager extends AlphabeticPager {
 function wfSpecialListusers( $par = null ) {
        global $wgRequest, $wgOut;
 
-       list( $limit, $offset ) = wfCheckLimits();
-
-       $groupTarget = isset($par) ? $par : $wgRequest->getVal( 'group' );
-
        $up = new UsersPager($par);
 
        # getBody() first to check, if empty
index 22b1a91..cfc434a 100644 (file)
@@ -266,7 +266,7 @@ class MovePageForm {
        }
 
        function showSuccess() {
-               global $wgOut, $wgRequest, $wgUser, $wgRawHtml;
+               global $wgOut, $wgRequest, $wgUser;
                
                $old = Title::newFromText( $wgRequest->getVal( 'oldtitle' ) );
                $new = Title::newFromText( $wgRequest->getVal( 'newtitle' ) );
index 416e861..122ca8f 100644 (file)
@@ -241,7 +241,6 @@ class ProtectedPagesPager extends AlphabeticPager {
        }
        
        function formatRow( $row ) {
-               $block = new Block;
                return $this->mForm->formatRow( $row );
        }
 
@@ -278,8 +277,6 @@ class ProtectedPagesPager extends AlphabeticPager {
  */
 function wfSpecialProtectedpages() {
 
-       list( $limit, $offset ) = wfCheckLimits();
-
        $ppForm = new ProtectedPagesForm();
 
        $ppForm->showList();
index ad88906..5678a81 100644 (file)
@@ -328,7 +328,6 @@ class PageArchive {
                $restoreAll = empty( $timestamps );
                
                $dbw = wfGetDB( DB_MASTER );
-               $page = $dbw->tableName( 'archive' );
 
                # Does this page already exist? We'll have to update it...
                $article = new Article( $this->title );
index 1c738cc..597fd14 100644 (file)
@@ -443,6 +443,7 @@ class UploadForm {
                        }
                        // Success, redirect to description page
                        $wgOut->redirect( $this->mLocalFile->getTitle()->getFullURL() );
+                       $img = null; // @todo: added to avoid passing a ref to null - should this be defined somewhere?
                        wfRunHooks( 'UploadComplete', array( &$img ) );
                }
        }
@@ -736,7 +737,7 @@ class UploadForm {
        function mainUploadForm( $msg='' ) {
                global $wgOut, $wgUser, $wgContLang;
                global $wgUseCopyrightUpload, $wgUseAjax, $wgAjaxUploadDestCheck, $wgAjaxLicensePreview;
-               global $wgRequest, $wgAllowCopyUploads, $wgEnableAPI;
+               global $wgRequest, $wgAllowCopyUploads;
                global $wgStylePath, $wgStyleVersion;
 
                $useAjaxDestCheck = $wgUseAjax && $wgAjaxUploadDestCheck;
index 2689c03..f358c1f 100644 (file)
@@ -8,7 +8,6 @@
  * constructor
  */
 function wfSpecialUserlogin() {
-       global $wgCommandLineMode;
        global $wgRequest;
        if( session_id() == '' ) {
                wfSetupSession();
index 93c1f38..b97e516 100644 (file)
@@ -319,7 +319,7 @@ class UserrightsForm extends HTMLForm {
         * @return Array array( 'add' => array( addablegroups ), 'remove' => array( removablegroups ) )
         */
        private function changeableGroups() {
-               global $wgUser, $wgGroupPermissions;
+               global $wgUser;
 
                $groups = array( 'add' => array(), 'remove' => array() );
                $addergroups = $wgUser->getEffectiveGroups();
index 02cd112..5fc45a8 100644 (file)
@@ -71,7 +71,6 @@ class WantedPagesPage extends QueryPage {
         * @return string
         */
        public function formatResult( $skin, $result ) {
-               global $wgLang;
                $title = Title::makeTitleSafe( $result->namespace, $result->title );
                if( $title instanceof Title ) {
                        if( $this->isCached() ) {
index 924edc2..e9aa7e6 100644 (file)
@@ -15,7 +15,7 @@ require_once( dirname(__FILE__) . '/SpecialRecentchanges.php' );
  * @param $par Parameter passed to the page
  */
 function wfSpecialWatchlist( $par ) {
-       global $wgUser, $wgOut, $wgLang, $wgRequest, $wgContLang;
+       global $wgUser, $wgOut, $wgLang, $wgRequest;
        global $wgRCShowWatchingUsers, $wgEnotifWatchlist, $wgShowUpdatedMarker;
        global $wgEnotifWatchlist;
        $fname = 'wfSpecialWatchlist';
index 73bf7ad..e37ed07 100644 (file)
@@ -279,6 +279,7 @@ class Title {
                $redir = MagicWord::get( 'redirect' );
                if( $redir->matchStart( $text ) ) {
                        // Extract the first link and see if it's usable
+                       $m = array();
                        if( preg_match( '!\[{2}(.*?)(?:\||\]{2})!', $text, $m ) ) {
                                // Strip preceding colon used to "escape" categories, etc.
                                // and URL-decode links
@@ -1034,7 +1035,7 @@ class Title {
                        $errors[] = array( 'readonlytext' );
                }
 
-               global $wgEmailConfirmToEdit;
+               global $wgEmailConfirmToEdit, $wgUser;
 
                if ( $wgEmailConfirmToEdit && !$wgUser->isEmailConfirmed() )
                {
@@ -1147,7 +1148,7 @@ class Title {
                                        $right = ( $right == 'sysop' ) ? 'protect' : $right;
                                        if( '' != $right && !$user->isAllowed( $right ) ) {
                                                $pages = '';
-                                               foreach( $cascadingSources as $id => $page )
+                                               foreach( $cascadingSources as $page )
                                                        $pages .= '* [[:' . $page->getPrefixedText() . "]]\n";
                                                $errors[] = array( 'cascadeprotected', count( $cascadingSources ), $pages );
                                        }
@@ -1294,7 +1295,7 @@ class Title {
                         */
                        if( $this->getNamespace() == NS_SPECIAL ) {
                                $name = $this->getText();
-                               list( $name, $subpage ) = SpecialPage::resolveAliasWithSubpage( $name );
+                               list( $name, /* $subpage */) = SpecialPage::resolveAliasWithSubpage( $name );
                                $pure = SpecialPage::getTitleFor( $name )->getPrefixedText();
                                if( in_array( $pure, $wgWhitelistRead, true ) )
                                        return true;
@@ -1396,7 +1397,7 @@ class Title {
         * @return bool If the page is subject to cascading restrictions.
         */
        public function isCascadeProtected() {
-               list( $sources, $restrictions ) = $this->getCascadeProtectionSources( false );
+               list( $sources, /* $restrictions */ ) = $this->getCascadeProtectionSources( false );
                return ( $sources > 0 );
        }
 
index ee85e25..835dd31 100644 (file)
@@ -250,7 +250,6 @@ class EmailNotification {
 
        function notifyOnPageChange($editor, &$title, $timestamp, $summary, $minorEdit, $oldid = false) {
                global $wgEnotifUseJobQ;
-               global $wgEnotifWatchlist, $wgShowUpdatedMarker;
        
                if( $title->getNamespace() < 0 )
                        return;
index 283275a..0a1245e 100644 (file)
@@ -106,12 +106,13 @@ class ApiQuerySiteinfo extends ApiQueryBase {
                $this->addTables('interwiki');
                $this->addFields(array('iw_prefix', 'iw_local', 'iw_url'));
 
-               if($filter === 'local')
+               if($filter === 'local') {
                        $this->addWhere('iw_local = 1');
-               else if($filter === '!local')
+               } elseif($filter === '!local') {
                        $this->addWhere('iw_local = 0');
-               else if($filter !== false)
+               } elseif($filter !== false) {
                        ApiBase :: dieDebug(__METHOD__, "Unknown filter=$filter");
+               }
 
                $this->addOption('ORDER BY', 'iw_prefix');
                
index d8f49ef..a2e19c8 100644 (file)
@@ -210,7 +210,6 @@ class LocalFile extends File
                }
                $decoded = array();
                foreach ( $array as $name => $value ) {
-                       $deprefixedName = substr( $name, $prefixLength );
                        $decoded[substr( $name, $prefixLength )] = $value;
                }
                $decoded['timestamp'] = wfTimestamp( TS_MW, $decoded['timestamp'] );
@@ -539,7 +538,6 @@ class LocalFile extends File
                $dir = $this->getThumbPath();
                $urls = array();
                foreach ( $files as $file ) {
-                       $m = array();
                        # Check that the base file name is part of the thumb name
                        # This is a basic sanity check to avoid erasing unrelated directories
                        if ( strpos( $file, $this->getName() ) !== false ) {
@@ -1243,7 +1241,6 @@ class LocalFileDeleteBatch {
                $dbw = $this->file->repo->getMasterDB();
                list( $oldRels, $deleteCurrent ) = $this->getOldRels();
                if ( $deleteCurrent ) {
-                       $where = array( 'img_name' => $this->file->getName() );
                        $dbw->delete( 'image', array( 'img_name' => $this->file->getName() ), __METHOD__ );
                }
                if ( count( $oldRels ) ) {
@@ -1425,7 +1422,6 @@ class LocalFileRestoreBatch {
                        if ( $first && !$exists ) {
                                // This revision will be published as the new current version
                                $destRel = $this->file->getRel();
-                               $info = $this->file->repo->getFileProps( $deletedUrl );
                                $insertCurrent = array(
                                        'img_name'        => $row->fa_name,
                                        'img_size'        => $row->fa_size,
index 0e35df6..850a8d8 100644 (file)
@@ -207,7 +207,6 @@ class OldLocalFile extends LocalFile {
 
                $dbw = $this->repo->getMasterDB();
                list( $major, $minor ) = self::splitMime( $this->mime );
-               $mime = $this->mime;
 
                wfDebug(__METHOD__.': upgrading '.$this->archive_name." to the current schema\n");
                $dbw->update( 'oldimage',
index 89096a5..23d222a 100644 (file)
@@ -135,7 +135,7 @@ class RepoGroup {
 
        function getFileProps( $fileName ) {
                if ( FileRepo::isVirtualUrl( $fileName ) ) {
-                       list( $repoName, $zone, $rel ) = $this->splitVirtualUrl( $fileName );
+                       list( $repoName, /* $zone */, /* $rel */ ) = $this->splitVirtualUrl( $fileName );
                        if ( $repoName === '' ) {
                                $repoName = 'local';
                        }
index b97fa04..145054c 100644 (file)
@@ -207,6 +207,7 @@ abstract class MediaHandler {
                global $wgLang;
                $nbytes = '(' . wfMsgExt( 'nbytes', array( 'parsemag', 'escape' ),
                        $wgLang->formatNum( $file->getSize() ) ) . ')';
+               return "($nbytes)";
        }
 
        function getLongDesc( $file ) {