Incorrect array bounds in docs

[lstrojny]

Bug Report

Q	A
BC Break	?
Version	0.10.0

Summary

The array key boundaries generated in XsdTypeFormatter are incorrect. They attempt to define the length of the array but instead they define the shape of the keys.

Current behaviour

Given this element:

<xs:element name="SomeElement" type="xs:string" maxOccurs="20">

The resulting type is this:

array<int<1,20>,string>

This would reject a 0 indexed array like this (because it only allows keys from 1 to 20):

['foo']

Such an array would be totally fine though.

Expected behaviour

Instead of limiting the array keys I think it would be good enough to differentiate list and non-empty-list.

$isList ? $min > 0 ? 'non-empty-list' : 'list' : '',

See

wsdl-reader/src/Formatter/XsdTypeFormatter.php

Line 21 in 32d32e0

$isList ? 'array<int<'.($min === -1 ? 'min' : $min).', '.($max === -1 ? 'max' : $max).'>, ' : '',

[lstrojny]

I misattributed the issue, yes, the above code is also incorrect but the code generation problem comes from https://github.com/phpro/soap-client/blob/05bded4ca8b46c463de1337f256bb9763532e0b5/src/Phpro/SoapClient/CodeGenerator/TypeEnhancer/MetaTypeEnhancer.php#L33

[lstrojny]

Want me to open a separate issue?

[veewee]

The problem indeed is not located in this repository.
Since the formatter here is some kind of meta-styling, I don't think it makes sense to change in here.
It just displays the min - max occurences in some kind of recognisable way.

However, you are right : Static analysis complains in the ArrayBoundsCalculator inside the code generator MetaTypeEnhancer and should be fixed there.

This format works for psalm:

(given minOccurs=1 and maxOccurs=2)

/**
 * @param array{0: string, 1 ?: string} $items
 */
function x(array $items): void {}

x([]);
x(['1']);
x(['1', '2']);
x(['1', '2', '3']);

https://psalm.dev/r/cb9ee08186

However phpstan does not seem to understand the max bound:

https://phpstan.org/r/d38f0bd5-2a03-4798-96a5-8c5a5f42b501

Not sure what the best way forward would be:

• We could generate the array shape based on min and max occurs, but it could become quite complex/large shapes if the max is set to for example "1000"
  • for unbounded maxOccurs, we could use for example: array{0: string, ...string}
  • For items between minOccurs and maxOccurs, we can make an entry optional as in the example 1 ?: string
  • when min=0 and max is unbound, it could be a regular list or use the same logic as now.
• Limit ourselfs to list and non-empty-list (which is not accurately typed)

How do you see this?

[lstrojny]

1. Defining shape (array{0: Type}):
   • Will be huge given a big enough max occurs
   • PHPStan does not understand array{0: string, ...string}
2. Use list for min occurs 0 and non-empty-list for min occurs > 0
   • Too strict (will e.g. not accept non-contiguous arrays)
3. Use array<int, …>for min occurs 0 and non-empty-array<int, …> for min occurs > 0
   • Will be correct but not validate bounds

An option could be to do shapes (option 1) for small enough max occurs (e.g. < 100) and fall back to 3

[lstrojny]

I take it back, the only correct way is option 3 because also shapes don’t allow for non-contiguous arrays.

[veewee]

The result is actually always a list: it starts at zero and does not skip indexes

[lstrojny]

Yeah, you are right, non-contiguous arrays do not work anyway. So, list and non-empty-list it 😄

[veewee]

Option 1 would still be the better option for smaller shapes though. Phpstan doesn't check the upper bound, but it would still be mor viable regarding the lower bounds.

So I'm thinking about having option 1 and maybe limit it to a specific amount of items - which then would fallback to option 3.

One other possibility is to not use the list, but only use the maxOccurs to set an upper bound like this:

array<int<0, maxOccurs>, TV>
non-empty-array<int<0, maxOccurs>, TV>

For completeness : There is also the IteratorAssembler. That one could never use option 1, but could use option 3 or the newly introduced option.

I'm kinda thinking the last option I proposed would be the most exact and easiest option to implement at this point which can be reused for the iterator as well.

What's your preference?

[veewee]

Provided a fix in phpro/soap-client#518

Can you take a look to see if this would work for you?