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

Adds parameter typehints #94

Merged
merged 3 commits into from Apr 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions composer.json
Expand Up @@ -11,7 +11,7 @@
}
],
"require": {
"php": ">=5.3.0"
"php": "^7.2 || ^8.0"
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd vote for going straight to 8.x and be done with it. Use the union types.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As noted on #95, I think it makes sense to allow v7 here, due to the large install base of RHEL 7, Ubuntu 20.04, and Debian 11, all of which pin to PHP 7.4. We will have PHP 7 users for some years, even if it's not technically in support, and there's no reason to restrict its usage artificially.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't there some places where you wanted to use mixed, though? Why punt on that unnecessarily?

Most of the other PSR updates have targeted 8.x (they used mixed or union types), and in practice... how many people use the pinned old versions of PHP on distros rather than PPCs? I admittedly only know from one corporate environment (my own), but we're ignoring that. I don't get the sense that distro versions matters anywhere near as much as it used to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Aren't there some places where you wanted to use mixed, though? Why punt on that unnecessarily?

We can do a 2.1 release that restricts to 8.0 and up and adds mixed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get the sense that distro versions matters anywhere near as much as it used to.

They absolutely do.

We (Zend) just released our annual PHP Landscape Report. It is very representative of the broader PHP community, has a representative mix of developers to decision makers, and covers a broad spectrum of company sizes.

This year, as in all years past that I can remember, 60% of respondents are currently using EOL versions of PHP.

There's no reason to artificially restrict what features or security fixes they can get in the PHP libraries they consume because they are on an EOL version of PHP. They may be getting security patches for the runtime already (e.g., via an operating system, or via a commercial LTS provider), so limiting what they can use makes it harder to keep the libraries they use updated.

},
"autoload": {
"psr-4": {
Expand All @@ -20,7 +20,7 @@
},
"extra": {
"branch-alias": {
"dev-master": "1.0.x-dev"
"dev-master": "1.1.x-dev"
}
}
}
16 changes: 9 additions & 7 deletions src/MessageInterface.php
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Psr\Http\Message;

/**
Expand Down Expand Up @@ -38,7 +40,7 @@ public function getProtocolVersion();
* @param string $version HTTP protocol version
* @return static
*/
public function withProtocolVersion($version);
public function withProtocolVersion(string $version);

/**
* Retrieves all message header values.
Expand Down Expand Up @@ -75,7 +77,7 @@ public function getHeaders();
* name using a case-insensitive string comparison. Returns false if
* no matching header name is found in the message.
*/
public function hasHeader($name);
public function hasHeader(string $name);

/**
* Retrieves a message header value by the given case-insensitive name.
Expand All @@ -91,7 +93,7 @@ public function hasHeader($name);
* header. If the header does not appear in the message, this method MUST
* return an empty array.
*/
public function getHeader($name);
public function getHeader(string $name);

/**
* Retrieves a comma-separated string of the values for a single header.
Expand All @@ -112,7 +114,7 @@ public function getHeader($name);
* concatenated together using a comma. If the header does not appear in
* the message, this method MUST return an empty string.
*/
public function getHeaderLine($name);
public function getHeaderLine(string $name);

/**
* Return an instance with the provided value replacing the specified header.
Expand All @@ -129,7 +131,7 @@ public function getHeaderLine($name);
* @return static
* @throws \InvalidArgumentException for invalid header names or values.
*/
public function withHeader($name, $value);
public function withHeader(string $name, $value);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is interesting; when we bump to PHP 8, we could make $value into string|array, though an array type would still require validation.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So should we go ahead and do that before the vote? It seems wise to do so if that's what the spec already calls for.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - we can do it for a later minor version that bumps the minimum version to 8. For this one, we should keep PHP 7 support (per my other comments).


/**
* Return an instance with the specified header appended with the given value.
Expand All @@ -147,7 +149,7 @@ public function withHeader($name, $value);
* @return static
* @throws \InvalidArgumentException for invalid header names or values.
*/
public function withAddedHeader($name, $value);
public function withAddedHeader(string $name, $value);

/**
* Return an instance without the specified header.
Expand All @@ -161,7 +163,7 @@ public function withAddedHeader($name, $value);
* @param string $name Case-insensitive header field name to remove.
* @return static
*/
public function withoutHeader($name);
public function withoutHeader(string $name);

/**
* Gets the body of the message.
Expand Down
10 changes: 6 additions & 4 deletions src/RequestInterface.php
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Psr\Http\Message;

/**
Expand Down Expand Up @@ -55,10 +57,10 @@ public function getRequestTarget();
*
* @link http://tools.ietf.org/html/rfc7230#section-5.3 (for the various
* request-target forms allowed in request messages)
* @param mixed $requestTarget
* @param string $requestTarget
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this an error before?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes - if you look at the linked RFC 7230 section, you'll see it can only be a string.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refs #78

* @return static
*/
public function withRequestTarget($requestTarget);
public function withRequestTarget(string $requestTarget);

/**
* Retrieves the HTTP method of the request.
Expand All @@ -82,7 +84,7 @@ public function getMethod();
* @return static
* @throws \InvalidArgumentException for invalid HTTP methods.
*/
public function withMethod($method);
public function withMethod(string $method);

/**
* Retrieves the URI instance.
Expand Down Expand Up @@ -125,5 +127,5 @@ public function getUri();
* @param bool $preserveHost Preserve the original state of the Host header.
* @return static
*/
public function withUri(UriInterface $uri, $preserveHost = false);
public function withUri(UriInterface $uri, bool $preserveHost = false);
}
4 changes: 3 additions & 1 deletion src/ResponseInterface.php
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Psr\Http\Message;

