chore(phpstan): raise static analysis to level 3 (46→0, fix-forward)

[marcelfolaron]

Raise PHPStan from level 2 → 3

Follow-up to #3559 (level 2). Climbs static analysis to level 3 (config level: 2 → 3), fix-forward, no baseline. make phpstan is green at level 3 (0 errors). Clean master had 46 level-3 findings.

Level 3 checks return-type and property-type correctness — i.e. the value a method actually returns / assigns matches its declared type. Most fixes are reconciling declared types with reality.

What changed

return.type — declared types corrected/widened to match what's actually returned:
getClassPath/getToggleState/getTokenUrl → string|false; Db::__get → ?PDO; Cast::castDateTime → CarbonImmutable; Template::get → mixed; Files\Services::upload → array|string|false; Sprints::getUpcomingSprint → the Sprints model the repo returns; ExceptionHandler::reportable/whoopsHandler → the concrete/contract types actually returned; StartSession::cache → the Cache contract Repository (Cache::store()'s real return — the lock() gap stub moves to Illuminate\Contracts\Cache\Repository to match).

Repository bool methods (editUser/removeFromClient/patchUser/updateTicketStatus/removeCollaborators) now return update()/delete() > 0, so the declared bool matches (they were returning the int row count).

return.void / assign.propertyType: EventDispatcher::listen no longer returns its Collection (the Dispatcher contract is void); Frontcontroller::$config is resolved via app(Environment::class) (the config() helper is typed as the Laravel Repository); HandleExceptions::$app @var made nullable; CsvImport::getValues @return void → array|false.

🔐 Sensitive: user provider/guards now return a real Authenticatable

The UserProvider/Guard contracts require ?Authenticatable, but AuthUser::retrieveById/retrieveByToken and ApiGuard::user() returned a (object) stdClass cast — a contract violation, and the reason the guards' id() couldn't call getAuthIdentifier().

New Leantime\Domain\Auth\Models\AuthenticatableUser: a #[AllowDynamicProperties] wrapper that is a behavioural drop-in for the old stdClass cast (same dynamic property reads, same json/array serialization, truthy even when empty) and only adds the Authenticatable accessor methods. Both guards' id() go back to getAuthIdentifier(). Auth::getRememberToken returns '' (the contract types it string); setRememberToken stays a no-op (Leantime doesn't persist remember tokens).

⚠️ This touches the bearer/sanctum auth path that has regressed before. The change is designed to be behaviour-preserving, but please give it a close look — the api / login / bearer-api / user acceptance groups in CI are the validation gate. (I was unable to run the Docker acceptance suite locally.)

🐛 Real bugs surfaced by level 3 (fixed here)

• PathManifestRepository::shouldRefresh did $manifest[$manifest] — an array used as an array key (runtime TypeError). Now $manifest != $paths.
• Files /download.php route used redirect()->to(...), but Leantime's redirect() helper returns a Symfony RedirectResponse (no ->to()) — it would fatal. Changed to redirect($url, 301) (the helper's real signature; it was the only redirect()->to() caller in the codebase).
• AuthUser::getOrCreateUser overwrote its $user param with the empty email lookup, then built the new user from the empty result — auto-provisioned (OAuth) users got empty fields.
• Format::timestamp/jsTimestamp returned '' (string) for the empty case under an int|bool signature → false.
• Tickets::sortItemsHierarchically returned a non-array fallback under an : array signature → [].
• DelMilestone/ShowClient/ShowTicket read $result['msg'] on a falsy/non-array result → guarded.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Raises PHPStan strictness from level 2 to level 3 and fix-forwards newly surfaced findings (mostly return/property type correctness), including a sensitive auth-path change to return real Authenticatable instances instead of stdClass.

Changes:

