Fix variadic parameter element type in foreach loops

Author: AJenbo

docs/CHANGELOG.md

@@ -75,6 +75,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Callable types inside unions displayed ambiguously.** `(Closure(int): string)|Foo` was formatted as `Closure(int): string|Foo`, which reads as a callable returning `string|Foo`. Callable types inside unions are now wrapped in parentheses.
 - **Hover and go-to-definition on attributes.** Attributes on properties, class constants, function/method parameters, and enum cases are now recognized by the symbol map. Previously only attributes on classes, methods, and top-level functions produced navigable symbols; hovering or Ctrl+Clicking an attribute on a property (e.g. `#[Assert\NotBlank]`) did nothing.
 - **Function-level `@template` with `array<TKey, TValue>` parameters.** Template substitution at call sites now correctly resolves generic wrapper names like `array`, `iterable`, and `list`. Previously, `collect($users)->first()->` failed to infer the element type because the wrapper name was incorrectly discarded during binding classification.
+- **Variadic parameter element type lost in `foreach`.** Iterating over a variadic parameter (e.g. `foreach ($placeholders as $value)` where the method declares `HtmlString|int|string ...$placeholders`) now resolves the loop variable to the element type. Previously the variable was untyped, breaking both completion and `instanceof` narrowing inside the loop body.
 - **Stack overflow when a foreach value variable shadows the iterator receiver.** Patterns like `foreach ($category->getBranch() as $category)` caused infinite recursion during type resolution because resolving the value variable re-entered the same foreach. The foreach resolver now detects this cycle at the AST level and skips the recursive path. A depth guard on `resolve_variable_types` provides a safety net for any remaining recursive patterns.
 - **PHPStan diagnostics hidden when a native diagnostic exists on the same line.** Full-line PHPStan diagnostics were suppressed whenever any precise native diagnostic appeared on the same line, even for completely unrelated issues. For example, `class.prefixed` was hidden because a native `unknown_class` diagnostic covered the same line. Deduplication now only suppresses a full-line diagnostic when the precise diagnostic on that line reports a related issue.
 - **Deprecated class in `implements` now renders with strikethrough.** Verified and tested that `DiagnosticTag::DEPRECATED` applies correctly for deprecated classes referenced in `implements` clauses, matching the existing coverage for `new`, type hints, and `extends`. Also verified that `$this`/`self`/`static` resolve to the correct class in files with multiple class declarations.

docs/todo.md

