Skip to content

Commit ed7b308

Browse files
committed
Fix false positives for bare array and foreach class-string diagnostics
1 parent 6713952 commit ed7b308

5 files changed

Lines changed: 159 additions & 3 deletions

File tree

docs/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3030
- **Magic `__call` method return type.** Calling undefined methods on objects with a `__call` method now resolves to `__call`'s declared return type for hover and type inference.
3131
- **SoapClient arbitrary methods.** Calling any method on `SoapClient` (or subclasses) no longer produces false-positive "unknown member" diagnostics. SoapClient is a SOAP proxy where any method name is valid at runtime.
3232
- **`self::class` and `static::class` in template arguments.** Passing `self::class` or `static::class` to a method with `@template T` + `@param class-string<T>` now correctly resolves T to the enclosing class instead of failing to resolve or resolving to the called class.
33+
- **Bare array return values no longer flagged against typed array parameters.** When a method returns `array` (without generic parameters), passing the result to a parameter expecting `array<T>` no longer triggers a false type mismatch error.
34+
- **Foreach over `::class` literal arrays resolves static access.** Variables assigned from `foreach ([A::class, B::class] as $className)` are now recognized as class-strings, so `$className::CONST` and `$className::method()` no longer produce unresolved-member-access diagnostics.
3335
- **Use-map shadowing no longer causes false type errors.** When a file imports a namespaced class under the same short name as a global class (e.g. `use App\Exceptions\Exception;`), type hierarchy checks no longer cycle and incorrectly flag subclass arguments as incompatible with parent interfaces like `Throwable`.
3436
- **Multi-namespace function return type resolution.** In files with multiple `namespace { }` blocks, function return types were resolved against the first namespace instead of the function's own namespace. This caused incorrect type inference for variables assigned from function calls in later namespace blocks.
3537
- **Template inference through stub interfaces.** `@template-implements` on stub-loaded interfaces (e.g. `IteratorAggregate<int, Foo>`) now correctly propagates substituted return types to child methods that omit a return type annotation. Previously these resolved as untyped.

src/completion/variable/class_string_resolution.rs

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,56 @@ fn walk_class_string_assignments<'b>(
157157
if stmt.span().start.offset >= ctx.cursor_offset {
158158
continue;
159159
}
160-
if let Statement::Expression(expr_stmt) = stmt {
161-
check_class_string_assignment(expr_stmt.expression, ctx, results);
160+
match stmt {
161+
Statement::Expression(expr_stmt) => {
162+
check_class_string_assignment(expr_stmt.expression, ctx, results);
163+
}
164+
Statement::Foreach(foreach) => {
165+
// Check if the foreach value variable matches our target
166+
// and the iterated expression is an array of ::class literals.
167+
let value_expr = foreach.target.value();
168+
let value_name = match value_expr {
169+
Expression::Variable(Variable::Direct(dv)) => Some(dv.name.to_string()),
170+
_ => None,
171+
};
172+
if let Some(name) = value_name
173+
&& name == ctx.var_name
174+
{
175+
// Extract class names from the iterated expression
176+
// (e.g. `[Page::class, CustomPage::class]`).
177+
let class_names =
178+
extract_class_string_names_from_array(foreach.expression);
179+
if !class_names.is_empty() {
180+
results.clear();
181+
for cn in class_names {
182+
let resolved_name = if let Some(resolved) =
183+
resolve_class_keyword(&cn, Some(ctx.current_class))
184+
{
185+
resolved
186+
} else {
187+
cn
188+
};
189+
let lookup = short_name(&resolved_name);
190+
if let Some(cls) =
191+
ctx.all_classes.iter().find(|c| c.name == lookup)
192+
{
193+
ClassInfo::push_unique(results, ClassInfo::clone(cls));
194+
} else if let Some(cls) = (ctx.class_loader)(&resolved_name) {
195+
ClassInfo::push_unique(results, Arc::unwrap_or_clone(cls));
196+
}
197+
}
198+
}
199+
}
200+
// Also walk the foreach body for nested assignments.
201+
let body_stmts: Vec<&Statement> = match &foreach.body {
202+
mago_syntax::ast::ForeachBody::Statement(s) => vec![s],
203+
mago_syntax::ast::ForeachBody::ColonDelimited(b) => {
204+
b.statements.iter().collect()
205+
}
206+
};
207+
walk_class_string_assignments(body_stmts.into_iter(), ctx, results);
208+
}
209+
_ => {}
162210
}
163211
}
164212
}
@@ -235,3 +283,31 @@ fn extract_class_string_names(expr: &Expression<'_>) -> Vec<String> {
235283
_ => vec![],
236284
}
237285
}
286+
287+
/// Extract class names from array elements that are `::class` literals.
288+
///
289+
/// Handles `[Page::class, CustomPage::class]` and similar array
290+
/// expressions used as foreach iterables.
291+
fn extract_class_string_names_from_array(expr: &Expression<'_>) -> Vec<String> {
292+
match expr {
293+
Expression::Array(array) => {
294+
let mut names = Vec::new();
295+
for item in array.elements.iter() {
296+
if let ArrayElement::Value(val) = item {
297+
names.extend(extract_class_string_names(val.value));
298+
}
299+
}
300+
names
301+
}
302+
Expression::LegacyArray(array) => {
303+
let mut names = Vec::new();
304+
for item in array.elements.iter() {
305+
if let ArrayElement::Value(val) = item {
306+
names.extend(extract_class_string_names(val.value));
307+
}
308+
}
309+
names
310+
}
311+
_ => vec![],
312+
}
313+
}

