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

Fix some missing return/throw types. #801

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/Api/Attachment.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
namespace Mailgun\Api;

use Mailgun\Assert;
use Mailgun\Exception\HttpClientException;
use Mailgun\Exception\HttpServerException;
use Mailgun\Exception\UnknownErrorException;
use Psr\Http\Message\ResponseInterface;

/**
Expand All @@ -20,9 +23,9 @@
class Attachment extends HttpApi
{
/**
* @return ResponseInterface
* @throws HttpClientException|HttpServerException|UnknownErrorException
*/
public function show(string $url)
public function show(string $url): ResponseInterface
{
Assert::stringNotEmpty($url);
Assert::regex($url, '@https://.*mailgun\.(net|org)/v.+@');
Expand Down
4 changes: 1 addition & 3 deletions src/Api/EmailValidationV4.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,8 @@ public function getBulkPreview(string $previewId)

/**
* @param string $previewId ID given when the list created
*
* @return bool
*/
public function deleteBulkPreview(string $previewId)
public function deleteBulkPreview(string $previewId): bool
{
Assert::stringNotEmpty($previewId);

Expand Down
6 changes: 4 additions & 2 deletions src/Api/HttpApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,11 @@ protected function hydrateResponse(ResponseInterface $response, string $class)
/**
* Throw the correct exception for this error.
*
* @throws \Exception
* @throws HttpClientException|HttpServerException|UnknownErrorException
*
* @return never
*/
protected function handleErrors(ResponseInterface $response)
protected function handleErrors(ResponseInterface $response): void
{
$statusCode = $response->getStatusCode();
switch ($statusCode) {
Expand Down
9 changes: 8 additions & 1 deletion src/Assert.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,14 @@
*/
final class Assert extends \Webmozart\Assert\Assert
{
protected static function reportInvalidArgument($message)
/**
* @psalm-pure
*
* @throws InvalidArgumentException
*
* @return never
*/
protected static function reportInvalidArgument($message): void
{
throw new InvalidArgumentException($message);
}
Expand Down
16 changes: 8 additions & 8 deletions src/Exception/HttpClientException.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public function __construct(string $message, int $code, ResponseInterface $respo
}
}

public static function badRequest(ResponseInterface $response)
public static function badRequest(ResponseInterface $response): self
{
$body = $response->getBody()->__toString();
if (0 !== strpos($response->getHeaderLine('Content-Type'), 'application/json')) {
Expand All @@ -63,37 +63,37 @@ public static function badRequest(ResponseInterface $response)
return new self($message, 400, $response);
}

public static function unauthorized(ResponseInterface $response)
public static function unauthorized(ResponseInterface $response): self
{
return new self('Your credentials are incorrect.', 401, $response);
}

public static function requestFailed(ResponseInterface $response)
public static function requestFailed(ResponseInterface $response): self
{
return new self('Parameters were valid but request failed. Try again.', 402, $response);
}

public static function notFound(ResponseInterface $response)
public static function notFound(ResponseInterface $response): self
{
return new self('The endpoint you have tried to access does not exist. Check if the domain matches the domain you have configure on Mailgun.', 404, $response);
}

public static function conflict(ResponseInterface $response)
public static function conflict(ResponseInterface $response): self
{
return new self('Request conflicts with current state of the target resource.', 409, $response);
}

public static function payloadTooLarge(ResponseInterface $response)
public static function payloadTooLarge(ResponseInterface $response): self
{
return new self('Payload too large, your total attachment size is too big.', 413, $response);
}

public static function tooManyRequests(ResponseInterface $response)
public static function tooManyRequests(ResponseInterface $response): self
{
return new self('Too many requests.', 429, $response);
}

public static function forbidden(ResponseInterface $response)
public static function forbidden(ResponseInterface $response): self
{
$body = $response->getBody()->__toString();
if (0 !== strpos($response->getHeaderLine('Content-Type'), 'application/json')) {
Expand Down
6 changes: 3 additions & 3 deletions src/Exception/HttpServerException.php
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,17 @@
*/
final class HttpServerException extends \RuntimeException implements Exception
{
public static function serverError(int $httpStatus = 500)
public static function serverError(int $httpStatus = 500): self
{
return new self('An unexpected error occurred at Mailgun\'s servers. Try again later and contact support if the error still exists.', $httpStatus);
}

public static function networkError(\Throwable $previous)
public static function networkError(\Throwable $previous): self
{
return new self('Mailgun\'s servers are currently unreachable.', 0, $previous);
}

public static function unknownHttpResponseCode(int $code)
public static function unknownHttpResponseCode(int $code): self
{
return new self(sprintf('Unknown HTTP response code ("%d") received from the API server', $code));
}
Expand Down
7 changes: 2 additions & 5 deletions src/HttpClient/Plugin/History.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,12 @@ final class History implements Journal
*/
private $lastResponse;

/**
* @return ResponseInterface|null
*/
public function getLastResponse()
public function getLastResponse(): ?ResponseInterface
{
return $this->lastResponse;
}

public function addSuccess(RequestInterface $request, ResponseInterface $response)
public function addSuccess(RequestInterface $request, ResponseInterface $response): void
{
$this->lastResponse = $response;
}
Expand Down
8 changes: 6 additions & 2 deletions src/HttpClient/RequestBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,12 @@ public function setMultipartStreamBuilder(MultipartStreamBuilder $multipartStrea
return $this;
}

private function createRequest(string $method, string $uri, array $headers, StreamInterface $stream)
{
private function createRequest(
string $method,
string $uri,
array $headers,
StreamInterface $stream
): RequestInterface {
$request = $this->getRequestFactory()->createRequest($method, $uri);
$request = $request->withBody($stream);
foreach ($headers as $name => $value) {
Expand Down
4 changes: 2 additions & 2 deletions src/Hydrator/ArrayHydrator.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ final class ArrayHydrator implements Hydrator
/**
* @param class-string $class
*
* @return array
* @throws HydrationException
*/
public function hydrate(ResponseInterface $response, string $class)
public function hydrate(ResponseInterface $response, string $class): array
{
$body = $response->getBody()->__toString();
if (0 !== strpos($response->getHeaderLine('Content-Type'), 'application/json')) {
Expand Down
2 changes: 2 additions & 0 deletions src/Hydrator/ModelHydrator.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ final class ModelHydrator implements Hydrator
/**
* @param class-string $class
*
* @throws HydrationException
*
* @return ResponseInterface
*/
public function hydrate(ResponseInterface $response, string $class)
Expand Down
2 changes: 2 additions & 0 deletions src/Hydrator/NoopHydrator.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ final class NoopHydrator implements Hydrator
* @param class-string $class
*
* @throws \LogicException
*
* @return never
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems unnecessary to me

Copy link
Contributor Author

@ZebulanStanphill ZebulanStanphill Oct 20, 2021

Choose a reason for hiding this comment

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

This helps Psalm and PHPStan recognize that not only does this function throw, but it specifically always throws. So if you put any code after a call to this function, the static analyzers will know you've made a mistake, because the code would never be executed. PHP 8.1 is actually adding a native never return type for this reason.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I understand, but this SDK is based on PHP 7.3 or greater, which means we should not use code that has been specifically added (or will be added) in future versions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, since it's just a comment annotation (rather than a native return type) it has no effect on the code's PHP 7.3 compatibility. But Psalm and PHPStan already support this annotation, so it will provide static analysis to everyone using those tools regardless of which PHP version they're using.

*/
public function hydrate(ResponseInterface $response, string $class)
{
Expand Down
2 changes: 1 addition & 1 deletion src/Message/Exceptions/MissingRequiredParameter.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

class MissingRequiredParameter extends \Exception implements Exception
{
public static function create(string $parameter, string $message = null)
public static function create(string $parameter, string $message = null): self
{
if (null === $message) {
$message = 'The parameters passed to the API were invalid. Please specify "%s".';
Expand Down
5 changes: 1 addition & 4 deletions src/Model/EmailValidation/Parts.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,7 @@ private function __construct()
{
}

/**
* @return Parts
*/
public static function create(array $data)
public static function create(array $data): self
{
$model = new self();
$model->displayName = $data['display_name'] ?? null;
Expand Down
2 changes: 1 addition & 1 deletion src/Model/Event/EventResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ private function __construct()
{
}

public static function create(array $data)
public static function create(array $data): self
{
$events = [];
if (isset($data['items'])) {
Expand Down
2 changes: 1 addition & 1 deletion src/Model/Ip/IndexResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ private function __construct()
{
}

public static function create(array $data)
public static function create(array $data): self
{
$model = new self();
$model->items = $data['items'];
Expand Down
2 changes: 1 addition & 1 deletion src/Model/Ip/ShowResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ private function __construct()
{
}

public static function create(array $data)
public static function create(array $data): self
{
$model = new self();
$model->ip = $data['ip'];
Expand Down
2 changes: 1 addition & 1 deletion src/Model/Ip/UpdateResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ private function __construct()
{
}

public static function create(array $data)
public static function create(array $data): self
{
$model = new self();
$model->message = $data['message'];
Expand Down
5 changes: 1 addition & 4 deletions src/Model/MailingList/PagesResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,7 @@ final class PagesResponse implements ApiResponse, PagingProvider

private $items;

/**
* @return self
*/
public static function create(array $data)
public static function create(array $data): self
{
$items = [];

Expand Down
4 changes: 2 additions & 2 deletions src/Model/Route/Action.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ final class Action
/**
* Action Named Constructor to build several Action DTOs provided by an Array.
*
* @return Action[]
* @return self[]
*/
public static function createMultiple(array $data)
public static function createMultiple(array $data): array
{
$items = [];

Expand Down