diff --git a/docs/en/02_Developer_Guides/00_Model/05_Extending_DataObjects.md b/docs/en/02_Developer_Guides/00_Model/05_Extending_DataObjects.md index 0a413e8596e..6d60febeb4d 100644 --- a/docs/en/02_Developer_Guides/00_Model/05_Extending_DataObjects.md +++ b/docs/en/02_Developer_Guides/00_Model/05_Extending_DataObjects.md @@ -27,25 +27,23 @@ use SilverStripe\ORM\DataObject; class Player extends DataObject { private static $has_many = [ - "Teams" => "Team", + 'Teams' => 'Team', ]; public function onBeforeWrite() { // check on first write action, aka "database row creation" (ID-property is not set) - if(!$this->isInDb()) { + if (!$this->isInDb()) { $currentPlayer = Security::getCurrentUser(); - if(!$currentPlayer->IsTeamManager()) { - user_error('Player-creation not allowed', E_USER_ERROR); - exit(); + if (!$currentPlayer->IsTeamManager()) { + throw new \Exception('Player-creation not allowed'); } } // check on every write action - if(!$this->record['TeamID']) { - user_error('Cannot save player without a valid team', E_USER_ERROR); - exit(); + if (!$this->record['TeamID']) { + throw new \Exception('Cannot save player without a valid team'); } // CAUTION: You are required to call the parent-function, otherwise diff --git a/docs/en/02_Developer_Guides/07_Debugging/01_Error_Handling.md b/docs/en/02_Developer_Guides/07_Debugging/01_Error_Handling.md index fd319a5a2bf..68d7045b22e 100644 --- a/docs/en/02_Developer_Guides/07_Debugging/01_Error_Handling.md +++ b/docs/en/02_Developer_Guides/07_Debugging/01_Error_Handling.md @@ -115,7 +115,8 @@ developers know: * 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 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 diff --git a/src/Control/Controller.php b/src/Control/Controller.php index ffbaadb91c3..3cb1b9833c9 100644 --- a/src/Control/Controller.php +++ b/src/Control/Controller.php @@ -199,7 +199,7 @@ protected function afterHandleRequest() public function handleRequest(HTTPRequest $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 diff --git a/src/Control/HTTPRequest.php b/src/Control/HTTPRequest.php index ad44687f29a..6f9212cc280 100644 --- a/src/Control/HTTPRequest.php +++ b/src/Control/HTTPRequest.php @@ -858,7 +858,7 @@ public function httpMethod() public function setHttpMethod($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); @@ -912,12 +912,11 @@ public static function detect_method($origMethod, $postVars) { if (isset($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']); - } else { - return $origMethod; } + return $origMethod; } /** diff --git a/src/Control/RequestHandler.php b/src/Control/RequestHandler.php index 9d6122779cc..a10d88047ca 100644 --- a/src/Control/RequestHandler.php +++ b/src/Control/RequestHandler.php @@ -187,7 +187,7 @@ public function handleRequest(HTTPRequest $request) } $action = "index"; } 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; diff --git a/src/Core/Extensible.php b/src/Core/Extensible.php index 1cc1c177ac0..48de82f65d2 100644 --- a/src/Core/Extensible.php +++ b/src/Core/Extensible.php @@ -183,17 +183,17 @@ public static function add_extension($classOrExtension, $extension = null) } $extensionClass = $matches[1]; if (!class_exists($extensionClass)) { - user_error( - sprintf('Object::add_extension() - Can\'t find extension class for "%s"', $extensionClass), - E_USER_ERROR - ); + throw new InvalidArgumentException(sprintf( + 'Object::add_extension() - Can\'t find extension class for "%s"', + $extensionClass + )); } - if (!is_subclass_of($extensionClass, 'SilverStripe\\Core\\Extension')) { - user_error( - sprintf('Object::add_extension() - Extension "%s" is not a subclass of Extension', $extensionClass), - E_USER_ERROR - ); + if (!is_subclass_of($extensionClass, Extension::class)) { + throw new InvalidArgumentException(sprintf( + 'Object::add_extension() - Extension "%s" is not a subclass of Extension', + $extensionClass + )); } // unset some caches @@ -334,7 +334,7 @@ public static function get_extra_config_sources($class = null) $sources = []; foreach ($extensions as $extension) { - list($extensionClass, $extensionArgs) = ClassInfo::parse_class_spec($extension); + [$extensionClass, $extensionArgs] = ClassInfo::parse_class_spec($extension); // Strip service name specifier $extensionClass = strtok($extensionClass, '.'); $sources[] = $extensionClass; diff --git a/src/Dev/CsvBulkLoader.php b/src/Dev/CsvBulkLoader.php index f90f76ecabe..efa3773a0e1 100644 --- a/src/Dev/CsvBulkLoader.php +++ b/src/Dev/CsvBulkLoader.php @@ -344,7 +344,7 @@ protected function processRecord($record, $columnMap, &$results, $preview = fals } } elseif (strpos($fieldName, '.') !== false) { // 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) $relationObj = $obj->getComponent($relationName); if (!$preview) { @@ -445,15 +445,19 @@ public function findExistingObject($record, $columnMap = []) } elseif ($SNG_objectClass->hasMethod($duplicateCheck['callback'])) { $existingRecord = $SNG_objectClass->{$duplicateCheck['callback']}($record[$fieldName], $record); } else { - user_error("CsvBulkLoader::processRecord():" - . " {$duplicateCheck['callback']} not found on importer or object class.", E_USER_ERROR); + throw new \RuntimeException( + "CsvBulkLoader::processRecord():" + . " {$duplicateCheck['callback']} not found on importer or object class." + ); } if ($existingRecord) { return $existingRecord; } } else { - user_error('CsvBulkLoader::processRecord(): Wrong format for $duplicateChecks', E_USER_ERROR); + throw new \InvalidArgumentException( + 'CsvBulkLoader::processRecord(): Wrong format for $duplicateChecks' + ); } } diff --git a/src/Dev/SapphireTest.php b/src/Dev/SapphireTest.php index 83a294859bc..b79e1e5c566 100644 --- a/src/Dev/SapphireTest.php +++ b/src/Dev/SapphireTest.php @@ -480,11 +480,11 @@ protected function idFromFixture($className, $identifier) $id = $state->getFixtureFactory(static::class)->getId($className, $identifier); if (!$id) { - user_error(sprintf( + throw new \InvalidArgumentException(sprintf( "Couldn't find object '%s' (class: %s)", $identifier, $className - ), E_USER_ERROR); + )); } return $id; @@ -519,11 +519,11 @@ protected function objFromFixture($className, $identifier) $obj = $state->getFixtureFactory(static::class)->get($className, $identifier); if (!$obj) { - user_error(sprintf( + throw new \InvalidArgumentException(sprintf( "Couldn't find object '%s' (class: %s)", $identifier, $className - ), E_USER_ERROR); + )); } return $obj; @@ -1021,16 +1021,16 @@ public static function start() $app = new HTTPApplication($kernel); $flush = array_key_exists('flush', $request->getVars()); - + // Custom application $app->execute($request, function (HTTPRequest $request) { // Start session and execute $request->getSession()->init($request); - + // Invalidate classname spec since the test manifest will now pull out new subclasses for each internal class // (e.g. Member will now have various subclasses of DataObjects that implement TestOnly) DataObject::reset(); - + // Set dummy controller; $controller = Controller::create(); $controller->setRequest($request); diff --git a/src/Forms/CompositeField.php b/src/Forms/CompositeField.php index 20517d0b532..8a4bb5b42df 100644 --- a/src/Forms/CompositeField.php +++ b/src/Forms/CompositeField.php @@ -251,11 +251,10 @@ public function collateDataFields(&$list, $saveableOnly = false) if (isset($list[$name])) { $fieldClass = get_class($field); $otherFieldClass = get_class($list[$name]); - user_error( + throw new \RuntimeException( "collateDataFields() I noticed that a field called '$name' appears twice in" . " your form: '{$formName}'. One is a '{$fieldClass}' and the other is a" - . " '{$otherFieldClass}'", - E_USER_ERROR + . " '{$otherFieldClass}'" ); } $list[$name] = $field; diff --git a/src/Forms/FieldList.php b/src/Forms/FieldList.php index eac95b74db5..c1d117c500a 100644 --- a/src/Forms/FieldList.php +++ b/src/Forms/FieldList.php @@ -165,15 +165,12 @@ protected function fieldNameError(FormField $field, $functionName) $errorSuffix = ''; } - user_error( - sprintf( - "%s() I noticed that a field called '%s' appears twice%s", - $functionName, - $field->getName(), - $errorSuffix - ), - E_USER_ERROR - ); + throw new \RuntimeException(sprintf( + "%s() I noticed that a field called '%s' appears twice%s", + $functionName, + $field->getName(), + $errorSuffix + )); } protected function flushFieldsCache() @@ -213,9 +210,8 @@ protected function collateDataFields(&$list, $saveableOnly = false) } else { $errSuffix = ''; } - user_error( - "collateDataFields() I noticed that a field called '$name' appears twice$errSuffix.", - E_USER_ERROR + throw new \RuntimeException( + "collateDataFields() I noticed that a field called '$name' appears twice$errSuffix." ); } $list[$name] = $field; @@ -493,10 +489,9 @@ public function findOrMakeTab($tabName, $title = null) ? " named '{$parentPointer->getName()}'" : null; $parentPointerClass = get_class($parentPointer); - user_error( + throw new \InvalidArgumentException( "FieldList::addFieldToTab() Tried to add a tab to object" - . " '{$parentPointerClass}'{$withName} - '{$part}' didn't exist.", - E_USER_ERROR + . " '{$parentPointerClass}'{$withName} - '{$part}' didn't exist." ); } } diff --git a/src/Forms/FormField.php b/src/Forms/FormField.php index fc6526589c3..414ba280898 100644 --- a/src/Forms/FormField.php +++ b/src/Forms/FormField.php @@ -1414,12 +1414,7 @@ public function rootFieldList() } $class = static::class; - user_error( - "rootFieldList() called on {$class} object without a containerFieldList", - E_USER_ERROR - ); - - return null; + throw new \RuntimeException("rootFieldList() called on {$class} object without a containerFieldList"); } /** diff --git a/src/Forms/SelectField.php b/src/Forms/SelectField.php index 2a93589defe..ec054cb2e75 100644 --- a/src/Forms/SelectField.php +++ b/src/Forms/SelectField.php @@ -173,7 +173,7 @@ protected function getListMap($source) $source = $source->toArray(); } 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; diff --git a/src/Forms/TreeMultiselectField.php b/src/Forms/TreeMultiselectField.php index c7ee21f3051..2857a5c30f1 100644 --- a/src/Forms/TreeMultiselectField.php +++ b/src/Forms/TreeMultiselectField.php @@ -2,6 +2,7 @@ namespace SilverStripe\Forms; +use http\Exception\InvalidArgumentException; use SilverStripe\Control\Controller; use SilverStripe\Core\Convert; use SilverStripe\ORM\ArrayList; @@ -252,10 +253,9 @@ public function saveInto(DataObjectInterface $record) $saveDest = $record->$fieldName(); if (!$saveDest) { $recordClass = get_class($record); - user_error( + throw new \RuntimeException( "TreeMultiselectField::saveInto() Field '$fieldName' not found on" - . " {$recordClass}.{$record->ID}", - E_USER_ERROR + . " {$recordClass}.{$record->ID}" ); } diff --git a/src/ORM/Connect/Database.php b/src/ORM/Connect/Database.php index 75cfae10f6b..da66d931444 100644 --- a/src/ORM/Connect/Database.php +++ b/src/ORM/Connect/Database.php @@ -431,9 +431,8 @@ public function manipulate($manipulation) break; default: - user_error( - "SS_Database::manipulate() Can't recognise command '{$writeInfo['command']}'", - E_USER_ERROR + throw new \InvalidArgumentException( + "SS_Database::manipulate() Can't recognise command '{$writeInfo['command']}'" ); } } diff --git a/src/ORM/Connect/MySQLiConnector.php b/src/ORM/Connect/MySQLiConnector.php index 2b2ae023a21..354b05e6872 100644 --- a/src/ORM/Connect/MySQLiConnector.php +++ b/src/ORM/Connect/MySQLiConnector.php @@ -2,9 +2,9 @@ namespace SilverStripe\ORM\Connect; -use SilverStripe\Core\Config\Config; use mysqli; use mysqli_stmt; +use SilverStripe\Core\Config\Config; /** * Connector for MySQL using the MySQLi method @@ -238,11 +238,9 @@ public function parsePreparedParameters($parameters, &$blobs) case 'array': case 'unknown type': default: - user_error( - "Cannot bind parameter \"$value\" as it is an unsupported type ($phpType)", - E_USER_ERROR + throw new \InvalidArgumentException( + "Cannot bind parameter \"$value\" as it is an unsupported type ($phpType)" ); - break; } $values[] = $value; } diff --git a/src/ORM/DataList.php b/src/ORM/DataList.php index ea5077a088d..6d6275fc240 100644 --- a/src/ORM/DataList.php +++ b/src/ORM/DataList.php @@ -1274,7 +1274,7 @@ public function offsetGet($key) */ 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 @@ public function offsetSet($key, $value) */ 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"); } } diff --git a/src/ORM/DataObject.php b/src/ORM/DataObject.php index 511bfaef09a..2e8888bde26 100644 --- a/src/ORM/DataObject.php +++ b/src/ORM/DataObject.php @@ -1391,8 +1391,10 @@ protected function preWrite() $this->brokenOnWrite = true; $this->onBeforeWrite(); if ($this->brokenOnWrite) { - user_error(static::class . " has a broken onBeforeWrite() function." - . " Make sure that you call parent::onBeforeWrite().", E_USER_ERROR); + throw new LogicException( + static::class . " has a broken onBeforeWrite() function." + . " Make sure that you call parent::onBeforeWrite()." + ); } } @@ -1737,8 +1739,10 @@ public function delete() $this->brokenOnDelete = true; $this->onBeforeDelete(); if ($this->brokenOnDelete) { - user_error(static::class . " has a broken onBeforeDelete() function." - . " Make sure that you call parent::onBeforeDelete().", E_USER_ERROR); + throw new LogicException( + static::class . " has a broken onBeforeDelete() function." + . " Make sure that you call parent::onBeforeDelete()." + ); } // Deleting a record without an ID shouldn't do anything @@ -2874,7 +2878,7 @@ public function setField($fieldName, $val) public function setCastedField($fieldName, $value) { 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); if ($fieldObj) { diff --git a/src/ORM/FieldType/DBBoolean.php b/src/ORM/FieldType/DBBoolean.php index eae84b0882c..0a75eea95e7 100644 --- a/src/ORM/FieldType/DBBoolean.php +++ b/src/ORM/FieldType/DBBoolean.php @@ -35,7 +35,7 @@ public function requireField() 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() @@ -50,7 +50,7 @@ public function saveInto($dataObject) $dataObject->$fieldName = ($this->value) ? 1 : 0; } else { $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 @@ public function scaffoldFormField($title = null, $params = null) public function scaffoldSearchField($title = null) { - $anyText = _t('SilverStripe\\ORM\\FieldType\\DBBoolean.ANY', 'Any'); + $anyText = _t(__CLASS__ . '.ANY', 'Any'); $source = [ - 1 => _t('SilverStripe\\ORM\\FieldType\\DBBoolean.YESANSWER', 'Yes'), - 0 => _t('SilverStripe\\ORM\\FieldType\\DBBoolean.NOANSWER', 'No') + 1 => _t(__CLASS__ . '.YESANSWER', 'Yes'), + 0 => _t(__CLASS__ . '.NOANSWER', 'No') ]; $field = new DropdownField($this->name, $title, $source); diff --git a/src/ORM/FieldType/DBDecimal.php b/src/ORM/FieldType/DBDecimal.php index d0b39a616c0..6e11b7bdc0e 100644 --- a/src/ORM/FieldType/DBDecimal.php +++ b/src/ORM/FieldType/DBDecimal.php @@ -90,7 +90,9 @@ public function saveInto($dataObject) if ($fieldName) { $dataObject->$fieldName = (float)preg_replace('/[^0-9.\-\+]/', '', $this->value); } 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" + ); } } diff --git a/src/ORM/FieldType/DBEnum.php b/src/ORM/FieldType/DBEnum.php index 28ea758a8cd..ab9da9781e7 100644 --- a/src/ORM/FieldType/DBEnum.php +++ b/src/ORM/FieldType/DBEnum.php @@ -5,6 +5,7 @@ use SilverStripe\Core\Config\Config; use SilverStripe\Forms\DropdownField; use SilverStripe\ORM\ArrayLib; +use SilverStripe\ORM\Connect\MySQLDatabase; use SilverStripe\ORM\DB; /** @@ -77,9 +78,8 @@ public function __construct($name = null, $enum = null, $default = 0, $options = if (in_array($default, $enum)) { $this->setDefault($default); } else { - user_error( - "Enum::__construct() The default value '$default' does not match any item in the enumeration", - E_USER_ERROR + throw new \InvalidArgumentException( + "Enum::__construct() The default value '$default' does not match any item in the enumeration" ); } } elseif (is_int($default) && $default < count($enum)) { @@ -99,8 +99,8 @@ public function __construct($name = null, $enum = null, $default = 0, $options = */ public function requireField() { - $charset = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'charset'); - $collation = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'collation'); + $charset = Config::inst()->get(MySQLDatabase::class, 'charset'); + $collation = Config::inst()->get(MySQLDatabase::class, 'collation'); $parts = [ 'datatype' => 'enum', diff --git a/src/ORM/FieldType/DBMultiEnum.php b/src/ORM/FieldType/DBMultiEnum.php index a39c5aa1edf..b4afcad4c8a 100644 --- a/src/ORM/FieldType/DBMultiEnum.php +++ b/src/ORM/FieldType/DBMultiEnum.php @@ -3,6 +3,7 @@ namespace SilverStripe\ORM\FieldType; use SilverStripe\Core\Config\Config; +use SilverStripe\ORM\Connect\MySQLDatabase; use SilverStripe\ORM\DB; use SilverStripe\Forms\CheckboxSetField; @@ -22,9 +23,10 @@ public function __construct($name = null, $enum = null, $default = null) $defaults = preg_split('/ *, */', trim($default)); foreach ($defaults as $thisDefault) { if (!in_array($thisDefault, $this->enum)) { - user_error("Enum::__construct() The default value '$thisDefault' does not match " - . "any item in the enumeration", E_USER_ERROR); - return; + throw new \InvalidArgumentException( + "Enum::__construct() The default value '$thisDefault' does not match " + . "any item in the enumeration" + ); } } $this->default = implode(',', $defaults); @@ -34,8 +36,8 @@ public function __construct($name = null, $enum = null, $default = null) public function requireField() { // @todo: Remove mysql-centric logic from this - $charset = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'charset'); - $collation = Config::inst()->get('SilverStripe\ORM\Connect\MySQLDatabase', 'collation'); + $charset = Config::inst()->get(MySQLDatabase::class, 'charset'); + $collation = Config::inst()->get(MySQLDatabase::class, 'collation'); $values=[ 'type'=>'set', 'parts'=>[ diff --git a/src/ORM/HasManyList.php b/src/ORM/HasManyList.php index 13fcbacaa20..c93c9cb6ea0 100644 --- a/src/ORM/HasManyList.php +++ b/src/ORM/HasManyList.php @@ -74,7 +74,7 @@ public function add($item) if (is_numeric($item)) { $item = DataObject::get_by_id($this->dataClass, $item); } 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(); @@ -123,10 +123,7 @@ public function removeByID($itemID) public function remove($item) { if (!($item instanceof $this->dataClass)) { - throw new InvalidArgumentException( - "HasManyList::remove() expecting a $this->dataClass object, or ID", - E_USER_ERROR - ); + throw new InvalidArgumentException("HasManyList::remove() expecting a $this->dataClass object, or ID"); } // Don't remove item which doesn't belong to this list diff --git a/src/ORM/Hierarchy/Hierarchy.php b/src/ORM/Hierarchy/Hierarchy.php index f5eb22375dc..0b344e449e1 100644 --- a/src/ORM/Hierarchy/Hierarchy.php +++ b/src/ORM/Hierarchy/Hierarchy.php @@ -368,9 +368,8 @@ public static function prepopulate_numchildren_cache($baseClass, $idList = null) // Validate the ID list foreach ($idList as $id) { if (!is_numeric($id)) { - user_error( - "Bad ID passed to Versioned::prepopulate_numchildren_cache() in \$idList: " . $id, - E_USER_ERROR + throw new \InvalidArgumentException( + "Bad ID passed to Versioned::prepopulate_numchildren_cache() in \$idList: " . $id ); } } diff --git a/src/ORM/Map.php b/src/ORM/Map.php index 127d55fb60d..c3dac10d4b1 100644 --- a/src/ORM/Map.php +++ b/src/ORM/Map.php @@ -212,10 +212,7 @@ public function offsetSet($key, $value) $this->lastItems[$key] = $value; } - user_error( - 'Map is read-only. Please use $map->push($key, $value) to append values', - E_USER_ERROR - ); + throw new \BadMethodCallException('Map is read-only. Please use $map->push($key, $value) to append values'); } /** @@ -243,9 +240,8 @@ public function offsetUnset($key) return; } - user_error( - "Map is read-only. Unset cannot be called on keys derived from the DataQuery", - E_USER_ERROR + throw new \BadMethodCallException( + 'Map is read-only. Unset cannot be called on keys derived from the DataQuery' ); } diff --git a/src/ORM/PolymorphicHasManyList.php b/src/ORM/PolymorphicHasManyList.php index cb9ddbbea41..3086869874f 100644 --- a/src/ORM/PolymorphicHasManyList.php +++ b/src/ORM/PolymorphicHasManyList.php @@ -109,8 +109,7 @@ public function remove($item) { if (!($item instanceof $this->dataClass)) { throw new InvalidArgumentException( - "HasManyList::remove() expecting a $this->dataClass object, or ID", - E_USER_ERROR + "HasManyList::remove() expecting a $this->dataClass object, or ID" ); } diff --git a/src/ORM/Queries/SQLConditionalExpression.php b/src/ORM/Queries/SQLConditionalExpression.php index 9ba2d97dca4..be5e37bf917 100644 --- a/src/ORM/Queries/SQLConditionalExpression.php +++ b/src/ORM/Queries/SQLConditionalExpression.php @@ -612,9 +612,8 @@ protected function parsePredicate($key, $value) if ($parameterCount === 1) { $key .= " = ?"; } elseif ($parameterCount > 1) { - user_error( - "Incorrect number of '?' in predicate $key. Expected $parameterCount but none given.", - E_USER_ERROR + throw new \InvalidArgumentException( + "Incorrect number of '?' in predicate $key. Expected $parameterCount but none given." ); } } @@ -622,9 +621,11 @@ protected function parsePredicate($key, $value) } elseif (is_array($value)) { // If predicates are nested one per array (as per the internal format) // then run a quick check over the contents and recursively parse - if (count($value) != 1) { - user_error('Nested predicates should be given as a single item array in ' - . 'array($predicate => array($prameters)) format)', E_USER_ERROR); + if (count($value) !== 1) { + throw new \InvalidArgumentException( + 'Nested predicates should be given as a single item array in ' + . 'array($predicate => array($prameters)) format)' + ); } foreach ($value as $key => $pairValue) { return $this->parsePredicate($key, $pairValue); diff --git a/src/ORM/UnsavedRelationList.php b/src/ORM/UnsavedRelationList.php index ba65e2ba902..1e940383bed 100644 --- a/src/ORM/UnsavedRelationList.php +++ b/src/ORM/UnsavedRelationList.php @@ -95,10 +95,10 @@ public function changeToList(RelationList $list) public function push($item, $extraFields = null) { if ((is_object($item) && !$item instanceof $this->dataClass) - || (!is_object($item) && !is_numeric($item))) { + || (!is_object($item) && !is_numeric($item)) + ) { throw new InvalidArgumentException( - "UnsavedRelationList::add() expecting a $this->dataClass object, or ID value", - E_USER_ERROR + "UnsavedRelationList::add() expecting a $this->dataClass object, or ID value" ); } if (is_object($item) && $item->ID) { diff --git a/src/Security/Member_GroupSet.php b/src/Security/Member_GroupSet.php index f2f948c6b79..bc51f6d8dec 100644 --- a/src/Security/Member_GroupSet.php +++ b/src/Security/Member_GroupSet.php @@ -19,7 +19,7 @@ protected function linkJoinTable() { // Do not join the table directly 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'); } } diff --git a/src/Security/Permission.php b/src/Security/Permission.php index 4db499ed797..6866d3d194e 100644 --- a/src/Security/Permission.php +++ b/src/Security/Permission.php @@ -246,7 +246,7 @@ public static function checkMember($member, $code, $arg = "any", $strict = true) $argClause = "AND \"Arg\" IN (?, ?) "; $argParams = [-1, $arg]; } else { - user_error("Permission::checkMember: bad arg '$arg'", E_USER_ERROR); + throw new \InvalidArgumentException("Permission::checkMember: bad arg '$arg'"); } } @@ -408,10 +408,7 @@ public static function grant($groupID, $code, $arg = "any") if (is_numeric($arg)) { $perm->Arg = $arg; } else { - user_error( - "Permission::checkMember: bad arg '$arg'", - E_USER_ERROR - ); + throw new \InvalidArgumentException("Permission::checkMember: bad arg '$arg'"); } } @@ -446,10 +443,7 @@ public static function deny($groupID, $code, $arg = "any") if (is_numeric($arg)) { $perm->Arg = $arg; } else { - user_error( - "Permission::checkMember: bad arg '$arg'", - E_USER_ERROR - ); + throw new \InvalidArgumentException("Permission::checkMember: bad arg '$arg'"); } } diff --git a/src/View/Parsers/ShortcodeParser.php b/src/View/Parsers/ShortcodeParser.php index ffa26cd33d9..1a4f468cc2e 100644 --- a/src/View/Parsers/ShortcodeParser.php +++ b/src/View/Parsers/ShortcodeParser.php @@ -191,7 +191,7 @@ public function getShortcodeReplacementText($tag, $extra = [], $isHTMLAllowed = // Missing tag if ($content === false) { 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) { $content = '' . $tag['text'] . ''; } elseif (ShortcodeParser::$error_behavior == ShortcodeParser::STRIP) { @@ -381,7 +381,7 @@ public function extractTags($content) if ($err) { if (self::$error_behavior == self::ERROR) { - user_error($err, E_USER_ERROR); + throw new \Exception($err); } } else { if ($tags[$i]['escaped']) { @@ -603,7 +603,7 @@ protected function moveMarkerToCompliantHome($node, $parent, $location) } // NOP } 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 @@ public function parse($content) // Now parse the result into a DOM if (!$htmlvalue->isValid()) { 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 { $continue = false; } @@ -706,9 +706,8 @@ public function parse($content) if (!$parent) { if ($location !== self::INLINE) { - user_error( - "Parent block for shortcode couldn't be found, but location wasn't INLINE", - E_USER_ERROR + throw new \RuntimeException( + "Parent block for shortcode couldn't be found, but location wasn't INLINE" ); } } else { diff --git a/src/View/SSViewer_DataPresenter.php b/src/View/SSViewer_DataPresenter.php index dafc9e3e344..1d37875eeb1 100644 --- a/src/View/SSViewer_DataPresenter.php +++ b/src/View/SSViewer_DataPresenter.php @@ -249,7 +249,7 @@ public function obj($name, $arguments = [], $cache = false, $cacheName = null) case 'Up': $upIndex = $this->getUpIndex(); 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 diff --git a/src/View/SSViewer_Scope.php b/src/View/SSViewer_Scope.php index fdbce4c730d..250a965dd1a 100644 --- a/src/View/SSViewer_Scope.php +++ b/src/View/SSViewer_Scope.php @@ -191,7 +191,7 @@ public function obj($name, $arguments = [], $cache = false, $cacheName = null) switch ($name) { case 'Up': 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( diff --git a/tests/php/Control/HTTPRequestTest.php b/tests/php/Control/HTTPRequestTest.php index 51ea76fa3c4..52ce61b4923 100644 --- a/tests/php/Control/HTTPRequestTest.php +++ b/tests/php/Control/HTTPRequestTest.php @@ -160,11 +160,9 @@ public function testDetectMethod($realMethod, $post, $expected) $this->assertEquals($expected, $actual); } - /** - * @expectedException PHPUnit_Framework_Error - */ public function testBadDetectMethod() { + $this->expectException(\InvalidArgumentException::class); HTTPRequest::detect_method('POST', ['_method' => 'Boom']); } @@ -191,11 +189,9 @@ public function testSetHttpMethod($method, $expected) $this->assertEquals($request, $returnedRequest); } - /** - * @expectedException PHPUnit_Framework_Error - */ public function testBadSetHttpMethod() { + $this->expectException(\InvalidArgumentException::class); $request = new HTTPRequest('GET', '/hello'); $request->setHttpMethod('boom'); } diff --git a/tests/php/Forms/CompositeFieldTest.php b/tests/php/Forms/CompositeFieldTest.php index 382c339dce7..6bc72cda452 100644 --- a/tests/php/Forms/CompositeFieldTest.php +++ b/tests/php/Forms/CompositeFieldTest.php @@ -2,7 +2,6 @@ namespace SilverStripe\Forms\Tests; -use PHPUnit_Framework_Error; use SilverStripe\Dev\CSSContentParser; use SilverStripe\Dev\SapphireTest; use SilverStripe\Forms\CompositeField; @@ -161,12 +160,13 @@ public function testGetAttributesReturnsEmptyTitleForFieldSets() $this->assertNull($result['title']); } - /** - * @expectedException PHPUnit_Framework_Error - * @expectedExceptionMessageRegExp /a field called 'Test' appears twice in your form.*TextField.*TextField/ - */ public function testCollateDataFieldsThrowsErrorOnDuplicateChildren() { + $this->expectException(\RuntimeException::class); + $this->expectExceptionMessageRegExp( + "/a field called 'Test' appears twice in your form.*TextField.*TextField/" + ); + $field = CompositeField::create( TextField::create('Test'), TextField::create('Test')