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

Wrong return type when overwriting Request::user() #1718

Open
AJenbo opened this issue Aug 14, 2023 · 4 comments · May be fixed by #1861 or #1951
Open

Wrong return type when overwriting Request::user() #1718

AJenbo opened this issue Aug 14, 2023 · 4 comments · May be fixed by #1861 or #1951
Labels
bug Something isn't working

Comments

@AJenbo
Copy link

AJenbo commented Aug 14, 2023

  • Larastan Version: 2.6.4
  • --level used: 8

Description

Larastan will overwrite the return type of requests that narrow the return of user().

  5     Cannot access property $id on App\User|null.

Laravel code where the issue was found

<?php

namespace App\Http\Requests;

resolve(AuthedRequest::class)->user()->id
<?php

namespace App\Http\Requests;

use Illuminate\Foundation\Http\FormRequest;
use App\User;

class AuthedRequest extends FormRequest
{
    public function authorize(): bool
    {
        return parent::user() instanceof User;
    }

    public function user($guard = null): User
    {
        $user = parent::user($guard);
        if (!$user instanceof User) {
            abort(403);
        }

        return $user;
    }
}
@szepeviktor
Copy link
Collaborator

szepeviktor commented Aug 14, 2023

I think Larastan does not know that AuthedRequest may not have an instance thus it cannot be null.

@AJenbo
Copy link
Author

AJenbo commented Aug 14, 2023

Laravel (9) hints it as mixed via phpdoc. Larastan then deduces it down to ?User. But also overwrites children with a more narrow and native typehint which should not happen.

@canvural
Copy link
Collaborator

canvural commented Oct 2, 2023

Issue can be in this extension. We need to check if the user method is the parent one or not I guess.

@canvural canvural added the bug Something isn't working label Oct 2, 2023
@sandeshjangam
Copy link

sandeshjangam commented May 19, 2024

I am facing the same issue

Define the new request class overriding user method

namespace App\Http\Requests;

use App\Helpers\Response;
use App\Models\User;
use Illuminate\Http\Exceptions\HttpResponseException;
use Illuminate\Http\Request;

class TopRequest extends Request
{
    /**
     * Get the user or throws an exception.
     *
     * @param  string|null  $guard
     * @return User User model
     *
     * @throws HttpResponseException
     */
    public function user($guard = null)
    {
        $user = parent::user($guard);

        if (! $user instanceof User) {
            throw new HttpResponseException(
                Response::unauthorized(__('User doesn\'t exist'))
            );
        }

        return $user;
    }
}

Use the new request in the controller

class AbcController extends Controller
{
     /**
     * List
     *
     * @since x.x.x
     */
    public function index(TopRequest $request): JsonResponse
    {
        // Get logged in user.
        $user = $request->user();

        $email = $user->email
   }
}

Error

 :147   Cannot access property $email on App\Models\User|null. 

If I define new getUser method and use it instead of user it works perfecly fine. So issue is definilty lies in stan check.

Any solution for this?

@sandeshjangam sandeshjangam linked a pull request May 19, 2024 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
4 participants