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

[Help wanted] Listing PHP 8.1 features #6395

Closed
24 of 25 tasks
orklah opened this issue Sep 3, 2021 · 28 comments
Closed
24 of 25 tasks

[Help wanted] Listing PHP 8.1 features #6395

orklah opened this issue Sep 3, 2021 · 28 comments
Milestone

Comments

@orklah
Copy link
Collaborator

orklah commented Sep 3, 2021

The first release candidate for PHP 8.1 is out now and the feature freeze is past, We may start looking at the new features and ensuring they're implemented in Psalm. Some were already implemented by Matt but not all.

This thread will regroup every new feature to come and what impact it will have on Psalm.

PHP release notes can be found here: https://github.com/php/php-src/blob/PHP-8.1/UPGRADING.

If you want to help, please respond here with the feature name, a small reproducer of the feature(and the impact it must have on Psalm analysis). For example:

Callmap/stub changes

Parser changes

Enum support

I'll update this post with new items as well as progress

@psalm-github-bot
Copy link

psalm-github-bot bot commented Sep 3, 2021

I found these snippets:

https://psalm.dev/r/8bef57c13a
<?php

array_is_list([]);
Psalm output (using commit a655ca8):

ERROR: UndefinedFunction - 3:1 - Function array_is_list does not exist
https://psalm.dev/r/fe157fd547
<?php

$a = [];
if(array_is_list($a)) {
 	/** @psalm-trace $a */
    //$a should be `list<mixed>` here
}
Psalm output (using commit a655ca8):

ERROR: UndefinedFunction - 4:4 - Function array_is_list does not exist

INFO: Trace - 6:0 - $a: array<array-key, mixed>
https://psalm.dev/r/bb9d8800b1
<?php

$a = ['a' => 5];
if(array_is_list($a)) { //Psalm should emit an issue because the types don't match
 	
}
Psalm output (using commit a655ca8):

ERROR: UndefinedFunction - 4:4 - Function array_is_list does not exist

@niconoe-
Copy link

niconoe- commented Sep 3, 2021

I'm thinking about managing readonly in promoted constructors: https://psalm.dev/r/ec599a194c

I'm using this blog to have a list of new features and deprecated stuff in PHP 8.1. You could probably extract a list from this, as it regroups a lot.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/ec599a194c
<?php

class TheReadonlyFeature
{
    public function __construct(
        public readonly string $bar
    ) {}
}

new TheReadonlyFeature('string');
Psalm output (using commit a655ca8):

ERROR: ParseError - 6:25 - Syntax error, unexpected T_STRING, expecting T_VARIABLE on line 6

ERROR: TooManyArguments - 10:1 - Too many arguments for TheReadonlyFeature::__construct - expecting 0 but saw 1

ERROR: InvalidDocblock - 6:9 - Param1 of TheReadonlyFeature::__construct has invalid syntax

@orklah
Copy link
Collaborator Author

orklah commented Sep 4, 2021

@niconoe- thanks, I added your case. Yeah, I know where to find the new features, but creating every repro, thinking of impact on Psalm would take me a long time. It's a good opportunity to make the community participate, even when they lack technical insight of Psalm.

Ideally, I'll then create issues for the simpler cases with an explanation how to do it and encourage people to try and implement the feature

@simPod
Copy link
Contributor

simPod commented Sep 4, 2021

Array unpacking with string keys should be supported and new array type inferred

https://psalm.dev/r/a25339af39

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/a25339af39
<?php

$x = [
    "a" => 0,
    ...["a" => 1],
    ...["b" => 2]
];

/** @psalm-trace $x */
// $x should be ['a' => 1, 'b' => 2] as `a` is overwritten and `b` added.
Psalm output (using commit a655ca8):

ERROR: DuplicateArrayKey - 5:8 - String keys are not supported in unpacked arrays

ERROR: DuplicateArrayKey - 6:8 - String keys are not supported in unpacked arrays

INFO: Trace - 10:74 - $x: array{a: 0}

INFO: UnusedVariable - 3:1 - $x is never referenced or the value is not used

@orklah
Copy link
Collaborator Author

orklah commented Sep 4, 2021

Thanks @simPod!

@weirdan
Copy link
Collaborator

weirdan commented Sep 4, 2021

