client: Fix implicit nullable types

The rules of thumb I followed are mostly: always leave the default argument for LoggerInterface; leave the default argument in “factory” methods in tests, whether or not it’s currently used; otherwise, if the method isn’t currently called without the argument, remove the default argument (make the parameter required). Also includes a few minor related function signature cleanups. Bug: T379509 Change-Id: I77f082aadee69c21dc9622e527198a99b8fb4812
wikimedia · Dec 10, 2024 · 95a1d86 · 95a1d86
1 parent 529e2ac
commit 95a1d86
Show file tree

Hide file tree

Showing 37 changed files with 66 additions and 82 deletions.
diff --git a/.phpcs.xml b/.phpcs.xml
@@ -109,7 +109,6 @@
 	</rule>
 
 	<rule ref="MediaWiki.Usage.NullableType.ExplicitNullableTypes">
-		<exclude-pattern>client/</exclude-pattern>
 		<exclude-pattern>lib/</exclude-pattern>
 		<exclude-pattern>repo/</exclude-pattern>
 	</rule>

diff --git a/client/includes/Api/ApiListEntityUsage.php b/client/includes/Api/ApiListEntityUsage.php
@@ -48,7 +48,7 @@ public function execute(): void {
 		$this->run();
 	}
 
-	public function run( ApiPageSet $resultPageSet = null ): void {
+	public function run( ?ApiPageSet $resultPageSet = null ): void {
 		$params = $this->extractRequestParams();
 
 		$res = $this->doQuery( $params, $resultPageSet );
@@ -161,7 +161,7 @@ public function getCacheMode( $params ): string {
 		return 'public';
 	}
 
-	public function doQuery( array $params, ApiPageSet $resultPageSet = null ): ?IResultWrapper {
+	public function doQuery( array $params, ?ApiPageSet $resultPageSet ): ?IResultWrapper {
 		if ( !$params['entities'] ) {
 			return null;
 		}

diff --git a/client/includes/DataAccess/ReferenceFormatterFactory.php b/client/includes/DataAccess/ReferenceFormatterFactory.php
@@ -30,7 +30,7 @@ class ReferenceFormatterFactory {
 	public function __construct(
 		DataAccessSnakFormatterFactory $snakFormatterFactory,
 		WellKnownReferenceProperties $properties,
-		LoggerInterface $logger = null
+		?LoggerInterface $logger = null
 	) {
 		$this->snakFormatterFactory = $snakFormatterFactory;
 		$this->properties = $properties;

diff --git a/client/includes/DataAccess/Scribunto/EntityAccessor.php b/client/includes/DataAccess/Scribunto/EntityAccessor.php
@@ -97,7 +97,7 @@ public function __construct(
 		TermLanguageFallbackChain $termFallbackChain,
 		Language $language,
 		ContentLanguages $termsLanguages,
-		LoggerInterface $logger = null
+		?LoggerInterface $logger = null
 	) {
 		$this->entityIdParser = $entityIdParser;
 		$this->entityLookup = $entityLookup;

diff --git a/client/includes/DataAccess/Scribunto/WikibaseEntityLibrary.php b/client/includes/DataAccess/Scribunto/WikibaseEntityLibrary.php
@@ -254,7 +254,7 @@ public function register() {
 	 * @throws ScribuntoException
 	 * @return string[]|null[]
 	 */
-	public function formatPropertyValues( string $entityId, string $propertyLabelOrId, array $acceptableRanks = null ): array {
+	public function formatPropertyValues( string $entityId, string $propertyLabelOrId, ?array $acceptableRanks ): array {
 		try {
 			return [
 				$this->getImplementation()->formatPropertyValues(
@@ -284,7 +284,7 @@ public function formatPropertyValues( string $entityId, string $propertyLabelOrI
 	 * @throws ScribuntoException
 	 * @return string[]|null[]
 	 */
-	public function formatStatements( string $entityId, string $propertyLabelOrId, array $acceptableRanks = null ): array {
+	public function formatStatements( string $entityId, string $propertyLabelOrId, ?array $acceptableRanks ): array {
 		try {
 			return [
 				$this->getImplementation()->formatStatements(

diff --git a/client/includes/DataAccess/Scribunto/WikibaseLibrary.php b/client/includes/DataAccess/Scribunto/WikibaseLibrary.php
@@ -494,7 +494,7 @@ public function getEntityStatements( string $prefixedEntityId, string $propertyI
 	/**
 	 * Wrapper for getEntityId in WikibaseLanguageIndependentLuaBindings
 	 */
-	public function getEntityId( string $pageTitle, string $globalSiteId = null ): array {
+	public function getEntityId( string $pageTitle, ?string $globalSiteId = null ): array {
 		return [ $this->getLanguageIndependentLuaBindings()->getEntityId( $pageTitle, $globalSiteId ) ];
 	}
 

diff --git a/client/includes/DataAccess/SnaksFinder.php b/client/includes/DataAccess/SnaksFinder.php
@@ -25,7 +25,7 @@ class SnaksFinder {
 	public function findSnaks(
 		StatementListProvider $statementListProvider,
 		NumericPropertyId $propertyId,
-		array $acceptableRanks = null
+		?array $acceptableRanks
 	) {
 		$statementList = $this->getStatementsWithPropertyId( $statementListProvider, $propertyId );
 		if ( $acceptableRanks === null ) {

diff --git a/client/includes/DataAccess/StatementTransclusionInteractor.php b/client/includes/DataAccess/StatementTransclusionInteractor.php
@@ -77,7 +77,7 @@ public function __construct(
 	public function render(
 		EntityId $entityId,
 		$propertyLabelOrId,
-		array $acceptableRanks = null
+		?array $acceptableRanks = null
 	) {
 		try {
 			$entity = $this->entityLookup->getEntity( $entityId );

diff --git a/client/includes/Hooks/DataUpdateHookHandler.php b/client/includes/Hooks/DataUpdateHookHandler.php
@@ -90,7 +90,7 @@ public function __construct(
 		JobQueueGroup $jobScheduler,
 		UsageLookup $usageLookup,
 		UsageAccumulatorFactory $usageAccumulatorFactory,
-		LoggerInterface $logger = null
+		?LoggerInterface $logger = null
 	) {
 		$this->usageUpdater = $usageUpdater;
 		$this->jobScheduler = $jobScheduler;

diff --git a/client/includes/Hooks/SkinAfterBottomScriptsHandler.php b/client/includes/Hooks/SkinAfterBottomScriptsHandler.php
@@ -63,22 +63,13 @@ public function createSchemaElement(
 		return $html;
 	}
 
-	/**
-	 * @param Title $title
-	 * @param string|null $revisionTimestamp
-	 * @param string $entityConceptUri
-	 * @param File|null $imageFile
-	 * @param string|null $description
-	 *
-	 * @return array
-	 */
 	public function createSchema(
 		Title $title,
-		$revisionTimestamp,
-		$entityConceptUri,
-		File $imageFile = null,
-		$description = null
-		) {
+		?string $revisionTimestamp,
+		string $entityConceptUri,
+		?File $imageFile,
+		?string $description
+	): array {
 		$revisionRecord = $this->revisionLookup->getFirstRevision( $title );
 		$schemaTimestamp = $revisionRecord ? $revisionRecord->getTimestamp() : null;
 		$schema = [

diff --git a/client/includes/NamespaceChecker.php b/client/includes/NamespaceChecker.php
@@ -36,7 +36,7 @@ class NamespaceChecker {
 	public function __construct(
 		array $excludedNamespaces,
 		array $enabledNamespaces = [],
-		NamespaceInfo $namespaceInfo = null
+		?NamespaceInfo $namespaceInfo = null
 	) {
 		$this->excludedNamespaces = $excludedNamespaces;
 		$this->enabledNamespaces = $enabledNamespaces;

diff --git a/client/includes/ParserOutput/ClientParserOutputDataUpdater.php b/client/includes/ParserOutput/ClientParserOutputDataUpdater.php
@@ -73,7 +73,7 @@ public function __construct(
 		EntityLookup $entityLookup,
 		UsageAccumulatorFactory $usageAccumulatorFactory,
 		string $siteId,
-		LoggerInterface $logger = null
+		?LoggerInterface $logger = null
 	) {
 		$this->otherProjectsSidebarGeneratorFactory = $otherProjectsSidebarGeneratorFactory;
 		$this->entityLookup = $entityLookup;

diff --git a/client/includes/PropertyLabelNotResolvedException.php b/client/includes/PropertyLabelNotResolvedException.php
@@ -17,7 +17,7 @@ public function __construct(
 		string $label,
 		string $languageCode,
 		?string $message = null,
-		Exception $previous = null
+		?Exception $previous = null
 	) {
 		parent::__construct(
 			'wikibase-property-notfound',

diff --git a/client/includes/RecentChanges/RecentChangeFactory.php b/client/includes/RecentChanges/RecentChangeFactory.php
@@ -73,8 +73,8 @@ public function __construct(
 		SiteLinkCommentCreator $siteLinkCommentCreator,
 		EntitySourceDefinitions $entitySourceDefinitions,
 		ClientDomainDb $clientDomainDb,
-		CentralIdLookup $centralIdLookup = null,
-		ExternalUserNames $externalUsernames = null
+		?CentralIdLookup $centralIdLookup = null,
+		?ExternalUserNames $externalUsernames = null
 	) {
 		$this->language = $language;
 		$this->siteLinkCommentCreator = $siteLinkCommentCreator;
@@ -95,7 +95,7 @@ public function __construct(
 	 *
 	 * @return RecentChange
 	 */
-	public function newRecentChange( EntityChange $change, Title $target, array $preparedAttribs = null ) {
+	public function newRecentChange( EntityChange $change, Title $target, ?array $preparedAttribs = null ) {
 		$preparedAttribs ??= $this->prepareChangeAttributes( $change );
 
 		$targetSpecificAttributes = $this->buildTargetSpecificAttributes( $change, $target );
@@ -307,7 +307,7 @@ private function buildTargetSpecificComment( EntityChange $change, Title $target
 	 *
 	 * @return string
 	 */
-	private function getEditCommentMulti( EntityChange $change, Title $target = null ) {
+	private function getEditCommentMulti( EntityChange $change, ?Title $target = null ) {
 		$info = $change->getInfo();
 
 		if ( isset( $info['changes'] ) ) {
@@ -343,7 +343,7 @@ private function getEditCommentMulti( EntityChange $change, Title $target = null
 	 *
 	 * @return string
 	 */
-	private function getEditComment( EntityChange $change, Title $target = null ) {
+	private function getEditComment( EntityChange $change, ?Title $target ) {
 		$siteLinkDiff = $change instanceof ItemChange
 			? $change->getSiteLinkDiff()
 			: null;

diff --git a/client/includes/RecentChanges/SiteLinkCommentCreator.php b/client/includes/RecentChanges/SiteLinkCommentCreator.php
@@ -61,7 +61,7 @@ public function __construct( Language $language, SiteLookup $siteLookup, $siteId
 	 * @return string|null A human readable edit summary (limited wikitext),
 	 *         or null if no summary could be created for the sitelink change.
 	 */
-	public function getEditComment( ?Diff $siteLinkDiff, $action, Title $title = null ) {
+	public function getEditComment( ?Diff $siteLinkDiff, $action, ?Title $title ) {
 		if ( $siteLinkDiff !== null && !$siteLinkDiff->isEmpty() ) {
 			$siteLinkMessage = $this->getSiteLinkMessage( $action, $siteLinkDiff, $title );
 
@@ -126,7 +126,7 @@ public function needsTargetSpecificSummary( Diff $siteLinkDiff, Title $title ) {
 	 *
 	 * @return array|null
 	 */
-	private function getSiteLinkMessage( $action, Diff $siteLinkDiff, Title $title = null ) {
+	private function getSiteLinkMessage( $action, Diff $siteLinkDiff, ?Title $title ) {
 		if ( $siteLinkDiff->isEmpty() ) {
 			return null;
 		}
@@ -252,7 +252,7 @@ private function getChangeParamsForDiffOp( DiffOp $diffOp, $siteId, $messagePref
 	 *
 	 * @return array|null
 	 */
-	private function getSiteLinkAddRemoveParams( DiffOp $diffOp, $action, $siteId, Title $title = null ) {
+	private function getSiteLinkAddRemoveParams( DiffOp $diffOp, $action, $siteId, ?Title $title ) {
 		$params = [];
 
 		if ( in_array( $action, [ 'remove', 'restore' ] ) ) {

diff --git a/client/includes/RepoItemLinkGenerator.php b/client/includes/RepoItemLinkGenerator.php
@@ -95,7 +95,7 @@ public function getLink( IContextSource $context, $action, $hasLangLinks, ?array
 	 *
 	 * @return bool
 	 */
-	private function canHaveLink( Title $title, $action, array $noExternalLangLinks = null ) {
+	private function canHaveLink( Title $title, $action, ?array $noExternalLangLinks ) {
 		if ( $action !== 'view' ) {
 			return false;
 		}
@@ -115,7 +115,7 @@ private function canHaveLink( Title $title, $action, array $noExternalLangLinks
 	 *
 	 * @return bool
 	 */
-	private function isSuppressed( array $noExternalLangLinks = null ) {
+	private function isSuppressed( ?array $noExternalLangLinks ) {
 		return $noExternalLangLinks !== null && in_array( '*', $noExternalLangLinks );
 	}
 
@@ -145,7 +145,7 @@ private function getEditLinksLink( IContextSource $context, EntityId $entityId )
 	 *
 	 * @return string HTML
 	 */
-	private function getAddLinksLink( IContextSource $context, EntityId $entityId = null ) {
+	private function getAddLinksLink( IContextSource $context, ?EntityId $entityId ) {
 		if ( $entityId ) {
 			$href = $this->getEntityUrl( $entityId );
 		} else {

diff --git a/client/includes/RepoLinker.php b/client/includes/RepoLinker.php
@@ -83,7 +83,7 @@ public function formatLink( string $url, string $text, array $attribs = [] ): st
 	 *
 	 * @return string (html)
 	 */
-	public function buildEntityLink( EntityId $entityId, array $classes = [], string $text = null ): string {
+	public function buildEntityLink( EntityId $entityId, array $classes = [], ?string $text = null ): string {
 		$class = 'wb-entity-link';
 		if ( $classes !== [] ) {
 			$class .= ' ' . implode( ' ', $classes );

diff --git a/client/includes/Store/Sql/BulkSubscriptionUpdater.php b/client/includes/Store/Sql/BulkSubscriptionUpdater.php
@@ -92,7 +92,7 @@ public function setProgressReporter( MessageReporter $progressReporter ) {
 	 *
 	 * @param EntityId|null $startEntity The entity to start with.
 	 */
-	public function updateSubscriptions( EntityId $startEntity = null ) {
+	public function updateSubscriptions( ?EntityId $startEntity = null ) {
 		$this->repoConnectionManager->prepareForUpdates();
 
 		$continuation = $startEntity ? [ $startEntity->getSerialization() ] : null;
@@ -118,7 +118,7 @@ public function updateSubscriptions( EntityId $startEntity = null ) {
 	 *
 	 * @return int The number of subscriptions inserted.
 	 */
-	private function processUpdateBatch( array &$continuation = null ) {
+	private function processUpdateBatch( ?array &$continuation ) {
 		$entityIds = $this->getUpdateBatch( $continuation );
 
 		if ( !$entityIds ) {
@@ -157,7 +157,7 @@ private function insertUpdateBatch( array $entities ) {
 	 *
 	 * @return string[] A list of entity id strings.
 	 */
-	private function getUpdateBatch( array &$continuation = null ) {
+	private function getUpdateBatch( ?array &$continuation ) {
 		$dbr = $this->localConnectionManager->getReadConnection();
 		$queryBuilder = $dbr->newSelectQueryBuilder();
 		$queryBuilder->distinct()
@@ -209,7 +209,7 @@ private function makeSubscriptionRows( array $entities ) {
 	 *
 	 * @return string[] A list of entity ids strings.
 	 */
-	private function getEntityIdsFromRows( IResultWrapper $res, $entityIdField, array &$continuation = null ) {
+	private function getEntityIdsFromRows( IResultWrapper $res, $entityIdField, ?array &$continuation ) {
 		$entities = [];
 
 		foreach ( $res as $row ) {
@@ -228,7 +228,7 @@ private function getEntityIdsFromRows( IResultWrapper $res, $entityIdField, arra
 	 *
 	 * @param EntityId|null $startEntity The entity to start with.
 	 */
-	public function purgeSubscriptions( EntityId $startEntity = null ) {
+	public function purgeSubscriptions( ?EntityId $startEntity = null ) {
 		$continuation = $startEntity ? [ $startEntity->getSerialization() ] : null;
 
 		$this->repoConnectionManager->prepareForUpdates();
@@ -254,7 +254,7 @@ public function purgeSubscriptions( EntityId $startEntity = null ) {
 	 *
 	 * @return int The number of subscriptions deleted.
 	 */
-	private function processDeletionBatch( array &$continuation = null ) {
+	private function processDeletionBatch( ?array &$continuation ) {
 		$deletionRange = $this->getDeletionRange( $continuation );
 
 		if ( $deletionRange === false ) {
@@ -274,7 +274,7 @@ private function processDeletionBatch( array &$continuation = null ) {
 	 *
 	 * @return array|false list( $minId, $maxId, $count ), or false if there is nothing to delete
 	 */
-	private function getDeletionRange( array &$continuation = null ) {
+	private function getDeletionRange( ?array &$continuation ) {
 		/**
 		 * @note Below, we query and iterate all rows we want to delete in the current batch. That
 		 * is rather ugly, but appears to be the best solution, because:

diff --git a/client/includes/Usage/UsageTrackingLanguageFallbackLabelDescriptionLookup.php b/client/includes/Usage/UsageTrackingLanguageFallbackLabelDescriptionLookup.php
@@ -100,7 +100,7 @@ public function getDescription( EntityId $entityId ) {
 	 *
 	 * @return string[]|null[]
 	 */
-	private function getTouchedLanguages( TermFallback $termFallback = null ) {
+	private function getTouchedLanguages( ?TermFallback $termFallback ) {
 		if ( $this->trackUsagesInAllLanguages ) {
 			// On multi-lingual wikis where users can request pages in any language, we can not
 			// optimize for one language fallback chain only. Since all possible language fallback

diff --git a/...sts/phpunit/integration/includes/ChangeModification/ChangeDeletionNotificationJobTest.php b/...sts/phpunit/integration/includes/ChangeModification/ChangeDeletionNotificationJobTest.php
@@ -19,7 +19,7 @@
  */
 class ChangeDeletionNotificationJobTest extends RecentChangesModificationTestBase {
 
-	private function countRevisions( array $conditions = null ): int {
+	private function countRevisions( ?array $conditions = null ): int {
 		$selectQueryBuilder = $this->getDb()->newSelectQueryBuilder();
 		$selectQueryBuilder->table( 'recentchanges' );
 		if ( $conditions !== null ) { // ->where( $conditions ?: IDatabase::ALL_ROWS doesn’t work (T332329)

diff --git a/client/tests/phpunit/integration/includes/Changes/ChangeHandlerTest.php b/client/tests/phpunit/integration/includes/Changes/ChangeHandlerTest.php
@@ -69,7 +69,7 @@ private function getChangeRunCoalescer() {
 
 	private function getChangeHandler(
 		array $pageNamesPerItemId = [],
-		PageUpdater $updater = null,
+		?PageUpdater $updater = null,
 		array $hooks = []
 	) {
 		$siteLinkLookup = $this->getSiteLinkLookup( $pageNamesPerItemId );

diff --git a/...unit/integration/includes/DataAccess/Scribunto/WikitextPreprocessingSnakFormatterTest.php b/...unit/integration/includes/DataAccess/Scribunto/WikitextPreprocessingSnakFormatterTest.php
@@ -26,13 +26,7 @@
  */
 class WikitextPreprocessingSnakFormatterTest extends MediaWikiIntegrationTestCase {
 
-	/**
-	 * @param Snak|null $expectedSnak
-	 * @param string|null $formattedValue
-	 *
-	 * @return SnakFormatter
-	 */
-	private function newMockSnakFormatter( Snak $expectedSnak = null, $formattedValue = null ): SnakFormatter {
+	private function newMockSnakFormatter( ?Snak $expectedSnak = null, ?string $formattedValue = null ): SnakFormatter {
 		$mockFormatter = $this->createMock( SnakFormatter::class );
 
 		$mockFormatter->expects( $expectedSnak ? $this->once() : $this->never() )

diff --git a/...t/tests/phpunit/integration/includes/DataAccess/WikibaseDataAccessTestItemSetUpHelper.php b/...t/tests/phpunit/integration/includes/DataAccess/WikibaseDataAccessTestItemSetUpHelper.php
@@ -168,7 +168,7 @@ private function getTestProperty( NumericPropertyId $id, $dataTypeId, $label ) {
 	 *
 	 * @return Item
 	 */
-	private function createTestItem( ItemId $id, array $labels, array $statements = null, array $siteLinks = null ) {
+	private function createTestItem( ItemId $id, array $labels, ?array $statements = null, ?array $siteLinks = null ) {
 		$item = new Item( $id );
 		$item->setDescription( 'de', 'Description of ' . $id->getSerialization() );