Cleanup some incorrect return annotations

Most of these are simply changing annotations to reflect
reality. If a function can return false to indicate failure
the @return should indicate it.

Some are fixing preg_match calls, preg match returns 1, 0 or false,
but the functions all claim to return booleans.

This is far from all the incorrect return types in mediawiki, there
are around 250 detected by phan, but have to start somewhere.

Change-Id: I1bbdfee6190747bde460f8a7084212ccafe169ef

includes/Category.php

@@ -168,7 +168,7 @@ class Category {
 	 * @param Title $title Optional title object for the category represented by
 	 *   the given row. May be provided if it is already known, to avoid having
 	 *   to re-create a title object later.
-	 * @return Category
+	 * @return Category|false
 	 */
 	public static function newFromRow( $row, $title = null ) {
 		$cat = new self();

includes/EditPage.php

@@ -295,7 +295,7 @@ class EditPage {
 	/** @var bool Has a summary been preset using GET parameter &summary= ? */
 	public $hasPresetSummary = false;
 
-	/** @var bool */
+	/** @var Revision|bool */
 	public $mBaseRevision = false;
 
 	/** @var bool */

includes/GitInfo.php

@@ -46,7 +46,7 @@ class GitInfo {
 	protected $cache = [];
 
 	/**
-	 * Map of repo URLs to viewer URLs. Access via static method getViewers().
+	 * @var array|false Map of repo URLs to viewer URLs. Access via static method getViewers().
 	 */
 	private static $viewers = false;
 

includes/GlobalFunctions.php

@@ -545,7 +545,7 @@ function wfAppendQuery( $url, $query ) {
  * @param string $url Either fully-qualified or a local path + query
  * @param string $defaultProto One of the PROTO_* constants. Determines the
  *    protocol to use if $url or $wgServer is protocol-relative
- * @return string Fully-qualified URL, current-path-relative URL or false if
+ * @return string|false Fully-qualified URL, current-path-relative URL or false if
  *    no valid URL can be constructed
  */
 function wfExpandUrl( $url, $defaultProto = PROTO_CURRENT ) {

includes/HistoryBlob.php

@@ -239,7 +239,7 @@ class HistoryBlobStub {
 	}
 
 	/**
-	 * @return string
+	 * @return string|false
 	 */
 	function getText() {
 		if ( isset( self::$blobCache[$this->mOldId] ) ) {

includes/OutputPage.php

@@ -2176,7 +2176,7 @@ class OutputPage extends ContextSource {
 	 * if there isn't one. This is used by Skin to determine whether to enable
 	 * JavaScript frame-breaking, for clients that don't support X-Frame-Options.
 	 *
-	 * @return string
+	 * @return string|false
 	 */
 	public function getFrameOptions() {
 		$config = $this->getConfig();

includes/PathRouter.php

@@ -364,7 +364,7 @@ class PathRouterPatternReplacer {
 	 * difference between a $1 that was not replaced and a $1 that was part of
 	 * the content a $1 was replaced with.
 	 * @param string $value
-	 * @return string
+	 * @return string|false
 	 */
 	public function replace( $value ) {
 		$this->error = false;

includes/ProtectionForm.php

@@ -148,7 +148,7 @@ class ProtectionForm {
 	 *
 	 * @param string $action
 	 *
-	 * @return string 14-char timestamp or "infinity", or false if the input was invalid
+	 * @return string|false 14-char timestamp or "infinity", or false if the input was invalid
 	 */
 	function getExpiry( $action ) {
 		if ( $this->mExpirySelection[$action] == 'existing' ) {

includes/Revision.php

@@ -874,7 +874,7 @@ class Revision implements IDBAccessObject {
 	/**
 	 * Fetch revision's user id without regard for the current user's permissions
 	 *
-	 * @return string
+	 * @return int
 	 * @deprecated since 1.25, use getUser( Revision::RAW )
 	 */
 	public function getRawUser() {
@@ -1269,7 +1269,7 @@ class Revision implements IDBAccessObject {
 	 *   (same as the the wiki $row was loaded from) or false to indicate the local
 	 *   wiki (this is the default). Otherwise, it must be a symbolic wiki database
 	 *   identifier as understood by the LoadBalancer class.
-	 * @return string Text the text requested or false on failure
+	 * @return string|false Text the text requested or false on failure
 	 */
 	public static function getRevisionText( $row, $prefix = 'old_', $wiki = false ) {
 

includes/SiteConfiguration.php

@@ -274,7 +274,7 @@ class SiteConfiguration {
 	 * @param string $from
 	 * @param string $to
 	 * @param string|array $in
-	 * @return string
+	 * @return string|array
 	 */
 	function doReplace( $from, $to, $in ) {
 		if ( is_string( $in ) ) {

includes/Title.php

@@ -835,7 +835,7 @@ class Title implements LinkTarget {
 	/**
 	 * Returns the DB name of the distant wiki which owns the object.
 	 *
-	 * @return string The DB name
+	 * @return string|false The DB name
 	 */
 	public function getTransWikiID() {
 		if ( !$this->isExternal() ) {
@@ -974,7 +974,7 @@ class Title implements LinkTarget {
 	/**
 	 * Get the namespace text
 	 *
-	 * @return string Namespace text
+	 * @return string|false Namespace text
 	 */
 	public function getNsText() {
 		if ( $this->isExternal() ) {
@@ -4436,7 +4436,7 @@ class Title implements LinkTarget {
 	 * Get the last touched timestamp
 	 *
 	 * @param IDatabase $db Optional db
-	 * @return string Last-touched timestamp
+	 * @return string|false Last-touched timestamp
 	 */
 	public function getTouched( $db = null ) {
 		if ( $db === null ) {

includes/WebRequest.php

@@ -238,7 +238,7 @@ class WebRequest {
 	 * This is for use prior to Setup.php, when no WebRequest object is available.
 	 * At other times, use the non-static function getProtocol().
 	 *
-	 * @return array
+	 * @return string
 	 */
 	public static function detectProtocol() {
 		if ( ( !empty( $_SERVER['HTTPS'] ) && $_SERVER['HTTPS'] !== 'off' ) ||

includes/WikiMap.php

@@ -141,7 +141,7 @@ class WikiMap {
 	 * @param string $wikiID Wiki'd id (generally database name)
 	 * @param string $page Page name (must be normalised before calling this function!)
 	 * @param string $text Link's text; optional, default to $page
-	 * @return string HTML link or false if the wiki was not found
+	 * @return string|false HTML link or false if the wiki was not found
 	 */
 	public static function makeForeignLink( $wikiID, $page, $text = null ) {
 		if ( !$text ) {

includes/api/ApiBase.php

@@ -2229,7 +2229,7 @@ abstract class ApiBase extends ContextSource {
 	 * "apihelp-{$this->getModulePath()}-description".
 	 *
 	 * @deprecated since 1.25
-	 * @return Message|string|array
+	 * @return Message|string|array|false
 	 */
 	protected function getDescription() {
 		return false;

includes/api/ApiParamInfo.php

@@ -237,7 +237,7 @@ class ApiParamInfo extends ApiBase {
 
 	/**
 	 * @param ApiBase $module
-	 * @return ApiResult
+	 * @return array
 	 */
 	private function getModuleInfo( $module ) {
 		$ret = [];

includes/api/ApiQueryBase.php

@@ -584,7 +584,7 @@ abstract class ApiQueryBase extends ApiBase {
 	 * @return bool
 	 */
 	public function validateSha1Hash( $hash ) {
-		return preg_match( '/^[a-f0-9]{40}$/', $hash );
+		return (bool)preg_match( '/^[a-f0-9]{40}$/', $hash );
 	}
 
 	/**
@@ -592,7 +592,7 @@ abstract class ApiQueryBase extends ApiBase {
 	 * @return bool
 	 */
 	public function validateSha1Base36Hash( $hash ) {
-		return preg_match( '/^[a-z0-9]{31}$/', $hash );
+		return (bool)preg_match( '/^[a-z0-9]{31}$/', $hash );
 	}
 
 	/**

includes/api/ApiQueryCategoryMembers.php

@@ -53,7 +53,7 @@ class ApiQueryCategoryMembers extends ApiQueryGeneratorBase {
 	 */
 	private function validateHexSortkey( $hexSortkey ) {
 		// A hex sortkey has an unbound number of 2 letter pairs
-		return preg_match( '/^(?:[a-fA-F0-9]{2})*$/D', $hexSortkey );
+		return (bool)preg_match( '/^(?:[a-fA-F0-9]{2})*$/D', $hexSortkey );
 	}
 
 	/**

includes/api/ApiStashEdit.php

@@ -168,7 +168,7 @@ class ApiStashEdit extends ApiBase {
 	 * @param Content $content Edit content
 	 * @param User $user
 	 * @param string $summary Edit summary
-	 * @return integer ApiStashEdit::ERROR_* constant
+	 * @return string ApiStashEdit::ERROR_* constant
 	 * @since 1.25
 	 */
 	public static function parseAndStash( WikiPage $page, Content $content, User $user, $summary ) {

includes/cache/FileCacheBase.php

@@ -154,7 +154,7 @@ abstract class FileCacheBase {
 	/**
 	 * Save and compress text to the cache
 	 * @param string $text
-	 * @return string Compressed text
+	 * @return string|false Compressed text
 	 */
 	public function saveText( $text ) {
 		if ( $this->useGzip() ) {

includes/cache/localisation/LocalisationCache.php

@@ -313,7 +313,7 @@ class LocalisationCache {
 	 * array.
 	 * @param string $code
 	 * @param string $key
-	 * @return bool|null|string
+	 * @return bool|null|string|string[]
 	 */
 	public function getSubitemList( $code, $key ) {
 		if ( in_array( $key, self::$splitKeys ) ) {

includes/db/DatabaseMssql.php

@@ -332,7 +332,7 @@ class DatabaseMssql extends Database {
 	}
 
 	/**
-	 * @return string
+	 * @return string|int
 	 */
 	public function lastErrno() {
 		$err = sqlsrv_errors( SQLSRV_ERR_ALL );

includes/db/DatabaseOracle.php

@@ -993,7 +993,7 @@ class DatabaseOracle extends Database {
 	 *
 	 * @param array|string $table
 	 * @param string $field
-	 * @return ORAField|ORAResult
+	 * @return ORAField|ORAResult|false
 	 */
 	private function fieldInfoMulti( $table, $field ) {
 		$field = strtoupper( $field );

includes/debug/logger/LegacyLogger.php

@@ -378,7 +378,7 @@ class LegacyLogger extends AbstractLogger {
 			if ( is_nan( $item ) ) {
 				return 'NaN';
 			}
-			return $item;
+			return (string)$item;
 		}
 
 		if ( is_scalar( $item ) ) {

includes/diff/DairikiDiff.php

@@ -67,7 +67,7 @@ abstract class DiffOp {
 
 	/**
 	 * @param int $i
-	 * @return string|null
+	 * @return string[]|string|null
 	 */
 	public function getClosing( $i = null ) {
 		if ( $i === null ) {

includes/diff/DifferenceEngine.php

@@ -210,7 +210,7 @@ class DifferenceEngine extends ContextSource {
 		if ( $link ) {
 			return "[$link $id]";
 		} else {
-			return $id;
+			return (string)$id;
 		}
 	}
 

includes/filerepo/FileRepo.php

@@ -99,7 +99,7 @@ class FileRepo {
 	 */
 	protected $pathDisclosureProtection = 'simple';
 
-	/** @var bool Public zone URL. */
+	/** @var string|false Public zone URL. */
 	protected $url;
 
 	/** @var string The base thumbnail URL. Defaults to "<url>/thumb". */
@@ -309,7 +309,7 @@ class FileRepo {
 	 * @return bool Whether non-ASCII path characters are allowed
 	 */
 	public function backendSupportsUnicodePaths() {
-		return ( $this->getBackend()->getFeatures() & FileBackend::ATTR_UNICODE_PATHS );
+		return (bool)( $this->getBackend()->getFeatures() & FileBackend::ATTR_UNICODE_PATHS );
 	}
 
 	/**
@@ -737,7 +737,7 @@ class FileRepo {
 	 * constructor, whereas local repositories use the local Title functions.
 	 *
 	 * @param string $name
-	 * @return string
+	 * @return string|false
 	 */
 	public function getDescriptionUrl( $name ) {
 		$encName = wfUrlencode( $name );
@@ -771,7 +771,7 @@ class FileRepo {
 	 *
 	 * @param string $name Name of image to fetch
 	 * @param string $lang Language to fetch it in, if any.
-	 * @return string
+	 * @return string|false
 	 */
 	public function getDescriptionRenderUrl( $name, $lang = null ) {
 		$query = 'action=render';

includes/filerepo/ForeignAPIRepo.php

@@ -109,7 +109,7 @@ class ForeignAPIRepo extends FileRepo {
 	 *
 	 * @param Title $title
 	 * @param string|bool $time
-	 * @return File
+	 * @return File|false
 	 */
 	function newFile( $title, $time = false ) {
 		if ( $time ) {

includes/filerepo/RepoGroup.php

@@ -334,7 +334,7 @@ class RepoGroup {
 	/**
 	 * Get the repo instance by its name
 	 * @param string $name
-	 * @return bool
+	 * @return FileRepo|bool
 	 */
 	function getRepoByName( $name ) {
 		if ( !$this->reposInitialised ) {

includes/filerepo/file/ArchivedFile.php

@@ -81,7 +81,7 @@ class ArchivedFile {
 	/** @var string SHA-1 hash of file content */
 	private $sha1;
 
-	/** @var string Number of pages of a multipage document, or false for
+	/** @var int|false Number of pages of a multipage document, or false for
 	 * documents which aren't multipage documents
 	 */
 	private $pageCount;
@@ -496,7 +496,7 @@ class ArchivedFile {
 	 * Return the user name of the uploader.
 	 *
 	 * @deprecated since 1.23 Use getUser( 'text' ) instead.
-	 * @return string
+	 * @return string|int
 	 */
 	public function getUserText() {
 		wfDeprecated( __METHOD__, '1.23' );
@@ -511,7 +511,7 @@ class ArchivedFile {
 	/**
 	 * Return upload description.
 	 *
-	 * @return string
+	 * @return string|int
 	 */
 	public function getDescription() {
 		$this->load();

includes/filerepo/file/File.php

@@ -127,7 +127,7 @@ abstract class File implements IDBAccessObject {
 	/** @var string Relative path including trailing slash */
 	protected $hashPath;
 
-	/** @var string Number of pages of a multipage document, or false for
+	/** @var string|false Number of pages of a multipage document, or false for
 	 *    documents which aren't multipage documents
 	 */
 	protected $pageCount;
@@ -535,7 +535,7 @@ abstract class File implements IDBAccessObject {
 	/**
 	 * Get the duration of a media file in seconds
 	 *
-	 * @return int
+	 * @return float|int
 	 */
 	public function getLength() {
 		$handler = $this->getHandler();
@@ -909,7 +909,7 @@ abstract class File implements IDBAccessObject {
 	 *
 	 * @param array $handlerParams
 	 *
-	 * @return string
+	 * @return ThumbnailImage|MediaTransformOutput|bool False on failure
 	 */
 	function getUnscaledThumb( $handlerParams = [] ) {
 		$hp =& $handlerParams;
@@ -1963,7 +1963,7 @@ abstract class File implements IDBAccessObject {
 	 * Returns the number of pages of a multipage document, or false for
 	 * documents which aren't multipage documents
 	 *
-	 * @return bool|int
+	 * @return string|bool|int
 	 */
 	function pageCount() {
 		if ( !isset( $this->pageCount ) ) {
@@ -1991,7 +1991,7 @@ abstract class File implements IDBAccessObject {
 		if ( $srcWidth == 0 ) {
 			return 0;
 		} else {
-			return round( $srcHeight * $dstWidth / $srcWidth );
+			return (int)round( $srcHeight * $dstWidth / $srcWidth );
 		}
 	}
 
@@ -2003,7 +2003,7 @@ abstract class File implements IDBAccessObject {
 	 *  a good reason. This method skips all caches.
 	 *
 	 * @param string $filePath The path to the file (e.g. From getLocalPathRef() )
-	 * @return array The width, followed by height, with optionally more things after
+	 * @return array|false The width, followed by height, with optionally more things after
 	 */
 	function getImageSize( $filePath ) {
 		if ( !$this->getHandler() ) {
@@ -2031,7 +2031,7 @@ abstract class File implements IDBAccessObject {
 	 * Get the HTML text of the description page, if available
 	 *
 	 * @param bool|Language $lang Optional language to fetch description in
-	 * @return string
+	 * @return string|false
 	 */
 	function getDescriptionText( $lang = false ) {
 		global $wgLang;
@@ -2122,7 +2122,7 @@ abstract class File implements IDBAccessObject {
 	/**
 	 * Get the deletion archive key, "<sha1>.<ext>"
 	 *
-	 * @return string
+	 * @return string|false
 	 */
 	function getStorageKey() {
 		$hash = $this->getSha1();

includes/filerepo/file/ForeignDBFile.php

@@ -121,7 +121,7 @@ class ForeignDBFile extends LocalFile {
 
 	/**
 	 * @param bool|Language $lang Optional language to fetch description in.
-	 * @return string
+	 * @return string|false
 	 */
 	function getDescriptionText( $lang = false ) {
 		global $wgLang;

includes/htmlform/HTMLFormField.php

@@ -1034,7 +1034,7 @@ abstract class HTMLFormField {
 	 * with integer 0 as a value.
 	 *
 	 * @param array $array
-	 * @return array
+	 * @return array|string
 	 */
 	public static function forceToStringRecursive( $array ) {
 		if ( is_array( $array ) ) {

includes/htmlform/fields/HTMLButtonField.php

@@ -127,6 +127,6 @@ class HTMLButtonField extends HTMLFormField {
 		$request = $this->mParent
 			? $this->mParent->getRequest()
 			: RequestContext::getMain()->getRequest();
-		return preg_match( '/MSIE [1-7]\./i', $request->getHeader( 'User-Agent' ) );
+		return (bool)preg_match( '/MSIE [1-7]\./i', $request->getHeader( 'User-Agent' ) );
 	}
 }

includes/htmlform/fields/HTMLMultiSelectField.php

@@ -149,7 +149,7 @@ class HTMLMultiSelectField extends HTMLFormField implements HTMLNestedFilterable
 	/**
 	 * @param WebRequest $request
 	 *
-	 * @return string
+	 * @return string|array
 	 */
 	public function loadDataFromRequest( $request ) {
 		if ( $this->isSubmitAttempt( $request ) ) {

includes/htmlform/fields/HTMLSizeFilterField.php

@@ -49,7 +49,7 @@ class HTMLSizeFilterField extends HTMLIntField {
 	/**
 	 * @param WebRequest $request
 	 *
-	 * @return string
+	 * @return string|int
 	 */
 	public function loadDataFromRequest( $request ) {
 		$size = $request->getInt( $this->mName );

includes/http/Http.php

@@ -142,7 +142,7 @@ class Http {
 	 * @return bool
 	 */
 	public static function isValidURI( $uri ) {
-		return preg_match(
+		return (bool)preg_match(
 			'/^https?:\/\/[^\/\s]\S*$/D',
 			$uri
 		);

includes/installer/Installer.php

@@ -567,7 +567,7 @@ abstract class Installer {
 	/**
 	 * Determine if LocalSettings.php exists. If it does, return its variables.
 	 *
-	 * @return array
+	 * @return array|false
 	 */
 	public static function getExistingLocalSettings() {
 		global $IP;
@@ -1080,7 +1080,7 @@ abstract class Installer {
 	/**
 	 * Convert a hex string representing a Unicode code point to that code point.
 	 * @param string $c
-	 * @return string
+	 * @return string|false
 	 */
 	protected function unicodeChar( $c ) {
 		$c = hexdec( $c );

includes/installer/LocalSettingsGenerator.php

@@ -109,7 +109,7 @@ class LocalSettingsGenerator {
 	 *
 	 * @param string $string
 	 *
-	 * @return string
+	 * @return string|false
 	 */
 	public static function escapePhpString( $string ) {
 		if ( is_array( $string ) || is_object( $string ) ) {

includes/interwiki/ClassicInterwikiLookup.php

@@ -187,7 +187,7 @@ class ClassicInterwikiLookup implements InterwikiLookup {
 	 * @note More logic is explained in DefaultSettings.
 	 *
 	 * @param string $prefix Interwiki prefix
-	 * @return Interwiki
+	 * @return Interwiki|false
 	 */
 	private function getInterwikiCached( $prefix ) {
 		$value = $this->getInterwikiCacheEntry( $prefix );

includes/jobqueue/aggregator/JobQueueAggregatorRedis.php

@@ -126,7 +126,7 @@ class JobQueueAggregatorRedis extends JobQueueAggregator {
 
 	/**
 	 * @param string $name
-	 * @return string
+	 * @return string[]
 	 */
 	private function decodeQueueName( $name ) {
 		list( $type, $wiki ) = explode( '/', $name, 2 );

includes/libs/ExplodeIterator.php

@@ -40,7 +40,7 @@ class ExplodeIterator implements Iterator {
 	// The position after the end of the next delimiter
 	private $endPos;
 
-	// The current token
+	/** @var string|false The current token */
 	private $current;
 
 	/**

includes/libs/HashRing.php

@@ -32,7 +32,7 @@ class HashRing {
 	/** @var Array (location => (start, end)) */
 	protected $ring = [];
 
-	/** @var Array (location => (start, end)) */
+	/** @var HashRing|null */
 	protected $liveRing;
 	/** @var Array (location => UNIX timestamp) */
 	protected $ejectionExpiries = [];

includes/libs/filebackend/FileBackendStore.php

@@ -729,7 +729,7 @@ abstract class FileBackendStore extends FileBackend {
 	/**
 	 * @see FileBackendStore::getFileXAttributes()
 	 * @param array $params
-	 * @return bool|string
+	 * @return array[][]
 	 */
 	protected function doGetFileXAttributes( array $params ) {
 		return [ 'headers' => [], 'metadata' => [] ]; // not supported

includes/libs/filebackend/SwiftFileBackend.php

@@ -1701,7 +1701,7 @@ class SwiftFileBackend extends FileBackendStore {
 	 * @param array $creds From getAuthentication()
 	 * @param string $container
 	 * @param string $object
-	 * @return array
+	 * @return string
 	 */
 	protected function storageUrl( array $creds, $container = null, $object = null ) {
 		$parts = [ $creds['storage_url'] ];

languages/Language.php

@@ -2577,7 +2577,7 @@ class Language {
 
 	/**
 	 * @param string $key
-	 * @return array|null
+	 * @return string|null
 	 */
 	public function getMessage( $key ) {
 		return self::$dataCache->getSubitem( $this->mCode, 'messages', $key );