From f39847a55d104adb714e186ecdb6e4d8599b3cda Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 8 Sep 2024 17:23:13 +0200 Subject: [PATCH] Squiz/MultiLineFunctionDeclaration: bug fix - skip over attributes The sniff looks for `T_COMMA` tokens to find the start of the next parameter and skips over parenthesis sets and square brackets sets (like short arrays) to prevent mismatching on a `T_COMMA` which is not a parameter separator. This logic did not take parameter attributes into account, which can contain multiple comma-separated attributes, so should also be skipped over. Fixed now. Includes plenty of tests. Also includes minor stability fix for the parentheses/square brackets skipping. Notes: * It could be argued that the sniff should use the `File::getMethodParameters()` method to do the parameter parsing instead. This could be done in a future iteration, but will need to be evaluated carefully for side-effects. * This sniff extends the PEAR `FunctionDeclaration` sniff. It has been verified that that sniff is not affected by this bug. Fixes 608 --- .../MultiLineFunctionDeclarationSniff.php | 9 +++- .../MultiLineFunctionDeclarationUnitTest.inc | 52 ++++++++++++++++++ ...iLineFunctionDeclarationUnitTest.inc.fixed | 54 +++++++++++++++++++ .../MultiLineFunctionDeclarationUnitTest.php | 2 + 4 files changed, 115 insertions(+), 2 deletions(-) diff --git a/src/Standards/Squiz/Sniffs/Functions/MultiLineFunctionDeclarationSniff.php b/src/Standards/Squiz/Sniffs/Functions/MultiLineFunctionDeclarationSniff.php index 61d23ce286..e88d5c9a97 100644 --- a/src/Standards/Squiz/Sniffs/Functions/MultiLineFunctionDeclarationSniff.php +++ b/src/Standards/Squiz/Sniffs/Functions/MultiLineFunctionDeclarationSniff.php @@ -227,16 +227,21 @@ public function processBracket($phpcsFile, $openBracket, $tokens, $type='functio // Each line between the brackets should contain a single parameter. for ($i = ($openBracket + 1); $i < $closeBracket; $i++) { // Skip brackets, like arrays, as they can contain commas. - if (isset($tokens[$i]['bracket_opener']) === true) { + if (isset($tokens[$i]['bracket_closer']) === true) { $i = $tokens[$i]['bracket_closer']; continue; } - if (isset($tokens[$i]['parenthesis_opener']) === true) { + if (isset($tokens[$i]['parenthesis_closer']) === true) { $i = $tokens[$i]['parenthesis_closer']; continue; } + if (isset($tokens[$i]['attribute_closer']) === true) { + $i = $tokens[$i]['attribute_closer']; + continue; + } + if ($tokens[$i]['code'] !== T_COMMA) { continue; } diff --git a/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc b/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc index 811c56ec14..e9f019eb5d 100644 --- a/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc +++ b/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc @@ -302,3 +302,55 @@ new ExceptionMessage(), ) { } } + +// Issue #608 - multi-attributes are not handled correctly. +function ParamWithMultiAttributeOnSameLine( + #[AttributeA, AttributeB] string $param, +) { +} + +function ParamWithMultiAttributeOnSameLineWithParamsShouldNotBeSeenAsMultipleFnParams( + #[AttributeA(10, 'test'), AttributeB([1, 2, 3,])] string $param, +) { +} + +function ParamWithMultiAttributeOnSameLine( + #[AttributeA, AttributeB] string $paramA, int $paramB +) { +} + +function ParamWithMultiAttributeOnSameLineWithParamsShouldNotBeSeenAsMultipleFnParams( + #[AttributeA(10, 'test'), AttributeB([1, 2, 3,])] string $param, int $paramB +) { +} + +function ParamWithAttributeOnOwnLineShouldNotBeSeenAsMultipleFnParams( + #[Attribute] + string $param, +) { +} + +function ParamWithMultipleAttributesOnOwnLineShouldNotBeSeenAsMultipleFnParams( + #[AttributeA] + #[AttributeB] + string $param, +) { +} + +function ParamWithAttributeOnOwnLineWithParamsShouldNotBeSeenAsMultipleFnParamse( + #[Attribute(10, 20)] + string $param, +) { +} + +function ParamWithMultiAttributeOnOwnLineShouldNotBeSeenAsMultipleFnParams( + #[AttributeA, AttributeB] + string $param, +) { +} + +function ParamWithMultiAttributeOnOwnLineWithParamsShouldNotBeSeenAsMultipleFnParams( + #[AttributeA(10, 'test'), AttributeB([1, 2, 3,])] + string $param, +) { +} diff --git a/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc.fixed b/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc.fixed index c38e3ecc0a..bb6acd6aa7 100644 --- a/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc.fixed +++ b/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.inc.fixed @@ -314,3 +314,57 @@ new ExceptionMessage(), ) { } } + +// Issue #608 - multi-attributes are not handled correctly. +function ParamWithMultiAttributeOnSameLine( + #[AttributeA, AttributeB] string $param, +) { +} + +function ParamWithMultiAttributeOnSameLineWithParamsShouldNotBeSeenAsMultipleFnParams( + #[AttributeA(10, 'test'), AttributeB([1, 2, 3,])] string $param, +) { +} + +function ParamWithMultiAttributeOnSameLine( + #[AttributeA, AttributeB] string $paramA, + int $paramB +) { +} + +function ParamWithMultiAttributeOnSameLineWithParamsShouldNotBeSeenAsMultipleFnParams( + #[AttributeA(10, 'test'), AttributeB([1, 2, 3,])] string $param, + int $paramB +) { +} + +function ParamWithAttributeOnOwnLineShouldNotBeSeenAsMultipleFnParams( + #[Attribute] + string $param, +) { +} + +function ParamWithMultipleAttributesOnOwnLineShouldNotBeSeenAsMultipleFnParams( + #[AttributeA] + #[AttributeB] + string $param, +) { +} + +function ParamWithAttributeOnOwnLineWithParamsShouldNotBeSeenAsMultipleFnParamse( + #[Attribute(10, 20)] + string $param, +) { +} + +function ParamWithMultiAttributeOnOwnLineShouldNotBeSeenAsMultipleFnParams( + #[AttributeA, AttributeB] + string $param, +) { +} + +function ParamWithMultiAttributeOnOwnLineWithParamsShouldNotBeSeenAsMultipleFnParams( + #[AttributeA(10, 'test'), AttributeB([1, 2, 3,])] + string $param, +) { +} diff --git a/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.php b/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.php index 30490345d4..cc1dc35eda 100644 --- a/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.php +++ b/src/Standards/Squiz/Tests/Functions/MultiLineFunctionDeclarationUnitTest.php @@ -75,6 +75,8 @@ public function getErrorList($testFile='') 252 => 1, 253 => 1, 254 => 1, + 318 => 1, + 323 => 1, ]; } else { $errors = [