Merge "Re-enable Phan and fix warnings for `data-model-services'"

jenkins-bot · Gerrit Code Review · commit 445d76440ba2 · 2025-03-03T15:42:18.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-services/src/',
 		'lib/packages/wikibase/internal-serialization/src/',
 		'../../extensions/Babel/',
 		'../../extensions/CirrusSearch/',
diff --git a/lib/packages/wikibase/data-model-services/src/DataValue/ValuesFinder.php b/lib/packages/wikibase/data-model-services/src/DataValue/ValuesFinder.php
@@ -44,7 +44,14 @@ public function findFromSnaks( array $snaks, $dataType ) {
 				$this->isMatchingDataType( $snak->getPropertyId(), $dataType )
 			) {
 				$dataValue = $snak->getDataValue();
-				$found[$dataValue->getHash()] = $dataValue;
+				if ( method_exists( $dataValue, 'getHash' ) ) {
+					// We can only tag values as found that are
+					// hashable. DataValueObject should, but `getDataValue`
+					// only returns the `DataValue` interface, which doesn't
+					// define `getHash`
+					// @phan-suppress-next-line PhanUndeclaredMethod
+					$found[$dataValue->getHash()] = $dataValue;
+				}
 			}
 		}
 
diff --git a/lib/packages/wikibase/data-model-services/src/Diff/Internal/SiteLinkListPatcher.php b/lib/packages/wikibase/data-model-services/src/Diff/Internal/SiteLinkListPatcher.php
@@ -59,7 +59,7 @@ public function getPatchedSiteLinkList( SiteLinkList $siteLinks, Diff $patch ) {
 	 * @return ItemId[]|null
 	 */
 	private function getBadgesFromSiteLinkData( array $siteLinkData ) {
-		if ( !array_key_exists( 'badges', $siteLinkData ) ) {
+		if ( !array_key_exists( 'badges', $siteLinkData ) || !is_array( $siteLinkData['badges'] ) ) {
 			return null;
 		}
 
diff --git a/lib/packages/wikibase/data-model-services/src/Diff/ItemDiffer.php b/lib/packages/wikibase/data-model-services/src/Diff/ItemDiffer.php
@@ -49,16 +49,17 @@ public function canDiffEntityType( $entityType ) {
 	 * @throws InvalidArgumentException
 	 */
 	public function diffEntities( EntityDocument $from, EntityDocument $to ) {
-		$this->assertIsItem( $from );
-		$this->assertIsItem( $to );
+		$fromItem = $this->assertIsItemAndCast( $from );
+		$toItem = $this->assertIsItemAndCast( $to );
 
-		return $this->diffItems( $from, $to );
+		return $this->diffItems( $fromItem, $toItem );
 	}
 
-	private function assertIsItem( EntityDocument $item ) {
+	private function assertIsItemAndCast( EntityDocument $item ): Item {
 		if ( !( $item instanceof Item ) ) {
 			throw new InvalidArgumentException( '$item must be an instance of Item' );
 		}
+		return $item;
 	}
 
 	public function diffItems( Item $from, Item $to ) {
@@ -108,8 +109,8 @@ static function( ItemId $id ) {
 	 * @throws InvalidArgumentException
 	 */
 	public function getConstructionDiff( EntityDocument $entity ) {
-		$this->assertIsItem( $entity );
-		return $this->diffEntities( new Item(), $entity );
+		$item = $this->assertIsItemAndCast( $entity );
+		return $this->diffEntities( new Item(), $item );
 	}
 
 	/**
@@ -119,8 +120,8 @@ public function getConstructionDiff( EntityDocument $entity ) {
 	 * @throws InvalidArgumentException
 	 */
 	public function getDestructionDiff( EntityDocument $entity ) {
-		$this->assertIsItem( $entity );
-		return $this->diffEntities( $entity, new Item() );
+		$item = $this->assertIsItemAndCast( $entity );
+		return $this->diffEntities( $item, new Item() );
 	}
 
 }
diff --git a/lib/packages/wikibase/data-model-services/src/Diff/ItemPatcher.php b/lib/packages/wikibase/data-model-services/src/Diff/ItemPatcher.php
@@ -53,15 +53,16 @@ public function canPatchEntityType( $entityType ) {
 	 * @throws InvalidArgumentException
 	 */
 	public function patchEntity( EntityDocument $entity, EntityDiff $patch ) {
-		$this->assertIsItem( $entity );
+		$item = $this->assertIsItemAndCast( $entity );
 
-		$this->patchItem( $entity, $patch );
+		$this->patchItem( $item, $patch );
 	}
 
-	private function assertIsItem( EntityDocument $item ) {
+	private function assertIsItemAndCast( EntityDocument $item ): Item {
 		if ( !( $item instanceof Item ) ) {
 			throw new InvalidArgumentException( '$item must be an instance of Item' );
 		}
+		return $item;
 	}
 
 	private function patchItem( Item $item, EntityDiff $patch ) {
diff --git a/lib/packages/wikibase/data-model-services/src/Diff/PropertyDiffer.php b/lib/packages/wikibase/data-model-services/src/Diff/PropertyDiffer.php
@@ -49,16 +49,17 @@ public function canDiffEntityType( $entityType ) {
 	 * @throws InvalidArgumentException
 	 */
 	public function diffEntities( EntityDocument $from, EntityDocument $to ) {
-		$this->assertIsProperty( $from );
-		$this->assertIsProperty( $to );
+		$fromProperty = $this->assertIsPropertyAndCast( $from );
+		$toProperty = $this->assertIsPropertyAndCast( $to );
 
-		return $this->diffProperties( $from, $to );
+		return $this->diffProperties( $fromProperty, $toProperty );
 	}
 
-	private function assertIsProperty( EntityDocument $property ) {
+	private function assertIsPropertyAndCast( EntityDocument $property ): Property {
 		if ( !( $property instanceof Property ) ) {
 			throw new InvalidArgumentException( '$property must be an instance of Property' );
 		}
+		return $property;
 	}
 
 	public function diffProperties( Property $from, Property $to ) {
@@ -104,11 +105,10 @@ private function toDiffArray( Property $property ) {
 	 * @throws InvalidArgumentException
 	 */
 	public function getConstructionDiff( EntityDocument $entity ) {
-		$this->assertIsProperty( $entity );
+		$property = $this->assertIsPropertyAndCast( $entity );
 
-		/** @var Property $entity */
-		$diffOps = $this->diffPropertyArrays( [], $this->toDiffArray( $entity ) );
-		$diffOps['claim'] = $this->statementListDiffer->getDiff( new StatementList(), $entity->getStatements() );
+		$diffOps = $this->diffPropertyArrays( [], $this->toDiffArray( $property ) );
+		$diffOps['claim'] = $this->statementListDiffer->getDiff( new StatementList(), $property->getStatements() );
 
 		return new EntityDiff( $diffOps );
 	}
@@ -120,11 +120,10 @@ public function getConstructionDiff( EntityDocument $entity ) {
 	 * @throws InvalidArgumentException
 	 */
 	public function getDestructionDiff( EntityDocument $entity ) {
-		$this->assertIsProperty( $entity );
+		$property = $this->assertIsPropertyAndCast( $entity );
 
-		/** @var Property $entity */
-		$diffOps = $this->diffPropertyArrays( $this->toDiffArray( $entity ), [] );
-		$diffOps['claim'] = $this->statementListDiffer->getDiff( $entity->getStatements(), new StatementList() );
+		$diffOps = $this->diffPropertyArrays( $this->toDiffArray( $property ), [] );
+		$diffOps['claim'] = $this->statementListDiffer->getDiff( $property->getStatements(), new StatementList() );
 
 		return new EntityDiff( $diffOps );
 	}
diff --git a/lib/packages/wikibase/data-model-services/src/Diff/PropertyPatcher.php b/lib/packages/wikibase/data-model-services/src/Diff/PropertyPatcher.php
@@ -46,15 +46,16 @@ public function canPatchEntityType( $entityType ) {
 	 * @throws InvalidArgumentException
 	 */
 	public function patchEntity( EntityDocument $entity, EntityDiff $patch ) {
-		$this->assertIsProperty( $entity );
+		$property = $this->assertIsPropertyAndCast( $entity );
 
-		$this->patchProperty( $entity, $patch );
+		$this->patchProperty( $property, $patch );
 	}
 
-	private function assertIsProperty( EntityDocument $property ) {
+	private function assertIsPropertyAndCast( EntityDocument $property ): Property {
 		if ( !( $property instanceof Property ) ) {
 			throw new InvalidArgumentException( '$property must be an instance of Property' );
 		}
+		return $property;
 	}
 
 	private function patchProperty( Property $property, EntityDiff $patch ) {
diff --git a/lib/packages/wikibase/data-model-services/src/Diff/StatementListDiffer.php b/lib/packages/wikibase/data-model-services/src/Diff/StatementListDiffer.php
@@ -47,7 +47,12 @@ private function toDiffArray( StatementList $statementList ): array {
 		 * @var Statement $statement
 		 */
 		foreach ( $statementList as $statement ) {
-			$statementArray[$statement->getGuid()] = $statement;
+			$guid = $statement->getGuid();
+			if ( $guid === null ) {
+				$statementArray[] = $statement;
+			} else {
+				$statementArray[$guid] = $statement;
+			}
 		}
 
 		return $statementArray;
diff --git a/lib/packages/wikibase/data-model-services/src/Lookup/InMemoryEntityLookup.php b/lib/packages/wikibase/data-model-services/src/Lookup/InMemoryEntityLookup.php
@@ -72,7 +72,7 @@ public function addException( EntityLookupException $exception ) {
 	 * @param EntityId $entityId
 	 *
 	 * @throws EntityLookupException
-	 * @return EntityDocument
+	 * @return ?EntityDocument
 	 */
 	public function getEntity( EntityId $entityId ) {
 		$this->throwExceptionIfNeeded( $entityId );
diff --git a/lib/packages/wikibase/data-model-services/src/Lookup/ItemLookupException.php b/lib/packages/wikibase/data-model-services/src/Lookup/ItemLookupException.php
@@ -3,6 +3,7 @@
 namespace Wikibase\DataModel\Services\Lookup;
 
 use Exception;
+use LogicException;
 use Wikibase\DataModel\Entity\ItemId;
 
 /**
@@ -30,7 +31,12 @@ public function __construct( ItemId $itemId, $message = null, ?Exception $previo
 	 * @return ItemId
 	 */
 	public function getItemId() {
-		return $this->getEntityId();
+		$itemId = $this->getEntityId();
+		if ( !( $itemId instanceof ItemId ) ) {
+			throw new LogicException( 'expected $itemId to be of type ItemId' );
+		}
+
+		return $itemId;
 	}
 
 }
diff --git a/lib/packages/wikibase/data-model-services/src/Lookup/PropertyLookupException.php b/lib/packages/wikibase/data-model-services/src/Lookup/PropertyLookupException.php
@@ -3,6 +3,7 @@
 namespace Wikibase\DataModel\Services\Lookup;
 
 use Exception;
+use LogicException;
 use Wikibase\DataModel\Entity\PropertyId;
 
 /**
@@ -30,7 +31,11 @@ public function __construct( PropertyId $propertyId, $message = null, ?Exception
 	 * @return PropertyId
 	 */
 	public function getPropertyId() {
-		return $this->getEntityId();
+		$propertyId = $this->getEntityId();
+		if ( !( $propertyId instanceof PropertyId ) ) {
+			throw new LogicException( 'expected $propertyId to be of type PropertyId' );
+		}
+		return $propertyId;
 	}
 
 }

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-services/src/',`
`69`	`68`	`'lib/packages/wikibase/internal-serialization/src/',`
`70`	`69`	`'../../extensions/Babel/',`
`71`	`70`	`'../../extensions/CirrusSearch/',`
Original file line number	Diff line number	Diff line change
`@@ -59,7 +59,7 @@ public function getPatchedSiteLinkList( SiteLinkList $siteLinks, Diff $patch ) {`
`59`	`59`	`* @return ItemId[]\|null`
`60`	`60`	`*/`
`61`	`61`	`private function getBadgesFromSiteLinkData( array $siteLinkData ) {`
`62`		`- if ( !array_key_exists( 'badges', $siteLinkData ) ) {`
	`62`	`+ if ( !array_key_exists( 'badges', $siteLinkData ) \|\| !is_array( $siteLinkData['badges'] ) ) {`
`63`	`63`	`return null;`
`64`	`64`	`}`
`65`	`65`
Original file line number	Diff line number	Diff line change
`@@ -53,15 +53,16 @@ public function canPatchEntityType( $entityType ) {`
`53`	`53`	`* @throws InvalidArgumentException`
`54`	`54`	`*/`
`55`	`55`	`public function patchEntity( EntityDocument $entity, EntityDiff $patch ) {`
`56`		`- $this->assertIsItem( $entity );`
	`56`	`+ $item = $this->assertIsItemAndCast( $entity );`
`57`	`57`
`58`		`- $this->patchItem( $entity, $patch );`
	`58`	`+ $this->patchItem( $item, $patch );`
`59`	`59`	`}`
`60`	`60`
`61`		`- private function assertIsItem( EntityDocument $item ) {`
	`61`	`+ private function assertIsItemAndCast( EntityDocument $item ): Item {`
`62`	`62`	`if ( !( $item instanceof Item ) ) {`
`63`	`63`	`throw new InvalidArgumentException( '$item must be an instance of Item' );`
`64`	`64`	`}`
	`65`	`+ return $item;`
`65`	`66`	`}`
`66`	`67`
`67`	`68`	`private function patchItem( Item $item, EntityDiff $patch ) {`
Original file line number	Diff line number	Diff line change
`@@ -46,15 +46,16 @@ public function canPatchEntityType( $entityType ) {`
`46`	`46`	`* @throws InvalidArgumentException`
`47`	`47`	`*/`
`48`	`48`	`public function patchEntity( EntityDocument $entity, EntityDiff $patch ) {`
`49`		`- $this->assertIsProperty( $entity );`
	`49`	`+ $property = $this->assertIsPropertyAndCast( $entity );`
`50`	`50`
`51`		`- $this->patchProperty( $entity, $patch );`
	`51`	`+ $this->patchProperty( $property, $patch );`
`52`	`52`	`}`
`53`	`53`
`54`		`- private function assertIsProperty( EntityDocument $property ) {`
	`54`	`+ private function assertIsPropertyAndCast( EntityDocument $property ): Property {`
`55`	`55`	`if ( !( $property instanceof Property ) ) {`
`56`	`56`	`throw new InvalidArgumentException( '$property must be an instance of Property' );`
`57`	`57`	`}`
	`58`	`+ return $property;`
`58`	`59`	`}`
`59`	`60`
`60`	`61`	`private function patchProperty( Property $property, EntityDiff $patch ) {`
Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,7 @@ public function addException( EntityLookupException $exception ) {`
`72`	`72`	`* @param EntityId $entityId`
`73`	`73`	`*`
`74`	`74`	`* @throws EntityLookupException`
`75`		`- * @return EntityDocument`
	`75`	`+ * @return ?EntityDocument`
`76`	`76`	`*/`
`77`	`77`	`public function getEntity( EntityId $entityId ) {`
`78`	`78`	`$this->throwExceptionIfNeeded( $entityId );`