Skip to content

Commit ad1de27

Browse files
klausiklausi
authored
feat(MultiLineFunctionDeclaration): Add new sniff for multi-line function declarations and trailing commas (#3440603)
1 parent 0c6ba2c commit ad1de27

9 files changed

+321
-22
lines changed

‎coder_sniffer/Drupal/Sniffs/Functions/FunctionDeclarationSniff.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* before the opening parenthesis.
1818
*
1919
* @deprecated in Coder 8.x, will be removed in Coder 9.x.
20-
* Squiz.Functions.MultiLineFunctionDeclaration is used instead, see ruleset.xml.
20+
* MultiLineFunctionDeclarationSniff is used instead.
2121
*
2222
* @category PHP
2323
* @package PHP_CodeSniffer

‎coder_sniffer/Drupal/Sniffs/Functions/MultiLineFunctionDeclarationSniff.php‎

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
<?php
2+
/**
3+
* \Drupal\Sniffs\Functions\MultiLineFunctionDeclarationSniff
4+
*
5+
* @category PHP
6+
* @package PHP_CodeSniffer
7+
* @link http://pear.php.net/package/PHP_CodeSniffer
8+
*/
9+
10+
namespace Drupal\Sniffs\Functions;
11+
12+
use PHP_CodeSniffer\Standards\Generic\Sniffs\Functions\OpeningFunctionBraceKernighanRitchieSniff;
13+
use PHP_CodeSniffer\Standards\PEAR\Sniffs\Functions\FunctionDeclarationSniff as PearFunctionDeclarationSniff;
14+
use PHP_CodeSniffer\Standards\Squiz\Sniffs\Functions\MultiLineFunctionDeclarationSniff as SquizFunctionDeclarationSniff;
15+
use PHP_CodeSniffer\Util\Tokens;
16+
17+
/**
18+
* Multi-line function declarations need to have a trailing comma on the last
19+
* parameter. Modified from Squiz, whenever there is a function declaration
20+
* closing parenthesis on a new line we treat it as multi-line.
21+
*
22+
* @category PHP
23+
* @package PHP_CodeSniffer
24+
* @link http://pear.php.net/package/PHP_CodeSniffer
25+
*/
26+
class MultiLineFunctionDeclarationSniff extends SquizFunctionDeclarationSniff
27+
{
28+
29+
30+
/**
31+
* The number of spaces code should be indented.
32+
*
33+
* @var integer
34+
*/
35+
public $indent = 2;
36+
37+
38+
/**
39+
* Processes single-line declarations.
40+
*
41+
* Just uses the Generic Kernighan Ritchie sniff.
42+
*
43+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
44+
* @param int $stackPtr The position of the current token
45+
* in the stack passed in $tokens.
46+
* @param array<int, array<string, mixed>> $tokens The stack of tokens that make up
47+
* the file.
48+
*
49+
* @return void
50+
*/
51+
public function processSingleLineDeclaration($phpcsFile, $stackPtr, $tokens)
52+
{
53+
$sniff = new OpeningFunctionBraceKernighanRitchieSniff();
54+
$sniff->checkClosures = true;
55+
$sniff->process($phpcsFile, $stackPtr);
56+
57+
}//end processSingleLineDeclaration()
58+
59+
60+
/**
61+
* Determine if this is a multi-line function declaration.
62+
*
63+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
64+
* @param int $stackPtr The position of the current token
65+
* in the stack passed in $tokens.
66+
* @param int $openBracket The position of the opening bracket
67+
* in the stack passed in $tokens.
68+
* @param array<int, array<string, mixed>> $tokens The stack of tokens that make up
69+
* the file.
70+
*
71+
* @return bool
72+
*/
73+
public function isMultiLineDeclaration($phpcsFile, $stackPtr, $openBracket, $tokens)
74+
{
75+
$function = $tokens[$stackPtr];
76+
if ($tokens[$function['parenthesis_opener']]['line'] === $tokens[$function['parenthesis_closer']]['line']) {
77+
return false;
78+
}
79+
80+
return true;
81+
82+
}//end isMultiLineDeclaration()
83+
84+
85+
/**
86+
* Processes multi-line declarations.
87+
*
88+
* @param \PHP_CodeSniffer\Files\File $phpcsFile The file being scanned.
89+
* @param int $stackPtr The position of the current token
90+
* in the stack passed in $tokens.
91+
* @param array<int, array<string, mixed>> $tokens The stack of tokens that make up
92+
* the file.
93+
*
94+
* @return void
95+
*/
96+
public function processMultiLineDeclaration($phpcsFile, $stackPtr, $tokens)
97+
{
98+
// We do everything the grandparent sniff does, and a bit more.
99+
PearFunctionDeclarationSniff::processMultiLineDeclaration($phpcsFile, $stackPtr, $tokens);
100+
101+
$openBracket = $tokens[$stackPtr]['parenthesis_opener'];
102+
$this->processBracket($phpcsFile, $openBracket, $tokens, 'function');
103+
104+
// Trailing commas on the last function parameter are only possible in
105+
// PHP 8.0+.
106+
if (version_compare(PHP_VERSION, '8.0.0') < 0) {
107+
return;
108+
}
109+
110+
$function = $tokens[$stackPtr];
111+
112+
$lastTrailingComma = $phpcsFile->findPrevious(
113+
Tokens::$emptyTokens,
114+
($function['parenthesis_closer'] - 1),
115+
$function['parenthesis_opener'],
116+
true
117+
);
118+
if ($tokens[$lastTrailingComma]['code'] !== T_COMMA) {
119+
$error = 'Multi-line function declarations must have a trailing comma after the last parameter';
120+
$fix = $phpcsFile->addFixableError($error, $lastTrailingComma, 'MissingTrailingComma');
121+
if ($fix === true) {
122+
$phpcsFile->fixer->addContent($lastTrailingComma, ',');
123+
}
124+
}
125+
126+
}//end processMultiLineDeclaration()
127+
128+
129+
}//end class

‎coder_sniffer/Drupal/ruleset.xml‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,6 @@
5353
<rule ref="Generic.Functions.FunctionCallArgumentSpacing.NoSpaceAfterComma">
5454
<severity>0</severity>
5555
</rule>
56-
<rule ref="Generic.Functions.OpeningFunctionBraceKernighanRitchie">
57-
<properties>
58-
<property name="checkClosures" value="true"/>
59-
</properties>
60-
</rule>
6156

6257
<rule ref="Generic.NamingConventions.ConstructorName" />
6358
<rule ref="Generic.NamingConventions.UpperCaseConstantName" />
@@ -255,19 +250,6 @@
255250
<severity>0</severity>
256251
</rule>
257252

258-
<rule ref="Squiz.Functions.MultiLineFunctionDeclaration">
259-
<properties>
260-
<property name="indent" value="2"/>
261-
</properties>
262-
</rule>
263-
<!-- Disable some errors that are already coverd by other sniffs. -->
264-
<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.BraceOnSameLine">
265-
<severity>0</severity>
266-
</rule>
267-
<rule ref="Squiz.Functions.MultiLineFunctionDeclaration.ContentAfterBrace">
268-
<severity>0</severity>
269-
</rule>
270-
271253
<rule ref="Squiz.PHP.LowercasePHPFunctions" />
272254
<rule ref="Squiz.PHP.NonExecutableCode" />
273255
<rule ref="Squiz.Strings.ConcatenationSpacing">

‎tests/Drupal/Functions/MultiLineFunctionDeclarationUnitTest.inc‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
/**
4+
* @file
5+
* Some description.
6+
*/
7+
8+
/**
9+
* Test.
10+
*/
11+
function missing_trailing_comma(
12+
$a,
13+
$b
14+
) {
15+
16+
}
17+
18+
$foo = 1;
19+
$bar = 2;
20+
$x = function (
21+
$a,
22+
$b
23+
) use (
24+
$foo,
25+
$bar
26+
) {
27+
28+
};
29+
30+
/**
31+
*
32+
*/
33+
class Test {
34+
35+
/**
36+
* Test.
37+
*/
38+
public function lookupSource(string $key, string $migrationNames = '', array $options = [
39+
'all' => NULL,
40+
'group' => NULL,
41+
]): void {
42+
}
43+
44+
}
45+
46+
$x = function (
47+
$a,
48+
$b,
49+
) use ($foo, $bar) {
50+
51+
};

‎tests/Drupal/Functions/MultiLineFunctionDeclarationUnitTest.inc.fixed‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php
2+
3+
/**
4+
* @file
5+
* Some description.
6+
*/
7+
8+
/**
9+
* Test.
10+
*/
11+
function missing_trailing_comma(
12+
$a,
13+
$b,
14+
) {
15+
16+
}
17+
18+
$foo = 1;
19+
$bar = 2;
20+
$x = function (
21+
$a,
22+
$b,
23+
) use (
24+
$foo,
25+
$bar
26+
) {
27+
28+
};
29+
30+
/**
31+
*
32+
*/
33+
class Test {
34+
35+
/**
36+
* Test.
37+
*/
38+
public function lookupSource(
39+
string $key,
40+
string $migrationNames = '',
41+
array $options = [
42+
'all' => NULL,
43+
'group' => NULL,
44+
],
45+
): void {
46+
}
47+
48+
}
49+
50+
$x = function (
51+
$a,
52+
$b,
53+
) use ($foo, $bar) {
54+
55+
};

‎tests/Drupal/Functions/MultiLineFunctionDeclarationUnitTest.php‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
<?php
2+
3+
namespace Drupal\Test\Functions;
4+
5+
use Drupal\Test\CoderSniffUnitTest;
6+
7+
class MultiLineFunctionDeclarationUnitTest extends CoderSniffUnitTest
8+
{
9+
10+
11+
/**
12+
* Returns the lines where errors should occur.
13+
*
14+
* The key of the array should represent the line number and the value
15+
* should represent the number of errors that should occur on that line.
16+
*
17+
* @param string $testFile The name of the file being tested.
18+
*
19+
* @return array<int, int>
20+
*/
21+
protected function getErrorList(string $testFile): array
22+
{
23+
return [
24+
13 => 1,
25+
22 => 1,
26+
38 => 3,
27+
41 => 2,
28+
];
29+
30+
}//end getErrorList()
31+
32+
33+
/**
34+
* Returns the lines where warnings should occur.
35+
*
36+
* The key of the array should represent the line number and the value
37+
* should represent the number of warnings that should occur on that line.
38+
*
39+
* @param string $testFile The name of the file being tested.
40+
*
41+
* @return array<int, int>
42+
*/
43+
protected function getWarningList(string $testFile): array
44+
{
45+
return [];
46+
47+
}//end getWarningList()
48+
49+
50+
/**
51+
* Skip this test on PHP versions lower than 8 because the syntax is not allowed there.
52+
*
53+
* @return bool
54+
*/
55+
protected function shouldSkipTest()
56+
{
57+
if (version_compare(PHP_VERSION, '8.0.0') < 0) {
58+
return true;
59+
}
60+
61+
return false;
62+
63+
}//end shouldSkipTest()
64+
65+
66+
}//end class

‎tests/Drupal/bad/BadUnitTest.php‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -382,7 +382,7 @@ protected function getErrorList(string $testFile): array
382382
836 => 1,
383383
838 => 1,
384384
849 => 2,
385-
860 => 1,
385+
860 => 2,
386386
864 => 2,
387387
];
388388
}//end switch
@@ -480,4 +480,20 @@ protected function checkAllSniffCodes()
480480
}//end checkAllSniffCodes()
481481

482482

483+
/**
484+
* Skip this test on PHP versions lower than 8 because of MultiLineTrailingCommaSniff.
485+
*
486+
* @return bool
487+
*/
488+
protected function shouldSkipTest()
489+
{
490+
if (version_compare(PHP_VERSION, '8.0.0') < 0) {
491+
return true;
492+
}
493+
494+
return false;
495+
496+
}//end shouldSkipTest()
497+
498+
483499
}//end class

‎tests/Drupal/bad/bad.php.fixed‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -911,7 +911,7 @@ class ScopeKeyword {
911911
*/
912912
function test29(
913913
int $a,
914-
string $b
914+
string $b,
915915
) {
916916
echo "Hello";
917917
}

‎tests/Drupal/good/good.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1476,7 +1476,7 @@ function test18(
14761476
CacheTagsInvalidatorInterface $cache_invalidator,
14771477
ModuleHandlerInterface $module_handler,
14781478
EntityFieldManagerInterface $entity_field_manager,
1479-
EntityTypeBundleInfoInterface $entity_type_bundle_info
1479+
EntityTypeBundleInfoInterface $entity_type_bundle_info,
14801480
) {
14811481
return 0;
14821482
}

0 commit comments

Comments
 (0)