Optional parameters before required ones in constructor

[mfickers]

Describe the bug
After upgrading our project from PHP 7.4 to PHP 8.1 this deprecation warning appeared:
Deprecated Functionality: Optional parameter $baz declared before required parameter $quux is implicitly treated as a required parameter.

To Reproduce
This type:

<xs:complexType
    name="Foo">
    <xs:sequence>
        <xs:element name="baz" maxOccurs="1" minOccurs="0" nillable="true"
                    type="xs:string"/>
        <xs:element name="qux" maxOccurs="1" minOccurs="1" nillable="false"
                    type="xs:string"/>
        <xs:element name="quux" maxOccurs="1" minOccurs="1" nillable="true"
                    type="xs:date"/>
        <xs:element name="corge" maxOccurs="1" minOccurs="1" nillable="false"
                    type="xs:string"/>
        <xs:element name="grault" maxOccurs="unbounded" minOccurs="1" nillable="true"
                    type="tns:Bar"/>
    </xs:sequence>
</xs:complexType>

will result in this generated code:

public function __construct(string $qux, string $corge, ?string $baz = null, ?string $quux, ?array $grault)
{
    $this
        ->setQux($qux)
        ->setCorge($corge)
        ->setBaz($baz)
        ->setQuux($quux)
        ->setGrault($grault);
}

It seems that nillable parameters get sorted after non-nillable parameters, but other than that, the order of the XML is kept. Parameters with default values are not ordered last and thus can not be omitted when creating an object of this class.

Expected behavior
Constructor parameters of generated classes should appear in this order:

• Required parameters that are not nillable
• Required parameters that are nillable
• Optional parameters

This is the expected generated code for the given example:

public function __construct(string $qux, string $corge, ?string $quux, ?array $grault, ?string $baz = null)
{
    $this
        ->setQux($qux)
        ->setCorge($corge)
        ->setQuux($quux)
        ->setGrault($grault)
        ->setBaz($baz);
}

Additional context
(https://www.php.net/manual/en/migration80.deprecated.php)

[mikaelcom]

Hi @mfickers, I'm trying to improve the way the project is managed.

I created a team for reviewers "only" members so if you accept the invitation, you should be able to review and validate, if it's ok for you, the PR #282.

You can test the PR using one of these phars:

https://phar.wsdltophp.com/wsdltophp-feature-issue-280-php7.phar
https://phar.wsdltophp.com/wsdltophp-feature-issue-280-php81.phar
https://phar.wsdltophp.com/wsdltophp-feature-issue-280-php82.phar

You let me know

[mfickers]

First of all, thank you for the quick work on that fix.

I've tested the fix with the PHP 8.1 phar from your comment above, but there was no difference in the parameter order.

It seems like the phar does not contain the changes, though:

$ cat wsdltophp-feature-issue-280-php81.phar | grep "function putRequiredAttributesFirst" -A 10
    protected function putRequiredAttributesFirst(StructAttributeContainer $allAttributes): StructAttributeContainer
    {
        $attributes = new StructAttributeContainer($this->getGenerator());
        $requiredAttributes = new StructAttributeContainer($this->getGenerator());
        $notRequiredAttributes = new StructAttributeContainer($this->getGenerator());

        /** @var StructAttribute $attribute */
        foreach ($allAttributes as $attribute) {
            if ($attribute->isRequired() && !$attribute->isNullable()) {
                $requiredAttributes->add($attribute);
            } else {

[mikaelcom]

It seems like the phar does not contain the changes, though:

You're right, I had modiffied my script that generates the phar, which did not check out the branch 🤦‍♂️

Please, download it again now and let me know,

[lurkker]

This issue remains on version 4.1.8, to address this I reordered these lines in wsdltophp/packagegenerator/src/Model/Struct.php:412
within the function putRequiredAttributesFirst

from:

array_walk($requiredAttributes, [$attributes, 'add']);
array_walk($notRequiredAttributes, [$attributes, 'add']);
array_walk($nullableNotRequiredAttributes, [$attributes, 'add']);

to:

array_walk($requiredAttributes, [$attributes, 'add']);
array_walk($nullableNotRequiredAttributes, [$attributes, 'add']);
array_walk($notRequiredAttributes, [$attributes, 'add']);

There is probably a better way to solve this, but doing this doesn't break functionality and removes the deprecation warnings.

Thank you for the library.