Merge pull request #382 from wmde/derpp

thiemowmde · thiemowmde · commit c2522f2c84d0 · 2015-02-13T17:27:56.000+01:00
Merge master into 3.0.x-dev
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
@@ -10,6 +10,7 @@
 ## Version 2.6.0 (dev)
 
 * Empty strings are now detected as invalid in the `SiteLink` constructor
+* Fixed `EntityIdValue::unserialize` accidentally returning itself
 
 ## Version 2.5.0 (2014-01-20)
 
diff --git a/src/Entity/Entity.php b/src/Entity/Entity.php
@@ -345,7 +345,7 @@ public function setAllAliases( array $aliasLists ) {
 	 *
 	 * @since 0.1
 	 *
-	 * @return static
+	 * @return self
 	 */
 	public function copy() {
 		return unserialize( serialize( $this ) );
diff --git a/src/Entity/EntityIdValue.php b/src/Entity/EntityIdValue.php
@@ -56,7 +56,6 @@ protected function getNumericId() {
 	 * @param string $value
 	 *
 	 * @throws IllegalValueException
-	 * @return EntityIdValue
 	 */
 	public function unserialize( $value ) {
 		list( $entityType, $numericId ) = json_decode( $value );
@@ -67,7 +66,7 @@ public function unserialize( $value ) {
 			throw new IllegalValueException( 'Invalid EntityIdValue serialization.' );
 		}
 
-		return $this->__construct( $entityId );
+		$this->__construct( $entityId );
 	}
 
 	/**
@@ -97,7 +96,7 @@ public function getSortKey() {
 	 *
 	 * @since 0.5
 	 *
-	 * @return EntityId
+	 * @return EntityIdValue
 	 */
 	public function getValue() {
 		return $this;
diff --git a/src/HashableObjectStorage.php b/src/HashableObjectStorage.php
@@ -19,8 +19,6 @@
 class HashableObjectStorage extends \SplObjectStorage implements \Comparable {
 
 	/**
-	 * Constructor.
-	 *
 	 * @since 0.2
 	 *
 	 * @param array $objects
diff --git a/src/Snak/SnakList.php b/src/Snak/SnakList.php
@@ -116,15 +116,16 @@ public function getHash() {
 
 	/**
 	 * Orders the snaks in the list grouping them by property.
-	 * @param $order array of serliazed propertyIds to order by.
+	 *
+	 * @param string[] $order List of serliazed property ids to order by.
 	 *
 	 * @since 0.5
 	 */
 	public function orderByProperty( $order = array() ) {
 		$snaksByProperty = $this->getSnaksByProperty();
 		$orderedProperties = array_unique( array_merge( $order, array_keys( $snaksByProperty ) ) );
 
-		foreach( $orderedProperties as $property ){
+		foreach ( $orderedProperties as $property ) {
 			$snaks = $snaksByProperty[$property];
 			$this->moveSnaksToBottom( $snaks );
 		}
diff --git a/src/Term/AliasGroup.php b/src/Term/AliasGroup.php
@@ -92,7 +92,8 @@ public function equals( $target ) {
 			return true;
 		}
 
-		return $target instanceof self
+		return is_object( $target )
+			&& get_called_class() === get_class( $target )
 			&& $this->languageCode === $target->languageCode
 			&& $this->aliases == $target->aliases;
 	}
diff --git a/src/Term/AliasGroupFallback.php b/src/Term/AliasGroupFallback.php
@@ -61,7 +61,7 @@ public function getActualLanguageCode() {
 	}
 
 	/**
-	 * @return string
+	 * @return string|null
 	 */
 	public function getSourceLanguageCode() {
 		return $this->sourceLanguageCode;
diff --git a/src/Term/Term.php b/src/Term/Term.php
@@ -70,7 +70,8 @@ public function equals( $target ) {
 			return true;
 		}
 
-		return $target instanceof self
+		return is_object( $target )
+			&& get_called_class() === get_class( $target )
 			&& $this->languageCode === $target->languageCode
 			&& $this->text === $target->text;
 	}
diff --git a/src/Term/TermFallback.php b/src/Term/TermFallback.php
@@ -58,7 +58,7 @@ public function getActualLanguageCode() {
 	}
 
 	/**
-	 * @return string
+	 * @return string|null
 	 */
 	public function getSourceLanguageCode() {
 		return $this->sourceLanguageCode;
diff --git a/tests/unit/HashArray/HashArrayTest.php b/tests/unit/HashArray/HashArrayTest.php
@@ -43,7 +43,6 @@ public function instanceProvider() {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testHasElement( HashArray $array ) {
@@ -65,7 +64,6 @@ public function testHasElement( HashArray $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testRemoveElement( HashArray $array ) {
@@ -100,14 +98,18 @@ public function testRemoveElement( HashArray $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testEquals( HashArray $array ) {
 		$this->assertTrue( $array->equals( $array ) );
 		$this->assertFalse( $array->equals( 42 ) );
 	}
 
+	/**
+	 * @param array $elements
+	 *
+	 * @return array[]
+	 */
 	protected function arrayWrap( array $elements ) {
 		return array_map(
 			function ( $element ) {
@@ -118,4 +120,3 @@ function ( $element ) {
 	}
 
 }
-
diff --git a/tests/unit/HashArray/HashArrayWithDuplicatesTest.php b/tests/unit/HashArray/HashArrayWithDuplicatesTest.php
@@ -41,7 +41,6 @@ public function elementInstancesProvider() {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testAddElement( HashArray $array ) {
@@ -62,7 +61,6 @@ public function testAddElement( HashArray $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testRemoveDuplicates( HashArray $array ) {
@@ -93,7 +91,6 @@ public function testRemoveDuplicates( HashArray $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testGetHash( HashArray $array ) {
@@ -121,7 +118,6 @@ public function testGetHash( HashArray $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testIndicesAreUpToDate( HashArray $array ) {
diff --git a/tests/unit/HashArray/HashArrayWithoutDuplicatesTest.php b/tests/unit/HashArray/HashArrayWithoutDuplicatesTest.php
@@ -40,7 +40,6 @@ public function elementInstancesProvider() {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testAddElement( HashArray $array ) {
@@ -80,7 +79,6 @@ public function testAddElement( HashArray $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testRemoveDuplicates( HashArray $array ) {
@@ -96,7 +94,6 @@ public function testRemoveDuplicates( HashArray $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testGetHash( HashArray $array ) {
@@ -116,7 +113,6 @@ public function testGetHash( HashArray $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param HashArray $array
 	 */
 	public function testIndicesAreUpToDate( HashArray $array ) {
diff --git a/tests/unit/ReferenceTest.php b/tests/unit/ReferenceTest.php
@@ -3,6 +3,7 @@
 namespace Wikibase\DataModel\Tests;
 
 use DataValues\StringValue;
+use InvalidArgumentException;
 use Wikibase\DataModel\Entity\PropertyId;
 use Wikibase\DataModel\Reference;
 use Wikibase\DataModel\Snak\PropertyNoValueSnak;
@@ -254,9 +255,29 @@ public function testCanConstructWithSnakArray() {
 		);
 	}
 
-	public function testGivenInvalidArgument_constructorThrowsException() {
-		$this->setExpectedException( 'InvalidArgumentException' );
-		new Reference( new PropertyNoValueSnak( 1 ) );
+	/**
+	 * @dataProvider invalidConstructorArgumentsProvider
+	 * @param mixed $snaks
+	 * @expectedException InvalidArgumentException
+	 */
+	public function testGivenInvalidConstructorArguments_constructorThrowsException( $snaks ) {
+		new Reference( $snaks );
+	}
+
+	public function invalidConstructorArgumentsProvider() {
+		$id1 = new PropertyId( 'P1' );
+
+		return array(
+			array( false ),
+			array( 1 ),
+			array( 0.1 ),
+			array( 'string' ),
+			array( $id1 ),
+			array( new PropertyNoValueSnak( $id1 ) ),
+			array( new PropertyValueSnak( $id1, new StringValue( 'a' ) ) ),
+			array( array( new SnakList() ) ),
+			array( new Reference() ),
+		);
 	}
 
 }
diff --git a/tests/unit/Snak/SnakListTest.php b/tests/unit/Snak/SnakListTest.php
@@ -30,15 +30,12 @@
 class SnakListTest extends HashArrayTest {
 
 	/**
-	 * @see GenericArrayObjectTest::getInstanceClass
+	 * @see HashArrayTest::getInstanceClass
 	 */
 	public function getInstanceClass() {
 		return 'Wikibase\DataModel\Snak\SnakList';
 	}
 
-	/**
-	 * @see GenericArrayObjectTest::elementInstancesProvider
-	 */
 	public function elementInstancesProvider() {
 		$id42 = new PropertyId( 'P42' );
 
@@ -75,7 +72,6 @@ public function constructorProvider() {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param SnakList $array
 	 */
 	public function testHasSnak( SnakList $array ) {
@@ -95,7 +91,6 @@ public function testHasSnak( SnakList $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param SnakList $array
 	 */
 	public function testRemoveSnak( SnakList $array ) {
@@ -128,7 +123,6 @@ public function testRemoveSnak( SnakList $array ) {
 
 	/**
 	 * @dataProvider instanceProvider
-	 *
 	 * @param SnakList $array
 	 */
 	public function testAddSnak( SnakList $array ) {
@@ -241,7 +235,6 @@ public function orderByPropertyProvider() {
 
 	/**
 	 * @dataProvider orderByPropertyProvider
-	 *
 	 * @param SnakList $snakList
 	 * @param SnakList $expected
 	 * @param array $order
diff --git a/tests/unit/Term/AliasGroupTest.php b/tests/unit/Term/AliasGroupTest.php
@@ -4,6 +4,7 @@
 
 use InvalidArgumentException;
 use Wikibase\DataModel\Term\AliasGroup;
+use Wikibase\DataModel\Term\AliasGroupFallback;
 
 /**
  * @covers Wikibase\DataModel\Term\AliasGroup
@@ -59,6 +60,12 @@ public function testGroupDoesNotEqualWhenOrderIsDifferent() {
 		$this->assertFalse( $group->equals( new AliasGroup( 'en', array( 'baz', 'bar', 'foo' ) ) ) );
 	}
 
+	public function testGivenSimilarFallbackObject_equalsReturnsFalse() {
+		$aliasGroup = new AliasGroup( 'de' );
+		$aliasGroupFallback = new AliasGroupFallback( 'de', array(), 'en', null );
+		$this->assertFalse( $aliasGroup->equals( $aliasGroupFallback ) );
+	}
+
 	public function testDuplicatesAreRemoved() {
 		$group = new AliasGroup( 'en', array( 'foo', 'bar', 'spam', 'spam', 'spam', 'foo' ) );
 
diff --git a/tests/unit/Term/TermTest.php b/tests/unit/Term/TermTest.php
@@ -4,6 +4,7 @@
 
 use InvalidArgumentException;
 use Wikibase\DataModel\Term\Term;
+use Wikibase\DataModel\Term\TermFallback;
 
 /**
  * @covers Wikibase\DataModel\Term\Term
@@ -63,4 +64,10 @@ public function testEquality() {
 		$this->assertFalse( $term->equals( new Term( 'spam', 'spam' ) ) );
 	}
 
+	public function testGivenSimilarFallbackObject_equalsReturnsFalse() {
+		$term = new Term( 'de', 'foo' );
+		$termFallback = new TermFallback( 'de', 'foo', 'en', null );
+		$this->assertFalse( $term->equals( $termFallback ) );
+	}
+
 }

Original file line number	Diff line number	Diff line change
`@@ -345,7 +345,7 @@ public function setAllAliases( array $aliasLists ) {`
`345`	`345`	`*`
`346`	`346`	`* @since 0.1`
`347`	`347`	`*`
`348`		`- * @return static`
	`348`	`+ * @return self`
`349`	`349`	`*/`
`350`	`350`	`public function copy() {`
`351`	`351`	`return unserialize( serialize( $this ) );`
Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,6 @@ protected function getNumericId() {`
`56`	`56`	`* @param string $value`
`57`	`57`	`*`
`58`	`58`	`* @throws IllegalValueException`
`59`		`- * @return EntityIdValue`
`60`	`59`	`*/`
`61`	`60`	`public function unserialize( $value ) {`
`62`	`61`	`list( $entityType, $numericId ) = json_decode( $value );`
`@@ -67,7 +66,7 @@ public function unserialize( $value ) {`
`67`	`66`	`throw new IllegalValueException( 'Invalid EntityIdValue serialization.' );`
`68`	`67`	`}`
`69`	`68`
`70`		`- return $this->__construct( $entityId );`
	`69`	`+ $this->__construct( $entityId );`
`71`	`70`	`}`
`72`	`71`
`73`	`72`	`/**`
`@@ -97,7 +96,7 @@ public function getSortKey() {`
`97`	`96`	`*`
`98`	`97`	`* @since 0.5`
`99`	`98`	`*`
`100`		`- * @return EntityId`
	`99`	`+ * @return EntityIdValue`
`101`	`100`	`*/`
`102`	`101`	`public function getValue() {`
`103`	`102`	`return $this;`
Original file line number	Diff line number	Diff line change
`@@ -19,8 +19,6 @@`
`19`	`19`	`class HashableObjectStorage extends \SplObjectStorage implements \Comparable {`
`20`	`20`
`21`	`21`	`/**`
`22`		`- * Constructor.`
`23`		`- *`
`24`	`22`	`* @since 0.2`
`25`	`23`	`*`
`26`	`24`	`* @param array $objects`
Original file line number	Diff line number	Diff line change
`@@ -92,7 +92,8 @@ public function equals( $target ) {`
`92`	`92`	`return true;`
`93`	`93`	`}`
`94`	`94`
`95`		`- return $target instanceof self`
	`95`	`+ return is_object( $target )`
	`96`	`+ && get_called_class() === get_class( $target )`
`96`	`97`	`&& $this->languageCode === $target->languageCode`
`97`	`98`	`&& $this->aliases == $target->aliases;`
`98`	`99`	`}`
Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ public function getActualLanguageCode() {`
`61`	`61`	`}`
`62`	`62`
`63`	`63`	`/**`
`64`		`- * @return string`
	`64`	`+ * @return string\|null`
`65`	`65`	`*/`
`66`	`66`	`public function getSourceLanguageCode() {`
`67`	`67`	`return $this->sourceLanguageCode;`
Original file line number	Diff line number	Diff line change
`@@ -70,7 +70,8 @@ public function equals( $target ) {`
`70`	`70`	`return true;`
`71`	`71`	`}`
`72`	`72`
`73`		`- return $target instanceof self`
	`73`	`+ return is_object( $target )`
	`74`	`+ && get_called_class() === get_class( $target )`
`74`	`75`	`&& $this->languageCode === $target->languageCode`
`75`	`76`	`&& $this->text === $target->text;`
`76`	`77`	`}`
Original file line number	Diff line number	Diff line change
`@@ -58,7 +58,7 @@ public function getActualLanguageCode() {`
`58`	`58`	`}`
`59`	`59`
`60`	`60`	`/**`
`61`		`- * @return string`
	`61`	`+ * @return string\|null`
`62`	`62`	`*/`
`63`	`63`	`public function getSourceLanguageCode() {`
`64`	`64`	`return $this->sourceLanguageCode;`