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 @ignore-var and @psalm-ignore-var #5488

Merged
merged 2 commits into from
Mar 27, 2021
Merged

Add @ignore-var and @psalm-ignore-var #5488

merged 2 commits into from
Mar 27, 2021

Conversation

sj-i
Copy link
Contributor

@sj-i sj-i commented Mar 26, 2021

This annotation is used to ignore the @var annotation written in the same docblock.

Background

To take advantage of the IDE's auto-completion, you may sometimes want to use explicit @var annotations even when psalm can infer the type well enough. This weakens the effectiveness of type checking in many cases since the explicit @var annotation overrides the types inferred by psalm.

See this example for more detail.
https://psalm.dev/r/c19d749dbe

What is changed if this PR is merged

Psalm ignores the @var annotation which is co-located with @psalm-ignore-var. Then IDEs that don't fully understand complex types like generics can use the manually specified types with @var for auto-completion, while psalm can still use its own inferred types for type checking.

Disclaimer

I'm not confident about my English skills. The sentences may seem odd to you. I know you aren't my English teacher, but if any wordings of the added doc or this PR sounds unnatural, please feel free to correct them. Then I can train myself better.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/c19d749dbe
<?php
class A { public int $a = 0; }
class B { public int $b = 0; }

/** @template T */
interface Generic
{
    /** @psalm-return iterable<T> */
    public function fetchAll();
}

/** @param Generic<A> $ag */
function (Generic $ag): void {
    // psalm itself can infer the type well enough
    $collection = $ag->fetchAll();
    foreach ($collection as $_) {
        /** @psalm-trace $_ */
    }

    // need this @var to get the power of auto-completion in phpstorm
    /** @var A[] */
    $collection = $ag->fetchAll();
    foreach ($collection as $item) {
        echo $item->a; // auto-completion can be used here
        /** @psalm-trace $item */
    }

    // but explicitly declared @var hides the programmer's mistake
    /** @var B[] */
    $collection = $ag->fetchAll();
    foreach ($collection as $item) {
        echo $item->b; // the item is actually A, it has no member named b
        /** @psalm-trace $item */
    }

    // if psalm ignores @var which co-located with a specific annotation,
    // programmers can annotate it for auto-completion,
    // while psalm still catch type errors.
    /**
     * @var B[]
     * @psalm-ignore-var
     */
    $collection = $ag->fetchAll();
    foreach ($collection as $item) {
        echo $item->b; // if @var is ignored, this error can be caught in psalm
        /** @psalm-trace $item */
    }

};
Psalm output (using commit 9f74676):

ERROR: InvalidDocblock - 43:5 - Unrecognised annotation @psalm-ignore-var

INFO: Trace - 17:0 - $_: A

INFO: Trace - 25:0 - $item: A

INFO: Trace - 33:0 - $item: B

ERROR: UndefinedPropertyFetch - 45:14 - Instance property A::$b is not defined

INFO: MixedArgument - 45:14 - Argument 1 of echo cannot be mixed, expecting string

INFO: Trace - 46:0 - $item: A

@weirdan
Copy link
Collaborator

weirdan commented Mar 26, 2021

I would also suggest to file an issue with JetBrains (https://youtrack.jetbrains.com/issues/WI) to implement support for @phpstorm- vendor prefix.

@muglug muglug merged commit 19554de into vimeo:master Mar 27, 2021
@muglug
Copy link
Collaborator

muglug commented Mar 27, 2021

Thanks, this is great!

cc @ondrejmirtes @TysonAndre

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.

None yet

3 participants