Skip to content

Commit

Permalink
Handle @param when var passed by reference
Browse files Browse the repository at this point in the history
  • Loading branch information
fredden committed Jul 11, 2023
1 parent d415500 commit 84621dd
Show file tree
Hide file tree
Showing 4 changed files with 199 additions and 8 deletions.
32 changes: 27 additions & 5 deletions src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php
Original file line number Diff line number Diff line change
Expand Up @@ -555,24 +555,46 @@ protected function processParams(File $phpcsFile, $stackPtr, $commentStart)

// Make sure the param name is correct.
if (isset($realParams[$pos]) === true) {
$realName = $realParams[$pos]['name'];
if ($realName !== $param['var']) {
$realName = $realParams[$pos]['name'];
$paramVarName = $param['var'];

if ($param['var'][0] === '&') {
// Even when passed by reference, the variable name in $realParams does not have
// a leading '&'. This sniff will accept both '&$var' and '$var' in these cases.
$paramVarName = substr($param['var'], 1);

// This makes sure that the 'MissingParamTag' check won't throw a false positive.
$foundParams[(count($foundParams) - 1)] = $paramVarName;

if ($realParams[$pos]['pass_by_reference'] !== true && $realName === $paramVarName) {
// Don't complain about this unless the param name is otherwise correct.
$error = 'Doc comment for parameter %s is prefixed with "&" but parameter is not passed by reference';
$code = 'ParamNameUnexpectedAmpersandPrefix';
$data = [$paramVarName];

// We're not offering an auto-fix here because we can't tell if the docblock
// is wrong, or the parameter should be passed by reference.
$phpcsFile->addError($error, $param['tag'], $code, $data);
}
}

if ($realName !== $paramVarName) {
$code = 'ParamNameNoMatch';
$data = [
$param['var'],
$paramVarName,
$realName,
];

$error = 'Doc comment for parameter %s does not match ';
if (strtolower($param['var']) === strtolower($realName)) {
if (strtolower($paramVarName) === strtolower($realName)) {
$error .= 'case of ';
$code = 'ParamNameNoCaseMatch';
}

$error .= 'actual variable name %s';

$phpcsFile->addError($error, $param['tag'], $code, $data);
}
}//end if
} else if (substr($param['var'], -4) !== ',...') {
// We must have an extra parameter comment.
$error = 'Superfluous parameter comment';
Expand Down
75 changes: 75 additions & 0 deletions src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc
Original file line number Diff line number Diff line change
Expand Up @@ -1054,3 +1054,78 @@ function throwCommentOneLine() {}
* @return void
*/
function doublePipeFatalError(?stdClass $object) {}

/**
* Test for passing variables by reference
*
* This sniff treats the '&' as optional for parameters passed by reference, but
* forbidden for parameters which are not passed by reference.
*
* Because mismatches may be in either direction, we cannot auto-fix these.
*
* @param string $foo A string passed in by reference.
* @param string &$bar A string passed in by reference.
* @param string $baz A string NOT passed in by reference.
* @param string &$qux A string NOT passed in by reference.
* @param string &$case1 A string passed in by reference with a case mismatch.
* @param string &$CASE2 A string NOT passed in by reference, also with a case mismatch.
*
* @return void
*/
public function variablesPassedByReference(&$foo, &$bar, $baz, $qux, &$CASE1, $case2)
{
return;
}

/**
* Test for param tag containing ref, but param in declaration not being by ref.
*
* @param string &$foo This should be flagged as (only) ParamNameUnexpectedAmpersandPrefix.
* @param string &$bar This should be flagged as (only) ParamNameNoMatch.
* @param string &$baz This should be flagged as (only) ParamNameNoCaseMatch.
*
* @return void
*/
function passedByRefMismatch($foo, $bra, $BAZ) {
return;
}

/**
* Test variable case
*
* @param string $foo This parameter is lowercase.
* @param string $BAR This parameter is UPPERCASE.
* @param string $BazQux This parameter is TitleCase.
* @param string $corgeGrault This parameter is camelCase.
* @param string $GARPLY This parameter should be in lowercase.
* @param string $waldo This parameter should be in TitleCase.
* @param string $freD This parameter should be in UPPERCASE.
* @param string $PLUGH This parameter should be in TitleCase.
*
* @return void
*/
public function variableCaseTest(
$foo,
$BAR,
$BazQux,
$corgeGrault,
$garply,
$Waldo,
$FRED,
$PluGh
) {
return;
}

/**
* Test variable order mismatch
*
* @param string $foo This is the third parameter.
* @param string $bar This is the first parameter.
* @param string $baz This is the second parameter.
*
* @return void
*/
public function variableOrderMismatch($bar, $baz, $foo) {
return;
}
Original file line number Diff line number Diff line change
Expand Up @@ -1054,3 +1054,78 @@ function throwCommentOneLine() {}
* @return void
*/
function doublePipeFatalError(?stdClass $object) {}

/**
* Test for passing variables by reference
*
* This sniff treats the '&' as optional for parameters passed by reference, but
* forbidden for parameters which are not passed by reference.
*
* Because mismatches may be in either direction, we cannot auto-fix these.
*
* @param string $foo A string passed in by reference.
* @param string &$bar A string passed in by reference.
* @param string $baz A string NOT passed in by reference.
* @param string &$qux A string NOT passed in by reference.
* @param string &$case1 A string passed in by reference with a case mismatch.
* @param string &$CASE2 A string NOT passed in by reference, also with a case mismatch.
*
* @return void
*/
public function variablesPassedByReference(&$foo, &$bar, $baz, $qux, &$CASE1, $case2)
{
return;
}

/**
* Test for param tag containing ref, but param in declaration not being by ref.
*
* @param string &$foo This should be flagged as (only) ParamNameUnexpectedAmpersandPrefix.
* @param string &$bar This should be flagged as (only) ParamNameNoMatch.
* @param string &$baz This should be flagged as (only) ParamNameNoCaseMatch.
*
* @return void
*/
function passedByRefMismatch($foo, $bra, $BAZ) {
return;
}

/**
* Test variable case
*
* @param string $foo This parameter is lowercase.
* @param string $BAR This parameter is UPPERCASE.
* @param string $BazQux This parameter is TitleCase.
* @param string $corgeGrault This parameter is camelCase.
* @param string $GARPLY This parameter should be in lowercase.
* @param string $waldo This parameter should be in TitleCase.
* @param string $freD This parameter should be in UPPERCASE.
* @param string $PLUGH This parameter should be in TitleCase.
*
* @return void
*/
public function variableCaseTest(
$foo,
$BAR,
$BazQux,
$corgeGrault,
$garply,
$Waldo,
$FRED,
$PluGh
) {
return;
}

/**
* Test variable order mismatch
*
* @param string $foo This is the third parameter.
* @param string $bar This is the first parameter.
* @param string $baz This is the second parameter.
*
* @return void
*/
public function variableOrderMismatch($bar, $baz, $foo) {
return;
}
25 changes: 22 additions & 3 deletions src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,7 @@ public function getErrorList()
138 => 4,
139 => 4,
143 => 2,
152 => 1,
155 => 2,
155 => 1,
159 => 1,
166 => 1,
173 => 1,
Expand Down Expand Up @@ -117,6 +116,22 @@ public function getErrorList()
1006 => 1,
1029 => 1,
1053 => 1,
1058 => 2,
1069 => 1,
1070 => 1,
1071 => 1,
1080 => 2,
1083 => 1,
1084 => 1,
1085 => 1,
1093 => 4,
1100 => 1,
1101 => 1,
1102 => 1,
1103 => 1,
1123 => 1,
1124 => 1,
1125 => 1,
];

// Scalar type hints only work from PHP 7 onwards.
Expand All @@ -132,12 +147,16 @@ public function getErrorList()
$errors[575] = 2;
$errors[627] = 1;
$errors[1002] = 1;
$errors[1075] = 6;
$errors[1089] = 3;
$errors[1107] = 8;
$errors[1129] = 3;
} else {
$errors[729] = 4;
$errors[740] = 2;
$errors[752] = 2;
$errors[982] = 1;
}
}//end if

// Object type hints only work from PHP 7.2 onwards.
if (PHP_VERSION_ID >= 70200) {
Expand Down

0 comments on commit 84621dd

Please sign in to comment.