Pretend like PHP varialbes do not leak out of loops scopes

Author: AJenbo

src/completion/variable_completion.rs

@@ -371,16 +371,21 @@ fn collect_from_statements<'b>(
                 }
             },
             Statement::Foreach(foreach) => {
-                // Collect the key and value variables — they are defined
-                // in the foreach header which starts before the cursor
-                // (guaranteed by the span check above).
-                if let Some(key_expr) = foreach.target.key() {
-                    collect_var_name_from_expression(key_expr, vars);
-                }
-                collect_var_name_from_expression(foreach.target.value(), vars);
-                // Recurse into body
-                for inner in foreach.body.statements() {
-                    collect_from_statement(inner, cursor_offset, vars);
+                // Only collect the key/value variables when the cursor is
+                // inside the foreach body.  Outside the loop these
+                // iteration variables should not be in scope.
+                let body_span = foreach.body.span();
+                if cursor_offset >= body_span.start.offset
+                    && cursor_offset <= body_span.end.offset
+                {
+                    if let Some(key_expr) = foreach.target.key() {
+                        collect_var_name_from_expression(key_expr, vars);
+                    }
+                    collect_var_name_from_expression(foreach.target.value(), vars);
+                    // Recurse into body
+                    for inner in foreach.body.statements() {
+                        collect_from_statement(inner, cursor_offset, vars);
+                    }
                 }
             }
             Statement::For(for_stmt) => {

src/completion/variable_resolution.rs

@@ -476,29 +476,40 @@ impl Backend {
                     }
                 },
                 Statement::Foreach(foreach) => {
-                    // ── Foreach value type from generic iterables ────
-                    // When the variable we're resolving is the foreach
-                    // *value* variable, try to infer its type from the
-                    // iterated expression's generic type annotation.
-                    //
-                    // Example:
-                    //   /** @var list<User> $users */
-                    //   foreach ($users as $user) { $user-> }
-                    //
-                    // Here `$user` is resolved to `User`.
-                    Self::try_resolve_foreach_value_type(foreach, ctx, results, conditional);
+                    // Only resolve the foreach value variable and recurse
+                    // into the body when the cursor is actually inside it.
+                    // Outside the loop the iteration variables are out of
+                    // scope.
+                    let body_span = foreach.body.span();
+                    if ctx.cursor_offset >= body_span.start.offset
+                        && ctx.cursor_offset <= body_span.end.offset
+                    {
+                        // ── Foreach value type from generic iterables ──
+                        // When the variable we're resolving is the foreach
+                        // *value* variable, try to infer its type from the
+                        // iterated expression's generic type annotation.
+                        //
+                        // Example:
+                        //   /** @var list<User> $users */
+                        //   foreach ($users as $user) { $user-> }
+                        //
+                        // Here `$user` is resolved to `User`.
+                        Self::try_resolve_foreach_value_type(foreach, ctx, results, conditional);
 
-                    match &foreach.body {
-                        ForeachBody::Statement(inner) => {
-                            Self::check_statement_for_assignments(inner, ctx, results, true);
-                        }
-                        ForeachBody::ColonDelimited(body) => {
-                            Self::walk_statements_for_assignments(
-                                body.statements.iter(),
-                                ctx,
-                                results,
-                                true,
-                            );
+                        match &foreach.body {
+                            ForeachBody::Statement(inner) => {
+                                Self::check_statement_for_assignments(
+                                    inner, ctx, results, true,
+                                );
+                            }
+                            ForeachBody::ColonDelimited(body) => {
+                                Self::walk_statements_for_assignments(
+                                    body.statements.iter(),
+                                    ctx,
+                                    results,
+                                    true,
+                                );
+                            }
                         }
                     }
                 }

tests/completion_variable_names.rs

@@ -1297,6 +1297,121 @@ async fn test_completion_variable_used_in_different_contexts() {
 }
 
 /// Superglobals should not be duplicated even if they also appear in file content.
+#[tokio::test]
+async fn test_completion_foreach_variable_not_visible_after_loop() {
+    let backend = create_test_backend();
+
+    let uri = Url::parse("file:///var_foreach_scope.php").unwrap();
+    let text = concat!(
+        "<?php\n",
+        "$items = [1, 2, 3];\n",
+        "foreach ($items as $key => $value) {\n",
+        "    echo $value;\n",
+        "}\n",
+        "$\n",
+    );
+
+    // Cursor is after the foreach on line 5 — `$value` and `$key` should
+    // NOT appear because they are scoped to the loop body.
+    let items = complete_at(&backend, &uri, text, 5, 1).await;
+
+    let var_labels: Vec<&str> = items
+        .iter()
+        .filter(|i| i.kind == Some(CompletionItemKind::VARIABLE))
+        .map(|i| i.label.as_str())
+        .collect();
+
+    assert!(
+        !var_labels.contains(&"$value"),
+        "$value should NOT be visible after the foreach loop. Got: {:?}",
+        var_labels
+    );
+    assert!(
+        !var_labels.contains(&"$key"),
+        "$key should NOT be visible after the foreach loop. Got: {:?}",
+        var_labels
+    );
+    assert!(
+        var_labels.contains(&"$items"),
+        "$items should still be visible after the foreach loop. Got: {:?}",
+        var_labels
+    );
+}
+
+#[tokio::test]
+async fn test_completion_foreach_variable_not_visible_before_loop() {
+    let backend = create_test_backend();
+
+    let uri = Url::parse("file:///var_foreach_before.php").unwrap();
+    let text = concat!(
+        "<?php\n",
+        "$items = [1, 2, 3];\n",
+        "$\n",
+        "foreach ($items as $value) {\n",
+        "    echo $value;\n",
+        "}\n",
+    );
+
+    // Cursor is before the foreach on line 2
+    let items = complete_at(&backend, &uri, text, 2, 1).await;
+
+    let var_labels: Vec<&str> = items
+        .iter()
+        .filter(|i| i.kind == Some(CompletionItemKind::VARIABLE))
+        .map(|i| i.label.as_str())
+        .collect();
+
+    assert!(
+        !var_labels.contains(&"$value"),
+        "$value should NOT be visible before the foreach loop. Got: {:?}",
+        var_labels
+    );
+    assert!(
+        var_labels.contains(&"$items"),
+        "$items should be visible before the foreach loop. Got: {:?}",
+        var_labels
+    );
+}
+
+#[tokio::test]
+async fn test_completion_foreach_variable_visible_inside_loop() {
+    let backend = create_test_backend();
+
+    let uri = Url::parse("file:///var_foreach_inside.php").unwrap();
+    let text = concat!(
+        "<?php\n",
+        "$items = [1, 2, 3];\n",
+        "foreach ($items as $key => $value) {\n",
+        "    $\n",
+        "}\n",
+    );
+
+    // Cursor is inside the foreach on line 3
+    let items = complete_at(&backend, &uri, text, 3, 5).await;
+
+    let var_labels: Vec<&str> = items
+        .iter()
+        .filter(|i| i.kind == Some(CompletionItemKind::VARIABLE))
+        .map(|i| i.label.as_str())
+        .collect();
+
+    assert!(
+        var_labels.contains(&"$value"),
+        "$value should be visible inside the foreach loop. Got: {:?}",
+        var_labels
+    );
+    assert!(
+        var_labels.contains(&"$key"),
+        "$key should be visible inside the foreach loop. Got: {:?}",
+        var_labels
+    );
+    assert!(
+        var_labels.contains(&"$items"),
+        "$items should be visible inside the foreach loop. Got: {:?}",
+        var_labels
+    );
+}
+
 #[tokio::test]
 async fn test_completion_superglobal_not_duplicated() {
     let backend = create_test_backend();