Fixed compatibility with SoapClient

[yethee]

SoapClient

Some methods can return null but currently these cases are not detected by psalm. As instance, __getLastResponse() returns null when connection was closed before receiving a response.

https://psalm.dev/r/e7364a92c6

If we remove the check for null, then can get an error of incompatibility types, at runtime:

TypeError: preg_match() expects parameter 2 to be string, null given

Affected methods:

• __getFunctions
• __getLastRequest
• __getLastRequestHeaders
• __getLastResponse
• __getLastResponseHeaders
• __getTypes

SoapFault

Value of property detail can be mixed.

<SOAP-ENV:Fault>
  <faultcode>SOAP-ENV:Client</faultcode>
  <faultstring>Bad request</faultstring>
  <detail>
    <ns3:FaultResponse xmlns:ns3="http://direct.yandex.com/api/v5/general">
      <requestId>1010101010101010101</requestId>
      <errorCode>8000</errorCode>
      <errorDetail>Error message</errorDetail>
    </ns3:FaultResponse>
  </detail>
</SOAP-ENV:Fault>

print_r($fault->detail);

stdClass Object
(
    [FaultResponse] => stdClass Object
        (
            [requestId] => 1010101010101010101
            [errorCode] => 8000
            [errorDetail] => Error message
        )
)

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/e7364a92c6

<?php

function getRequestId(SoapClient $service): ?string
{
    $headers = $service->__getLastResponseHeaders();
    
    if ($headers !== null && preg_match('/^RequestId:\s?(.+)$/im', $headers, $m)) {
        return $m[1];
    }
    
    return null;
}

Psalm output (using commit 2a570fb):

ERROR: RedundantConditionGivenDocblockType - 7:9 - Docblock-defined type string can never contain null

INFO: PossiblyUndefinedIntArrayOffset - 8:16 - Possibly undefined array offset '1' is risky given expected type 'array-key'. Consider using isset beforehand.

[orklah]

@veewee You recently submitted changes in soap stubs, do you mind reviewing this PR if you're familiar with the extension?

[veewee]

Changing the details is ok, but the other props also don't seem to match the signature of SoapFault::__construct()

[yethee]

I have updated the constructor signature according to the documentation. And also the property map has been updated.

[veewee]

@orklah : Do the names of these parameters matter (e.g. for named params)?
Since It is actually:

#[Pure]
public function __construct(
        array|string|null $code,
        string $string, 
        ?string $actor = null,
        mixed $details = null,
        ?string $name = null,
        mixed $headerFault = null
    ) {

See:

• https://github.com/php/php-src/blob/1d54097435bc09a287cb50b6e166eccba19f2925/ext/soap/soap.stub.php#L38
• https://github.com/php/php-src/blob/1d54097435bc09a287cb50b6e166eccba19f2925/ext/soap/soap_arginfo.h#L25-L32

(It is more like an internal exception though, but maybe you'dd be creating it for testing purpose)

[weirdan]

Names do matter, for when Psalm checks calls with named arguments.

[yethee]

I've updated the signatures for SoapFault::__construct() and SoapFault::SoapFault().

[veewee]

@orklah Seems correct and documented:
https://www.php.net/manual/en/class.soapclient.php

I can imageine that they have always worked like this.

These can return null in non-wsdl mode or when overriding construct and calling these functions withouth the WSDL being parsed:

• __getTypes()
• __getFunctions()

$soap = new SoapClient(null, [
    'location' => 'http://127.0.0.1',
    'uri' => 'http://127.0.0.1',
]);

var_dump($soap->__getFunctions());
// > null
var_dump($soap->__getTypes());
// > null

These will return null if trace is disabled (or if the connection got a timeout - you can simulate this with the ini setting default_socket_timeout)

• __getLastRequest()
• __getLastRequestHeaders()
• __getLastResponse()
• __getLastResponseHeaders()

$soap = new SoapClient('http://www.dneonline.com/calculator.asmx?wsdl', [
    'trace' => 0,
]);
$result = $soap->Add(['intA' => 1, 'intB' => 2]);

var_dump($soap->__getLastRequest());
// > null
var_dump($soap->__getLastRequestHeaders());
// > null
var_dump($soap->__getLastResponse());
// > null
var_dump($soap->__getLastResponse());
// > null

The SoapFault indeed takes a ?mixed details prop.
It looks like the other properties are not fully in sync though.
See : https://www.php.net/manual/en/soapfault.construct.php

[weirdan]

test-with-real-projects/PHPUnit failure seems legit. You may want to PR a null check to PHPUnit, but for the purposes of our CI suite we will just baseline that error when it's merged.

[orklah]

Thanks!

@weirdan can you update the baseline for phpunit?

[weirdan]

@weirdan can you update the baseline for phpunit?

@orklah done.