Skip to content

Commit

Permalink
Squiz.Commenting.FunctionComment fixed for incorrect fixing of param …
Browse files Browse the repository at this point in the history 
…types and multi-line param comment indents
  • Loading branch information
@gsherwood
gsherwood committed Feb 14, 2017
1 parent 5ec60ca commit 4079d5f
Show file tree
Hide file tree
Showing 7 changed files with 1,038 additions and 183 deletions.
187 changes: 109 additions & 78 deletions CodeSniffer/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,92 +374,121 @@ protected function processParams(PHP_CodeSniffer_File $phpcsFile, $stackPtr, $co
}

// Check the param type value.
$typeNames = explode('|', $param['type']);
$typeNames = explode('|', $param['type']);
$suggestedTypeNames = array();

foreach ($typeNames as $typeName) {
$suggestedName = PHP_CodeSniffer::suggestType($typeName);
if ($typeName !== $suggestedName) {
$error = 'Expected "%s" but found "%s" for parameter type';
$data = array(
$suggestedName,
$typeName,
);
$suggestedName = PHP_CodeSniffer::suggestType($typeName);
$suggestedTypeNames[] = $suggestedName;

$fix = $phpcsFile->addFixableError($error, $param['tag'], 'IncorrectParamVarName', $data);
if ($fix === true) {
$content = $suggestedName;
$content .= str_repeat(' ', $param['type_space']);
$content .= $param['var'];
$content .= str_repeat(' ', $param['var_space']);
if (isset($param['commentLines'][0]) === true) {
$content .= $param['commentLines'][0]['comment'];
}
if (count($typeNames) > 1) {
continue;
}

$phpcsFile->fixer->replaceToken(($param['tag'] + 2), $content);
// Check type hint for array and custom type.
$suggestedTypeHint = '';
if (strpos($suggestedName, 'array') !== false || substr($suggestedName, -2) === '[]') {
$suggestedTypeHint = 'array';
} else if (strpos($suggestedName, 'callable') !== false) {
$suggestedTypeHint = 'callable';
} else if (strpos($suggestedName, 'callback') !== false) {
$suggestedTypeHint = 'callable';
} else if (in_array($typeName, PHP_CodeSniffer::$allowedTypes) === false) {
$suggestedTypeHint = $suggestedName;
} else if ($this->_phpVersion >= 70000) {
if ($typeName === 'string') {
$suggestedTypeHint = 'string';
} else if ($typeName === 'int' || $typeName === 'integer') {
$suggestedTypeHint = 'int';
} else if ($typeName === 'float') {
$suggestedTypeHint = 'float';
} else if ($typeName === 'bool' || $typeName === 'boolean') {
$suggestedTypeHint = 'bool';
}
} else if (count($typeNames) === 1) {
// Check type hint for array and custom type.
$suggestedTypeHint = '';
if (strpos($suggestedName, 'array') !== false || substr($suggestedName, -2) === '[]') {
$suggestedTypeHint = 'array';
} else if (strpos($suggestedName, 'callable') !== false) {
$suggestedTypeHint = 'callable';
} else if (strpos($suggestedName, 'callback') !== false) {
$suggestedTypeHint = 'callable';
} else if (in_array($typeName, PHP_CodeSniffer::$allowedTypes) === false) {
$suggestedTypeHint = $suggestedName;
} else if ($this->_phpVersion >= 70000) {
if ($typeName === 'string') {
$suggestedTypeHint = 'string';
} else if ($typeName === 'int' || $typeName === 'integer') {
$suggestedTypeHint = 'int';
} else if ($typeName === 'float') {
$suggestedTypeHint = 'float';
} else if ($typeName === 'bool' || $typeName === 'boolean') {
$suggestedTypeHint = 'bool';
}

if ($suggestedTypeHint !== '' && isset($realParams[$pos]) === true) {
$typeHint = $realParams[$pos]['type_hint'];
if ($typeHint === '') {
$error = 'Type hint "%s" missing for %s';
$data = array(
$suggestedTypeHint,
$param['var'],
);

$errorCode = 'TypeHintMissing';
if ($suggestedTypeHint === 'string'
|| $suggestedTypeHint === 'int'
|| $suggestedTypeHint === 'float'
|| $suggestedTypeHint === 'bool'
) {
$errorCode = 'Scalar'.$errorCode;
}

$phpcsFile->addError($error, $stackPtr, $errorCode, $data);
} else if ($typeHint !== substr($suggestedTypeHint, (strlen($typeHint) * -1))) {
$error = 'Expected type hint "%s"; found "%s" for %s';
$data = array(
$suggestedTypeHint,
$typeHint,
$param['var'],
);
$phpcsFile->addError($error, $stackPtr, 'IncorrectTypeHint', $data);
}//end if
} else if ($suggestedTypeHint === '' && isset($realParams[$pos]) === true) {
$typeHint = $realParams[$pos]['type_hint'];
if ($typeHint !== '') {
$error = 'Unknown type hint "%s" found for %s';
$data = array(
$typeHint,
$param['var'],
);
$phpcsFile->addError($error, $stackPtr, 'InvalidTypeHint', $data);
}
}//end if
}//end foreach

if ($suggestedTypeHint !== '' && isset($realParams[$pos]) === true) {
$typeHint = $realParams[$pos]['type_hint'];
if ($typeHint === '') {
$error = 'Type hint "%s" missing for %s';
$data = array(
$suggestedTypeHint,
$param['var'],
);

$errorCode = 'TypeHintMissing';
if ($suggestedTypeHint === 'string'
|| $suggestedTypeHint === 'int'
|| $suggestedTypeHint === 'float'
|| $suggestedTypeHint === 'bool'
) {
$errorCode = 'Scalar'.$errorCode;
}
$suggestedType = implode($suggestedTypeNames, '|');
if ($param['type'] !== $suggestedType) {
$error = 'Expected "%s" but found "%s" for parameter type';
$data = array(
$suggestedType,
$param['type'],
);

$fix = $phpcsFile->addFixableError($error, $param['tag'], 'IncorrectParamVarName', $data);
if ($fix === true) {
$phpcsFile->fixer->beginChangeset();

$content = $suggestedType;
$content .= str_repeat(' ', $param['type_space']);
$content .= $param['var'];
$content .= str_repeat(' ', $param['var_space']);
if (isset($param['commentLines'][0]) === true) {
$content .= $param['commentLines'][0]['comment'];
}

$phpcsFile->addError($error, $stackPtr, $errorCode, $data);
} else if ($typeHint !== substr($suggestedTypeHint, (strlen($typeHint) * -1))) {
$error = 'Expected type hint "%s"; found "%s" for %s';
$data = array(
$suggestedTypeHint,
$typeHint,
$param['var'],
);
$phpcsFile->addError($error, $stackPtr, 'IncorrectTypeHint', $data);
}//end if
} else if ($suggestedTypeHint === '' && isset($realParams[$pos]) === true) {
$typeHint = $realParams[$pos]['type_hint'];
if ($typeHint !== '') {
$error = 'Unknown type hint "%s" found for %s';
$data = array(
$typeHint,
$param['var'],
);
$phpcsFile->addError($error, $stackPtr, 'InvalidTypeHint', $data);
$phpcsFile->fixer->replaceToken(($param['tag'] + 2), $content);

// Fix up the indent of additional comment lines.
foreach ($param['commentLines'] as $lineNum => $line) {
if ($lineNum === 0
|| $param['commentLines'][$lineNum]['indent'] === 0
) {
continue;
}
}//end if

$diff = (strlen($param['type']) - strlen($suggestedType));
$newIndent = ($param['commentLines'][$lineNum]['indent'] - $diff);
$phpcsFile->fixer->replaceToken(
($param['commentLines'][$lineNum]['token'] - 1),
str_repeat(' ', $newIndent)
);
}

$phpcsFile->fixer->endChangeset();
}//end if
}//end foreach
}//end if

if ($param['var'] === '') {
continue;
Expand Down Expand Up @@ -572,7 +601,8 @@ protected function checkSpacingAfterParamType(PHP_CodeSniffer_File $phpcsFile, $
continue;
}

$newIndent = ($param['commentLines'][$lineNum]['indent'] + $spaces - $param['type_space']);
$diff = ($param['type_space'] - $spaces);
$newIndent = ($param['commentLines'][$lineNum]['indent'] - $diff);
$phpcsFile->fixer->replaceToken(
($param['commentLines'][$lineNum]['token'] - 1),
str_repeat(' ', $newIndent)
Expand Down Expand Up @@ -626,7 +656,8 @@ protected function checkSpacingAfterParamName(PHP_CodeSniffer_File $phpcsFile, $
continue;
}

$newIndent = ($param['commentLines'][$lineNum]['indent'] + $spaces - $param['var_space']);
$diff = ($param['var_space'] - $spaces);
$newIndent = ($param['commentLines'][$lineNum]['indent'] - $diff);
$phpcsFile->fixer->replaceToken(
($param['commentLines'][$lineNum]['token'] - 1),
str_repeat(' ', $newIndent)
Expand Down
50 changes: 0 additions & 50 deletions CodeSniffer/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.1.inc
View file

This file was deleted.

50 changes: 0 additions & 50 deletions CodeSniffer/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.1.inc.fixed
View file

This file was deleted.

46 changes: 46 additions & 0 deletions CodeSniffer/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -821,3 +821,49 @@ public function someFunc(): void
}
};
}

/**
* Return description function + mixed return types.
*
* @return bool|int This is a description.
*/
function returnTypeWithDescriptionA()
{
return 5;

}//end returnTypeWithDescriptionA()


/**
* Return description function + mixed return types.
*
* @return real|bool This is a description.
*/
function returnTypeWithDescriptionB()
{
return 5;

}//end returnTypeWithDescriptionB()


/**
* Return description function + lots of different mixed return types.
*
* @return int|object|string[]|real|double|float|bool|array(int=>MyClass)|callable And here we have a description
*/
function returnTypeWithDescriptionC()
{
return 5;

}//end returnTypeWithDescriptionC()


/**
* Return description function + lots of different mixed return types.
*
* @return array(int=>bool)|\OtherVendor\Package\SomeClass2|MyClass[]|void And here we have a description
*/
function returnTypeWithDescriptionD()
{

}//end returnTypeWithDescriptionD()
Loading

0 comments on commit 4079d5f

Please sign in to comment.