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

[12.x] Const to enum #53961

Closed
wants to merge 43 commits into from
Closed
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
3a251cf
const to enum
shaedrich Dec 18, 2024
5bf6f69
Fix syntax error
shaedrich Dec 18, 2024
dbf5145
Fix StyleCI
shaedrich Dec 18, 2024
86733fd
More linting fixes
shaedrich Dec 18, 2024
7ccc387
Fix Typo
shaedrich Dec 18, 2024
2ebb039
Fix typo
shaedrich Dec 18, 2024
af63fdd
Add missing input
shaedrich Dec 18, 2024
053c72a
stringify enums
shaedrich Dec 18, 2024
7d04309
Fix typo
shaedrich Dec 18, 2024
eaa84c0
enum_value
shaedrich Dec 18, 2024
8712ba9
Fix test
shaedrich Dec 18, 2024
01468a4
Another enum_value
shaedrich Dec 18, 2024
a7fd3e9
FIx StyleCI
shaedrich Dec 18, 2024
52ed299
Fix syntax error
shaedrich Dec 18, 2024
a248b4e
Add missing import
shaedrich Dec 18, 2024
27da74a
Fix StyleCI
shaedrich Dec 18, 2024
201a5e2
Only wrap in enum if it is none
shaedrich Dec 18, 2024
a8eca1b
Remove worker-related enum
shaedrich Dec 18, 2024
935efcc
Change casing for enum cases
shaedrich Dec 18, 2024
31cc1b3
Remove unused function
shaedrich Dec 18, 2024
7f0bda7
Revert to promoted property
shaedrich Dec 18, 2024
f49854a
Remove enum_value to the proper location
shaedrich Dec 18, 2024
4c17dcd
Cron expression is not an enum
shaedrich Dec 18, 2024
f44b034
Also make ScheduleOn cases PascalCase
shaedrich Dec 19, 2024
e065656
More casing
shaedrich Dec 19, 2024
a0a2ee9
Remove manually setting argument that has been reverted to a promited…
shaedrich Dec 19, 2024
41a889a
Deprecation messages and correct PHPDoc types
shaedrich Dec 19, 2024
1b19e79
Improve type-hint for days
shaedrich Dec 19, 2024
8ce6e6c
More PHPDocs
shaedrich Dec 19, 2024
f384afd
Fix StyleCI
shaedrich Dec 19, 2024
c66a581
More StyleCI
shaedrich Dec 19, 2024
0e26932
Remove accidentally added spacing
shaedrich Dec 19, 2024
76cc1d6
StyleCI leftovers
shaedrich Dec 19, 2024
ddfacc2
Months are 1-indexed
shaedrich Dec 19, 2024
2ddbab1
Add type-hints to respective facade
shaedrich Dec 19, 2024
1df5098
Add range type-hint for seconds
shaedrich Dec 19, 2024
4418d4f
Add array type-hint
shaedrich Dec 19, 2024
f08f992
Fix typo
shaedrich Dec 19, 2024
d4f76ad
formatting
taylorotwell Dec 19, 2024
eed57b8
Update PasswordResetResult.php
taylorotwell Dec 19, 2024
d86d3ad
still use strings for passwords
taylorotwell Dec 19, 2024
3c1ba5f
cant depend on auth component
taylorotwell Dec 19, 2024
680036d
dont use enums
taylorotwell Dec 19, 2024
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
31 changes: 31 additions & 0 deletions src/Illuminate/Auth/Enums/PasswordStatus.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace Illuminate\Auth\Enums;

enum PasswordStatus: string
{
/**
* Constant representing a successfully sent reminder.
*/
case ResetLinkSent = 'passwords.sent';

/**
* Constant representing a successfully reset password.
*/
case PasswordReset = 'passwords.reset';

/**
* Constant representing the user not found response.
*/
case InvalidUser = 'passwords.user';

/**
* Constant representing an invalid token.
*/
case InvalidToken = 'passwords.token';

/**
* Constant representing a throttled reset attempt.
*/
case ResetThrottled = 'passwords.throttled';
}
19 changes: 10 additions & 9 deletions src/Illuminate/Auth/Passwords/PasswordBroker.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Illuminate\Auth\Passwords;

