Merge pull request #9707 from robbieaverill/pulls/4.7/exceptions

This commit is contained in:
Guy Marriott 2020-10-01 17:16:43 -07:00 committed by GitHub
commit 478d487f0e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
34 changed files with 128 additions and 151 deletions

View File

@ -27,7 +27,7 @@ use SilverStripe\ORM\DataObject;
class Player extends DataObject class Player extends DataObject
{ {
private static $has_many = [ private static $has_many = [
"Teams" => "Team", 'Teams' => 'Team',
]; ];
public function onBeforeWrite() public function onBeforeWrite()
@ -37,15 +37,13 @@ class Player extends DataObject
$currentPlayer = Security::getCurrentUser(); $currentPlayer = Security::getCurrentUser();
if (!$currentPlayer->IsTeamManager()) { if (!$currentPlayer->IsTeamManager()) {
user_error('Player-creation not allowed', E_USER_ERROR); throw new \Exception('Player-creation not allowed');
exit();
} }
} }
// check on every write action // check on every write action
if (!$this->record['TeamID']) { if (!$this->record['TeamID']) {
user_error('Cannot save player without a valid team', E_USER_ERROR); throw new \Exception('Cannot save player without a valid team');
exit();
} }
// CAUTION: You are required to call the parent-function, otherwise // CAUTION: You are required to call the parent-function, otherwise

View File

@ -115,7 +115,8 @@ developers know:
* Things that will prevent an internal function from continuing. Throw a warning and return null. * Things that will prevent an internal function from continuing. Throw a warning and return null.
* **E_USER_ERROR:** Throwing one of these errors is going to take down the production site. So you should only throw * **E_USER_ERROR:** Throwing one of these errors is going to take down the production site. So you should only throw
E_USER_ERROR if it's going to be **dangerous** or **impossible** to continue with the request. E_USER_ERROR if it's going to be **dangerous** or **impossible** to continue with the request. Note that it is
preferable to now throw exceptions instead of `E_USER_ERROR`.
## Configuring error logging ## Configuring error logging

View File

