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

Feature completeness NewScalarTypeDeclarations sniff (code review). #168

Merged

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Aug 14, 2016

This adds a number of additional checks to the NewScalarTypeDeclarations sniff.

  • Check that type hints are not being used in pre-PHP 5.0 code.
  • Check for valid usage of the array and callable type hints.
  • Check for invalid type hints which are typical mistakes.
  • Check that the self type hint is only used within the class scope.

Ref: http://php.net/functions.arguments#functions.arguments.type-declaration

Also:

  • The description key in the $newTypes array did not actually add any value, so I've removed it.
  • Minor tweaks to the original error message text.
    • Was along the lines of: "int type is not present in PHP version 5.6 or earlier".
    • Now: "'int' type declaration is not present in PHP version 5.6 or earlier".

Includes unit tests covering all changes.

This PR also adds two new utility methods to the abstract PHPCompatibility_Sniff class.
One is a copy of an existing method from PHPCS with some minor tweaks which have been send in as a PR to PHPCS. That PR is expected to be merged for the 2.7.0 release. Unit tests for this method are available within the PHPCS test suite.
The other is a new PHPCompatibility native method inClassScope() and is accompanied by unit tests for the method.

Similarly to an earlier PR, the file/sniff name does now no longer cover what the sniff actually does. File and sniff renaming advised.

@jrfnl jrfnl force-pushed the feature/improve-type-declaration branch from 4c3332a to 5d5fd8d Compare August 14, 2016 22:41
This adds a number of additional checks to the `NewScalarTypeDeclarations` sniff.
* Check that type hints are not being used in pre-PHP 5.0 code.
* Check for valid usage of the `array` and `callable` type  hints.
* Check for invalid type hints which are typical mistakes.
* Check that the `self` type hint is only used within the class scope.

Also:
* The `description` key in the `$newTypes` array did not actually add any value, so I've removed it.
* Minor tweaks to the original error message text.
   Was along the lines of: _"int type is not present in PHP version 5.6 or earlier"_.
   Now: _"'int' type declaration is not present in PHP version 5.6 or earlier"_.

Includes unit tests covering all changes.

This PR also adds two new utility methods to the abstract `PHPCompatibility_Sniff` class.
One is a copy of an existing method from PHPCS with some minor tweaks which have been send in as a PR to PHPCS. That PR is expected to be merged for the 2.7.0 release. Unit tests for this method are available within the PHPCS test suite.
The other is a new PHPCompatibility native method `inClassScope` and is accompanied by unit tests for the method.

Similarly to an earlier PR, the file/sniff name does now no longer cover what the sniff actually does. File and sniff renaming advised.
@jrfnl jrfnl force-pushed the feature/improve-type-declaration branch from 5d5fd8d to 75f7799 Compare August 16, 2016 13:34
@wimg wimg merged commit 4143ec6 into PHPCompatibility:master Aug 16, 2016
@jrfnl jrfnl deleted the feature/improve-type-declaration branch August 17, 2016 02:58
@jrfnl jrfnl added this to the 7.0.3 milestone Apr 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants