Skip to content

Commit

Permalink
Feature completeness NewScalarTypeDeclarations sniff (code review).
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@jrfnl
jrfnl committed Aug 14, 2016
1 parent db2e39c commit 4c3332a
Show file tree
Hide file tree
Showing 6 changed files with 530 additions and 40 deletions.
210 changes: 210 additions & 0 deletions Sniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,39 @@ public function getFunctionCallParameterCount(PHP_CodeSniffer_File $phpcsFile, $
}


/**
* Verify whether a token is within a class scope.
*
* @param PHP_CodeSniffer_File $phpcsFile The file being scanned.
* @param int $stackPtr The position of the token.
*
* @return bool True if within class scope, false otherwise.
*/
public function inClassScope(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

// Check for the existence of the token.
if (isset($tokens[$stackPtr]) === false) {
return false;
}

// No conditions = no scope.
if (empty($tokens[$stackPtr]['conditions'])) {
return false;
}

// Check for class scope.
foreach ($tokens[$stackPtr]['conditions'] as $pointer => $type) {
if ($type === T_CLASS) {
return true;
}
}

return false;
}


/**
* Returns the fully qualified class name for a new class instantiation.
*
Expand Down Expand Up @@ -566,4 +599,181 @@ public function getDeclaredNamespaceName(PHP_CodeSniffer_File $phpcsFile, $stack
return $namespaceName;
}


/**
* Returns the method parameters for the specified T_FUNCTION token.
*
* Each parameter is in the following format:
*
* <code>
* 0 => array(
* 'name' => '$var', // The variable name.
* 'pass_by_reference' => false, // Passed by reference.
* 'type_hint' => string, // Type hint for array or custom type
* )
* </code>
*
* Parameters with default values have an additional array index of
* 'default' with the value of the default as a string.
*
* {@internal Duplicate of same method as contained in the `PHP_CodeSniffer_File`
* class, but with some improvements which were only introduced in PHPCS 2.7.
* Once the minimum supported PHPCS version for this sniff library goes beyond
* that, this method can be removed and calls to it replaced with
* `$phpcsFile->getMethodParameters($stackPtr)` calls.}}
*
* @param PHP_CodeSniffer_File $phpcsFile Instance of phpcsFile.
* @param int $stackPtr The position in the stack of the T_FUNCTION token
* to acquire the parameters for.
*
* @return array
* @throws PHP_CodeSniffer_Exception If the specified $stackPtr is not of
* type T_FUNCTION.
*/
public function getMethodParameters(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
{
$tokens = $phpcsFile->getTokens();

// Check for the existence of the token.
if (isset($tokens[$stackPtr]) === false) {
return false;
}

if ($tokens[$stackPtr]['code'] !== T_FUNCTION) {
throw new PHP_CodeSniffer_Exception('$stackPtr must be of type T_FUNCTION');
}

$opener = $tokens[$stackPtr]['parenthesis_opener'];
$closer = $tokens[$stackPtr]['parenthesis_closer'];

$vars = array();
$currVar = null;
$paramStart = ($opener + 1);
$defaultStart = null;
$paramCount = 0;
$passByReference = false;
$variableLength = false;
$typeHint = '';

for ($i = $paramStart; $i <= $closer; $i++) {
// Check to see if this token has a parenthesis or bracket opener. If it does
// it's likely to be an array which might have arguments in it. This
// could cause problems in our parsing below, so lets just skip to the
// end of it.
if (isset($tokens[$i]['parenthesis_opener']) === true) {
// Don't do this if it's the close parenthesis for the method.
if ($i !== $tokens[$i]['parenthesis_closer']) {
$i = ($tokens[$i]['parenthesis_closer'] + 1);
}
}

if (isset($tokens[$i]['bracket_opener']) === true) {
// Don't do this if it's the close parenthesis for the method.
if ($i !== $tokens[$i]['bracket_closer']) {
$i = ($tokens[$i]['bracket_closer'] + 1);
}
}

switch ($tokens[$i]['code']) {
case T_BITWISE_AND:
$passByReference = true;
break;
case T_VARIABLE:
$currVar = $i;
break;
case T_ELLIPSIS:
$variableLength = true;
break;
case T_ARRAY_HINT:
case T_CALLABLE:
$typeHint = $tokens[$i]['content'];
break;
case T_SELF:
case T_PARENT:
case T_STATIC:
// Self is valid, the others invalid, but were probably intended as type hints.
if (isset($defaultStart) === false) {
$typeHint = $tokens[$i]['content'];
}
break;
case T_STRING:
// This is a string, so it may be a type hint, but it could
// also be a constant used as a default value.
$prevComma = false;
for ($t = $i; $t >= $opener; $t--) {
if ($tokens[$t]['code'] === T_COMMA) {
$prevComma = $t;
break;
}
}

if ($prevComma !== false) {
$nextEquals = false;
for ($t = $prevComma; $t < $i; $t++) {
if ($tokens[$t]['code'] === T_EQUAL) {
$nextEquals = $t;
break;
}
}

if ($nextEquals !== false) {
break;
}
}

if ($defaultStart === null) {
$typeHint .= $tokens[$i]['content'];
}
break;
case T_NS_SEPARATOR:
// Part of a type hint or default value.
if ($defaultStart === null) {
$typeHint .= $tokens[$i]['content'];
}
break;
case T_CLOSE_PARENTHESIS:
case T_COMMA:
// If it's null, then there must be no parameters for this
// method.
if ($currVar === null) {
continue;
}

$vars[$paramCount] = array();
$vars[$paramCount]['name'] = $tokens[$currVar]['content'];

if ($defaultStart !== null) {
$vars[$paramCount]['default']
= $phpcsFile->getTokensAsString(
$defaultStart,
($i - $defaultStart)
);
}

$rawContent = trim($phpcsFile->getTokensAsString($paramStart, ($i - $paramStart)));

$vars[$paramCount]['pass_by_reference'] = $passByReference;
$vars[$paramCount]['variable_length'] = $variableLength;
$vars[$paramCount]['type_hint'] = $typeHint;
$vars[$paramCount]['raw'] = $rawContent;

// Reset the vars, as we are about to process the next parameter.
$defaultStart = null;
$paramStart = ($i + 1);
$passByReference = false;
$variableLength = false;
$typeHint = '';

$paramCount++;
break;
case T_EQUAL:
$defaultStart = ($i + 1);
break;
}//end switch
}//end for

return $vars;

}//end getMethodParameters()

}//end class
92 changes: 68 additions & 24 deletions Sniffs/PHP/NewScalarTypeDeclarationsSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,27 +26,46 @@ class PHPCompatibility_Sniffs_PHP_NewScalarTypeDeclarationsSniff extends PHPComp
* @var array(string => array(string => int|string|null))
*/
protected $newTypes = array (
'int' => array(
'5.6' => false,
'7.0' => true,
'description' => 'int type'
),
'float' => array(
'5.6' => false,
'7.0' => true,
'description' => 'float type'
),
'bool' => array(
'5.6' => false,
'7.0' => true,
'description' => 'bool type'
),
'string' => array(
'5.6' => false,
'7.0' => true,
'description' => 'string type'
),
);
'array' => array(
'5.0' => false,
'5.1' => true,
),
'callable' => array(
'5.3' => false,
'5.4' => true,
),
'int' => array(
'5.6' => false,
'7.0' => true,
),
'float' => array(
'5.6' => false,
'7.0' => true,
),
'bool' => array(
'5.6' => false,
'7.0' => true,
),
'string' => array(
'5.6' => false,
'7.0' => true,
),
);