/**
Expand Down Expand Up @@ -49,7 +51,7 @@ public function getStatusCode();
* @return static
* @throws \InvalidArgumentException For invalid status code arguments.
*/
public function withStatus($code, $reasonPhrase = '');
public function withStatus(int $code, string $reasonPhrase = '');

/**
* Gets the response reason phrase associated with the status code.
Expand Down
8 changes: 5 additions & 3 deletions src/ServerRequestInterface.php
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Psr\Http\Message;

/**
Expand Down Expand Up @@ -224,7 +226,7 @@ public function getAttributes();
* @param mixed $default Default value to return if the attribute does not exist.
* @return mixed
*/
public function getAttribute($name, $default = null);
public function getAttribute(string $name, $default = null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we move to PHP 8, we could add mixed as the type for $default.


/**
* Return an instance with the specified derived request attribute.
Expand All @@ -241,7 +243,7 @@ public function getAttribute($name, $default = null);
* @param mixed $value The value of the attribute.
* @return static
*/
public function withAttribute($name, $value);
public function withAttribute(string $name, $value);

/**
* Return an instance that removes the specified derived request attribute.
Expand All @@ -257,5 +259,5 @@ public function withAttribute($name, $value);
* @param string $name The attribute name.
* @return static
*/
public function withoutAttribute($name);
public function withoutAttribute(string $name);
}
12 changes: 7 additions & 5 deletions src/StreamInterface.php
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Psr\Http\Message;

/**
Expand Down Expand Up @@ -84,7 +86,7 @@ public function isSeekable();
* SEEK_END: Set position to end-of-stream plus offset.
* @throws \RuntimeException on failure.
*/
public function seek($offset, $whence = SEEK_SET);
public function seek(int $offset, int $whence = SEEK_SET);

/**
* Seek to the beginning of the stream.
Expand Down Expand Up @@ -112,7 +114,7 @@ public function isWritable();
* @return int Returns the number of bytes written to the stream.
* @throws \RuntimeException on failure.
*/
public function write($string);
public function write(string $string);

/**
* Returns whether or not the stream is readable.
Expand All @@ -131,7 +133,7 @@ public function isReadable();
* if no bytes are available.
* @throws \RuntimeException if an error occurs.
*/
public function read($length);
public function read(int $length);

/**
* Returns the remaining contents in a string
Expand All @@ -149,10 +151,10 @@ public function getContents();
* stream_get_meta_data() function.
*
* @link http://php.net/manual/en/function.stream-get-meta-data.php
* @param string $key Specific metadata to retrieve.
* @param string|null $key Specific metadata to retrieve.
* @return array|mixed|null Returns an associative array if no key is
* provided. Returns a specific key value if a key is provided and the
* value is found, or null if the key is not found.
*/
public function getMetadata($key = null);
public function getMetadata(?string $key = null);
}
4 changes: 3 additions & 1 deletion src/UploadedFileInterface.php
@@ -1,5 +1,7 @@
<?php

declare(strict_types=1);

namespace Psr\Http\Message;

/**
Expand Down Expand Up @@ -62,7 +64,7 @@ public function getStream();
* @throws \RuntimeException on any error during the move operation, or on
* the second or subsequent call to the method.
*/
public function moveTo($targetPath);
public function moveTo(string $targetPath);

/**
* Retrieve the file size.
Expand Down
17 changes: 10 additions & 7 deletions src/UriInterface.php
@@ -1,4 +1,7 @@
<?php

declare(strict_types=1);

namespace Psr\Http\Message;

/**
Expand Down Expand Up @@ -188,7 +191,7 @@ public function getFragment();
* @return static A new instance with the specified scheme.
* @throws \InvalidArgumentException for invalid or unsupported schemes.
*/
public function withScheme($scheme);
public function withScheme(string $scheme);

/**
* Return an instance with the specified user information.
Expand All @@ -204,7 +207,7 @@ public function withScheme($scheme);
* @param null|string $password The password associated with $user.
* @return static A new instance with the specified user information.
*/
public function withUserInfo($user, $password = null);
public function withUserInfo(string $user, ?string $password = null);

/**
* Return an instance with the specified host.
Expand All @@ -218,7 +221,7 @@ public function withUserInfo($user, $password = null);
* @return static A new instance with the specified host.
* @throws \InvalidArgumentException for invalid hostnames.
*/
public function withHost($host);
public function withHost(string $host);

/**
* Return an instance with the specified port.
Expand All @@ -237,7 +240,7 @@ public function withHost($host);
* @return static A new instance with the specified port.
* @throws \InvalidArgumentException for invalid ports.
*/
public function withPort($port);
public function withPort(?int $port);

/**
* Return an instance with the specified path.
Expand All @@ -261,7 +264,7 @@ public function withPort($port);
* @return static A new instance with the specified path.
* @throws \InvalidArgumentException for invalid paths.
*/
public function withPath($path);
public function withPath(string $path);

/**
* Return an instance with the specified query string.
Expand All @@ -278,7 +281,7 @@ public function withPath($path);
* @return static A new instance with the specified query string.
* @throws \InvalidArgumentException for invalid query strings.
*/
public function withQuery($query);
public function withQuery(string $query);

/**
* Return an instance with the specified URI fragment.
Expand All @@ -294,7 +297,7 @@ public function withQuery($query);
* @param string $fragment The fragment to use with the new instance.
* @return static A new instance with the specified fragment.
*/
public function withFragment($fragment);
public function withFragment(string $fragment);

/**
* Return the string representation as a URI reference.
Expand Down