From 104aec2afaf9a5d92905c6bccd2884c3551fb3bf Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 21 May 2023 01:27:00 +0200 Subject: [PATCH 01/18] :sparkles: New `Universal.FunctionDeclarations.NoLongClosures` sniff New sniff to check for "long" closures and recommend using named functions instead. The sniff is fully configurable via the following properties: * (int) `recommendedLines` - this determines when a warning will be thrown. Defaults to `5`, which means that a warning will be thrown if the closure is more than 5 lines long. * (int) `maxLines` - this determines when an error will be thrown. Defaults to `8`, which means that an error will be thrown if the closure is more than 8 lines long. * (bool) `ignoreCommentLines` - whether or not comment-only lines should be ignored for the lines count. Defaults to `true`. * (bool) `ignoreEmptyLines` - whether or not blank lines should be ignored for the lines count. Defaults to `true`. The error and the warning have their own error codes - `ExceedsMaximum` and `ExceedsRecommended` - and can be included/excluded separately from a ruleset. Includes unit tests. Includes documentation. Includes metrics. Closes 192 --- .../NoLongClosuresStandard.xml | 42 ++++ .../NoLongClosuresSniff.php | 233 ++++++++++++++++++ .../NoLongClosuresUnitTest.1.inc | 110 +++++++++ .../NoLongClosuresUnitTest.10.inc | 80 ++++++ .../NoLongClosuresUnitTest.11.inc | 80 ++++++ .../NoLongClosuresUnitTest.12.inc | 80 ++++++ .../NoLongClosuresUnitTest.13.inc | 80 ++++++ .../NoLongClosuresUnitTest.14.inc | 80 ++++++ .../NoLongClosuresUnitTest.15.inc | 80 ++++++ .../NoLongClosuresUnitTest.16.inc | 80 ++++++ .../NoLongClosuresUnitTest.17.inc | 80 ++++++ .../NoLongClosuresUnitTest.18.inc | 81 ++++++ .../NoLongClosuresUnitTest.19.inc | 81 ++++++ .../NoLongClosuresUnitTest.2.inc | 81 ++++++ .../NoLongClosuresUnitTest.20.inc | 81 ++++++ .../NoLongClosuresUnitTest.21.inc | 81 ++++++ .../NoLongClosuresUnitTest.3.inc | 81 ++++++ .../NoLongClosuresUnitTest.4.inc | 80 ++++++ .../NoLongClosuresUnitTest.5.inc | 80 ++++++ .../NoLongClosuresUnitTest.6.inc | 78 ++++++ .../NoLongClosuresUnitTest.7.inc | 78 ++++++ .../NoLongClosuresUnitTest.8.inc | 78 ++++++ .../NoLongClosuresUnitTest.9.inc | 78 ++++++ .../NoLongClosuresUnitTest.php | 150 +++++++++++ 24 files changed, 2133 insertions(+) create mode 100644 Universal/Docs/FunctionDeclarations/NoLongClosuresStandard.xml create mode 100644 Universal/Sniffs/FunctionDeclarations/NoLongClosuresSniff.php create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.1.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.10.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.11.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.12.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.13.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.14.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.15.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.16.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.17.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.18.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.19.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.2.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.20.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.21.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.3.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.4.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.5.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.6.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.7.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.8.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.9.inc create mode 100644 Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.php diff --git a/Universal/Docs/FunctionDeclarations/NoLongClosuresStandard.xml b/Universal/Docs/FunctionDeclarations/NoLongClosuresStandard.xml new file mode 100644 index 00000000..dc84e0b1 --- /dev/null +++ b/Universal/Docs/FunctionDeclarations/NoLongClosuresStandard.xml @@ -0,0 +1,42 @@ + + + + + + + + + + + + + + diff --git a/Universal/Sniffs/FunctionDeclarations/NoLongClosuresSniff.php b/Universal/Sniffs/FunctionDeclarations/NoLongClosuresSniff.php new file mode 100644 index 00000000..13abb9fe --- /dev/null +++ b/Universal/Sniffs/FunctionDeclarations/NoLongClosuresSniff.php @@ -0,0 +1,233 @@ +recommendedLines = (int) $this->recommendedLines; + $this->maxLines = (int) $this->maxLines; + + $tokens = $phpcsFile->getTokens(); + + if (isset($tokens[$stackPtr]['scope_opener'], $tokens[$stackPtr]['scope_closer']) === false) { + // Live coding/parse error. Shouldn't be possible as in that case tokenizer won't retokenize to T_CLOSURE. + return; // @codeCoverageIgnore + } + + $opener = $tokens[$stackPtr]['scope_opener']; + $closer = $tokens[$stackPtr]['scope_closer']; + + $currentLine = $tokens[$opener]['line']; + $closerLine = $tokens[$closer]['line']; + + $codeLines = 0; + $commentLines = 0; + $blankLines = 0; + + // Check whether the line of the scope opener needs to be counted, but ignore trailing comments on that line. + $firstNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($opener + 1), $closer, true); + if ($firstNonEmpty !== false && $tokens[$firstNonEmpty]['line'] === $currentLine) { + ++$codeLines; + } + + // Check whether the line of the scope closer needs to be counted. + if ($closerLine !== $currentLine) { + $hasCommentTokens = false; + $hasCodeTokens = false; + for ($i = ($closer - 1); $tokens[$i]['line'] === $closerLine && $i > $opener; $i--) { + if (isset(Tokens::$emptyTokens[$tokens[$i]['code']]) === false) { + $hasCodeTokens = true; + } elseif (isset(Tokens::$commentTokens[$tokens[$i]['code']]) === true) { + $hasCommentTokens = true; + } + } + + if ($hasCodeTokens === true) { + ++$codeLines; + } elseif ($hasCommentTokens === true) { + ++$commentLines; + } + } + + // We've already examined the opener line, so move to the next line. + for ($i = ($opener + 1); $tokens[$i]['line'] === $currentLine && $i < $closer; $i++); + $currentLine = $tokens[$i]['line']; + + // Walk tokens. + while ($currentLine !== $closerLine) { + $hasCommentTokens = false; + $hasCodeTokens = false; + + while ($tokens[$i]['line'] === $currentLine) { + if (isset(Tokens::$emptyTokens[$tokens[$i]['code']]) === false) { + $hasCodeTokens = true; + } elseif (isset(Tokens::$commentTokens[$tokens[$i]['code']]) === true) { + $hasCommentTokens = true; + } + + ++$i; + } + + if ($hasCodeTokens === true) { + ++$codeLines; + } elseif ($hasCommentTokens === true) { + ++$commentLines; + } else { + // Only option left is that this is an empty line. + ++$blankLines; + } + + $currentLine = $tokens[$i]['line']; + } + + $nonBlankLines = ($codeLines + $commentLines); + $totalLines = ($codeLines + $commentLines + $blankLines); + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME_CODE, $codeLines . ' lines'); + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME_COMMENTS, $nonBlankLines . ' lines'); + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME_ALL, $totalLines . ' lines'); + + $lines = $codeLines; + if ($this->ignoreCommentLines === false) { + $lines += $commentLines; + } + if ($this->ignoreEmptyLines === false) { + $lines += $blankLines; + } + + $errorSuffix = ' Declare a named function instead. Found closure containing %s lines'; + + if ($lines > $this->maxLines) { + $phpcsFile->addError( + 'Closures which are longer than %s lines are forbidden.' . $errorSuffix, + $stackPtr, + 'ExceedsMaximum', + [$this->maxLines, $lines] + ); + + return; + } + + if ($lines > $this->recommendedLines) { + $phpcsFile->addWarning( + 'It is recommended for closures to contain %s lines or less.' . $errorSuffix, + $stackPtr, + 'ExceedsRecommended', + [$this->recommendedLines, $lines] + ); + } + } +} diff --git a/Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.1.inc b/Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.1.inc new file mode 100644 index 00000000..9fd39d33 --- /dev/null +++ b/Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.1.inc @@ -0,0 +1,110 @@ + => + */ + public function getErrorList($testFile = '') + { + switch ($testFile) { + case 'NoLongClosuresUnitTest.1.inc': + return [ + 102 => 1, + 105 => 1, + 108 => 1, + ]; + + case 'NoLongClosuresUnitTest.2.inc': + return [ + 22 => 1, + 31 => 1, + 45 => 1, + 57 => 1, + ]; + + case 'NoLongClosuresUnitTest.3.inc': + case 'NoLongClosuresUnitTest.13.inc': + case 'NoLongClosuresUnitTest.17.inc': + return [ + 57 => 1, + ]; + + case 'NoLongClosuresUnitTest.5.inc': + case 'NoLongClosuresUnitTest.6.inc': + case 'NoLongClosuresUnitTest.10.inc': + case 'NoLongClosuresUnitTest.11.inc': + case 'NoLongClosuresUnitTest.14.inc': + case 'NoLongClosuresUnitTest.16.inc': + case 'NoLongClosuresUnitTest.18.inc': + case 'NoLongClosuresUnitTest.19.inc': + case 'NoLongClosuresUnitTest.20.inc': + case 'NoLongClosuresUnitTest.21.inc': + return [ + 45 => 1, + 57 => 1, + ]; + + case 'NoLongClosuresUnitTest.7.inc': + case 'NoLongClosuresUnitTest.8.inc': + case 'NoLongClosuresUnitTest.9.inc': + case 'NoLongClosuresUnitTest.12.inc': + case 'NoLongClosuresUnitTest.15.inc': + default: + return []; + } + } + + /** + * Returns the lines where warnings should occur. + * + * @param string $testFile The name of the file being tested. + * + * @return array => + */ + public function getWarningList($testFile = '') + { + switch ($testFile) { + case 'NoLongClosuresUnitTest.1.inc': + return [ + 42 => 1, + 50 => 1, + 58 => 1, + 83 => 1, + 91 => 1, + ]; + + case 'NoLongClosuresUnitTest.2.inc': + return [ + 11 => 1, + ]; + + case 'NoLongClosuresUnitTest.3.inc': + case 'NoLongClosuresUnitTest.17.inc': + return [ + 45 => 1, + ]; + + case 'NoLongClosuresUnitTest.4.inc': + return [ + 22 => 1, + 31 => 1, + 45 => 1, + 57 => 1, + ]; + + case 'NoLongClosuresUnitTest.6.inc': + case 'NoLongClosuresUnitTest.10.inc': + case 'NoLongClosuresUnitTest.11.inc': + case 'NoLongClosuresUnitTest.14.inc': + case 'NoLongClosuresUnitTest.16.inc': + case 'NoLongClosuresUnitTest.18.inc': + case 'NoLongClosuresUnitTest.19.inc': + case 'NoLongClosuresUnitTest.20.inc': + case 'NoLongClosuresUnitTest.21.inc': + return [ + 22 => 1, + 31 => 1, + ]; + + case 'NoLongClosuresUnitTest.13.inc': + return [ + 31 => 1, + 45 => 1, + ]; + + case 'NoLongClosuresUnitTest.5.inc': + case 'NoLongClosuresUnitTest.7.inc': + case 'NoLongClosuresUnitTest.8.inc': + case 'NoLongClosuresUnitTest.9.inc': + case 'NoLongClosuresUnitTest.12.inc': + case 'NoLongClosuresUnitTest.15.inc': + default: + return []; + } + } +} From 94da037a541cba2903e314a3b92e0ef58fb81341 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 17 Jun 2023 16:27:01 +0200 Subject: [PATCH 02/18] :sparkles: New `Universal.UseStatements.DisallowMixedGroupUse` sniff New sniff to disallow group use statements which combine imports for namespace/OO, functions and/or constants in one statement. Note: the fixer will use a semi-standardized format for group use statements. If there are more specific requirements for the formatting of group use statements, the ruleset configurator should ensure that additional sniffs are included in the ruleset to enforce the required format. Includes unit tests. Includes documentation. Includes metrics. --- .../DisallowMixedGroupUseStandard.xml | 39 ++++ .../DisallowMixedGroupUseSniff.php | 221 ++++++++++++++++++ .../DisallowMixedGroupUseUnitTest.inc | 123 ++++++++++ .../DisallowMixedGroupUseUnitTest.inc.fixed | 135 +++++++++++ .../DisallowMixedGroupUseUnitTest.php | 67 ++++++ 5 files changed, 585 insertions(+) create mode 100644 Universal/Docs/UseStatements/DisallowMixedGroupUseStandard.xml create mode 100644 Universal/Sniffs/UseStatements/DisallowMixedGroupUseSniff.php create mode 100644 Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.inc create mode 100644 Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.inc.fixed create mode 100644 Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.php diff --git a/Universal/Docs/UseStatements/DisallowMixedGroupUseStandard.xml b/Universal/Docs/UseStatements/DisallowMixedGroupUseStandard.xml new file mode 100644 index 00000000..2b930f27 --- /dev/null +++ b/Universal/Docs/UseStatements/DisallowMixedGroupUseStandard.xml @@ -0,0 +1,39 @@ + + + + + + + + + + + + + + diff --git a/Universal/Sniffs/UseStatements/DisallowMixedGroupUseSniff.php b/Universal/Sniffs/UseStatements/DisallowMixedGroupUseSniff.php new file mode 100644 index 00000000..0b332674 --- /dev/null +++ b/Universal/Sniffs/UseStatements/DisallowMixedGroupUseSniff.php @@ -0,0 +1,221 @@ +findNext([\T_SEMICOLON, \T_CLOSE_TAG], ($stackPtr + 1)); + $groupStart = $phpcsFile->findNext(\T_OPEN_USE_GROUP, ($stackPtr + 1), $endOfStatement); + + if ($groupStart === false) { + // Not a group use statement. Just record the metric. + if ($totalCount === 1) { + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'single import'); + } else { + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'multi import'); + } + + return; + } + + if ($totalCount === 1 + || ($ooCount !== 0 && $functionCount === 0 && $constantCount === 0) + || ($ooCount === 0 && $functionCount !== 0 && $constantCount === 0) + || ($ooCount === 0 && $functionCount === 0 && $constantCount !== 0) + ) { + // Not a *mixed* group use statement. + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'group use, single type'); + return; + } + + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'group use, multi type'); + + $error = 'Group use statements should import one type of construct.' + . ' Mixed group use statement found importing %d namespaces/OO names, %d functions and %d constants.'; + $code = 'Found'; + $data = [$ooCount, $functionCount, $constantCount]; + + $hasComment = $phpcsFile->findNext(Tokens::$commentTokens, ($stackPtr + 1), $endOfStatement); + if ($hasComment !== false) { + // Don't attempt to auto-fix is there are comments or PHPCS annotations in the statement. + $phpcsFile->addError($error, $stackPtr, $code, $data); + return; + } + + $fix = $phpcsFile->addFixableError($error, $stackPtr, $code, $data); + + if ($fix === false) { + return; + } + + /* + * Fix it. + * + * This fixer complies with the following (arbitrary) requirements: + * - It will re-use the original base "group" name, i.e. the part before \{. + * - It take take aliases into account, but only when something is aliased to a different name. + * Aliases re-using the original name will be removed. + * - The fix will not add a trailing comma after the last group use sub-statement. + * This is a PHP 7.2+ feature. + * If a standard wants to enforce trailing commas, they should use a separate sniff for that. + * - If there is only 1 statement of a certain type, the replacement will be a single + * import use statement, not a group use statement. + */ + + $phpcsFile->fixer->beginChangeset(); + + // Ensure that a potential close PHP tag ending the statement is not removed. + $tokens = $phpcsFile->getTokens(); + $endRemoval = $endOfStatement; + if ($tokens[$endOfStatement]['code'] !== \T_SEMICOLON) { + $endRemoval = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($endOfStatement - 1), null, true); + } + + // Remove old statement with the exception of the `use` keyword. + for ($i = ($stackPtr + 1); $i <= $endRemoval; $i++) { + $phpcsFile->fixer->replaceToken($i, ''); + } + + // Build up the new use import statements. + $newStatements = []; + + $useIndent = \str_repeat(' ', ($tokens[$stackPtr]['column'] - 1)); + $insideIndent = $useIndent . \str_repeat(' ', 4); + + $baseGroupName = GetTokensAsString::noEmpties($phpcsFile, ($stackPtr + 1), ($groupStart - 1)); + + foreach ($useStatements as $type => $statements) { + $count = \count($statements); + if ($count === 0) { + continue; + } + + $typeName = $type . ' '; + if ($type === 'name') { + $typeName = ''; + } + + if ($count === 1) { + $fqName = \reset($statements); + $alias = \key($statements); + + $newStatement = 'use ' . $typeName . $fqName; + + $unqualifiedName = \ltrim(\substr($fqName, \strrpos($fqName, '\\')), '\\'); + if ($unqualifiedName !== $alias) { + $newStatement .= ' as ' . $alias; + } + + $newStatement .= ';'; + + $newStatements[] = $newStatement; + continue; + } + + // Multiple statements, add a single-type group use statement. + $newStatement = 'use ' . $typeName . $baseGroupName . '{' . $phpcsFile->eolChar; + + foreach ($statements as $alias => $fqName) { + $partialName = \str_replace($baseGroupName, '', $fqName); + $newStatement .= $insideIndent . $partialName; + + $unqualifiedName = \ltrim(\substr($partialName, \strrpos($partialName, '\\')), '\\'); + if ($unqualifiedName !== $alias) { + $newStatement .= ' as ' . $alias; + } + + $newStatement .= ',' . $phpcsFile->eolChar; + } + + // Remove trailing comma after last statement as that's PHP 7.2+. + $newStatement = \rtrim($newStatement, ',' . $phpcsFile->eolChar); + + $newStatement .= $phpcsFile->eolChar . $useIndent . '};'; + $newStatements[] = $newStatement; + } + + $replacement = \implode($phpcsFile->eolChar . $useIndent, $newStatements); + + $phpcsFile->fixer->replaceToken($stackPtr, $replacement); + + $phpcsFile->fixer->endChangeset(); + } +} diff --git a/Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.inc b/Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.inc new file mode 100644 index 00000000..822208a6 --- /dev/null +++ b/Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.inc @@ -0,0 +1,123 @@ + + +tabWidth = 4; + } + + /** + * Returns the lines where errors should occur. + * + * @return array => + */ + public function getErrorList() + { + return [ + 47 => 1, + 54 => 1, + 62 => 1, + 73 => 1, + 83 => 1, + 93 => 1, + 100 => 1, + 107 => 1, + 113 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() + { + return []; + } +} From 6ee98241cb45023d2981fad66a04af90d5c8ff93 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 18 Jun 2023 05:17:06 +0200 Subject: [PATCH 03/18] :sparkles: New `Universal.CodeAnalysis.NoEchoSprintf` sniff Detects use of the inefficient `echo [v]sprintf(...);` combi. Use `[v]printf()` instead. Refs: * https://www.php.net/manual/en/function.printf.php * https://www.php.net/manual/en/function.sprintf.php * https://www.php.net/manual/en/function.vprintf.php * https://www.php.net/manual/en/function.vsprintf.php Includes unit tests. Includes documentation. --- .../CodeAnalysis/NoEchoSprintfStandard.xml | 25 ++++ .../CodeAnalysis/NoEchoSprintfSniff.php | 131 ++++++++++++++++++ .../CodeAnalysis/NoEchoSprintfUnitTest.1.inc | 35 +++++ .../NoEchoSprintfUnitTest.1.inc.fixed | 35 +++++ .../CodeAnalysis/NoEchoSprintfUnitTest.2.inc | 5 + .../CodeAnalysis/NoEchoSprintfUnitTest.3.inc | 5 + .../CodeAnalysis/NoEchoSprintfUnitTest.4.inc | 5 + .../CodeAnalysis/NoEchoSprintfUnitTest.5.inc | 5 + .../CodeAnalysis/NoEchoSprintfUnitTest.php | 59 ++++++++ 9 files changed, 305 insertions(+) create mode 100644 Universal/Docs/CodeAnalysis/NoEchoSprintfStandard.xml create mode 100644 Universal/Sniffs/CodeAnalysis/NoEchoSprintfSniff.php create mode 100644 Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.1.inc create mode 100644 Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.1.inc.fixed create mode 100644 Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.2.inc create mode 100644 Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.3.inc create mode 100644 Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.4.inc create mode 100644 Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.5.inc create mode 100644 Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.php diff --git a/Universal/Docs/CodeAnalysis/NoEchoSprintfStandard.xml b/Universal/Docs/CodeAnalysis/NoEchoSprintfStandard.xml new file mode 100644 index 00000000..270a6f11 --- /dev/null +++ b/Universal/Docs/CodeAnalysis/NoEchoSprintfStandard.xml @@ -0,0 +1,25 @@ + + + + + + + + printf('text %s text', $var); +echo callMe('text %s text', $var); + ]]> + + + echo sprintf('text %s text', $var); +echo vsprintf('text %s text', [$var]); + ]]> + + + diff --git a/Universal/Sniffs/CodeAnalysis/NoEchoSprintfSniff.php b/Universal/Sniffs/CodeAnalysis/NoEchoSprintfSniff.php new file mode 100644 index 00000000..3273e228 --- /dev/null +++ b/Universal/Sniffs/CodeAnalysis/NoEchoSprintfSniff.php @@ -0,0 +1,131 @@ + + */ + private $targetFunctions = [ + 'sprintf' => 'printf', + 'vsprintf' => 'vprintf', + ]; + + /** + * Returns an array of tokens this test wants to listen for. + * + * @since 1.1.0 + * + * @return array + */ + public function register() + { + return [\T_ECHO]; + } + + /** + * Processes this test, when one of its tokens is encountered. + * + * @since 1.1.0 + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token + * in the stack passed in $tokens. + * + * @return void + */ + public function process(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + + $skip = Tokens::$emptyTokens; + $skip[] = \T_NS_SEPARATOR; + + $next = $phpcsFile->findNext($skip, ($stackPtr + 1), null, true); + if ($next === false + || $tokens[$next]['code'] !== \T_STRING + || isset($this->targetFunctions[\strtolower($tokens[$next]['content'])]) === false + ) { + // Not our target. + return; + } + + $detectedFunction = \strtolower($tokens[$next]['content']); + + $openParens = $phpcsFile->findNext(Tokens::$emptyTokens, ($next + 1), null, true); + if ($next === false + || $tokens[$openParens]['code'] !== \T_OPEN_PARENTHESIS + || isset($tokens[$openParens]['parenthesis_closer']) === false + ) { + // Live coding/parse error. + return; + } + + $closeParens = $tokens[$openParens]['parenthesis_closer']; + $afterFunctionCall = $phpcsFile->findNext(Tokens::$emptyTokens, ($closeParens + 1), null, true); + if ($afterFunctionCall === false + || ($tokens[$afterFunctionCall]['code'] !== \T_SEMICOLON + && $tokens[$afterFunctionCall]['code'] !== \T_CLOSE_TAG) + ) { + // Live coding/parse error or compound echo statement. + return; + } + + $fix = $phpcsFile->addFixableError( + 'Unnecessary "echo %s(...)" found. Use "%s(...)" instead.', + $next, + 'Found', + [ + $tokens[$next]['content'], + $this->targetFunctions[$detectedFunction], + ] + ); + + if ($fix === true) { + $phpcsFile->fixer->beginChangeset(); + + // Remove echo and whitespace. + $phpcsFile->fixer->replaceToken($stackPtr, ''); + + for ($i = ($stackPtr + 1); $i < $next; $i++) { + if ($tokens[$i]['code'] !== \T_WHITESPACE) { + break; + } + + $phpcsFile->fixer->replaceToken($i, ''); + } + + $phpcsFile->fixer->replaceToken($next, $this->targetFunctions[$detectedFunction]); + + $phpcsFile->fixer->endChangeset(); + } + } +} diff --git a/Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.1.inc b/Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.1.inc new file mode 100644 index 00000000..f575d4a6 --- /dev/null +++ b/Universal/Tests/CodeAnalysis/NoEchoSprintfUnitTest.1.inc @@ -0,0 +1,35 @@ +' . sprintf('%s - %d', $string, $number) . ''; + +echo \sprintf('%s - %d', $string, $number), 'text', sprintf('%s - %d', $string, $number); + +echo 'text' . sprintf('%s - %d', $string, $number); + +echo sprintf('%s - %d', $string, $number), \sprintf('%s - %d', $string, $number); + +/* + * The issue. + */ +echo sprintf('%s - %d', $string, $number); +echo \sprintf( + '%s', + $string, +) ?> +' . sprintf('%s - %d', $string, $number) . ''; + +echo \sprintf('%s - %d', $string, $number), 'text', sprintf('%s - %d', $string, $number); + +echo 'text' . sprintf('%s - %d', $string, $number); + +echo sprintf('%s - %d', $string, $number), \sprintf('%s - %d', $string, $number); + +/* + * The issue. + */ +printf('%s - %d', $string, $number); +\printf( + '%s', + $string, +) ?> + => + */ + public function getErrorList($testFile = '') + { + switch ($testFile) { + case 'NoEchoSprintfUnitTest.1.inc': + return [ + 19 => 1, + 20 => 1, + 26 => 1, + 28 => 1, + 30 => 1, + 31 => 1, + ]; + + default: + return []; + } + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() + { + return []; + } +} From ab858ff9eaf5fd587f7920564251eb345b299d77 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 11 Mar 2023 18:35:54 +0100 Subject: [PATCH 04/18] :sparkles: New `Universal.OOStructures.RequireFinalMethodsInTraits` sniff New sniff to enforce non-private, non-abstract methods in traits to be declared as `final`. Includes public `$includeMagicMethods` property, which defaults to `false`, to allow for configuring whether or not magic methods declared in a trait should be declared as `final` or not. Includes unit tests. Includes documentation. Includes metrics. --- .../RequireFinalMethodsInTraitsStandard.xml | 35 +++++ .../RequireFinalMethodsInTraitsSniff.php | 129 ++++++++++++++++++ .../RequireFinalMethodsInTraitsUnitTest.inc | 120 ++++++++++++++++ ...uireFinalMethodsInTraitsUnitTest.inc.fixed | 120 ++++++++++++++++ .../RequireFinalMethodsInTraitsUnitTest.php | 69 ++++++++++ 5 files changed, 473 insertions(+) create mode 100644 Universal/Docs/OOStructures/RequireFinalMethodsInTraitsStandard.xml create mode 100644 Universal/Sniffs/OOStructures/RequireFinalMethodsInTraitsSniff.php create mode 100644 Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc create mode 100644 Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc.fixed create mode 100644 Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.php diff --git a/Universal/Docs/OOStructures/RequireFinalMethodsInTraitsStandard.xml b/Universal/Docs/OOStructures/RequireFinalMethodsInTraitsStandard.xml new file mode 100644 index 00000000..b78e6d29 --- /dev/null +++ b/Universal/Docs/OOStructures/RequireFinalMethodsInTraitsStandard.xml @@ -0,0 +1,35 @@ + + + + + + + + final public function bar() {} + final public static function baz() {} + + // Also valid (out of scope): + protected abstract function overload() {} + private function okay() {} +} + ]]> + + + public function bar() {} + protected static function baz() {} +} + ]]> + + + diff --git a/Universal/Sniffs/OOStructures/RequireFinalMethodsInTraitsSniff.php b/Universal/Sniffs/OOStructures/RequireFinalMethodsInTraitsSniff.php new file mode 100644 index 00000000..b286a369 --- /dev/null +++ b/Universal/Sniffs/OOStructures/RequireFinalMethodsInTraitsSniff.php @@ -0,0 +1,129 @@ +getTokens(); + if (isset($tokens[$stackPtr]['parenthesis_opener']) === false) { + // Parse error/live coding. + return; + } + + $scopePtr = Scopes::validDirectScope($phpcsFile, $stackPtr, \T_TRAIT); + if ($scopePtr === false) { + // Not a trait method. + return; + } + + $methodName = FunctionDeclarations::getName($phpcsFile, $stackPtr); + if ($this->includeMagicMethods === false + && FunctionDeclarations::isMagicMethodName($methodName) === true + ) { + // Magic methods are excluded. Bow out. + return; + } + + $methodProps = FunctionDeclarations::getProperties($phpcsFile, $stackPtr); + if ($methodProps['scope'] === 'private') { + // Private methods can't be final. + return; + } + + if ($methodProps['is_final'] === true) { + // Already final, nothing to do. + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'final'); + return; + } + + if ($methodProps['is_abstract'] === true) { + // Abstract classes can't be final. + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'abstract'); + return; + } + + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'not abstract, not final'); + + $data = [ + $methodProps['scope'], + $methodName, + ObjectDeclarations::getName($phpcsFile, $scopePtr), + ]; + + $fix = $phpcsFile->addFixableError( + 'The non-abstract, %s method "%s()" in trait %s should be declared as final.', + $stackPtr, + 'NonFinalMethodFound', + $data + ); + + if ($fix === true) { + $phpcsFile->fixer->addContentBefore($stackPtr, 'final '); + } + } +} diff --git a/Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc b/Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc new file mode 100644 index 00000000..773debe8 --- /dev/null +++ b/Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc @@ -0,0 +1,120 @@ + => + */ + public function getErrorList() + { + return [ + 81 => 1, + 82 => 1, + 84 => 1, + 85 => 1, + 87 => 1, + 92 => 1, + 94 => 1, + 96 => 1, + 101 => 1, + 102 => 1, + 103 => 1, + 104 => 1, + 105 => 1, + 106 => 1, + 107 => 1, + 108 => 1, + 109 => 1, + 110 => 1, + 111 => 1, + 112 => 1, + 113 => 1, + 114 => 1, + 115 => 1, + 116 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() + { + return []; + } +} From 0d8aef917b6e00c8b4ba5910dff6b941bf8a761b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 17 Jun 2023 19:51:20 +0200 Subject: [PATCH 05/18] :sparkles: New `Universal.UseStatements.NoUselessAliases` sniff New sniff to detect useless aliases in import use statements. Aliasing something to the same name as the original construct is considered useless (though allowed in PHP). Note: as OO and function names in PHP are case-insensitive, aliasing to the same name, using a different case is also considered useless. Includes unit tests. Includes documentation. Fixes 233 --- .../NoUselessAliasesStandard.xml | 30 ++++ .../UseStatements/NoUselessAliasesSniff.php | 164 ++++++++++++++++++ .../NoUselessAliasesUnitTest.inc | 84 +++++++++ .../NoUselessAliasesUnitTest.inc.fixed | 82 +++++++++ .../NoUselessAliasesUnitTest.php | 63 +++++++ 5 files changed, 423 insertions(+) create mode 100644 Universal/Docs/UseStatements/NoUselessAliasesStandard.xml create mode 100644 Universal/Sniffs/UseStatements/NoUselessAliasesSniff.php create mode 100644 Universal/Tests/UseStatements/NoUselessAliasesUnitTest.inc create mode 100644 Universal/Tests/UseStatements/NoUselessAliasesUnitTest.inc.fixed create mode 100644 Universal/Tests/UseStatements/NoUselessAliasesUnitTest.php diff --git a/Universal/Docs/UseStatements/NoUselessAliasesStandard.xml b/Universal/Docs/UseStatements/NoUselessAliasesStandard.xml new file mode 100644 index 00000000..39bb90d1 --- /dev/null +++ b/Universal/Docs/UseStatements/NoUselessAliasesStandard.xml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + diff --git a/Universal/Sniffs/UseStatements/NoUselessAliasesSniff.php b/Universal/Sniffs/UseStatements/NoUselessAliasesSniff.php new file mode 100644 index 00000000..ebf7fe27 --- /dev/null +++ b/Universal/Sniffs/UseStatements/NoUselessAliasesSniff.php @@ -0,0 +1,164 @@ +findNext([\T_SEMICOLON, \T_CLOSE_TAG], ($stackPtr + 1)); + if ($endOfStatement === false) { + // Parse error or live coding. + return; + } + + $hasAliases = $phpcsFile->findNext(\T_AS, ($stackPtr + 1), $endOfStatement); + if ($hasAliases === false) { + // This use import statement does not alias anything, bow out. + return; + } + + $useStatements = UseStatements::splitImportUseStatement($phpcsFile, $stackPtr); + if (\count($useStatements, \COUNT_RECURSIVE) <= 3) { + // No statements found. Shouldn't be possible, but still. Bow out. + return; + } + + $tokens = $phpcsFile->getTokens(); + + // Collect all places where aliases are used in this use statement. + $aliasPtrs = []; + $currentAs = $hasAliases; + do { + $aliasPtr = $phpcsFile->findNext(Tokens::$emptyTokens, ($currentAs + 1), null, true); + if ($aliasPtr !== false && $tokens[$aliasPtr]['code'] === \T_STRING) { + $aliasPtrs[$currentAs] = $aliasPtr; + } + + $currentAs = $phpcsFile->findNext(\T_AS, ($currentAs + 1), $endOfStatement); + } while ($currentAs !== false); + + // Now check the names in each use statement for useless aliases. + foreach ($useStatements as $type => $statements) { + foreach ($statements as $alias => $fqName) { + $unqualifiedName = \ltrim(\substr($fqName, \strrpos($fqName, '\\')), '\\'); + + $uselessAlias = false; + if ($type === 'const') { + // Do a case-sensitive comparison for constants. + if ($unqualifiedName === $alias) { + $uselessAlias = true; + } + } elseif (NamingConventions::isEqual($unqualifiedName, $alias)) { + $uselessAlias = true; + } + + if ($uselessAlias === false) { + continue; + } + + // Now check if this is actually used as an alias or just the actual name. + foreach ($aliasPtrs as $asPtr => $aliasPtr) { + if ($tokens[$aliasPtr]['content'] !== $alias) { + continue; + } + + // Make sure this is really the right one. + $prev = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($asPtr - 1), null, true); + if ($tokens[$prev]['code'] !== \T_STRING + || $tokens[$prev]['content'] !== $unqualifiedName + ) { + continue; + } + + $error = 'Useless alias "%s" found for import of "%s"'; + $code = 'Found'; + $data = [$alias, $fqName]; + + // Okay, so this is the one which should be flagged. + $hasComments = $phpcsFile->findNext(Tokens::$commentTokens, ($prev + 1), $aliasPtr); + if ($hasComments !== false) { + // Don't auto-fix if there are comments. + $phpcsFile->addError($error, $aliasPtr, $code, $data); + break; + } + + $fix = $phpcsFile->addFixableError($error, $aliasPtr, $code, $data); + + if ($fix === true) { + $phpcsFile->fixer->beginChangeset(); + + for ($i = ($prev + 1); $i <= $aliasPtr; $i++) { + $phpcsFile->fixer->replaceToken($i, ''); + } + + $phpcsFile->fixer->endChangeset(); + } + + break; + } + } + } + } +} diff --git a/Universal/Tests/UseStatements/NoUselessAliasesUnitTest.inc b/Universal/Tests/UseStatements/NoUselessAliasesUnitTest.inc new file mode 100644 index 00000000..6652d254 --- /dev/null +++ b/Universal/Tests/UseStatements/NoUselessAliasesUnitTest.inc @@ -0,0 +1,84 @@ + => + */ + public function getErrorList() + { + return [ + 26 => 1, + 30 => 1, + 32 => 1, + 33 => 1, + 34 => 1, + 36 => 1, + 39 => 1, + 41 => 1, + 45 => 1, + 51 => 1, + 56 => 1, + 59 => 1, + 60 => 1, + 61 => 1, + 68 => 1, + 69 => 1, + 72 => 1, + 76 => 1, + ]; + } + + /** + * Returns the lines where warnings should occur. + * + * @return array => + */ + public function getWarningList() + { + return []; + } +} From b15621230beecea197327f52e411b779766fd8fc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 19 Jun 2023 01:18:04 +0200 Subject: [PATCH 06/18] Universal/RequireFinalMethodsInTraits: move sniff to `FunctionDeclarations` category ... which feels more appropriate. --- .../RequireFinalMethodsInTraitsStandard.xml | 0 .../RequireFinalMethodsInTraitsSniff.php | 2 +- .../RequireFinalMethodsInTraitsUnitTest.inc | 4 ++-- .../RequireFinalMethodsInTraitsUnitTest.inc.fixed | 4 ++-- .../RequireFinalMethodsInTraitsUnitTest.php | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) rename Universal/Docs/{OOStructures => FunctionDeclarations}/RequireFinalMethodsInTraitsStandard.xml (100%) rename Universal/Sniffs/{OOStructures => FunctionDeclarations}/RequireFinalMethodsInTraitsSniff.php (98%) rename Universal/Tests/{OOStructures => FunctionDeclarations}/RequireFinalMethodsInTraitsUnitTest.inc (94%) rename Universal/Tests/{OOStructures => FunctionDeclarations}/RequireFinalMethodsInTraitsUnitTest.inc.fixed (94%) rename Universal/Tests/{OOStructures => FunctionDeclarations}/RequireFinalMethodsInTraitsUnitTest.php (90%) diff --git a/Universal/Docs/OOStructures/RequireFinalMethodsInTraitsStandard.xml b/Universal/Docs/FunctionDeclarations/RequireFinalMethodsInTraitsStandard.xml similarity index 100% rename from Universal/Docs/OOStructures/RequireFinalMethodsInTraitsStandard.xml rename to Universal/Docs/FunctionDeclarations/RequireFinalMethodsInTraitsStandard.xml diff --git a/Universal/Sniffs/OOStructures/RequireFinalMethodsInTraitsSniff.php b/Universal/Sniffs/FunctionDeclarations/RequireFinalMethodsInTraitsSniff.php similarity index 98% rename from Universal/Sniffs/OOStructures/RequireFinalMethodsInTraitsSniff.php rename to Universal/Sniffs/FunctionDeclarations/RequireFinalMethodsInTraitsSniff.php index b286a369..8fa1f91b 100644 --- a/Universal/Sniffs/OOStructures/RequireFinalMethodsInTraitsSniff.php +++ b/Universal/Sniffs/FunctionDeclarations/RequireFinalMethodsInTraitsSniff.php @@ -8,7 +8,7 @@ * @link https://github.com/PHPCSStandards/PHPCSExtra */ -namespace PHPCSExtra\Universal\Sniffs\OOStructures; +namespace PHPCSExtra\Universal\Sniffs\FunctionDeclarations; use PHP_CodeSniffer\Files\File; use PHP_CodeSniffer\Sniffs\Sniff; diff --git a/Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc similarity index 94% rename from Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc rename to Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc index 773debe8..b48bfaa3 100644 --- a/Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc +++ b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc @@ -96,7 +96,7 @@ trait FixMe { /*comment*/ protected function withCommentBeforeKeyword() {} } -// phpcs:set Universal.OOStructures.RequireFinalMethodsInTraits includeMagicMethods true +// phpcs:set Universal.FunctionDeclarations.RequireFinalMethodsInTraits includeMagicMethods true trait MagicMethodsAreFlaggedOnRequest { public function __construct() {} public function __destruct() {} @@ -117,4 +117,4 @@ trait MagicMethodsAreFlaggedOnRequest { } // Reset property to default value. -// phpcs:set Universal.OOStructures.RequireFinalMethodsInTraits includeMagicMethods false +// phpcs:set Universal.FunctionDeclarations.RequireFinalMethodsInTraits includeMagicMethods false diff --git a/Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc.fixed b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc.fixed similarity index 94% rename from Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc.fixed rename to Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc.fixed index 0fd73365..19e9a868 100644 --- a/Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.inc.fixed +++ b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc.fixed @@ -96,7 +96,7 @@ trait FixMe { /*comment*/ protected final function withCommentBeforeKeyword() {} } -// phpcs:set Universal.OOStructures.RequireFinalMethodsInTraits includeMagicMethods true +// phpcs:set Universal.FunctionDeclarations.RequireFinalMethodsInTraits includeMagicMethods true trait MagicMethodsAreFlaggedOnRequest { public final function __construct() {} public final function __destruct() {} @@ -117,4 +117,4 @@ trait MagicMethodsAreFlaggedOnRequest { } // Reset property to default value. -// phpcs:set Universal.OOStructures.RequireFinalMethodsInTraits includeMagicMethods false +// phpcs:set Universal.FunctionDeclarations.RequireFinalMethodsInTraits includeMagicMethods false diff --git a/Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.php b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.php similarity index 90% rename from Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.php rename to Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.php index 71c737b5..005e0525 100644 --- a/Universal/Tests/OOStructures/RequireFinalMethodsInTraitsUnitTest.php +++ b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.php @@ -8,14 +8,14 @@ * @link https://github.com/PHPCSStandards/PHPCSExtra */ -namespace PHPCSExtra\Universal\Tests\OOStructures; +namespace PHPCSExtra\Universal\Tests\FunctionDeclarations; use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest; /** * Unit test class for the RequireFinalMethodsInTraits sniff. * - * @covers PHPCSExtra\Universal\Sniffs\OOStructures\RequireFinalMethodsInTraitsSniff + * @covers PHPCSExtra\Universal\Sniffs\FunctionDeclarations\RequireFinalMethodsInTraitsSniff * * @since 1.1.0 */ From 787f3f9d9c11550688d543f02f30b072cd5575ba Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 19 Jun 2023 01:20:43 +0200 Subject: [PATCH 07/18] Universal/RequireFinalMethodsInTraits: remove property in favour of error code differentiation This removes the `public` `$includeMagicMethods` property in favour of using an error code to allow people to include/exclude magic methods from this sniff. --- .../RequireFinalMethodsInTraitsStandard.xml | 2 - .../RequireFinalMethodsInTraitsSniff.php | 33 +++++-------- .../RequireFinalMethodsInTraitsUnitTest.inc | 25 +--------- ...uireFinalMethodsInTraitsUnitTest.inc.fixed | 25 +--------- .../RequireFinalMethodsInTraitsUnitTest.php | 48 +++++++++---------- 5 files changed, 38 insertions(+), 95 deletions(-) diff --git a/Universal/Docs/FunctionDeclarations/RequireFinalMethodsInTraitsStandard.xml b/Universal/Docs/FunctionDeclarations/RequireFinalMethodsInTraitsStandard.xml index b78e6d29..4b2622de 100644 --- a/Universal/Docs/FunctionDeclarations/RequireFinalMethodsInTraitsStandard.xml +++ b/Universal/Docs/FunctionDeclarations/RequireFinalMethodsInTraitsStandard.xml @@ -6,8 +6,6 @@ diff --git a/Universal/Sniffs/FunctionDeclarations/RequireFinalMethodsInTraitsSniff.php b/Universal/Sniffs/FunctionDeclarations/RequireFinalMethodsInTraitsSniff.php index 8fa1f91b..da9e2415 100644 --- a/Universal/Sniffs/FunctionDeclarations/RequireFinalMethodsInTraitsSniff.php +++ b/Universal/Sniffs/FunctionDeclarations/RequireFinalMethodsInTraitsSniff.php @@ -33,17 +33,6 @@ final class RequireFinalMethodsInTraitsSniff implements Sniff */ const METRIC_NAME = 'Non-private method in trait is abstract or final ?'; - /** - * Whether or not this rule applies to magic methods. - * - * Defaults to `false`. - * - * @since 1.1.0 - * - * @var bool - */ - public $includeMagicMethods = false; - /** * Returns an array of tokens this test wants to listen for. * @@ -81,14 +70,6 @@ public function process(File $phpcsFile, $stackPtr) return; } - $methodName = FunctionDeclarations::getName($phpcsFile, $stackPtr); - if ($this->includeMagicMethods === false - && FunctionDeclarations::isMagicMethodName($methodName) === true - ) { - // Magic methods are excluded. Bow out. - return; - } - $methodProps = FunctionDeclarations::getProperties($phpcsFile, $stackPtr); if ($methodProps['scope'] === 'private') { // Private methods can't be final. @@ -109,16 +90,26 @@ public function process(File $phpcsFile, $stackPtr) $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'not abstract, not final'); + $methodName = FunctionDeclarations::getName($phpcsFile, $stackPtr); + $magic = ''; + $code = 'NonFinalMethodFound'; + if (FunctionDeclarations::isMagicMethodName($methodName) === true) { + // Use separate error code for magic methods. + $magic = 'magic '; + $code = 'NonFinalMagicMethodFound'; + } + $data = [ $methodProps['scope'], + $magic, $methodName, ObjectDeclarations::getName($phpcsFile, $scopePtr), ]; $fix = $phpcsFile->addFixableError( - 'The non-abstract, %s method "%s()" in trait %s should be declared as final.', + 'The non-abstract, %s %smethod "%s()" in trait %s should be declared as final.', $stackPtr, - 'NonFinalMethodFound', + $code, $data ); diff --git a/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc index b48bfaa3..6c388b05 100644 --- a/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc +++ b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc @@ -54,25 +54,6 @@ trait FinalMagicMethodsAreNotFlagged { final public function __unserialize($data) {} } -trait MagicMethodsAreNotFlaggedByDefault { - public function __construct() {} - public function __destruct() {} - public function __clone() {} - public function __debugInfo() {} - public function __invoke() {} - public function __get($name) {} - public function __set($name, $value) {} - public function __isset($name) {} - public function __unset($name) {} - public function __call($name, $arguments) {} - public static function __callStatic($name, $arguments) {} - public function __sleep() {} - public function __toString() {} - public static function __set_state($properties) {} - public function __serialize() {} - public function __unserialize($data) {} -} - /* * Bad. @@ -96,8 +77,7 @@ trait FixMe { /*comment*/ protected function withCommentBeforeKeyword() {} } -// phpcs:set Universal.FunctionDeclarations.RequireFinalMethodsInTraits includeMagicMethods true -trait MagicMethodsAreFlaggedOnRequest { +trait MagicMethodsAreAlsoFlagged { public function __construct() {} public function __destruct() {} public function __clone() {} @@ -115,6 +95,3 @@ trait MagicMethodsAreFlaggedOnRequest { public function __serialize() {} public function __unserialize($data) {} } - -// Reset property to default value. -// phpcs:set Universal.FunctionDeclarations.RequireFinalMethodsInTraits includeMagicMethods false diff --git a/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc.fixed b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc.fixed index 19e9a868..34173802 100644 --- a/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc.fixed +++ b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.inc.fixed @@ -54,25 +54,6 @@ trait FinalMagicMethodsAreNotFlagged { final public function __unserialize($data) {} } -trait MagicMethodsAreNotFlaggedByDefault { - public function __construct() {} - public function __destruct() {} - public function __clone() {} - public function __debugInfo() {} - public function __invoke() {} - public function __get($name) {} - public function __set($name, $value) {} - public function __isset($name) {} - public function __unset($name) {} - public function __call($name, $arguments) {} - public static function __callStatic($name, $arguments) {} - public function __sleep() {} - public function __toString() {} - public static function __set_state($properties) {} - public function __serialize() {} - public function __unserialize($data) {} -} - /* * Bad. @@ -96,8 +77,7 @@ trait FixMe { /*comment*/ protected final function withCommentBeforeKeyword() {} } -// phpcs:set Universal.FunctionDeclarations.RequireFinalMethodsInTraits includeMagicMethods true -trait MagicMethodsAreFlaggedOnRequest { +trait MagicMethodsAreAlsoFlagged { public final function __construct() {} public final function __destruct() {} public final function __clone() {} @@ -115,6 +95,3 @@ trait MagicMethodsAreFlaggedOnRequest { public final function __serialize() {} public final function __unserialize($data) {} } - -// Reset property to default value. -// phpcs:set Universal.FunctionDeclarations.RequireFinalMethodsInTraits includeMagicMethods false diff --git a/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.php b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.php index 005e0525..168fa928 100644 --- a/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.php +++ b/Universal/Tests/FunctionDeclarations/RequireFinalMethodsInTraitsUnitTest.php @@ -30,30 +30,30 @@ final class RequireFinalMethodsInTraitsUnitTest extends AbstractSniffUnitTest public function getErrorList() { return [ - 81 => 1, - 82 => 1, - 84 => 1, - 85 => 1, - 87 => 1, - 92 => 1, - 94 => 1, - 96 => 1, - 101 => 1, - 102 => 1, - 103 => 1, - 104 => 1, - 105 => 1, - 106 => 1, - 107 => 1, - 108 => 1, - 109 => 1, - 110 => 1, - 111 => 1, - 112 => 1, - 113 => 1, - 114 => 1, - 115 => 1, - 116 => 1, + 62 => 1, + 63 => 1, + 65 => 1, + 66 => 1, + 68 => 1, + 73 => 1, + 75 => 1, + 77 => 1, + 81 => 1, + 82 => 1, + 83 => 1, + 84 => 1, + 85 => 1, + 86 => 1, + 87 => 1, + 88 => 1, + 89 => 1, + 90 => 1, + 91 => 1, + 92 => 1, + 93 => 1, + 94 => 1, + 95 => 1, + 96 => 1, ]; } From 3c592c5d2b977d8bb5b965ce2a1e89c3802754b9 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 19 Jun 2023 02:39:07 +0200 Subject: [PATCH 08/18] Universal/DisallowMixedGroupUse: improve grammar of error message Only mention the imported constructs found and use correct singular/plural phrasing. --- .../DisallowMixedGroupUseSniff.php | 31 +++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/Universal/Sniffs/UseStatements/DisallowMixedGroupUseSniff.php b/Universal/Sniffs/UseStatements/DisallowMixedGroupUseSniff.php index 0b332674..3303eac1 100644 --- a/Universal/Sniffs/UseStatements/DisallowMixedGroupUseSniff.php +++ b/Universal/Sniffs/UseStatements/DisallowMixedGroupUseSniff.php @@ -108,10 +108,37 @@ public function process(File $phpcsFile, $stackPtr) $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME, 'group use, multi type'); + // Build up the error message. + $foundPhrases = []; + if ($ooCount > 1) { + $foundPhrases[] = \sprintf('%d namespaces/OO names', $ooCount); + } elseif ($ooCount === 1) { + $foundPhrases[] = \sprintf('%d namespace/OO name', $ooCount); + } + + if ($functionCount > 1) { + $foundPhrases[] = \sprintf('%d functions', $functionCount); + } elseif ($functionCount === 1) { + $foundPhrases[] = \sprintf('%d function', $functionCount); + } + + if ($constantCount > 1) { + $foundPhrases[] = \sprintf('%d constants', $constantCount); + } elseif ($constantCount === 1) { + $foundPhrases[] = \sprintf('%d constant', $constantCount); + } + + if (\count($foundPhrases) === 2) { + $found = \implode(' and ', $foundPhrases); + } else { + $found = \array_shift($foundPhrases) . ', '; + $found .= \implode(' and ', $foundPhrases); + } + $error = 'Group use statements should import one type of construct.' - . ' Mixed group use statement found importing %d namespaces/OO names, %d functions and %d constants.'; + . ' Mixed group use statement found importing %s.'; $code = 'Found'; - $data = [$ooCount, $functionCount, $constantCount]; + $data = [$found]; $hasComment = $phpcsFile->findNext(Tokens::$commentTokens, ($stackPtr + 1), $endOfStatement); if ($hasComment !== false) { From 4fbf9d4addc322c813c913997084308b834c1785 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 19 Jun 2023 05:01:12 +0200 Subject: [PATCH 09/18] :sparkles: New `Universal.UseStatements.KeywordSpacing` sniff New sniff to enforce the use of a single space after the `use`, `function`, `const` keywords and both before and after the `as` keyword in import `use` statements. This sniff is complementary to the PHPCS native `Generic.WhiteSpace.LanguageConstructSpacing` sniff which only checks the spacing after the `use` keyword in an import `use statement. The sniff has modular error codes to allow for disabling individual checks. The error codes are: `SpaceAfterUse`, `SpaceAfterFunction`, `SpaceAfterConst`, `SpaceBeforeAs` and `SpaceAfterAs`. Includes fixer. Includes unit tests. Includes documentation. Includes metrics. --- .../UseStatements/KeywordSpacingStandard.xml | 29 +++ .../UseStatements/KeywordSpacingSniff.php | 207 ++++++++++++++++++ .../KeywordSpacingUnitTest.1.inc | 78 +++++++ .../KeywordSpacingUnitTest.1.inc.fixed | 67 ++++++ .../KeywordSpacingUnitTest.2.inc | 11 + .../KeywordSpacingUnitTest.3.inc | 5 + .../KeywordSpacingUnitTest.4.inc | 6 + .../UseStatements/KeywordSpacingUnitTest.php | 80 +++++++ 8 files changed, 483 insertions(+) create mode 100644 Universal/Docs/UseStatements/KeywordSpacingStandard.xml create mode 100644 Universal/Sniffs/UseStatements/KeywordSpacingSniff.php create mode 100644 Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc create mode 100644 Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc.fixed create mode 100644 Universal/Tests/UseStatements/KeywordSpacingUnitTest.2.inc create mode 100644 Universal/Tests/UseStatements/KeywordSpacingUnitTest.3.inc create mode 100644 Universal/Tests/UseStatements/KeywordSpacingUnitTest.4.inc create mode 100644 Universal/Tests/UseStatements/KeywordSpacingUnitTest.php diff --git a/Universal/Docs/UseStatements/KeywordSpacingStandard.xml b/Universal/Docs/UseStatements/KeywordSpacingStandard.xml new file mode 100644 index 00000000..55d430e2 --- /dev/null +++ b/Universal/Docs/UseStatements/KeywordSpacingStandard.xml @@ -0,0 +1,29 @@ + + + + + + + + function strpos; +use const PHP_EOL as MY_EOL; + ]]> + + + function strpos; +use + const + PHP_EOL + as + MY_EOL; + ]]> + + + diff --git a/Universal/Sniffs/UseStatements/KeywordSpacingSniff.php b/Universal/Sniffs/UseStatements/KeywordSpacingSniff.php new file mode 100644 index 00000000..ffef7cb8 --- /dev/null +++ b/Universal/Sniffs/UseStatements/KeywordSpacingSniff.php @@ -0,0 +1,207 @@ + string) + */ + protected $keywords = [ + 'const' => true, + 'function' => true, + ]; + + /** + * Returns an array of tokens this sniff wants to listen for. + * + * @since 1.1.0 + * + * @return array + */ + public function register() + { + return [\T_USE]; + } + + /** + * Processes this test, when one of its tokens is encountered. + * + * @since 1.1.0 + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token in the + * stack passed in $tokens. + * + * @return void + */ + public function process(File $phpcsFile, $stackPtr) + { + if (UseStatements::isImportUse($phpcsFile, $stackPtr) === false) { + // Trait or closure use statement. + return; + } + + $tokens = $phpcsFile->getTokens(); + $endOfStatement = $phpcsFile->findNext([\T_SEMICOLON, \T_CLOSE_TAG], ($stackPtr + 1)); + if ($endOfStatement === false) { + // Live coding or parse error. + return; + } + + // Check the spacing after the `use` keyword. + $this->checkSpacingAfterKeyword($phpcsFile, $stackPtr, $tokens[$stackPtr]['content']); + + // Check the spacing before and after each `as` keyword. + $current = $stackPtr; + do { + $current = $phpcsFile->findNext(\T_AS, ($current + 1), $endOfStatement); + if ($current === false) { + break; + } + + // Prevent false positives when "as" is used within a "name". + if (isset(Tokens::$emptyTokens[$tokens[($current - 1)]['code']]) === true) { + $this->checkSpacingBeforeKeyword($phpcsFile, $current, $tokens[$current]['content']); + $this->checkSpacingAfterKeyword($phpcsFile, $current, $tokens[$current]['content']); + } + } while (true); + + /* + * Check the spacing after `function` and `const` keywords. + */ + $nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), null, true); + if (isset($this->keywords[\strtolower($tokens[$nextNonEmpty]['content'])]) === true) { + // Keyword found at start of statement, applies to whole statement. + $this->checkSpacingAfterKeyword($phpcsFile, $nextNonEmpty, $tokens[$nextNonEmpty]['content']); + return; + } + + // This may still be a group use statement with function/const substatements. + $openGroup = $phpcsFile->findNext(\T_OPEN_USE_GROUP, ($stackPtr + 1), $endOfStatement); + if ($openGroup === false) { + // Not a group use statement. + return; + } + + $closeGroup = $phpcsFile->findNext(\T_CLOSE_USE_GROUP, ($openGroup + 1), $endOfStatement); + + $current = $openGroup; + do { + $current = $phpcsFile->findNext(Tokens::$emptyTokens, ($current + 1), $closeGroup, true); + if ($current === false) { + return; + } + + if (isset($this->keywords[\strtolower($tokens[$current]['content'])]) === true) { + $this->checkSpacingAfterKeyword($phpcsFile, $current, $tokens[$current]['content']); + } + + // We're within the use group, so find the next comma. + $current = $phpcsFile->findNext(\T_COMMA, ($current + 1), $closeGroup); + } while ($current !== false); + } + + /** + * Check the spacing before a found keyword. + * + * @since 1.1.0 + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the keyword in the token stack. + * @param string $content The keyword as found. + * + * @return void + */ + public function checkSpacingBeforeKeyword(File $phpcsFile, $stackPtr, $content) + { + $contentLC = \strtolower($content); + $prevNonEmpty = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true); + + SpacesFixer::checkAndFix( + $phpcsFile, + $stackPtr, + $prevNonEmpty, + 1, // Expected spaces. + 'Expected %s before the "' . $contentLC . '" keyword. Found: %s', + 'SpaceBefore' . \ucfirst($contentLC), + 'error', + 0, // Severity. + \sprintf(self::METRIC_NAME_BEFORE, $contentLC) + ); + } + + /** + * Check the spacing after a found keyword. + * + * @since 1.1.0 + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the keyword in the token stack. + * @param string $content The keyword as found. + * + * @return void + */ + public function checkSpacingAfterKeyword(File $phpcsFile, $stackPtr, $content) + { + $contentLC = \strtolower($content); + $nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), null, true); + + SpacesFixer::checkAndFix( + $phpcsFile, + $stackPtr, + $nextNonEmpty, + 1, // Expected spaces. + 'Expected %s after the "' . $contentLC . '" keyword. Found: %s', + 'SpaceAfter' . \ucfirst($contentLC), + 'error', + 0, // Severity. + \sprintf(self::METRIC_NAME_AFTER, $contentLC) + ); + } +} diff --git a/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc new file mode 100644 index 00000000..da0a9f62 --- /dev/null +++ b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc @@ -0,0 +1,78 @@ + $v) {} + +// Ignore, spacing is already correct. +use Vendor\Package\Name as OtherName; +use function Vendor\Package\functionName as otherFunction; +use const Vendor\Package\CONSTANT_NAME as OTHER_CONSTANT; + +use Vendor\Package\MultiStatement as Multi, + DateTime as dateT; + +use function Vendor\Package\MultiFunction as MFunction, + strpos as pos; + +USE Some\NS\ { + ClassName As OtherClassName, + Function SubLevel\functionName AS OtherFunctionName, + CONST Constants\MYCONSTANT as OTHERCONSTANT, +}; + +// Ignore, "function", "const", "as" are used as part of a name (PHP 8.0+), not as the keyword. +use Vendor\Package\As\Name as Something; +use function Vendor\Function\Name as some_function; +use Vendor\Const\Name as CONSTANT_HANDLER; + +/* + * Error. + */ +use Vendor\Package\Name as OtherName; +use + + Function + + Vendor\Package\functionName + + as + + otherFunction; +use FuncTion\Util\functionB; +use CONST Vendor\Package\CONSTANT_NAME as OTHER_CONSTANT; +use Const\Util\MyClass\CONSTANT_Y; + +use Vendor\Package\MultiStatement as Multi, + DateTime AS dateT; + +Use function Vendor\Package\MultiFunction as MFunction, + strpos as pos; + +use Some\NS\{ + ClassName + as OtherClassName, + function SubLevel\functionName + as OtherFunctionName, + const Constants\MYCONSTANT + as OTHERCONSTANT, +}; + +// Invalid code, but will still be handled. +use; diff --git a/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc.fixed b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc.fixed new file mode 100644 index 00000000..3a84dd22 --- /dev/null +++ b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc.fixed @@ -0,0 +1,67 @@ + $v) {} + +// Ignore, spacing is already correct. +use Vendor\Package\Name as OtherName; +use function Vendor\Package\functionName as otherFunction; +use const Vendor\Package\CONSTANT_NAME as OTHER_CONSTANT; + +use Vendor\Package\MultiStatement as Multi, + DateTime as dateT; + +use function Vendor\Package\MultiFunction as MFunction, + strpos as pos; + +USE Some\NS\ { + ClassName As OtherClassName, + Function SubLevel\functionName AS OtherFunctionName, + CONST Constants\MYCONSTANT as OTHERCONSTANT, +}; + +// Ignore, "function", "const", "as" are used as part of a name (PHP 8.0+), not as the keyword. +use Vendor\Package\As\Name as Something; +use function Vendor\Function\Name as some_function; +use Vendor\Const\Name as CONSTANT_HANDLER; + +/* + * Error. + */ +use Vendor\Package\Name as OtherName; +use Function Vendor\Package\functionName as otherFunction; +use FuncTion \Util\functionB; +use CONST Vendor\Package\CONSTANT_NAME as OTHER_CONSTANT; +use Const \Util\MyClass\CONSTANT_Y; + +use Vendor\Package\MultiStatement as Multi, + DateTime AS dateT; + +Use function Vendor\Package\MultiFunction as MFunction, + strpos as pos; + +use Some\NS\{ + ClassName as OtherClassName, + function SubLevel\functionName as OtherFunctionName, + const Constants\MYCONSTANT as OTHERCONSTANT, +}; + +// Invalid code, but will still be handled. +use ; diff --git a/Universal/Tests/UseStatements/KeywordSpacingUnitTest.2.inc b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.2.inc new file mode 100644 index 00000000..162e98b3 --- /dev/null +++ b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.2.inc @@ -0,0 +1,11 @@ + + */ + public function getErrorList($testFile = '') + { + switch ($testFile) { + case 'KeywordSpacingUnitTest.1.inc': + return [ + 48 => 3, + 49 => 1, + 51 => 1, + 55 => 2, + 58 => 1, + 59 => 4, + 60 => 1, + 62 => 1, + 63 => 2, + 65 => 4, + 66 => 2, + 68 => 1, + 70 => 2, + 71 => 1, + 72 => 2, + 73 => 1, + 74 => 2, + 78 => 1, + ]; + + case 'KeywordSpacingUnitTest.2.inc': + if (\PHP_VERSION_ID >= 80000) { + return []; + } + + return [ + 11 => 1, + ]; + + default: + return []; + } + } + + /** + * Returns the lines where warnings should occur. + * + * @return array + */ + public function getWarningList() + { + return []; + } +} From 647eb222613e8bf5e7bf8ad0fda2f03aaddc71eb Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 19 Jun 2023 06:47:38 +0200 Subject: [PATCH 10/18] Universal/KeywordSpacing: add some tests for unfixable errors ... where there is a comment between the keyword and the previous/next relevant token. --- .../Tests/UseStatements/KeywordSpacingUnitTest.1.inc | 5 +++-- .../UseStatements/KeywordSpacingUnitTest.1.inc.fixed | 8 +++++--- .../Tests/UseStatements/KeywordSpacingUnitTest.php | 12 ++++++------ 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc index da0a9f62..449e0439 100644 --- a/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc +++ b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc @@ -56,7 +56,7 @@ use otherFunction; use FuncTion\Util\functionB; -use CONST Vendor\Package\CONSTANT_NAME as OTHER_CONSTANT; +use CONST Vendor\Package\CONSTANT_NAME as /*comment*/ OTHER_CONSTANT; use Const\Util\MyClass\CONSTANT_Y; use Vendor\Package\MultiStatement as Multi, @@ -67,8 +67,9 @@ Use function Vendor\Package\MultiFunction as MFunction, use Some\NS\{ ClassName + // phpcs:ignore Stnd.Cat.Sniff --for reasons. as OtherClassName, - function SubLevel\functionName + function/*comment*/ SubLevel\functionName as OtherFunctionName, const Constants\MYCONSTANT as OTHERCONSTANT, diff --git a/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc.fixed b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc.fixed index 3a84dd22..f7aae45f 100644 --- a/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc.fixed +++ b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.1.inc.fixed @@ -48,7 +48,7 @@ use Vendor\Const\Name as CONSTANT_HANDLER; use Vendor\Package\Name as OtherName; use Function Vendor\Package\functionName as otherFunction; use FuncTion \Util\functionB; -use CONST Vendor\Package\CONSTANT_NAME as OTHER_CONSTANT; +use CONST Vendor\Package\CONSTANT_NAME as /*comment*/ OTHER_CONSTANT; use Const \Util\MyClass\CONSTANT_Y; use Vendor\Package\MultiStatement as Multi, @@ -58,8 +58,10 @@ Use function Vendor\Package\MultiFunction as MFunction, strpos as pos; use Some\NS\{ - ClassName as OtherClassName, - function SubLevel\functionName as OtherFunctionName, + ClassName + // phpcs:ignore Stnd.Cat.Sniff --for reasons. + as OtherClassName, + function/*comment*/ SubLevel\functionName as OtherFunctionName, const Constants\MYCONSTANT as OTHERCONSTANT, }; diff --git a/Universal/Tests/UseStatements/KeywordSpacingUnitTest.php b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.php index 9e3909da..b54638a0 100644 --- a/Universal/Tests/UseStatements/KeywordSpacingUnitTest.php +++ b/Universal/Tests/UseStatements/KeywordSpacingUnitTest.php @@ -46,12 +46,12 @@ public function getErrorList($testFile = '') 65 => 4, 66 => 2, 68 => 1, - 70 => 2, - 71 => 1, - 72 => 2, - 73 => 1, - 74 => 2, - 78 => 1, + 71 => 2, + 72 => 1, + 73 => 2, + 74 => 1, + 75 => 2, + 79 => 1, ]; case 'KeywordSpacingUnitTest.2.inc': From 1462afa10e8356257c7b07588d521168d68bf6f6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 10 Jul 2023 17:11:47 +0200 Subject: [PATCH 11/18] Composer: update minimum PHPCSUtils version Ref: https://github.com/PHPCSStandards/PHPCSUtils/releases/tag/1.0.7 --- README.md | 2 +- composer.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 2120ce3d..6a6bf3a3 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ Minimum Requirements * PHP 5.4 or higher. * [PHP_CodeSniffer][phpcs-gh] version **3.7.1** or higher. -* [PHPCSUtils][phpcsutils-gh] version **1.0.0** or higher. +* [PHPCSUtils][phpcsutils-gh] version **1.0.7** or higher. Installation diff --git a/composer.json b/composer.json index 9e56a009..45c8f558 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,7 @@ "require" : { "php" : ">=5.4", "squizlabs/php_codesniffer" : "^3.7.1", - "phpcsstandards/phpcsutils" : "^1.0.6" + "phpcsstandards/phpcsutils" : "^1.0.7" }, "require-dev" : { "php-parallel-lint/php-parallel-lint": "^1.3.2", From 5a55c4c3374c832071e786ceefcae2855ca964c2 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sat, 15 Jul 2023 14:12:35 +0200 Subject: [PATCH 12/18] GH Actions: special case Dependabot PRs for Coveralls Follow up on PR 227. Turns out Dependabot PRs do not have access to secrets with the exception of (read-only) access to the `GITHUB_TOKEN`. As the coverage test runs and the Coveralls status are required builds, this blocks Dependabot PRs from being merged without overruling the required statuses. As I'd like to avoid that situation, I'm special casing Dependabot PRs for the token selection. Unfortunately using a condition like `${{ github.actor != 'dependabot[bot]' || secrets.COVERALLS_TOKEN && secrets.GITHUB_TOKEN }}` won't work when it involves secrets, so we need to use duplicate steps to get round this. Refs: * lemurheavy/coveralls-public 1721 * https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions#responding-to-events --- .github/workflows/test.yml | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index aa2b3201..80d610c1 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -181,14 +181,25 @@ jobs: if: ${{ success() }} run: composer global require php-coveralls/php-coveralls:"^2.5.3" --no-interaction - - name: Upload coverage results to Coveralls - if: ${{ success() }} + - name: Upload coverage results to Coveralls (normal) + if: ${{ success() && github.actor != 'dependabot[bot]' }} env: COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_TOKEN }} COVERALLS_PARALLEL: true COVERALLS_FLAG_NAME: php-${{ matrix.php }}-phpcs-${{ matrix.phpcs_version }} run: php-coveralls -v -x build/logs/clover.xml + # Dependabot does not have access to secrets, other than the GH token. + # Ref: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions + # Ref: https://github.com/lemurheavy/coveralls-public/issues/1721 + - name: Upload coverage results to Coveralls (Dependabot) + if: ${{ success() && github.actor == 'dependabot[bot]' }} + env: + COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }} + COVERALLS_PARALLEL: true + COVERALLS_FLAG_NAME: php-${{ matrix.php }}-phpcs-${{ matrix.phpcs_version }} + run: php-coveralls -v -x build/logs/clover.xml + coveralls-finish: needs: coverage if: always() && needs.coverage.result == 'success' @@ -196,8 +207,19 @@ jobs: runs-on: ubuntu-latest steps: - - name: Coveralls Finished + - name: Coveralls Finished (normal) + if: ${{ github.actor != 'dependabot[bot]' }} uses: coverallsapp/github-action@v2 with: github-token: ${{ secrets.COVERALLS_TOKEN }} parallel-finished: true + + # Dependabot does not have access to secrets, other than the GH token. + # Ref: https://docs.github.com/en/code-security/dependabot/working-with-dependabot/automating-dependabot-with-github-actions + # Ref: https://github.com/lemurheavy/coveralls-public/issues/1721 + - name: Coveralls Finished (Dependabot) + if: ${{ github.actor == 'dependabot[bot]' }} + uses: coverallsapp/github-action@v2 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + parallel-finished: true From 4ffc1730be293372f6d3e0b4c6999e00d9829aba Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 16 Jul 2023 00:33:33 +0200 Subject: [PATCH 13/18] Universal/DuplicateArrayKey: minor efficiency tweak Only retrieve the value for `php_version` once, as the value won't change during the run. --- Universal/Sniffs/Arrays/DuplicateArrayKeySniff.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Universal/Sniffs/Arrays/DuplicateArrayKeySniff.php b/Universal/Sniffs/Arrays/DuplicateArrayKeySniff.php index 92532e5c..f9ed534b 100644 --- a/Universal/Sniffs/Arrays/DuplicateArrayKeySniff.php +++ b/Universal/Sniffs/Arrays/DuplicateArrayKeySniff.php @@ -69,7 +69,7 @@ final class DuplicateArrayKeySniff extends AbstractArrayDeclarationSniff private $currentMaxIntKeyGt8; /** - * The current PHP version. + * PHP version as configured or -1 if unknown. * * @since 1.0.0 * @@ -97,6 +97,9 @@ public function processArray(File $phpcsFile) $this->keysSeenGt8 = []; if (isset($this->phpVersion) === false) { + // Set default value to prevent this code from running every time the sniff is triggered. + $this->phpVersion = -1; + $phpVersion = Helper::getConfigData('php_version'); if ($phpVersion !== null) { $this->phpVersion = (int) $phpVersion; @@ -146,7 +149,7 @@ public function processKey(File $phpcsFile, $startPtr, $endPtr, $itemNr) /* * Check if we've seen the key before. */ - if ((isset($this->phpVersion) === false || $this->phpVersion < 80000) + if (($this->phpVersion === -1 || $this->phpVersion < 80000) && isset($this->keysSeenLt8[$key]) === true ) { $errors['phplt8'] = [ @@ -166,7 +169,7 @@ public function processKey(File $phpcsFile, $startPtr, $endPtr, $itemNr) $errors['phplt8']['data_subset'][] = $this->tokens[$firstNonEmptyFirstSeen]['line']; } - if ((isset($this->phpVersion) === false || $this->phpVersion >= 80000) + if (($this->phpVersion === -1 || $this->phpVersion >= 80000) && isset($this->keysSeenGt8[$key]) === true ) { $errors['phpgt8'] = [ From 30189eaf84a2dc591d832dda808fe1cf824c1b32 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 16 Jul 2023 00:28:40 +0200 Subject: [PATCH 14/18] Universal/ConstructorDestructorReturn: minor efficiency tweak Only retrieve the value for `php_version` once, as the value won't change during the run. Includes provision to allow for the unit tests setting this value. --- .../ConstructorDestructorReturnSniff.php | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/Universal/Sniffs/CodeAnalysis/ConstructorDestructorReturnSniff.php b/Universal/Sniffs/CodeAnalysis/ConstructorDestructorReturnSniff.php index d199019e..ce66d4d1 100644 --- a/Universal/Sniffs/CodeAnalysis/ConstructorDestructorReturnSniff.php +++ b/Universal/Sniffs/CodeAnalysis/ConstructorDestructorReturnSniff.php @@ -31,6 +31,15 @@ final class ConstructorDestructorReturnSniff implements Sniff { + /** + * PHP version as configured or 0 if unknown. + * + * @since 1.1.0 + * + * @var int + */ + private $phpVersion; + /** * Registers the tokens that this sniff wants to listen for. * @@ -56,6 +65,16 @@ public function register() */ public function process(File $phpcsFile, $stackPtr) { + if (isset($this->phpVersion) === false || \defined('PHP_CODESNIFFER_IN_TESTS')) { + // Set default value to prevent this code from running every time the sniff is triggered. + $this->phpVersion = 0; + + $phpVersion = Helper::getConfigData('php_version'); + if ($phpVersion !== null) { + $this->phpVersion = (int) $phpVersion; + } + } + $scopePtr = Scopes::validDirectScope($phpcsFile, $stackPtr, Tokens::$ooScopeTokens); if ($scopePtr === false) { // Not an OO method. @@ -69,7 +88,7 @@ public function process(File $phpcsFile, $stackPtr) $functionType = \sprintf('A "%s()" magic method', $functionNameLC); } else { // If the PHP version is explicitly set to PHP 8.0 or higher, ignore PHP 4-style constructors. - if ((int) Helper::getConfigData('php_version') >= 80000) { + if ($this->phpVersion >= 80000) { return; } From 48bf93b319698b0cdf97d8f662f5820a53d4d8ef Mon Sep 17 00:00:00 2001 From: jrfnl Date: Sun, 16 Jul 2023 11:51:30 +0200 Subject: [PATCH 15/18] GH Actions: update for php-coveralls 2.6.0 PHP-Coveralls 2.6.0 has just been released and includes a fix for the last known PHP 8.x issue. This means that it should now be safe to install php-coveralls on PHP 8.x and upload from there, which means we now only need the work-around for the PHP version when on PHP < 5.5 (as Coveralls v1 does not work with GH Actions). Ref: * https://github.com/php-coveralls/php-coveralls/releases/tag/v2.6.0 --- .github/workflows/test.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 80d610c1..fefcc5c5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -168,18 +168,18 @@ jobs: - name: Run the unit tests with code coverage run: composer coverage - # Uploading the results with PHP Coveralls v1 won't work from GH Actions, so switch the PHP version. - - name: Switch to PHP 7.4 - if: ${{ success() && matrix.php != '7.4' }} + # PHP Coveralls v2 (which supports GH Actions) has a PHP 5.5 minimum, so switch the PHP version. + - name: Switch to PHP latest + if: ${{ success() && matrix.php == '5.4' }} uses: shivammathur/setup-php@v2 with: - php-version: 7.4 + php-version: 'latest' coverage: none - # Global install is used to prevent a conflict with the local composer.lock in PHP 8.0+. + # Global install is used to prevent a conflict with the local composer.lock. - name: Install Coveralls if: ${{ success() }} - run: composer global require php-coveralls/php-coveralls:"^2.5.3" --no-interaction + run: composer global require php-coveralls/php-coveralls:"^2.6.0" --no-interaction - name: Upload coverage results to Coveralls (normal) if: ${{ success() && github.actor != 'dependabot[bot]' }} From 4bab273d305b85cafc24cf95ab8b9a11df1fda4f Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 17 Feb 2020 06:31:17 +0100 Subject: [PATCH 16/18] :sparkles: New `Universal.WhiteSpace.CommaSpacing` sniff New sniff to enforce no space before a comma and exactly one space, or a new line, after a comma. Additionally, the sniff also enforces that the comma should follow the code and not be placed after a trailing comment. For the spacing part, the sniff makes the following exceptions: 1. A comma preceded or followed by a parenthesis, curly or square bracket. These will not be flagged to prevent conflicts with sniffs handling spacing around braces. 2. A comma preceded or followed by another comma, like for skipping items in a list assignment. These will not be flagged. 3. A comma preceded by a non-indented heredoc/nowdoc closer. In that case, unless the `php_version` config directive is set to a version higher than PHP 7.3.0, a new line before will be enforced to prevent parse errors on PHP < 7.3. The sniff has a separate error code for when a comma is found with more than one space after it, followed by a trailing comment. That way trailing comment alignment can be allowed by excluding that error code. Additionally, the sniff uses modular error code suffixes for select situations, like `*InFunctionDeclaration`, `*InFunctionCall`, to allow for preventing duplicate messages if another sniff is already handling the comma spacing. Includes raising the minimum PHPCSUtils version to `1.0.8` to prevent running into a particular bug in the `SpacesFixer`. Note: a few of the test files will only run when the tests are run on PHP 7.3+ as the tests involve PHP 7.3+ flexible heredoc/nowdoc tokens, which is the one syntax which PHPCS cannot polyfill in the Tokenizer. Includes fixers. Includes unit tests. Includes documentation. Includes metrics. --- README.md | 2 +- .../Docs/WhiteSpace/CommaSpacingStandard.xml | 94 ++++ .../Sniffs/WhiteSpace/CommaSpacingSniff.php | 408 ++++++++++++++++++ .../WhiteSpace/CommaSpacingUnitTest.1.inc | 203 +++++++++ .../CommaSpacingUnitTest.1.inc.fixed | 191 ++++++++ .../WhiteSpace/CommaSpacingUnitTest.2.inc | 64 +++ .../CommaSpacingUnitTest.2.inc.fixed | 64 +++ .../WhiteSpace/CommaSpacingUnitTest.3.inc | 25 ++ .../CommaSpacingUnitTest.3.inc.fixed | 25 ++ .../WhiteSpace/CommaSpacingUnitTest.4.inc | 12 + .../CommaSpacingUnitTest.4.inc.fixed | 62 +++ .../WhiteSpace/CommaSpacingUnitTest.5.inc | 57 +++ .../CommaSpacingUnitTest.5.inc.fixed | 55 +++ .../WhiteSpace/CommaSpacingUnitTest.6.inc | 59 +++ .../CommaSpacingUnitTest.6.inc.fixed | 63 +++ .../WhiteSpace/CommaSpacingUnitTest.7.inc | 60 +++ .../CommaSpacingUnitTest.7.inc.fixed | 61 +++ .../WhiteSpace/CommaSpacingUnitTest.8.inc | 5 + .../WhiteSpace/CommaSpacingUnitTest.9.inc | 5 + .../CommaSpacingUnitTest.9.inc.fixed | 5 + .../Tests/WhiteSpace/CommaSpacingUnitTest.php | 236 ++++++++++ composer.json | 2 +- 22 files changed, 1756 insertions(+), 2 deletions(-) create mode 100644 Universal/Docs/WhiteSpace/CommaSpacingStandard.xml create mode 100644 Universal/Sniffs/WhiteSpace/CommaSpacingSniff.php create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.1.inc create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.1.inc.fixed create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.2.inc create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.2.inc.fixed create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.3.inc create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.3.inc.fixed create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.4.inc create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.4.inc.fixed create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.5.inc create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.5.inc.fixed create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.6.inc create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.6.inc.fixed create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.7.inc create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.7.inc.fixed create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.8.inc create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.9.inc create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.9.inc.fixed create mode 100644 Universal/Tests/WhiteSpace/CommaSpacingUnitTest.php diff --git a/README.md b/README.md index 6a6bf3a3..a1c4014b 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,7 @@ Minimum Requirements * PHP 5.4 or higher. * [PHP_CodeSniffer][phpcs-gh] version **3.7.1** or higher. -* [PHPCSUtils][phpcsutils-gh] version **1.0.7** or higher. +* [PHPCSUtils][phpcsutils-gh] version **1.0.8** or higher. Installation diff --git a/Universal/Docs/WhiteSpace/CommaSpacingStandard.xml b/Universal/Docs/WhiteSpace/CommaSpacingStandard.xml new file mode 100644 index 00000000..503ae6f7 --- /dev/null +++ b/Universal/Docs/WhiteSpace/CommaSpacingStandard.xml @@ -0,0 +1,94 @@ + + + + + + + + , $param2, $param3); + +function_call( + $param1, + $param2, + $param3 +); + +$array = array($item1, $item2, $item3); +$array = [ + $item1, + $item2, +]; + +list(, $a, $b,,) = $array; +list( + , + $a, + $b, +) = $array; + ]]> + + + , $param2,$param3); + +function_call( + $a + ,$b + ,$c +); + +$array = array($item1,$item2 , $item3); +$array = [ + $item1, + $item2 , +]; + +list( ,$a, $b ,,) = $array; +list( + , + $a, + $b , +) = $array; + ]]> + + + + + + + + , // Comment. + $param2, /* Comment. */ +); + ]]> + + + , + $param2 /* Comment. */, +); + ]]> + + + diff --git a/Universal/Sniffs/WhiteSpace/CommaSpacingSniff.php b/Universal/Sniffs/WhiteSpace/CommaSpacingSniff.php new file mode 100644 index 00000000..29e5b335 --- /dev/null +++ b/Universal/Sniffs/WhiteSpace/CommaSpacingSniff.php @@ -0,0 +1,408 @@ +phpVersion) === false || \defined('PHP_CODESNIFFER_IN_TESTS')) { + // Set default value to prevent this code from running every time the sniff is triggered. + $this->phpVersion = 0; + + $phpVersion = Helper::getConfigData('php_version'); + if ($phpVersion !== null) { + $this->phpVersion = (int) $phpVersion; + } + } + + $this->processSpacingBefore($phpcsFile, $stackPtr); + $this->processSpacingAfter($phpcsFile, $stackPtr); + } + + /** + * Check the spacing before the comma. + * + * @since 1.1.0 + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token + * in the stack passed in $tokens. + * + * @return void + */ + protected function processSpacingBefore(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + + $prevNonWhitespace = $phpcsFile->findPrevious(\T_WHITESPACE, ($stackPtr - 1), null, true); + $prevNonEmpty = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($stackPtr - 1), null, true); + $nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), null, true); + + if ($prevNonWhitespace !== $prevNonEmpty + && $tokens[$prevNonEmpty]['code'] !== \T_COMMA + && $tokens[$prevNonEmpty]['line'] !== $tokens[$nextNonEmpty]['line'] + ) { + // Special case: comma after a trailing comment - the comma should be moved to before the comment. + $fix = $phpcsFile->addFixableError( + 'Comma found after comment, expected the comma after the end of the code', + $stackPtr, + 'CommaAfterComment' + ); + + if ($fix === true) { + $phpcsFile->fixer->beginChangeset(); + + $phpcsFile->fixer->replaceToken($stackPtr, ''); + $phpcsFile->fixer->addContent($prevNonEmpty, ','); + + // Clean up potential trailing whitespace left behind, but don't remove blank lines. + $nextNonWhitespace = $phpcsFile->findNext(\T_WHITESPACE, ($stackPtr + 1), null, true); + if ($tokens[($stackPtr - 1)]['code'] === \T_WHITESPACE + && $tokens[($stackPtr - 1)]['line'] === $tokens[$stackPtr]['line'] + && $tokens[$stackPtr]['line'] !== $tokens[$nextNonWhitespace]['line'] + ) { + $phpcsFile->fixer->replaceToken(($stackPtr - 1), ''); + } + + $phpcsFile->fixer->endChangeset(); + } + return; + } + + if ($tokens[$prevNonWhitespace]['code'] === \T_COMMA) { + // This must be a list assignment with ignored items. Ignore. + return; + } + + if (isset(Tokens::$blockOpeners[$tokens[$prevNonWhitespace]['code']]) === true + || $tokens[$prevNonWhitespace]['code'] === \T_OPEN_SHORT_ARRAY + || $tokens[$prevNonWhitespace]['code'] === \T_OPEN_USE_GROUP + ) { + // Should only realistically be possible for lists. Leave for a block brace spacing sniff to sort out. + return; + } + + $expectedSpaces = 0; + + if ($tokens[$prevNonEmpty]['code'] === \T_END_HEREDOC + || $tokens[$prevNonEmpty]['code'] === \T_END_NOWDOC + ) { + /* + * If php_version is explicitly set to PHP < 7.3, enforce a new line between the closer and the comma. + * + * If php_version is *not* explicitly set, let the indent be leading and only enforce + * a new line between the closer and the comma when this is an old-style heredoc/nowdoc. + */ + if ($this->phpVersion !== 0 && $this->phpVersion < 70300) { + $expectedSpaces = 'newline'; + } + + if ($this->phpVersion === 0 + && \ltrim($tokens[$prevNonEmpty]['content']) === $tokens[$prevNonEmpty]['content'] + ) { + $expectedSpaces = 'newline'; + } + } + + $error = 'Expected %1$s between "' . $this->escapePlaceholders($tokens[$prevNonWhitespace]['content']) + . '" and the comma. Found: %2$s'; + $codeSuffix = $this->getSuffix($phpcsFile, $stackPtr); + $metricSuffix = $this->codeSuffixToMetric($codeSuffix); + + SpacesFixer::checkAndFix( + $phpcsFile, + $stackPtr, + $prevNonWhitespace, + $expectedSpaces, + $error, + 'SpaceBefore' . $codeSuffix, + 'error', + 0, + self::METRIC_NAME_BEFORE . $metricSuffix + ); + } + + /** + * Check the spacing after the comma. + * + * @since 1.1.0 + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token + * in the stack passed in $tokens. + * + * @return void + */ + protected function processSpacingAfter(File $phpcsFile, $stackPtr) + { + $tokens = $phpcsFile->getTokens(); + + $nextNonWhitespace = $phpcsFile->findNext(\T_WHITESPACE, ($stackPtr + 1), null, true); + if ($nextNonWhitespace === false) { + // Live coding/parse error. Ignore. + return; + } + + if ($tokens[$nextNonWhitespace]['code'] === \T_COMMA) { + // This must be a list assignment with ignored items. Ignore. + return; + } + + if ($tokens[$nextNonWhitespace]['code'] === \T_CLOSE_CURLY_BRACKET + || $tokens[$nextNonWhitespace]['code'] === \T_CLOSE_SQUARE_BRACKET + || $tokens[$nextNonWhitespace]['code'] === \T_CLOSE_PARENTHESIS + || $tokens[$nextNonWhitespace]['code'] === \T_CLOSE_SHORT_ARRAY + || $tokens[$nextNonWhitespace]['code'] === \T_CLOSE_USE_GROUP + ) { + // Ignore. Leave for a block spacing sniff to sort out. + return; + } + + $nextToken = $tokens[($stackPtr + 1)]; + + $error = 'Expected %1$s between the comma and "' + . $this->escapePlaceholders($tokens[$nextNonWhitespace]['content']) . '". Found: %2$s'; + + $codeSuffix = $this->getSuffix($phpcsFile, $stackPtr); + $metricSuffix = $this->codeSuffixToMetric($codeSuffix); + + if ($nextToken['code'] === \T_WHITESPACE) { + if ($nextToken['content'] === ' ') { + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME_AFTER . $metricSuffix, '1 space'); + return; + } + + // Note: this check allows for trailing whitespace between the comma and a new line char. + // The trailing whitespace is not the concern of this sniff. + if (\ltrim($nextToken['content'], ' ') === $phpcsFile->eolChar) { + $phpcsFile->recordMetric($stackPtr, self::METRIC_NAME_AFTER . $metricSuffix, 'a new line'); + return; + } + + $errorCode = 'TooMuchSpaceAfter' . $codeSuffix; + + $nextNonEmpty = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), null, true); + if (isset(Tokens::$commentTokens[$tokens[$nextNonWhitespace]['code']]) === true + && ($nextNonEmpty === false || $tokens[$stackPtr]['line'] !== $tokens[$nextNonEmpty]['line']) + ) { + // Separate error code to allow for aligning trailing comments. + $errorCode = 'TooMuchSpaceAfterCommaBeforeTrailingComment'; + } + + SpacesFixer::checkAndFix( + $phpcsFile, + $stackPtr, + $nextNonWhitespace, + 1, + $error, + $errorCode, + 'error', + 0, + self::METRIC_NAME_AFTER . $metricSuffix + ); + return; + } + + SpacesFixer::checkAndFix( + $phpcsFile, + $stackPtr, + $nextNonWhitespace, + 1, + $error, + 'NoSpaceAfter' . $codeSuffix, + 'error', + 0, + self::METRIC_NAME_AFTER . $metricSuffix + ); + } + + /** + * Escape arbitrary token content for *printf() placeholders. + * + * @since 1.1.0 + * + * @param string $text Arbitrary text string. + * + * @return string + */ + private function escapePlaceholders($text) + { + return \preg_replace('`(?:^|[^%])(%)(?:[^%]|$)`', '%%', \trim($text)); + } + + /** + * Retrieve a text string for use as a suffix to an error code. + * + * This allows for modular error codes, which in turn allow for selectively excluding + * error codes. + * + * {@internal Closure use will be parentheses owner in PHPCS 4.x, this code will + * need an update for that in due time.} + * + * @since 1.1.0 + * + * @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned. + * @param int $stackPtr The position of the current token + * in the stack passed in $tokens. + * + * @return string + */ + private function getSuffix($phpcsFile, $stackPtr) + { + $opener = Parentheses::getLastOpener($phpcsFile, $stackPtr); + if ($opener === false) { + return ''; + } + + $tokens = $phpcsFile->getTokens(); + + $owner = Parentheses::getOwner($phpcsFile, $opener); + if ($owner !== false) { + switch ($tokens[$owner]['code']) { + case \T_FUNCTION: + case \T_CLOSURE: + case \T_FN: + return 'InFunctionDeclaration'; + + case \T_DECLARE: + return 'InDeclare'; + + case \T_ANON_CLASS: + case \T_ISSET: + case \T_UNSET: + return 'InFunctionCall'; + + // Long array, long list, isset, unset, empty, exit, eval, control structures. + default: + return ''; + } + } + + $prevNonEmpty = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($opener - 1), null, true); + + if (isset(Collections::nameTokens()[$tokens[$prevNonEmpty]['code']]) === true) { + return 'InFunctionCall'; + } + + switch ($tokens[$prevNonEmpty]['code']) { + case \T_USE: + return 'InClosureUse'; + + case \T_VARIABLE: + case \T_SELF: + case \T_STATIC: + case \T_PARENT: + return 'InFunctionCall'; + + default: + return ''; + } + } + + /** + * Transform a suffix for an error code into a suffix for a metric. + * + * @since 1.1.0 + * + * @param string $suffix Error code suffix. + * + * @return string + */ + private function codeSuffixToMetric($suffix) + { + return \strtolower(\preg_replace('`([A-Z])`', ' $1', $suffix)); + } +} diff --git a/Universal/Tests/WhiteSpace/CommaSpacingUnitTest.1.inc b/Universal/Tests/WhiteSpace/CommaSpacingUnitTest.1.inc new file mode 100644 index 00000000..538270e6 --- /dev/null +++ b/Universal/Tests/WhiteSpace/CommaSpacingUnitTest.1.inc @@ -0,0 +1,203 @@ + $param1 * $param2 + $param3; +class Foo { + public function name($param1 , $param2,$param3) {} +} + +// *InClosureUse suffix. +$closure = function () use($param1 , $param2,$param3) {}; + +// *InFunctionCall suffix. +do_something($param1 , $param2,$param3); +$obj = new Foo($param1 , $param2,$param3); +$obj = new self($param1 , $param2,$param3); +$obj = new parent($param1 , $param2,$param3); +$obj = new static($param1 , $param2,$param3); +$anonClass = new class($param1 , $param2,$param3) {}; +$var($param1 , $param2,$param3); +#[MyAttribute(1 , 'foo',false)] +function name() {} +isset($item1 , $item2,$item3); +unset($item1 , $item2,$item3); + +// No suffix. +$a = array($item1 , $item2,$item3); +$a = [$item1 , $item2,$item3]; + +list($item1 , $item2,$item3) = $array; +[$item1 , $item2,$item3] = $array; + +for ($i = 0 , $j = 1,$k = 2; $i < $j && $j < $k; $i++ , $j++,$k++) {} + +echo $item1 , $item2,$item3; + +use Vendor\Package\{NameA , NameB,NameC}; + +class Multi { + const CONST_A = 1 , CONST_B = 2,CONST_C = 3; + public $propA = 1 , $propB = 2,$propC = 3; + + public function name() { + global $var1 , $var2,$var3; + static $localA = 1 , $localB,$localC = 'foo'; + } +} + +$value = match($value) { + 1 , 2,3 => 123, +}; + +// Parse error, but not our concern. +print ($item1 , $item2,$item3); diff --git a/Universal/Tests/WhiteSpace/CommaSpacingUnitTest.2.inc.fixed b/Universal/Tests/WhiteSpace/CommaSpacingUnitTest.2.inc.fixed new file mode 100644 index 00000000..4c4d4bf2 --- /dev/null +++ b/Universal/Tests/WhiteSpace/CommaSpacingUnitTest.2.inc.fixed @@ -0,0 +1,64 @@ + $param1 * $param2 + $param3; +class Foo { + public function name($param1, $param2, $param3) {} +} + +// *InClosureUse suffix. +$closure = function () use($param1, $param2, $param3) {}; + +// *InFunctionCall suffix. +do_something($param1, $param2, $param3); +$obj = new Foo($param1, $param2, $param3); +$obj = new self($param1, $param2, $param3); +$obj = new parent($param1, $param2, $param3); +$obj = new static($param1, $param2, $param3); +$anonClass = new class($param1, $param2, $param3) {}; +$var($param1, $param2, $param3); +#[MyAttribute(1, 'foo', false)] +function name() {} +isset($item1, $item2, $item3); +unset($item1, $item2, $item3); + +// No suffix. +$a = array($item1, $item2, $item3); +$a = [$item1, $item2, $item3]; + +list($item1, $item2, $item3) = $array; +[$item1, $item2, $item3] = $array; + +for ($i = 0, $j = 1, $k = 2; $i < $j && $j < $k; $i++, $j++, $k++) {} + +echo $item1, $item2, $item3; + +use Vendor\Package\{NameA, NameB, NameC}; + +class Multi { + const CONST_A = 1, CONST_B = 2, CONST_C = 3; + public $propA = 1, $propB = 2, $propC = 3; + + public function name() { + global $var1, $var2, $var3; + static $localA = 1, $localB, $localC = 'foo'; + } +} + +$value = match($value) { + 1, 2, 3 => 123, +}; + +// Parse error, but not our concern. +print ($item1, $item2, $item3); diff --git a/Universal/Tests/WhiteSpace/CommaSpacingUnitTest.3.inc b/Universal/Tests/WhiteSpace/CommaSpacingUnitTest.3.inc new file mode 100644 index 00000000..d071f45c --- /dev/null +++ b/Universal/Tests/WhiteSpace/CommaSpacingUnitTest.3.inc @@ -0,0 +1,25 @@ + $path) { + if (\substr($path, -6) === '.5.inc' + || \substr($path, -6) === '.6.inc' + || \substr($path, -6) === '.7.inc' + ) { + unset($testFiles[$key]); + } + } + } + + return $testFiles; + } + + /** + * Returns the lines where errors should occur. + * + * @param string $testFile The name of the file being tested. + * + * @return array + */ + public function getErrorList($testFile = '') + { + switch ($testFile) { + case 'CommaSpacingUnitTest.1.inc': + return [ + 85 => 1, + 86 => 1, + 87 => 1, + 88 => 1, + 91 => 1, + 92 => 1, + 93 => 1, + 99 => 3, + 101 => 1, + 106 => 1, + 107 => 1, + 111 => 2, + 115 => 2, + 116 => 2, + 117 => 2, + 122 => 1, + 124 => 1, + 126 => 1, + 131 => 1, + 132 => 1, + 137 => 2, + 138 => 2, + 139 => 3, + 143 => 1, + 144 => 1, + 145 => 1, + 146 => 1, + 149 => 3, + 150 => 4, + 154 => 1, + 157 => 2, + 158 => 2, + 161 => 4, + 162 => 4, + 166 => 1, + 168 => 1, + 174 => 2, + 178 => 1, + 182 => 2, + 186 => 1, + 189 => 1, + 195 => 1, + 196 => 1, + 197 => 1, + 201 => 1, + 202 => 1, + ]; + + // Modular error code check. + case 'CommaSpacingUnitTest.2.inc': + return [ + 10 => 3, + 13 => 3, + 14 => 3, + 15 => 3, + 17 => 3, + 21 => 3, + 24 => 3, + 25 => 3, + 26 => 3, + 27 => 3, + 28 => 3, + 29 => 3, + 30 => 3, + 31 => 3, + 33 => 3, + 34 => 3, + 37 => 3, + 38 => 3, + 40 => 3, + 41 => 3, + 43 => 6, + 45 => 3, + 47 => 3, + 50 => 3, + 51 => 3, + 54 => 3, + 55 => 3, + 60 => 3, + 64 => 3, + ]; + + // Comma before trailing comment. + case 'CommaSpacingUnitTest.3.inc': + return [ + 6 => 2, + 10 => 1, + 11 => 1, + 13 => 1, + 18 => 1, + 20 => 1, + 23 => 1, + 24 => 1, + ]; + + /* + * PHP 7.3+ flexible heredoc/nowdoc tests. + * These tests will not be run on PHP < 7.3 as the results will be unreliable. + * The tests files will be removed from the tests to be run via the overloaded getTestFiles() method. + */ + case 'CommaSpacingUnitTest.5.inc': + return [ + 43 => 1, + 46 => 1, + 53 => 2, + 56 => 1, + ]; + + case 'CommaSpacingUnitTest.6.inc': + return [ + 46 => 1, + 49 => 1, + 56 => 2, + 58 => 1, + ]; + + case 'CommaSpacingUnitTest.7.inc': + return [ + 47 => 2, + 49 => 1, + 56 => 2, + 59 => 1, + ]; + + // Parse error test. + case 'CommaSpacingUnitTest.9.inc': + return [ + 5 => 1, + ]; + + default: + return []; + } + } + + /** + * Returns the lines where warnings should occur. + * + * @return array + */ + public function getWarningList() + { + return []; + } +} diff --git a/composer.json b/composer.json index 45c8f558..f4bbf2b9 100644 --- a/composer.json +++ b/composer.json @@ -22,7 +22,7 @@ "require" : { "php" : ">=5.4", "squizlabs/php_codesniffer" : "^3.7.1", - "phpcsstandards/phpcsutils" : "^1.0.7" + "phpcsstandards/phpcsutils" : "^1.0.8" }, "require-dev" : { "php-parallel-lint/php-parallel-lint": "^1.3.2", From 5df93c310322ce12f7acf0ff0b63a0f1e43e3424 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 19 Jun 2023 01:02:29 +0200 Subject: [PATCH 17/18] Minor docs tweaks --- Universal/Docs/CodeAnalysis/NoEchoSprintfStandard.xml | 2 +- Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.php | 2 +- Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.php | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Universal/Docs/CodeAnalysis/NoEchoSprintfStandard.xml b/Universal/Docs/CodeAnalysis/NoEchoSprintfStandard.xml index 270a6f11..914eb65d 100644 --- a/Universal/Docs/CodeAnalysis/NoEchoSprintfStandard.xml +++ b/Universal/Docs/CodeAnalysis/NoEchoSprintfStandard.xml @@ -5,7 +5,7 @@ > diff --git a/Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.php b/Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.php index 75a15813..8a812650 100644 --- a/Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.php +++ b/Universal/Tests/FunctionDeclarations/NoLongClosuresUnitTest.php @@ -17,7 +17,7 @@ * * @covers PHPCSExtra\Universal\Sniffs\FunctionDeclarations\NoLongClosuresSniff * - * @since 1.0.0 + * @since 1.1.0 */ final class NoLongClosuresUnitTest extends AbstractSniffUnitTest { diff --git a/Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.php b/Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.php index 5cdf601c..9e2a73b1 100644 --- a/Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.php +++ b/Universal/Tests/UseStatements/DisallowMixedGroupUseUnitTest.php @@ -17,7 +17,7 @@ * * @covers PHPCSExtra\Universal\Sniffs\UseStatements\DisallowMixedGroupUseSniff * - * @since 1.0.0 + * @since 1.1.0 */ final class DisallowMixedGroupUseUnitTest extends AbstractSniffUnitTest { From ea52221fc54de72dc6355f96821c3fd2ba630460 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Mon, 19 Jun 2023 00:24:03 +0200 Subject: [PATCH 18/18] Changelog and readme updates for PHPCSExtra 1.1.0 Includes some minor formatting tweaks to older releases. --- CHANGELOG.md | 52 ++++++++++++++++++++++++++++++++++++-- README.md | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 118 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 62e346e7..9e06a2e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,13 +14,60 @@ This projects adheres to [Keep a CHANGELOG](http://keepachangelog.com/) and uses _Nothing yet._ +## [1.1.0] - 2023-07-19 + +### Added + +#### Universal + +* :wrench: :books: New `Universal.CodeAnalysis.NoEchoSprintf` sniff to detect use of the inefficient `echo [v]sprintf(...);` combi and recommends using `[v]printf()` instead. [#242] +* :bar_chart: :books: New `Universal.FunctionDeclarations.NoLongClosures` sniff to detect "long" closures and recommend using a named function instead. [#240] + The sniff offers the following properties to influence its behaviour: `recommendedLines` (defaults to `5`), `maxLines` (defaults to `8`), `ignoreCommentLines` (defaults to `true`) and `ignoreEmptyLines` (defaults to `true`). +* :wrench: :bar_chart: :books: New `Universal.FunctionDeclarations.RequireFinalMethodsInTraits` sniff to enforce non-private, non-abstract methods in traits to be declared as `final`. [#243], [#245] + There is a separate `NonFinalMagicMethodFound` error code for magic methods to allow those to be excluded from the check. +* :wrench: :bar_chart: :books: New `Universal.UseStatements.DisallowMixedGroupUse` sniff to disallow group use statements which import a combination of namespace/OO construct, functions and/or constants in one statement. [#241], [#246] + Note: the fixer will use a semi-standardized format for group use statements. If there are more specific requirements for the formatting of group use statements, the ruleset configurator should ensure that additional sniffs are included in the ruleset to enforce the required format. +* :wrench: :bar_chart: :books: New `Universal.UseStatements.KeywordSpacing` sniff to enforce the use of a single space after the `use`, `function`, `const` keywords and both before and after the `as` keyword in import `use` statements. [#247] + The sniff has modular error codes to allow for disabling individual checks. +* :wrench: :books: New `Universal.UseStatements.NoUselessAliases` sniff to detect useless aliases (aliasing something to its original name) in import use statements. [#244] + Note: as OO and function names in PHP are case-insensitive, aliasing to the same name, using a different case is also considered useless. +* :wrench: :bar_chart: :books: New `Universal.WhiteSpace.CommaSpacing` sniff to enforce that there is no space before a comma and exactly one space, or a new line, after a comma. [#254] + Additionally, the sniff also enforces that the comma should follow the code and not be placed after a trailing comment. + The sniff has modular error codes to allow for disabling individual checks and checks in certain contexts. + The sniff will respect a potentially set [`php_version` configuration option][php_version-config] when deciding how to handle the spacing after a heredoc/nowdoc closer. + +### Changed + +#### Universal + +* Minor performance improvements for the `Universal.Arrays.DuplicateArrayKey` and the `Universal.CodeAnalysis.ConstructorDestructorReturn` sniffs. [#251], [#252] + +#### Other + +* Composer: The minimum `PHPCSUtils` requirement has been updated to `^1.0.8` (was `^1.0.6`). [#249], [#254] +* Various housekeeping. + +[#240]: https://github.com/PHPCSStandards/PHPCSExtra/pull/240 +[#241]: https://github.com/PHPCSStandards/PHPCSExtra/pull/241 +[#242]: https://github.com/PHPCSStandards/PHPCSExtra/pull/242 +[#243]: https://github.com/PHPCSStandards/PHPCSExtra/pull/243 +[#244]: https://github.com/PHPCSStandards/PHPCSExtra/pull/244 +[#245]: https://github.com/PHPCSStandards/PHPCSExtra/pull/245 +[#246]: https://github.com/PHPCSStandards/PHPCSExtra/pull/246 +[#247]: https://github.com/PHPCSStandards/PHPCSExtra/pull/247 +[#249]: https://github.com/PHPCSStandards/PHPCSExtra/pull/249 +[#251]: https://github.com/PHPCSStandards/PHPCSExtra/pull/251 +[#252]: https://github.com/PHPCSStandards/PHPCSExtra/pull/252 +[#254]: https://github.com/PHPCSStandards/PHPCSExtra/pull/254 + + ## [1.0.4] - 2023-06-18 ### Changed #### Other -* Composer: The minimum `PHPCSUtils` requirement has been updated to `^1.0.6` (was ^1.0.0). [#237] +* Composer: The minimum `PHPCSUtils` requirement has been updated to `^1.0.6` (was `^1.0.0`). [#237] * Various housekeeping. ### Fixed @@ -183,7 +230,7 @@ For the full list of features, please see the changelogs of the alpha/rc release * Updated the sniffs for compatibility with PHPCSUtils 1.0.0-alpha4. [#134] * Updated the sniffs to correctly handle PHP 8.0/8.1/8.2 features whenever relevant. * Readme: Updated installation instructions for compatibility with Composer 2.2+. [#101] -* Composer: The minimum `PHP_CodeSniffer` requirement has been updated to `^3.7.1` (was ^3.3.1). [#115], [#130] +* Composer: The minimum `PHP_CodeSniffer` requirement has been updated to `^3.7.1` (was `^3.3.1`). [#115], [#130] * Composer: The package will now identify itself as a static analysis tool. Thanks [@GaryJones]! [#126] * All non-`abstract` classes in this package are now `final`. [#121] * All XML documentation now has a schema annotation. [#128] @@ -441,6 +488,7 @@ This initial alpha release contains the following sniffs: [php_version-config]: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Configuration-Options#setting-the-php-version [Unreleased]: https://github.com/PHPCSStandards/PHPCSExtra/compare/stable...HEAD +[1.1.0]: https://github.com/PHPCSStandards/PHPCSExtra/compare/1.0.4...1.1.0 [1.0.4]: https://github.com/PHPCSStandards/PHPCSExtra/compare/1.0.3...1.0.4 [1.0.3]: https://github.com/PHPCSStandards/PHPCSExtra/compare/1.0.2...1.0.3 [1.0.2]: https://github.com/PHPCSStandards/PHPCSExtra/compare/1.0.1...1.0.2 diff --git a/README.md b/README.md index a1c4014b..51c90888 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ Installing via Composer is highly recommended. Run the following from the root of your project: ```bash composer config allow-plugins.dealerdirect/phpcodesniffer-composer-installer true -composer require --dev phpcsstandards/phpcsextra:"^1.0" +composer require --dev phpcsstandards/phpcsextra:"^1.1.0" ``` ### Composer Global Installation @@ -69,7 +69,7 @@ composer require --dev phpcsstandards/phpcsextra:"^1.0" Alternatively, you may want to install this standard globally: ```bash composer global config allow-plugins.dealerdirect/phpcodesniffer-composer-installer true -composer global require --dev phpcsstandards/phpcsextra:"^1.0" +composer global require --dev phpcsstandards/phpcsextra:"^1.1.0" ``` ### Updating to a newer version @@ -220,6 +220,10 @@ Require a consistent modifier keyword order for class declarations. If a [`php_version` configuration option][php_version-config] has been passed to PHPCS using either `--config-set` or `--runtime-set`, it will be respected by the sniff. In effect, this means that the sniff will only report on PHP4-style constructors if the configured PHP version is less than 8.0. +#### `Universal.CodeAnalysis.NoEchoSprintf` :wrench: :books: + +Detects use of the inefficient `echo [v]sprintf(...);` combi. Use `[v]printf()` instead. + #### `Universal.CodeAnalysis.ForeachUniqueAssignment` :wrench: :books: Detects `foreach` control structures which use the same variable for both the key as well as the value assignment as this will lead to unexpected - and most likely unintended - behaviour. @@ -278,6 +282,26 @@ Enforce for a file to either declare (global/namespaced) functions or declare OO * Also note: This sniff has no opinion on multiple OO structures being declared in one file. If you want to sniff for that, use the PHPCS native `Generic.Files.OneObjectStructurePerFile` sniff. +#### `Universal.FunctionDeclarations.NoLongClosures` :bar_chart: :books: + +Detects "long" closures and recommends using a named function instead. + +The sniff is configurable by setting any of the following properties in a custom ruleset: +* `recommendedLines` (int): determines when a warning will be thrown. + Defaults to `5`, meaning a warning with the errorcode `ExceedsRecommended` will be thrown if the closure is more than 5 lines long. +* `maxLines` (int): determines when an error will be thrown. + Defaults to `8`, meaning that an error with the errorcode `ExceedsMaximum` will be thrown if the closure is more than 8 lines long. +* `ignoreCommentLines` (bool): whether or not comment-only lines should be ignored for the lines count. + Defaults to `true`. +* `ignoreEmptyLines` (bool): whether or not blank lines should be ignored for the lines count. + Defaults to `true`. + +#### `Universal.FunctionDeclarations.RequireFinalMethodsInTraits` :wrench: :bar_chart: :books: + +Enforce non-private, non-abstract methods in traits to be declared as `final`. + +The available error codes are: `NonFinalMethodFound` and `NonFinalMagicMethodFound`. + #### `Universal.Lists.DisallowLongListSyntax` :wrench: :books: Disallow the use of long `list`s. @@ -368,6 +392,13 @@ The available error codes are: `UnionTypeSpacesBefore`, `UnionTypeSpacesAfter`, Disallow short open echo tags `