• Bumps .phpstan/phpstan.neon to level 3 and updates stubs/docs/signatures to satisfy stricter type expectations.
• Fixes a handful of runtime bugs uncovered by analysis (e.g., manifest refresh comparison, redirect helper misuse, safe array access guards).
• Refactors auth provider/guards to return an Authenticatable wrapper and restores id() to getAuthIdentifier().

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
app/helpers.php	Updates redirect() return type docs (but still contains duplicate helper definitions).
app/Domain/Users/Services/Users.php	Adjusts createUserInvite() return type to match reality.
app/Domain/Users/Repositories/Users.php	Makes update-style repository methods return bool (> 0).
app/Domain/Tickets/Services/Tickets.php	Ensures sortItemsHierarchically() always returns an array.
app/Domain/Tickets/Repositories/Tickets.php	Makes update/delete methods return bool (> 0).
app/Domain/Tickets/Controllers/ShowTicket.php	Guards notification message access for non-array results.
app/Domain/Tickets/Controllers/DelMilestone.php	Guards notification message access for non-array results.
app/Domain/Sprints/Services/Sprints.php	Fixes getUpcomingSprint() declared return type to match repo model return.
app/Domain/Oidc/Services/Oidc.php	Corrects getTokenUrl() declared return type to include false.
app/Domain/Files/Services/Files.php	Widens upload() return type to include false.
app/Domain/Files/routes.php	Fixes redirect usage to match project redirect() helper signature.
app/Domain/Dashboard/Services/Dashboard.php	Avoids undefined index on parsed URL path.
app/Domain/CsvImport/Services/CsvImport.php	Corrects getValues() doc return type.
app/Domain/Clients/Controllers/ShowClient.php	Guards notification message access for non-array results.
app/Domain/Blueprints/Repositories/Blueprints.php	Casts new canvas id to int to satisfy declared return type.
app/Domain/Auth/Services/AuthUser.php	Returns AuthenticatableUser wrappers (needs null-on-miss handling).
app/Domain/Auth/Services/Auth.php	Makes remember token accessors comply with string contract.
app/Domain/Auth/Models/AuthenticatableUser.php	Adds Authenticatable-compliant dynamic-property user wrapper.
app/Domain/Auth/Guards/WebGuard.php	Restores id() via getAuthIdentifier().
app/Domain/Auth/Guards/ApiGuard.php	Wraps API-key user data in AuthenticatableUser; restores id() contract.
app/Core/UI/Template.php	Updates return type for getToggleState() (docblock needs sync).
app/Core/Support/Format.php	Returns false instead of '' for invalid timestamps to match signature.
app/Core/Support/Cast.php	Updates doc return type for datetime casting.
app/Core/Sessions/PathManifestRepository.php	Fixes manifest refresh comparison and normalizes missing-manifest to null.
app/Core/Middleware/TrustProxies.php	Corrects $headers doc type to int.
app/Core/Middleware/StartSession.php	Adjusts cache return type to contract interface.
app/Core/Exceptions/HandleExceptions.php	Makes $app doc type nullable.
app/Core/Exceptions/ExceptionHandler.php	Corrects reportable/whoops handler return types.
app/Core/Events/EventDispatcher.php	Aligns listen() behavior with void return expectations.
app/Core/Db/Db.php	Corrects __get doc return type to `\PDO
app/Core/Controller/Frontcontroller.php	Fixes $config typing and widens getClassPath() to `string
app/Core/Bootloader.php	Removes incorrect static-instance docblock.
.phpstan/stubs/laravel-gaps.stub	Moves cache lock() gap to Illuminate\\Contracts\\Cache\\Repository for analysis.
.phpstan/phpstan.neon	Raises PHPStan analysis level from 2 to 3.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[marcelfolaron]

Fixed in da42e23: getValidControllerCall now guards $classPath === false and throws NotFoundHttpException (a 404, consistent with getValidControllerMethod) before passing it on — no more TypeError on the disabled-plugin / missing-controller path.

[marcelfolaron]

Good catch — fixed in da42e23: retrieveById/retrieveByToken now return null when the lookup fails and only build an AuthenticatableUser from a real row, so a 'not found' can't yield a non-null (authenticated-looking) user.

[marcelfolaron]

Fixed in da42e23: getToggleState docblock @return → string|false.

[marcelfolaron]

Fixed in da42e23: shouldRefresh @param $manifest → array|null.

[marcelfolaron]

Fixed in da42e23: removed the duplicate (dead) redirect()/currentRoute() definitions — the second block sat behind the same function_exists guard and never ran. Single canonical definition kept.

[marcelfolaron]

Fixed in da42e23: signature + docblock → false|string (addUser returns false|string; the method never returns true or an int).

[Copilot]

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

app/Domain/Oidc/Services/Oidc.php:462

• getTokenUrl() now advertises string|false, but its only caller (requestTokens()) passes the return value directly into Http::post(), which expects a string URL. If endpoint discovery fails, returning false will cause a runtime TypeError (bool passed where string is required) during the OIDC callback.

Consider making getTokenUrl() always return a non-empty string and throw a descriptive exception when the token endpoint can’t be resolved (so the failure is explicit and can be handled/logged cleanly).

    private function getTokenUrl(): string|false
    {
        if (! empty($this->tokenUrl || $this->loadEndpoints())) {
            return $this->tokenUrl;
        }