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

Add psalm annotations to ignored annotation name #422

Merged
merged 1 commit into from
Aug 5, 2021

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