src/diagnostics/type_errors.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ struct ResolvedCallArgs {
5757
/// Returns `true` when the type is a bare unparameterised `array`.
5858
fn is_bare_array(ty: &PhpType) -> bool {
5959
matches!(ty, PhpType::Named(n) if n.eq_ignore_ascii_case("array"))
60+
|| matches!(ty, PhpType::Array(inner) if inner.is_mixed())
6061
}
6162

6263
/// Check if an argument type is compatible with a parameter type.

tests/integration/diag_timing.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
//! invalidation, classes from unedited files stay cached and the second
1616
//! pass is significantly faster.
1717
18-
use crate::common::{create_psr4_workspace, create_test_backend_with_full_stubs};
18+
use crate::common::{create_psr4_workspace, create_test_backend, create_test_backend_with_full_stubs};
1919
use std::time::Instant;
2020
use tower_lsp::lsp_types::NumberOrString;
2121

@@ -2096,3 +2096,52 @@ class Calculator {
20962096
out.iter().map(|d| &d.message).collect::<Vec<_>>()
20972097
);
20982098
}
2099+
2100+
// ─── Foreach over array of ::class literals resolves static access ───────────
2101+
2102+
#[test]
2103+
fn foreach_class_string_array_static_access_no_false_positive() {
2104+
let php = r#"<?php
2105+
class Page {
2106+
public const TABLE_NAME = 'pages';
2107+
}
2108+
2109+
class Newsletter {
2110+
public const TABLE_NAME = 'newsletters';
2111+
}
2112+
2113+
class Controller {
2114+
public function test(): void {
2115+
foreach ([Page::class, Newsletter::class] as $className) {
2116+
echo $className::TABLE_NAME;
2117+
}
2118+
}
2119+
}
2120+
"#;
2121+
2122+
let uri = "file:///test/foreach_class.php";
2123+
let backend = create_test_backend();
2124+
{
2125+
let mut cfg = backend.config();
2126+
cfg.diagnostics.unresolved_member_access = Some(true);
2127+
backend.set_config(cfg);
2128+
}
2129+
backend.update_ast(uri, php);
2130+
2131+
let mut out = Vec::new();
2132+
backend.collect_unknown_member_diagnostics(uri, php, &mut out);
2133+
2134+
let relevant: Vec<_> = out
2135+
.iter()
2136+
.filter(|d| {
2137+
d.code.as_ref().is_some_and(|c| {
2138+
matches!(c, NumberOrString::String(s) if s == "unknown_member" || s == "unresolved_member_access")
2139+
})
2140+
})
2141+
.collect();
2142+
assert!(
2143+
relevant.is_empty(),
2144+
"Should not flag static access on foreach class-string variable, got: {:?}",
2145+
relevant.iter().map(|d| &d.message).collect::<Vec<_>>()
2146+
);
2147+
}

tests/integration/diagnostics_type_errors.rs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3803,3 +3803,31 @@ class MyException extends NativeException {}
38033803
"Should not flag Exception subclass as incompatible with Throwable, got: {msgs:?}"
38043804
);
38053805
}
3806+
3807+
// ─── Bare array (Array(mixed)) passed to typed array parameter ──────────────
3808+
3809+
#[test]
3810+
fn no_false_positive_bare_array_from_method_call_to_typed_array_param() {
3811+
let php = r#"<?php
3812+
class ORM {
3813+
/** @return array */
3814+
public function getByQuery(string $class, string $query): array { return []; }
3815+
}
3816+
3817+
class Controller {
3818+
/** @param array<Item> $items */
3819+
public function process(array $items): void {}
3820+
3821+
public function test(ORM $orm): void {
3822+
$items = $orm->getByQuery('Item', 'SELECT * FROM items');
3823+
$this->process($items);
3824+
}
3825+
}
3826+
"#;
3827+
let diags = collect(php);
3828+
let msgs = type_error_messages(&diags);
3829+
assert!(
3830+
msgs.is_empty(),
3831+
"Should not flag bare array from method return as incompatible with typed array param, got: {msgs:?}"
3832+
);
3833+
}

0 commit comments

Comments
 (0)