Skip to content

Conversation

@azjezz
Copy link
Contributor

@azjezz azjezz commented Jul 28, 2021

@pure annotation is mainly used by psalm to mark pure functions.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi! Please squash both commits together.

What is the reason for Psalm supporting @pure in addition to @psalm-pure? Can it really claim that annotation for itself, as opposed to PHPStan (for instance)?

@azjezz
Copy link
Contributor Author

azjezz commented Aug 4, 2021

done.

This issue appears when using azjezz/psl library, and PSL has a CS rule that forbids Psalm specific annotations, in hopes that other tools in the future understand these ( e.g if PhpStan would to add pure function support, PSL wouldn't need to add a new annotation ) - ref: https://github.com/azjezz/psl/blob/1.8.x/src/Psl/Collection/Vector.php#L47

@malarzm
Copy link
Member

malarzm commented Aug 4, 2021

PSL has a CS rule that forbids Psalm specific annotations

Maybe nitpicking, but linked file has other @psalm- annotations: @psalm-readonly and @psalm-mutation-free

@greg0ire
Copy link
Member

greg0ire commented Aug 4, 2021

This issue appears when using azjezz/psl library, and PSL has a CS rule that forbids Psalm specific annotations, in hopes that other tools in the future understand these ( e.g if PhpStan would to add pure function support, PSL wouldn't need to add a new annotation ) - ref: https://github.com/azjezz/psl/blob/1.8.x/src/Psl/Collection/Vector.php#L47

Ah yes, that library I should really try out someday 🙂

When I see the rest of the file, there are mostly annotations that are unlikely to be picked by any other tools. There are also annotations that are attributed to PhpStan but work with Psalm too

Maybe using tool names as consts is a bad idea because of this, in some cases? How about having

    private const StaticAnalysis = [
        // PhpStan, Psalm 
        'extends' => true,
        'implements' => true,
        'template' => true,
        'use' => true,

       // Psalm
        'pure' => true,
        'immutable' => true,
    ];

?

@azjezz
Copy link
Contributor Author

azjezz commented Aug 5, 2021

Maybe nitpicking, but linked file has other @psalm- annotations: @psalm-readonly and @psalm-mutation-free

because @mutation-free and @readonly don't work with psalm IIRC.

@azjezz
Copy link
Contributor Author

azjezz commented Aug 5, 2021

Maybe using tool names as consts is a bad idea because of this, in some cases? How about having

That sounds like a good idea 👍

@azjezz azjezz force-pushed the patch-1 branch 2 times, most recently from 7b34453 to 810557b Compare August 5, 2021 07:51
Signed-off-by: azjezz <azjezz@protonmail.com>
@azjezz azjezz changed the title Add @pure to ignored annotation names. Add psalm annotations to ignored annotation name Aug 5, 2021
@greg0ire greg0ire merged commit 5b668ae into doctrine:1.13.x Aug 5, 2021
@greg0ire
Copy link
Member

greg0ire commented Aug 5, 2021

Thanks @azjezz !

@greg0ire greg0ire added this to the 1.13.2 milestone Aug 5, 2021
@azjezz azjezz deleted the patch-1 branch August 5, 2021 20:50
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