Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document GuzzleHttp\Client::__construct() parameters at type level? #3194

Open
Ocramius opened this issue Dec 14, 2023 · 3 comments
Open

Document GuzzleHttp\Client::__construct() parameters at type level? #3194

Ocramius opened this issue Dec 14, 2023 · 3 comments

Comments

@Ocramius
Copy link

Description

I just happened to stumble upon

guzzle/src/Client.php

Lines 26 to 57 in 41042bc

/**
* Clients accept an array of constructor parameters.
*
* Here's an example of creating a client using a base_uri and an array of
* default request options to apply to each request:
*
* $client = new Client([
* 'base_uri' => 'http://www.foo.com/1.0/',
* 'timeout' => 0,
* 'allow_redirects' => false,
* 'proxy' => '192.168.16.1:10'
* ]);
*
* Client configuration settings include the following options:
*
* - handler: (callable) Function that transfers HTTP requests over the
* wire. The function is called with a Psr7\Http\Message\RequestInterface
* and array of transfer options, and must return a
* GuzzleHttp\Promise\PromiseInterface that is fulfilled with a
* Psr7\Http\Message\ResponseInterface on success.
* If no handler is provided, a default handler will be created
* that enables all of the request options below by attaching all of the
* default middleware to the handler.
* - base_uri: (string|UriInterface) Base URI of the client that is merged
* into relative URIs. Can be a string or instance of UriInterface.
* - **: any request option
*
* @param array $config Client configuration settings.
*
* @see \GuzzleHttp\RequestOptions for a list of available request options.
*/
public function __construct(array $config = [])
while configuring the Client.

One thing I noticed here is that the docblock mentions:

@see \GuzzleHttp\RequestOptions for a list of available request options.

I'm wondering if we could add type-level documentation for this constructor for PHPStan, Psalm and IntelliJ, such as:

/**
 * @psalm-import OptionsArray from \GuzzleHttp\RequestOptions
 */
class Client // snip
{
// <snip>
    /**
     * <snip>
     *
     * @psalm-param OptionsArray $config
     */
    public function __construct(array $config = [])

Then, in RequestOptions:

/**
 * <snip>
 *
 * @psalm-type OptionsArray = array{
 *    timeout?: int<0, max>,
 *    more?: stuff-here,
 *    ...
 * }
 */
final class RequestOptions
{
// <snip>

Note how the 3 dots at the end are necessary to allow options to expand over time.

This kind of addition would be a massive ergonomic improvement, IMO.

Similar to getsentry/sentry-php#1656

@Ocramius
Copy link
Author

Also noticed: defaults are not really 100% clear, so I had a project where the timeout was 0, and production just got stuck forever somewhere :D

Not sure if we can document that at type level, somehow.

@GrahamCampbell
Copy link
Member

Yeh, I agree we should do this. I'm looking to add better generic and shape typing in Guzzle 8. I am a little hesitant to make too many changes to Guzzle 7, as the blast radius can be large for even changes that we could argue are not really breaking, but can change the outputs of static analyzers.

@Ocramius
Copy link
Author

Eh, that's kinda normal, TBH: there's a ton of SA shifts at every single tool upgrade regardless

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants