feat(MultiLineFunctionDeclaration): Add new sniff for multi-line func…

…tion declarations and trailing commas

coder_sniffer/Drupal/Sniffs/Functions/FunctionDeclarationSniff.php

@@ -17,7 +17,7 @@
  * before the opening parenthesis.
  *
  * @deprecated in Coder 8.x, will be removed in Coder 9.x.
- * Squiz.Functions.MultiLineFunctionDeclaration is used instead, see ruleset.xml.
+ * MultiLineFunctionDeclarationSniff is used instead.
  *
  * @category PHP
  * @package  PHP_CodeSniffer

coder_sniffer/Drupal/Sniffs/Functions/MultiLineFunctionDeclarationSniff.php

@@ -0,0 +1,125 @@
+<?php
+/**
+ * \Drupal\Sniffs\Functions\MultiLineTrailingCommaSniff.
+ *
+ * @category PHP
+ * @package  PHP_CodeSniffer
+ * @link     http://pear.php.net/package/PHP_CodeSniffer
+ */
+
+ namespace Drupal\Sniffs\Functions;
+
+use PHP_CodeSniffer\Standards\Generic\Sniffs\Functions\OpeningFunctionBraceKernighanRitchieSniff;
+use PHP_CodeSniffer\Standards\Squiz\Sniffs\Functions\MultiLineFunctionDeclarationSniff as SquizFunctionDeclarationSniff;
+use PHP_CodeSniffer\Util\Tokens;
+
+/**
+ * Multi-line function declarations need to have a trailing comma on the last
+ * parameter. Modified from Squiz, whenever there is a function declaration
+ * closing parenthesis on a new line we treat it as multi-line.
+ *
+ * @category PHP
+ * @package  PHP_CodeSniffer
+ * @link     http://pear.php.net/package/PHP_CodeSniffer
+ */
+class MultiLineFunctionDeclarationSniff extends SquizFunctionDeclarationSniff
+{
+
+
+    /**
+     * The number of spaces code should be indented.
+     *
+     * @var integer
+     */
+    public $indent = 2;
+
+
+    /**
+     * Processes single-line declarations.
+     *
+     * Just uses the Generic Kernighan Ritchie sniff.
+     *
+     * @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.
+     * @param array<int, array<string, mixed>> $tokens    The stack of tokens that make up
+     *                                                    the file.
+     *
+     * @return void
+     */
+    public function processSingleLineDeclaration($phpcsFile, $stackPtr, $tokens)
+    {
+        $sniff = new OpeningFunctionBraceKernighanRitchieSniff();
+        $sniff->checkClosures = true;
+        $sniff->process($phpcsFile, $stackPtr);
+
+    }//end processSingleLineDeclaration()
+
+
+    /**
+     * Determine if this is a multi-line function declaration.
+     *
+     * @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.
+     * @param int                              $openBracket The position of the opening bracket
+     *                                                      in the stack passed in $tokens.
+     * @param array<int, array<string, mixed>> $tokens      The stack of tokens that make up
+     *                                                      the file.
+     *
+     * @return bool
+     */
+    public function isMultiLineDeclaration($phpcsFile, $stackPtr, $openBracket, $tokens)
+    {
+        $function = $tokens[$stackPtr];
+        if ($tokens[$function['parenthesis_opener']]['line'] === $tokens[$function['parenthesis_closer']]['line']) {
+            return false;
+        }
+
+        return true;
+
+    }//end isMultiLineDeclaration()
+
+
+    /**
+     * Processes multi-line declarations.
+     *
+     * @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.
+     * @param array<int, array<string, mixed>> $tokens    The stack of tokens that make up
+     *                                                    the file.
+     *
+     * @return void
+     */
+    public function processMultiLineDeclaration($phpcsFile, $stackPtr, $tokens)
+    {
+        // We do everything the parent sniff does, and a bit more.
+        parent::processMultiLineDeclaration($phpcsFile, $stackPtr, $tokens);
+
+        // Trailing commas on the last function parameter are only possible in
+        // PHP 8.0+.
+        if (version_compare(PHP_VERSION, '8.0.0') < 0) {
+            return;
+        }
+
+        $function = $tokens[$stackPtr];
+
+        $lastTrailingComma = $phpcsFile->findPrevious(
+            Tokens::$emptyTokens,
+            ($function['parenthesis_closer'] - 1),
+            $function['parenthesis_opener'],
+            true
+        );
+        if ($tokens[$lastTrailingComma]['code'] !== T_COMMA) {
+            $error = 'Multi-line function declarations must have a trailing comma after the last parameter';
+            $fix   = $phpcsFile->addFixableError($error, $lastTrailingComma, 'MissingTrailingComma');
+            if ($fix === true) {
+                $phpcsFile->fixer->addContent($lastTrailingComma, ',');
+            }
+        }
+
+    }//end processMultiLineDeclaration()
+
+
+}//end class