@ -199,7 +199,7 @@ class Controller extends RequestHandler implements TemplateGlobalProvider
public function handleRequest(HTTPRequest $request) public function handleRequest(HTTPRequest $request)
{ {
if (!$request) { if (!$request) {
user_error("Controller::handleRequest() not passed a request!", E_USER_ERROR); throw new \RuntimeException('Controller::handleRequest() not passed a request!');
} }
//set up the controller for the incoming request //set up the controller for the incoming request

View File

@ -858,7 +858,7 @@ class HTTPRequest implements ArrayAccess
public function setHttpMethod($method) public function setHttpMethod($method)
{ {
if (!self::isValidHttpMethod($method)) { if (!self::isValidHttpMethod($method)) {
user_error('HTTPRequest::setHttpMethod: Invalid HTTP method', E_USER_ERROR); throw new \InvalidArgumentException('HTTPRequest::setHttpMethod: Invalid HTTP method');
} }
$this->httpMethod = strtoupper($method); $this->httpMethod = strtoupper($method);
@ -912,12 +912,11 @@ class HTTPRequest implements ArrayAccess
{ {
if (isset($postVars['_method'])) { if (isset($postVars['_method'])) {
if (!self::isValidHttpMethod($postVars['_method'])) { if (!self::isValidHttpMethod($postVars['_method'])) {
user_error('HTTPRequest::detect_method(): Invalid "_method" parameter', E_USER_ERROR); throw new InvalidArgumentException('HTTPRequest::detect_method(): Invalid "_method" parameter');
} }
return strtoupper($postVars['_method']); return strtoupper($postVars['_method']);
} else {
return $origMethod;
} }
return $origMethod;
} }
/** /**

View File

@ -187,7 +187,7 @@ class RequestHandler extends ViewableData
} }
$action = "index"; $action = "index";
} elseif (!is_string($action)) { } elseif (!is_string($action)) {
user_error("Non-string method name: " . var_export($action, true), E_USER_ERROR); throw new InvalidArgumentException("Non-string method name: " . var_export($action, true));
} }
$classMessage = Director::isLive() ? 'on this handler' : 'on class ' . static::class; $classMessage = Director::isLive() ? 'on this handler' : 'on class ' . static::class;

View File

@ -183,17 +183,17 @@ trait Extensible
} }
$extensionClass = $matches[1]; $extensionClass = $matches[1];
if (!class_exists($extensionClass)) { if (!class_exists($extensionClass)) {
user_error( throw new InvalidArgumentException(sprintf(
sprintf('Object::add_extension() - Can\'t find extension class for "%s"', $extensionClass), 'Object::add_extension() - Can\'t find extension class for "%s"',
E_USER_ERROR $extensionClass
); ));
} }
if (!is_subclass_of($extensionClass, 'SilverStripe\\Core\\Extension')) { if (!is_subclass_of($extensionClass, Extension::class)) {
user_error( throw new InvalidArgumentException(sprintf(
sprintf('Object::add_extension() - Extension "%s" is not a subclass of Extension', $extensionClass), 'Object::add_extension() - Extension "%s" is not a subclass of Extension',
E_USER_ERROR $extensionClass
); ));
} }
// unset some caches // unset some caches
@ -334,7 +334,7 @@ trait Extensible
$sources = []; $sources = [];
foreach ($extensions as $extension) { foreach ($extensions as $extension) {
list($extensionClass, $extensionArgs) = ClassInfo::parse_class_spec($extension); [$extensionClass, $extensionArgs] = ClassInfo::parse_class_spec($extension);
// Strip service name specifier // Strip service name specifier
$extensionClass = strtok($extensionClass, '.'); $extensionClass = strtok($extensionClass, '.');
$sources[] = $extensionClass; $sources[] = $extensionClass;

View File

@ -344,7 +344,7 @@ class CsvBulkLoader extends BulkLoader
} }
} elseif (strpos($fieldName, '.') !== false) { } elseif (strpos($fieldName, '.') !== false) {
// we have a relation column with dot notation // we have a relation column with dot notation
list($relationName, $columnName) = explode('.', $fieldName); [$relationName, $columnName] = explode('.', $fieldName);
// always gives us an component (either empty or existing) // always gives us an component (either empty or existing)
$relationObj = $obj->getComponent($relationName); $relationObj = $obj->getComponent($relationName);
if (!$preview) { if (!$preview) {
@ -445,15 +445,19 @@ class CsvBulkLoader extends BulkLoader
} elseif ($SNG_objectClass->hasMethod($duplicateCheck['callback'])) { } elseif ($SNG_objectClass->hasMethod($duplicateCheck['callback'])) {
$existingRecord = $SNG_objectClass->{$duplicateCheck['callback']}($record[$fieldName], $record); $existingRecord = $SNG_objectClass->{$duplicateCheck['callback']}($record[$fieldName], $record);
} else { } else {
user_error("CsvBulkLoader::processRecord():" throw new \RuntimeException(
. " {$duplicateCheck['callback']} not found on importer or object class.", E_USER_ERROR); "CsvBulkLoader::processRecord():"
. " {$duplicateCheck['callback']} not found on importer or object class."
);
} }
if ($existingRecord) { if ($existingRecord) {
return $existingRecord; return $existingRecord;
} }
} else { } else {
user_error('CsvBulkLoader::processRecord(): Wrong format for $duplicateChecks', E_USER_ERROR); throw new \InvalidArgumentException(
'CsvBulkLoader::processRecord(): Wrong format for $duplicateChecks'
);
} }
} }

View File

@ -480,11 +480,11 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
$id = $state->getFixtureFactory(static::class)->getId($className, $identifier); $id = $state->getFixtureFactory(static::class)->getId($className, $identifier);
if (!$id) { if (!$id) {
user_error(sprintf( throw new \InvalidArgumentException(sprintf(
"Couldn't find object '%s' (class: %s)", "Couldn't find object '%s' (class: %s)",
$identifier, $identifier,
$className $className
), E_USER_ERROR); ));
} }
return $id; return $id;
@ -519,11 +519,11 @@ class SapphireTest extends PHPUnit_Framework_TestCase implements TestOnly
$obj = $state->getFixtureFactory(static::class)->get($className, $identifier); $obj = $state->getFixtureFactory(static::class)->get($className, $identifier);
if (!$obj) { if (!$obj) {
user_error(sprintf( throw new \InvalidArgumentException(sprintf(
"Couldn't find object '%s' (class: %s)", "Couldn't find object '%s' (class: %s)",
$identifier, $identifier,
$className $className
), E_USER_ERROR); ));
} }
return $obj; return $obj;

View File

@ -251,11 +251,10 @@ class CompositeField extends FormField
if (isset($list[$name])) { if (isset($list[$name])) {
$fieldClass = get_class($field); $fieldClass = get_class($field);
$otherFieldClass = get_class($list[$name]); $otherFieldClass = get_class($list[$name]);
user_error( throw new \RuntimeException(
"collateDataFields() I noticed that a field called '$name' appears twice in" "collateDataFields() I noticed that a field called '$name' appears twice in"
. " your form: '{$formName}'. One is a '{$fieldClass}' and the other is a" . " your form: '{$formName}'. One is a '{$fieldClass}' and the other is a"
. " '{$otherFieldClass}'", . " '{$otherFieldClass}'"
E_USER_ERROR
); );
} }
$list[$name] = $field; $list[$name] = $field;

View File

@ -165,15 +165,12 @@ class FieldList extends ArrayList
$errorSuffix = ''; $errorSuffix = '';
} }
user_error( throw new \RuntimeException(sprintf(
sprintf(
"%s() I noticed that a field called '%s' appears twice%s", "%s() I noticed that a field called '%s' appears twice%s",
$functionName, $functionName,
$field->getName(), $field->getName(),
$errorSuffix $errorSuffix
), ));
E_USER_ERROR
);
} }
protected function flushFieldsCache() protected function flushFieldsCache()
@ -213,9 +210,8 @@ class FieldList extends ArrayList
} else { } else {
$errSuffix = ''; $errSuffix = '';
} }
user_error( throw new \RuntimeException(
"collateDataFields() I noticed that a field called '$name' appears twice$errSuffix.", "collateDataFields() I noticed that a field called '$name' appears twice$errSuffix."
E_USER_ERROR
); );
} }
$list[$name] = $field; $list[$name] = $field;
@ -493,10 +489,9 @@ class FieldList extends ArrayList
? " named '{$parentPointer->getName()}'" ? " named '{$parentPointer->getName()}'"
: null; : null;
$parentPointerClass = get_class($parentPointer); $parentPointerClass = get_class($parentPointer);
user_error( throw new \InvalidArgumentException(
"FieldList::addFieldToTab() Tried to add a tab to object" "FieldList::addFieldToTab() Tried to add a tab to object"
. " '{$parentPointerClass}'{$withName} - '{$part}' didn't exist.", . " '{$parentPointerClass}'{$withName} - '{$part}' didn't exist."
E_USER_ERROR
); );
} }
} }

View File

@ -1414,12 +1414,7 @@ class FormField extends RequestHandler
} }
$class = static::class; $class = static::class;
user_error( throw new \RuntimeException("rootFieldList() called on {$class} object without a containerFieldList");
"rootFieldList() called on {$class} object without a containerFieldList",
E_USER_ERROR
);
return null;
} }
/** /**

View File

@ -173,7 +173,7 @@ abstract class SelectField extends FormField
$source = $source->toArray(); $source = $source->toArray();
} }
if (!is_array($source) && !($source instanceof ArrayAccess)) { if (!is_array($source) && !($source instanceof ArrayAccess)) {
user_error('$source passed in as invalid type', E_USER_ERROR); throw new \InvalidArgumentException('$source passed in as invalid type');
} }
return $source; return $source;

View File

@ -2,6 +2,7 @@
namespace SilverStripe\Forms; namespace SilverStripe\Forms;
use http\Exception\InvalidArgumentException;
use SilverStripe\Control\Controller; use SilverStripe\Control\Controller;
use SilverStripe\Core\Convert; use SilverStripe\Core\Convert;
use SilverStripe\ORM\ArrayList; use SilverStripe\ORM\ArrayList;
@ -252,10 +253,9 @@ class TreeMultiselectField extends TreeDropdownField
$saveDest = $record->$fieldName(); $saveDest = $record->$fieldName();
if (!$saveDest) { if (!$saveDest) {
$recordClass = get_class($record); $recordClass = get_class($record);
user_error( throw new \RuntimeException(
"TreeMultiselectField::saveInto() Field '$fieldName' not found on" "TreeMultiselectField::saveInto() Field '$fieldName' not found on"
. " {$recordClass}.{$record->ID}", . " {$recordClass}.{$record->ID}"
E_USER_ERROR
); );
} }

View File

@ -431,9 +431,8 @@ abstract class Database
break; break;
default: default:
user_error( throw new \InvalidArgumentException(
"SS_Database::manipulate() Can't recognise command '{$writeInfo['command']}'", "SS_Database::manipulate() Can't recognise command '{$writeInfo['command']}'"
E_USER_ERROR
); );
} }
} }

View File

@ -2,9 +2,9 @@
namespace SilverStripe\ORM\Connect; namespace SilverStripe\ORM\Connect;
use SilverStripe\Core\Config\Config;
use mysqli; use mysqli;
use mysqli_stmt; use mysqli_stmt;
use SilverStripe\Core\Config\Config;
/** /**
* Connector for MySQL using the MySQLi method * Connector for MySQL using the MySQLi method
@ -238,11 +238,9 @@ class MySQLiConnector extends DBConnector
case 'array': case 'array':
case 'unknown type': case 'unknown type':
default: default:
user_error( throw new \InvalidArgumentException(
"Cannot bind parameter \"$value\" as it is an unsupported type ($phpType)", "Cannot bind parameter \"$value\" as it is an unsupported type ($phpType)"
E_USER_ERROR
); );
break;
} }
$values[] = $value; $values[] = $value;
} }

View File

@ -1274,7 +1274,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
*/ */
public function offsetSet($key, $value) public function offsetSet($key, $value)
{ {
user_error("Can't alter items in a DataList using array-access", E_USER_ERROR); throw new \BadMethodCallException("Can't alter items in a DataList using array-access");
} }
/** /**
@ -1284,6 +1284,6 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
*/ */
public function offsetUnset($key) public function offsetUnset($key)
{ {
user_error("Can't alter items in a DataList using array-access", E_USER_ERROR); throw new \BadMethodCallException("Can't alter items in a DataList using array-access");
} }
} }

View File

@ -1391,8 +1391,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
$this->brokenOnWrite = true; $this->brokenOnWrite = true;
$this->onBeforeWrite(); $this->onBeforeWrite();
if ($this->brokenOnWrite) { if ($this->brokenOnWrite) {
user_error(static::class . " has a broken onBeforeWrite() function." throw new LogicException(
. " Make sure that you call parent::onBeforeWrite().", E_USER_ERROR); static::class . " has a broken onBeforeWrite() function."
. " Make sure that you call parent::onBeforeWrite()."
);
} }
} }
@ -1737,8 +1739,10 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
$this->brokenOnDelete = true; $this->brokenOnDelete = true;
$this->onBeforeDelete(); $this->onBeforeDelete();
if ($this->brokenOnDelete) { if ($this->brokenOnDelete) {
user_error(static::class . " has a broken onBeforeDelete() function." throw new LogicException(
. " Make sure that you call parent::onBeforeDelete().", E_USER_ERROR); static::class . " has a broken onBeforeDelete() function."
. " Make sure that you call parent::onBeforeDelete()."
);
} }
// Deleting a record without an ID shouldn't do anything // Deleting a record without an ID shouldn't do anything
@ -2874,7 +2878,7 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity
public function setCastedField($fieldName, $value) public function setCastedField($fieldName, $value)
{ {
if (!$fieldName) { if (!$fieldName) {
user_error("DataObject::setCastedField: Called without a fieldName", E_USER_ERROR); throw new InvalidArgumentException("DataObject::setCastedField: Called without a fieldName");
} }
$fieldObj = $this->dbObject($fieldName); $fieldObj = $this->dbObject($fieldName);
if ($fieldObj) { if ($fieldObj) {

View File

@ -35,7 +35,7 @@ class DBBoolean extends DBField
public function Nice() public function Nice()
{ {
return ($this->value) ? _t('SilverStripe\\ORM\\FieldType\\DBBoolean.YESANSWER', 'Yes') : _t('SilverStripe\\ORM\\FieldType\\DBBoolean.NOANSWER', 'No'); return ($this->value) ? _t(__CLASS__ . '.YESANSWER', 'Yes') : _t(__CLASS__ . '.NOANSWER', 'No');
} }
public function NiceAsBoolean() public function NiceAsBoolean()
@ -50,7 +50,7 @@ class DBBoolean extends DBField
$dataObject->$fieldName = ($this->value) ? 1 : 0; $dataObject->$fieldName = ($this->value) ? 1 : 0;
} else { } else {
$class = static::class; $class = static::class;
user_error("DBField::saveInto() Called on a nameless '$class' object", E_USER_ERROR); throw new \RuntimeException("DBField::saveInto() Called on a nameless '$class' object");
} }
} }
@ -61,10 +61,10 @@ class DBBoolean extends DBField
public function scaffoldSearchField($title = null) public function scaffoldSearchField($title = null)
{ {
$anyText = _t('SilverStripe\\ORM\\FieldType\\DBBoolean.ANY', 'Any'); $anyText = _t(__CLASS__ . '.ANY', 'Any');
$source = [ $source = [
1 => _t('SilverStripe\\ORM\\FieldType\\DBBoolean.YESANSWER', 'Yes'), 1 => _t(__CLASS__ . '.YESANSWER', 'Yes'),
0 => _t('SilverStripe\\ORM\\FieldType\\DBBoolean.NOANSWER', 'No') 0 => _t(__CLASS__ . '.NOANSWER', 'No')
]; ];
$field = new DropdownField($this->name, $title, $source); $field = new DropdownField($this->name, $title, $source);

View File

@ -90,7 +90,9 @@ class DBDecimal extends DBField
if ($fieldName) { if ($fieldName) {
$dataObject->$fieldName = (float)preg_replace('/[^0-9.\-\+]/', '', $this->value); $dataObject->$fieldName = (float)preg_replace('/[^0-9.\-\+]/', '', $this->value);
} else { } else {
user_error("DBField::saveInto() Called on a nameless '" . static::class . "' object", E_USER_ERROR); throw new \UnexpectedValueException(
"DBField::saveInto() Called on a nameless '" . static::class . "' object"
);
} }
} }

