Skip to content

Commit 97bd952

Browse files
jrfnljrfnl
authored
Merge pull request #3813 from fredden/auto-fix-function-var-by-reference
Handle `@param` in docblock for variables passed by reference
2 parents 6c3f42a + 84621dd commit 97bd952

File tree

4 files changed

+199
-8
lines changed

4 files changed

+199
-8
lines changed

‎src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -555,24 +555,46 @@ protected function processParams(File $phpcsFile, $stackPtr, $commentStart)
555555

556556
// Make sure the param name is correct.
557557
if (isset($realParams[$pos]) === true) {
558-
$realName = $realParams[$pos]['name'];
559-
if ($realName !== $param['var']) {
558+
$realName = $realParams[$pos]['name'];
559+
$paramVarName = $param['var'];
560+
561+
if ($param['var'][0] === '&') {
562+
// Even when passed by reference, the variable name in $realParams does not have
563+
// a leading '&'. This sniff will accept both '&$var' and '$var' in these cases.
564+
$paramVarName = substr($param['var'], 1);
565+
566+
// This makes sure that the 'MissingParamTag' check won't throw a false positive.
567+
$foundParams[(count($foundParams) - 1)] = $paramVarName;
568+
569+
if ($realParams[$pos]['pass_by_reference'] !== true && $realName === $paramVarName) {
570+
// Don't complain about this unless the param name is otherwise correct.
571+
$error = 'Doc comment for parameter %s is prefixed with "&" but parameter is not passed by reference';
572+
$code = 'ParamNameUnexpectedAmpersandPrefix';
573+
$data = [$paramVarName];
574+
575+
// We're not offering an auto-fix here because we can't tell if the docblock
576+
// is wrong, or the parameter should be passed by reference.
577+
$phpcsFile->addError($error, $param['tag'], $code, $data);
578+
}
579+
}
580+
581+
if ($realName !== $paramVarName) {
560582
$code = 'ParamNameNoMatch';
561583
$data = [
562-
$param['var'],
584+
$paramVarName,
563585
$realName,
564586
];
565587

566588
$error = 'Doc comment for parameter %s does not match ';
567-
if (strtolower($param['var']) === strtolower($realName)) {
589+
if (strtolower($paramVarName) === strtolower($realName)) {
568590
$error .= 'case of ';
569591
$code = 'ParamNameNoCaseMatch';
570592
}
571593

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

574596
$phpcsFile->addError($error, $param['tag'], $code, $data);
575-
}
597+
}//end if
576598
} else if (substr($param['var'], -4) !== ',...') {
577599
// We must have an extra parameter comment.
578600
$error = 'Superfluous parameter comment';

‎src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,3 +1054,78 @@ function throwCommentOneLine() {}
10541054
* @return void
10551055
*/
10561056
function doublePipeFatalError(?stdClass $object) {}
1057+
1058+
/**
1059+
* Test for passing variables by reference
1060+
*
1061+
* This sniff treats the '&' as optional for parameters passed by reference, but
1062+
* forbidden for parameters which are not passed by reference.
1063+
*
1064+
* Because mismatches may be in either direction, we cannot auto-fix these.
1065+
*
1066+
* @param string $foo A string passed in by reference.
1067+
* @param string &$bar A string passed in by reference.
1068+
* @param string $baz A string NOT passed in by reference.
1069+
* @param string &$qux A string NOT passed in by reference.
1070+
* @param string &$case1 A string passed in by reference with a case mismatch.
1071+
* @param string &$CASE2 A string NOT passed in by reference, also with a case mismatch.
1072+
*
1073+
* @return void
1074+
*/
1075+
public function variablesPassedByReference(&$foo, &$bar, $baz, $qux, &$CASE1, $case2)
1076+
{
1077+
return;
1078+
}
1079+
1080+
/**
1081+
* Test for param tag containing ref, but param in declaration not being by ref.
1082+
*
1083+
* @param string &$foo This should be flagged as (only) ParamNameUnexpectedAmpersandPrefix.
1084+
* @param string &$bar This should be flagged as (only) ParamNameNoMatch.
1085+
* @param string &$baz This should be flagged as (only) ParamNameNoCaseMatch.
1086+
*
1087+
* @return void
1088+
*/
1089+
function passedByRefMismatch($foo, $bra, $BAZ) {
1090+
return;
1091+
}
1092+
1093+
/**
1094+
* Test variable case
1095+
*
1096+
* @param string $foo This parameter is lowercase.
1097+
* @param string $BAR This parameter is UPPERCASE.
1098+
* @param string $BazQux This parameter is TitleCase.
1099+
* @param string $corgeGrault This parameter is camelCase.
1100+
* @param string $GARPLY This parameter should be in lowercase.
1101+
* @param string $waldo This parameter should be in TitleCase.
1102+
* @param string $freD This parameter should be in UPPERCASE.
1103+
* @param string $PLUGH This parameter should be in TitleCase.
1104+
*
1105+
* @return void
1106+
*/
1107+
public function variableCaseTest(
1108+
$foo,
1109+
$BAR,
1110+
$BazQux,
1111+
$corgeGrault,
1112+
$garply,
1113+
$Waldo,
1114+
$FRED,
1115+
$PluGh
1116+
) {
1117+
return;
1118+
}
1119+
1120+
/**
1121+
* Test variable order mismatch
1122+
*
1123+
* @param string $foo This is the third parameter.
1124+
* @param string $bar This is the first parameter.
1125+
* @param string $baz This is the second parameter.
1126+
*
1127+
* @return void
1128+
*/
1129+
public function variableOrderMismatch($bar, $baz, $foo) {
1130+
return;
1131+
}

‎src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,3 +1054,78 @@ function throwCommentOneLine() {}
10541054
* @return void
10551055
*/
10561056
function doublePipeFatalError(?stdClass $object) {}
1057+
1058+
/**
1059+
* Test for passing variables by reference
1060+
*
1061+
* This sniff treats the '&' as optional for parameters passed by reference, but
1062+
* forbidden for parameters which are not passed by reference.
1063+
*
1064+
* Because mismatches may be in either direction, we cannot auto-fix these.
1065+
*
1066+
* @param string $foo A string passed in by reference.
1067+
* @param string &$bar A string passed in by reference.
1068+
* @param string $baz A string NOT passed in by reference.
1069+
* @param string &$qux A string NOT passed in by reference.
1070+
* @param string &$case1 A string passed in by reference with a case mismatch.
1071+
* @param string &$CASE2 A string NOT passed in by reference, also with a case mismatch.
1072+
*
1073+
* @return void
1074+
*/
1075+
public function variablesPassedByReference(&$foo, &$bar, $baz, $qux, &$CASE1, $case2)
1076+
{
1077+
return;
1078+
}
1079+
1080+
/**
1081+
* Test for param tag containing ref, but param in declaration not being by ref.
1082+
*
1083+
* @param string &$foo This should be flagged as (only) ParamNameUnexpectedAmpersandPrefix.
1084+
* @param string &$bar This should be flagged as (only) ParamNameNoMatch.
1085+
* @param string &$baz This should be flagged as (only) ParamNameNoCaseMatch.
1086+
*
1087+
* @return void
1088+
*/
1089+
function passedByRefMismatch($foo, $bra, $BAZ) {
1090+
return;
1091+
}
1092+
1093+
/**
1094+
* Test variable case
1095+
*
1096+
* @param string $foo This parameter is lowercase.
1097+
* @param string $BAR This parameter is UPPERCASE.
1098+
* @param string $BazQux This parameter is TitleCase.
1099+
* @param string $corgeGrault This parameter is camelCase.
1100+
* @param string $GARPLY This parameter should be in lowercase.
1101+
* @param string $waldo This parameter should be in TitleCase.
1102+
* @param string $freD This parameter should be in UPPERCASE.
1103+
* @param string $PLUGH This parameter should be in TitleCase.
1104+
*
1105+
* @return void
1106+
*/
1107+
public function variableCaseTest(
1108+
$foo,
1109+
$BAR,
1110+
$BazQux,
1111+
$corgeGrault,
1112+
$garply,
1113+
$Waldo,
1114+
$FRED,
1115+
$PluGh
1116+
) {
1117+
return;
1118+
}
1119+
1120+
/**
1121+
* Test variable order mismatch
1122+
*
1123+
* @param string $foo This is the third parameter.
1124+
* @param string $bar This is the first parameter.
1125+
* @param string $baz This is the second parameter.
1126+
*
1127+
* @return void
1128+
*/
1129+
public function variableOrderMismatch($bar, $baz, $foo) {
1130+
return;
1131+
}

‎src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,7 @@ public function getErrorList()
4848
138 => 4,
4949
139 => 4,
5050
143 => 2,
51-
152 => 1,
52-
155 => 2,
51+
155 => 1,
5352
159 => 1,
5453
166 => 1,
5554
173 => 1,
@@ -117,6 +116,22 @@ public function getErrorList()
117116
1006 => 1,
118117
1029 => 1,
119118
1053 => 1,
119+
1058 => 2,
120+
1069 => 1,
121+
1070 => 1,
122+
1071 => 1,
123+
1080 => 2,
124+
1083 => 1,
125+
1084 => 1,
126+
1085 => 1,
127+
1093 => 4,
128+
1100 => 1,
129+
1101 => 1,
130+
1102 => 1,
131+
1103 => 1,
132+
1123 => 1,
133+
1124 => 1,
134+
1125 => 1,
120135
];
121136

122137
// Scalar type hints only work from PHP 7 onwards.
@@ -132,12 +147,16 @@ public function getErrorList()
132147
$errors[575] = 2;
133148
$errors[627] = 1;
134149
$errors[1002] = 1;
150+
$errors[1075] = 6;
151+
$errors[1089] = 3;
152+
$errors[1107] = 8;
153+
$errors[1129] = 3;
135154
} else {
136155
$errors[729] = 4;
137156
$errors[740] = 2;
138157
$errors[752] = 2;
139158
$errors[982] = 1;
140-
}
159+
}//end if
141160

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

0 commit comments

Comments
 (0)