Validation: support placeholders for anything

system/Validation/Validation.php

@@ -17,6 +17,7 @@
 use CodeIgniter\View\RendererInterface;
 use Config\Validation as ValidationConfig;
 use InvalidArgumentException;
+use TypeError;
 
 /**
  * Validator
@@ -362,18 +363,30 @@ public function withRequest(RequestInterface $request): ValidationInterface
  * 'rule' => 'message'
  * ]
  *
+ * @param array|string $rules
+ *
+ * @throws TypeError
+ *
  * @return $this
  */
- public function setRule(string $field, ?string $label, string $rules, array $errors = [])
+ public function setRule(string $field, ?string $label, $rules, array $errors = [])
  {
- $this->rules[$field] = [
- 'label' => $label,
- 'rules' => $rules,
+ if (! is_array($rules) && ! is_string($rules)) {
+ throw new TypeError('$rules must be of type string|array');
+ }
+
+ $ruleSet = [
+ $field => [
+ 'label' => $label,
+ 'rules' => $rules,
+ ],
  ];
 
- $this->customErrors = array_merge($this->customErrors, [
- $field => $errors,
- ]);
+ if ($errors) {
+ $ruleSet[$field]['errors'] = $errors;
+ }
+
+ $this->setRules($ruleSet + $this->getRules());
 
  return $this;
  }
@@ -401,16 +414,18 @@ public function setRules(array $rules, array $errors = []): ValidationInterface
  $this->customErrors = $errors;
 
  foreach ($rules as $field => &$rule) {
- if (! is_array($rule)) {
- continue;
- }
+ if (is_array($rule)) {
+ if (array_key_exists('errors', $rule)) {
+ $this->customErrors[$field] = $rule['errors'];
+ unset($rule['errors']);
+ }
 
- if (! array_key_exists('errors', $rule)) {
- continue;
+ // if $rule is already a rule collection, just move it to "rules"
+ // transforming [foo => [required, foobar]] to [foo => [rules => [required, foobar]]]
+ if (! array_key_exists('rules', $rule)) {
+ $rule = ['rules' => $rule];
+ }
  }
-
- $this->customErrors[$field] = $rule['errors'];
- unset($rule['errors']);
  }
 
  $this->rules = $rules;
@@ -581,23 +596,27 @@ protected function fillPlaceholders(array $rules, array $data): array
  $replacements["{{$key}}"] = $value;
  }
 
- if (! empty($replacements)) {
+ if ($replacements !== []) {
  foreach ($rules as &$rule) {
- if (is_array($rule)) {
- foreach ($rule as &$row) {
- // Should only be an `errors` array
- // which doesn't take placeholders.
- if (is_array($row)) {
- continue;
- }
+ $ruleSet = $rule['rules'] ?? $rule;
 
- $row = strtr($row ?? '', $replacements);
+ if (is_array($ruleSet)) {
+ foreach ($ruleSet as &$row) {
+ if (is_string($row)) {
+ $row = strtr($row, $replacements);
+ }
  }
+ }
 
- continue;
+ if (is_string($ruleSet)) {
+ $ruleSet = strtr($ruleSet, $replacements);
  }
 
- $rule = strtr($rule ?? '', $replacements);
+ if (isset($rule['rules'])) {
+ $rule['rules'] = $ruleSet;
+ } else {
+ $rule = $ruleSet;
+ }
  }
  }
 

tests/system/Validation/ValidationTest.php

@@ -19,7 +19,9 @@
 use Config\App;
 use Config\Services;
 use Generator;
+use PHPUnit\Framework\ExpectationFailedException;
 use Tests\Support\Validation\TestRules;
+use TypeError;
 
 /**
  * @internal
@@ -83,7 +85,109 @@ public function testSetRulesStoresRules(): void
  $this->assertSame($rules, $this->validation->getRules());
  }
 
- public function testRunReturnsFalseWithNothingToDo(): void
+ public function testSetRuleStoresRule()
+ {
+ $this->validation->setRules([]);
+ $this->validation->setRule('foo', null, 'bar|baz');
+
+ $this->assertSame([
+ 'foo' => [
+ 'label' => null,
+ 'rules' => 'bar|baz',
+ ],
+ ], $this->validation->getRules());
+ }
+
+ public function testSetRuleAddsRule()
+ {
+ $this->validation->setRules([
+ 'bar' => [
+ 'label' => null,
+ 'rules' => 'bar|baz',
+ ],
+ ]);
+ $this->validation->setRule('foo', null, 'foo|foz');
+
+ $this->assertSame([
+ 'foo' => [
+ 'label' => null,
+ 'rules' => 'foo|foz',
+ ],
+ 'bar' => [
+ 'label' => null,
+ 'rules' => 'bar|baz',
+ ],
+ ], $this->validation->getRules());
+ }
+
+ public function testSetRuleOverwritesRule()
+ {
+ $this->validation->setRules([
+ 'foo' => [
+ 'label' => null,
+ 'rules' => 'bar|baz',
+ ],
+ ]);
+ $this->validation->setRule('foo', null, 'foo|foz');
+
+ $this->assertSame([
+ 'foo' => [
+ 'label' => null,
+ 'rules' => 'foo|foz',
+ ],
+ ], $this->validation->getRules());
+ }
+
+ /**
+ * @dataProvider setRuleRulesFormatCaseProvider
+ *
+ * @param mixed $rules
+ */
+ public function testSetRuleRulesFormat(bool $expected, $rules): void
+ {
+ if (! $expected) {
+ $this->expectException(TypeError::class);
+ $this->expectExceptionMessage('$rules must be of type string|array');
+ }
+
+ $this->validation->setRule('foo', null, $rules);
+ $this->addToAssertionCount(1);
+ }
+
+ public function setRuleRulesFormatCaseProvider(): iterable
+ {
+ yield 'fail-simple-object' => [
+ false,
+ (object) ['required'],
+ ];
+
+ yield 'pass-single-string' => [
+ true,
+ 'required',
+ ];
+
+ yield 'pass-single-array' => [
+ true,
+ ['required'],
+ ];
+
+ yield 'fail-deep-object' => [
+ false,
+ new Validation((object) $this->config, Services::renderer()),
+ ];
+
+ yield 'pass-multiple-string' => [
+ true,
+ 'required|alpha',
+ ];
+
+ yield 'pass-multiple-array' => [
+ true,
+ ['required', 'alpha'],
+ ];
+ }
+
+ public function testRunReturnsFalseWithNothingToDo()
  {
  $this->validation->setRules([]);
  $this->assertFalse($this->validation->run([]));
@@ -1008,4 +1112,102 @@ public function provideStringRulesCases(): iterable
  ['required', 'regex_match[/^(01|2689|09)[0-9]{8}$/]', 'numeric'],
  ];
  }
+
+ /**
+ * internal method to simplify placeholder replacement test
+ * REQUIRES THE RULES TO BE SET FOR THE FIELD "foo"
+ *
+ * @param array|null $data optional POST data, needs to contain the key $placeholderField to pass
+ *
+ * @source https://github.com/codeigniter4/CodeIgniter4/pull/3910#issuecomment-784922913
+ */
+ private function placeholderReplacementResultDetermination(string $placeholder = 'id', ?array $data = null)
+ {
+ if ($data === null) {
+ $data = [$placeholder => 'placeholder-value'];
+ }
+
+ $validationRules = $this->getPrivateMethodInvoker($this->validation, 'fillPlaceholders')($this->validation->getRules(), $data);
+ $fieldRules = $validationRules['foo']['rules'] ?? $validationRules['foo'];
+ if (is_string($fieldRules)) {
+ $fieldRules = $this->getPrivateMethodInvoker($this->validation, 'splitRules')($fieldRules);
+ }
+
+ // loop all rules for this field
+ foreach ($fieldRules as $rule) {
+ // only string type rules are supported
+ if (is_string($rule)) {
+ $this->assertStringNotContainsString('{' . $placeholder . '}', $rule);
+ }
+ }
+ }
+
+ /**
+ * @see ValidationTest::placeholderReplacementResultDetermination()
+ */
+ public function testPlaceholderReplacementTestFails()
+ {
+ // to test if placeholderReplacementResultDetermination() works we provoke and expect an exception
+ $this->expectException(ExpectationFailedException::class);
+ $this->expectExceptionMessage('Failed asserting that \'filter[{id}]\' does not contain "{id}".');
+
+ $this->validation->setRule('foo', 'foo-label', 'required|filter[{id}]');
+
+ // calling with empty $data should produce an exception since {id} can't be replaced
+ $this->placeholderReplacementResultDetermination('id', []);
+ }
+
+ public function testPlaceholderReplacementSetSingleRuleString()
+ {
+ $this->validation->setRule('foo', null, 'required|filter[{id}]');
+
+ $this->placeholderReplacementResultDetermination();
+ }
+
+ public function testPlaceholderReplacementSetSingleRuleArray()
+ {
+ $this->validation->setRule('foo', null, ['required', 'filter[{id}]']);
+
+ $this->placeholderReplacementResultDetermination();
+ }
+
+ public function testPlaceholderReplacementSetMultipleRulesSimpleString()
+ {
+ $this->validation->setRules([
+ 'foo' => 'required|filter[{id}]',
+ ]);
+
+ $this->placeholderReplacementResultDetermination();
+ }
+
+ public function testPlaceholderReplacementSetMultipleRulesSimpleArray()
+ {
+ $this->validation->setRules([
+ 'foo' => ['required', 'filter[{id}]'],
+ ]);
+
+ $this->placeholderReplacementResultDetermination();
+ }
+
+ public function testPlaceholderReplacementSetMultipleRulesComplexString()
+ {
+ $this->validation->setRules([
+ 'foo' => [
+ 'rules' => 'required|filter[{id}]',
+ ],
+ ]);
+
+ $this->placeholderReplacementResultDetermination();
+ }
+
+ public function testPlaceholderReplacementSetMultipleRulesComplexArray()
+ {
+ $this->validation->setRules([
+ 'foo' => [
+ 'rules' => ['required', 'filter[{id}]'],
+ ],
+ ]);
+
+ $this->placeholderReplacementResultDetermination();
+ }
 }

user_guide_src/source/changelogs/v4.2.0.rst

@@ -12,6 +12,7 @@ Release Date: Not Released
 BREAKING
 ********
 
+- The method signature of ``Validation::setRule()`` has been changed. The ``string`` typehint on the ``$rules`` parameter was removed. Extending classes should likewise remove the parameter so as not to break LSP.
 
 
 Enhancements

user_guide_src/source/libraries/validation.rst

@@ -261,20 +261,28 @@ methods.
 setRule()
 =========
 
-This method sets a single rule. It takes the name of the field as
-the first parameter, an optional label and a string with a pipe-delimited list of rules
-that should be applied::
+This method sets a single rule. It has the method signature
 
- $validation->setRule('username', 'Username', 'required');
+ setRule(string $field, ?string $label, array|string $rules[, array $errors = []])
 
-The **field name** must match the key of any data array that is sent in. If
+The ``$rules`` either takes in a pipe-delimited list of rules or an array collection of rules.
+
+ $validation->setRule('username', 'Username', 'required|min_length[3]');
+ $validation->setRule('password', 'Password', ['required', 'min_length[8]', 'alpha_numeric_punct']);
+
+The value you pass to ``$field`` must match the key of any data array that is sent in. If
 the data is taken directly from ``$_POST``, then it must be an exact match for
 the form input name.
 
+.. warning:: Prior to v4.2.0, this method's third parameter, ``$rules``, was typehinted to accept
+ ``string``. In v4.2.0 and after, the typehint was removed to allow arrays, too. To avoid LSP being
+ broken in extending classes overriding this method, the child class's method should also be modified
+ to remove the typehint.
+
 setRules()
 ==========
 
-Like, ``setRule()``, but accepts an array of field names and their rules::
+Like ``setRule()``, but accepts an array of field names and their rules::
 
  $validation->setRules([
  'username' => 'required',