Un-blacklist PhanUndeclaredVariable
authorErik Bernhardson <ebernhardson@wikimedia.org>
Wed, 18 Jan 2017 00:48:59 +0000 (16:48 -0800)
committerErik Bernhardson <ebernhardson@wikimedia.org>
Wed, 18 Jan 2017 21:07:39 +0000 (13:07 -0800)
Undeclared variables are a very common error type that we want to catch
as often as possible. To avoid needing to refactor a variety of global
level code (mostly in old-style maintenance scripts) this ignores
undeclared variables in global scope. This is still a good improvement
over what was happening previously.

Change-Id: I50b41d571724244552074b9408abbdf6160aca59

12 files changed:
includes/api/ApiFeedWatchlist.php
includes/api/ApiUpload.php
includes/installer/Installer.php
includes/libs/StringUtils.php
includes/libs/rdbms/lbfactory/LBFactory.php
includes/libs/xmp/XMP.php
includes/parser/DateFormatter.php
includes/specials/pagers/MergeHistoryPager.php
includes/tidy/Balancer.php
maintenance/generateJsonI18n.php
maintenance/rebuildtextindex.php
tests/phan/config.php

index b9bb761..7f349bc 100644 (file)
@@ -52,6 +52,7 @@ class ApiFeedWatchlist extends ApiBase {
        public function execute() {
                $config = $this->getConfig();
                $feedClasses = $config->get( 'FeedClasses' );
+               $params = [];
                try {
                        $params = $this->extractRequestParams();
 
index f821374..6b38302 100644 (file)
@@ -637,7 +637,8 @@ class ApiUpload extends ApiBase {
                                break;
 
                        case UploadBase::HOOK_ABORTED:
-                               $this->dieWithError( $params, 'hookaborted', [ 'details' => $verification['error'] ] );
+                               $msg = $verification['error'] === '' ? 'hookaborted' : $verification['error'];
+                               $this->dieWithError( $msg, 'hookaborted', [ 'details' => $verification['error'] ] );
                                break;
                        default:
                                $this->dieWithError( 'apierror-unknownerror-nocode', 'unknown-error',
index 9dc8032..d887a13 100644 (file)
@@ -1421,6 +1421,7 @@ abstract class Installer {
                $wgAutoloadClasses += $data['autoload'];
 
                $hooksWeWant = isset( $wgHooks['LoadExtensionSchemaUpdates'] ) ?
+                       /** @suppress PhanUndeclaredVariable $wgHooks is set by DefaultSettings */
                        $wgHooks['LoadExtensionSchemaUpdates'] : [];
 
                if ( isset( $data['globals']['wgHooks']['LoadExtensionSchemaUpdates'] ) ) {
index 26f3c4a..cffb5a3 100644 (file)
@@ -168,6 +168,7 @@ class StringUtils {
        ) {
                $inputPos = 0;
                $outputPos = 0;
+               $contentPos = 0;
                $output = '';
                $foundStart = false;
                $encStart = preg_quote( $startDelim, '!' );
index 15a5c0d..77d4aa9 100644 (file)
@@ -100,7 +100,7 @@ abstract class LBFactory implements ILBFactory {
                                trigger_error( E_USER_WARNING, get_class( $e ) . ': ' . $e->getMessage() );
                        };
 
-               $this->profiler = isset( $params['profiler'] ) ? $params['profiler'] : null;
+               $this->profiler = isset( $conf['profiler'] ) ? $conf['profiler'] : null;
                $this->trxProfiler = isset( $conf['trxProfiler'] )
                        ? $conf['trxProfiler']
                        : new TransactionProfiler();
@@ -111,9 +111,9 @@ abstract class LBFactory implements ILBFactory {
                        'ChronologyProtection' => 'true'
                ];
 
-               $this->cliMode = isset( $params['cliMode'] ) ? $params['cliMode'] : PHP_SAPI === 'cli';
+               $this->cliMode = isset( $conf['cliMode'] ) ? $conf['cliMode'] : PHP_SAPI === 'cli';
                $this->hostname = isset( $conf['hostname'] ) ? $conf['hostname'] : gethostname();
-               $this->agent = isset( $params['agent'] ) ? $params['agent'] : '';
+               $this->agent = isset( $conf['agent'] ) ? $conf['agent'] : '';
 
                $this->ticket = mt_rand();
        }
index a657a33..0d171f5 100644 (file)
@@ -1086,7 +1086,7 @@ class XMPReader implements LoggerAwareInterface {
                                }
                        } else {
                                array_unshift( $this->mode, self::MODE_IGNORE );
-                               array_unshift( $this->curItem, $elm );
+                               array_unshift( $this->curItem, $ns . ' ' . $tag );
 
                                return;
                        }
index 40da368..0653725 100644 (file)
@@ -217,15 +217,17 @@ class DateFormatter {
                        }
                }
 
-               return $this->formatDate( $bits, $linked );
+               return $this->formatDate( $bits, $matches[0], $linked );
        }
 
        /**
         * @param array $bits
+        * @param string $orig Original input string, to be returned
+        *  on formatting failure.
         * @param bool $link
         * @return string
         */
-       public function formatDate( $bits, $link = true ) {
+       private function formatDate( $bits, $orig, $link = true ) {
                $format = $this->targets[$this->mTarget];
 
                if ( !$link ) {
@@ -300,8 +302,9 @@ class DateFormatter {
                        }
                }
                if ( $fail ) {
-                       /** @todo FIXME: $matches doesn't exist here, what's expected? */
-                       $text = $matches[0];
+                       // This occurs when parsing a date with day or month outside the bounds
+                       // of possibilities.
+                       $text = $orig;
                }
 
                $isoBits = [];
index 56229b3..bbf97e1 100644 (file)
@@ -54,15 +54,17 @@ class MergeHistoryPager extends ReverseChronologicalPager {
                $batch = new LinkBatch();
                # Give some pointers to make (last) links
                $this->mForm->prevId = [];
+               $rev_id = null;
                foreach ( $this->mResult as $row ) {
                        $batch->addObj( Title::makeTitleSafe( NS_USER, $row->user_name ) );
                        $batch->addObj( Title::makeTitleSafe( NS_USER_TALK, $row->user_name ) );
 
-                       $rev_id = isset( $rev_id ) ? $rev_id : $row->rev_id;
-                       if ( $rev_id > $row->rev_id ) {
-                               $this->mForm->prevId[$rev_id] = $row->rev_id;
-                       } elseif ( $rev_id < $row->rev_id ) {
-                               $this->mForm->prevId[$row->rev_id] = $rev_id;
+                       if ( isset( $rev_id ) ) {
+                               if ( $rev_id > $row->rev_id ) {
+                                       $this->mForm->prevId[$rev_id] = $row->rev_id;
+                               } elseif ( $rev_id < $row->rev_id ) {
+                                       $this->mForm->prevId[$row->rev_id] = $rev_id;
+                               }
                        }
 
                        $rev_id = $row->rev_id;
index 1346e1c..95cbe09 100644 (file)
@@ -1410,6 +1410,7 @@ class BalanceActiveFormattingElements {
        private $noahTableStack = [ [] ];
 
        public function __destruct() {
+               $next = null;
                for ( $node = $this->head; $node; $node = $next ) {
                        $next = $node->nextAFE;
                        $node->prevAFE = $node->nextAFE = $node->nextNoah = null;
index d3041d9..a84f2ae 100644 (file)
@@ -108,6 +108,7 @@ class GenerateJsonI18n extends Maintenance {
                if ( !is_readable( $phpfile ) ) {
                        $this->error( "Error reading $phpfile", 1 );
                }
+               $messages = null;
                include $phpfile;
                $phpfileContents = file_get_contents( $phpfile );
 
index 37636c8..b7f0762 100644 (file)
@@ -108,9 +108,8 @@ class RebuildTextIndex extends Maintenance {
                        );
 
                        foreach ( $res as $s ) {
+                               $title = Title::makeTitle( $s->page_namespace, $s->page_title );
                                try {
-                                       $title = Title::makeTitle( $s->page_namespace, $s->page_title );
-
                                        $rev = new Revision( $s );
                                        $content = $rev->getContent();
 
index cef03ee..17c1fb8 100644 (file)
@@ -203,7 +203,7 @@ return [
         * with complicated cross-file globals that you have no
         * hope of fixing.
         */
-       'ignore_undeclared_variables_in_global_scope' => false,
+       'ignore_undeclared_variables_in_global_scope' => true,
 
        /**
         * Set to true in order to attempt to detect dead
@@ -339,8 +339,6 @@ return [
                "PhanUndeclaredProperty",
                // approximate error count: 3
                "PhanUndeclaredStaticMethod",
-               // approximate error count: 79
-               "PhanUndeclaredVariable",
        ],
 
        /**