jsonmapper
jsonmapper copied to clipboard
Do not accept null in typed arrays
this behaviour is completely unexpected, and no static analyzer would spot it. null ought to be treated like all other simple types.
Can you add a bit more context and explain the change you are proposing? Currently you are referring to "this behaviour" without explicitly stating which.
What do you do? What do you get? What do you want instead as the consumer of this library?
I thought the unit test changes would adequately explain the problem, but here goes:
In my project, a model property of type string[]
was expected to contain exclusively string
. However, jsonmapper allows the property to contain null
elements, which can lead to crashes in strict code, and is not detectable by any static analyser.
For example:
$mapper = new \JsonMapper;
$mapper->bExceptionOnMissingData = true;
$mapper->bExceptionOnUndefinedProperty = true;
$mapper->bEnforceMapType = false;
$chainData = $mapper->map(["chain" => [null,"hello"]], new JwtChain);
with the following model:
final class JwtChain{
/**
* @var string[]
* @required
*/
public array $chain;
}
should either coerce the NULL
to ""
, or bail out due to incompatible types (string[]
should not accept null
).
I opted for coercion since it's the same behaviour already applied to every scalar type.
You modify testMapTypedSimpleArray()
, which explicitly tested that an incoming array of strings may contain a null value, and will lead to a null value in the generated array of objects.
Changing this behavior will break many existing applications that rely on it.
I do see that this behavior is ill-suited for some use cases, though.
In hindsight the better way would have been to allow null in arrays only when the type said it, e.g. ?ClassName[]
(a syntax that did not exist seven years ago).
I will not modify the default existing behavior, but adding a new switch (e.g. strict null checks in typed arrays) will be fine.
(bStrictNullTypes
was originally meant for such things, but because of BC I won't change that.)
In PHPDoc the more idiomatic way to describe the type would be @var (string|null)[]
or @var (ClassName|null)[]
.
I will not modify the default existing behavior, but adding a new switch (e.g. strict null checks in typed arrays) will be fine.
I could live with that solution, although perhaps some factory method that sets some properties by default would be useful (I currently always use strictProperties and some other things)
Will you provide a patch?
Closing because of no answer in 3 months.