Unsuppress PhanParamReqAfterOpt, use PHP71 nullable types

These were all checked with codesearch to ensure nothing is overriding
these methods.
For the most part, I've updated the signature to use nullable types; for
two Pager's, I've just made all parameters non-optional, because you're
already forced to pass them with a required parameter at the end.

Bug: T231636
Change-Id: Ie047891f55fcd322039194cfa9a8549e4f1f6f14

.phan/config.php

@@ -91,9 +91,6 @@ $cfg['exclude_analysis_directory_list'] = [
 // either in-line with @phan-suppress-next-line and similar, at block-level (via @suppress), or at
 // file-level (with @phan-file-suppress), so that it stays enabled for the rest of the codebase.
 $cfg['suppress_issue_types'] = array_merge( $cfg['suppress_issue_types'], [
-	// approximate error count: 19
-	"PhanParamReqAfterOpt", // False positives with nullables (phan issue #3159). Use real nullables
-	//after dropping HHVM
 	// approximate error count: 110
 	"PhanParamTooMany", // False positives with variargs. Unsuppress after dropping HHVM
 ] );

.phpcs.xml

@@ -19,6 +19,7 @@
 		<exclude name="MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.NewLineComment" />
 		<exclude name="MediaWiki.WhiteSpace.SpaceBeforeSingleLineComment.SingleSpaceBeforeSingleLineComment" />
 		<exclude name="Squiz.Scope.MethodScope.Missing" />
+		<exclude name="MediaWiki.PHP71Features.NullableType.NotAllowed" />
 	</rule>
 	<rule ref="MediaWiki.NamingConventions.PrefixedGlobalFunctions">
 		<properties>

includes/CommentStore.php

@@ -285,7 +285,7 @@ class CommentStore {
 	 * @param bool $fallback
 	 * @return CommentStoreComment
 	 */
-	private function getCommentInternal( IDatabase $db = null, $key, $row, $fallback = false ) {
+	private function getCommentInternal( ?IDatabase $db, $key, $row, $fallback = false ) {
 		$row = (array)$row;
 		if ( array_key_exists( "{$key}_text", $row ) && array_key_exists( "{$key}_data", $row ) ) {
 			$cid = $row["{$key}_cid"] ?? null;

includes/api/ApiUsageException.php

@@ -36,7 +36,7 @@ class ApiUsageException extends MWException implements ILocalizedException {
 	 * @param int $httpCode HTTP error code to use
 	 */
 	public function __construct(
-		ApiBase $module = null, StatusValue $status, $httpCode = 0
+		?ApiBase $module, StatusValue $status, $httpCode = 0
 	) {
 		if ( $status->isOK() ) {
 			throw new InvalidArgumentException( __METHOD__ . ' requires a fatal Status' );
@@ -61,7 +61,7 @@ class ApiUsageException extends MWException implements ILocalizedException {
 	 * @return static
 	 */
 	public static function newWithMessage(
-		ApiBase $module = null, $msg, $code = null, $data = null, $httpCode = 0
+		?ApiBase $module, $msg, $code = null, $data = null, $httpCode = 0
 	) {
 		return new static(
 			$module,

includes/changes/CategoryMembershipChange.php

@@ -155,7 +155,7 @@ class CategoryMembershipChange {
 	private function notifyCategorization(
 		$timestamp,
 		Title $categoryTitle,
-		User $user = null,
+		?User $user,
 		$comment,
 		Title $pageTitle,
 		$lastTimestamp,

includes/changes/RecentChange.php

@@ -907,7 +907,7 @@ class RecentChange implements Taggable {
 	public static function newForCategorization(
 		$timestamp,
 		Title $categoryTitle,
-		User $user = null,
+		?User $user,
 		$comment,
 		Title $pageTitle,
 		$oldRevId,

includes/diff/DifferenceEngine.php

@@ -1662,7 +1662,7 @@ class DifferenceEngine extends ContextSource {
 	 * @param RevisionRecord $newRevision
 	 */
 	public function setRevisions(
-		RevisionRecord $oldRevision = null, RevisionRecord $newRevision
+		?RevisionRecord $oldRevision, RevisionRecord $newRevision
 	) {
 		if ( $oldRevision ) {
 			$this->mOldRev = new Revision( $oldRevision );

includes/export/XmlDumpWriter.php

@@ -297,14 +297,14 @@ class XmlDumpWriter {
 	 *
 	 * @param object $obj
 	 * @param string $method
-	 * @param array $args
 	 * @param string $warning The warning to output in case of a storage related exception.
+	 * @param array $args
 	 *
 	 * @return mixed Returns the method's return value,
 	 *         or null in case of a storage related exception.
 	 * @throws Exception
 	 */
-	private function invokeLenient( $obj, $method, $args = [], $warning ) {
+	private function invokeLenient( $obj, $method, $warning, $args = [] ) {
 		try {
 			return call_user_func_array( [ $obj, $method ], $args );
 		} catch ( SuppressedDataException $ex ) {
@@ -386,7 +386,6 @@ class XmlDumpWriter {
 			$sha1 = $this->invokeLenient(
 				$rev,
 				'getSha1',
-				[],
 				'failed to determine sha1 for revision ' . $rev->getId()
 			);
 			$out .= "      " . Xml::element( 'sha1', null, strval( $sha1 ) ) . "\n";
@@ -400,8 +399,8 @@ class XmlDumpWriter {
 			$content = $this->invokeLenient(
 				$rev,
 				'getContent',
-				[ SlotRecord::MAIN, RevisionRecord::RAW ],
-				'Failed to load main slot content of revision ' . $rev->getId()
+				'Failed to load main slot content of revision ' . $rev->getId(),
+				[ SlotRecord::MAIN, RevisionRecord::RAW ]
 			);
 
 			$text = $content ? $content->serialize() : '';
@@ -456,7 +455,6 @@ class XmlDumpWriter {
 			'bytes' => $this->invokeLenient(
 				$slot,
 				'getSize',
-				[],
 				'failed to determine size for slot ' . $slot->getRole() . ' of revision '
 				. $slot->getRevision()
 			) ?: '0'
@@ -466,7 +464,6 @@ class XmlDumpWriter {
 			$textAttributes['sha1'] = $this->invokeLenient(
 				$slot,
 				'getSha1',
-				[],
 				'failed to determine sha1 for slot ' . $slot->getRole() . ' of revision '
 				. $slot->getRevision()
 			) ?: '';
@@ -476,7 +473,6 @@ class XmlDumpWriter {
 			$content = $this->invokeLenient(
 				$slot,
 				'getContent',
-				[],
 				'failed to load content for slot ' . $slot->getRole() . ' of revision '
 				. $slot->getRevision()
 			);

includes/jobqueue/jobs/EmaillingJob.php

@@ -28,7 +28,7 @@
  * @ingroup JobQueue
  */
 class EmaillingJob extends Job {
-	function __construct( Title $title = null, array $params ) {
+	function __construct( ?Title $title, array $params ) {
 		parent::__construct( 'sendMail', Title::newMainPage(), $params );
 	}
 

includes/libs/rdbms/exception/DBError.php

@@ -37,7 +37,7 @@ class DBError extends RuntimeException {
 	 * @param string $error A simple error message to be used for debugging
 	 * @param \Exception|\Throwable|null $prev Previous exception
 	 */
-	public function __construct( IDatabase $db = null, $error, $prev = null ) {
+	public function __construct( ?IDatabase $db, $error, $prev = null ) {
 		parent::__construct( $error, 0, $prev );
 		$this->db = $db;
 	}

includes/libs/rdbms/exception/DBExpectedError.php

@@ -40,7 +40,7 @@ class DBExpectedError extends DBError implements MessageSpecifier {
 	 * @param \Exception|\Throwable|null $prev
 	 */
 	public function __construct(
-		IDatabase $db = null, $error, array $params = [], $prev = null
+		?IDatabase $db, $error, array $params = [], $prev = null
 	) {
 		parent::__construct( $db, $error, $prev );
 		$this->params = $params;

includes/parser/Parser.php

@@ -831,7 +831,7 @@ class Parser {
 	 * @param bool|PPFrame $frame
 	 * @return mixed|string
 	 */
-	public function preprocess( $text, Title $title = null,
+	public function preprocess( $text, ?Title $title,
 		ParserOptions $options, $revid = null, $frame = false
 	) {
 		$magicScopeVariable = $this->lock();
@@ -4882,7 +4882,7 @@ class Parser {
 	 * @param bool $clearState
 	 * @param int|null $revId
 	 */
-	public function startExternalParse( Title $title = null, ParserOptions $options,
+	public function startExternalParse( ?Title $title, ParserOptions $options,
 		$outputType, $clearState = true, $revId = null
 	) {
 		$this->startParse( $title, $options, $outputType, $clearState );
@@ -4897,7 +4897,7 @@ class Parser {
 	 * @param int $outputType
 	 * @param bool $clearState
 	 */
-	private function startParse( Title $title = null, ParserOptions $options,
+	private function startParse( ?Title $title, ParserOptions $options,
 		$outputType, $clearState = true
 	) {
 		$this->setTitle( $title );

includes/resourceloader/ResourceLoaderWikiModule.php

@@ -531,7 +531,7 @@ class ResourceLoaderWikiModule extends ResourceLoaderModule {
 	 * @since 1.28
 	 */
 	public static function invalidateModuleCache(
-		Title $title, Revision $old = null, Revision $new = null, $domain
+		Title $title, ?Revision $old, ?Revision $new, $domain
 	) {
 		static $formats = [ CONTENT_FORMAT_CSS, CONTENT_FORMAT_JAVASCRIPT ];
 

includes/session/CookieSessionProvider.php

@@ -267,6 +267,7 @@ class CookieSessionProvider extends SessionProvider {
 	 * @param bool $set Whether the cookie should be set or not
 	 * @param SessionBackend|null $backend
 	 * @param WebRequest $request
+	 * @suppress PhanParamReqAfterOpt Overridden in CentralAuth
 	 */
 	protected function setForceHTTPSCookie(
 		$set, SessionBackend $backend = null, WebRequest $request

includes/session/UserInfo.php

@@ -55,7 +55,7 @@ final class UserInfo {
 	/** @var User|null */
 	private $user = null;
 
-	private function __construct( User $user = null, $verified ) {
+	private function __construct( ?User $user, $verified ) {
 		if ( $user && $user->isAnon() && !User::isUsableName( $user->getName() ) ) {
 			$this->verified = true;
 			$this->user = null;

includes/specials/pagers/ActiveUsersPager.php

@@ -55,7 +55,7 @@ class ActiveUsersPager extends UsersPager {
 	 * @param IContextSource|null $context
 	 * @param FormOptions $opts
 	 */
-	public function __construct( IContextSource $context = null, FormOptions $opts ) {
+	public function __construct( ?IContextSource $context, FormOptions $opts ) {
 		parent::__construct( $context );
 
 		$this->RCMaxAge = $this->getConfig()->get( 'ActiveUserDays' );

includes/specials/pagers/AllMessagesTablePager.php

@@ -66,7 +66,7 @@ class AllMessagesTablePager extends TablePager {
 	 * @param FormOptions $opts
 	 * @param LinkRenderer $linkRenderer
 	 */
-	public function __construct( IContextSource $context = null, FormOptions $opts,
+	public function __construct( ?IContextSource $context, FormOptions $opts,
 		LinkRenderer $linkRenderer
 	) {
 		parent::__construct( $context, $linkRenderer );

includes/specials/pagers/DeletedContribsPager.php

@@ -61,7 +61,7 @@ class DeletedContribsPager extends IndexPager {
 	 */
 	protected $mNavigationBar;
 
-	public function __construct( IContextSource $context, $target, $namespace = false,
+	public function __construct( IContextSource $context, $target, $namespace,
 		LinkRenderer $linkRenderer
 	) {
 		parent::__construct( $context, $linkRenderer );

includes/specials/pagers/ImageListPager.php

@@ -51,8 +51,8 @@ class ImageListPager extends TablePager {
 
 	protected $mTableName = 'image';
 
-	public function __construct( IContextSource $context, $userName = null, $search = '',
-		$including = false, $showAll = false, LinkRenderer $linkRenderer
+	public function __construct( IContextSource $context, $userName, $search,
+		$including, $showAll, LinkRenderer $linkRenderer
 	) {
 		$this->setContext( $context );
 

includes/widget/search/FullSearchResultWidget.php

@@ -164,7 +164,7 @@ class FullSearchResultWidget implements SearchResultWidget {
 	 *  to use the title
 	 * @return string HTML
 	 */
-	protected function generateAltTitleHtml( $msgKey, Title $title = null, $text ) {
+	protected function generateAltTitleHtml( $msgKey, ?Title $title, $text ) {
 		$inner = $title === null
 			? $text
 			: $this->linkRenderer->makeLink( $title, $text ? new HtmlArmor( $text ) : null );