From 8737ddefbaea13505737fe2fc231c4ea36b24ea3 Mon Sep 17 00:00:00 2001
From: Daniel Hensby <dhensby@silverstripe.com>
Date: Fri, 19 Aug 2016 10:37:58 +0100
Subject: [PATCH] Cleanup of RelationList, HasManyList and ManyManyList

---
 model/HasManyList.php  | 13 ++++++++++---
 model/ManyManyList.php | 13 ++++++++-----
 model/RelationList.php |  8 +++++++-
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/model/HasManyList.php b/model/HasManyList.php
index a9cac0818..afc4979f3 100644
--- a/model/HasManyList.php
+++ b/model/HasManyList.php
@@ -8,6 +8,9 @@
  */
 class HasManyList extends RelationList {
 
+	/**
+	 * @var string
+	 */
 	protected $foreignKey;
 
 	/**
@@ -34,6 +37,10 @@ class HasManyList extends RelationList {
 		return $this->foreignKey;
 	}
 
+	/**
+	 * @param null|int $id
+	 * @return array
+	 */
 	protected function foreignIDFilter($id = null) {
 		if ($id === null) $id = $this->getForeignID();
 
@@ -51,7 +58,7 @@ class HasManyList extends RelationList {
 	 *
 	 * It does so by setting the relationFilters.
 	 *
-	 * @param $item The DataObject to be added, or its ID
+	 * @param DataObject|int $item The DataObject to be added, or its ID
 	 */
 	public function add($item) {
 		if(is_numeric($item)) {
@@ -83,7 +90,7 @@ class HasManyList extends RelationList {
 	 *
 	 * Doesn't actually remove the item, it just clears the foreign key value.
 	 *
-	 * @param $itemID The ID of the item to be removed.
+	 * @param int $itemID The ID of the item to be removed.
 	 */
 	public function removeByID($itemID) {
 		$item = $this->byID($itemID);
@@ -95,7 +102,7 @@ class HasManyList extends RelationList {
 	 * Remove an item from this relation.
 	 * Doesn't actually remove the item, it just clears the foreign key value.
 	 *
-	 * @param $item The DataObject to be removed
+	 * @param DataObject $item The DataObject to be removed
 	 * @todo Maybe we should delete the object instead?
 	 */
 	public function remove($item) {
diff --git a/model/ManyManyList.php b/model/ManyManyList.php
index 96eb7332e..e5b041195 100644
--- a/model/ManyManyList.php
+++ b/model/ManyManyList.php
@@ -47,7 +47,7 @@ class ManyManyList extends RelationList {
 	 * @param string $joinTable The name of the table whose entries define the content of this many_many relation.
 	 * @param string $localKey The key in the join table that maps to the dataClass' PK.
 	 * @param string $foreignKey The key in the join table that maps to joined class' PK.
-	 * @param string $extraFields A map of field => fieldtype of extra fields on the join table.
+	 * @param array $extraFields A map of field => fieldtype of extra fields on the join table.
 	 *
 	 * @example new ManyManyList('Group','Group_Members', 'GroupID', 'MemberID');
 	 */
@@ -151,9 +151,9 @@ class ManyManyList extends RelationList {
 	 * Return a filter expression for when getting the contents of the
 	 * relationship for some foreign ID
 	 *
-	 * @param int $id
+	 * @param int|null $id
 	 *
-	 * @return string
+	 * @return array
 	 */
 	protected function foreignIDFilter($id = null) {
 		if ($id === null) {
@@ -176,7 +176,7 @@ class ManyManyList extends RelationList {
 	 * entries. However some subclasses of ManyManyList (Member_GroupSet) modify foreignIDFilter to
 	 * include additional calculated entries, so we need different filters when reading and when writing
 	 *
-	 * @param array|integer $id (optional) An ID or an array of IDs - if not provided, will use the current ids
+	 * @param array|int|null $id (optional) An ID or an array of IDs - if not provided, will use the current ids
 	 * as per getForeignID
 	 * @return array Condition In array(SQL => parameters format)
 	 */
@@ -188,7 +188,10 @@ class ManyManyList extends RelationList {
 	 * Add an item to this many_many relationship
 	 * Does so by adding an entry to the joinTable.
 	 *
-	 * @param mixed $item
+	 * @throws InvalidArgumentException
+	 * @throws Exception
+	 *
+	 * @param DataObject|int $item
 	 * @param array $extraFields A map of additional columns to insert into the joinTable.
 	 * Column names should be ANSI quoted.
 	 */
diff --git a/model/RelationList.php b/model/RelationList.php
index bd14f98d2..dfd39e19a 100644
--- a/model/RelationList.php
+++ b/model/RelationList.php
@@ -10,6 +10,9 @@
  */
 abstract class RelationList extends DataList {
 
+	/**
+	 * @return string|null
+	 */
 	public function getForeignID() {
 		return $this->dataQuery->getQueryParam('Foreign.ID');
 	}
@@ -19,6 +22,8 @@ abstract class RelationList extends DataList {
 	 * the given foreign ID.
 	 *
 	 * @param int|array $id An ID or an array of IDs.
+	 *
+	 * @return DataList
 	 */
 	public function forForeignID($id) {
 		// Turn a 1-element array into a simple value
@@ -27,7 +32,8 @@ abstract class RelationList extends DataList {
 		// Calculate the new filter
 		$filter = $this->foreignIDFilter($id);
 
-		$list = $this->alterDataQuery(function($query, $list) use ($id, $filter){
+		$list = $this->alterDataQuery(function($query) use ($id, $filter){
+			/** @var DataQuery $query */
 			// Check if there is an existing filter, remove if there is
 			$currentFilter = $query->getQueryParam('Foreign.Filter');
 			if($currentFilter) {