Merge pull request #502 from PHPCSStandards/feature/110-437-generic-s…

…copeindent-fix-undefined-array-index-notice File::findStartOfStatement(): 3 bug fixes related to `match` expressions
PHPCSStandards · May 21, 2024 · 027c0cb · 027c0cb
2 parents 808dff8 + 28c376e
commit 027c0cb
Show file tree

Hide file tree

Showing 8 changed files with 553 additions and 44 deletions.
diff --git a/src/Files/File.php b/src/Files/File.php
@@ -2435,51 +2435,88 @@ public function findStartOfStatement($start, $ignore=null)
         // If the start token is inside the case part of a match expression,
         // find the start of the condition. If it's in the statement part, find
         // the token that comes after the match arrow.
-        $matchExpression = $this->getCondition($start, T_MATCH);
-        if ($matchExpression !== false) {
-            for ($prevMatch = $start; $prevMatch > $this->tokens[$matchExpression]['scope_opener']; $prevMatch--) {
-                if ($prevMatch !== $start
-                    && ($this->tokens[$prevMatch]['code'] === T_MATCH_ARROW
-                    || $this->tokens[$prevMatch]['code'] === T_COMMA)
-                ) {
-                    break;
-                }
+        if (empty($this->tokens[$start]['conditions']) === false) {
+            $conditions         = $this->tokens[$start]['conditions'];
+            $lastConditionOwner = end($conditions);
+            $matchExpression    = key($conditions);
+
+            if ($lastConditionOwner === T_MATCH
+                // Check if the $start token is at the same parentheses nesting level as the match token.
+                && ((empty($this->tokens[$matchExpression]['nested_parenthesis']) === true
+                && empty($this->tokens[$start]['nested_parenthesis']) === true)
+                || ((empty($this->tokens[$matchExpression]['nested_parenthesis']) === false
+                && empty($this->tokens[$start]['nested_parenthesis']) === false)
+                && $this->tokens[$matchExpression]['nested_parenthesis'] === $this->tokens[$start]['nested_parenthesis']))
+            ) {
+                // Walk back to the previous match arrow (if it exists).
+                $lastComma          = null;
+                $inNestedExpression = false;
+                for ($prevMatch = $start; $prevMatch > $this->tokens[$matchExpression]['scope_opener']; $prevMatch--) {
+                    if ($prevMatch !== $start && $this->tokens[$prevMatch]['code'] === T_MATCH_ARROW) {
+                        break;
+                    }
 
-                // Skip nested statements.
-                if (isset($this->tokens[$prevMatch]['bracket_opener']) === true
-                    && $prevMatch === $this->tokens[$prevMatch]['bracket_closer']
-                ) {
-                    $prevMatch = $this->tokens[$prevMatch]['bracket_opener'];
-                } else if (isset($this->tokens[$prevMatch]['parenthesis_opener']) === true
-                    && $prevMatch === $this->tokens[$prevMatch]['parenthesis_closer']
-                ) {
-                    $prevMatch = $this->tokens[$prevMatch]['parenthesis_opener'];
-                }
-            }
+                    if ($prevMatch !== $start && $this->tokens[$prevMatch]['code'] === T_COMMA) {
+                        $lastComma = $prevMatch;
+                        continue;
+                    }
 
-            if ($prevMatch <= $this->tokens[$matchExpression]['scope_opener']) {
-                // We're before the arrow in the first case.
-                $next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true);
-                if ($next === false) {
-                    return $start;
-                }
+                    // Skip nested statements.
+                    if (isset($this->tokens[$prevMatch]['bracket_opener']) === true
+                        && $prevMatch === $this->tokens[$prevMatch]['bracket_closer']
+                    ) {
+                        $prevMatch = $this->tokens[$prevMatch]['bracket_opener'];
+                        continue;
+                    }
 
-                return $next;
-            }
+                    if (isset($this->tokens[$prevMatch]['parenthesis_opener']) === true
+                        && $prevMatch === $this->tokens[$prevMatch]['parenthesis_closer']
+                    ) {
+                        $prevMatch = $this->tokens[$prevMatch]['parenthesis_opener'];
+                        continue;
+                    }
 
-            if ($this->tokens[$prevMatch]['code'] === T_COMMA) {
-                // We're before the arrow, but not in the first case.
-                $prevMatchArrow = $this->findPrevious(T_MATCH_ARROW, ($prevMatch - 1), $this->tokens[$matchExpression]['scope_opener']);
-                if ($prevMatchArrow === false) {
-                    // We're before the arrow in the first case.
-                    $next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true);
-                    return $next;
-                }
+                    // Stop if we're _within_ a nested short array statement, which may contain comma's too.
+                    // No need to deal with parentheses, those are handled above via the `nested_parenthesis` checks.
+                    if (isset($this->tokens[$prevMatch]['bracket_opener']) === true
+                        && $this->tokens[$prevMatch]['bracket_closer'] > $start
+                    ) {
+                        $inNestedExpression = true;
+                        break;
+                    }
+                }//end for
+
+                if ($inNestedExpression === false) {
+                    // $prevMatch will now either be the scope opener or a match arrow.
+                    // If it is the scope opener, go the first non-empty token after. $start will have been part of the first condition.
+                    if ($prevMatch <= $this->tokens[$matchExpression]['scope_opener']) {
+                        // We're before the arrow in the first case.
+                        $next = $this->findNext(Tokens::$emptyTokens, ($this->tokens[$matchExpression]['scope_opener'] + 1), null, true);
+                        if ($next === false) {
+                            // Shouldn't be possible.
+                            return $start;
+                        }
 
-                $end  = $this->findEndOfStatement($prevMatchArrow);
-                $next = $this->findNext(Tokens::$emptyTokens, ($end + 1), null, true);
-                return $next;
-            }
+                        return $next;
+                    }
+
+                    // Okay, so we found a match arrow.
+                    // If $start was part of the "next" condition, the last comma will be set.
+                    // Otherwise, $start must have been part of a return expression.
+                    if (isset($lastComma) === true && $lastComma > $prevMatch) {
+                        $prevMatch = $lastComma;
+                    }
+
+                    // In both cases, go to the first non-empty token after.
+                    $next = $this->findNext(Tokens::$emptyTokens, ($prevMatch + 1), null, true);
+                    if ($next === false) {
+                        // Shouldn't be possible.
+                        return $start;
+                    }
+
+                    return $next;
+                }//end if
+            }//end if
         }//end if
 
         $lastNotEmpty = $start;

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc
@@ -1579,6 +1579,41 @@ foo(function ($foo) {
     ];
 });
 
