Fix some PHPStorm inspection warnings in includes/api

Notably:
* In ApiManageTags, I used a switch instead of a dynamic function name,
  so that the call graph will be correct.
* In ApiImageRotate, checkTitleUserPermissions() has always returned
  void, this was an error introduced in 4e6810e4a2

Change-Id: Iea22616b8e7e2e0cc804619a54f8690898b2cb82

includes/api/ApiAuthManagerHelper.php

@@ -148,9 +148,12 @@ class ApiAuthManagerHelper {
 			$wantedRequests = [ $params['request'] => true ];
 		}
 		if ( $wantedRequests !== null ) {
-			$reqs = array_filter( $reqs, function ( $req ) use ( $wantedRequests ) {
-				return isset( $wantedRequests[$req->getUniqueId()] );
-			} );
+			$reqs = array_filter(
+				$reqs,
+				function ( AuthenticationRequest $req ) use ( $wantedRequests ) {
+					return isset( $wantedRequests[$req->getUniqueId()] );
+				}
+			);
 		}
 
 		// Collect the fields for all the requests

includes/api/ApiCSPReport.php

@@ -21,6 +21,7 @@
  */
 
 use MediaWiki\Logger\LoggerFactory;
+use Psr\Log\LoggerInterface;
 
 /**
  * Api module to receive and log CSP violation reports
@@ -29,6 +30,7 @@ use MediaWiki\Logger\LoggerFactory;
  */
 class ApiCSPReport extends ApiBase {
 
+	/** @var LoggerInterface */
 	private $log;
 
 	/**

includes/api/ApiComparePages.php

@@ -38,7 +38,9 @@ class ApiComparePages extends ApiBase {
 	/** @var \MediaWiki\Revision\SlotRoleRegistry */
 	private $slotRoleRegistry;
 
-	private $guessedTitle = false, $props;
+	/** @var Title|false */
+	private $guessedTitle = false;
+	private $props;
 
 	/** @var IContentHandlerFactory */
 	private $contentHandlerFactory;

includes/api/ApiEditPage.php

@@ -387,7 +387,6 @@ class ApiEditPage extends ApiBase {
 		$ep->setApiEditOverride( true );
 		$ep->setContextTitle( $titleObj );
 		$ep->importFormData( $req );
-		$content = $ep->textbox1;
 
 		// Do the actual save
 		$oldRevId = $articleObject->getRevIdFetched();

includes/api/ApiHelp.php

@@ -192,6 +192,7 @@ class ApiHelp extends ApiBase {
 		$doc = $formatter->getDoc();
 		$xpath = new DOMXPath( $doc );
 		$nodes = $xpath->query( '//a[@href][not(contains(@class,\'apihelp-linktrail\'))]' );
+		/** @var DOMElement $node */
 		foreach ( $nodes as $node ) {
 			$href = $node->getAttribute( 'href' );
 			do {

includes/api/ApiImageRotate.php

@@ -83,15 +83,7 @@ class ApiImageRotate extends ApiBase {
 			}
 
 			// Check whether we're allowed to rotate this file
-			$permError = $this->checkTitleUserPermissions( $file->getTitle(), [ 'edit', 'upload' ] );
-			if ( $permError ) {
-				$r['result'] = 'Failure';
-				$r['errors'] = $this->getErrorFormatter()->arrayFromStatus(
-					$this->errorArrayToStatus( $permError )
-				);
-				$result[] = $r;
-				continue;
-			}
+			$this->checkTitleUserPermissions( $file->getTitle(), [ 'edit', 'upload' ] );
 
 			$srcPath = $file->getLocalRefPath();
 			if ( $srcPath === false ) {

includes/api/ApiMain.php

@@ -170,7 +170,6 @@ class ApiMain extends ApiBase {
 	 * @param IContextSource|WebRequest|null $context If this is an instance of
 	 *    FauxRequest, errors are thrown and no printing occurs
 	 * @param bool $enableWrite Should be set to true if the api may modify data
-	 * @suppress PhanUndeclaredMethod
 	 */
 	public function __construct( $context = null, $enableWrite = false ) {
 		if ( $context === null ) {
@@ -181,10 +180,11 @@ class ApiMain extends ApiBase {
 			$context = RequestContext::getMain();
 		}
 		// We set a derivative context so we can change stuff later
-		$this->setContext( new DerivativeContext( $context ) );
+		$derivativeContext = new DerivativeContext( $context );
+		$this->setContext( $derivativeContext );
 
 		if ( isset( $request ) ) {
-			$this->getContext()->setRequest( $request );
+			$derivativeContext->setRequest( $request );
 		} else {
 			$request = $this->getRequest();
 		}
@@ -203,7 +203,7 @@ class ApiMain extends ApiBase {
 				global $wgUser;
 				wfDebug( "API: stripping user credentials when the same-origin policy is not applied\n" );
 				$wgUser = new User();
-				$this->getContext()->setUser( $wgUser );
+				$derivativeContext->setUser( $wgUser );
 				$request->response()->header( 'MediaWiki-Login-Suppressed: true' );
 			}
 		}
@@ -226,10 +226,10 @@ class ApiMain extends ApiBase {
 				$uselang = MediaWikiServices::getInstance()->getContentLanguage()->getCode();
 			}
 			$code = RequestContext::sanitizeLangCode( $uselang );
-			$this->getContext()->setLanguage( $code );
+			$derivativeContext->setLanguage( $code );
 			if ( !$this->mInternalMode ) {
 				global $wgLang;
-				$wgLang = $this->getContext()->getLanguage();
+				$wgLang = $derivativeContext->getLanguage();
 				RequestContext::getMain()->setLanguage( $wgLang );
 			}
 		}
@@ -677,7 +677,6 @@ class ApiMain extends ApiBase {
 		$request = $this->getRequest();
 		$response = $request->response();
 
-		$matchedOrigin = false;
 		$allowTiming = false;
 		$varyOrigin = true;
 

includes/api/ApiManageTags.php

@@ -47,15 +47,27 @@ class ApiManageTags extends ApiBase {
 		}
 
 		$result = $this->getResult();
-		$funcName = "{$params['operation']}TagWithChecks";
-		$status = ChangeTags::$funcName(
-			$params['tag'],
-			$params['reason'],
-			$user,
-			$params['ignorewarnings'],
-			$params['tags'] ?: []
-		);
-
+		$tag = $params['tag'];
+		$reason = $params['reason'];
+		$ignoreWarnings = $params['ignorewarnings'];
+		$tags = $params['tags'] ?: [];
+		switch ( $params['operation'] ) {
+			case 'create':
+				$status = ChangeTags::createTagWithChecks( $tag, $reason, $user, $ignoreWarnings, $tags );
+				break;
+			case 'delete':
+				$status = ChangeTags::deleteTagWithChecks( $tag, $reason, $user, $ignoreWarnings, $tags );
+				break;
+			case 'activate':
+				$status = ChangeTags::activateTagWithChecks( $tag, $reason, $user, $ignoreWarnings, $tags );
+				break;
+			case 'deactivate':
+				$status = ChangeTags::deactivateTagWithChecks( $tag, $reason, $user, $ignoreWarnings, $tags );
+				break;
+			default:
+				// unreachable
+				throw new \UnexpectedValueException( 'invalid operation' );
+		}
 		if ( !$status->isOK() ) {
 			$this->dieStatus( $status );
 		}

includes/api/ApiMove.php

@@ -216,6 +216,7 @@ class ApiMove extends ApiBase {
 		// At least some pages could be moved
 		// Report each of them separately
 		foreach ( $result->getValue() as $oldTitle => $status ) {
+			/** @var Status $status */
 			$r = [ 'from' => $oldTitle ];
 			if ( $status->isOK() ) {
 				$r['to'] = $status->getValue();

includes/api/ApiOpenSearch.php

@@ -71,7 +71,8 @@ class ApiOpenSearch extends ApiBase {
 
 			case 'xml':
 				$printer = $this->getMain()->createPrinterByName( 'xml' . $this->fm );
-				'@phan-var ApiFormatXML $printer';
+				'@phan-var ApiFormatXml $printer';
+				/** @var ApiFormatXml $printer */
 				$printer->setRootElement( 'SearchSuggestion' );
 				return $printer;
 

includes/api/ApiPageSet.php

@@ -415,7 +415,7 @@ class ApiPageSet extends ApiBase {
 
 	/**
 	 * Title objects for good and missing titles.
-	 * @return array
+	 * @return Title[]
 	 */
 	public function getGoodAndMissingTitles() {
 		return $this->mGoodTitles + $this->mMissingTitles;
@@ -1117,6 +1117,7 @@ class ApiPageSet extends ApiBase {
 
 		if ( $this->mPendingRedirectSpecialPages ) {
 			foreach ( $this->mPendingRedirectSpecialPages as $key => list( $from, $to ) ) {
+				/** @var Title $from */
 				$fromKey = $from->getPrefixedText();
 				$this->mResolvedRedirectTitles[$fromKey] = $from;
 				$this->mRedirectTitles[$fromKey] = $to;
@@ -1163,7 +1164,6 @@ class ApiPageSet extends ApiBase {
 	 * @return LinkBatch
 	 */
 	private function processTitlesArray( $titles ) {
-		$usernames = [];
 		$linkBatch = new LinkBatch();
 		$services = MediaWikiServices::getInstance();
 		$contLang = $services->getContentLanguage();
@@ -1172,6 +1172,7 @@ class ApiPageSet extends ApiBase {
 		foreach ( $titles as $index => $title ) {
 			if ( is_string( $title ) ) {
 				try {
+					/** @var Title $titleObj */
 					$titleObj = Title::newFromTextThrow( $title, $this->mDefaultNamespace );
 				} catch ( MalformedTitleException $ex ) {
 					// Handle invalid titles gracefully

includes/api/ApiParamInfo.php

@@ -26,6 +26,8 @@
 class ApiParamInfo extends ApiBase {
 
 	private $helpFormat;
+
+	/** @var RequestContext */
 	private $context;
 
 	public function __construct( ApiMain $main, $action ) {

includes/api/ApiQueryBlockInfoTrait.php

@@ -83,6 +83,7 @@ trait ApiQueryBlockInfoTrait {
 
 	/**
 	 * @see IContextSource::getUser
+	 * @return User
 	 */
 	abstract public function getUser();
 

includes/api/ApiQueryFileRepoInfo.php

@@ -51,12 +51,14 @@ class ApiQueryFileRepoInfo extends ApiQueryBase {
 		$repoGroup = $this->getInitialisedRepoGroup();
 		$foreignTargets = $conf->get( 'ForeignUploadTargets' );
 
-		$repoGroup->forEachForeignRepo( function ( $repo ) use ( &$repos, $props, $foreignTargets ) {
-			$repoProps = $repo->getInfo();
-			$repoProps['canUpload'] = in_array( $repoProps['name'], $foreignTargets );
+		$repoGroup->forEachForeignRepo(
+			function ( FileRepo $repo ) use ( &$repos, $props, $foreignTargets ) {
+				$repoProps = $repo->getInfo();
+				$repoProps['canUpload'] = in_array( $repoProps['name'], $foreignTargets );
 
-			$repos[] = array_intersect_key( $repoProps, $props );
-		} );
+				$repos[] = array_intersect_key( $repoProps, $props );
+			}
+		);
 
 		$localInfo = $repoGroup->getLocalRepo()->getInfo();
 		$localInfo['canUpload'] = $conf->get( 'EnableUploads' );
@@ -90,7 +92,7 @@ class ApiQueryFileRepoInfo extends ApiQueryBase {
 		$props = [];
 		$repoGroup = $this->getInitialisedRepoGroup();
 
-		$repoGroup->forEachForeignRepo( function ( $repo ) use ( &$props ) {
+		$repoGroup->forEachForeignRepo( function ( FileRepo $repo ) use ( &$props ) {
 			$props = array_merge( $props, array_keys( $repo->getInfo() ) );
 		} );
 

includes/api/ApiQueryLogEvents.php

@@ -31,6 +31,7 @@ use MediaWiki\Storage\NameTableAccessException;
  */
 class ApiQueryLogEvents extends ApiQueryBase {
 
+	/** @var CommentStore */
 	private $commentStore;
 
 	public function __construct( ApiQuery $query, $moduleName ) {

includes/api/ApiQueryRecentChanges.php

@@ -37,6 +37,7 @@ class ApiQueryRecentChanges extends ApiQueryGeneratorBase {
 		parent::__construct( $query, $moduleName, 'rc' );
 	}
 
+	/** @var CommentStore */
 	private $commentStore;
 
 	private $fld_comment = false, $fld_parsedcomment = false, $fld_user = false, $fld_userid = false,

includes/api/ApiQueryRevisionsBase.php

@@ -421,6 +421,7 @@ abstract class ApiQueryRevisionsBase extends ApiQueryGeneratorBase {
 				// @todo Move this into extractSlotInfo() (and remove its $content parameter)
 				// when extractDeprecatedContent() is no more.
 				if ( $content ) {
+					/** @var Content $content */
 					$vals['slots'][$role]['contentmodel'] = $content->getModel();
 					$vals['slots'][$role]['contentformat'] = $content->getDefaultFormat();
 					ApiResult::setContentValue(

includes/api/ApiQueryUserContribs.php

@@ -36,7 +36,10 @@ class ApiQueryUserContribs extends ApiQueryBase {
 		parent::__construct( $query, $moduleName, 'uc' );
 	}
 
-	private $params, $multiUserMode, $orderBy, $parentLens, $commentStore;
+	private $params, $multiUserMode, $orderBy, $parentLens;
+
+	/** @var CommentStore */
+	private $commentStore;
 
 	private $fld_ids = false, $fld_title = false, $fld_timestamp = false,
 		$fld_comment = false, $fld_parsedcomment = false, $fld_flags = false,

includes/api/ApiQueryUsers.php

@@ -98,8 +98,6 @@ class ApiQueryUsers extends ApiQueryBase {
 
 	public function execute() {
 		$db = $this->getDB();
-		$commentStore = CommentStore::getStore();
-
 		$params = $this->extractRequestParams();
 		$this->requireMaxOneParameter( $params, 'userids', 'users' );
 

includes/api/ApiQueryWatchlist.php

@@ -190,6 +190,7 @@ class ApiQueryWatchlist extends ApiQueryGeneratorBase {
 			$nsInfo = $services->getNamespaceInfo();
 			$usernames = [];
 			foreach ( $items as list( $watchedItem, $recentChangeInfo ) ) {
+				/** @var WatchedItem $watchedItem */
 				$linkTarget = $watchedItem->getLinkTarget();
 				if ( $nsInfo->hasGenderDistinction( $linkTarget->getNamespace() ) ) {
 					$usernames[] = $linkTarget->getText();

includes/api/ApiRemoveAuthenticationData.php

@@ -20,6 +20,7 @@
  * @file
  */
 
+use MediaWiki\Auth\AuthenticationRequest;
 use MediaWiki\Auth\AuthManager;
 
 /**
@@ -61,7 +62,7 @@ class ApiRemoveAuthenticationData extends ApiBase {
 			: [];
 		$reqs = array_filter(
 			$manager->getAuthenticationRequests( $this->authAction, $this->getUser() ),
-			function ( $req ) use ( $params, $blacklist ) {
+			function ( AuthenticationRequest $req ) use ( $params, $blacklist ) {
 				return $req->getUniqueId() === $params['request'] &&
 					!isset( $blacklist[get_class( $req )] );
 			}

includes/api/ApiRevisionDelete.php

@@ -125,7 +125,7 @@ class ApiRevisionDelete extends ApiBase {
 		$result->addValue( null, $this->getModuleName(), $data );
 	}
 
-	private function extractStatusInfo( $status ) {
+	private function extractStatusInfo( Status $status ) {
 		$ret = [
 			'status' => $status->isOK() ? 'Success' : 'Fail',
 		];

includes/api/Validator/ApiParamValidatorCallbacks.php

@@ -77,6 +77,7 @@ class ApiParamValidatorCallbacks implements Callbacks {
 	public function recordCondition(
 		DataMessageValue $message, $name, $value, array $settings, array $options
 	) {
+		/** @var \ApiBase $module */
 		$module = $options['module'];
 
 		$code = $message->getCode();

includes/api/Validator/SubmoduleDef.php

@@ -85,6 +85,7 @@ class SubmoduleDef extends EnumDef {
 
 	public function getParamInfo( $name, array $settings, array $options ) {
 		$info = parent::getParamInfo( $name, $settings, $options );
+		/** @var ApiBase $module */
 		$module = $options['module'];
 
 		if ( isset( $settings[self::PARAM_SUBMODULE_MAP] ) ) {