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.

Sign up for GitHub

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

[12.x] use type declarations for Auth events #53969

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[12.x] use type declarations for Auth events #53969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions src/Illuminate/Auth/Events/Attempting.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@ class Attempting
* @return void
*/
public function __construct(
public $guard,
#[\SensitiveParameter] public $credentials,
public $remember,
public string $guard,
#[\SensitiveParameter] public array $credentials,
public bool $remember,
) {
}
}
5 changes: 3 additions & 2 deletions src/Illuminate/Auth/Events/Authenticated.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Queue\SerializesModels;

class Authenticated
Expand All @@ -16,8 +17,8 @@ class Authenticated
* @return void
*/
public function __construct(
public $guard,
public $user,
public string $guard,
public Authenticatable $user,
) {
}
}
5 changes: 3 additions & 2 deletions src/Illuminate/Auth/Events/CurrentDeviceLogout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Queue\SerializesModels;

class CurrentDeviceLogout
Expand All @@ -16,8 +17,8 @@ class CurrentDeviceLogout
* @return void
*/
public function __construct(
public $guard,
public $user,
public string $guard,
public Authenticatable $user,
) {
}
}
8 changes: 5 additions & 3 deletions src/Illuminate/Auth/Events/Failed.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\Authenticatable;

class Failed
{
/**
Expand All @@ -13,9 +15,9 @@ class Failed
* @return void
*/
public function __construct(
public $guard,
public $user,
#[\SensitiveParameter] public $credentials,
public string $guard,
public ?Authenticatable $user,
#[\SensitiveParameter] public array $credentials,
) {
}
}
13 changes: 3 additions & 10 deletions src/Illuminate/Auth/Events/Lockout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,14 @@

class Lockout
{
/**
* The throttled request.
*
* @var \Illuminate\Http\Request
*/
public $request;

/**
* Create a new event instance.
*
* @param \Illuminate\Http\Request $request
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we want to add the docblock here that we would otherwise lose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, thanks

* @return void
*/
public function __construct(Request $request)
{
$this->request = $request;
public function __construct(
public Request $request,
) {
}
}
7 changes: 4 additions & 3 deletions src/Illuminate/Auth/Events/Login.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Queue\SerializesModels;

class Login
Expand All @@ -17,9 +18,9 @@ class Login
* @return void
*/
public function __construct(
public $guard,
public $user,
public $remember,
public string $guard,
public Authenticatable $user,
public bool $remember,
) {
}
}
5 changes: 3 additions & 2 deletions src/Illuminate/Auth/Events/Logout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Queue\SerializesModels;

class Logout
Expand All @@ -16,8 +17,8 @@ class Logout
* @return void
*/
public function __construct(
public $guard,
public $user,
public string $guard,
public Authenticatable $user,
Copy link
Member

Choose a reason for hiding this comment

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

This is potentially null.

Copy link
Contributor Author

@browner12 browner12 Dec 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

yah, i need to dig into this PR a little more. this is where it gets tricky, because do we change what the Event accepts? or do we force the calling code to actually follow what the event wants.

even with just this small PR, it's clear there are some things that just naturally slipped through since we've been looser with our types so far.

Copy link
Member

Choose a reason for hiding this comment

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

I was chatting to Taylor about this and suggested we go deeper on static analysis insights before we make refactors like this. Hopefully that would help expose places where the dockblocks are lying.

We don't want to be unintentionally introducing breaking changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yah, anything we can do to move us towards more strict typing I'm down for that.

I'd think I'd be on the side of forcing the calling code to adjust, for the most part.

glad you guys are talking about it internally. would love to help how I can.

) {
}
}
5 changes: 3 additions & 2 deletions src/Illuminate/Auth/Events/OtherDeviceLogout.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Queue\SerializesModels;

class OtherDeviceLogout
Expand All @@ -16,8 +17,8 @@ class OtherDeviceLogout
* @return void
*/
public function __construct(
public $guard,
public $user,
public string $guard,
public Authenticatable $user,
) {
}
}
3 changes: 2 additions & 1 deletion src/Illuminate/Auth/Events/PasswordReset.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Queue\SerializesModels;

class PasswordReset
Expand All @@ -15,7 +16,7 @@ class PasswordReset
* @return void
*/
public function __construct(
public $user,
public Authenticatable $user,
) {
}
}
3 changes: 2 additions & 1 deletion src/Illuminate/Auth/Events/PasswordResetLinkSent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\CanResetPassword;
use Illuminate\Queue\SerializesModels;

class PasswordResetLinkSent
Expand All @@ -15,7 +16,7 @@ class PasswordResetLinkSent
* @return void
*/
public function __construct(
public $user,
public CanResetPassword $user,
) {
}
}
3 changes: 2 additions & 1 deletion src/Illuminate/Auth/Events/Registered.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Queue\SerializesModels;

class Registered
Expand All @@ -15,7 +16,7 @@ class Registered
* @return void
*/
public function __construct(
public $user,
public Authenticatable $user,
) {
}
}
5 changes: 3 additions & 2 deletions src/Illuminate/Auth/Events/Validated.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\Authenticatable;
use Illuminate\Queue\SerializesModels;

class Validated
Expand All @@ -16,8 +17,8 @@ class Validated
* @return void
*/
public function __construct(
public $guard,
public $user,
public string $guard,
public Authenticatable $user,
) {
}
}
3 changes: 2 additions & 1 deletion src/Illuminate/Auth/Events/Verified.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

namespace Illuminate\Auth\Events;

use Illuminate\Contracts\Auth\MustVerifyEmail;
use Illuminate\Queue\SerializesModels;

class Verified
Expand All @@ -15,7 +16,7 @@ class Verified
* @return void
*/
public function __construct(
public $user,
public MustVerifyEmail $user,
) {
}
}
Loading