From 06c1a5976cf7733e9bee5ca673b658b6b88cb9f2 Mon Sep 17 00:00:00 2001 From: Thiemo Kreuz Date: Thu, 28 Feb 2019 12:13:49 +0100 Subject: [PATCH] maintenance: Deprecate Maintenance::hasArg/getArg with no param Benefit of keeping the parameter optional: - In maintenance scripts that really only have one parameter, it's a little more convenient to be able to ask for *the* parameter via an empty getArg(). Disadvantages: - It's unclear what getArg() means when there is no indication *which* argument the code asks for. This might as well return the last argument, or an array of all arguments. - In scripts with two or more arguments, it's confusing to see getArg( 1 ) next to an empty getArg(). - The methods are more complex and a bit more complicated to use with the extra feature of this parameter being optional. Users need to look up what the default is to be able to use it safely. Change-Id: I22a43bfdfc0f0c9ffdb468c13aba73b888d1f15e --- RELEASE-NOTES-1.33 | 2 ++ maintenance/Maintenance.php | 8 ++++++++ maintenance/benchmarks/benchmarkParse.php | 2 +- maintenance/cleanupSpam.php | 2 +- maintenance/deleteBatch.php | 4 ++-- maintenance/edit.php | 2 +- maintenance/importDump.php | 4 ++-- maintenance/jsparse.php | 2 +- maintenance/mctest.php | 4 ++-- maintenance/moveBatch.php | 4 ++-- maintenance/nukePage.php | 2 +- maintenance/pageExists.php | 2 +- maintenance/protect.php | 2 +- maintenance/storage/dumpRev.php | 2 +- maintenance/undelete.php | 2 +- maintenance/view.php | 2 +- 16 files changed, 28 insertions(+), 18 deletions(-) diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33 index ddd6da96d3..ed8b5f864a 100644 --- a/RELEASE-NOTES-1.33 +++ b/RELEASE-NOTES-1.33 @@ -409,6 +409,8 @@ because of Phabricator reports. instead. The setTags() method was overriding the tags, addTags() doesn't override, only adds new tags. * Block::isValid is deprecated, since it is no longer needed in core. +* Calling Maintenance::hasArg() as well as Maintenance::getArg() with no + parameter has been deprecated. Please pass the argument number 0. === Other changes in 1.33 === * (T201747) Html::openElement() warns if given an element name with a space diff --git a/maintenance/Maintenance.php b/maintenance/Maintenance.php index 3476a32a6a..7430e1e0c4 100644 --- a/maintenance/Maintenance.php +++ b/maintenance/Maintenance.php @@ -324,6 +324,10 @@ abstract class Maintenance { * @return bool */ protected function hasArg( $argId = 0 ) { + if ( func_num_args() === 0 ) { + wfDeprecated( __METHOD__ . ' without an $argId', '1.33' ); + } + return isset( $this->mArgs[$argId] ); } @@ -334,6 +338,10 @@ abstract class Maintenance { * @return mixed */ protected function getArg( $argId = 0, $default = null ) { + if ( func_num_args() === 0 ) { + wfDeprecated( __METHOD__ . ' without an $argId', '1.33' ); + } + return $this->hasArg( $argId ) ? $this->mArgs[$argId] : $default; } diff --git a/maintenance/benchmarks/benchmarkParse.php b/maintenance/benchmarks/benchmarkParse.php index c42fcd9c34..a3a49740ee 100644 --- a/maintenance/benchmarks/benchmarkParse.php +++ b/maintenance/benchmarks/benchmarkParse.php @@ -75,7 +75,7 @@ class BenchmarkParse extends Maintenance { // Set as a member variable to avoid function calls when we're timing the parse $this->linkCache = MediaWikiServices::getInstance()->getLinkCache(); - $title = Title::newFromText( $this->getArg() ); + $title = Title::newFromText( $this->getArg( 0 ) ); if ( !$title ) { $this->error( "Invalid title" ); exit( 1 ); diff --git a/maintenance/cleanupSpam.php b/maintenance/cleanupSpam.php index 17d2e188d2..04f278f389 100644 --- a/maintenance/cleanupSpam.php +++ b/maintenance/cleanupSpam.php @@ -52,7 +52,7 @@ class CleanupSpam extends Maintenance { // Hack: Grant bot rights so we don't flood RecentChanges $wgUser->addGroup( 'bot' ); - $spec = $this->getArg(); + $spec = $this->getArg( 0 ); $protConds = []; foreach ( [ 'http://', 'https://' ] as $prot ) { diff --git a/maintenance/deleteBatch.php b/maintenance/deleteBatch.php index fe3bea05de..4f9e488343 100644 --- a/maintenance/deleteBatch.php +++ b/maintenance/deleteBatch.php @@ -69,8 +69,8 @@ class DeleteBatch extends Maintenance { } $wgUser = $user; - if ( $this->hasArg() ) { - $file = fopen( $this->getArg(), 'r' ); + if ( $this->hasArg( 0 ) ) { + $file = fopen( $this->getArg( 0 ), 'r' ); } else { $file = $this->getStdin(); } diff --git a/maintenance/edit.php b/maintenance/edit.php index d4a7befd75..3609cf2249 100644 --- a/maintenance/edit.php +++ b/maintenance/edit.php @@ -71,7 +71,7 @@ class EditCLI extends Maintenance { $wgUser->addToDatabase(); } - $title = Title::newFromText( $this->getArg() ); + $title = Title::newFromText( $this->getArg( 0 ) ); if ( !$title ) { $this->fatalError( "Invalid title" ); } diff --git a/maintenance/importDump.php b/maintenance/importDump.php index 7c20748c51..c2c5ccf40e 100644 --- a/maintenance/importDump.php +++ b/maintenance/importDump.php @@ -112,8 +112,8 @@ TEXT $this->setNsfilter( explode( '|', $this->getOption( 'namespaces' ) ) ); } - if ( $this->hasArg() ) { - $this->importFromFile( $this->getArg() ); + if ( $this->hasArg( 0 ) ) { + $this->importFromFile( $this->getArg( 0 ) ); } else { $this->importFromStdin(); } diff --git a/maintenance/jsparse.php b/maintenance/jsparse.php index 661ec9862b..aa5077d82d 100644 --- a/maintenance/jsparse.php +++ b/maintenance/jsparse.php @@ -38,7 +38,7 @@ class JSParseHelper extends Maintenance { } public function execute() { - if ( $this->hasArg() ) { + if ( $this->hasArg( 0 ) ) { $files = $this->mArgs; } else { $this->maybeHelp( true ); // @todo fixme this is a lame API :) diff --git a/maintenance/mctest.php b/maintenance/mctest.php index c976bd7048..98f0eb99a5 100644 --- a/maintenance/mctest.php +++ b/maintenance/mctest.php @@ -50,8 +50,8 @@ class McTest extends Maintenance { $this->fatalError( "MediaWiki isn't configured with a cache named '$cache'" ); } $servers = $wgObjectCaches[$cache]['servers']; - } elseif ( $this->hasArg() ) { - $servers = [ $this->getArg() ]; + } elseif ( $this->hasArg( 0 ) ) { + $servers = [ $this->getArg( 0 ) ]; } elseif ( $wgMainCacheType === CACHE_MEMCACHED ) { global $wgMemCachedServers; $servers = $wgMemCachedServers; diff --git a/maintenance/moveBatch.php b/maintenance/moveBatch.php index 9c20c67ded..47828e690f 100644 --- a/maintenance/moveBatch.php +++ b/maintenance/moveBatch.php @@ -65,8 +65,8 @@ class MoveBatch extends Maintenance { $reason = $this->getOption( 'r', '' ); $interval = $this->getOption( 'i', 0 ); $noredirects = $this->hasOption( 'noredirects' ); - if ( $this->hasArg() ) { - $file = fopen( $this->getArg(), 'r' ); + if ( $this->hasArg( 0 ) ) { + $file = fopen( $this->getArg( 0 ), 'r' ); } else { $file = $this->getStdin(); } diff --git a/maintenance/nukePage.php b/maintenance/nukePage.php index baead9476c..ba0fe5da5d 100644 --- a/maintenance/nukePage.php +++ b/maintenance/nukePage.php @@ -39,7 +39,7 @@ class NukePage extends Maintenance { } public function execute() { - $name = $this->getArg(); + $name = $this->getArg( 0 ); $delete = $this->hasOption( 'delete' ); $dbw = $this->getDB( DB_MASTER ); diff --git a/maintenance/pageExists.php b/maintenance/pageExists.php index 10d37de48c..6a5654f52d 100644 --- a/maintenance/pageExists.php +++ b/maintenance/pageExists.php @@ -32,7 +32,7 @@ class PageExists extends Maintenance { } public function execute() { - $titleArg = $this->getArg(); + $titleArg = $this->getArg( 0 ); $title = Title::newFromText( $titleArg ); $pageExists = $title && $title->exists(); diff --git a/maintenance/protect.php b/maintenance/protect.php index e2c1a8b340..1603ab0407 100644 --- a/maintenance/protect.php +++ b/maintenance/protect.php @@ -62,7 +62,7 @@ class Protect extends Maintenance { $this->fatalError( "Invalid username" ); } - $t = Title::newFromText( $this->getArg() ); + $t = Title::newFromText( $this->getArg( 0 ) ); if ( !$t ) { $this->fatalError( "Invalid title" ); } diff --git a/maintenance/storage/dumpRev.php b/maintenance/storage/dumpRev.php index b00db2083d..7d9883eb4f 100644 --- a/maintenance/storage/dumpRev.php +++ b/maintenance/storage/dumpRev.php @@ -39,7 +39,7 @@ class DumpRev extends Maintenance { } public function execute() { - $id = (int)$this->getArg(); + $id = (int)$this->getArg( 0 ); $lookup = MediaWikiServices::getInstance()->getRevisionLookup(); $rev = $lookup->getRevisionById( $id ); diff --git a/maintenance/undelete.php b/maintenance/undelete.php index e9b2abd0f9..144518e40a 100644 --- a/maintenance/undelete.php +++ b/maintenance/undelete.php @@ -37,7 +37,7 @@ class Undelete extends Maintenance { $user = $this->getOption( 'user', false ); $reason = $this->getOption( 'reason', '' ); - $pageName = $this->getArg(); + $pageName = $this->getArg( 0 ); $title = Title::newFromText( $pageName ); if ( !$title ) { diff --git a/maintenance/view.php b/maintenance/view.php index 24b5007745..671369aff9 100644 --- a/maintenance/view.php +++ b/maintenance/view.php @@ -36,7 +36,7 @@ class ViewCLI extends Maintenance { } public function execute() { - $title = Title::newFromText( $this->getArg() ); + $title = Title::newFromText( $this->getArg( 0 ) ); if ( !$title ) { $this->fatalError( "Invalid title" ); } -- 2.20.1