Skip to content

[10.x] phpdoc: Auth\Access\Response constructor allows null message#48394

Merged
taylorotwell merged 1 commit into
laravel:10.xfrom
snmatsui:fix/auth-response-phpdoc
Sep 14, 2023
Merged

[10.x] phpdoc: Auth\Access\Response constructor allows null message#48394
taylorotwell merged 1 commit into
laravel:10.xfrom
snmatsui:fix/auth-response-phpdoc

Conversation

@snmatsui

@snmatsui snmatsui commented Sep 14, 2023

Copy link
Copy Markdown
Contributor

Problem

Response::allow() and Response::deny() construct Response with a null $message.
However, the Response constructor's phpdoc only allows string $message.
Response::$message is passed to AuthorizationException, so a null $message creates an Exception with the default message This action is unauthorized. However, an empty string $message creates an Exception with an empty string message.

https://github.com/laravel/framework/blob/master/src/Illuminate/Auth/Access/AuthorizationException.php#L34

Solution

I modified Phpdoc to allow a null $message.

Users who get benefits from this

Users of static analysis tools (like PHPStan) would not encounter this error when using directly new Response():

Parameter #2 $message of class Illuminate\Auth\Access\Response constructor expects string, null given.

@snmatsui

Copy link
Copy Markdown
Contributor Author

It looks like it would be desirable to change constructor $message = '' to $message = null as well,
However, considering the size of the impact, it is left unchanged here.

@taylorotwell taylorotwell merged commit 03613ec into laravel:10.x Sep 14, 2023
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

Successfully merging this pull request may close these issues.

2 participants