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",