From 1c295bb0121247ca06ed4efd2d4e85d710da75d0 Mon Sep 17 00:00:00 2001 From: "Eloy Lafuente (stronk7)" Date: Wed, 29 Nov 2023 11:43:50 +0100 Subject: [PATCH] Pick the PHPCSExtra CommaAfterLastSniff Sniff to moodle We need this applied ASAP, because it's causing troubles with many arrays that are allowed, so we cannot wait for PHPCSExtra v1.2.0 to be released. So, with this commit, we are moving to own (copied verbatim) Sniff. And adjusting tests and other small bits. Note that this commit will be 100% reverted once PHPCSExtra is released and then we'll be back to the upstream Sniff. See https://github.com/moodlehq/moodle-cs/issues/82 --- moodle/Sniffs/Arrays/CommaAfterLastSniff.php | 226 ++++++++++++++++++ ...rmalizedArraysArraysCommaAfterLastTest.php | 8 +- ...normalizedarrays_arrays_commaafterlast.php | 2 +- ...izedarrays_arrays_commaafterlast.php.fixed | 2 +- moodle/ruleset.xml | 7 +- 5 files changed, 238 insertions(+), 7 deletions(-) create mode 100644 moodle/Sniffs/Arrays/CommaAfterLastSniff.php diff --git a/moodle/Sniffs/Arrays/CommaAfterLastSniff.php b/moodle/Sniffs/Arrays/CommaAfterLastSniff.php new file mode 100644 index 0000000..0345cc3 --- /dev/null +++ b/moodle/Sniffs/Arrays/CommaAfterLastSniff.php @@ -0,0 +1,226 @@ + + */ + private $validValues = [ + 'enforce' => true, + 'forbid' => true, + 'skip' => true, + ]; + + /** + * Returns an array of tokens this test wants to listen for. + * + * @since 1.0.0 + * + * @return array + */ + public function register() + { + return Collections::arrayOpenTokensBC(); + } + + /** + * Processes this test, when one of its tokens is encountered. + * + * @since 1.0.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) + { + // Validate the property input. Invalid values will result in the check being skipped. + if (isset($this->validValues[$this->singleLine]) === false) { + $this->singleLine = 'skip'; + } + if (isset($this->validValues[$this->multiLine]) === false) { + $this->multiLine = 'skip'; + } + + $openClose = Arrays::getOpenClose($phpcsFile, $stackPtr); + if ($openClose === false) { + // Short list, real square bracket, live coding or parse error. + return; + } + + $tokens = $phpcsFile->getTokens(); + $opener = $openClose['opener']; + $closer = $openClose['closer']; + + $action = $this->singleLine; + $phrase = 'single-line'; + $errorCode = 'SingleLine'; + if ($tokens[$opener]['line'] !== $tokens[$closer]['line']) { + $action = $this->multiLine; + $phrase = 'multi-line'; + $errorCode = 'MultiLine'; + } + + if ($action === 'skip') { + // Nothing to do. + return; + } + + $lastNonEmpty = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($closer - 1), $opener, true); + if ($lastNonEmpty === false || $lastNonEmpty === $opener) { + // Bow out: empty array. + return; + } + + // If the closer is on the same line as the last element, change the error code for multi-line arrays. + if ($errorCode === 'MultiLine' + && $tokens[$lastNonEmpty]['line'] === $tokens[$closer]['line'] + ) { + $errorCode .= 'CloserSameLine'; + } + + $isComma = ($tokens[$lastNonEmpty]['code'] === \T_COMMA); + + $phpcsFile->recordMetric( + $stackPtr, + \sprintf(self::METRIC_NAME, \ucfirst($phrase)), + ($isComma === true ? 'yes' : 'no') + ); + + switch ($action) { + case 'enforce': + if ($isComma === true) { + return; + } + + $error = 'There should be a comma after the last array item in a %s array.'; + $errorCode = 'Missing' . $errorCode; + $data = [$phrase]; + $fix = $phpcsFile->addFixableError($error, $lastNonEmpty, $errorCode, $data); + if ($fix === true) { + $extraContent = ','; + + if (($tokens[$lastNonEmpty]['code'] === \T_END_HEREDOC + || $tokens[$lastNonEmpty]['code'] === \T_END_NOWDOC) + // Check for indentation, if indented, it's a PHP 7.3+ heredoc/nowdoc. + && $tokens[$lastNonEmpty]['content'] === \ltrim($tokens[$lastNonEmpty]['content']) + ) { + // Prevent parse errors in PHP < 7.3 which doesn't support flexible heredoc/nowdoc. + $extraContent = $phpcsFile->eolChar . $extraContent; + } + + $phpcsFile->fixer->addContent($lastNonEmpty, $extraContent); + } + + return; + + case 'forbid': + if ($isComma === false) { + return; + } + + $error = 'A comma after the last array item in a %s array is not allowed.'; + $errorCode = 'Found' . $errorCode; + $data = [$phrase]; + $fix = $phpcsFile->addFixableError($error, $lastNonEmpty, $errorCode, $data); + if ($fix === true) { + $start = $lastNonEmpty; + $end = $lastNonEmpty; + + // Make sure we're not leaving a superfluous blank line behind. + $prevNonWhitespace = $phpcsFile->findPrevious(\T_WHITESPACE, ($lastNonEmpty - 1), $opener, true); + $nextNonWhitespace = $phpcsFile->findNext(\T_WHITESPACE, ($lastNonEmpty + 1), ($closer + 1), true); + if ($prevNonWhitespace !== false + && $tokens[$prevNonWhitespace]['line'] < $tokens[$lastNonEmpty]['line'] + && $nextNonWhitespace !== false + && $tokens[$nextNonWhitespace]['line'] > $tokens[$lastNonEmpty]['line'] + ) { + $start = ($prevNonWhitespace + 1); + } + + $phpcsFile->fixer->beginChangeset(); + + for ($i = $start; $i <= $end; $i++) { + $phpcsFile->fixer->replaceToken($i, ''); + } + + $phpcsFile->fixer->endChangeset(); + } + + return; + } + } +} diff --git a/moodle/Tests/NormalizedArraysArraysCommaAfterLastTest.php b/moodle/Tests/NormalizedArraysArraysCommaAfterLastTest.php index 7ab7254..ca882d2 100644 --- a/moodle/Tests/NormalizedArraysArraysCommaAfterLastTest.php +++ b/moodle/Tests/NormalizedArraysArraysCommaAfterLastTest.php @@ -26,7 +26,7 @@ * @copyright 2023 onwards Eloy Lafuente (stronk7) {@link http://stronk7.com} * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later * - * @covers \PHPCSExtra\NormalizedArrays\Sniffs\Arrays\CommaAfterLastSniff + * @covers MoodleHQ\MoodleCS\moodle\Sniffs\Arrays\CommaAfterLastSniff */ class NormalizedArraysArraysCommaAfterLastTest extends MoodleCSBaseTestCase { @@ -37,7 +37,7 @@ public function testNormalizedArraysArraysCommaAfterLast() { // Define the standard, sniff and fixture to use. $this->set_standard('moodle'); - $this->set_sniff('NormalizedArrays.Arrays.CommaAfterLast'); + $this->set_sniff('moodle.Arrays.CommaAfterLast'); $this->set_fixture(__DIR__ . '/fixtures/normalizedarrays_arrays_commaafterlast.php'); // Define expected results (errors and warnings). Format, array of: @@ -45,8 +45,8 @@ public function testNormalizedArraysArraysCommaAfterLast() { // - line => array of contents for message / source problem matching. // - line => string of contents for message / source problem matching (only 1). $this->set_errors([ - 79 => '@Source: NormalizedArrays.Arrays.CommaAfterLast.FoundSingleLine', - 82 => '@Source: NormalizedArrays.Arrays.CommaAfterLast.MissingMultiLine', + 79 => '@Source: moodle.Arrays.CommaAfterLast.FoundSingleLine', + 82 => '@Source: moodle.Arrays.CommaAfterLast.MissingMultiLine', 87 => 1, 95 => 1, 97 => 1, diff --git a/moodle/Tests/fixtures/normalizedarrays_arrays_commaafterlast.php b/moodle/Tests/fixtures/normalizedarrays_arrays_commaafterlast.php index 582efe1..5ffb6e7 100644 --- a/moodle/Tests/fixtures/normalizedarrays_arrays_commaafterlast.php +++ b/moodle/Tests/fixtures/normalizedarrays_arrays_commaafterlast.php @@ -2,7 +2,7 @@ defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures. // Disabling this error as we do in our ruleset, so tests run the same. -// phpcs:disable NormalizedArrays.Arrays.CommaAfterLast.MissingMultiLineCloserSameLine +// phpcs:disable moodle.Arrays.CommaAfterLast.MissingMultiLineCloserSameLine // All these are good / valid code. diff --git a/moodle/Tests/fixtures/normalizedarrays_arrays_commaafterlast.php.fixed b/moodle/Tests/fixtures/normalizedarrays_arrays_commaafterlast.php.fixed index 4b04d8e..e5ef10b 100644 --- a/moodle/Tests/fixtures/normalizedarrays_arrays_commaafterlast.php.fixed +++ b/moodle/Tests/fixtures/normalizedarrays_arrays_commaafterlast.php.fixed @@ -2,7 +2,7 @@ defined('MOODLE_INTERNAL') || die(); // Make this always the 1st line in all CS fixtures. // Disabling this error as we do in our ruleset, so tests run the same. -// phpcs:disable NormalizedArrays.Arrays.CommaAfterLast.MissingMultiLineCloserSameLine +// phpcs:disable moodle.Arrays.CommaAfterLast.MissingMultiLineCloserSameLine // All these are good / valid code. diff --git a/moodle/ruleset.xml b/moodle/ruleset.xml index 775c127..74ec915 100644 --- a/moodle/ruleset.xml +++ b/moodle/ruleset.xml @@ -20,10 +20,15 @@ Affects all major branches since Moodle 3.9. Require a comma after the last element in a multi-line array, but prevent in a single-line array definition - --> + + TODO: Enable this back and remove our copied Sniff below once PHPCSExtra v1.2.0 is released. + --> + + +