View File

@ -5,6 +5,7 @@ namespace SilverStripe\ORM\FieldType;
use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Config;
use SilverStripe\Forms\DropdownField; use SilverStripe\Forms\DropdownField;
use SilverStripe\ORM\ArrayLib; use SilverStripe\ORM\ArrayLib;
use SilverStripe\ORM\Connect\MySQLDatabase;
use SilverStripe\ORM\DB; use SilverStripe\ORM\DB;
/** /**
@ -77,9 +78,8 @@ class DBEnum extends DBString
if (in_array($default, $enum)) { if (in_array($default, $enum)) {
$this->setDefault($default); $this->setDefault($default);
} else { } else {
user_error( throw new \InvalidArgumentException(
"Enum::__construct() The default value '$default' does not match any item in the enumeration", "Enum::__construct() The default value '$default' does not match any item in the enumeration"
E_USER_ERROR
); );
} }
} elseif (is_int($default) && $default < count($enum)) { } elseif (is_int($default) && $default < count($enum)) {
@ -99,8 +99,8 @@ class DBEnum extends DBString
*/ */
public function requireField() public function requireField()
{ {
$charset = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'charset'); $charset = Config::inst()->get(MySQLDatabase::class, 'charset');
$collation = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'collation'); $collation = Config::inst()->get(MySQLDatabase::class, 'collation');
$parts = [ $parts = [
'datatype' => 'enum', 'datatype' => 'enum',

View File

@ -3,6 +3,7 @@
namespace SilverStripe\ORM\FieldType; namespace SilverStripe\ORM\FieldType;
use SilverStripe\Core\Config\Config; use SilverStripe\Core\Config\Config;
use SilverStripe\ORM\Connect\MySQLDatabase;
use SilverStripe\ORM\DB; use SilverStripe\ORM\DB;
use SilverStripe\Forms\CheckboxSetField; use SilverStripe\Forms\CheckboxSetField;
@ -22,9 +23,10 @@ class DBMultiEnum extends DBEnum
$defaults = preg_split('/ *, */', trim($default)); $defaults = preg_split('/ *, */', trim($default));
foreach ($defaults as $thisDefault) { foreach ($defaults as $thisDefault) {
if (!in_array($thisDefault, $this->enum)) { if (!in_array($thisDefault, $this->enum)) {
user_error("Enum::__construct() The default value '$thisDefault' does not match " throw new \InvalidArgumentException(
. "any item in the enumeration", E_USER_ERROR); "Enum::__construct() The default value '$thisDefault' does not match "
return; . "any item in the enumeration"
);
} }
} }
$this->default = implode(',', $defaults); $this->default = implode(',', $defaults);
@ -34,8 +36,8 @@ class DBMultiEnum extends DBEnum
public function requireField() public function requireField()
{ {
// @todo: Remove mysql-centric logic from this // @todo: Remove mysql-centric logic from this
$charset = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'charset'); $charset = Config::inst()->get(MySQLDatabase::class, 'charset');
$collation = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'collation'); $collation = Config::inst()->get(MySQLDatabase::class, 'collation');
$values=[ $values=[
'type'=>'set', 'type'=>'set',
'parts'=>[ 'parts'=>[

View File

@ -74,7 +74,7 @@ class HasManyList extends RelationList
if (is_numeric($item)) { if (is_numeric($item)) {
$item = DataObject::get_by_id($this->dataClass, $item); $item = DataObject::get_by_id($this->dataClass, $item);
} elseif (!($item instanceof $this->dataClass)) { } elseif (!($item instanceof $this->dataClass)) {
user_error("HasManyList::add() expecting a $this->dataClass object, or ID value", E_USER_ERROR); throw new InvalidArgumentException("HasManyList::add() expecting a $this->dataClass object, or ID value");
} }
$foreignID = $this->getForeignID(); $foreignID = $this->getForeignID();
@ -123,10 +123,7 @@ class HasManyList extends RelationList
public function remove($item) public function remove($item)
{ {
if (!($item instanceof $this->dataClass)) { if (!($item instanceof $this->dataClass)) {
throw new InvalidArgumentException( throw new InvalidArgumentException("HasManyList::remove() expecting a $this->dataClass object, or ID");
"HasManyList::remove() expecting a $this->dataClass object, or ID",
E_USER_ERROR
);
} }
// Don't remove item which doesn't belong to this list // Don't remove item which doesn't belong to this list

View File

@ -368,9 +368,8 @@ class Hierarchy extends DataExtension
// Validate the ID list // Validate the ID list
foreach ($idList as $id) { foreach ($idList as $id) {
if (!is_numeric($id)) { if (!is_numeric($id)) {
user_error( throw new \InvalidArgumentException(
"Bad ID passed to Versioned::prepopulate_numchildren_cache() in \$idList: " . $id, "Bad ID passed to Versioned::prepopulate_numchildren_cache() in \$idList: " . $id
E_USER_ERROR
); );
} }
} }

View File

@ -212,10 +212,7 @@ class Map implements ArrayAccess, Countable, IteratorAggregate
$this->lastItems[$key] = $value; $this->lastItems[$key] = $value;
} }
user_error( throw new \BadMethodCallException('Map is read-only. Please use $map->push($key, $value) to append values');
'Map is read-only. Please use $map->push($key, $value) to append values',
E_USER_ERROR
);
} }
/** /**
@ -243,9 +240,8 @@ class Map implements ArrayAccess, Countable, IteratorAggregate
return; return;
} }
user_error( throw new \BadMethodCallException(
"Map is read-only. Unset cannot be called on keys derived from the DataQuery", 'Map is read-only. Unset cannot be called on keys derived from the DataQuery'
E_USER_ERROR
); );
} }

View File

@ -109,8 +109,7 @@ class PolymorphicHasManyList extends HasManyList
{ {
if (!($item instanceof $this->dataClass)) { if (!($item instanceof $this->dataClass)) {
throw new InvalidArgumentException( throw new InvalidArgumentException(
"HasManyList::remove() expecting a $this->dataClass object, or ID", "HasManyList::remove() expecting a $this->dataClass object, or ID"
E_USER_ERROR
); );
} }

View File

@ -612,9 +612,8 @@ abstract class SQLConditionalExpression extends SQLExpression
if ($parameterCount === 1) { if ($parameterCount === 1) {
$key .= " = ?"; $key .= " = ?";
} elseif ($parameterCount > 1) { } elseif ($parameterCount > 1) {
user_error( throw new \InvalidArgumentException(
"Incorrect number of '?' in predicate $key. Expected $parameterCount but none given.", "Incorrect number of '?' in predicate $key. Expected $parameterCount but none given."
E_USER_ERROR
); );
} }
} }
@ -622,9 +621,11 @@ abstract class SQLConditionalExpression extends SQLExpression
} elseif (is_array($value)) { } elseif (is_array($value)) {
// If predicates are nested one per array (as per the internal format) // If predicates are nested one per array (as per the internal format)
// then run a quick check over the contents and recursively parse // then run a quick check over the contents and recursively parse
if (count($value) != 1) { if (count($value) !== 1) {
user_error('Nested predicates should be given as a single item array in ' throw new \InvalidArgumentException(
. 'array($predicate => array($prameters)) format)', E_USER_ERROR); 'Nested predicates should be given as a single item array in '
. 'array($predicate => array($prameters)) format)'
);
} }
foreach ($value as $key => $pairValue) { foreach ($value as $key => $pairValue) {
return $this->parsePredicate($key, $pairValue); return $this->parsePredicate($key, $pairValue);

View File

@ -95,10 +95,10 @@ class UnsavedRelationList extends ArrayList implements Relation
public function push($item, $extraFields = null) public function push($item, $extraFields = null)
{ {
if ((is_object($item) && !$item instanceof $this->dataClass) if ((is_object($item) && !$item instanceof $this->dataClass)
|| (!is_object($item) && !is_numeric($item))) { || (!is_object($item) && !is_numeric($item))
) {
throw new InvalidArgumentException( throw new InvalidArgumentException(
"UnsavedRelationList::add() expecting a $this->dataClass object, or ID value", "UnsavedRelationList::add() expecting a $this->dataClass object, or ID value"
E_USER_ERROR
); );
} }
if (is_object($item) && $item->ID) { if (is_object($item) && $item->ID) {

View File

@ -19,7 +19,7 @@ class Member_GroupSet extends ManyManyList
{ {
// Do not join the table directly // Do not join the table directly
if ($this->extraFields) { if ($this->extraFields) {
user_error('Member_GroupSet does not support many_many_extraFields', E_USER_ERROR); throw new \BadMethodCallException('Member_GroupSet does not support many_many_extraFields');
} }
} }

View File

@ -246,7 +246,7 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl
$argClause = "AND \"Arg\" IN (?, ?) "; $argClause = "AND \"Arg\" IN (?, ?) ";
$argParams = [-1, $arg]; $argParams = [-1, $arg];
} else { } else {
user_error("Permission::checkMember: bad arg '$arg'", E_USER_ERROR); throw new \InvalidArgumentException("Permission::checkMember: bad arg '$arg'");
} }
} }
@ -408,10 +408,7 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl
if (is_numeric($arg)) { if (is_numeric($arg)) {
$perm->Arg = $arg; $perm->Arg = $arg;
} else { } else {
user_error( throw new \InvalidArgumentException("Permission::checkMember: bad arg '$arg'");
"Permission::checkMember: bad arg '$arg'",
E_USER_ERROR
);
} }
} }
@ -446,10 +443,7 @@ class Permission extends DataObject implements TemplateGlobalProvider, Resettabl
if (is_numeric($arg)) { if (is_numeric($arg)) {
$perm->Arg = $arg; $perm->Arg = $arg;
} else { } else {
user_error( throw new \InvalidArgumentException("Permission::checkMember: bad arg '$arg'");
"Permission::checkMember: bad arg '$arg'",
E_USER_ERROR
);
} }
} }

View File

@ -191,7 +191,7 @@ class ShortcodeParser
// Missing tag // Missing tag
if ($content === false) { if ($content === false) {
if (ShortcodeParser::$error_behavior == ShortcodeParser::ERROR) { if (ShortcodeParser::$error_behavior == ShortcodeParser::ERROR) {
user_error('Unknown shortcode tag ' . $tag['open'], E_USER_ERROR); throw new \InvalidArgumentException('Unknown shortcode tag ' . $tag['open']);
} elseif (self::$error_behavior == self::WARN && $isHTMLAllowed) { } elseif (self::$error_behavior == self::WARN && $isHTMLAllowed) {
$content = '<strong class="warning">' . $tag['text'] . '</strong>'; $content = '<strong class="warning">' . $tag['text'] . '</strong>';
} elseif (ShortcodeParser::$error_behavior == ShortcodeParser::STRIP) { } elseif (ShortcodeParser::$error_behavior == ShortcodeParser::STRIP) {
@ -381,7 +381,7 @@ class ShortcodeParser
if ($err) { if ($err) {
if (self::$error_behavior == self::ERROR) { if (self::$error_behavior == self::ERROR) {
user_error($err, E_USER_ERROR); throw new \Exception($err);
} }
} else { } else {
if ($tags[$i]['escaped']) { if ($tags[$i]['escaped']) {
@ -603,7 +603,7 @@ class ShortcodeParser
} }
// NOP // NOP
} else { } else {
user_error('Unknown value for $location argument ' . $location, E_USER_ERROR); throw new \UnexpectedValueException('Unknown value for $location argument ' . $location);
} }
} }
@ -664,7 +664,7 @@ class ShortcodeParser
// Now parse the result into a DOM // Now parse the result into a DOM
if (!$htmlvalue->isValid()) { if (!$htmlvalue->isValid()) {
if (self::$error_behavior == self::ERROR) { if (self::$error_behavior == self::ERROR) {
user_error('Couldn\'t decode HTML when processing short codes', E_USER_ERROR); throw new \Exception('Couldn\'t decode HTML when processing short codes');
} else { } else {
$continue = false; $continue = false;
} }
@ -706,9 +706,8 @@ class ShortcodeParser
if (!$parent) { if (!$parent) {
if ($location !== self::INLINE) { if ($location !== self::INLINE) {
user_error( throw new \RuntimeException(
"Parent block for shortcode couldn't be found, but location wasn't INLINE", "Parent block for shortcode couldn't be found, but location wasn't INLINE"
E_USER_ERROR
); );
} }
} else { } else {

View File

@ -249,7 +249,7 @@ class SSViewer_DataPresenter extends SSViewer_Scope
case 'Up': case 'Up':
$upIndex = $this->getUpIndex(); $upIndex = $this->getUpIndex();
if ($upIndex === null) { if ($upIndex === null) {
user_error('Up called when we\'re already at the top of the scope', E_USER_ERROR); throw new \LogicException('Up called when we\'re already at the top of the scope');
} }
$overlayIndex = $upIndex; // Parent scope $overlayIndex = $upIndex; // Parent scope

View File

@ -191,7 +191,7 @@ class SSViewer_Scope
switch ($name) { switch ($name) {
case 'Up': case 'Up':
if ($this->upIndex === null) { if ($this->upIndex === null) {
user_error('Up called when we\'re already at the top of the scope', E_USER_ERROR); throw new \LogicException('Up called when we\'re already at the top of the scope');
} }
list( list(

View File

@ -160,11 +160,9 @@ class HTTPRequestTest extends SapphireTest
$this->assertEquals($expected, $actual); $this->assertEquals($expected, $actual);
} }
/**
* @expectedException PHPUnit_Framework_Error
*/
public function testBadDetectMethod() public function testBadDetectMethod()
{ {
$this->expectException(\InvalidArgumentException::class);
HTTPRequest::detect_method('POST', ['_method' => 'Boom']); HTTPRequest::detect_method('POST', ['_method' => 'Boom']);
} }
@ -191,11 +189,9 @@ class HTTPRequestTest extends SapphireTest
$this->assertEquals($request, $returnedRequest); $this->assertEquals($request, $returnedRequest);
} }
/**
* @expectedException PHPUnit_Framework_Error
*/
public function testBadSetHttpMethod() public function testBadSetHttpMethod()
{ {
$this->expectException(\InvalidArgumentException::class);
$request = new HTTPRequest('GET', '/hello'); $request = new HTTPRequest('GET', '/hello');
$request->setHttpMethod('boom'); $request->setHttpMethod('boom');
} }

View File

@ -2,7 +2,6 @@
namespace SilverStripe\Forms\Tests; namespace SilverStripe\Forms\Tests;
use PHPUnit_Framework_Error;
use SilverStripe\Dev\CSSContentParser; use SilverStripe\Dev\CSSContentParser;
use SilverStripe\Dev\SapphireTest; use SilverStripe\Dev\SapphireTest;
use SilverStripe\Forms\CompositeField; use SilverStripe\Forms\CompositeField;
@ -161,12 +160,13 @@ class CompositeFieldTest extends SapphireTest
$this->assertNull($result['title']); $this->assertNull($result['title']);
} }
/**
* @expectedException PHPUnit_Framework_Error
* @expectedExceptionMessageRegExp /a field called 'Test' appears twice in your form.*TextField.*TextField/
*/
public function testCollateDataFieldsThrowsErrorOnDuplicateChildren() public function testCollateDataFieldsThrowsErrorOnDuplicateChildren()
{ {
$this->expectException(\RuntimeException::class);
$this->expectExceptionMessageRegExp(
"/a field called 'Test' appears twice in your form.*TextField.*TextField/"
);
$field = CompositeField::create( $field = CompositeField::create(
TextField::create('Test'), TextField::create('Test'),
TextField::create('Test') TextField::create('Test')