Skip to content

[8.x] Improves generic types on the skeleton #5740

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

Merged
merged 1 commit into from
Dec 3, 2021

Conversation

nunomaduro
Copy link
Member

This pull request is a follow up for laravel/framework#38538, and it improves the DX of Laravel's skeleton on editors like PHPStorm that now support generics. And of course, it also improves the DX with static analyse tools in the process.

Here is a few examples where the DX was improved while using PHPStorm:
Screenshot 2021-12-03 at 11 52 40
Screenshot 2021-12-03 at 11 53 24
Screenshot 2021-12-03 at 12 00 57
Screenshot 2021-12-03 at 12 05 02

@taylorotwell taylorotwell merged commit 8a62ca2 into 8.x Dec 3, 2021
@taylorotwell taylorotwell deleted the feat/improve-generic-types branch December 3, 2021 15:05
@mintbridge
Copy link

Running Psalm at max level with a clean laravel install has issues with these, as they are different to their parent type hints.

You get these type of psalm errors:

ERROR: NonInvariantDocblockPropertyType - app/Exceptions/Handler.php:24:15 - Property App\Exceptions\Handler::$dontFlash has type array<int, string>, not invariant with Illuminate\Foundation\Exceptions\Handler::$dontFlash of type array<array-key, string> (see https://psalm.dev/267)
    protected $dontFlash = [

changing to array<array-key, string> (e.g. below) fixes the psalm errors but i don't have phpstorm to check:

    /**
     * A list of the inputs that are never flashed for validation exceptions.
     *
     * @var array<array-key, string>
     */
    protected $dontFlash = [
        'current_password',
        'password',
        'password_confirmation',
    ];

@mintbridge
Copy link

@nunomaduro should i raise this as an issue?

@driesvints
Copy link
Member

@mintbridge best to just try a PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants