Skip to content

Switched to non-null defaults in exception constructors #40271

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

Merged
merged 1 commit into from
Feb 22, 2021
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public function load($resource, $type = null)
// - this handles the case and prevents the second fatal error
// by triggering an exception beforehand.

throw new LoaderLoadException($resource, null, null, null, $type);
throw new LoaderLoadException($resource, null, 0, null, $type);
}
$this->loading = true;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/
class FileLoaderImportCircularReferenceException extends LoaderLoadException
{
public function __construct(array $resources, int $code = null, \Throwable $previous = null)
public function __construct(array $resources, ?int $code = 0, \Throwable $previous = null)
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't we pass $code ?? 0 to the parent?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a decision we have to make. PHP 8.1 deprecates passing null as $code. Do we want to hide that deprecation from the caller? Then we should map null to zero. Or do we want to pass that deprecation down to the caller instead?

Note that all of our exception classes that don't override the constructor will trigger the new deprecation on null arguments, so not mapping null arguments would be more or less consistent behavior.

Copy link
Member

Choose a reason for hiding this comment

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

ok, I see, let's keep as is then, we can figure out later anyway

Copy link
Contributor

Choose a reason for hiding this comment

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

@derrabus we hit this in 4.4 now. I fix it on our side which has to be done anyway, but yet seems unhandled and undecided in this codebase.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, you have to do it anyway. See my explanation above.

{
$message = sprintf('Circular reference detected in "%s" ("%s" > "%s").', $this->varToString($resources[0]), implode('" > "', $resources), $resources[0]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@
class FileLoaderLoadException extends \Exception
{
/**
* @param string $resource The resource that could not be imported
* @param string $sourceResource The original resource importing the new resource
* @param int $code The error code
* @param \Throwable $previous A previous exception
* @param string $type The type of resource
* @param string $resource The resource that could not be imported
* @param string|null $sourceResource The original resource importing the new resource
* @param int|null $code The error code
* @param \Throwable|null $previous A previous exception
* @param string|null $type The type of resource
*/
public function __construct(string $resource, string $sourceResource = null, int $code = null, \Throwable $previous = null, string $type = null)
public function __construct(string $resource, string $sourceResource = null, ?int $code = 0, \Throwable $previous = null, string $type = null)
{
$message = '';
if ($previous) {
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Config/Loader/DelegatingLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function __construct(LoaderResolverInterface $resolver)
public function load($resource, $type = null)
{
if (false === $loader = $this->resolver->resolve($resource, $type)) {
throw new LoaderLoadException($resource, null, null, null, $type);
throw new LoaderLoadException($resource, null, 0, null, $type);
}

return $loader->load($resource, $type);
Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Config/Loader/FileLoader.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ private function doImport($resource, string $type = null, bool $ignoreErrors = f
throw $e;
}

throw new LoaderLoadException($resource, $sourceResource, null, $e, $type);
throw new LoaderLoadException($resource, $sourceResource, 0, $e, $type);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Symfony/Component/Config/Loader/Loader.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public function resolve($resource, $type = null)
$loader = null === $this->resolver ? false : $this->resolver->resolve($resource, $type);

if (false === $loader) {
throw new LoaderLoadException($resource, null, null, null, $type);
throw new LoaderLoadException($resource, null, 0, null, $type);
}

return $loader;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ public function testMessageCannotLoadResource()

public function testMessageCannotLoadResourceWithType()
{
$exception = new LoaderLoadException('resource', null, null, null, 'foobar');
$exception = new LoaderLoadException('resource', null, 0, null, 'foobar');
$this->assertEquals('Cannot load resource "resource". Make sure there is a loader supporting the "foobar" type.', $exception->getMessage());
}

public function testMessageCannotLoadResourceWithAnnotationType()
{
$exception = new LoaderLoadException('resource', null, null, null, 'annotation');
$exception = new LoaderLoadException('resource', null, 0, null, 'annotation');
$this->assertEquals('Cannot load resource "resource". Make sure annotations are installed and enabled.', $exception->getMessage());
}

Expand All @@ -56,7 +56,7 @@ public function testMessageHasPreviousErrorWithDotAndUnableToLoad()
$exception = new LoaderLoadException(
'resource',
null,
null,
0,
new \Exception('There was a previous error with an ending dot.')
);
$this->assertEquals(
Expand All @@ -70,7 +70,7 @@ public function testMessageHasPreviousErrorWithoutDotAndUnableToLoad()
$exception = new LoaderLoadException(
'resource',
null,
null,
0,
new \Exception('There was a previous error with no ending dot')
);
$this->assertEquals(
Expand All @@ -84,7 +84,7 @@ public function testMessageHasPreviousErrorAndUnableToLoadBundle()
$exception = new LoaderLoadException(
'@resource',
null,
null,
0,
new \Exception('There was a previous error with an ending dot.')
);
$this->assertEquals(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ class CommandNotFoundException extends \InvalidArgumentException implements Exce
private $alternatives;

/**
* @param string $message Exception message to throw
* @param array $alternatives List of similar defined names
* @param int $code Exception code
* @param \Throwable $previous Previous exception used for the exception chaining
* @param string $message Exception message to throw
* @param string[] $alternatives List of similar defined names
* @param int $code Exception code
* @param \Throwable|null $previous Previous exception used for the exception chaining
*/
public function __construct(string $message, array $alternatives = [], int $code = 0, \Throwable $previous = null)
{
Expand All @@ -34,7 +34,7 @@ public function __construct(string $message, array $alternatives = [], int $code
}

/**
* @return array A list of similar defined names
* @return string[] A list of similar defined names
*/
public function getAlternatives()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@
class AccessDeniedHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(403, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class BadRequestHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(400, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class ConflictHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(409, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class GoneHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(410, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ class HttpException extends \RuntimeException implements HttpExceptionInterface
private $statusCode;
private $headers;

public function __construct(int $statusCode, string $message = null, \Throwable $previous = null, array $headers = [], ?int $code = 0)
public function __construct(int $statusCode, ?string $message = '', \Throwable $previous = null, array $headers = [], ?int $code = 0)
{
$this->statusCode = $statusCode;
$this->headers = $headers;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class LengthRequiredHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(411, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
class MethodNotAllowedHttpException extends HttpException
{
/**
* @param array $allow An array of allowed methods
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string[] $allow An array of allowed methods
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int|null $code The internal exception code
*/
public function __construct(array $allow, string $message = null, \Throwable $previous = null, ?int $code = 0, array $headers = [])
public function __construct(array $allow, ?string $message = '', \Throwable $previous = null, ?int $code = 0, array $headers = [])
{
$headers['Allow'] = strtoupper(implode(', ', $allow));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class NotAcceptableHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(406, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class NotFoundHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(404, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class PreconditionFailedHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(412, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@
class PreconditionRequiredHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(428, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
class ServiceUnavailableHttpException extends HttpException
{
/**
* @param int|string $retryAfter The number of seconds or HTTP-date after which the request may be retried
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param int|string|null $retryAfter The number of seconds or HTTP-date after which the request may be retried
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int|null $code The internal exception code
*/
public function __construct($retryAfter = null, string $message = null, \Throwable $previous = null, ?int $code = 0, array $headers = [])
public function __construct($retryAfter = null, ?string $message = '', \Throwable $previous = null, ?int $code = 0, array $headers = [])
{
if ($retryAfter) {
$headers['Retry-After'] = $retryAfter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@
class TooManyRequestsHttpException extends HttpException
{
/**
* @param int|string $retryAfter The number of seconds or HTTP-date after which the request may be retried
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param int|string|null $retryAfter The number of seconds or HTTP-date after which the request may be retried
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int|null $code The internal exception code
*/
public function __construct($retryAfter = null, string $message = null, \Throwable $previous = null, ?int $code = 0, array $headers = [])
public function __construct($retryAfter = null, ?string $message = '', \Throwable $previous = null, ?int $code = 0, array $headers = [])
{
if ($retryAfter) {
$headers['Retry-After'] = $retryAfter;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
class UnauthorizedHttpException extends HttpException
{
/**
* @param string $challenge WWW-Authenticate challenge string
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string $challenge WWW-Authenticate challenge string
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int|null $code The internal exception code
*/
public function __construct(string $challenge, string $message = null, \Throwable $previous = null, ?int $code = 0, array $headers = [])
public function __construct(string $challenge, ?string $message = '', \Throwable $previous = null, ?int $code = 0, array $headers = [])
{
$headers['WWW-Authenticate'] = $challenge;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class UnprocessableEntityHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(422, $message, $previous, $headers, $code);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
class UnsupportedMediaTypeHttpException extends HttpException
{
/**
* @param string $message The internal exception message
* @param \Throwable $previous The previous exception
* @param int $code The internal exception code
* @param string|null $message The internal exception message
* @param \Throwable|null $previous The previous exception
* @param int $code The internal exception code
*/
public function __construct(string $message = null, \Throwable $previous = null, int $code = 0, array $headers = [])
public function __construct(?string $message = '', \Throwable $previous = null, int $code = 0, array $headers = [])
{
parent::__construct(415, $message, $previous, $headers, $code);
}
Expand Down
Loading