coder_sniffer/Drupal/ruleset.xml

@@ -53,11 +53,6 @@
  <rule ref="Generic.Functions.FunctionCallArgumentSpacing.NoSpaceAfterComma">
   <severity>0</severity>
  </rule>
- <rule ref="Generic.Functions.OpeningFunctionBraceKernighanRitchie">
-  <properties>
-   <property name="checkClosures" value="true"/>
-  </properties>
- </rule>
 
  <rule ref="Generic.NamingConventions.ConstructorName" />
  <rule ref="Generic.NamingConventions.UpperCaseConstantName" />
@@ -255,19 +250,6 @@
   <severity>0</severity>
  </rule>
 
- <rule ref="Squiz.Functions.MultiLineFunctionDeclaration">
-  <properties>
-   <property name="indent" value="2"/>
-  </properties>
- </rule>
- <!-- Disable some errors that are already coverd by other sniffs. -->
- <rule ref="Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine">
-  <severity>0</severity>
- </rule>
- <rule ref="Squiz.Functions.MultiLineFunctionDeclaration.ContentAfterBrace">
-  <severity>0</severity>
- </rule>
-
  <rule ref="Squiz.PHP.LowercasePHPFunctions" />
  <rule ref="Squiz.PHP.NonExecutableCode" />
  <rule ref="Squiz.Strings.ConcatenationSpacing">

tests/Drupal/Functions/MultiLineFunctionDeclarationUnitTest.inc

@@ -0,0 +1,44 @@
+<?php
+
+/**
+ * @file
+ * Some description.
+ */
+
+/**
+ * Test.
+ */
+function missing_trailing_comma(
+  $a,
+  $b
+) {
+
+}
+
+$foo = 1;
+$bar = 2;
+$x = function (
+  $a,
+  $b
+) use (
+  $foo,
+  $bar
+) {
+
+};
+
+/**
+ *
+ */
+class Test {
+
+  /**
+   * Test.
+   */
+  public function lookupSource(string $key, string $migrationNames = '', array $options = [
+    'all' => NULL,
+    'group' => NULL,
+  ]): void {
+  }
+
+}

tests/Drupal/Functions/MultiLineFunctionDeclarationUnitTest.inc.fixed

@@ -0,0 +1,48 @@
+<?php
+
+/**
+ * @file
+ * Some description.
+ */
+
+/**
+ * Test.
+ */
+function missing_trailing_comma(
+  $a,
+  $b,
+) {
+
+}
+
+$foo = 1;
+$bar = 2;
+$x = function (
+  $a,
+  $b,
+) use (
+  $foo,
+  $bar
+) {
+
+};
+
+/**
+ *
+ */
+class Test {
+
+  /**
+   * Test.
+   */
+  public function lookupSource(
+    string $key,
+    string $migrationNames = '',
+    array $options = [
+      'all' => NULL,
+      'group' => NULL,
+    ],
+  ): void {
+  }
+
+}

tests/Drupal/Functions/MultiLineTrailingCommaUnitTest.php → tests/Drupal/Functions/MultiLineFunctionDeclarationUnitTest.php

@@ -4,7 +4,7 @@
 
 use Drupal\Test\CoderSniffUnitTest;
 
-class MultiLineTrailingCommaUnitTest extends CoderSniffUnitTest
+class MultiLineFunctionDeclarationUnitTest extends CoderSniffUnitTest
 {
 
 
@@ -21,8 +21,10 @@ class MultiLineTrailingCommaUnitTest extends CoderSniffUnitTest
     protected function getErrorList(string $testFile): array
     {
         return [
-            8  => 1,
-            17 => 1,
+            13 => 1,
+            22 => 1,
+            38 => 3,
+            41 => 2,
         ];
 
     }//end getErrorList()