Cleanup PHP and PHPDoc

Remove some deprecated code
Deprecate some code

control/injector/Injector.php

@@ -918,6 +918,7 @@ class Injector {
 	 * Additional parameters are passed through as
 	 *
 	 * @param string $name
+	 * @param mixed $arguments,... arguments to pass to the constructor
 	 * @return mixed A new instance of the specified object
 	 */
 	public function create($name) {

core/ClassInfo.php

@@ -14,6 +14,8 @@ class ClassInfo {
 
 	/**
 	 * Wrapper for classes getter.
+	 *
+	 * @return array
 	 */
 	public static function allClasses() {
 		return SS_ClassLoader::instance()->getManifest()->getClasses();
@@ -93,7 +95,9 @@ class ClassInfo {
 		);
 
 		foreach ($classes as $class) {
-			if (DataObject::has_own_table($class)) $result[$class] = $class;
+			if (DataObject::has_own_table($class)) {
+				$result[$class] = $class;
+			}
 		}
 
 		return $result;
@@ -143,7 +147,9 @@ class ClassInfo {
 	 * @return array Names of all subclasses as an associative array.
 	 */
 	public static function subclassesFor($class) {
-		if(is_string($class) && !class_exists($class)) return array();
+		if(is_string($class) && !class_exists($class)) {
+			return [];
+		}
 
 		//normalise class case
 		$className = self::class_name($class);
@@ -163,21 +169,14 @@ class ClassInfo {
 	 * eg: self::class_name('dataobJEct'); //returns 'DataObject'
 	 *
 	 * @param string|object $nameOrObject The classname or object you want to normalise
-	 *
 	 * @return string The normalised class name
 	 */
 	public static function class_name($nameOrObject) {
 		if (is_object($nameOrObject)) {
 			return get_class($nameOrObject);
 		} elseif (!self::exists($nameOrObject)) {
-			Deprecation::notice(
+			throw new InvalidArgumentException("Class {$nameOrObject} doesn't exist");
-				'4.0',
-				"ClassInfo::class_name() passed a class that doesn't exist. Support for this will be removed in 4.0",
-				Deprecation::SCOPE_GLOBAL
-			);
-			return $nameOrObject;
 		}
-
 		$reflection = new ReflectionClass($nameOrObject);
 		return $reflection->getName();
 	}

dev/Debug.php

@@ -90,10 +90,8 @@ class Debug {
 	}
 
 	/**
-	 * ??
+	 * @param mixed $val
-	 *
+	 * @return string
-	 * @param unknown_type $val
-	 * @return unknown
 	 */
 	public static function text($val) {
 		if(is_object($val)) {

model/DataList.php

@@ -111,8 +111,9 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * Note that this function is re-entrant - it's safe to call this inside a callback passed to
 	 * alterDataQuery
 	 *
-	 * @param $callback
+	 * @param callable $callback
 	 * @return DataList
+	 * @throws Exception
 	 */
 	public function alterDataQuery($callback) {
 		if ($this->inAlterDataQueryCall) {
@@ -197,7 +198,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @return DataList
 	 */
 	public function where($filter) {
-		return $this->alterDataQuery(function($query) use ($filter){
+		return $this->alterDataQuery(function(DataQuery $query) use ($filter){
 			$query->where($filter);
 		});
 	}
@@ -215,7 +216,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @return DataList
 	 */
 	public function whereAny($filter) {
-		return $this->alterDataQuery(function($query) use ($filter){
+		return $this->alterDataQuery(function(DataQuery $query) use ($filter){
 			$query->whereAny($filter);
 		});
 	}
@@ -250,9 +251,10 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 *
 	 * @param int $limit
 	 * @param int $offset
+	 * @return DataList
 	 */
 	public function limit($limit, $offset = 0) {
-		return $this->alterDataQuery(function($query) use ($limit, $offset){
+		return $this->alterDataQuery(function(DataQuery $query) use ($limit, $offset){
 			$query->limit($limit, $offset);
 		});
 	}
@@ -261,9 +263,10 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * Return a new DataList instance with distinct records or not
 	 *
 	 * @param bool $value
+	 * @return DataList
 	 */
 	public function distinct($value) {
-		return $this->alterDataQuery(function($query) use ($value){
+		return $this->alterDataQuery(function(DataQuery $query) use ($value){
 			$query->distinct($value);
 		});
 	}
@@ -293,9 +296,9 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 			throw new InvalidArgumentException('This method takes zero, one or two arguments');
 		}
 
-		$sort = $col = $dir = null;
-
 		if ($count == 2) {
+			$col = null;
+			$dir = null;
 			list($col, $dir) = func_get_args();
 
 			// Validate direction
@@ -372,6 +375,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * Return a new instance of the list with an added filter
 	 *
 	 * @param array $filterArray
+	 * @return DataList
 	 */
 	public function addFilter($filterArray) {
 		$list = $this;
@@ -423,7 +427,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 			throw new InvalidArgumentException('Incorrect number of arguments passed to exclude()');
 		}
 
-		return $this->alterDataQuery(function($query, $list) use ($whereArguments) {
+		return $this->alterDataQuery(function(DataQuery $query) use ($whereArguments) {
 			$subquery = $query->disjunctiveGroup();
 
 			foreach($whereArguments as $field => $value) {
@@ -432,8 +436,6 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 				$filterType = array_shift($fieldArgs);
 				$modifiers = $fieldArgs;
 
-				// This is here since PHP 5.3 can't call protected/private methods in a closure.
-				$t = singleton($list->dataClass())->dbObject($field);
 				if($filterType) {
 					$className = "{$filterType}Filter";
 				} else {
@@ -443,8 +445,8 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 					$className = 'ExactMatchFilter';
 					array_unshift($modifiers, $filterType);
 				}
-				$t = new $className($field, $value, $modifiers);
+				$filter = Injector::inst()->create($className, $field, $value, $modifiers);
-				$t->apply($subquery);
+				$filter->apply($subquery);
 			}
 		});
 	}
@@ -465,9 +467,12 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 				gettype($callback)
 			));
 		}
+		/** @var ArrayList $output */
 		$output = ArrayList::create();
 		foreach($this as $item) {
-			if(call_user_func($callback, $item, $this)) $output->push($item);
+			if(call_user_func($callback, $item, $this)) {
+				$output->push($item);
+			}
 		}
 		return $output;
 	}
@@ -498,7 +503,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 		}
 
 		return $this->alterDataQuery(
-			function(DataQuery $query, DataList $list) use ($field, &$columnName, $linearOnly) {
+			function(DataQuery $query) use ($field, &$columnName, $linearOnly) {
 				$relations = explode('.', $field);
 				$fieldName = array_pop($relations);
 
@@ -533,7 +538,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @param string $filter - example StartsWith, relates to a filtercontext
 	 * @param array $modifiers - Modifiers to pass to the filter, ie not,nocase
 	 * @param string $value - the value that the filtercontext will use for matching
-	 * @todo Deprecated SearchContexts and pull their functionality into the core of the ORM
+	 * @return DataList
 	 */
 	private function applyFilterContext($field, $filter, $modifiers, $value) {
 		if($filter) {
@@ -581,7 +586,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 			throw new InvalidArgumentException('Incorrect number of arguments passed to exclude()');
 		}
 
-		return $this->alterDataQuery(function($query, $list) use ($whereArguments) {
+		return $this->alterDataQuery(function(DataQuery $query) use ($whereArguments) {
 			$subquery = $query->disjunctiveGroup();
 
 			foreach($whereArguments as $field => $value) {
@@ -590,8 +595,6 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 				$filterType = array_shift($fieldArgs);
 				$modifiers = $fieldArgs;
 
-				// This is here since PHP 5.3 can't call protected/private methods in a closure.
-				$t = singleton($list->dataClass())->dbObject($field);
 				if($filterType) {
 					$className = "{$filterType}Filter";
 				} else {
@@ -601,8 +604,8 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 					$className = 'ExactMatchFilter';
 					array_unshift($modifiers, $filterType);
 				}
-				$t = new $className($field, $value, $modifiers);
+				$filter = Injector::inst()->create($className, $field, $value, $modifiers);
-				$t->exclude($subquery);
+				$filter->exclude($subquery);
 			}
 		});
 	}
@@ -612,16 +615,16 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 *
 	 * The $list passed needs to contain the same dataclass as $this
 	 *
-	 * @param SS_List $list
+	 * @param DataList $list
 	 * @return DataList
 	 * @throws BadMethodCallException
 	 */
-	public function subtract(SS_List $list) {
+	public function subtract(DataList $list) {
-		if($this->dataclass() != $list->dataclass()) {
+		if($this->dataClass() != $list->dataClass()) {
 			throw new InvalidArgumentException('The list passed must have the same dataclass as this class');
 		}
 
-		return $this->alterDataQuery(function($query) use ($list){
+		return $this->alterDataQuery(function(DataQuery $query) use ($list){
 			$query->subtract($list->dataQuery());
 		});
 	}
@@ -639,7 +642,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @return DataList
 	 */
 	public function innerJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) {
-		return $this->alterDataQuery(function($query) use ($table, $onClause, $alias, $order, $parameters){
+		return $this->alterDataQuery(function(DataQuery $query) use ($table, $onClause, $alias, $order, $parameters){
 			$query->innerJoin($table, $onClause, $alias, $order, $parameters);
 		});
 	}
@@ -657,7 +660,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @return DataList
 	 */
 	public function leftJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) {
-		return $this->alterDataQuery(function($query) use ($table, $onClause, $alias, $order, $parameters){
+		return $this->alterDataQuery(function(DataQuery $query) use ($table, $onClause, $alias, $order, $parameters){
 			$query->leftJoin($table, $onClause, $alias, $order, $parameters);
 		});
 	}
@@ -833,6 +836,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 		foreach($this->dataQuery->firstRow()->execute() as $row) {
 			return $this->createDataObject($row);
 		}
+		return null;
 	}
 
 	/**
@@ -844,6 +848,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 		foreach($this->dataQuery->lastRow()->execute() as $row) {
 			return $this->createDataObject($row);
 		}
+		return null;
 	}
 
 	/**
@@ -873,7 +878,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @return DataList
 	 */
 	public function setQueriedColumns($queriedColumns) {
-		return $this->alterDataQuery(function($query) use ($queriedColumns){
+		return $this->alterDataQuery(function(DataQuery $query) use ($queriedColumns){
 			$query->setQueriedColumns($queriedColumns);
 		});
 	}
@@ -1038,6 +1043,8 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * Return a new item to add to this DataList.
 	 *
 	 * @todo This doesn't factor in filters.
+	 * @param array $initialFields
+	 * @return DataObject
 	 */
 	public function newObject($initialFields = null) {
 		$class = $this->dataClass;
@@ -1047,7 +1054,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	/**
 	 * Remove this item by deleting it
 	 *
-	 * @param DataClass $item
+	 * @param DataObject $item
 	 * @todo Allow for amendment of this behaviour - for example, we can remove an item from
 	 * an "ActiveItems" DataList by chaning the status to inactive.
 	 */
@@ -1059,13 +1066,13 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	/**
 	 * Remove an item from this DataList by ID
 	 *
-	 * @param int $itemID - The primary ID
+	 * @param int $itemID The primary ID
 	 */
 	public function removeByID($itemID) {
 		$item = $this->byID($itemID);
 
 		if($item) {
-			return $item->delete();
+			$item->delete();
 		}
 	}
 
@@ -1075,62 +1082,11 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @return DataList
 	 */
 	public function reverse() {
-		return $this->alterDataQuery(function($query){
+		return $this->alterDataQuery(function(DataQuery $query){
 			$query->reverseSort();
 		});
 	}
 
-	/**
-	 * This method won't function on DataLists due to the specific query that it represent
-	 *
-	 * @param mixed $item
-	 */
-	public function push($item) {
-		user_error("Can't call DataList::push() because its data comes from a specific query.", E_USER_ERROR);
-	}
-
-	/**
-	 * This method won't function on DataLists due to the specific query that it represent
-	 *
-	 * @param mixed $item
-	 */
-	public function insertFirst($item) {
-		user_error("Can't call DataList::insertFirst() because its data comes from a specific query.", E_USER_ERROR);
-	}
-
-	/**
-	 * This method won't function on DataLists due to the specific query that it represent
-	 *
-	 */
-	public function shift() {
-		user_error("Can't call DataList::shift() because its data comes from a specific query.", E_USER_ERROR);
-	}
-
-	/**
-	 * This method won't function on DataLists due to the specific query that it represent
-	 *
-	 */
-	public function replace() {
-		user_error("Can't call DataList::replace() because its data comes from a specific query.", E_USER_ERROR);
-	}
-
-	/**
-	 * This method won't function on DataLists due to the specific query that it represent
-	 *
-	 */
-	public function merge() {
-		user_error("Can't call DataList::merge() because its data comes from a specific query.", E_USER_ERROR);
-	}
-
-	/**
-	 * This method won't function on DataLists due to the specific query that it represent
-	 *
-	 */
-	public function removeDuplicates() {
-		user_error("Can't call DataList::removeDuplicates() because its data comes from a specific query.",
-			E_USER_ERROR);
-	}
-
 	/**
 	 * Returns whether an item with $key exists
 	 *
@@ -1138,7 +1094,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @return bool
 	 */
 	public function offsetExists($key) {
-		return ($this->limit(1,$key)->First() != null);
+		return ($this->limit(1,$key)->first() != null);
 	}
 
 	/**
@@ -1148,7 +1104,7 @@ class DataList extends ViewableData implements SS_List, SS_Filterable, SS_Sortab
 	 * @return DataObject
 	 */
 	public function offsetGet($key) {
-		return $this->limit(1, $key)->First();
+		return $this->limit(1, $key)->first();
 	}
 
 	/**

model/DataObject.php

@@ -1,9 +1,7 @@
 <?php
 
-use SilverStripe\Model\FieldType\DBPolymorphicForeignKey;
 use SilverStripe\Model\FieldType\DBField;
 use SilverStripe\Model\FieldType\DBDatetime;
-use SilverStripe\Model\FieldType\DBPrimaryKey;
 use SilverStripe\Model\FieldType\DBComposite;
 use SilverStripe\Model\FieldType\DBClassName;
 
@@ -558,15 +556,15 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 
 	/**
 	 * Helper function to duplicate relations from one object to another
-	 * @param $sourceObject the source object to duplicate from
+	 * @param DataObject $sourceObject the source object to duplicate from
-	 * @param $destinationObject the destination object to populate with the duplicated relations
+	 * @param DataObject $destinationObject the destination object to populate with the duplicated relations
-	 * @param $name the name of the relation to duplicate (e.g. members)
+	 * @param string $name the name of the relation to duplicate (e.g. members)
 	 */
 	private function duplicateRelations($sourceObject, $destinationObject, $name) {
 		$relations = $sourceObject->$name();
 		if ($relations) {
 			if ($relations instanceOf RelationList) {   //many-to-something relation
-				if ($relations->Count() > 0) {  //with more than one thing it is related to
+				if ($relations->count() > 0) {  //with more than one thing it is related to
 					foreach($relations as $relation) {
 						$destinationObject->$name()->add($relation);
 					}
@@ -1432,7 +1430,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * Write the cached components to the database. Cached components could refer to two different instances of the
 	 * same record.
 	 *
-	 * @param $recursive Recursively write components
+	 * @param bool $recursive Recursively write components
 	 * @return DataObject $this
 	 */
 	public function writeComponents($recursive = false) {
@@ -1503,13 +1501,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * @return array Class ancestry
 	 */
 	public function getClassAncestry() {
-		if(!isset(self::$_cache_get_class_ancestry[$this->class])) {
+		return ClassInfo::ancestry(get_class($this));
-			self::$_cache_get_class_ancestry[$this->class] = array($this->class);
-			while(($class=get_parent_class(self::$_cache_get_class_ancestry[$this->class][0])) != "DataObject") {
-				array_unshift(self::$_cache_get_class_ancestry[$this->class], $class);
-			}
-		}
-		return self::$_cache_get_class_ancestry[$this->class];
 	}
 
 	/**
@@ -2006,7 +1998,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * Return all of the database fields in this object
 	 *
 	 * @param string $fieldName Limit the output to a specific field name
-	 * @param string $includeTable If returning a single column, prefix the column with the table name
+	 * @param bool $includeTable If returning a single column, prefix the column with the table name
 	 * in Table.Column(spec) format
 	 * @return array|string|null The database fields, or if searching a single field, just this one field if found
 	 * Field will be a string in ClassName(args) format, or Table.ClassName(args) format if $includeTable is true
@@ -2243,15 +2235,17 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 *
 	 * This is experimental, and is currently only a Postgres-specific enhancement.
 	 *
+	 * @param $class
 	 * @return array or false
 	 */
 	public function database_extensions($class){
 		$extensions = Config::inst()->get($class, 'database_extensions', Config::UNINHERITED);
 
-		if($extensions)
+		if($extensions) {
 			return $extensions;
-		else
+		} else {
 			return false;
+		}
 	}
 
 	/**
@@ -2418,7 +2412,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * need to be overload by solid dataobject, so that the customised actions of that dataobject,
 	 * including that dataobject's extensions customised actions could be added to the EditForm.
 	 *
-	 * @return an Empty FieldList(); need to be overload by solid subclass
+	 * @return FieldList an Empty FieldList(); need to be overload by solid subclass
 	 */
 	public function getCMSActions() {
 		$actions = new FieldList();
@@ -2767,7 +2761,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 */
 	public function hasDatabaseField($field) {
 		return $this->db($field)
-			&& ! self::is_composite_field(get_class($this), $field);
+ 			&& ! self::is_composite_field(get_class($this), $field);
 	}
 
 	/**
@@ -2795,8 +2789,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 		// Remove string-based "constructor-arguments" from the DBField definition
 		if(isset($fieldMap[$field])) {
 			$spec = $fieldMap[$field];
-			if(is_string($spec)) return strtok($spec,'(');
+			if(is_string($spec)) {
-			else return $spec['type'];
+				return strtok($spec,'(');
+			} else {
+				return $spec['type'];
+			}
 		}
 	}
 
@@ -3110,7 +3107,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * Temporary hack to return an association name, based on class, to get around the mangle
 	 * of having to deal with reverse lookup of relationships to determine autogenerated foreign keys.
 	 *
-	 * @return String
+	 * @param string $className
+	 * @return string
 	 */
 	public function getReverseAssociation($className) {
 		if (is_array($this->manyMany())) {
@@ -3310,7 +3308,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	}
 
 	/**
-	 * @var Array Parameters used in the query that built this object.
+	 * @var array Parameters used in the query that built this object.
 	 * This can be used by decorators (e.g. lazy loading) to
 	 * run additional queries using the same context.
 	 */
@@ -3345,7 +3343,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 
 	/**
 	 * @see $sourceQueryParams
-	 * @param array
+	 * @param string $key
+	 * @param string $value
 	 */
 	public function setSourceQueryParam($key, $value) {
 		$this->sourceQueryParams[$key] = $value;
@@ -3353,11 +3352,14 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 
 	/**
 	 * @see $sourceQueryParams
-	 * @return Mixed
+	 * @param string $key
+	 * @return string
 	 */
 	public function getSourceQueryParam($key) {
-		if(isset($this->sourceQueryParams[$key])) return $this->sourceQueryParams[$key];
+		if(isset($this->sourceQueryParams[$key])) {
-		else return null;
+			return $this->sourceQueryParams[$key];
+		}
+		return null;
 	}
 
 	//-------------------------------------------------------------------------------------------//
@@ -3368,8 +3370,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 	 * the value is the type of index.
 	 */
 	public function databaseIndexes() {
-		$has_one = $this->uninherited('has_one',true);
+		$has_one = $this->uninherited('has_one');
-		$classIndexes = $this->uninherited('indexes',true);
+		$classIndexes = $this->uninherited('indexes');
 		//$fileIndexes = $this->uninherited('fileIndexes', true);
 
 		$indexes = array();
@@ -3417,8 +3419,8 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 		}
 
 		// Build any child tables for many_many items
-		if($manyMany = $this->uninherited('many_many', true)) {
+		if($manyMany = $this->uninherited('many_many')) {
-			$extras = $this->uninherited('many_many_extraFields', true);
+			$extras = $this->uninherited('many_many_extraFields');
 			foreach($manyMany as $relationship => $childClass) {
 				// Build field list
 				$manymanyFields = array(
@@ -3682,7 +3684,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 			$fields = array();
 			// try to scaffold a couple of usual suspects
 			if ($this->hasField('Name')) $fields['Name'] = 'Name';
-			if ($this->hasDataBaseField('Title')) $fields['Title'] = 'Title';
+			if ($this->hasDatabaseField('Title')) $fields['Title'] = 'Title';
 			if ($this->hasField('Description')) $fields['Description'] = 'Description';
 			if ($this->hasField('FirstName')) $fields['FirstName'] = 'First Name';
 		}
@@ -3718,8 +3720,6 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
 		$filters = array();
 
 		foreach($this->searchableFields() as $name => $spec) {
-			$filterClass = $spec['filter'];
-
 			if($spec['filter'] instanceof SearchFilter) {
 				$filters[$name] = $spec['filter'];
 			} else {

model/DataQuery.php

@@ -43,7 +43,7 @@ class DataQuery {
 	/**
 	 * Create a new DataQuery.
 	 *
-	 * @param String The name of the DataObject class that you wish to query
+	 * @param string $dataClass The name of the DataObject class that you wish to query
 	 */
 	public function __construct($dataClass) {
 		$this->dataClass = $dataClass;
@@ -59,6 +59,8 @@ class DataQuery {
 
 	/**
 	 * Return the {@link DataObject} class that is being queried.
+	 *
+	 * @return string
 	 */
 	public function dataClass() {
 		return $this->dataClass;
@@ -67,6 +69,8 @@ class DataQuery {
 	/**
 	 * Return the {@link SQLSelect} object that represents the current query; note that it will
 	 * be a clone of the object.
+	 *
+	 * @return SQLSelect
 	 */
 	public function query() {
 		return $this->getFinalisedQuery();
@@ -126,21 +130,12 @@ class DataQuery {
 	/**
 	 * Set up the simplest initial query
 	 */
-	public function initialiseQuery() {
+	protected function initialiseQuery() {
 		// Get the tables to join to.
 		// Don't get any subclass tables - let lazy loading do that.
 		$tableClasses = ClassInfo::ancestry($this->dataClass, true);
-
-		// Error checking
 		if(!$tableClasses) {
-			if(!SS_ClassLoader::instance()->hasManifest()) {
+			throw new InvalidArgumentException("DataQuery::create() Can't find data classes for '{$this->dataClass}'");
-				user_error("DataObjects have been requested before the manifest is loaded. Please ensure you are not"
-					. " querying the database in _config.php.", E_USER_ERROR);
-			} else {
-				user_error("DataList::create Can't find data classes (classes linked to tables) for"
-					. " $this->dataClass. Please ensure you run dev/build after creating a new DataObject.",
-					E_USER_ERROR);
-			}
 		}
 
 		$baseClass = array_shift($tableClasses);
@@ -255,7 +250,6 @@ class DataQuery {
 			if($this->dataClass != $baseClass) {
 				// Get the ClassName values to filter to
 				$classNames = ClassInfo::subclassesFor($this->dataClass);
-				if(!$classNames) user_error("DataList::create() Can't find data sub-classes for '$callerClass'");
 				$classNamesPlaceholders = DB::placeholders($classNames);
 				$query->addWhere(array(
 					"\"$baseClass\".\"ClassName\" IN ($classNamesPlaceholders)" => $classNames
@@ -285,6 +279,7 @@ class DataQuery {
 	 * Ensure that if a query has an order by clause, those columns are present in the select.
 	 *
 	 * @param SQLSelect $query
+	 * @param array $originalSelect
 	 * @return null
 	 */
 	protected function ensureSelectContainsOrderbyColumns($query, $originalSelect = array()) {
@@ -385,6 +380,7 @@ class DataQuery {
 	 *
 	 * @param String $field Unquoted database column name. Will be ANSI quoted
 	 * automatically so must not contain double quotes.
+	 * @return string
 	 */
 	public function max($field) {
 		return $this->aggregate("MAX(\"$field\")");
@@ -393,8 +389,9 @@ class DataQuery {
 	/**
 	 * Return the minimum value of the given field in this DataList
 	 *
-	 * @param String $field Unquoted database column name. Will be ANSI quoted
+	 * @param string $field Unquoted database column name. Will be ANSI quoted
 	 * automatically so must not contain double quotes.
+	 * @return string
 	 */
 	public function min($field) {
 		return $this->aggregate("MIN(\"$field\")");
@@ -403,8 +400,9 @@ class DataQuery {
 	/**
 	 * Return the average value of the given field in this DataList
 	 *
-	 * @param String $field Unquoted database column name. Will be ANSI quoted
+	 * @param string $field Unquoted database column name. Will be ANSI quoted
 	 * automatically so must not contain double quotes.
+	 * @return string
 	 */
 	public function avg($field) {
 		return $this->aggregate("AVG(\"$field\")");
@@ -413,8 +411,9 @@ class DataQuery {
 	/**
 	 * Return the sum of the values of the given field in this DataList
 	 *
-	 * @param String $field Unquoted database column name. Will be ANSI quoted
+	 * @param string $field Unquoted database column name. Will be ANSI quoted
 	 * automatically so must not contain double quotes.
+	 * @return string
 	 */
 	public function sum($field) {
 		return $this->aggregate("SUM(\"$field\")");
@@ -422,6 +421,10 @@ class DataQuery {
 
 	/**
 	 * Runs a raw aggregate expression.  Please handle escaping yourself
+	 *
+	 * @param string $expression An aggregate expression, such as 'MAX("Balance")', or a set of them
+	 * (as an escaped SQL statement)
+	 * @return string
 	 */
 	public function aggregate($expression) {
 		return $this->getFinalisedQuery()->aggregate($expression)->execute()->value();
@@ -445,6 +448,10 @@ class DataQuery {
 
 	/**
 	 * Update the SELECT clause of the query with the columns from the given table
+	 *
+	 * @param SQLSelect $query
+	 * @param string $tableClass
+	 * @param array $columns
 	 */
 	protected function selectColumnsFromTable(SQLSelect &$query, $tableClass, $columns = null) {
 		// Add SQL for multi-value fields
@@ -475,7 +482,8 @@ class DataQuery {
 	/**
 	 * Append a GROUP BY clause to this query.
 	 *
-	 * @param String $groupby Escaped SQL statement
+	 * @param string $groupby Escaped SQL statement
+	 * @return $this
 	 */
 	public function groupby($groupby) {
 		$this->query->addGroupBy($groupby);
@@ -485,7 +493,8 @@ class DataQuery {
 	/**
 	 * Append a HAVING clause to this query.
 	 *
-	 * @param String $having Escaped SQL statement
+	 * @param string $having Escaped SQL statement
+	 * @return $this
 	 */
 	public function having($having) {
 		$this->query->addHaving($having);
@@ -583,6 +592,7 @@ class DataQuery {
 	 *
 	 * @param int $limit
 	 * @param int $offset
+	 * @return $this
 	 */
 	public function limit($limit, $offset = 0) {
 		$this->query->setLimit($limit, $offset);
@@ -610,6 +620,7 @@ class DataQuery {
 	 * will cause the query to appear first. The default is 20, and joins created automatically by the
 	 * ORM have a value of 10.
 	 * @param array $parameters Any additional parameters if the join is a parameterised subquery
+	 * @return $this
 	 */
 	public function innerJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) {
 		if($table) {
@@ -628,6 +639,7 @@ class DataQuery {
 	 * will cause the query to appear first. The default is 20, and joins created automatically by the
 	 * ORM have a value of 10.
 	 * @param array $parameters Any additional parameters if the join is a parameterised subquery
+	 * @return $this
 	 */
 	public function leftJoin($table, $onClause, $alias = null, $order = 20, $parameters = array()) {
 		if($table) {
@@ -760,6 +772,7 @@ class DataQuery {
 		}
 
 		// Join table with associated has_one
+		/** @var DataObject $model */
 		$model = singleton($localClass);
 		$ancestry = $model->getClassAncestry();
 		$foreignKey = $model->getRemoteJoinField($localField, 'has_many', $polymorphic);
@@ -830,6 +843,7 @@ class DataQuery {
 	 *
 	 * @param DataQuery $subtractQuery
 	 * @param string $field
+	 * @return $this
 	 */
 	public function subtract(DataQuery $subtractQuery, $field='ID') {
 		$fieldExpression = $subtractQuery->expressionForField($field);
@@ -846,8 +860,9 @@ class DataQuery {
 	/**
 	 * Select the given fields from the given table.
 	 *
-	 * @param String $table Unquoted table name (will be escaped automatically)
+	 * @param string $table Unquoted table name (will be escaped automatically)
-	 * @param Array $fields Database column names (will be escaped automatically)
+	 * @param array $fields Database column names (will be escaped automatically)
+	 * @return $this
 	 */
 	public function selectFromTable($table, $fields) {
 		$fieldExpressions = array_map(function($item) use($table) {
@@ -882,19 +897,20 @@ class DataQuery {
 	 * @return String The expression used to query this field via this DataQuery
 	 */
 	protected function expressionForField($field) {
-
 		// Prepare query object for selecting this field
 		$query = $this->getFinalisedQuery(array($field));
 
 		// Allow query to define the expression for this field
 		$expression = $query->expressionForField($field);
-		if(!empty($expression)) return $expression;
+		if(!empty($expression)) {
+			return $expression;
+		}
 
 		// Special case for ID, if not provided
 		if($field === 'ID') {
-			$baseClass = ClassInfo::baseDataClass($this->dataClass);
+			return DataObject::quoted_column('ID', $this->dataClass);
-			return "\"$baseClass\".\"ID\"";
 		}
+		return null;
 	}
 
 	/**
@@ -918,17 +934,27 @@ class DataQuery {
 	/**
 	 * Set an arbitrary query parameter, that can be used by decorators to add additional meta-data to the query.
 	 * It's expected that the $key will be namespaced, e.g, 'Versioned.stage' instead of just 'stage'.
+	 *
+	 * @param string $key
+	 * @param string $value
+	 * @return $this
 	 */
 	public function setQueryParam($key, $value) {
 		$this->queryParams[$key] = $value;
+		return $this;
 	}
 
 	/**
 	 * Set an arbitrary query parameter, that can be used by decorators to add additional meta-data to the query.
+	 *
+	 * @param string $key
+	 * @return string
 	 */
 	public function getQueryParam($key) {
-		if(isset($this->queryParams[$key])) return $this->queryParams[$key];
+		if(isset($this->queryParams[$key])) {
-		else return null;
+			return $this->queryParams[$key];
+		}
+		return null;
 	}
 
 	/**
@@ -958,7 +984,7 @@ class DataQuery_SubGroup extends DataQuery implements SQLConditionGroup {
 	protected $whereQuery;
 
 	public function __construct(DataQuery $base, $connective) {
-		$this->dataClass = $base->dataClass;
+		parent::__construct($base->dataClass);
 		$this->query = $base->query;
 		$this->whereQuery = new SQLSelect();
 		$this->whereQuery->setConnective($connective);

model/HasManyList.php

@@ -51,7 +51,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 +83,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 +95,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) {

model/queries/SQLExpression.php

@@ -23,24 +23,6 @@ abstract class SQLExpression {
 	 */
 	protected $replacementsNew = array();
 
-	/**
-	 * @deprecated since version 4.0
-	 */
-	public function __get($field) {
-		Deprecation::notice('4.0', 'use get{Field} to get the necessary protected field\'s value');
-		return $this->$field;
-	}
-
-	/**
-	 * @deprecated since version 4.0
-	 */
-	public function __set($field, $value) {
-		Deprecation::notice('4.0', 'use set{Field} to set the necessary protected field\'s value');
-		return $this->$field = $value;
-	}
-
-
-
 	/**
 	 * Swap some text in the SQL query with another.
 	 *
@@ -125,7 +107,7 @@ abstract class SQLExpression {
 	 * Copies the query parameters contained in this object to another
 	 * SQLExpression
 	 *
-	 * @param SQLExpression $expression The object to copy properties to
+	 * @param SQLExpression $object The object to copy properties to
 	 */
 	protected function copyTo(SQLExpression $object) {
 		$target = array_keys(get_object_vars($object));

search/SearchContext.php

@@ -108,11 +108,12 @@ class SearchContext extends Object {
 	 *  If a filter is applied to a relationship in dot notation,
 	 *  the parameter name should have the dots replaced with double underscores,
 	 *  for example "Comments__Name" instead of the filter name "Comments.Name".
-	 * @param string|array $sort Database column to sort on.
+	 * @param array|bool|string $sort Database column to sort on.
 	 *  Falls back to {@link DataObject::$default_sort} if not provided.
-	 * @param string|array $limit
+	 * @param array|bool|string $limit
 	 * @param DataList $existingQuery
 	 * @return DataList
+	 * @throws Exception
 	 */
 	public function getQuery($searchParams, $sort = false, $limit = false, $existingQuery = null) {
 		if($existingQuery) {
@@ -140,7 +141,6 @@ class SearchContext extends Object {
 		$query = $query->sort($sort);
 
 		// hack to work with $searchParems when it's an Object
-		$searchParamArray = array();
 		if (is_object($searchParams)) {
 			$searchParamArray = $searchParams->getVars();
 		} else {
@@ -171,9 +171,10 @@ class SearchContext extends Object {
 	 * @todo rearrange start and limit params to reflect DataObject
 	 *
 	 * @param array $searchParams
-	 * @param string|array $sort
+	 * @param array|bool|string $sort
-	 * @param string|array $limit
+	 * @param array|bool|string $limit
 	 * @return SS_List
+	 * @throws Exception
 	 */
 	public function getResults($searchParams, $sort = false, $limit = false) {
 		$searchParams = array_filter((array)$searchParams, array($this,'clearEmptySearchFields'));
@@ -186,7 +187,7 @@ class SearchContext extends Object {
 	 * Callback map function to filter fields with empty values from
 	 * being included in the search expression.
 	 *
-	 * @param unknown_type $value
+	 * @param mixed $value
 	 * @return boolean
 	 */
 	public function clearEmptySearchFields($value) {

search/filters/ExactMatchFilter.php

@@ -27,6 +27,7 @@ class ExactMatchFilter extends SearchFilter {
 	/**
 	 * Applies an exact match (equals) on a field value.
 	 *
+	 * @param DataQuery $query
 	 * @return DataQuery
 	 */
 	protected function applyOne(DataQuery $query) {
@@ -36,6 +37,7 @@ class ExactMatchFilter extends SearchFilter {
 	/**
 	 * Excludes an exact match (equals) on a field value.
 	 *
+	 * @param DataQuery $query
 	 * @return DataQuery
 	 */
 	protected function excludeOne(DataQuery $query) {
@@ -81,6 +83,7 @@ class ExactMatchFilter extends SearchFilter {
 	 * Applies an exact match (equals) on a field value against multiple
 	 * possible values.
 	 *
+	 * @param DataQuery $query
 	 * @return DataQuery
 	 */
 	protected function applyMany(DataQuery $query) {
@@ -91,6 +94,7 @@ class ExactMatchFilter extends SearchFilter {
 	 * Excludes an exact match (equals) on a field value against multiple
 	 * possible values.
 	 *
+	 * @param DataQuery $query
 	 * @return DataQuery
 	 */
 	protected function excludeMany(DataQuery $query) {