Phpstan: Adding `@var` Should not be needed when the property is set by the constructor

[lyrixx]

Hello

Summary of a problem or a feature request

With this code:

class Foobar
{
    private $httpClient;

    public function __construct(HttpClient $httpClient)
    {
        $this->httpClient = $httpClient;
    }

    public function fetchOrientationToken()
    {
        $this->httpClient->methodDoesNotExist();
    }
}

There are no errors

With this code:

class Foobar
{
    /** @var HttpClient */ // <----------- the difference is here 
    private $httpClient;

    public function __construct(HttpClient $httpClient)
    {
        $this->httpClient = $httpClient;
    }

    public function fetchOrientationToken()
    {
        $this->httpClient->methodDoesNotExist();
    }
}

A violation is raised

Code snippet that reproduces the problem

https://phpstan.org/r/a6aeb2711d867c2bda6e388ee515e9c4

Expected output

A violation should be raised in both situation

[JanTvrdik]

That's expected behavior. Solving this would require analyzing each class twice.

[lyrixx]

That's too bad. I really don't want to add @var XXX on each properties. This adds nothing for other developers, nor IDE, nor PHP.

Is fixing this something you want to consider ?

[iluuu1994]

@lyrixx It does. The type of the property can change at any time, it is not fixed after the constructor. Adding a @var pins the type and prevents you from setting it to something it shouldn't be.

[lyrixx]

It's just PHP Doc :)

You made an assumption here: It's not because @var is added that it could not change... So we could make the same assumption with the constructor.

[iluuu1994]

It's just PHP Doc :)

No, if you're using PHPStan it's not just PHPDoc ;)
PHPStan will complain.

[iluuu1994]

Here's what I mean.

[lyrixx]

Yes, It tested it too :)

See the first case : https://phpstan.org/r/1955eb69ba057ce8dfdb7e814368f6ff

[lyrixx]

Could you consider adding a configuration option to "virtually" add @var XXX on top of each properties that is a constructor arguments?

It would be an opt-in option of course.

[iluuu1994]

By the way, this has been proposed before:

https://github.com/phpstan/phpstan/issues/541
https://github.com/phpstan/phpstan/issues/615

@honzatrtik Did you ever open-source the extension mentioned here?

[dunglas]

Symfony’s coding standards state to not add the PHPDoc to a property if it can be inferred from the constructor, it is also the default configuration of PHP CS Fixer and it is considered a best practice by some.

It would be nice to have this option at least for interoperablility with other tools in the ecosystem.

[ondrejmirtes]

I think that you should add @vars to your properties, it's future-proofing for PHP 7.4 where you will be able to type your properties natively. I'm quite sure that there will be a fixable sniff in slevomat/coding-standard that will move your typehints from phpDoc to native once 7.4 is out :)

[lyrixx]

I understand you don't want to add this option. I will live with it but I really wanted this feature.
More over as @dunglas said, we remove all theses PHP Doc in Symfony, and it's the same for the SF community.

Do you know if we can build a plugin to do that?

[ondrejmirtes]

You might try to do something like I suggested in https://github.com/phpstan/phpstan/issues/615#issuecomment-346997092 but it's gonna be a big hack :)

[JanTvrdik]

@lyrixx Would you expect the following code to report error?
https://phpstan.org/r/4315bb3d9a7f14278be47f2ffb775344

[lyrixx]

I would say yes

[ondrejmirtes]

What about this one?

https://phpstan.org/r/671b8ff43e7ac67a20058866de81d7f1

And this one?

https://phpstan.org/r/f0e0a1d235be96f4b2f0cef60c4fa2e5

You see, it's not that easy...

[lyrixx]

Sure it's not easy. that's why I think the option should be opt-in and stupid. It's juste a 1-1 mapping between the property name, and the property given through the constructor

[lyrixx]

Here we go: https://gist.github.com/lyrixx/72d0fddae44b07bd32b31b87a27afa6c

Thanks for the help and BTW your sofware is really flexible. Very easy to hook into :+1: Good job

[ondrejmirtes]

The extension could be written in a much better way. For example you don't provide inference for scalar parameters. Also I'd shy away from using reflection and setAccessible.

[lyrixx]

I'm quite new to php stan internal ! That's why I would like to have this feature in the core :)

[ondrejmirtes]

It's never gonna be in the core - the direction of PHP itself is different - typehint your properties properly :)

[lyrixx]

Sure, I really want to use PHP 7.4, and I will use typed properties for sure :)
But I have to wait a bit more than one year !

[ondrejmirtes]

My point is that you can write them today in @var, get the same benefits,
and transform them automatically to native in a year.

On Wed, 31 Oct 2018 at 19:09, Grégoire Pineau [e-mail]
wrote:

Sure, I really want to use PHP 7.4, and I will use typed properties for
sure :)
But I have to wait a bit more than one year !

—
You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub
https://github.com/phpstan/phpstan/issues/1562#issuecomment-434786551,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGZuJZyKvj6W8i0frUt8t4scXdwZAW9ks5uqec7gaJpZM4YD27U
.

>

Ondřej Mirtes

[adrienbrault]

@ondrejmirtes Is there a way to issue some kind of warning/error if PHPStan doesn't know the type of something ?

[ondrejmirtes]

Yes, there’s a rule in strict-rules for that.

On Tue, 6 Nov 2018 at 00:13, Adrien Brault [e-mail] wrote:

@ondrejmirtes https://github.com/ondrejmirtes Is there a way to issue
some kind of warning/error if PHPStan doesn't know the type of something ?

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
https://github.com/phpstan/phpstan/issues/1562#issuecomment-436071291,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAGZuFgGXJCHR7jNzT9AhCQtn2-oqqIRks5usMYcgaJpZM4YD27U
.

>

Ondřej Mirtes

[ondrejmirtes]

Good news! PHPStan 0.11.10 includes support for inferring private property type from constructor! https://github.com/phpstan/phpstan/releases/tag/0.11.10

[lyrixx]

Thanks a lot