Merge "Re-enable Phan and fix warnings / errors for `data-model/src'"

jenkins-bot · Gerrit Code Review · commit bd2f9cf90f68 · 2025-03-03T15:27:20.000Z
diff --git a/.phan/config.php b/.phan/config.php
@@ -65,7 +65,6 @@
 $cfg['exclude_analysis_directory_list'] = array_merge(
 	$cfg['exclude_analysis_directory_list'],
 	[
-		'lib/packages/wikibase/data-model/src/',
 		'lib/packages/wikibase/data-model-serialization/src/',
 		'lib/packages/wikibase/data-model-services/src/',
 		'lib/packages/wikibase/internal-serialization/src/',
diff --git a/lib/packages/wikibase/data-model/src/ByPropertyIdArray.php b/lib/packages/wikibase/data-model/src/ByPropertyIdArray.php
@@ -307,6 +307,9 @@ private function movePropertyGroup( PropertyId $propertyId, $toIndex ) {
 
 		$oldIndex = $this->getPropertyGroupIndex( $propertyId );
 		$byIdClone = $this->byId;
+		if ( $byIdClone === null ) {
+			throw new RuntimeException( 'LogicError - index should have been built' );
+		}
 
 		// Remove "property group" to calculate the groups new index:
 		unset( $this->byId[$propertyId->getSerialization()] );
@@ -432,7 +435,7 @@ public function addObjectAtIndex( $object, $index = null ) {
 		if ( count( $this ) === 0 ) {
 			// Array is empty, just append object.
 			$this->append( $object );
-		} elseif ( empty( $validIndices ) ) {
+		} elseif ( $validIndices === [] ) {
 			// No objects featuring that property exist. The object may be inserted at a place
 			// between existing "property groups".
 			$this->append( $object );
@@ -461,7 +464,7 @@ private function addObjectToPropertyGroup( $object, $index = null ) {
 		$propertyId = $object->getPropertyId();
 		$validIndices = $this->getFlatArrayIndices( $propertyId );
 
-		if ( empty( $validIndices ) ) {
+		if ( $validIndices === [] ) {
 			throw new OutOfBoundsException( 'No objects featuring the object\'s property exist' );
 		}
 
@@ -473,6 +476,10 @@ private function addObjectToPropertyGroup( $object, $index = null ) {
 			$index = end( $validIndices );
 		}
 
+		if ( $this->byId === null ) {
+			throw new RuntimeException( 'LogicError - index should have been built' );
+		}
+
 		if ( in_array( $index, $validIndices ) ) {
 			// Add object at index within "property group".
 			$this->byId[$propertyId->getSerialization()][] = $object;
diff --git a/lib/packages/wikibase/data-model/src/Entity/BasicEntityIdParser.php b/lib/packages/wikibase/data-model/src/Entity/BasicEntityIdParser.php
@@ -21,7 +21,7 @@ public function __construct() {
 	/**
 	 * @param string $idSerialization
 	 *
-	 * @return ItemId|PropertyId
+	 * @return EntityId
 	 * @throws EntityIdParsingException
 	 */
 	public function parse( $idSerialization ) {
diff --git a/lib/packages/wikibase/data-model/src/Entity/Item.php b/lib/packages/wikibase/data-model/src/Entity/Item.php
@@ -87,7 +87,7 @@ public function getId() {
 	/**
 	 * @since 0.5, can be null since 1.0
 	 *
-	 * @param ItemId|null $id
+	 * @param EntityId|null $id
 	 *
 	 * @throws InvalidArgumentException
 	 */
diff --git a/lib/packages/wikibase/data-model/src/Entity/ItemIdSet.php b/lib/packages/wikibase/data-model/src/Entity/ItemIdSet.php
@@ -51,7 +51,7 @@ public function count(): int {
 	/**
 	 * @see IteratorAggregate::getIterator
 	 *
-	 * @return Iterator|ItemId[]
+	 * @return Traversable<ItemId>
 	 */
 	public function getIterator(): Traversable {
 		return new ArrayIterator( $this->ids );
diff --git a/lib/packages/wikibase/data-model/src/Entity/Property.php b/lib/packages/wikibase/data-model/src/Entity/Property.php
@@ -90,7 +90,7 @@ public function getId() {
 	/**
 	 * @since 0.5, can be null since 1.0
 	 *
-	 * @param PropertyId|null $id
+	 * @param EntityId|null $id
 	 *
 	 * @throws InvalidArgumentException
 	 */
diff --git a/lib/packages/wikibase/data-model/src/Internal/MapValueHasher.php b/lib/packages/wikibase/data-model/src/Internal/MapValueHasher.php
@@ -4,6 +4,7 @@
 
 use InvalidArgumentException;
 use Traversable;
+use Wikibase\DataModel\Reference;
 
 /**
  * Generates hashes for associative arrays based on the values of their elements.
@@ -27,14 +28,14 @@ public function __construct( $holdOrderIntoAccount = false ) {
 	 *
 	 * @since 0.1
 	 *
-	 * @param Traversable $map
+	 * @param Traversable|Reference[] $map
 	 *
 	 * @return string
 	 * @throws InvalidArgumentException
 	 */
 	public function hash( $map ) {
 		if ( !is_iterable( $map ) ) {
-			throw new InvalidArgumentException( '$map must be an array or an instance of Traversable' );
+			throw new InvalidArgumentException( '$map must be a Reference array or an instance of Traversable' );
 		}
 
 		$hashes = [];
diff --git a/lib/packages/wikibase/data-model/src/ReferenceList.php b/lib/packages/wikibase/data-model/src/ReferenceList.php
@@ -269,7 +269,7 @@ public function unserialize( $serialized ) {
 	 * @return bool
 	 */
 	public function isEmpty() {
-		return empty( $this->references );
+		return $this->references === [];
 	}
 
 	/**
@@ -316,7 +316,7 @@ public function count(): int {
 	 *
 	 * @since 5.0
 	 *
-	 * @return Iterator|Reference[]
+	 * @return Traversable<Reference>
 	 */
 	public function getIterator(): Traversable {
 		return new ArrayIterator( array_values( $this->references ) );
diff --git a/lib/packages/wikibase/data-model/src/SiteLinkList.php b/lib/packages/wikibase/data-model/src/SiteLinkList.php
@@ -5,7 +5,6 @@
 use ArrayIterator;
 use Countable;
 use InvalidArgumentException;
-use Iterator;
 use IteratorAggregate;
 use OutOfBoundsException;
 use Traversable;
@@ -103,7 +102,7 @@ public function setNewSiteLink( $siteId, $pageName, $badges = null ) {
 	 *
 	 * Returns an Iterator of SiteLink in which the keys are the site ids.
 	 *
-	 * @return Iterator|SiteLink[]
+	 * @return Traversable<SiteLink>
 	 */
 	public function getIterator(): Traversable {
 		return new ArrayIterator( $this->siteLinks );
@@ -172,7 +171,7 @@ public function equals( $target ) {
 	 * @return bool
 	 */
 	public function isEmpty() {
-		return empty( $this->siteLinks );
+		return $this->siteLinks === [];
 	}
 
 	/**
diff --git a/lib/packages/wikibase/data-model/src/Snak/PropertyValueSnak.php b/lib/packages/wikibase/data-model/src/Snak/PropertyValueSnak.php
@@ -72,14 +72,14 @@ private function getDataValueSerializationForHash(): string {
 		if ( method_exists( $this->dataValue, 'getSerializationForHash' ) ) {
 			// If our DataValue provides/ needs a special serialization for
 			// hashing, use it (currently only EntityIdValue).
+			// @phan-suppress-next-line PhanUndeclaredMethod
 			return $this->dataValue->getSerializationForHash();
-		} else {
-			$innerSerialization = $this->dataValue->serialize();
 		}
+		$innerSerialization = $this->dataValue->serialize();
 		$className = get_class( $this->dataValue );
 
 		return 'C:' . strlen( $className ) . ':"' . $className .
-			'":' . strlen( $innerSerialization ) . ':{' . $innerSerialization . '}';
+			'":' . strlen( $innerSerialization ?? '' ) . ':{' . $innerSerialization . '}';
 	}
 
 	/**
diff --git a/lib/packages/wikibase/data-model/src/Snak/SnakList.php b/lib/packages/wikibase/data-model/src/Snak/SnakList.php
@@ -241,7 +241,10 @@ public function offsetSet( $index, $value ): void {
 	 * all common code needed for set operations, including
 	 * type checking and offset resolving.
 	 *
-	 * @param int|string $index
+	 * If null is supplied as an index, the next new offset
+	 * will be assigned.
+	 *
+	 * @param int|string|null $index
 	 * @param Snak $value
 	 *
 	 * @throws InvalidArgumentException
diff --git a/lib/packages/wikibase/data-model/src/Statement/StatementByGuidMap.php b/lib/packages/wikibase/data-model/src/Statement/StatementByGuidMap.php
@@ -45,11 +45,12 @@ public function __construct( $statements = [] ) {
 	 * @param Statement $statement
 	 */
 	public function addStatement( Statement $statement ) {
-		if ( $statement->getGuid() === null ) {
+		$index = $statement->getGuid();
+		if ( $index === null ) {
 			throw new InvalidArgumentException( 'Can only add statements that have a non-null GUID' );
 		}
 
-		$this->statements[$statement->getGuid()] = $statement;
+		$this->statements[$index] = $statement;
 	}
 
 	/**
@@ -110,7 +111,7 @@ public function count(): int {
 	 * The iterator has the GUIDs of the statements as keys.
 	 *
 	 * @see IteratorAggregate::getIterator
-	 * @return Iterator|Statement[]
+	 * @return Traversable<Statement>
 	 */
 	public function getIterator(): Traversable {
 		return new ArrayIterator( $this->statements );
diff --git a/lib/packages/wikibase/data-model/src/Statement/StatementList.php b/lib/packages/wikibase/data-model/src/Statement/StatementList.php
@@ -5,8 +5,8 @@
 use ArrayIterator;
 use Countable;
 use InvalidArgumentException;
-use Iterator;
 use IteratorAggregate;
+use Traversable;
 use Wikibase\DataModel\Entity\PropertyId;
 use Wikibase\DataModel\Exception\PropertyChangedException;
 use Wikibase\DataModel\Exception\StatementGuidChangedException;
@@ -254,9 +254,9 @@ public function getMainSnaks(): array {
 	}
 
 	/**
-	 * @return Iterator|Statement[]
+	 * @return Traversable<Statement>
 	 */
-	public function getIterator(): ArrayIterator {
+	public function getIterator(): Traversable {
 		return new ArrayIterator( $this->statements );
 	}
 
@@ -314,7 +314,7 @@ private function statementsEqual( array $statements ): bool {
 	 * @return bool
 	 */
 	public function isEmpty(): bool {
-		return empty( $this->statements );
+		return $this->statements === [];
 	}
 
 	/**
@@ -328,6 +328,9 @@ public function isEmpty(): bool {
 	 */
 	public function getFirstStatementWithGuid( ?string $statementGuid ): ?Statement {
 		$index = $this->getIndexOfFirstStatementWithGuid( $statementGuid );
+		if ( $index === null ) {
+			return null;
+		}
 		return $this->statements[$index] ?? null;
 	}
 
diff --git a/lib/packages/wikibase/data-model/src/Term/AliasGroup.php b/lib/packages/wikibase/data-model/src/Term/AliasGroup.php
@@ -77,7 +77,7 @@ public function getAliases() {
 	 * @return bool
 	 */
 	public function isEmpty() {
-		return empty( $this->aliases );
+		return $this->aliases === [];
 	}
 
 	/**
diff --git a/lib/packages/wikibase/data-model/src/Term/AliasGroupList.php b/lib/packages/wikibase/data-model/src/Term/AliasGroupList.php
@@ -52,7 +52,7 @@ public function count(): int {
 
 	/**
 	 * @see IteratorAggregate::getIterator
-	 * @return Iterator|AliasGroup[]
+	 * @return Traversable<AliasGroup>
 	 */
 	public function getIterator(): Traversable {
 		return new ArrayIterator( $this->groups );
@@ -149,7 +149,7 @@ public function equals( $target ) {
 	 * @return bool
 	 */
 	public function isEmpty() {
-		return empty( $this->groups );
+		return $this->groups === [];
 	}
 
 	/**
diff --git a/lib/packages/wikibase/data-model/src/Term/TermList.php b/lib/packages/wikibase/data-model/src/Term/TermList.php
@@ -5,10 +5,8 @@
 use ArrayIterator;
 use Countable;
 use InvalidArgumentException;
-use Iterator;
 use IteratorAggregate;
 use OutOfBoundsException;
-use Traversable;
 
 /**
  * Unordered list of Term objects.
@@ -60,9 +58,9 @@ public function toTextArray() {
 
 	/**
 	 * @see IteratorAggregate::getIterator
-	 * @return Iterator|Term[]
+	 * @return ArrayIterator<Term>
 	 */
-	public function getIterator(): Traversable {
+	public function getIterator(): ArrayIterator {
 		return new ArrayIterator( $this->terms );
 	}
 
@@ -164,7 +162,7 @@ public function equals( $target ) {
 	 * @return bool
 	 */
 	public function isEmpty() {
-		return empty( $this->terms );
+		return $this->terms === [];
 	}
 
 	/**
diff --git a/repo/domains/crud/src/RouteHandlers/Middleware/StatementRedirectMiddleware.php b/repo/domains/crud/src/RouteHandlers/Middleware/StatementRedirectMiddleware.php
@@ -43,12 +43,15 @@ public function run( Handler $routeHandler, callable $runNext ): Response {
 		$requestedStatementId = $routeHandler->getRequest()->getPathParam( $this->statementIdPathParameter );
 		$statementSubject = $this->getStatementSubject( $requestedSubjectId, $requestedStatementId );
 
-		if ( $statementSubject === null ) {
+		if ( $statementSubject === null || $requestedStatementId === null ) {
 			return $response;
 		}
 
 		foreach ( $statementSubject->getStatements() as $statement ) {
 			$statementId = $statement->getGuid();
+			if ( $statementId === null ) {
+				continue;
+			}
 			if ( strtolower( $statementId ) === strtolower( $requestedStatementId ) ) {
 				$response = $routeHandler->getResponseFactory()->create();
 				$response->setStatus( 308 );
diff --git a/repo/includes/Api/RemoveClaims.php b/repo/includes/Api/RemoveClaims.php
@@ -205,6 +205,9 @@ private function assertStatementListContainsGuids( StatementList $statements, ar
 		/** @var Statement $statement */
 		foreach ( $statements as $statement ) {
 			$guid = $statement->getGuid();
+			if ( $guid === null ) {
+				continue;
+			}
 			// This array is used as a HashSet where only the keys are used.
 			$existingGuids[$guid] = null;
 		}
diff --git a/repo/includes/Merge/StatementsMerger.php b/repo/includes/Merge/StatementsMerger.php
@@ -127,7 +127,11 @@ private function generateRemoveStatementOps( StatementListProvider $source ): Ch
 		$changeOps = new ChangeOps();
 
 		foreach ( $source->getStatements() as $statement ) {
-			$changeOps->add( $this->changeOpFactory->newRemoveStatementOp( $statement->getGuid() ) );
+			$guid = $statement->getGuid();
+			if ( $guid === null ) {
+				continue;
+			}
+			$changeOps->add( $this->changeOpFactory->newRemoveStatementOp( $guid ) );
 		}
 
 		return $changeOps;

Original file line number	Diff line number	Diff line change
`@@ -65,7 +65,6 @@`
`65`	`65`	`$cfg['exclude_analysis_directory_list'] = array_merge(`
`66`	`66`	`$cfg['exclude_analysis_directory_list'],`
`67`	`67`	`[`
`68`		`- 'lib/packages/wikibase/data-model/src/',`
`69`	`68`	`'lib/packages/wikibase/data-model-serialization/src/',`
`70`	`69`	`'lib/packages/wikibase/data-model-services/src/',`
`71`	`70`	`'lib/packages/wikibase/internal-serialization/src/',`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ public function __construct() {`
`21`	`21`	`/**`
`22`	`22`	`* @param string $idSerialization`
`23`	`23`	`*`
`24`		`- * @return ItemId\|PropertyId`
	`24`	`+ * @return EntityId`
`25`	`25`	`* @throws EntityIdParsingException`
`26`	`26`	`*/`
`27`	`27`	`public function parse( $idSerialization ) {`
Original file line number	Diff line number	Diff line change
`@@ -87,7 +87,7 @@ public function getId() {`
`87`	`87`	`/**`
`88`	`88`	`* @since 0.5, can be null since 1.0`
`89`	`89`	`*`
`90`		`- * @param ItemId\|null $id`
	`90`	`+ * @param EntityId\|null $id`
`91`	`91`	`*`
`92`	`92`	`* @throws InvalidArgumentException`
`93`	`93`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ public function count(): int {`
`51`	`51`	`/**`
`52`	`52`	`* @see IteratorAggregate::getIterator`
`53`	`53`	`*`
`54`		`- * @return Iterator\|ItemId[]`
	`54`	`+ * @return Traversable<ItemId>`
`55`	`55`	`*/`
`56`	`56`	`public function getIterator(): Traversable {`
`57`	`57`	`return new ArrayIterator( $this->ids );`
Original file line number	Diff line number	Diff line change
`@@ -90,7 +90,7 @@ public function getId() {`
`90`	`90`	`/**`
`91`	`91`	`* @since 0.5, can be null since 1.0`
`92`	`92`	`*`
`93`		`- * @param PropertyId\|null $id`
	`93`	`+ * @param EntityId\|null $id`
`94`	`94`	`*`
`95`	`95`	`* @throws InvalidArgumentException`
`96`	`96`	`*/`
Original file line number	Diff line number	Diff line change
`@@ -4,6 +4,7 @@`
`4`	`4`
`5`	`5`	`use InvalidArgumentException;`
`6`	`6`	`use Traversable;`
	`7`	`+use Wikibase\DataModel\Reference;`
`7`	`8`
`8`	`9`	`/**`
`9`	`10`	`* Generates hashes for associative arrays based on the values of their elements.`
`@@ -27,14 +28,14 @@ public function __construct( $holdOrderIntoAccount = false ) {`
`27`	`28`	`*`
`28`	`29`	`* @since 0.1`
`29`	`30`	`*`
`30`		`- * @param Traversable $map`
	`31`	`+ * @param Traversable\|Reference[] $map`
`31`	`32`	`*`
`32`	`33`	`* @return string`
`33`	`34`	`* @throws InvalidArgumentException`
`34`	`35`	`*/`
`35`	`36`	`public function hash( $map ) {`
`36`	`37`	`if ( !is_iterable( $map ) ) {`
`37`		`- throw new InvalidArgumentException( '$map must be an array or an instance of Traversable' );`
	`38`	`+ throw new InvalidArgumentException( '$map must be a Reference array or an instance of Traversable' );`
`38`	`39`	`}`
`39`	`40`
`40`	`41`	`$hashes = [];`
Original file line number	Diff line number	Diff line change
`@@ -269,7 +269,7 @@ public function unserialize( $serialized ) {`
`269`	`269`	`* @return bool`
`270`	`270`	`*/`
`271`	`271`	`public function isEmpty() {`
`272`		`- return empty( $this->references );`
	`272`	`+ return $this->references === [];`
`273`	`273`	`}`
`274`	`274`
`275`	`275`	`/**`
`@@ -316,7 +316,7 @@ public function count(): int {`
`316`	`316`	`*`
`317`	`317`	`* @since 5.0`
`318`	`318`	`*`
`319`		`- * @return Iterator\|Reference[]`
	`319`	`+ * @return Traversable<Reference>`
`320`	`320`	`*/`
`321`	`321`	`public function getIterator(): Traversable {`
`322`	`322`	`return new ArrayIterator( array_values( $this->references ) );`