InputFilterInterface::getInvalidInput() and InputFilterInterface::getValidInput() are incosistent with InputFilterInterface::add()

[func0der]

Bug Report

Q	A
Versions	Since version 2.0

Summary

While InputFilterInterface::add() allows InputInterface|InputFilterInterface|array|Traversable to be added, InputFilterInterface::getInvalidInput() and InputFilterInterface::getValidInput() only return InputInterface to be returned.

This leads to inconsistent behavior, because InputFilterInterface does not implement InputInterface and would cause errors in instances where they are treated as equal (for example here: https://github.com/mvlabs/ze-content-validation/blob/master/src/Validator/ValidationResult.php#L68-L70).

The included implementation of InputFilterInterface \Laminas\InputFilter\BaseInputFilter does not comply to the interface itself either. It fills its properties BaseInputFilter::$validInputs and BaseInputFilter::$invalidInputs with InputInterface and InputFilterInterface alike and though only type hints @return InputInterface[] on BaseInputFilter::getInvalidInputs() and BaseInputFilter::getValidInputs().

Current behavior

BaseInputFilter::getInvalidInputs() and BaseInputFilter::getValidInputs() return InputFilterInterface and InputFilter alike.

How to reproduce

$requiredInput = new \Laminas\InputFilter\Input();
$requiredInput->setRequired(true);

$inputFilter = new \Laminas\InputFilter\BaseInputFilter();
$inputFilter->add(clone $requiredInput, 'foo');

$nestedInputFilter = new \Laminas\InputFilter\BaseInputFilter();
$nestedInputFilter->add(clone $requiredInput, 'bar1');
$inputFilter->add($nestedInputFilter, 'bar');

// Date in here is purposely commented out, to land their respective inputs/input filter in `$invalidInput`
$data = [
    // 'foo' => 'ff',
    'bar' => [
        // 'bar1' => 'gg',
    ]
];
$inputFilter->setData($data);
$inputFilter->isValid();

foreach ($inputFilter->getInvalidInput() as $input) {
    echo $input->getName();
}

This code is no beauty, but it shows the problem.

Expected behavior

This is not 100% clear, because I can not say, if the interface is right or the implementation.

The result that I would wish for is, that the InputFilterInterface would type hint InputFilterInterface and InputInterface.

---

There are two ways of this problem at this point in time.

Either the output of BaseInputFilter::getInvalidInputs() and BaseInputFilter::getValidInputs() is filtered before it is returned. This would result in a bugfix release with minimal impact on version numbers and people who follow interfaces.

Since we are using PHP and PHP is just growing into following interfaces properly, there would probably be a lot of backlash from just filtering out the InputFilterInterfaces from the above mentioned results.

If the interface was changed, there would need to be a new major release. This would be the cleanest way to deal with this problem.

[func0der]

Might I suggest, that phpstan is brought into this project also? It should have caught that mis-implementation at least in BaseInputFilter :)

[froschdesign]

The result that I would wish for is, that the InputFilterInterface would type hint InputFilterInterface and InputInterface.

The problem was fixed with version 2.16.0:

https://github.com/laminas/laminas-inputfilter/blob/da425f72808aea54322b2fe5a23914f1763218fb/src/InputFilterInterface.php#L65-L71 https://github.com/laminas/laminas-inputfilter/blob/da425f72808aea54322b2fe5a23914f1763218fb/src/InputFilterInterface.php#L120-L128 https://github.com/laminas/laminas-inputfilter/blob/da425f72808aea54322b2fe5a23914f1763218fb/src/InputFilterInterface.php#L130-L138

And more:

https://github.com/laminas/laminas-inputfilter/blob/da425f72808aea54322b2fe5a23914f1763218fb/src/InputFilterInterface.php#L14-L49

[func0der]

closed this as not planned

Was "not planned" a miss click? :thinking: As this bug is fixed, isn't it?

[Ocramius]

That's just what the "close" button does on github nowadays 🤷

[froschdesign]

@func0der Better now? 😉

[func0der]

That's just what the "close" button does on github nowadays shrug

i need a facepalm reaction emoji :facepalm: :D