ToNull filter created via factory with no options has different state in new ZF release

[roelvanduijnhoven]

When you initialize this filter in the following way using the form builder it is incorrectly constructed with an unknown value of 0.

'filters' => [['name' => 'Zend\Filter\ToNull']],

This is similar to what #44 tries to fix and thus related to https://github.com/zendframework/zend-servicemanager/pull/164.

I would suggest to fix this at the least in the ToNull class. It should be impossible to construct this filter using an incorrect type. I could provide a PR if we are :+1: on this.

The fix is to drop https://github.com/zendframework/zend-filter/blob/master/src/ToNull.php#L60 this line. It makes to sense to set an array as type.

[roelvanduijnhoven]

For those suffering from this the fix is obviously:

['name' => 'Zend\Filter\ToNull', 'options' => ['type' => ToNull::TYPE_ALL]]

[adamlundrigan]

The purpose of ToNull.php#L60 is to allow passing the empty types which should be nullable as a short-hand notation:

$v = new ToNull([ToNull::TYPE_BOOLEAN, ToNull::TYPE_INTEGER])

// ...or the equivalent when using the form builder:
// ['name' => 'Zend\Filter\ToNull', 'options' => [ToNull::TYPE_BOOLEAN, ToNull::TYPE_INTEGER]]

Since this is established behaviour dropping that Line 60 would be a backwards-compatibility break. I'd instead argue this is a documentation issue: ToNull makes no attempt to infer a base set of types to null when you don't pass a type, so passing a type option is not optional.

[roelvanduijnhoven]

I see. I wasn't aware of this. It thus does makes sense that new ToNull([]) resolves to a type of 0. Namely, a filter that does not filter anything.

However if https://github.com/zendframework/zend-servicemanager/pull/164 is not reverted, this still results in a BC break. I updated the issue subject to reflect this. So only updating documentation won't do the trick.

Also it results in unwanted behaviour. If you take this:

['name' => 'Zend\Filter\ToNull']

That should not yield new Zend\Filter\ToNull([]) or new Zend\Filter\ToNull(0) because that represents a filter that does precisely nothing!

I see two ways going forward:

• Make sure the form builder does not pass an empty array when the type has not been set. This makes most sense to me!
• Constructor of ToNull should not proxy an array to the type parameter. Instead one should explicit set the type as new ToNull(['type' => []]). This is a BC break for this component.

[sjuvonen]

It makes no sense to allow the filter have such a default option that makes it passthrough all given input. If one specifies no type option explicitly, the filter will do nothing at all. At least it should then throw an exception or something. Since the $constants variable already contains all the known PHP types, an automatic type resolving would also be possible. (Although gettype(0.0) == 'double' and not 'float' as in ToNull options.)

[roelvanduijnhoven]

Who has some authority to make decisions here? Would be willing to contribute towards a solution.

[adamlundrigan]

@roelvanduijnhoven

Make sure the form builder does not pass an empty array when the type has not been set. This makes most sense to me!

I'm not familiar enough with that part of this component to pass judgment on how to do that or whether it's a good idea.

Constructor of ToNull should not proxy an array to the type parameter. Instead one should explicit set the type as new ToNull(['type' => []]). This is a BC break for this component.

The type parameter can be an array. If it's an empty array the internal type field gets set to 0, which results in no filtering being applied. It's a completely valid option, but not a particularly useful one.

@sjuvonen

It makes no sense to allow the filter have such a default option that makes it passthrough all given input. If one specifies no type option explicitly, the filter will do nothing at all.

It's not intended to work this way. The documentation says ToNull should essentially work like this when no types are specified:

empty($value) ? null : $value

(doc reference)

If I'm understanding this ticket correctly the issue is that default is only applied if you don't specify a type parameter. If you put an empty array in the type parameter it behaves as if nothing is nullable, because that's what you asked it to do by specifying no nullable types. To get the default behaviour the type key must be null or not defined.

So, I was wrong in my initial assertion that it's a documentation issue, but a note in there stating that passing an empty array as the type parameter means the filter does nothing might be a good addition.

At least it should then throw an exception or something

It should throw an exception if it's improperly configured, but that's a BC break. It could be considered for the next major version of this component. The question is whether a useless (no-op) setting is the same thing as improperly configured (ie: broken).

Since the $constants variable already contains all the known PHP types, an automatic type resolving would also be possible. (Although gettype(0.0) == 'double' and not 'float' as in ToNull options.)

Possible reason for not having automatic type resolving is to force you to choose which types should be nulled. I don't know the exact reasoning behind that choice or if it was even a conscious choice, but it fits with the general theme I saw in ZF2 development: a sort of rebellion against "magic", which ZF1 overused.

---

Reflecting on this, I don't think there's anything to fix here. It's an upstream problem that traces back to https://github.com/zendframework/zend-servicemanager/pull/127 and https://github.com/zendframework/zend-servicemanager/pull/164 will fix if it's accepted, as @roelvanduijnhoven pointed out originally.

[roelvanduijnhoven]

@adamlundrigan Allright thanks. Activitely on that particular PR seems low. Should we bump it?

[amcsi]

This can be closed because of zendframework/zend-servicemanager#164

It's available in the Service Manager release of 2.7.8 https://github.com/zendframework/zend-servicemanager/releases/tag/release-2.7.8

[roelvanduijnhoven]

Nais!