Skip to content

Commit

Permalink
PEAR/FunctionDeclaration: prevent fixer conflict for unfinished closu…
Browse files Browse the repository at this point in the history
…res/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
  • Loading branch information
jrfnl committed Feb 16, 2025
1 parent f117c2b commit 791e61b
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,10 @@ public function process(File $phpcsFile, $stackPtr)
// enforced by the previous check because there is no content between the keywords
// and the opening parenthesis.
// Unfinished closures are tokenized as T_FUNCTION however, and can be excluded
// by checking for the scope_opener.
// by checking if the function has a name.
$methodProps = $phpcsFile->getMethodProperties($stackPtr);
if ($tokens[$stackPtr]['code'] === T_FUNCTION
&& (isset($tokens[$stackPtr]['scope_opener']) === true || $methodProps['has_body'] === false)
) {
$methodName = $phpcsFile->getDeclarationName($stackPtr);
if ($tokens[$stackPtr]['code'] === T_FUNCTION && $methodName !== null) {
if ($tokens[($openBracket - 1)]['content'] === $phpcsFile->eolChar) {
$spaces = 'newline';
} else if ($tokens[($openBracket - 1)]['code'] === T_WHITESPACE) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

// Intentional parse error/live coding.
// Ensuring that the sniff does not get into a fixer conflict with itself for unfinished closure declarations
// (missing close parenthesis for import use).
// This must be the only test in this file.
$closure = function (string $param) use ($var
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

// Intentional parse error/live coding.
// Ensuring that the sniff does not get into a fixer conflict with itself for unfinished closure declarations
// (missing close parenthesis for import use).
// This must be the only test in this file.
$closure = function(string $param) use ($var
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<?php

// Intentional parse error/live coding.
// Ensuring that the sniff does not get into a fixer conflict with itself for unfinished closure declarations
// (missing close parenthesis for import use).
// This must be the only test in this file.
$closure = function (string $param) use ($var
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,11 @@ public function getErrorList($testFile='')
48 => 1,
];

case 'FunctionDeclarationUnitTest.4.inc':
return [
7 => 1,
];

default:
return [];
}//end switch
Expand Down

0 comments on commit 791e61b

Please sign in to comment.