use Closure;
use Illuminate\Auth\Enums\PasswordStatus;
use Illuminate\Auth\Events\PasswordResetLinkSent;
use Illuminate\Contracts\Auth\CanResetPassword as CanResetPasswordContract;
use Illuminate\Contracts\Auth\PasswordBroker as PasswordBrokerContract;
Expand Down Expand Up @@ -54,7 +55,7 @@ public function __construct(#[\SensitiveParameter] TokenRepositoryInterface $tok
*
* @param array $credentials
* @param \Closure|null $callback
* @return string
* @return \Illuminate\Auth\Enums\PasswordStatus
*/
public function sendResetLink(#[\SensitiveParameter] array $credentials, ?Closure $callback = null)
{
Expand All @@ -64,17 +65,17 @@ public function sendResetLink(#[\SensitiveParameter] array $credentials, ?Closur
$user = $this->getUser($credentials);

if (is_null($user)) {
return static::INVALID_USER;
return PasswordStatus::InvalidUser;
}

if ($this->tokens->recentlyCreatedToken($user)) {
return static::RESET_THROTTLED;
return PasswordStatus::ResetThrottled;
}

$token = $this->tokens->create($user);

if ($callback) {
return $callback($user, $token) ?? static::RESET_LINK_SENT;
return $callback($user, $token) ?? PasswordStatus::ResetLinkSent;
}

// Once we have the reset token, we are ready to send the message out to this
Expand All @@ -84,7 +85,7 @@ public function sendResetLink(#[\SensitiveParameter] array $credentials, ?Closur

$this->events?->dispatch(new PasswordResetLinkSent($user));

return static::RESET_LINK_SENT;
return PasswordStatus::ResetLinkSent;
}

/**
Expand Down Expand Up @@ -114,23 +115,23 @@ public function reset(#[\SensitiveParameter] array $credentials, Closure $callba

$this->tokens->delete($user);

return static::PASSWORD_RESET;
return PasswordStatus::PasswordReset;
}

/**
* Validate a password reset for the given credentials.
*
* @param array $credentials
* @return \Illuminate\Contracts\Auth\CanResetPassword|string
* @return \Illuminate\Contracts\Auth\CanResetPassword|\Illuminate\Auth\Enums\PasswordStatus
*/
protected function validateReset(#[\SensitiveParameter] array $credentials)
{
if (is_null($user = $this->getUser($credentials))) {
return static::INVALID_USER;
return PasswordStatus::InvalidUser;
}

if (! $this->tokens->exists($user, $credentials['token'])) {
return static::INVALID_TOKEN;
return PasswordStatus::InvalidToken;
}

return $user;
Expand Down
20 changes: 20 additions & 0 deletions src/Illuminate/Console/Enums/ScheduleOn.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

namespace Illuminate\Console\Enums;

enum ScheduleOn: int
{
case SUNDAY = 0;
shaedrich marked this conversation as resolved.
Show resolved Hide resolved

case MONDAY = 1;

case TUESDAY = 2;

case WEDNESDAY = 3;

case THURSDAY = 4;

case FRIDAY = 5;

case SATURDAY = 6;
}
25 changes: 15 additions & 10 deletions src/Illuminate/Console/Scheduling/ManagesFrequencies.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@

namespace Illuminate\Console\Scheduling;

use Illuminate\Console\Enums\ScheduleOn;
use Illuminate\Support\Carbon;
use InvalidArgumentException;

use function Illuminate\Support\enum_value;

trait ManagesFrequencies
{
/**
Expand Down Expand Up @@ -401,7 +404,7 @@ protected function hourBasedSchedule($minutes, $hours)
*/
public function weekdays()
{
return $this->days(Schedule::MONDAY.'-'.Schedule::FRIDAY);
return $this->days(ScheduleOn::MONDAY->value.'-'.ScheduleOn::FRIDAY->value);
}

/**
Expand All @@ -411,7 +414,7 @@ public function weekdays()
*/
public function weekends()
{
return $this->days(Schedule::SATURDAY.','.Schedule::SUNDAY);
return $this->days(ScheduleOn::SATURDAY->value.','.ScheduleOn::SUNDAY->value);
}

/**
Expand All @@ -421,7 +424,7 @@ public function weekends()
*/
public function mondays()
{
return $this->days(Schedule::MONDAY);
return $this->days(ScheduleOn::MONDAY);
}

/**
Expand All @@ -431,7 +434,7 @@ public function mondays()
*/
public function tuesdays()
{
return $this->days(Schedule::TUESDAY);
return $this->days(ScheduleOn::TUESDAY);
}

/**
Expand All @@ -441,7 +444,7 @@ public function tuesdays()
*/
public function wednesdays()
{
return $this->days(Schedule::WEDNESDAY);
return $this->days(ScheduleOn::WEDNESDAY);
}

/**
Expand All @@ -451,7 +454,7 @@ public function wednesdays()
*/
public function thursdays()
{
return $this->days(Schedule::THURSDAY);
return $this->days(ScheduleOn::THURSDAY);
}

/**
Expand All @@ -461,7 +464,7 @@ public function thursdays()
*/
public function fridays()
{
return $this->days(Schedule::FRIDAY);
return $this->days(ScheduleOn::FRIDAY);
}

/**
Expand All @@ -471,7 +474,7 @@ public function fridays()
*/
public function saturdays()
{
return $this->days(Schedule::SATURDAY);
return $this->days(ScheduleOn::SATURDAY);
}

/**
Expand All @@ -481,7 +484,7 @@ public function saturdays()
*/
public function sundays()
{
return $this->days(Schedule::SUNDAY);
return $this->days(ScheduleOn::SUNDAY);
}

/**
Expand Down Expand Up @@ -633,6 +636,8 @@ public function days($days)
{
$days = is_array($days) ? $days : func_get_args();

$days = array_map(enum_value(...), $days);

return $this->spliceIntoPosition(5, implode(',', $days));
}

Expand Down Expand Up @@ -660,7 +665,7 @@ protected function spliceIntoPosition($position, $value)
{
$segments = preg_split("/\s+/", $this->expression);

$segments[$position - 1] = $value;
$segments[$position - 1] = enum_value($value);

return $this->cron(implode(' ', $segments));
}
Expand Down
22 changes: 15 additions & 7 deletions src/Illuminate/Console/Scheduling/Schedule.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use DateTimeInterface;
use Illuminate\Bus\UniqueLock;
use Illuminate\Console\Application;
use Illuminate\Console\Enums\ScheduleOn;
use Illuminate\Container\Container;
use Illuminate\Contracts\Bus\Dispatcher;
use Illuminate\Contracts\Cache\Repository as Cache;
Expand All @@ -28,19 +29,26 @@ class Schedule
__call as macroCall;
}

const SUNDAY = 0;
/** @deprecated Use \Illuminate\Console\Enums\ScheduleOn::SUNDAY instead */
const SUNDAY = ScheduleOn::SUNDAY;
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is scheduled for the next version, I would take the opportunity to move completely to the new approach, and document these changes.

But this is better to check with one of the maintainers. Maybe it is better to keep those for a smooth transition


const MONDAY = 1;
/** @deprecated Use \Illuminate\Console\Enums\ScheduleOn::MONDAY instead */
const MONDAY = ScheduleOn::MONDAY;

const TUESDAY = 2;
/** @deprecated Use \Illuminate\Console\Enums\ScheduleOn::TUESDAY instead */
const TUESDAY = ScheduleOn::TUESDAY;

const WEDNESDAY = 3;
/** @deprecated Use \Illuminate\Console\Enums\ScheduleOn::WEDNESDAY instead */
const WEDNESDAY = ScheduleOn::WEDNESDAY;

const THURSDAY = 4;
/** @deprecated Use \Illuminate\Console\Enums\ScheduleOn::THURSDAY instead */
const THURSDAY = ScheduleOn::THURSDAY;

const FRIDAY = 5;
/** @deprecated Use \Illuminate\Console\Enums\ScheduleOn::FRIDAY instead */
const FRIDAY = ScheduleOn::FRIDAY;

const SATURDAY = 6;
/** @deprecated Use \Illuminate\Console\Enums\ScheduleOn::SATURDAY instead */
const SATURDAY = ScheduleOn::SATURDAY;
Copy link
Contributor

Choose a reason for hiding this comment

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

I like your deprecation strategy.


/**
* All of the events on the schedule.
Expand Down
13 changes: 7 additions & 6 deletions src/Illuminate/Contracts/Auth/PasswordBroker.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace Illuminate\Contracts\Auth;

use Closure;
use Illuminate\Auth\Enums\PasswordStatus;

interface PasswordBroker
{
Expand All @@ -11,42 +12,42 @@ interface PasswordBroker
*
* @var string
*/
const RESET_LINK_SENT = 'passwords.sent';
const RESET_LINK_SENT = PasswordStatus::ResetLinkSent;

/**
* Constant representing a successfully reset password.
*
* @var string
*/
const PASSWORD_RESET = 'passwords.reset';
const PASSWORD_RESET = PasswordStatus::PasswordReset;

/**
* Constant representing the user not found response.
*
* @var string
*/
const INVALID_USER = 'passwords.user';
const INVALID_USER = PasswordStatus::InvalidUser;

/**
* Constant representing an invalid token.
*
* @var string
*/
const INVALID_TOKEN = 'passwords.token';
const INVALID_TOKEN = PasswordStatus::InvalidToken;

/**
* Constant representing a throttled reset attempt.
*
* @var string
*/
const RESET_THROTTLED = 'passwords.throttled';
const RESET_THROTTLED = PasswordStatus::ResetThrottled;

/**
* Send a password reset link to a user.
*
* @param array $credentials
* @param \Closure|null $callback
* @return string
* @return \Illuminate\Auth\Enums\PasswordStatus
*/
public function sendResetLink(array $credentials, ?Closure $callback = null);

Expand Down
1 change: 1 addition & 0 deletions src/Illuminate/Queue/Events/WorkerStopping.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ public function __construct(
public $status = 0,
public $workerOptions = null
) {
$this->status = $status;
shaedrich marked this conversation as resolved.
Show resolved Hide resolved
shaedrich marked this conversation as resolved.
Show resolved Hide resolved
}
}
14 changes: 7 additions & 7 deletions src/Illuminate/Queue/Worker.php
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ protected function registerTimeoutHandler($job, WorkerOptions $options)
));
}

$this->kill(static::EXIT_ERROR, $options);
$this->kill(self::EXIT_ERROR, $options);
}, true);

pcntl_alarm(
Expand Down Expand Up @@ -304,12 +304,12 @@ protected function pauseWorker(WorkerOptions $options, $lastRestart)
protected function stopIfNecessary(WorkerOptions $options, $lastRestart, $startTime = 0, $jobsProcessed = 0, $job = null)
{
return match (true) {
$this->shouldQuit => static::EXIT_SUCCESS,
$this->memoryExceeded($options->memory) => static::EXIT_MEMORY_LIMIT,
$this->queueShouldRestart($lastRestart) => static::EXIT_SUCCESS,
$options->stopWhenEmpty && is_null($job) => static::EXIT_SUCCESS,
$options->maxTime && hrtime(true) / 1e9 - $startTime >= $options->maxTime => static::EXIT_SUCCESS,
$options->maxJobs && $jobsProcessed >= $options->maxJobs => static::EXIT_SUCCESS,
$this->shouldQuit => self::EXIT_SUCCESS,
$this->memoryExceeded($options->memory) => self::EXIT_MEMORY_LIMIT,
$this->queueShouldRestart($lastRestart) => self::EXIT_SUCCESS,
$options->stopWhenEmpty && is_null($job) => self::EXIT_SUCCESS,
$options->maxTime && hrtime(true) / 1e9 - $startTime >= $options->maxTime => self::EXIT_SUCCESS,
$options->maxJobs && $jobsProcessed >= $options->maxJobs => self::EXIT_SUCCESS,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this also be a leftover? Or is the change from static to self intentionally left as a part of this PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

While reviewing this file, I also felt compelled to say this change was unnecessary.

But after some thinking, and considering your previous remark regarding when enums should be used versus when constants should be used, I thought that, even if the class isn't final, there is no point on reading these constants from subclasses, as they are basically hard-coded exit values from processes, and should not change.

Using self:: instead of static::, improves -- IMO -- intention declaration and correctness in this case.

default => null
};
}
Expand Down
Loading