@@ -26,7 +26,6 @@ within the same impact tier.
 | B6  | [Scope methods not found on Builder in analyzer chains](todo/bugs.md#b6-scope-methods-not-found-on-builder-in-analyzer-chains)                   | High   | Medium |
 | L11 | [`whereHas` closure parameter type from relation traversal](todo/laravel.md#l11-wherehas--wheredoesnthave-closure-parameter-type-from-relation-traversal) | Medium | Medium |
 | B7  | [PHPDoc `@param` generic array type not merged with native `array` hint](todo/bugs.md#b7-phpdoc-param-generic-array-type-not-merged-with-native-array-hint) | Low | Medium |
-| B8  | [Variadic parameter element type lost in `foreach`](todo/bugs.md#b8-variadic-parameter-element-type-lost-in-foreach)                             | Low    | Low    |
 | B9  | [Eloquent relationship property lookup is case-sensitive](todo/bugs.md#b9-eloquent-relationship-property-lookup-is-case-sensitive)                | Low    | Low    |
 | H10 | [`return.unusedType` — remove unused type from return union](todo/phpstan-actions.md#h10-returnunusedtype--remove-unused-type-from-return-union) | Medium | Medium |
 | H6  | `return.type` — update return type to match actual returns                                                                                       | Medium | Medium |

docs/todo/bugs.md

@@ -70,38 +70,6 @@ native `array` with the docblock's `list<Request>`.
 **Impact:** 1 diagnostic in the shared project
 (`MobilePayConnection:76`).
 
-## B8: Variadic parameter element type lost in `foreach`
-
-When a method declares a variadic parameter with a union type like
-`HtmlString|int|string ...$placeholders`, iterating with
-`foreach ($placeholders as $value)` should give `$value` the element
-type `HtmlString|int|string`. Instead, the LSP resolves `$value` as
-untyped (hover returns nothing).
-
-Real-world example — `ShortTexts.php`:
-
-```php
-public static function get(int $id, Country $lang, HtmlString|int|string ...$placeholders): HtmlString|string
-{
-    // ...
-    foreach ($placeholders as $value) {
-        $isHTMLValue = $value instanceof HtmlString;
-        if ($isHTML) {
-            $replace[] = $isHTMLValue ? $value->toHtml() : htmlentities((string)$value);
-            //                          ^^^^^^ unresolved
-        }
-    }
-}
-```
-
-The variadic `...$placeholders` is internally `array<int, HtmlString|int|string>`,
-but the LSP doesn't propagate the element type into the `foreach` loop
-variable. This is a prerequisite for the `instanceof` narrowing (which
-would further narrow `$value` to `HtmlString` in the truthy ternary
-branch), but the primary failure is the missing element type.
-
-**Impact:** 1 diagnostic in the shared project (`ShortTexts:79`).
-
 ## B9: Eloquent relationship property lookup is case-sensitive
 
 Laravel normalises property names via `Str::snake()` at runtime, so

example.php

@@ -1284,6 +1284,30 @@ public function demo(): void
 }
 
 
+// ── Variadic Parameter Foreach ──────────────────────────────────────────────
+
+class VariadicForeachDemo
+{
+    public function demo(Pen ...$pens): void
+    {
+        // Variadic parameters are arrays: foreach extracts the element type
+        foreach ($pens as $pen) {
+            $pen->write();                // element type from variadic Pen ...$pens
+        }
+    }
+
+    public function unionVariadic(Pen|Pencil ...$tools): void
+    {
+        // Union variadic: foreach value is Pen|Pencil
+        foreach ($tools as $tool) {
+            if ($tool instanceof Pen) {
+                $tool->write();           // narrowed to Pen via instanceof
+            }
+        }
+    }
+}
+
+
 // ── Array Function Type Preservation ────────────────────────────────────────
 
 class ArrayFuncDemo
@@ -5668,6 +5692,22 @@ function runDemoAssertions(): void
     assert(CT_ALLOWED_HOSTS === ['localhost', '127.0.0.1'], 'CT_ALLOWED_HOSTS must match');
     assert(CT_APP_VERSION === '2.0.0', 'CT_APP_VERSION must be "2.0.0"');
 
+    // ── Variadic foreach ────────────────────────────────────────────────
+    $vfDemo = new VariadicForeachDemo();
+    $vfPens = [new Pen('a'), new Pen('b')];
+    // demo() accepts Pen ...$pens — foreach inside should see Pen elements
+    $vfDemo->demo(...$vfPens);
+    foreach ($vfPens as $vfPen) {
+        assert($vfPen instanceof Pen, 'Variadic Pen element must be Pen');
+    }
+    $vfTools = [new Pen('x'), new Pencil()];
+    foreach ($vfTools as $vfTool) {
+        assert(
+            $vfTool instanceof Pen || $vfTool instanceof Pencil,
+            'Variadic union element must be Pen or Pencil'
+        );
+    }
+
     echo "All assertions passed.\n";
 }
 

src/completion/variable/closure_resolution.rs

@@ -1269,9 +1269,11 @@ fn resolve_closure_params_with_inferred(
     results: &mut Vec<ResolvedType>,
     inferred_types: &[String],
 ) {
+    let mut matched_param_is_variadic = false;
     for (idx, param) in parameter_list.parameters.iter().enumerate() {
         let pname = param.variable.name.to_string();
         if pname == ctx.var_name {
+            matched_param_is_variadic = param.ellipsis.is_some();
             // 1. Try the explicit type hint first.
             if let Some(hint) = &param.hint {
                 let type_str = extract_hint_string(hint);
@@ -1390,6 +1392,22 @@ fn resolve_closure_params_with_inferred(
             break;
         }
     }
+
+    // ── Variadic parameter wrapping ─────────────────────────────
+    // When the matched parameter is variadic (e.g.
+    // `HtmlString|int|string ...$placeholders`), the native type
+    // hint describes the *element* type, but the variable itself
+    // holds `list<ElementType>`.  Wrap the resolved types so that
+    // foreach iteration can extract the element type via
+    // `PhpType::extract_value_type`.
+    if matched_param_is_variadic && !results.is_empty() {
+        for rt in results.iter_mut() {
+            rt.type_string = PhpType::Generic("list".to_string(), vec![rt.type_string.clone()]);
+            // The variable is now an array, not a class instance,
+            // so clear the class_info.
+            rt.class_info = None;
+        }
+    }
 }
 
 /// Check whether the inferred callable-signature type is a more specific

src/completion/variable/resolution.rs

@@ -744,9 +744,11 @@ fn resolve_variable_in_members<'b>(
             // We no longer return early here so that the method body
             // can be scanned for instanceof narrowing / reassignments.
             let mut param_results: Vec<ResolvedType> = Vec::new();
+            let mut matched_param_is_variadic = false;
             for param in method.parameter_list.parameters.iter() {
                 let pname = param.variable.name.to_string();
                 if pname == ctx.var_name {
+                    matched_param_is_variadic = param.ellipsis.is_some();
                     // Try the native AST type hint first.
                     let native_type_str = param.hint.as_ref().map(|h| extract_hint_string(h));
 
@@ -927,6 +929,23 @@ fn resolve_variable_in_members<'b>(
                 }
             }
 
+            // ── Variadic parameter wrapping ─────────────────────────
+            // When the matched parameter is variadic (e.g.
+            // `HtmlString|int|string ...$placeholders`), the native
+            // type hint describes the *element* type, but the variable
+            // itself holds `list<ElementType>`.  Wrap the resolved
+            // types so that foreach iteration can extract the element
+            // type via `PhpType::extract_value_type`.
+            if matched_param_is_variadic && !param_results.is_empty() {
+                for rt in &mut param_results {
+                    rt.type_string =
+                        PhpType::Generic("list".to_string(), vec![rt.type_string.clone()]);
+                    // The variable is now an array, not a class instance,
+                    // so clear the class_info.
+                    rt.class_info = None;
+                }
+            }
+
             if let MethodBody::Concrete(block) = &method.body {
                 let blk_start = block.left_brace.start.offset;
                 let blk_end = block.right_brace.end.offset;

tests/integration/completion_variables.rs

@@ -20283,3 +20283,199 @@ async fn test_completion_magic_call_static_fallback() {
         _ => panic!("Expected CompletionResponse::Array"),
     }
 }
+
+/// Foreach over a variadic parameter with a class type resolves the
+/// loop variable to the element type.
+#[tokio::test]
+async fn test_completion_foreach_variadic_param_class_type() {
+    let backend = create_test_backend();
+
+    let uri = Url::parse("file:///variadic_foreach.php").unwrap();
+    let text = concat!(
+        "<?php\n",
+        "class HtmlString {\n",
+        "    public function toHtml(): string { return ''; }\n",
+        "}\n",
+        "class Formatter {\n",
+        "    public function format(HtmlString ...$items): void {\n",
+        "        foreach ($items as $value) {\n",
+        "            $value->\n",
+        "        }\n",
+        "    }\n",
+        "}\n",
+    );
+
+    let open_params = DidOpenTextDocumentParams {
+        text_document: TextDocumentItem {
+            uri: uri.clone(),
+            language_id: "php".to_string(),
+            version: 1,
+            text: text.to_string(),
+        },
+    };
+    backend.did_open(open_params).await;
+
+    let completion_params = CompletionParams {
+        text_document_position: TextDocumentPositionParams {
+            text_document: TextDocumentIdentifier { uri },
+            position: Position {
+                line: 7,
+                character: 20,
+            },
+        },
+        work_done_progress_params: WorkDoneProgressParams::default(),
+        partial_result_params: PartialResultParams::default(),
+        context: None,
+    };
+
+    let result = backend.completion(completion_params).await.unwrap();
+    assert!(
+        result.is_some(),
+        "Completion should return results for foreach value from variadic param"
+    );
+
+    match result.unwrap() {
+        CompletionResponse::Array(items) => {
+            let labels: Vec<&str> = items.iter().map(|i| i.label.as_str()).collect();
+            assert!(
+                labels.iter().any(|l| l.starts_with("toHtml")),
+                "Should include toHtml method from HtmlString, got: {:?}",
+                labels
+            );
+        }
+        _ => panic!("Expected CompletionResponse::Array"),
+    }
+}
+
+/// Foreach over a variadic parameter with a union type containing a
+/// class resolves the loop variable so that instanceof narrowing can
+/// work on it.
+#[tokio::test]
+async fn test_completion_foreach_variadic_param_union_type() {
+    let backend = create_test_backend();
+
+    let uri = Url::parse("file:///variadic_union_foreach.php").unwrap();
+    let text = concat!(
+        "<?php\n",
+        "class HtmlString {\n",
+        "    public function toHtml(): string { return ''; }\n",
+        "}\n",
+        "class ShortTexts {\n",
+        "    public static function get(int $id, HtmlString|int|string ...$placeholders): void {\n",
+        "        foreach ($placeholders as $value) {\n",
+        "            if ($value instanceof HtmlString) {\n",
+        "                $value->\n",
+        "            }\n",
+        "        }\n",
+        "    }\n",
+        "}\n",
+    );
+
+    let open_params = DidOpenTextDocumentParams {
+        text_document: TextDocumentItem {
+            uri: uri.clone(),
+            language_id: "php".to_string(),
+            version: 1,
+            text: text.to_string(),
+        },
+    };
+    backend.did_open(open_params).await;
+
+    let completion_params = CompletionParams {
+        text_document_position: TextDocumentPositionParams {
+            text_document: TextDocumentIdentifier { uri },
+            position: Position {
+                line: 8,
+                character: 24,
+            },
+        },
+        work_done_progress_params: WorkDoneProgressParams::default(),
+        partial_result_params: PartialResultParams::default(),
+        context: None,
+    };
+
+    let result = backend.completion(completion_params).await.unwrap();
+    assert!(
+        result.is_some(),
+        "Completion should return results after instanceof narrowing on variadic foreach value"
+    );
+
+    match result.unwrap() {
+        CompletionResponse::Array(items) => {
+            let labels: Vec<&str> = items.iter().map(|i| i.label.as_str()).collect();
+            assert!(
+                labels.iter().any(|l| l.starts_with("toHtml")),
+                "Should include toHtml from HtmlString after instanceof narrowing, got: {:?}",
+                labels
+            );
+        }
+        _ => panic!("Expected CompletionResponse::Array"),
+    }
+}
+
+/// Foreach over a variadic parameter in a standalone function (not a
+/// method) resolves the loop variable to the element type.
+#[tokio::test]
+async fn test_completion_foreach_variadic_param_standalone_function() {
+    let backend = create_test_backend();
+
+    let uri = Url::parse("file:///variadic_func_foreach.php").unwrap();
+    let text = concat!(
+        "<?php\n",
+        "class Widget {\n",
+        "    public string $name;\n",
+        "    public function render(): string { return ''; }\n",
+        "}\n",
+        "function processWidgets(Widget ...$widgets): void {\n",
+        "    foreach ($widgets as $w) {\n",
+        "        $w->\n",
+        "    }\n",
+        "}\n",
+    );
+
+    let open_params = DidOpenTextDocumentParams {
+        text_document: TextDocumentItem {
+            uri: uri.clone(),
+            language_id: "php".to_string(),
+            version: 1,
+            text: text.to_string(),
+        },
+    };
+    backend.did_open(open_params).await;
+
+    let completion_params = CompletionParams {
+        text_document_position: TextDocumentPositionParams {
+            text_document: TextDocumentIdentifier { uri },
+            position: Position {
+                line: 7,
+                character: 12,
+            },
+        },
+        work_done_progress_params: WorkDoneProgressParams::default(),
+        partial_result_params: PartialResultParams::default(),
+        context: None,
+    };
+
+    let result = backend.completion(completion_params).await.unwrap();
+    assert!(
+        result.is_some(),
+        "Completion should return results for foreach value from variadic function param"
+    );
+
+    match result.unwrap() {
+        CompletionResponse::Array(items) => {
+            let labels: Vec<&str> = items.iter().map(|i| i.label.as_str()).collect();
+            assert!(
+                labels.iter().any(|l| l.starts_with("name")),
+                "Should include name property from Widget, got: {:?}",
+                labels
+            );
+            assert!(
+                labels.iter().any(|l| l.starts_with("render")),
+                "Should include render method from Widget, got: {:?}",
+                labels
+            );
+        }
+        _ => panic!("Expected CompletionResponse::Array"),
+    }
+}