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

Handle @param in docblock for variables passed by reference #3813

Merged
merged 1 commit into from
Jul 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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