-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Comments
Add support to use short forms of type keywords#3139
Add support to use short forms of type keywords#3139filips123 wants to merge 1 commit intosquizlabs:masterfrom
Conversation
This adds support to use short forms of type keywords in comments for function parameters and returns and variable types. It fixes #1864 and fixes #1434.
It adds useShortTypes property, which is by default false, to Squiz.Commenting.FunctionComment and Squiz.Commenting.VariableComment to allow users to use short type checks. If it is enabled, it will force short types in comments and suggest them when long ones are used.
P.S. I would appreciate if you add hacktoberfest-accepted label to this PR, so it will count for this year's Hacktoberfest.
1d2170d to
87a6ef3
Compare
| * @var boolean | ||
| */ | ||
| public $useShortTypes = false; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this empty line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't request changes which are in violation with the coding standard used by the library.
| 'resource', | ||
| 'callable', | ||
| ]; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this empty line.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't request changes which are in violation with the coding standard used by the library.
| * | ||
| * @var boolean | ||
| */ | ||
| public $useShortTypes = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this default to true due to PSR-12?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this default to true due to PSR-12?
Problem is that this might be considered as a breaking change. I'm not sure how this was done in the past, but maybe it should remain false in PHPCS 3.x and changed to true in 4.x.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand. In the mean time it's easy to just change that property in the config file.
|
Any chance to get this merged? Short return types are the way to go, especially because it does not comply to the rules of PSR-12 currently. "Short form of type keywords MUST be used i.e. bool instead of boolean, int instead of integer etc.". |
|
Until such time as this can be merged, I've added a workaround here: https://packagist.org/packages/zebra-north/phpcs-short-types It's not a proper fix, it would be great if this PR could be merged. |
|
I would love for this to be merged, we are using PHPStorm and it automatically (and correctly) uses short types which we have to manually update to pass our code sniffers |
|
Merging this in would be useful |
|
Would definitely love to see this merged and released! Super odd to have code type hints be one thing and docblock types not enforced to match. |
|
@Justintime50 it is very unlikely that this will be merged here. See #3932. |
|
Seeing as it's unlikely this makes it into PHPCS, I've created a sniff that enforces short scalar type names with auto-fix as well: https://github.com/Justintime50/phpcs-short-scalar-types. This can be used in lieu of the missing 1st party support. This sniff will error if Hope it's helpful for future travelers! |