repo/tests: Fix implicit nullable types

And clean up a few adjacent function signatures (but not too many, because this change is already big enough tbh). Bug: T379509 Change-Id: I7532b864f53bb3507e91fb88619684f1ef4f2528
wikimedia · Dec 18, 2024 · fa29e9a · fa29e9a
1 parent 4c0fba2
commit fa29e9a
Show file tree

Hide file tree

Showing 60 changed files with 141 additions and 148 deletions.
diff --git a/.phpcs.xml b/.phpcs.xml
@@ -108,10 +108,6 @@
 		</properties>
 	</rule>
 
-	<rule ref="MediaWiki.Usage.NullableType.ExplicitNullableTypes">
-		<exclude-pattern>repo/tests/</exclude-pattern>
-	</rule>
-
 	<file>.</file>
 	<arg name="extensions" value="php" />
 	<arg name="encoding" value="UTF-8" />

diff --git a/repo/tests/phpunit/includes/Actions/ActionTestCase.php b/repo/tests/phpunit/includes/Actions/ActionTestCase.php
@@ -158,7 +158,7 @@ private function makeTestItemData() {
 	 *
 	 * @return Action
 	 */
-	protected function createAction( string $actionName, WikiPage $page, array $params = null, $wasPosted = false ) {
+	protected function createAction( string $actionName, WikiPage $page, ?array $params = null, $wasPosted = false ) {
 		global $wgLang;
 
 		$params ??= [];

diff --git a/repo/tests/phpunit/includes/Actions/EditEntityActionTest.php b/repo/tests/phpunit/includes/Actions/EditEntityActionTest.php
@@ -437,13 +437,13 @@ public static function provideUndoForm() {
 	 * @dataProvider provideUndoForm
 	 */
 	public function testUndoForm(
-		$action,
+		string $action,
 		$page,
 		array $params,
-		$post = false,
-		User $user = null,
-		$htmlPattern = null,
-		array $expectedProps = null
+		bool $post,
+		?User $user,
+		?string $htmlPattern,
+		?array $expectedProps = null
 	) {
 		$this->tryUndoAction( $action, $page, $params, $post, $user, $htmlPattern, $expectedProps );
 	}
@@ -893,13 +893,13 @@ public static function provideUndoSubmit() {
 	 * @dataProvider provideUndoSubmit
 	 */
 	public function testUndoSubmit(
-		$action,
+		string $action,
 		$page,
 		array $params,
-		$post = false,
-		User $user = null,
-		$htmlPattern = null,
-		array $expectedProps = null
+		bool $post,
+		?User $user,
+		?string $htmlPattern,
+		?array $expectedProps = null
 	) {
 		if ( is_string( $page ) ) {
 			self::resetTestItem( $page );
@@ -918,17 +918,17 @@ public function testUndoSubmit(
 	 * @param array $params
 	 * @param bool $post
 	 * @param User|null $user
-	 * @param string|bool|null $htmlPattern
+	 * @param string|null $htmlPattern
 	 * @param string[]|null $expectedProps
 	 */
 	protected function tryUndoAction(
-		$action,
+		string $action,
 		$page,
 		array $params,
-		$post = false,
-		User $user = null,
-		$htmlPattern = null,
-		array $expectedProps = null
+		bool $post,
+		?User $user,
+		?string $htmlPattern,
+		?array $expectedProps
 	) {
 		if ( $user ) {
 			$this->setUser( $user );
@@ -950,7 +950,7 @@ protected function tryUndoAction(
 
 		$out = $this->callAction( $action, $page, $params, $post );
 
-		if ( $htmlPattern !== null && $htmlPattern !== false ) {
+		if ( $htmlPattern !== null ) {
 			$this->assertMatchesRegularExpression( $htmlPattern, $out->getHTML() );
 		}
 

diff --git a/repo/tests/phpunit/includes/Api/CreateRedirectTest.php b/repo/tests/phpunit/includes/Api/CreateRedirectTest.php
@@ -102,7 +102,7 @@ public function getMockEditFilterHookRunner(): EditFilterHookRunner {
 	private function newApiModule(
 		array $params,
 		User $user,
-		ItemRedirectCreationInteractor $interactor = null
+		?ItemRedirectCreationInteractor $interactor = null
 	): CreateRedirect {
 		$request = new FauxRequest( $params, true );
 		$main = new ApiMain( $request, true );
@@ -153,8 +153,8 @@ private function getMockEntityTitleLookup(): EntityTitleStoreLookup {
 
 	private function callApiModule(
 		array $params,
-		User $user = null,
-		ItemRedirectCreationInteractor $interactor = null
+		?User $user = null,
+		?ItemRedirectCreationInteractor $interactor = null
 	): array {
 		if ( !$user ) {
 			$user = $this->getTestUser()->getUser();

diff --git a/repo/tests/phpunit/includes/Api/EntityLoadingHelperTest.php b/repo/tests/phpunit/includes/Api/EntityLoadingHelperTest.php
@@ -58,9 +58,9 @@ protected function getMockRevisionLookup( ?int $revisionId, ?RevisionRecord $rev
 	 * @return EntityRevisionLookup|MockObject
 	 */
 	protected function getMockEntityRevisionLookup(
-		EntityId $entityId = null,
-		EntityRevision $entityRevision = null,
-		Exception $exception = null
+		?EntityId $entityId = null,
+		?EntityRevision $entityRevision = null,
+		?Exception $exception = null
 	) {
 		$mock = $this->createMock( EntityRevisionLookup::class );
 

diff --git a/repo/tests/phpunit/includes/Api/EntityTestHelper.php b/repo/tests/phpunit/includes/Api/EntityTestHelper.php
@@ -312,7 +312,7 @@ public static function getEntityData( $handle ) {
 	 * @throws OutOfBoundsException
 	 * @return array
 	 */
-	public static function getEntityOutput( $handle, array $props = null, array $langs = null ) {
+	public static function getEntityOutput( $handle, ?array $props = null, ?array $langs = null ) {
 		if ( !array_key_exists( $handle, self::$entityOutput ) ) {
 			throw new OutOfBoundsException( "No entity output defined with handle {$handle}" );
 		}
@@ -336,7 +336,7 @@ public static function getEntityOutput( $handle, array $props = null, array $lan
 	private static function stripUnwantedOutputValues(
 		array $entityOutput,
 		array $props,
-		array $languageCodes = null
+		?array $languageCodes
 	) {
 		$entityProps = [];
 		$props[] = 'type'; // always return the type so we can demobilize
@@ -373,7 +373,7 @@ private static function stripUnwantedOutputValues(
 	 * @param string $id
 	 * @param array|null $entity
 	 */
-	public static function registerEntity( $handle, $id, array $entity = null ) {
+	public static function registerEntity( $handle, $id, ?array $entity ) {
 		self::$activeHandles[ $handle ] = $id;
 		self::$activeIds[ $id ] = $handle;
 		if ( $entity ) {

diff --git a/repo/tests/phpunit/includes/Api/GetEntitiesTest.php b/repo/tests/phpunit/includes/Api/GetEntitiesTest.php
@@ -566,8 +566,8 @@ public static function provideLanguageFallback() {
 	public function testLanguageFallback(
 		$handle,
 		array $languages,
-		array $expectedLabels = null,
-		array $expectedDescriptions = null,
+		?array $expectedLabels,
+		?array $expectedDescriptions,
 		array $props = []
 	) {
 		$id = EntityTestHelper::getId( $handle );

diff --git a/repo/tests/phpunit/includes/Api/MergeItemsTest.php b/repo/tests/phpunit/includes/Api/MergeItemsTest.php
@@ -245,7 +245,7 @@ private function getTermValidatorFactory(): TermValidatorFactory {
 		);
 	}
 
-	private function callApiModule( array $params, EntityRedirect $expectedRedirect = null ): array {
+	private function callApiModule( array $params, ?EntityRedirect $expectedRedirect = null ): array {
 		$module = $this->newMergeItemsApiModule( $params, $expectedRedirect );
 
 		$module->execute();
@@ -385,7 +385,7 @@ private function assertItemsCorrect( array $result, array $expectedFrom, array $
 		$this->entityModificationTestHelper->assertEntityEquals( $expectedTo, $actualTo );
 	}
 
-	private function assertRedirectCorrect( array $result, EntityRedirect $redirect = null ): void {
+	private function assertRedirectCorrect( array $result, ?EntityRedirect $redirect ): void {
 		$this->assertArrayHasKey( 'redirected', $result );
 
 		if ( $redirect ) {

diff --git a/repo/tests/phpunit/includes/Api/PermissionsTestCase.php b/repo/tests/phpunit/includes/Api/PermissionsTestCase.php
@@ -30,7 +30,7 @@ public function addDBDataOnce(): void {
 	 *
 	 * @todo try to do this without messing with the globals, or at least without hardcoding them.
 	 */
-	protected function applyPermissions( array $permissions = null, array $groups = null ) {
+	protected function applyPermissions( ?array $permissions = null, ?array $groups = null ) {
 		$userGroupManager = $this->getServiceContainer()->getUserGroupManager();
 
 		if ( !$permissions ) {
@@ -66,7 +66,7 @@ protected function applyPermissions( array $permissions = null, array $groups =
 	protected function doPermissionsTest(
 		$action,
 		array $params,
-		array $permissions = null,
+		?array $permissions = null,
 		$expectedError = null
 	) {
 		$this->applyPermissions( $permissions );

diff --git a/repo/tests/phpunit/includes/Api/QuerySearchEntitiesTest.php b/repo/tests/phpunit/includes/Api/QuerySearchEntitiesTest.php
@@ -131,8 +131,8 @@ private function getMockApiPageSet( array $expected ): ApiPageSet {
 	private function callApi(
 		array $params,
 		array $matches,
-		ApiPageSet $resultPageSet = null,
-		Status $failureStatus = null
+		?ApiPageSet $resultPageSet = null,
+		?Status $failureStatus = null
 	): ?array {
 		// defaults from SearchEntities
 		$params = array_merge( [

diff --git a/repo/tests/phpunit/includes/Api/SearchEntitiesTest.php b/repo/tests/phpunit/includes/Api/SearchEntitiesTest.php
@@ -108,7 +108,7 @@ private function getMockEntitySearchHelper( array $params, array $returnResults
 	 *
 	 * @return array[]
 	 */
-	private function callApiModule( array $params, EntitySearchHelper $entitySearchHelper = null ): array {
+	private function callApiModule( array $params, ?EntitySearchHelper $entitySearchHelper = null ): array {
 		$entitySourceDefinitions = new EntitySourceDefinitions( [
 			new DatabaseEntitySource(
 				'items',

diff --git a/repo/tests/phpunit/includes/Api/StatementModificationHelperTest.php b/repo/tests/phpunit/includes/Api/StatementModificationHelperTest.php
@@ -189,7 +189,7 @@ public function testApplyChangeOp_reportsErrorWhenApplyFails() {
 	 *
 	 * @return StatementModificationHelper
 	 */
-	private function getNewInstance( ApiErrorReporter $errorReporter = null ) {
+	private function getNewInstance( ?ApiErrorReporter $errorReporter = null ) {
 		$entityIdParser = new ItemIdParser();
 
 		return new StatementModificationHelper(

diff --git a/repo/tests/phpunit/includes/Api/TermTestHelper.php b/repo/tests/phpunit/includes/Api/TermTestHelper.php
@@ -22,7 +22,7 @@ class TermTestHelper {
 	 */
 	public static function makeOverlyLongString(
 		string $text = "Test",
-		int $length = null
+		?int $length = null
 	): string {
 		if ( $length === null ) {
 			$limits = WikibaseRepo::getSettings()

diff --git a/repo/tests/phpunit/includes/Api/WikibaseApiTestCase.php b/repo/tests/phpunit/includes/Api/WikibaseApiTestCase.php
@@ -191,8 +191,8 @@ protected function createUppercaseStringTestProperty(): PropertyId {
 	protected function doTestQueryExceptions(
 		array $params,
 		array $exception,
-		Authority $user = null,
-		$token = true
+		?Authority $user = null,
+		bool $token = true
 	) {
 		try {
 			if ( $token ) {
@@ -444,8 +444,8 @@ protected function assertRevisionSummary( $regex, $revid ) {
 
 	protected function assertCanTagSuccessfulRequest(
 		array $params,
-		array $session = null,
-		Authority $user = null,
+		?array $session = null,
+		?Authority $user = null,
 		$tokenType = 'csrf'
 	) {
 		$dummyTag = __METHOD__ . '-dummy-tag';

diff --git a/repo/tests/phpunit/includes/ChangeModification/DispatchChangeDeletionNotificationJobTest.php b/repo/tests/phpunit/includes/ChangeModification/DispatchChangeDeletionNotificationJobTest.php
@@ -147,7 +147,7 @@ private function unpackRevisionIdentifiers( string $revisionIdentifiersJson ): a
 		return $revisionIdentifiers;
 	}
 
-	private function countArchive( array $conditions = null ): int {
+	private function countArchive( ?array $conditions = null ): int {
 		$selectQueryBuilder = $this->getDb()->newSelectQueryBuilder();
 		$selectQueryBuilder->table( 'archive' );
 		if ( $conditions !== null ) { // ->where( $conditions ?: IDatabase::ALL_ROWS doesn’t work (T332329)

diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpAliasesTest.php b/repo/tests/phpunit/includes/ChangeOp/ChangeOpAliasesTest.php
@@ -88,7 +88,7 @@ public static function changeOpAliasesProvider(): iterable {
 	public function testApply(
 		Item $item,
 		array $changeOpAliasesParams,
-		array $expectedAliases = null
+		?array $expectedAliases
 	) {
 		$changeOpAliasesParams[] = $this->getTermValidatorFactory();
 		$changeOpAliases = new ChangeOpAliases( ...$changeOpAliasesParams );

diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpMainSnakTest.php b/repo/tests/phpunit/includes/ChangeOp/ChangeOpMainSnakTest.php
@@ -108,7 +108,7 @@ public static function provideChangeOps(): iterable {
 	/**
 	 * @dataProvider provideChangeOps
 	 */
-	public function testApply( Item $item, array $changeOpParams, DataValue $expected = null ) {
+	public function testApply( Item $item, array $changeOpParams, ?DataValue $expected ) {
 		$changeOp = $this->newChangeOpMainSnak( ...$changeOpParams );
 		$changeOpResult = $changeOp->apply( $item );
 		$this->assertNotEmpty( $changeOp->getStatementGuid() );

diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpRemoveStatementTest.php b/repo/tests/phpunit/includes/ChangeOp/ChangeOpRemoveStatementTest.php
@@ -63,7 +63,7 @@ public static function changeOpProvider() {
 	/**
 	 * @dataProvider changeOpProvider
 	 */
-	public function testApplyAddNewClaim( Item $item, ChangeOpRemoveStatement $changeOp, DataValue $expected = null ) {
+	public function testApplyAddNewClaim( Item $item, ChangeOpRemoveStatement $changeOp, ?DataValue $expected ) {
 		$changeOpResult = $changeOp->apply( $item );
 		$this->assertNotEmpty( $changeOp->getGuid() );
 		$statements = $item->getStatements();

diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpResultStub.php b/repo/tests/phpunit/includes/ChangeOp/ChangeOpResultStub.php
@@ -21,9 +21,9 @@ class ChangeOpResultStub implements ChangeOpResult {
 	public ?array $validationErrors;
 
 	public function __construct(
-		EntityId $entityId = null,
-		bool $isEntityChanged = false,
-		array $validationErrors = null
+		?EntityId $entityId,
+		bool $isEntityChanged,
+		?array $validationErrors = null
 	) {
 		$this->isEntityChanged = $isEntityChanged;
 		$this->entityId = $entityId;

diff --git a/repo/tests/phpunit/includes/ChangeOp/ChangeOpSiteLinkTest.php b/repo/tests/phpunit/includes/ChangeOp/ChangeOpSiteLinkTest.php
@@ -41,7 +41,7 @@ public static function invalidConstructorProvider(): array {
 	/**
 	 * @dataProvider invalidConstructorProvider
 	 */
-	public function testConstructorWithInvalidArguments( $siteId, $linkPage, array $badges = null ): void {
+	public function testConstructorWithInvalidArguments( $siteId, $linkPage, ?array $badges ): void {
 		$this->expectException( InvalidArgumentException::class );
 		new ChangeOpSiteLink( $siteId, $linkPage, $badges );
 	}

diff --git a/repo/tests/phpunit/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializerTest.php b/repo/tests/phpunit/includes/ChangeOp/Deserialization/SiteLinksChangeOpDeserializerTest.php
@@ -43,7 +43,7 @@ private static function newSiteLinkTargetProvider() {
 		return new SiteLinkTargetProvider( new HashSiteStore( [ $wiki ] ) );
 	}
 
-	private function newSiteLinksChangeOpDeserializer( Item $item = null ): SiteLinksChangeOpDeserializer {
+	private function newSiteLinksChangeOpDeserializer( ?Item $item = null ): SiteLinksChangeOpDeserializer {
 		$entityLookupMock = $this->createStub( EntityLookup::class );
 		if ( $item !== null ) {
 			$entityLookupMock->method( 'getEntity' )

diff --git a/repo/tests/phpunit/includes/ChangeOp/LanguageBoundChangeOpResultStub.php b/repo/tests/phpunit/includes/ChangeOp/LanguageBoundChangeOpResultStub.php
@@ -17,10 +17,10 @@ class LanguageBoundChangeOpResultStub extends ChangeOpResultStub implements Lang
 	private $languageCode;
 
 	public function __construct(
-		EntityId $entityId = null,
-		$isEntityChanged = false,
-		$languageCode = '',
-		array $validationErrors = null
+		?EntityId $entityId,
+		bool $isEntityChanged,
+		string $languageCode,
+		?array $validationErrors = null
 	) {
 		parent::__construct( $entityId, $isEntityChanged, $validationErrors );
 		$this->languageCode = $languageCode;

diff --git a/repo/tests/phpunit/includes/Content/DeferredDecodingEntityHolderTest.php b/repo/tests/phpunit/includes/Content/DeferredDecodingEntityHolderTest.php
@@ -36,14 +36,11 @@ private function newEntity() {
 		return $item;
 	}
 
-	/**
-	 * @param EntityDocument $entity
-	 * @param string|null $expectedEntityType
-	 * @param EntityId|null $expectedEntityId
-	 *
-	 * @return EntityHolder
-	 */
-	private function newHolder( EntityDocument $entity, $expectedEntityType = null, EntityId $expectedEntityId = null ) {
+	private function newHolder(
+		EntityDocument $entity,
+		?string $expectedEntityType = null,
+		?EntityId $expectedEntityId = null
+	): EntityHolder {
 		$codec = new EntityContentDataCodec(
 			new ItemIdParser(),
 			WikibaseRepo::getStorageEntitySerializer(),

diff --git a/repo/tests/phpunit/includes/Content/EntityContentTestCase.php b/repo/tests/phpunit/includes/Content/EntityContentTestCase.php
@@ -79,7 +79,7 @@ abstract protected static function newEmpty();
 	 *
 	 * @return EntityContent A content object that contains an entity that is empty.
 	 */
-	abstract protected static function newBlank( EntityId $entityId = null );
+	abstract protected static function newBlank( ?EntityId $entityId = null );
 
 	public function testIsEmpty() {
 		$this->assertTrue( $this->newEmpty()->isEmpty(), 'empty' );
@@ -414,7 +414,7 @@ public static function entityRedirectProvider() {
 	/**
 	 * @dataProvider entityRedirectProvider
 	 */
-	public function testGetEntityRedirect( callable $contentProvider, EntityRedirect $redirect = null ) {
+	public function testGetEntityRedirect( callable $contentProvider, ?EntityRedirect $redirect ) {
 		$content = $contentProvider( $this );
 		$this->assertEquals( $content->getEntityRedirect(), $redirect );