/**
* Invalid types
*
* The array lists : the invalid type hint => what was probably intended/alternative.
*
* @var array(string => string)
*/
protected $invalidTypes = array (
'parent' => 'self',
'static' => 'self',
'boolean' => 'bool',
'integer' => 'int',
);


/**
Expand All @@ -72,11 +91,36 @@ public function register()
public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
{
// Get all parameters from method signature.
$paramNames = $phpcsFile->getMethodParameters($stackPtr);
$paramNames = $this->getMethodParameters($phpcsFile, $stackPtr);
$tokens = $phpcsFile->getTokens();
$supportsPHP4 = $this->supportsBelow('4.4');

foreach ($paramNames as $param) {
if (in_array($param['type_hint'], array_keys($this->newTypes))) {
if ($param['type_hint'] === '') {
continue;
}

if ($supportsPHP4 === true) {
$error = 'Type hints were not present in PHP 4.4 or earlier.';
$phpcsFile->addError($error, $stackPtr, 'TypeHintFound');
}
else if (isset($this->newTypes[$param['type_hint']])) {
$this->addError($phpcsFile, $stackPtr, $param['type_hint']);
}
else if (isset($this->invalidTypes[$param['type_hint']])) {
$error = "'%s' is not a valid type declaration. Did you mean %s ?";
$data = array(
$param['type_hint'],
$this->invalidTypes[$param['type_hint']],
);
$phpcsFile->addError($error, $stackPtr, 'InvalidTypeHintFound', $data);
}
else if ($param['type_hint'] === 'self') {
if ($this->inClassScope($phpcsFile, $stackPtr) === false) {
$error = "'self' type cannot be used outside of class scope";
$phpcsFile->addError($error, $stackPtr, 'SelfOutsideClassScopeFound');
}
}
}
}//end process()

Expand Down Expand Up @@ -110,7 +154,7 @@ protected function addError($phpcsFile, $stackPtr, $typeName, $pattern=null)
}
}
if (strlen($error) > 0) {
$error = $this->newTypes[$typeName]['description'] . ' is ' . $error;
$error = "'{$typeName}' type declaration is " . $error;

if ($isError === true) {
$phpcsFile->addError($error, $stackPtr);
Expand Down
55 changes: 55 additions & 0 deletions Tests/InClassScopeTest.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php
/**
* In class scope test file
*
* @package PHPCompatibility
*/


/**
* In class scope function tests
*
* @uses BaseSniffTest
* @package PHPCompatibility
*/
class InClassScopeTest extends BaseAbstractClassMethodTest
{

public $filename = 'sniff-examples/utility-functions/in_class_scope.php';

/**
* testInClassScope
*
* @group utilityFunctions
*
* @dataProvider dataInClassScope
*
* @param int $stackPtr Stack pointer for an arbitrary token in the test file.
* @param string $expected The expected boolean return value.
*/
public function testInClassScope($stackPtr, $expected)
{
$result = $this->helperClass->inClassScope($this->_phpcsFile, $stackPtr);
$this->assertSame($expected, $result);
}

/**
* dataInClassScope
*
* @see testInClassScope()
*
* @return array
*/
public function dataInClassScope()
{
return array(
array(2, false), // $var
array(9, false), // function
array(28, true), // $property
array(32, true), // function
array(49, false), // function
array(67, true), // function
);
}

}

0 comments on commit 4c3332a

Please sign in to comment.