+// Issue #110.
+echo match (1) {
+    0 => match (2) {
+        2 => match (3) {
+            3 => 3,
+            default => -1,
+        },
+    },
+    1 => match (2) {
+        1 => match (3) {
+            3 => 3,
+            default => -1,
+        },
+        2 => match (3) {
+            3 => 3,
+            default => -1,
+        },
+    },
+};
+
+// Issue #437.
+match (true) {
+    default => [
+        'unrelated' => '',
+        'example' => array_filter(
+            array_map(
+                function () {
+                    return null;
+                },
+                []
+            )
+        )
+    ]
+};
+
 /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
 ?>
 

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed
@@ -1579,6 +1579,41 @@ foo(function ($foo) {
     ];
 });
 
+// Issue #110.
+echo match (1) {
+    0 => match (2) {
+        2 => match (3) {
+            3 => 3,
+            default => -1,
+        },
+    },
+    1 => match (2) {
+        1 => match (3) {
+            3 => 3,
+            default => -1,
+        },
+        2 => match (3) {
+            3 => 3,
+            default => -1,
+        },
+    },
+};
+
+// Issue #437.
+match (true) {
+    default => [
+        'unrelated' => '',
+        'example' => array_filter(
+            array_map(
+                function () {
+                    return null;
+                },
+                []
+            )
+        )
+    ]
+};
+
 /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
 ?>
 

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc
@@ -1579,6 +1579,41 @@ foo(function ($foo) {
 	];
 });
 
