diff --git a/package.xml b/package.xml index dadcb89a03..00ecd60554 100644 --- a/package.xml +++ b/package.xml @@ -17,8 +17,8 @@ http://pear.php.net/dtd/package-2.0.xsd"> 2022-06-18 - 3.7.2 - 3.7.2 + 3.8.0 + 3.8.0 stable @@ -41,11 +41,6 @@ http://pear.php.net/dtd/package-2.0.xsd"> -- Existing code will continue to work but will throw a deprecation error -- The backwards compatiblity layer will be removed in PHPCS 4.0 -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Newer versions of Composer will now suggest installing PHPCS using require-dev instead of require - -- Thanks to Gary Jones (@GaryJones) for the patch - - A custom Out Of Memory error will now be shown if PHPCS or PHPCBF run out of memory during a run - -- Error message provides actionable information about how to fix the problem and ensures the error is not silent - -- Thanks to Juliette Reinders Folmer (@jrfnl) and Alain Schlesser (@schlessera) for the patch - When using auto report width (the default) a value of 80 columns will be used if an auto width cannot be determined -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - Sniff error messages are now more informative to help bugs get reported to the correct project @@ -64,13 +59,14 @@ http://pear.php.net/dtd/package-2.0.xsd"> -- Squiz.Commenting.FileComment -- Squiz.Commenting.InlineComment -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Generic.PHP.LowerCaseType sniff now correctly examines types inside arrow functions - -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - PSR2.Classes.PropertyDeclaration now enforces that the readonly modifier comes after the visibility modifier - PSR2 and PSR12 do not have documented rules for this as they pre-date the readonly modifier - PSR-PER has been used to confirm the order of this keyword so it can be applied to PSR2 and PSR12 correctly -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Squiz.Formatting.OperatorBracket no longer reports false positives in match() structures + - Squiz.Commenting.FunctionComment: new ParamNameUnexpectedAmpersandPrefix error for parameters annotated as passed by reference while the parameter is not passed by reference + -- Thanks to Dan Wallis (@fredden) for the patch + - Squiz.PHP.InnerFunctions sniff no longer reports on OO methods for OO structures declared within a function or closure + -- Thanks to @Daimona for the patch - Documentation has been added for the following sniffs: -- PSR2.Files.ClosingTag -- PSR2.Methods.FunctionCallSignature @@ -78,27 +74,8 @@ http://pear.php.net/dtd/package-2.0.xsd"> -- Thanks to Atsushi Okui (@blue32a) for the patch - Fixed bug #3557 : Squiz.Arrays.ArrayDeclaration will now ignore PHP 7.4 array unpacking when determining whether an array is associative -- Thanks to Volker Dusch (@edorian) for the patch - - Fixed bug #3616 : Squiz.PHP.DisallowComparisonAssignment false positive for PHP 8 match expression - -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Fixed bug #3618 : Generic.WhiteSpace.ArbitraryParenthesesSpacing false positive for return new parent() - -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Fixed bug #3632 : Short list not tokenized correctly in control structures without braces - -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Fixed bug #3639 : Tokenizer not applying tab replacement to heredoc/nowdoc closers - -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Fixed bug #3640 : Generic.WhiteSpace.DisallowTabIndent not reporting errors for PHP 7.3 flexible heredoc/nowdoc syntax - -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Fixed bug #3645 : PHPCS can show 0 exit code when running in parallel even if child process has fatal error - -- Thanks to Alex Panshin (@enl) for the patch - - Fixed bug #3653 : False positives for match() in OperatorSpacingSniff - -- Thanks to Jaroslav Hanslík (@kukulich) for the patch - - Fixed bug #3666 : PEAR.Functions.FunctionCallSignature incorrect indent fix when checking mixed HTML/PHP files - - Fixed bug #3668 : PSR12.Classes.ClassInstantiation.MissingParentheses false positive when instantiating parent classes - -- Similar issues also fixed in Generic.Functions.FunctionCallArgumentSpacing and Squiz.Formatting.OperatorBracket - -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Fixed bug #3672 : Incorrect ScopeIndent.IncorrectExact report for match inside array literal - - Fixed bug #3694 : Generic.WhiteSpace.SpreadOperatorSpacingAfter does not ignore spread operator in PHP 8.1 first class callables - -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch + - Fixed bug #3717 : Squiz.Commenting.FunctionComment: fixed false positive for InvalidNoReturn when type is never + -- Thanks to Choraimy Kroonstuiver (@axlon) for the patch - Fixed bug #3722 : Potential "Uninitialized string offset 1" in octal notation backfill -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - Fixed bug #3728 : PHP 8.2 | PSR1/SideEffects: allow for readonly classes @@ -109,7 +86,7 @@ http://pear.php.net/dtd/package-2.0.xsd"> -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - Fixed bug #3779 : Squiz/LowercasePHPFunctions + Generic/ForbiddenFunctions: bug fix for class names in attributes -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - - Fixed bug #3785 : Squiz/FunctionComment: potential "Uninitialized string offset 0" when a type contains a duplicate pipe symbol + - Fixed bug #3785 : Squiz.Commenting.FunctionComment: potential "Uninitialized string offset 0" when a type contains a duplicate pipe symbol -- Thanks to Dan Wallis (@fredden) for the patch - Fixed bug #3787 : PEAR/Squiz/[MultiLine]FunctionDeclaration: allow for PHP 8.1 new in initializers -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch @@ -121,6 +98,10 @@ http://pear.php.net/dtd/package-2.0.xsd"> -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch - Fixed bug #3805 : Generic/FunctionCallArgumentSpacing: prevent fixer conflict over PHP 7.3+ trailing comma's in function calls -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch + - Fixed bug #3806 : Squiz.PHP.InnerFunctions sniff now correctly reports inner functions declared within a closure + -- Thanks to @Daimona for the patch + - Fixed bug #3813 : Squiz.Commenting.FunctionComment: false positive for parameter name mismatch on parameters annotated as passed by reference + -- Thanks to Dan Wallis (@fredden) for the patch - Fixed bug #3816 : PSR12/FileHeader: bug fix - false positives on PHP 8.2+ readonly classes -- Thanks to Juliette Reinders Folmer (@jrfnl) for the patch diff --git a/src/Config.php b/src/Config.php index 8d3fb173e6..1ae4c77ee3 100644 --- a/src/Config.php +++ b/src/Config.php @@ -80,7 +80,7 @@ class Config * * @var string */ - const VERSION = '3.7.2'; + const VERSION = '3.8.0'; /** * Package stability; either stable, beta or alpha. diff --git a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php index f63d09c43b..3c79a7a8c4 100644 --- a/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php +++ b/src/Standards/Squiz/Sniffs/Commenting/FunctionCommentSniff.php @@ -143,9 +143,12 @@ protected function processReturn(File $phpcsFile, $stackPtr, $commentStart) } } }//end if - } else if ($returnType !== 'mixed' && in_array('void', $typeNames, true) === false) { - // If return type is not void, there needs to be a return statement - // somewhere in the function that returns something. + } else if ($returnType !== 'mixed' + && $returnType !== 'never' + && in_array('void', $typeNames, true) === false + ) { + // If return type is not void, never, or mixed, there needs to be a + // return statement somewhere in the function that returns something. if (isset($tokens[$stackPtr]['scope_closer']) === true) { $endToken = $tokens[$stackPtr]['scope_closer']; for ($returnToken = $stackPtr; $returnToken < $endToken; $returnToken++) { @@ -555,16 +558,38 @@ 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'; } @@ -572,7 +597,7 @@ protected function processParams(File $phpcsFile, $stackPtr, $commentStart) $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'; diff --git a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php index 4e3ee2152c..e42ef5a16a 100644 --- a/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php +++ b/src/Standards/Squiz/Sniffs/PHP/InnerFunctionsSniff.php @@ -11,6 +11,7 @@ use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; +use PHP_CodeSniffer\Util\Tokens; class InnerFunctionsSniff implements Sniff { @@ -41,21 +42,28 @@ public function process(File $phpcsFile, $stackPtr) { $tokens = $phpcsFile->getTokens(); - $function = $phpcsFile->getCondition($stackPtr, T_FUNCTION); - if ($function === false) { - // Not a nested function. + if (isset($tokens[$stackPtr]['conditions']) === false) { return; } - $class = $phpcsFile->getCondition($stackPtr, T_ANON_CLASS, false); - if ($class !== false && $class > $function) { - // Ignore methods in anon classes. - return; + $conditions = $tokens[$stackPtr]['conditions']; + $reversedConditions = array_reverse($conditions, true); + + $outerFuncToken = null; + foreach ($reversedConditions as $condToken => $condition) { + if ($condition === T_FUNCTION || $condition === T_CLOSURE) { + $outerFuncToken = $condToken; + break; + } + + if (\array_key_exists($condition, Tokens::$ooScopeTokens) === true) { + // Ignore methods in OOP structures defined within functions. + return; + } } - $prev = $phpcsFile->findPrevious(T_WHITESPACE, ($stackPtr - 1), null, true); - if ($tokens[$prev]['code'] === T_EQUAL) { - // Ignore closures. + if ($outerFuncToken === null) { + // Not a nested function. return; } diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc index d57017daf5..a44f5e0e2e 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc @@ -1054,3 +1054,83 @@ 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; +} + +/** + * @return never + */ +function foo() {} diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed index 10e939709b..3d2e10fd6a 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.inc.fixed @@ -1054,3 +1054,83 @@ 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; +} + +/** + * @return never + */ +function foo() {} diff --git a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php index 06ba08f72a..ff1d10c487 100644 --- a/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php +++ b/src/Standards/Squiz/Tests/Commenting/FunctionCommentUnitTest.php @@ -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, @@ -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. @@ -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) { diff --git a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc index dd851461b9..d16c7f2eb6 100644 --- a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc +++ b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.inc @@ -48,3 +48,40 @@ new class { } } }; + +$outerClosure = function () +{ + // Functions inside closures are not allowed. + function innerFunction() { + } +}; + +// Allow methods in classes/traits/interfaces defined inside functions +function foo() { + if (class_exists('MyClass') === false) { + class MyClass { + function foo() {} + } + } + + if (trait_exists('MyTrait') === false) { + trait MyTrait { + function foo() {} + } + } + + if (interface_exists('MyInterface') === false) { + interface MyInterface { + function foo(); + } + } + + // But disallow functions nested inside those methods + if (class_exists('NestedFunctionInMethod') === false) { + class NestedFunctionInMethod { + function foo() { + function innerFunction() {} + } + } + } +} diff --git a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php index 3c9ad07bd3..b0b13b6147 100644 --- a/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php +++ b/src/Standards/Squiz/Tests/PHP/InnerFunctionsUnitTest.php @@ -28,6 +28,8 @@ public function getErrorList() return [ 5 => 1, 46 => 1, + 55 => 1, + 83 => 1, ]; }//end getErrorList()