-
-
Notifications
You must be signed in to change notification settings - Fork 998
chore(phpstan): raise static analysis to level 3 (46→0, fix-forward) #3561
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
Changes from 2 commits
59b95a7
c5ba319
c71aafa
da42e23
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,7 +58,7 @@ public function loadManifest(string $manifestName) | |
| } | ||
| } | ||
|
|
||
| return false; | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -70,7 +70,7 @@ public function loadManifest(string $manifestName) | |
| */ | ||
| public function shouldRefresh($manifest, $paths) | ||
| { | ||
| return is_null($manifest) || $manifest[$manifest] != $paths; | ||
| return is_null($manifest) || $manifest != $paths; | ||
|
Comment on lines
69
to
+73
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in da42e23: shouldRefresh @param |
||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -188,8 +188,6 @@ public function assign(string $name, mixed $value): void | |
|
|
||
| /** | ||
| * get - get assigned values | ||
| * | ||
| * @return array | ||
| */ | ||
| public function get(string $name): mixed | ||
| { | ||
|
|
@@ -692,7 +690,7 @@ public function setNotification(string $msg, string $type, string $event_id = '' | |
| * | ||
|
Comment on lines
688
to
690
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| * @deprecated this should be in a component | ||
| */ | ||
| public function getToggleState(string $name): string | ||
| public function getToggleState(string $name): string|false | ||
| { | ||
| if (session()->exists('usersettings.submenuToggle.'.$name)) { | ||
| return session('usersettings.submenuToggle.'.$name); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| <?php | ||
|
|
||
| namespace Leantime\Domain\Auth\Models; | ||
|
|
||
| use Illuminate\Contracts\Auth\Authenticatable; | ||
|
|
||
| /** | ||
| * Lightweight Authenticatable wrapper around a user-data row. | ||
| * | ||
| * Replaces the `(object) $userRow` stdClass casts in AuthUser/ApiGuard so the provider/guard | ||
| * methods satisfy their `?Authenticatable` contracts. It uses dynamic properties on purpose so it | ||
| * stays a behavioural drop-in for the old stdClass cast — same property reads, same json/array | ||
| * serialization, truthy even when empty — and merely ADDS the Authenticatable accessor methods. | ||
| */ | ||
| #[\AllowDynamicProperties] | ||
| class AuthenticatableUser implements Authenticatable | ||
| { | ||
| /** | ||
| * @param array<string, mixed> $attributes A user row (column => value). | ||
| */ | ||
| public function __construct(array $attributes = []) | ||
| { | ||
| foreach ($attributes as $key => $value) { | ||
| $this->{$key} = $value; | ||
| } | ||
| } | ||
|
|
||
| public function getAuthIdentifierName(): string | ||
| { | ||
| return 'id'; | ||
| } | ||
|
|
||
| public function getAuthIdentifier(): mixed | ||
| { | ||
| return $this->id ?? null; | ||
| } | ||
|
|
||
| public function getAuthPasswordName(): string | ||
| { | ||
| return 'password'; | ||
| } | ||
|
|
||
| public function getAuthPassword(): string | ||
| { | ||
| return $this->password ?? ''; | ||
| } | ||
|
|
||
| public function getRememberToken(): string | ||
| { | ||
| return $this->remember_token ?? ''; | ||
| } | ||
|
|
||
| public function setRememberToken($value): void | ||
| { | ||
| // No-op: Leantime does not persist remember tokens (mirrors Auth::setRememberToken and | ||
| // AuthUser::updateRememberToken, which are likewise not implemented). | ||
| } | ||
|
|
||
| public function getRememberTokenName(): string | ||
| { | ||
| return 'remember_token'; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ | |
| use Illuminate\Contracts\Auth\Authenticatable; | ||
| use Illuminate\Contracts\Auth\UserProvider; | ||
| use Laravel\Sanctum\HasApiTokens; | ||
| use Leantime\Domain\Auth\Models\AuthenticatableUser; | ||
| use Leantime\Domain\Auth\Services\Auth as AuthService; | ||
|
|
||
| class AuthUser implements UserProvider | ||
|
|
@@ -26,12 +27,12 @@ public function __construct( | |
|
|
||
| public function retrieveById($identifier) | ||
| { | ||
| return (object) $this->userRepo->getUser($identifier); | ||
| return new AuthenticatableUser((array) $this->userRepo->getUser($identifier)); | ||
| } | ||
|
|
||
| public function retrieveByToken($identifier, $token) | ||
| { | ||
| return (object) $this->authService->getUserByToken($token); | ||
| return new AuthenticatableUser((array) $this->authService->getUserByToken($token)); | ||
| } | ||
|
Comment on lines
28
to
50
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch — fixed in da42e23: retrieveById/retrieveByToken now return |
||
|
|
||
| public function updateRememberToken(Authenticatable $user, $token) | ||
|
|
@@ -63,10 +64,12 @@ public function rehashPasswordIfRequired(Authenticatable $user, array $credentia | |
|
|
||
| public function getOrCreateUser($user, $source) | ||
| { | ||
| // Look up the existing account in a separate variable — the $user param holds the | ||
| // external/OAuth profile data we need to create the account from, so it must not be | ||
| // overwritten by the lookup result (doing so previously built new users with empty fields). | ||
| $existingUser = $this->authRepo->getUserByEmail($user['email']); | ||
|
|
||
| $user = $this->authRepo->getUserByEmail($user['email']); | ||
|
|
||
| if (empty($user) && config()->get('auth.create_user')) { | ||
| if (empty($existingUser) && config()->get('auth.create_user')) { | ||
|
|
||
| $userArray = [ | ||
| 'firstname' => $user['firstname'], | ||
|
|
@@ -83,11 +86,11 @@ public function getOrCreateUser($user, $source) | |
| 'status' => 'a', | ||
| ]; | ||
|
|
||
| $userId = $this->userRepo->addUser($userArray); | ||
| $user = $this->authRepo->getUserByEmail($user['email']); | ||
| $this->userRepo->addUser($userArray); | ||
| $existingUser = $this->authRepo->getUserByEmail($user['email']); | ||
| } | ||
|
|
||
| return $user; | ||
| return $existingUser; | ||
| } | ||
|
|
||
| public function setUser($userId) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in da42e23: getValidControllerCall now guards
$classPath === falseand throws NotFoundHttpException (a 404, consistent with getValidControllerMethod) before passing it on — no more TypeError on the disabled-plugin / missing-controller path.