Skip to content

Commit 791e61b

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 f117c2b commit 791e61b

5 files changed

+29
-4
lines changed

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

+3-4
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,10 @@ public function process(File $phpcsFile, $stackPtr)
103103
// enforced by the previous check because there is no content between the keywords
104104
// and the opening parenthesis.
105105
// Unfinished closures are tokenized as T_FUNCTION however, and can be excluded
106-
// by checking for the scope_opener.
106+
// by checking if the function has a name.
107107
$methodProps = $phpcsFile->getMethodProperties($stackPtr);
108-
if ($tokens[$stackPtr]['code'] === T_FUNCTION
109-
&& (isset($tokens[$stackPtr]['scope_opener']) === true || $methodProps['has_body'] === false)
110-
) {
108+
$methodName = $phpcsFile->getDeclarationName($stackPtr);
109+
if ($tokens[$stackPtr]['code'] === T_FUNCTION && $methodName !== null) {
111110
if ($tokens[($openBracket - 1)]['content'] === $phpcsFile->eolChar) {
112111
$spaces = 'newline';
113112
} 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
@@ -124,6 +124,11 @@ public function getErrorList($testFile='')
124124
48 => 1,
125125
];
126126

127+
case 'FunctionDeclarationUnitTest.4.inc':
128+
return [
129+
7 => 1,
130+
];
131+
127132
default:
128133
return [];
129134
}//end switch

0 commit comments

Comments
 (0)