can you tag this with "help wanted" and pin the issue?

Done. I also added milestone that we can assign to issues / PRs.

@niconoe-
Copy link

niconoe- commented Sep 5, 2021

With Enum:

I think that's already something 😄

[@weirdan] Split into a separate ticket here: #6425

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/ceaa9c9892
<?php

enum Status {
  case Foo;
  case Bar;
  case Bar;
}
Psalm output (using commit 916b098):

ERROR: ParseError - 3:6 - Syntax error, unexpected T_STRING on line 3
https://psalm.dev/r/108275cc1c
<?php

enum Status: string
{
    case Foo = 'foo';
    case Bar = 'bar';
    case Baz = 'bar';
}
Psalm output (using commit 916b098):

ERROR: ParseError - 3:6 - Syntax error, unexpected T_STRING on line 3

ERROR: ParseError - 5:5 - Syntax error, unexpected T_CASE on line 5
https://psalm.dev/r/89eb27ab21
<?php

enum Status: array
{
    case IsEmpty = [];
    case IsNotEmpty = ['foo'];
}
Psalm output (using commit 916b098):

ERROR: ParseError - 3:6 - Syntax error, unexpected T_STRING on line 3

ERROR: ParseError - 4:1 - Syntax error, unexpected '{', expecting '(' on line 4

ERROR: ParseError - 6:5 - Syntax error, unexpected T_CASE on line 6

ERROR: UndefinedConstant - 3:1 - Const enum is not defined

ERROR: UndefinedConstant - 5:10 - Const IsEmpty is not defined

ERROR: UndefinedConstant - 6:10 - Const IsNotEmpty is not defined
https://psalm.dev/r/3a77b77825
<?php

enum Status: int
{
    case FOO = 1;
    case BAR = 2;
}

// Returns Status::FOO
Status::from(1);

// Throw ValueError Exception
Status::from(3);

// Retunrs null
Status::tryFrom(3);

// Returns [Status::FOO, Status::BAR]
Status::cases();
Psalm output (using commit 916b098):

ERROR: ParseError - 3:6 - Syntax error, unexpected T_STRING on line 3

ERROR: ParseError - 5:5 - Syntax error, unexpected T_CASE on line 5
https://psalm.dev/r/d153c9d4f0
<?php

enum Status: int
{
    case FOO = 1;
    case BAR = 2;
}

$statusA = Status::FOO;
$statusB = Status::FOO;
$statusC = Status::BAR;

$statusA === $statusB; // true
$statusA === $statusC; // false
$statusC instanceof Status; // true
Psalm output (using commit 916b098):

ERROR: ParseError - 3:6 - Syntax error, unexpected T_STRING on line 3

ERROR: ParseError - 5:5 - Syntax error, unexpected T_CASE on line 5
https://psalm.dev/r/ec911f5510
<?php

enum Status: int
{
    case FOO;
    case BAR = 42;
}

$reflectedEnum = new ReflectionEnum(Status::class);

$reflectedEnum->hasCase('FOO'); //returns true
$reflectedEnum->hasCase('BAZ'); //returns false

$reflectedEnum->getCases(); //returns array<ReflectionEnumPureCase|ReflectionEnumBackedCase>

$reflectedEnum->getCase('FOO'); //returns ReflectionEnumPureCase
$reflectedEnum->getCase('BAR'); //returns ReflectionEnumBackedCase
$reflectedEnum->getCase('BAZ'); //throws ReflectionException

$reflectedEnum->isBacked('FOO'); //returns false
$reflectedEnum->isBacked('BAR'); //returns true
$reflectedEnum->isBacked('BAZ'); //Unknown behavior? returns false or throw ReflectionException? RFC don't say.

$reflectedEnum->getBackingType('FOO'); //returns null
$reflectedEnum->getBackingType('BAR'); //returns ReflectionType<int|string> (in this example, ReflectionType<int>)
$reflectedEnum->getBackingType('BAZ'); //Unknown behavior? returns null or throw ReflectionException? RFC don't say.

// Also exists:
$reflectedUnitEnum = new ReflectionEnumUnitCase(Status::FOO);
$reflectedUnitEnum->getValue(); //returns Status::FOO
$reflectedUnitEnum->getEnum(); //returns ReflectionEnum<Status>

