Skip to content

fix: ValidationInterface #6253

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Sep 7, 2022
5 changes: 0 additions & 5 deletions phpstan-baseline.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -635,11 +635,6 @@ parameters:
count: 1
path: system/Throttle/Throttler.php

-
message: "#^Variable \\$error on left side of \\?\\? always exists and is always null\\.$#"
count: 1
path: system/Validation/Validation.php

-
message: "#^Property CodeIgniter\\\\View\\\\Cell\\:\\:\\$cache \\(CodeIgniter\\\\Cache\\\\CacheInterface\\) in empty\\(\\) is not falsy\\.$#"
count: 2
Expand Down
5 changes: 2 additions & 3 deletions system/BaseModel.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
use CodeIgniter\Exceptions\ModelException;
use CodeIgniter\I18n\Time;
use CodeIgniter\Pager\Pager;
use CodeIgniter\Validation\Validation;
use CodeIgniter\Validation\ValidationInterface;
use Config\Services;
use InvalidArgumentException;
Expand Down Expand Up @@ -198,7 +197,7 @@ abstract class BaseModel
/**
* Our validator instance.
*
* @var Validation
* @var ValidationInterface
*/
protected $validation;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me that it makes sense to specify the property type directly instead of docBlock in BaseModel and Controller.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing user's Model will break.
https://3v4l.org/NkQVr

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why override the property?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. I don't know, but we can override.
I should have said, the existing user's Model might break.


Expand Down Expand Up @@ -326,7 +325,7 @@ public function __construct(?ValidationInterface $validation = null)
$this->tempAllowCallbacks = $this->allowCallbacks;

