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
PhpdocToPropertyTypeFixer - introduction #4592
PhpdocToPropertyTypeFixer - introduction #4592
Conversation
2e8dfd2
to
e1fbd78
Compare
e1fbd78
to
a84d506
Compare
a84d506
to
f1948b6
Compare
👍 looking very nice :) some minors I found;
which seems to be because;
secondly, when there are a of candidate the fixer doesn't catch them all in the first run. Test sample; <?php
class ActivityLog
{
/**
* @var int
*/
private $id;
/**
* @var string
*/
private $id1;
/**
* @var string
*/
private $id2;
/**
* @var int
*/
private $id3;
/**
* @var string
*/
private $id4;
/**
* @var string
*/
private $id5;
/**
* @var string
*/
private $id6;
/**
* @var string
*/
private $id7;
/**
* @var int
*/
private $id8;
/**
* @var string
*/
private $id9;
/**
* @var int|null
*/
private $id410;
} |
Thanks for the test cases, I fixed them :) |
7225aa6
to
b3d57cc
Compare
+1 on that. Any estimation on when this can be merged? |
@julienfalque can you split this PR to one adding new fixers and 2nd (targeted to 2.15) with refactoring of |
@kubawerlos Is #4591 what you want (this PR depends on it)? It currently targets 2.16 though. |
@julienfalque yes, it is. Shame on me. |
b726858
to
7b00b95
Compare
Sorry for the conflicts @julienfalque - can you solve them? I believe thee review would be quite straightforward here. |
7b00b95
to
16be9fe
Compare
Rebased #4591 and here, please review :) |
I'd actually question if this belongs in php-cs-fixer. It's more a job for a refactoring tool like Rector, rather than a code style fixing tool. |
We already to a lot of refactoring in PHP-CS-Fixer. Moreover, PHP_CodeSniffer already does it: https://github.com/slevomat/coding-standard/blob/master/SlevomatCodingStandard/Sniffs/TypeHints/PropertyTypeHintSniff.php#L260-L296 |
I don't think we need to add more. It only adds more upgrading work every time a new version of PHP comes out. |
I understand your point: it would be good for maintenance to have a fixer tool decoupled from PHP version, and maybe other one (like Rector) for the upgrade path. But we are a bit late for such approach, aren't we? |
16be9fe
to
19752fa
Compare
@julienfalque please rebase and after that RTM 👍 |
@SpacePossum #4591 should be merged first. |
7f0cbc5
to
0382be5
Compare
Thank you @julienfalque. |
Thank you @keradus! |
thanks this will be useful |
This PR includes #4591 and depends on #4593 which should both be merged first.