+// Issue #110.
+echo match (1) {
+	0 => match (2) {
+		2 => match (3) {
+			3 => 3,
+			default => -1,
+		},
+	},
+	1 => match (2) {
+		1 => match (3) {
+			3 => 3,
+			default => -1,
+		},
+		2 => match (3) {
+			3 => 3,
+			default => -1,
+		},
+	},
+};
+
+// Issue #437.
+match (true) {
+	default => [
+		'unrelated' => '',
+		'example' => array_filter(
+			array_map(
+				function () {
+					return null;
+				},
+				[]
+			)
+		)
+	]
+};
+
 /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
 ?>
 

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed
@@ -1579,6 +1579,41 @@ foo(function ($foo) {
 	];
 });
 
+// Issue #110.
+echo match (1) {
+	0 => match (2) {
+		2 => match (3) {
+			3 => 3,
+			default => -1,
+		},
+	},
+	1 => match (2) {
+		1 => match (3) {
+			3 => 3,
+			default => -1,
+		},
+		2 => match (3) {
+			3 => 3,
+			default => -1,
+		},
+	},
+};
+
+// Issue #437.
+match (true) {
+	default => [
+		'unrelated' => '',
+		'example' => array_filter(
+			array_map(
+				function () {
+					return null;
+				},
+				[]
+			)
+		)
+	]
+};
+
 /* ADD NEW TESTS ABOVE THIS LINE AND MAKE SURE THAT THE 1 (space-based) AND 2 (tab-based) FILES ARE IN SYNC! */
 ?>
 

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php
@@ -192,10 +192,10 @@ public function getErrorList($testFile='')
             1527 => 1,
             1529 => 1,
             1530 => 1,
-            1590 => 1,
-            1591 => 1,
-            1592 => 1,
-            1593 => 1,
+            1625 => 1,
+            1626 => 1,
+            1627 => 1,
+            1628 => 1,
         ];
 
     }//end getErrorList()

diff --git a/tests/Core/File/FindStartOfStatementTest.inc b/tests/Core/File/FindStartOfStatementTest.inc
@@ -162,3 +162,39 @@ switch ($foo) {
         /* testInsideDefaultContinueStatement */
         continue $var;
 }
+
+match ($var) {
+    true =>
+        /* test437ClosureDeclaration */
+        function ($var) {
+            /* test437EchoNestedWithinClosureWithinMatch */
+            echo $var, 'text', PHP_EOL;
+        },
+    default => false
+};
+
+match ($var) {
+    /* test437NestedLongArrayWithinMatch */
+    'a' => array( 1, 2.5, $var),
+    /* test437NestedFunctionCallWithinMatch */
+    'b' => functionCall( 11, $var, 50.50),
+    /* test437NestedArrowFunctionWithinMatch */
+    'c' => fn($p1, /* test437FnSecondParamWithinMatch */ $p2) => $p1 + $p2,
+    default => false
+};
+
+callMe($paramA, match ($var) {
+    /* test437NestedLongArrayWithinNestedMatch */
+    'a' => array( 1, 2.5, $var),
+    /* test437NestedFunctionCallWithinNestedMatch */
+    'b' => functionCall( 11, $var, 50.50),
+    /* test437NestedArrowFunctionWithinNestedMatch */
+    'c' => fn($p1, /* test437FnSecondParamWithinNestedMatch */ $p2) => $p1 + $p2,
+    default => false
+});
+
+match ($var) {
+    /* test437NestedShortArrayWithinMatch */
+    'a' => [ 1, 2.5, $var],
+    default => false
+};