Skip to content

Commit e8abd9f

Browse files
committed
PEAR/FunctionDeclaration: prevent fixer conflict for unfinished closures/live coding
The `PEAR.Functions.FunctionDeclaration` sniff contained code to protect against a fixer conflict for unfinished closures, however, this code did not work correctly as an unfinished closure will generally also not have a function body, which "undoes" the protection via the scope opener check. In other words, the fixer conflict still existed and would result in one part of the sniff trying to _add_ a space between the `function` keyword and the open parenthesis, while another part of the sniff would be removing that space again. ``` => Fixing file: 1/1 violations remaining PEAR.Functions.FunctionDeclaration:124 replaced token 11 (T_WHITESPACE on line 7) " (" => "(" => Fixing file: 1/1 violations remaining [made 1 pass]... * fixed 1 violations, starting loop 2 * PEAR.Functions.FunctionDeclaration:94 replaced token 10 (T_FUNCTION on line 7) "function" => "function " => Fixing file: 1/1 violations remaining [made 2 passes]... * fixed 1 violations, starting loop 3 * PEAR.Functions.FunctionDeclaration:124 replaced token 11 (T_WHITESPACE on line 7) " (" => "(" => Fixing file: 1/1 violations remaining [made 3 passes]... * fixed 1 violations, starting loop 4 * PEAR.Functions.FunctionDeclaration:94 replaced token 10 (T_FUNCTION on line 7) "function" => "function " => Fixing file: 1/1 violations remaining [made 4 passes]... * fixed 1 violations, starting loop 5 * ``` Fixed now by verifying if the function is named instead. That way we can be sure it's not a closure. Includes test. Builds on the previously pulled fix from PR 816. Related to #152
1 parent 5fc33cc commit e8abd9f

5 files changed

+29
-4
lines changed

src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php

+3-4
Original file line numberDiff line numberDiff line change
@@ -93,11 +93,10 @@ public function process(File $phpcsFile, $stackPtr)
9393
// enforced by the previous check because there is no content between the keywords
9494
// and the opening parenthesis.
9595
// Unfinished closures are tokenized as T_FUNCTION however, and can be excluded
96-
// by checking for the scope_opener.
96+
// by checking if the function has a name.
9797
$methodProps = $phpcsFile->getMethodProperties($stackPtr);
98-
if ($tokens[$stackPtr]['code'] === T_FUNCTION
99-
&& (isset($tokens[$stackPtr]['scope_opener']) === true || $methodProps['has_body'] === false)
100-
) {
98+
$methodName = $phpcsFile->getDeclarationName($stackPtr);
99+
if ($tokens[$stackPtr]['code'] === T_FUNCTION && $methodName !== null) {
101100
if ($tokens[($openBracket - 1)]['content'] === $phpcsFile->eolChar) {
102101
$spaces = 'newline';
103102
} else if ($tokens[($openBracket - 1)]['code'] === T_WHITESPACE) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
// Intentional parse error/live coding.
4+
// Ensuring that the sniff does not get into a fixer conflict with itself for unfinished closure declarations
5+
// (missing close parenthesis for import use).
6+
// This must be the only test in this file.
7+
$closure = function (string $param) use ($var
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
// Intentional parse error/live coding.
4+
// Ensuring that the sniff does not get into a fixer conflict with itself for unfinished closure declarations
5+
// (missing close parenthesis for import use).
6+
// This must be the only test in this file.
7+
$closure = function(string $param) use ($var
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
<?php
2+
3+
// Intentional parse error/live coding.
4+
// Ensuring that the sniff does not get into a fixer conflict with itself for unfinished closure declarations
5+
// (missing close parenthesis for import use).
6+
// This must be the only test in this file.
7+
$closure = function (string $param) use ($var

src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php

+5
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,11 @@ public function getErrorList($testFile='')
110110
490 => 2,
111111
];
112112

113+
case 'FunctionDeclarationUnitTest.4.inc':
114+
return [
115+
7 => 1,
116+
];
117+
113118
default:
114119
return [];
115120
}//end switch

0 commit comments

Comments
 (0)