$reflectedBackedEnum = new ReflectionEnumBackedCase (Status::BAR);
$reflectedBackedEnum->getValue(); //returns Status::BAR
$reflectedBackedEnum->getEnum(); //returns ReflectionEnum<Status>
$reflectedBackedEnum->getBackingValue(); //returns int|string (in this example, 42)

// Also possible:
$foo = Status::FOO;
$bar = Status::BAR;
$foo->name === 'FOO'; // Automatically defined readonly property.
$bar->name === 'BAR'; // Automatically defined readonly property.

// Finally:
enum_exists(Status::FOO); //returns true
enum_exists(Status::BAZ); //returns false
Psalm output (using commit 916b098):

ERROR: ParseError - 3:6 - Syntax error, unexpected T_STRING on line 3

ERROR: ParseError - 5:5 - Syntax error, unexpected T_CASE on line 5
https://psalm.dev/r/9b5935609d
<?php

enum Status: int
{
    case FOO;
    case BAR = 42;
}

enum_exists(Status::FOO); //returns true
enum_exists(Status::BAZ); //returns false
Psalm output (using commit 916b098):

ERROR: ParseError - 3:6 - Syntax error, unexpected T_STRING on line 3

ERROR: ParseError - 5:5 - Syntax error, unexpected T_CASE on line 5

@weirdan
Copy link
Collaborator

weirdan commented Sep 5, 2021

Enum are objects

Can you elaborate? It seems Psalm already knows that: https://psalm.dev/r/a57335dce7?php=8.1

@psalm-github-bot
Copy link

psalm-github-bot bot commented Sep 5, 2021

I found these snippets:

https://psalm.dev/r/a57335dce7
<?php

enum Status: int
{
    case FOO = 1;
    case BAR = 2;
}


$statusC = Status::BAR;

if ($statusC instanceof Status) {}
Psalm output (using commit 916b098):

ERROR: RedundantCondition - 12:5 - Type enum(Status::BAR) for $statusC is always Status

@weirdan weirdan mentioned this issue Sep 5, 2021
15 tasks
@niconoe-
Copy link

niconoe- commented Sep 5, 2021

Enum are objects

Can you elaborate? It seems Psalm already knows that: https://psalm.dev/r/a57335dce7?php=8.1

Sorry, I didn't know I could test against PHP 8.1 using the GET parameter, I was tesing on PHP 8.0 (or default version).
You can remove this then, as it's already implemented.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/a57335dce7
<?php

enum Status: int
{
    case FOO = 1;
    case BAR = 2;
}


$statusC = Status::BAR;

if ($statusC instanceof Status) {}
Psalm output (using commit 916b098):

ERROR: RedundantCondition - 12:5 - Type enum(Status::BAR) for $statusC is always Status

@SpartakusMd
Copy link

Overriding final class constants should return error
https://psalm.dev/r/867638ab8b?php=8.1

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/867638ab8b
<?php

class A
{
    final const BAR = 'BAZ';
}

class B extends A {
    const BAR = 'BUZZ';
}
Psalm output (using commit f35df42):

INFO: UnusedClass - 8:7 - Class B is never used

@muglug
Copy link
Collaborator

muglug commented Oct 26, 2021

In PHP 8.1 there are now deprecation notices when extending a class without specifying its types.

Running

class Foo implements Iterator {
    public function current() {
        return 'foo';
    }
    
    public function next() {}
    
    public function key() {}
    
    public function valid() {
        return true;
    }
    
    public function rewind() {}
}

produces

Deprecated: Return type of Foo::current() should either be compatible with Iterator::current(): mixed, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /in/To1Ym on line 6

Deprecated: Return type of Foo::next() should either be compatible with Iterator::next(): void, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice in /in/To1Ym on line 10

<... more ...>

I've created a branch to add a lot of these return types: https://github.com/vimeo/psalm/compare/muglug-hard-typed-generic-stubs

I think some extra care will be needed when scanning in PHP < 8.1 so that we don't force implementers to use those types (especially where they're not supported, like mixed.

@kkmuffme
Copy link
Contributor

kkmuffme commented Nov 9, 2021

Those things that are merged already (outlined in OP), are already available when setting phpVersion="8.1" now?

@weirdan
Copy link
Collaborator

weirdan commented Nov 9, 2021

