WsdlToPhp / PackageGenerator

Generates a PHP SDK based on a WSDL, simple and powerful, WSDL to PHP
https://providr.io
MIT License
428 stars 73 forks source link

Optional parameters before required ones in constructor #280

Closed mfickers closed 1 year ago

mfickers commented 1 year ago

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:

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 :man_facepalming:

Please, download it again now and let me know,

lurkker commented 1 year ago

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.