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

Use of positive-int works for @psalm-param but @psalm-var not #6762

Closed
brzuchal opened this issue Oct 27, 2021 · 5 comments · Fixed by #6764
Closed

Use of positive-int works for @psalm-param but @psalm-var not #6762

brzuchal opened this issue Oct 27, 2021 · 5 comments · Fixed by #6764

Comments

@brzuchal
Copy link

When using positive-int in ctor argument promoted property DocBlock tag @psalm-var Pslam complains with LessSpecificReturnType in getter but not if the DocBlock on ctor instead of property use @psalm-param.
Why param tag is considered valid when var not?

https://psalm.dev/r/c4d7b97301
https://psalm.dev/r/223579e2f5

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/c4d7b97301
<?php
final class UserRole
{
    public function __construct(
        /** @psalm-var positive-int */
        protected int $id,
    ) {
    }

    /**
     * @psalm-return positive-int
     */
    public function getId(): int
    {
        return $this->id;
    }
}
Psalm output (using commit 526debb):

INFO: LessSpecificReturnStatement - 15:16 - The type 'int' is more general than the declared return type 'positive-int' for UserRole::getId

INFO: MoreSpecificReturnType - 11:22 - The declared return type 'positive-int' for UserRole::getId is more specific than the inferred return type 'int'
https://psalm.dev/r/223579e2f5
<?php
final class UserRole
{
    /** @psalm-param positive-int $id*/
    public function __construct(
        protected int $id,
    ) {
    }

    /**
     * @psalm-return positive-int
     */
    public function getId(): int
    {
        return $this->id;
    }
}
Psalm output (using commit 526debb):

No issues!

@orklah
Copy link
Collaborator

orklah commented Oct 27, 2021

Looks like there is no currently accepted syntax to directly document promoted param type
https://psalm.dev/r/9f3fa7459c

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/9f3fa7459c
<?php
final class UserRole
{
    public function __construct(
        /** @psalm-param stdClass */
        protected $id,
        /** @psalm-var stdClass */
        protected $id2,
        /** @psalm-param stdClass $id3 */
        protected $id3,
        /** @psalm-var stdClass $id4 */
        protected $id4,
    ) {
    }


    public function getId(): void
    {
        /** @psalm-trace $this->id */;
        /** @psalm-trace $this->id2 */;
        /** @psalm-trace $this->id3 */;
        /** @psalm-trace $this->id4 */;
    }
}
Psalm output (using commit 526debb):

INFO: MissingParamType - 6:19 - Parameter $id has no provided type

INFO: MissingParamType - 8:19 - Parameter $id2 has no provided type

INFO: MissingParamType - 10:19 - Parameter $id3 has no provided type

INFO: MissingParamType - 12:19 - Parameter $id4 has no provided type

INFO: Trace - 19:38 - $this->id: mixed

INFO: Trace - 20:39 - $this->id2: mixed

INFO: Trace - 21:39 - $this->id3: mixed

INFO: Trace - 22:39 - $this->id4: mixed

@orklah
Copy link
Collaborator

orklah commented Oct 27, 2021

@weirdan Do we want to be able to document the property like that ? (PHPStan allows this: https://phpstan.org/r/b6c12435-8a6c-4253-98a6-0d840597fa68). If we do, would you expect to be able to document separately the parameter and the property (like we have when we don't use promoted properties) or would you expect both type to be bound?

If you expect the latter, what should we do if param type and property type don't match? (PHPStan doesn't care and take just the property type: https://phpstan.org/r/228c951d-6b89-4307-a6cd-a46c32d10687)

@weirdan
Copy link
Collaborator

weirdan commented Oct 27, 2021

@orklah I'd expect Psalm to complain when both @var and @param are specified for a promoted property, and behave the same as PHPStan in other regards.

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

Successfully merging a pull request may close this issue.

3 participants