[PHP 7.4] Add @var removal to TypedPropertyRector

[TomasVotruba]

Closes #2367 propperly

Before

 /**
  * @var int
  */
-public $count;
+public int $count;

Now

-/**
- * @var int
- */
-public $count;
+public int $count;

/cc @brendt @enumag @collapso @mallardduck @ruudk

[ruudk]

Too bad that I did the work already in PHP-CS-Fixer/PHP-CS-Fixer#4713... Any idea how I can get that merged?

[TomasVotruba]

@ruudk That's ok, both tools have their user scope. I know many people who use CS only and many more who use Rector only.

Also, you give me next piece of motivation to make this happen. In older version of Rector this would be really complicatd. But with new API around php doc it was quite easy. So thank you as well 👍

Any idea how I can get that merged?

No idea, I don't contribute there.

[TomasVotruba]

@ruudk Btw, next time you can contribute here first. I do merge almost any feature improvements.

[ruudk]

It was more that you said in the other issue it belongs in CS fixer, that’s why I did the work there , and that’s why I was surprised by this PR. But in the end, it does not matter, great to have this automatically fixable by either CS fixer or Rector ☺️

[TomasVotruba]

At first yes, because it seemed as extra work not worth doing for me. Working PR with test is hard to reject :)

I do change my mind as a learning process, based on feedback and given data.

[enumag]

What exactly is this condition for? There are more types like these - Iterator, Generator, ext-ds types, Doctrine collections, some SPL types... trying to white-list them for some special handling doesn't make much sense to me.

[TomasVotruba]

These all need to be listed there then. For phpdoc-parser it's just a string.

[enumag]

There is no way to whitelist everything. What is the purpose of this?

[TomasVotruba]

Only those that have type in object fo PHPStan.

The mapPHPStanPhpDocTypeNodeToPHPStanType() method converts doc type to phpstan type

[TomasVotruba]

If you're able to make it simpler and more robust, that would be great

[enumag]

Only those that have type in object fo PHPStan.

Can you rephrase that? I don't understand...

Would something like === array || === iterable || is_subclass_of(Traversable) work?

[TomasVotruba]

Can you rephrase that? I don't understand...

Sure, this is phpdoc-paser doc type:

@var int

To use in the Rector/PHPStan, we need to convert it to PHPStan type:

$integerType = new IntegerType();

That's all :)

Would something like === array || === iterable || is_subclass_of(Traversable) work?

Hard to tell, testing needs to be done, as this is 3rd party code we convert

[TomasVotruba]

Let's address in new PR

[TomasVotruba]

Thanks for the feedback!

Feel free to add more failing cases in new issues. Merging as it is, so it can be used out in the wild

[ruudk]

I just pulled in master to test this out, but this rector only removes the superfluous PHPDoc when the property does not have a type but does have a phpdoc:

-    /**
-     * @var Currency
-     */
-    public $fromCurrency;
+    public Currency $fromCurrency;

but it ignores this code:

    /**
     * @var Currency
     */
    public Currency $toCurrency;

Since I already ran the TypedPropertyRector before on my code, all my properties now have types, with phpdocs. Any idea how to solve that with Rector?

[enumag]

@ruudk I'd recommend solving that with php-cs-fixer (with your patch).

[TomasVotruba]

If you send PR to add this feature, I'd be happy to merge it