/**
* @var Validation|null $validation
* @var ValidationInterface|null $validation
*/
$validation ??= Services::validation(null, false);
$this->validation = $validation;
Expand Down
4 changes: 2 additions & 2 deletions system/Config/BaseService.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
use CodeIgniter\Session\Session;
use CodeIgniter\Throttle\Throttler;
use CodeIgniter\Typography\Typography;
use CodeIgniter\Validation\Validation;
use CodeIgniter\Validation\ValidationInterface;
use CodeIgniter\View\Cell;
use CodeIgniter\View\Parser;
use CodeIgniter\View\RendererInterface;
Expand Down Expand Up @@ -127,7 +127,7 @@
* @method static Toolbar toolbar(ConfigToolbar $config = null, $getShared = true)
* @method static Typography typography($getShared = true)
* @method static URI uri($uri = null, $getShared = true)
* @method static Validation validation(ConfigValidation $config = null, $getShared = true)
* @method static ValidationInterface validation(ConfigValidation $config = null, $getShared = true)
* @method static Cell viewcell($getShared = true)
*/
class BaseService
Expand Down
3 changes: 2 additions & 1 deletion system/Config/Services.php
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
use CodeIgniter\Throttle\Throttler;
use CodeIgniter\Typography\Typography;
use CodeIgniter\Validation\Validation;
use CodeIgniter\Validation\ValidationInterface;
use CodeIgniter\View\Cell;
use CodeIgniter\View\Parser;
use CodeIgniter\View\RendererInterface;
Expand Down Expand Up @@ -733,7 +734,7 @@ public static function uri(?string $uri = null, bool $getShared = true)
/**
* The Validation class provides tools for validating input data.
*
* @return Validation
* @return ValidationInterface
*/
public static function validation(?ValidationConfig $config = null, bool $getShared = true)
{
Expand Down
4 changes: 2 additions & 2 deletions system/Controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
use CodeIgniter\HTTP\RequestInterface;
use CodeIgniter\HTTP\ResponseInterface;
use CodeIgniter\Validation\Exceptions\ValidationException;
use CodeIgniter\Validation\Validation;
use CodeIgniter\Validation\ValidationInterface;
use Config\Services;
use Psr\Log\LoggerInterface;

Expand Down Expand Up @@ -62,7 +62,7 @@ class Controller
/**
* Once validation has been run, will hold the Validation instance.
*
* @var Validation
* @var ValidationInterface
*/
protected $validator;

Expand Down
13 changes: 8 additions & 5 deletions system/Validation/Validation.php
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,7 @@ protected function processRules(

$param = ($param === false) ? '' : $param;

// @phpstan-ignore-next-line $error may be set by rule methods.
$this->errors[$field] = $error ?? $this->getErrorMessage(
$rule,
$field,
Expand Down Expand Up @@ -405,7 +406,7 @@ public function withRequest(RequestInterface $request): ValidationInterface
*
* [
* 'rule' => 'message',
* 'rule' => 'message'
* 'rule' => 'message',
* ]
*
* @param array|string $rules
Expand Down Expand Up @@ -499,7 +500,7 @@ public function hasRule(string $field): bool
*
* @param string $group Group.
*
* @throws InvalidArgumentException If group not found.
* @throws ValidationException If group not found.
*
* @return string[] Rule group.
*/
Expand All @@ -521,7 +522,7 @@ public function getRuleGroup(string $group): array
*
* @param string $group Group.
*
* @throws InvalidArgumentException If group not found.
* @throws ValidationException If group not found.
*/
public function setRuleGroup(string $group)
{
Expand Down Expand Up @@ -593,12 +594,14 @@ protected function loadRuleSets()
* same format used with setRules(). Additionally, check
* for {group}_errors for an array of custom error messages.
*
* @return array|ValidationException|null
* @throws ValidationException
*
* @return array
*/
public function loadRuleGroup(?string $group = null)
{
if (empty($group)) {
return null;
return [];
}

if (! isset($this->config->{$group})) {
Expand Down
74 changes: 70 additions & 4 deletions system/Validation/ValidationInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,13 @@ interface ValidationInterface
{
/**
* Runs the validation process, returning true/false determining whether
* or not validation was successful.
* validation was successful or not.
*
* @param array $data The array of data to validate.
* @param string $group The pre-defined group of rules to apply.
* @param array|null $data The array of data to validate.
* @param string|null $group The predefined group of rules to apply.
* @param string|null $dbGroup The database group to use.
*/
public function run(?array $data = null, ?string $group = null): bool;
public function run(?array $data = null, ?string $group = null, ?string $dbGroup = null): bool;

/**
* Check; runs the validation process, returning true or false
Expand All @@ -45,16 +46,54 @@ public function check($value, string $rule, array $errors = []): bool;
*/
public function withRequest(RequestInterface $request): ValidationInterface;

/**
* Sets an individual rule and custom error messages for a single field.
*
* The custom error message should be just the messages that apply to
* this field, like so:
*
* [
* 'rule' => 'message',
* 'rule' => 'message',
* ]
*
* @param array|string $rules
*
* @return $this
*/
public function setRule(string $field, ?string $label, $rules, array $errors = []);

/**
* Stores the rules that should be used to validate the items.
*/
public function setRules(array $rules, array $messages = []): ValidationInterface;

/**
* Returns all of the rules currently defined.
*/
public function getRules(): array;

/**
* Checks to see if the rule for key $field has been set or not.
*/
public function hasRule(string $field): bool;

/**
* Get rule group.
*
* @param string $group Group.
*
* @return string[] Rule group.
*/
public function getRuleGroup(string $group): array;

/**
* Set rule group.
*
* @param string $group Group.
*/
public function setRuleGroup(string $group);

/**
* Returns the error for a specified $field (or empty string if not set).
*/
Expand Down Expand Up @@ -83,4 +122,31 @@ public function setError(string $alias, string $error): ValidationInterface;
* you need to process more than one array.
*/
public function reset(): ValidationInterface;

/**
* Loads custom rule groups (if set) into the current rules.
*
* Rules can be pre-defined in Config\Validation and can
* be any name, but must all still be an array of the
* same format used with setRules(). Additionally, check
* for {group}_errors for an array of custom error messages.
*
* @return array
*/
public function loadRuleGroup(?string $group = null);

/**
* Checks to see if an error exists for the given field.
*/
public function hasError(string $field): bool;

/**
* Returns the rendered HTML of the errors as defined in $template.
*/
public function listErrors(string $template = 'list'): string;

/**
* Displays a single error in formatted HTML as defined in the $template view.
*/
public function showError(string $field, string $template = 'single'): string;
}
29 changes: 29 additions & 0 deletions user_guide_src/source/changelogs/v4.3.0.rst
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,35 @@ For example, the Exit code has been changed like the following:
- If an uncaught ``CastException`` occurs, the Exit code is ``EXIT_CONFIG`` (= ``3``) instead of ``9``.
- If an uncaught ``DatabaseException`` occurs, the Exit code is ``EXIT_DATABASE`` (= ``8``) instead of ``17``.

Method Signature Changes
========================

.. _v430_validation_changes:

Validation Changes
------------------

ValidationInterface
^^^^^^^^^^^^^^^^^^^

``ValidationInterface`` has been changed to eliminate the mismatch between ``ValidationInterface`` and the ``Validation`` class.

- The third parameter ``$dbGroup`` for ``ValidationInterface::run()`` has been added.
- The following methods are added to the interface:
- ``ValidationInterface::setRule()``
- ``ValidationInterface::getRules()``
- ``ValidationInterface::getRuleGroup()``
- ``ValidationInterface::setRuleGroup()``
- ``ValidationInterface::loadRuleGroup()``
- ``ValidationInterface::hasError()``
- ``ValidationInterface::listErrors()``
- ``ValidationInterface::showError()``

Validation
^^^^^^^^^^

The return value of ``Validation::loadRuleGroup()`` has been changed ``null`` to ``[]`` when the ``$group`` is empty.

Others
------

Expand Down
6 changes: 6 additions & 0 deletions user_guide_src/source/installation/upgrade_430.rst
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,12 @@ If you have code that depends on the bug, you need to change the code.
Use new Form helpers, :php:func:`validation_errors()`, :php:func:`validation_list_errors()` and :php:func:`validation_show_error()` to display Validation Errors,
instead of the Validation object.

Validation Changes
==================

- ``ValidationInterface`` has been changed. Implemented classes should likewise add the methods and the parameters so as not to break LSP. See :ref:`v430_validation_changes` for details.
- The return value of ``Validation::loadRuleGroup()`` has been changed ``null`` to ``[]`` when the ``$group`` is empty. Update the code if you depend on the behavior.

.. _upgrade-430-stream-filter:

Capturing STDERR and STDOUT streams in tests
Expand Down