Nyholm / psr7

A super lightweight PSR-7 implementation
MIT License
1.16k stars 75 forks source link

argument name of withHeader() is inconsistent with PSR-7 definition. #199

Closed shotanue closed 1 year ago

shotanue commented 2 years ago

Hi,

According to PSR-7 document, it defines the withHeader signature below.

    /**
     * Return an instance with the provided value replacing the specified header.
     *
     * While header names are case-insensitive, the casing of the header will
     * be preserved by this function, and returned from getHeaders().
     *
     * This method MUST be implemented in such a way as to retain the
     * immutability of the message, and MUST return an instance that has the
     * new and/or updated header and value.
     *
     * @param string $name Case-insensitive header field name.
     * @param string|string[] $value Header value(s).
     * @return static
     * @throws \InvalidArgumentException for invalid header names or values.
     */
    public function withHeader($name, $value);

However, the implementation is

public function withHeader($header, $value): self

Therefore, using named arguments like below, the inconsistency of argument names causes runtime or static analysis(e.g. psalm, intelephense, PhpStorm) errors.

$app->get('/foo', function (ServerRequestInterface $request, ResponseInterface $response, $args) {
     // PSR-7 interface
    $response->withHeader(name: 'Location', value: 'http://localhost/');  // runtime error occurs.

    // The implementation signature
    $response->withHeader(header: 'Location', value: 'http://localhost/');  // No runtime error occurs, but static analysis raises an error. 
Nyholm commented 2 years ago

This is interesting. The code was written way before named parameters was a thing. Renaming them now would be a BC break.

This is something we can only change in 2.0. :/

nicolas-grekas commented 1 year ago

PHP doesn't enforce the name on arguments on child implementations. This means they shouldn't be use when coding against interfaces.