From 791e61b46d7d58a8aba90a7244304c9ef616fff7 Mon Sep 17 00:00:00 2001
From: jrfnl <jrfnl@users.noreply.github.com>
Date: Thu, 13 Feb 2025 00:03:49 +0100
Subject: [PATCH] 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
---
 .../PEAR/Sniffs/Functions/FunctionDeclarationSniff.php     | 7 +++----
 .../PEAR/Tests/Functions/FunctionDeclarationUnitTest.3.inc | 7 +++++++
 .../PEAR/Tests/Functions/FunctionDeclarationUnitTest.4.inc | 7 +++++++
 .../Functions/FunctionDeclarationUnitTest.4.inc.fixed      | 7 +++++++
 .../PEAR/Tests/Functions/FunctionDeclarationUnitTest.php   | 5 +++++
 5 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100644 src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.3.inc
 create mode 100644 src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.4.inc
 create mode 100644 src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.4.inc.fixed

diff --git a/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php b/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php
index 1d0745b98b..93ccd89a2c 100644
--- a/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php
+++ b/src/Standards/PEAR/Sniffs/Functions/FunctionDeclarationSniff.php
@@ -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) {
diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.3.inc b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.3.inc
new file mode 100644
index 0000000000..f0253bbb3e
--- /dev/null
+++ b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.3.inc
@@ -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
diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.4.inc b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.4.inc
new file mode 100644
index 0000000000..f6f735c227
--- /dev/null
+++ b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.4.inc
@@ -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
diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.4.inc.fixed b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.4.inc.fixed
new file mode 100644
index 0000000000..f0253bbb3e
--- /dev/null
+++ b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.4.inc.fixed
@@ -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
diff --git a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php
index c81e7d4e99..1971a5dbe0 100644
--- a/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php
+++ b/src/Standards/PEAR/Tests/Functions/FunctionDeclarationUnitTest.php
@@ -124,6 +124,11 @@ public function getErrorList($testFile='')
                 48 => 1,
             ];
 
+        case 'FunctionDeclarationUnitTest.4.inc':
+            return [
+                7 => 1,
+            ];
+
         default:
             return [];
         }//end switch