@kkmuffme yes. Some may be available in master only though, but most are already a part of some release.

AndrolGenhald added a commit to AndrolGenhald/psalm that referenced this issue Feb 13, 2022
Add InvalidClassConstType issue as alternative to LessSpecificClassConstType when type isn't contravariant.
Handle final class consts (vimeo#6395).
Use double quotes for types in class const issues.
@GrahamCampbell
Copy link

Match statements seem to be partially supported, only. In a switch statement, if I switch on the type of an object, psalm understands it has that type in the case blocks. For a match statement, it does not.

@orklah
Copy link
Collaborator Author

orklah commented Apr 11, 2022

Could you provide a snippet please?

@GrahamCampbell
Copy link

Actually, both have issues:

<?php

class Foo
{
    function eg1(): bool { return false; }
}

class Bar
{
    function eg2(): bool { return false; }
}

class Baz
{
    function eg2(): bool { return false; }
}

function f(Foo|Bar|Baz $x): bool
{
    switch ($x::class) {
        case Foo::class:
            return $x->eg1();
        case Bar::class:
        case Baz::class:
            return $x->eg2();
    }
}

https://psalm.dev/r/98e758a602

<?php

class Foo
{
    function eg1(): bool { return false; }
}

class Bar
{
    function eg2(): bool { return false; }
}

class Baz
{
    function eg2(): bool { return false; }
}

function f(Foo|Bar|Baz $x): bool
{
    return match ($x::class) {
        Foo::class => $x->eg1(),
        Bar::class, Baz::class => $x->eg2(),
    };
}

https://psalm.dev/r/7d1f1bd52b

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/98e758a602
<?php

class Foo
{
    function eg1(): bool { return false; }
}

class Bar
{
    function eg2(): bool { return false; }
}

class Baz
{
    function eg2(): bool { return false; }
}

function f(Foo|Bar|Baz $x): bool
{
    switch ($x::class) {
        case Foo::class:
            return $x->eg1();
        case Bar::class:
        case Baz::class:
            return $x->eg2();
    }
}
Psalm output (using commit 916fddb):

ERROR: InvalidReturnType - 18:29 - Not all code paths of f end in a return statement, return type bool expected
https://psalm.dev/r/7d1f1bd52b
<?php

class Foo
{
    function eg1(): bool { return false; }
}

class Bar
{
    function eg2(): bool { return false; }
}

class Baz
{
    function eg2(): bool { return false; }
}

function f(Foo|Bar|Baz $x): bool
{
    return match ($x::class) {
        Foo::class => $x->eg1(),
        Bar::class, Baz::class => $x->eg2(),
    };
}
Psalm output (using commit 916fddb):

ERROR: PossiblyUndefinedMethod - 22:39 - Method Foo::eg2 does not exist

INFO: MixedReturnStatement - 20:12 - Possibly-mixed return value

INFO: MixedReturnStatement - 20:12 - Could not infer a return type

INFO: MixedInferredReturnType - 18:29 - Could not verify return type 'bool' for f

@orklah
Copy link
Collaborator Author

orklah commented Apr 11, 2022

Yeah, explaining to Psalm that mapping $x::class values with specific literals will make $x type change is possibly non-trivial, though it works with conditionals: https://psalm.dev/r/ae73672517

Could you create a new issue for that? It's not really related to PHP 8.1 if switch has the same issue

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/ae73672517
<?php

class Foo
{
    function eg1(): bool { return false; }
}

class Bar
{
    function eg2(): bool { return false; }
}

class Baz
{
    function eg2(): bool { return false; }
}

function f(Foo|Bar|Baz $x): bool
{
    if ($x::class === Foo::class) {
            return $x->eg1();
    }
}
Psalm output (using commit 916fddb):

ERROR: InvalidReturnType - 18:29 - Not all code paths of f end in a return statement, return type bool expected

@GrahamCampbell
Copy link

Switch has a different issue. It understands the type, but doesn't understand the matching is complete.

@kkmuffme
Copy link
Contributor

kkmuffme commented Aug 5, 2022

Pending #8374

@weirdan
Copy link
Collaborator

weirdan commented Mar 3, 2023

This is pretty much done.

@weirdan weirdan closed this as completed Mar 3, 2023
@weirdan weirdan unpinned this issue Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants