Throw exception rather than ignoring non-iterable

Just ran into this while working on new term store stuff

Author: JeroenDeDauw

RELEASE-NOTES.md

@@ -1,5 +1,10 @@
 # Wikibase DataModel release notes
 
+## Version 9.2.0 (dev)
+
+* `TermList` now throws `InvalidArgumentException` when given non-iterable rather than failing silently
+* `SiteLinkList` now throws `InvalidArgumentException` when given non-iterable rather than failing silently
+
 ## Version 9.1.0 (2019-01-24)
 
 * Raised minimum PHP version to 7.0 or HHVM

composer.json

@@ -49,7 +49,7 @@
 	},
 	"extra": {
 		"branch-alias": {
-			"dev-master": "9.1.x-dev"
+			"dev-master": "9.2.x-dev"
 		}
 	},
 	"scripts": {

src/SiteLinkList.php

@@ -35,6 +35,10 @@ class SiteLinkList implements IteratorAggregate, Countable, Comparable {
 	 * @throws InvalidArgumentException
 	 */
 	public function __construct( /* iterable */ $siteLinks = [] ) {
+		if ( !is_array( $siteLinks ) && !( $siteLinks instanceof \Traversable ) ) {
+			throw new InvalidArgumentException( '$siteLinks must be iterable' );
+		}
+
 		foreach ( $siteLinks as $siteLink ) {
 			if ( !( $siteLink instanceof SiteLink ) ) {
 				throw new InvalidArgumentException( 'Every element of $siteLinks must be an instance of SiteLink' );

src/Term/TermList.php

@@ -196,6 +196,10 @@ public function clear() {
 	 * @throws InvalidArgumentException
 	 */
 	public function addAll( /* iterable */ $terms ) {
+		if ( !is_array( $terms ) && !( $terms instanceof \Traversable ) ) {
+			throw new InvalidArgumentException( '$terms must be iterable' );
+		}
+
 		foreach ( $terms as $term ) {
 			if ( !( $term instanceof Term ) ) {
 				throw new InvalidArgumentException( 'Every element in $terms must be an instance of Term' );

tests/unit/SiteLinkListTest.php

@@ -346,4 +346,9 @@ public function testCanConstructWithIterable() {
 		);
 	}
 
+	public function testWhenProvidingNonIterable_constructorThrowsException() {
+		$this->setExpectedException( InvalidArgumentException::class );
+		new SiteLinkList( new SiteLink( 'enwiki', 'foo' ) );
+	}
+
 }

tests/unit/Term/TermListTest.php

@@ -479,4 +479,11 @@ public function testWhenProvidingNonTerms_addAllThrowsException() {
 		$list->addAll( [ 'no-a-term' ] );
 	}
 
+	public function testWhenProvidingNonIterable_addAllThrowsException() {
+		$list = new TermList( [] );
+
+		$this->setExpectedException( InvalidArgumentException::class );
+		$list->addAll( new Term( 'en', 'foo' ) );
+	}
+
 }