Skip to content

Commit

Permalink
Pick the PHPCSExtra CommaAfterLastSniff Sniff to moodle
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
stronk7 committed Nov 29, 2023
1 parent d3b8064 commit 5ff59e5
Show file tree
Hide file tree
Showing 5 changed files with 238 additions and 7 deletions.
226 changes: 226 additions & 0 deletions moodle/Sniffs/Arrays/CommaAfterLastSniff.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
<?php
/**
* PHPCSExtra, a collection of sniffs and standards for use with PHP_CodeSniffer.
*
* @package PHPCSExtra
* @copyright 2020 PHPCSExtra Contributors
* @license https://opensource.org/licenses/LGPL-3.0 LGPL3
* @link https://github.com/PHPCSStandards/PHPCSExtra
*/

namespace MoodleHQ\MoodleCS\moodle\Sniffs\Arrays;

use PHP_CodeSniffer\Files\File;
use PHP_CodeSniffer\Sniffs\Sniff;
use PHP_CodeSniffer\Util\Tokens;
use PHPCSUtils\Tokens\Collections;
use PHPCSUtils\Utils\Arrays;

/**
* Enforce/forbid a comma after the last item in an array declaration.
*
* Allows for different settings for single-line and multi-line arrays.
*
* @since 1.0.0
*/
final class CommaAfterLastSniff implements Sniff
{

/**
* Name of the metric.
*
* @since 1.0.0
*
* @var string
*/
const METRIC_NAME = '%s array - comma after last item';

/**
* Whether or not to enforce a comma after the last array item in a single-line array.
*
* Valid values:
* - 'enforce' to always demand a comma after the last item in single-line arrays;
* - 'forbid' to disallow a comma after the last item in single-line arrays;
* - 'skip' to ignore single-line arrays.
*
* Defaults to: 'forbid'.
*
* @since 1.0.0
*
* @var string
*/
public $singleLine = 'forbid';

/**
* Whether or not to enforce a comma after the last array item in a multi-line array.
*
* Valid values:
* - 'enforce' to always demand a comma after the last item in single-line arrays;
* - 'forbid' to disallow a comma after the last item in single-line arrays;
* - 'skip' to ignore multi-line arrays.
*
* Defaults to: 'enforce'.
*
* @since 1.0.0
*
* @var string
*/
public $multiLine = 'enforce';

/**
* The input values to consider as valid for the public properties.
*
* Used for input validation.
*
* @since 1.0.0
*
* @var array<string, true>
*/
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<int|string>
*/
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;
}
}
}
8 changes: 4 additions & 4 deletions moodle/Tests/NormalizedArraysArraysCommaAfterLastTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -37,16 +37,16 @@ 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:
// - line => number of problems, or
// - 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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
7 changes: 6 additions & 1 deletion moodle/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
<rule ref="NormalizedArrays.Arrays.CommaAfterLast">
<exclude name="NormalizedArrays.Arrays.CommaAfterLast.MissingMultiLineCloserSameLine" />
</rule>
-->
<rule ref="moodle.Arrays.CommaAfterLast">
<exclude name="moodle.Arrays.CommaAfterLast.MissingMultiLineCloserSameLine" />
</rule>

<rule ref="Generic.Classes.DuplicateClassName"/>
<rule ref="Generic.Classes.OpeningBraceSameLine"/>
Expand Down

0 comments on commit 5ff59e5

Please sign in to comment.