[RFC] Drop auto injection of `NotEmpty` validator for required inputs

[gsteel]

Problem

Auto injecting a NotEmpty validator is very possibly not wanted in some situations but completely unavoidable if you mark an input as required.

If you are translating your error messages, the auto-injected validator will not receive a translator instance and will yield the default (probably english) error message.

Iterating over and manipulating the validator inside Input relies on concrete methods in ValidatorChain that are not present in ValidatorChainInterface.
Furthemore, the chain that is provided or configured should not, IMO, be mutated after it has been attached.

Proposed Solution

Whether a field is "empty" or not, and whether that matters is a validation concern and should be entirely delegated to the validator chain.

It should be the users responsibility to add sufficient validation to each input for their use case, therefore, I'd suggest:

• Complete removal of Input::injectNotEmptyValidator(), Input::prepareRequiredValidationFailureMessage() and related methods
• Removal of Input::setRequired() and Input::isRequired() - again, this is a validation concern and part of the chain configuration

Impact to users

… is quite significant…

Any fields/inputs that do not configure validation beyond marking the input as required would pass validation for empty values where previously they would not have.

This could possibly be mitigated by throwing an exception when setRequired() is called, or, the required key is present in a specification, alerting users to the change.

This would force users to audit input filters and specs for sufficient validation.

[froschdesign]

Whether a field is "empty" or not, and whether that matters is a validation concern and should be entirely delegated to the validator chain.

This means all conditions before $validator->isValid(…) must be removed:

laminas-inputfilter/src/Input.php

Lines 338 to 371 in 8e1a130

	if (! $hasValue && $this->hasFallback()) {
	$this->setValue($this->getFallbackValue());
	return true;
	}
	if (! $hasValue && ! $required) {
	return true;
	}
	if (! $hasValue) { // required, but no value
	if ($this->errorMessage === null) {
	$this->errorMessage = $this->prepareRequiredValidationFailureMessage();
	}
	return false;
	}
	if ($empty && ! $required && ! $continueIfEmpty) {
	return true;
	}
	if ($empty && $allowEmpty && ! $continueIfEmpty) {
	return true;
	}
	// At this point, we need to run validators.
	// If we do not allow empty and the "continue if empty" flag are
	// BOTH false, we inject the "not empty" validator into the chain,
	// which adds that logic into the validation routine.
	if (! $allowEmpty && ! $continueIfEmpty) {
	$this->injectNotEmptyValidator();
	}
	$validator = $this->getValidatorChain();
	$result = $validator->isValid($value, $context);

Correct?

[froschdesign]

Related to #7

[gsteel]

Pretty much, yes.

An input will always have a value because it is given null when the source data misses the relevant key entirely.

But, the "Fallback Value" still has merit as a feature of input filter specifially.

Potentially, this could be simplified to:

private mixed $fallbackValue = null;
private mixed $value = null;

public function isValid(array $context): bool
{
    $value = $this->value ?? $this->fallbackValue;
    
    return $this->validatorChain->isValid($value, $context);
}

The problem is, "Continue if Empty" also has merit. It is useful to say "If the value is empty, I don't want the value to be validated at all"

private mixed $fallbackValue = null;
private mixed $value = null;
private bool $continueIfEmpty = true;

public function isValid(array $context): bool
{
    $value = $this->value ?? $this->fallbackValue;

    if ($this->continueIfEmpty === true && $value === null) {
        return true;
    }
    
    return $this->validatorChain->isValid($value, $context);
}

But we now have to decide what we should consider as an empty value. Currently this is ''|null|[]

So, maybe I'm eating my own words… I'm open to suggestions 😂

[froschdesign]

• Removal of Input::setRequired() and Input::isRequired() - again, this is a validation concern and part of the chain configuration

The meaning of require differs from what is often expected or what we know from HTML forms:

$inputFilter->add([
    'name'        => 'title',
    'required'    => true,
    'allow_empty' => true,
]);
$inputFilter->add([
    'name'     => 'artist',
    'required' => false,
]);

$inputFilter->setData([]);
$inputFilter->isValid(); // false

$inputFilter->setData(['title' => '']);
$inputFilter->isValid(); // true

In the first input, require simply means that the element must be present but can be empty. In the second input, require means that it can be ignored.

In HTML forms the meaning is different:

required is a Boolean attribute which, if present, indicates that the user must specify a value for the input before the owning form can be submitted.

https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input#required

If we move the checks to validators – which I believe is the right thing to do – this will result in changes in behaviour.

[froschdesign]

But, the "Fallback Value" still has merit as a feature of input filter specifially.

But should the fallback value be validated? Would that even work? I think not in all scenarios.