From e645de0d839c85a9adb5210c84081fc169dd29de Mon Sep 17 00:00:00 2001 From: Caleb White Date: Fri, 10 Jul 2026 08:26:54 -0500 Subject: [PATCH] feat: add return type and property type mismatch diagnostics Add two new diagnostic collectors to the type mismatch family: - type_mismatch_return: checks return statements against declared return types. Flags incompatible types, void functions returning values, and bare return; in non-void functions. Skips generators, abstract methods, mixed/untyped returns. - type_mismatch_property: checks $this->prop and self::$prop assignments against declared property types. Only plain = assignments; compound operators are skipped. Both reuse the existing is_type_compatible() policy layer and resolve_expression_type() infrastructure. A unified collect_type_mismatch_diagnostics() entry point groups all three collectors (argument, return, property). Closes #201 --- docs/CHANGELOG.md | 2 + src/analyse.rs | 8 +- src/diagnostics/mod.rs | 25 +- src/diagnostics/property_type_errors.rs | 483 +++ src/diagnostics/return_type_errors.rs | 807 +++++ src/diagnostics/type_errors.rs | 6 +- .../diagnostics_property_type_errors.rs | 2203 +++++++++++++ .../diagnostics_return_type_errors.rs | 2730 +++++++++++++++++ tests/integration/diagnostics_type_errors.rs | 10 +- tests/integration/main.rs | 2 + 10 files changed, 6265 insertions(+), 11 deletions(-) create mode 100644 src/diagnostics/property_type_errors.rs create mode 100644 src/diagnostics/return_type_errors.rs create mode 100644 tests/integration/diagnostics_property_type_errors.rs create mode 100644 tests/integration/diagnostics_return_type_errors.rs diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 44cc2980..1e156dde 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Return type mismatch diagnostics (`type_mismatch_return`).** Functions and methods with a declared return type are now checked against their `return` statements. Incompatible return values are flagged as errors. Void functions returning a value and bare `return;` in non-void functions are also flagged. Generators (functions using `yield`) are skipped. Uses the same conservative `is_type_compatible` policy as argument type checking to avoid false positives. Contributed by @calebdw. +- **Property type assignment diagnostics (`type_mismatch_property`).** Assignments to typed properties (`$this->prop = expr` and `self::$prop = expr`) are checked against the declared property type. Incompatible values are flagged as errors. Only plain `=` assignments are checked; compound operators (`+=`, `.=`, etc.) are skipped. Untyped and `mixed` properties are not flagged. Contributed by @calebdw. - **Completion candidates ranked by dependency provenance.** Class, function, and constant completions are now sorted by origin tier: project code first, then core/stub symbols, then explicit Composer dependencies (`require` / `require-dev`), then transitive vendor dependencies last. The provenance is inferred from `composer.json` and `installed.json` during indexing. Contributed by @calebdw. - **`update` command.** A new `phpantom_lsp update` subcommand downloads the latest release from GitHub and replaces the current binary. Supports `--check` (dry run, exit code 1 if update available) and `--no-confirm` (for CI). Handles `.tar.gz` (Unix) and `.zip` (Windows) archives across all 6 supported platforms. Contributed by @calebdw in https://github.com/PHPantom-dev/phpantom_lsp/pull/194. - **`array_map` infers the output element type from its callback.** The result of `array_map` now reflects what the callback actually returns instead of assuming the input element type is preserved. An explicit return type hint is honoured, including scalars like `string` or `int`, so `array_map(fn(Item $item): string => $item->id, $items)` produces `list` rather than `list`. When the callback has no return type hint, the type is inferred from its body expression, so `array_map(fn($item) => $item->id, $items)` over a `list` also produces `list`. Fixes #147. (contributed by @calebdw in https://github.com/PHPantom-dev/phpantom_lsp/pull/195) diff --git a/src/analyse.rs b/src/analyse.rs index 1b5cd917..53d30188 100644 --- a/src/analyse.rs +++ b/src/analyse.rs @@ -416,7 +416,13 @@ pub async fn run(options: AnalyseOptions) -> i32 { b.collect_argument_count_diagnostics(u, c, o) }), ("type_mismatch_argument", &|b, u, c, o| { - b.collect_type_error_diagnostics(u, c, o) + b.collect_argument_type_diagnostics(u, c, o) + }), + ("type_mismatch_return", &|b, u, c, o| { + b.collect_return_type_diagnostics(u, c, o) + }), + ("type_mismatch_property", &|b, u, c, o| { + b.collect_property_type_diagnostics(u, c, o) }), ("missing_implementation", &|b, u, c, o| { b.collect_implementation_error_diagnostics(u, c, o) diff --git a/src/diagnostics/mod.rs b/src/diagnostics/mod.rs index 915406e2..c0a4eb03 100644 --- a/src/diagnostics/mod.rs +++ b/src/diagnostics/mod.rs @@ -14,7 +14,9 @@ //! 3. **`unknown_*`** — symbol could not be resolved (class, function, //! member, variable). //! 4. **`unused_*`** — symbol is defined/imported but never referenced. -//! 5. **`type_mismatch_*`** — a value's type doesn't satisfy a constraint. +//! 5. **`type_mismatch_*`** — a value's type doesn't satisfy a constraint +//! (`type_mismatch_argument`, `type_mismatch_return`, +//! `type_mismatch_property`). //! 6. **`missing_*`** — a required declaration is absent (e.g. //! `missing_implementation` for unimplemented interface methods). //! 7. **`invalid_*`** — a structural/syntactic violation (e.g. @@ -165,6 +167,8 @@ mod deprecated; pub(crate) mod helpers; mod implementation_errors; mod invalid_class_kind; +mod property_type_errors; +mod return_type_errors; mod syntax_errors; mod type_errors; pub(crate) mod undefined_variables; @@ -272,12 +276,29 @@ impl Backend { // inside collect_unknown_member_diagnostics (in the Untyped arm) // to avoid a second full walk with duplicate type resolution. self.collect_argument_count_diagnostics(uri_str, content, out); - self.collect_type_error_diagnostics(uri_str, content, out); + self.collect_type_mismatch_diagnostics(uri_str, content, out); self.collect_implementation_error_diagnostics(uri_str, content, out); self.collect_deprecated_diagnostics(uri_str, content, out); self.collect_undefined_variable_diagnostics(uri_str, content, out); self.collect_invalid_class_kind_diagnostics(uri_str, content, out); } + + /// Collect all type mismatch diagnostics: argument types, return + /// types, and property assignment types. + /// + /// This is a convenience entry point that groups the three + /// `type_mismatch_*` collectors. The individual methods remain + /// available for selective use (e.g. in `analyse.rs`). + pub fn collect_type_mismatch_diagnostics( + &self, + uri_str: &str, + content: &str, + out: &mut Vec, + ) { + self.collect_argument_type_diagnostics(uri_str, content, out); + self.collect_return_type_diagnostics(uri_str, content, out); + self.collect_property_type_diagnostics(uri_str, content, out); + } } /// Check whether a cached PHPStan diagnostic is stale given the current diff --git a/src/diagnostics/property_type_errors.rs b/src/diagnostics/property_type_errors.rs new file mode 100644 index 00000000..d466fa92 --- /dev/null +++ b/src/diagnostics/property_type_errors.rs @@ -0,0 +1,483 @@ +//! Property type assignment mismatch diagnostics. +//! +//! Walk assignment expressions in the file and flag every assignment +//! to a typed property where the assigned value's resolved type is +//! incompatible with the declared property type. +//! +//! Handles both instance properties (`$this->prop = expr`) and static +//! properties (`ClassName::$prop = expr`). +//! +//! Uses the same conservative approach as argument type checking: +//! when in doubt (unresolved types, `mixed`, complex generics), +//! the diagnostic is suppressed to avoid false positives. + +use std::collections::HashMap; +use std::sync::Arc; + +use mago_span::HasSpan; +use mago_syntax::ast::access::Access; +use mago_syntax::ast::class_like::member::ClassLikeMemberSelector; +use mago_syntax::ast::expression::Expression; +use mago_syntax::ast::statement::Statement; +use mago_syntax::ast::variable::Variable; + +use tower_lsp::lsp_types::*; + +use crate::Backend; +use crate::atom::bytes_to_str; +use crate::completion::resolver::{Loaders, VarResolutionCtx}; +use crate::completion::variable::foreach_resolution::resolve_expression_type; +use crate::parser::{with_parse_cache, with_parsed_program}; +use crate::php_type::PhpType; +use crate::types::ClassInfo; + +use super::helpers::{find_innermost_enclosing_class, make_diagnostic}; +use super::type_errors::{has_strict_types, is_type_compatible}; + +/// Diagnostic code used for property type mismatch diagnostics. +pub(crate) const TYPE_MISMATCH_PROPERTY_CODE: &str = "type_mismatch_property"; + +// ── Collected assignment site info ────────────────────────────────────────── + +/// A single property assignment's resolved type plus the byte range +/// of the RHS expression in source. +struct ResolvedPropertyAssignment { + /// The resolved type of the RHS expression. + rhs_type: PhpType, + /// The declared type of the property. + declared_type: PhpType, + /// Byte offset of the RHS expression start (inclusive). + start: usize, + /// Byte offset of the RHS expression end (exclusive). + end: usize, + /// The property name (for diagnostic messages). + property_name: String, +} + +// ── Context struct ────────────────────────────────────────────────────────── + +/// Bundles the read-only context and output accumulator threaded through +/// every walker function, eliminating `clippy::too_many_arguments`. +struct PropertyCheckCtx<'a> { + content: &'a str, + file_ctx: &'a crate::types::FileContext, + class_loader: &'a dyn Fn(&str) -> Option>, + function_loader: &'a dyn Fn(&str) -> Option, + constant_loader: &'a dyn Fn(&str) -> Option>, + backend: &'a Backend, + out: &'a mut Vec, +} + +// ── AST walkers ───────────────────────────────────────────────────────────── + +/// Walk a top-level statement collecting property assignments. +fn collect_from_statement(stmt: &Statement<'_>, ctx: &mut PropertyCheckCtx<'_>) { + match stmt { + Statement::Namespace(ns) => { + for inner in ns.statements().iter() { + collect_from_statement(inner, ctx); + } + } + Statement::Class(class) => { + for member in class.members.iter() { + collect_from_class_member(member, ctx); + } + } + Statement::Interface(iface) => { + for member in iface.members.iter() { + collect_from_class_member(member, ctx); + } + } + Statement::Trait(trait_def) => { + for member in trait_def.members.iter() { + collect_from_class_member(member, ctx); + } + } + Statement::Enum(enum_def) => { + for member in enum_def.members.iter() { + collect_from_class_member(member, ctx); + } + } + Statement::Function(func) => { + for s in func.body.statements.iter() { + collect_from_statement(s, ctx); + } + } + Statement::Expression(expr_stmt) => { + check_expression_for_property_assignment(expr_stmt.expression, ctx); + } + Statement::Return(ret) => { + if let Some(val) = ret.value { + check_expression_for_property_assignment(val, ctx); + } + } + Statement::If(if_stmt) => { + collect_from_if_body(&if_stmt.body, ctx); + } + Statement::While(w) => { + for s in w.body.statements() { + collect_from_statement(s, ctx); + } + } + Statement::DoWhile(dw) => { + collect_from_statement(dw.statement, ctx); + } + Statement::For(f) => { + for s in f.body.statements() { + collect_from_statement(s, ctx); + } + } + Statement::Foreach(fe) => { + for s in fe.body.statements() { + collect_from_statement(s, ctx); + } + } + Statement::Switch(sw) => { + collect_from_switch_body(&sw.body, ctx); + } + Statement::Try(t) => { + for s in t.block.statements.iter() { + collect_from_statement(s, ctx); + } + for catch in t.catch_clauses.iter() { + for s in catch.block.statements.iter() { + collect_from_statement(s, ctx); + } + } + if let Some(ref finally) = t.finally_clause { + for s in finally.block.statements.iter() { + collect_from_statement(s, ctx); + } + } + } + Statement::Block(block) => { + for s in block.statements.iter() { + collect_from_statement(s, ctx); + } + } + Statement::Declare(declare) => { + use mago_syntax::ast::declare::DeclareBody; + match &declare.body { + DeclareBody::Statement(inner) => { + collect_from_statement(inner, ctx); + } + DeclareBody::ColonDelimited(body) => { + for s in body.statements.iter() { + collect_from_statement(s, ctx); + } + } + } + } + _ => {} + } +} + +fn collect_from_class_member( + member: &mago_syntax::ast::class_like::member::ClassLikeMember<'_>, + ctx: &mut PropertyCheckCtx<'_>, +) { + use mago_syntax::ast::class_like::member::ClassLikeMember; + use mago_syntax::ast::class_like::method::MethodBody; + + if let ClassLikeMember::Method(method) = member + && let MethodBody::Concrete(block) = &method.body + { + for s in block.statements.iter() { + collect_from_statement(s, ctx); + } + } +} + +fn collect_from_if_body( + body: &mago_syntax::ast::control_flow::r#if::IfBody<'_>, + ctx: &mut PropertyCheckCtx<'_>, +) { + use mago_syntax::ast::control_flow::r#if::IfBody; + match body { + IfBody::Statement(inner) => { + collect_from_statement(inner.statement, ctx); + for c in inner.else_if_clauses.iter() { + collect_from_statement(c.statement, ctx); + } + if let Some(ref c) = inner.else_clause { + collect_from_statement(c.statement, ctx); + } + } + IfBody::ColonDelimited(body) => { + for s in body.statements.iter() { + collect_from_statement(s, ctx); + } + for c in body.else_if_clauses.iter() { + for s in c.statements.iter() { + collect_from_statement(s, ctx); + } + } + if let Some(ref c) = body.else_clause { + for s in c.statements.iter() { + collect_from_statement(s, ctx); + } + } + } + } +} + +fn collect_from_switch_body( + body: &mago_syntax::ast::control_flow::switch::SwitchBody<'_>, + ctx: &mut PropertyCheckCtx<'_>, +) { + use mago_syntax::ast::control_flow::switch::SwitchBody; + match body { + SwitchBody::BraceDelimited(b) => { + for case in b.cases.iter() { + for s in case.statements().iter() { + collect_from_statement(s, ctx); + } + } + } + SwitchBody::ColonDelimited(b) => { + for case in b.cases.iter() { + for s in case.statements().iter() { + collect_from_statement(s, ctx); + } + } + } + } +} + +/// Check an expression for property assignments (`$this->prop = expr`). +fn check_expression_for_property_assignment(expr: &Expression<'_>, ctx: &mut PropertyCheckCtx<'_>) { + // Only handle plain `=` assignments, not compound (`+=`, `.=`, etc.). + let assign = match expr { + Expression::Assignment(a) if a.operator.is_assign() => a, + _ => return, + }; + + // Check if the LHS is a property access. + let (prop_name, prop_class) = match assign.lhs { + // Instance property: `$this->propName = expr` + Expression::Access(Access::Property(pa)) => { + let is_this = matches!( + pa.object, + Expression::Variable(Variable::Direct(dv)) if dv.name == b"$this" + ); + if !is_this { + return; + } + let name = match &pa.property { + ClassLikeMemberSelector::Identifier(ident) => bytes_to_str(ident.value).to_string(), + _ => return, // Dynamic property name -- skip + }; + + let offset = pa.object.span().start.offset; + let enclosing = find_innermost_enclosing_class(&ctx.file_ctx.classes, offset); + match enclosing { + Some(cls) => (name, cls), + None => return, + } + } + // Static property: `self::$propName = expr` or `static::$propName = expr` + Expression::Access(Access::StaticProperty(spa)) => { + let class_name = match spa.class { + Expression::Identifier(ident) => { + let raw = bytes_to_str(ident.value()); + let lower = raw.to_ascii_lowercase(); + if lower != "self" && lower != "static" { + return; // Only handle self/static for now + } + raw.to_string() + } + _ => return, + }; + let _ = class_name; // We use offset-based class lookup + + let prop_name = match &spa.property { + Variable::Direct(dv) => { + let raw = bytes_to_str(dv.name).to_string(); + raw.strip_prefix('$').unwrap_or(&raw).to_string() + } + _ => return, // Dynamic variable name -- skip + }; + + let offset = spa.class.span().start.offset; + let enclosing = find_innermost_enclosing_class(&ctx.file_ctx.classes, offset); + match enclosing { + Some(cls) => (prop_name, cls), + None => return, + } + } + _ => return, + }; + + // Look up the property's declared type. + let declared_type = prop_class + .properties + .iter() + .find(|p| p.name == prop_name) + .and_then(|p| p.type_hint.clone()); + + let declared_type = match declared_type { + Some(t) if !t.is_untyped() && !t.is_mixed() => t, + _ => return, + }; + + // Resolve `self`/`static`/`parent` in the declared property type. + let declared_type = declared_type.resolve_names(&|name: &str| { + let lower = name.to_ascii_lowercase(); + match lower.as_str() { + "self" | "static" | "$this" => prop_class.fqn().to_string(), + "parent" => prop_class + .parent_class + .as_ref() + .map(|p| p.to_string()) + .unwrap_or_else(|| name.to_string()), + _ => { + if name.contains("__anonymous@") { + return name.to_string(); + } + if let Some(cls) = (ctx.class_loader)(name) { + cls.fqn().to_string() + } else { + name.to_string() + } + } + } + }); + + // Resolve the RHS expression type. + let rhs_span = assign.rhs.span(); + let rhs_start = rhs_span.start.offset as usize; + let rhs_end = rhs_span.end.offset as usize; + + let loaders = Loaders { + function_loader: Some(ctx.function_loader), + constant_loader: Some(ctx.constant_loader), + }; + + let var_ctx = VarResolutionCtx { + var_name: "", + top_level_scope: None, + current_class: prop_class, + all_classes: &ctx.file_ctx.classes, + content: ctx.content, + cursor_offset: rhs_start as u32, + class_loader: ctx.class_loader, + loaders, + resolved_class_cache: Some(&ctx.backend.resolved_class_cache), + enclosing_return_type: None, + branch_aware: true, + match_arm_narrowing: HashMap::new(), + scope_var_resolver: None, + }; + + let rhs_type = resolve_expression_type(assign.rhs, &var_ctx).unwrap_or_else(PhpType::untyped); + + // Skip unresolved types. + if rhs_type.is_untyped() + || rhs_type.is_empty() + || matches!(&rhs_type, PhpType::Raw(s) if s.is_empty()) + { + return; + } + + // Resolve short class names to FQN. + let rhs_type = rhs_type.resolve_names(&|name: &str| { + if name.contains("__anonymous@") { + return name.to_string(); + } + if let Some(cls) = (ctx.class_loader)(name) { + cls.fqn().to_string() + } else { + name.to_string() + } + }); + + ctx.out.push(ResolvedPropertyAssignment { + rhs_type, + declared_type, + start: rhs_start, + end: rhs_end, + property_name: prop_name, + }); +} + +// ── Main diagnostic collection ────────────────────────────────────────────── + +impl Backend { + /// Collect property type assignment mismatch diagnostics for a single file. + /// + /// Appends diagnostics to `out`. The caller is responsible for + /// publishing them via `textDocument/publishDiagnostics`. + pub fn collect_property_type_diagnostics( + &self, + uri: &str, + content: &str, + out: &mut Vec, + ) { + let file_ctx = self.file_context(uri); + + let _parse_guard = with_parse_cache(content); + + let class_loader = self.class_loader(&file_ctx); + let function_loader_cl = self.function_loader(&file_ctx); + let constant_loader_cl = self.constant_loader(); + + let results: Vec = + with_parsed_program(content, "property_type_diagnostics", |program, _content| { + let mut resolved: Vec = Vec::new(); + + let mut ctx = PropertyCheckCtx { + content, + file_ctx: &file_ctx, + class_loader: &class_loader, + function_loader: &function_loader_cl, + constant_loader: &constant_loader_cl, + backend: self, + out: &mut resolved, + }; + + for stmt in program.statements.iter() { + collect_from_statement(stmt, &mut ctx); + } + + resolved + }); + + // Emit diagnostics for incompatible property assignments. + let strict_types = with_parsed_program(content, "property_strict", |program, _| { + has_strict_types(program) + }); + + for assignment in &results { + if is_type_compatible( + &assignment.rhs_type, + &assignment.declared_type, + &class_loader, + strict_types, + ) { + continue; + } + + let range = match self.offset_range_to_lsp_range( + uri, + content, + assignment.start, + assignment.end, + ) { + Some(r) => r, + None => continue, + }; + + let message = format!( + "Property ${} expects {}, got {}", + assignment.property_name, assignment.declared_type, assignment.rhs_type, + ); + + out.push(make_diagnostic( + range, + DiagnosticSeverity::ERROR, + TYPE_MISMATCH_PROPERTY_CODE, + message, + )); + } + } +} diff --git a/src/diagnostics/return_type_errors.rs b/src/diagnostics/return_type_errors.rs new file mode 100644 index 00000000..a1bb470e --- /dev/null +++ b/src/diagnostics/return_type_errors.rs @@ -0,0 +1,807 @@ +//! Return type mismatch diagnostics. +//! +//! Walk methods and functions in the file and flag every `return` +//! statement where the returned expression's resolved type is +//! incompatible with the declared return type. +//! +//! Uses the same conservative approach as argument type checking: +//! when in doubt (unresolved types, `mixed`, complex generics), +//! the diagnostic is suppressed to avoid false positives. + +use std::collections::HashMap; +use std::sync::Arc; + +use mago_span::HasSpan; +use mago_syntax::ast::expression::Expression; +use mago_syntax::ast::statement::Statement; + +use tower_lsp::lsp_types::*; + +use crate::Backend; +use crate::atom::bytes_to_str; +use crate::completion::resolver::{Loaders, VarResolutionCtx}; +use crate::completion::variable::foreach_resolution::resolve_expression_type; +use crate::parser::{with_parse_cache, with_parsed_program}; +use crate::php_type::PhpType; +use crate::types::ClassInfo; + +use super::helpers::{find_innermost_enclosing_class, make_diagnostic}; +use super::type_errors::{has_strict_types, is_type_compatible}; + +/// Diagnostic code used for return type mismatch diagnostics. +pub(crate) const TYPE_MISMATCH_RETURN_CODE: &str = "type_mismatch_return"; + +// ── Collected return site info ────────────────────────────────────────────── + +/// A single return statement's resolved type plus the byte range of +/// the return expression in source. +struct ResolvedReturn { + /// The resolved type of the return expression, or `None` for bare `return;`. + ty: Option, + /// Byte offset of the return expression (or `return` keyword for bare returns) start (inclusive). + start: usize, + /// Byte offset of the return expression (or `return` keyword for bare returns) end (exclusive). + end: usize, + /// The declared return type of the enclosing function/method. + declared_type: PhpType, +} + +// ── AST walkers ───────────────────────────────────────────────────────────── + +/// Check whether a statement list contains any `yield` expression +/// (indicating a generator function whose return type semantics differ). +fn body_contains_yield(stmts: &mago_syntax::ast::sequence::Sequence<'_, Statement<'_>>) -> bool { + for stmt in stmts.iter() { + if stmt_contains_yield(stmt) { + return true; + } + } + false +} + +fn stmt_contains_yield(stmt: &Statement<'_>) -> bool { + match stmt { + Statement::Expression(expr_stmt) => expr_contains_yield(expr_stmt.expression), + Statement::Return(ret) => ret.value.is_some_and(|v| expr_contains_yield(v)), + Statement::Echo(echo) => echo.values.iter().any(|e| expr_contains_yield(e)), + Statement::If(if_stmt) => { + expr_contains_yield(if_stmt.condition) || if_body_contains_yield(&if_stmt.body) + } + Statement::While(w) => { + expr_contains_yield(w.condition) + || w.body.statements().iter().any(|s| stmt_contains_yield(s)) + } + Statement::DoWhile(dw) => { + stmt_contains_yield(dw.statement) || expr_contains_yield(dw.condition) + } + Statement::For(f) => { + f.initializations.iter().any(|e| expr_contains_yield(e)) + || f.conditions.iter().any(|e| expr_contains_yield(e)) + || f.increments.iter().any(|e| expr_contains_yield(e)) + || f.body.statements().iter().any(|s| stmt_contains_yield(s)) + } + Statement::Foreach(fe) => { + expr_contains_yield(fe.expression) + || fe.body.statements().iter().any(|s| stmt_contains_yield(s)) + } + Statement::Switch(sw) => { + expr_contains_yield(sw.expression) || switch_body_contains_yield(&sw.body) + } + Statement::Try(t) => { + t.block.statements.iter().any(|s| stmt_contains_yield(s)) + || t.catch_clauses + .iter() + .any(|c| c.block.statements.iter().any(|s| stmt_contains_yield(s))) + || t.finally_clause + .as_ref() + .is_some_and(|f| f.block.statements.iter().any(|s| stmt_contains_yield(s))) + } + Statement::Block(b) => b.statements.iter().any(|s| stmt_contains_yield(s)), + // Don't recurse into nested functions/closures — their yields + // don't make the *enclosing* function a generator. + _ => false, + } +} + +fn if_body_contains_yield(body: &mago_syntax::ast::control_flow::r#if::IfBody<'_>) -> bool { + use mago_syntax::ast::control_flow::r#if::IfBody; + match body { + IfBody::Statement(inner) => { + stmt_contains_yield(inner.statement) + || inner + .else_if_clauses + .iter() + .any(|c| stmt_contains_yield(c.statement)) + || inner + .else_clause + .as_ref() + .is_some_and(|c| stmt_contains_yield(c.statement)) + } + IfBody::ColonDelimited(body) => { + body.statements.iter().any(|s| stmt_contains_yield(s)) + || body + .else_if_clauses + .iter() + .any(|c| c.statements.iter().any(|s| stmt_contains_yield(s))) + || body + .else_clause + .as_ref() + .is_some_and(|c| c.statements.iter().any(|s| stmt_contains_yield(s))) + } + } +} + +fn switch_body_contains_yield( + body: &mago_syntax::ast::control_flow::switch::SwitchBody<'_>, +) -> bool { + use mago_syntax::ast::control_flow::switch::SwitchBody; + match body { + SwitchBody::BraceDelimited(b) => b + .cases + .iter() + .any(|c| c.statements().iter().any(|s| stmt_contains_yield(s))), + SwitchBody::ColonDelimited(b) => b + .cases + .iter() + .any(|c| c.statements().iter().any(|s| stmt_contains_yield(s))), + } +} + +fn expr_contains_yield(expr: &Expression<'_>) -> bool { + matches!(expr, Expression::Yield(_)) +} + +/// Collect return expressions from a function/method body. +/// +/// Recurses into control flow blocks (if/else, for, while, try/catch, +/// etc.) but does NOT recurse into closures, arrow functions, or nested +/// function declarations — those have their own return types. +fn collect_returns_from_body<'a>( + stmts: &mago_syntax::ast::sequence::Sequence<'a, Statement<'a>>, + returns: &mut Vec<(Option<&'a Expression<'a>>, usize, usize)>, +) { + for stmt in stmts.iter() { + collect_returns_from_stmt(stmt, returns); + } +} + +fn collect_returns_from_stmt<'a>( + stmt: &Statement<'a>, + returns: &mut Vec<(Option<&'a Expression<'a>>, usize, usize)>, +) { + match stmt { + Statement::Return(ret) => { + if let Some(val) = ret.value { + let span = val.span(); + returns.push(( + Some(val), + span.start.offset as usize, + span.end.offset as usize, + )); + } else { + // Bare `return;` — use the `return` keyword span. + let kw_span = ret.r#return.span; + returns.push(( + None, + kw_span.start.offset as usize, + kw_span.end.offset as usize, + )); + } + } + Statement::Namespace(ns) => { + for inner in ns.statements().iter() { + collect_returns_from_stmt(inner, returns); + } + } + Statement::If(if_stmt) => { + collect_returns_from_if_body(&if_stmt.body, returns); + } + Statement::While(w) => { + for s in w.body.statements() { + collect_returns_from_stmt(s, returns); + } + } + Statement::DoWhile(dw) => { + collect_returns_from_stmt(dw.statement, returns); + } + Statement::For(f) => { + for s in f.body.statements() { + collect_returns_from_stmt(s, returns); + } + } + Statement::Foreach(fe) => { + for s in fe.body.statements() { + collect_returns_from_stmt(s, returns); + } + } + Statement::Switch(sw) => { + collect_returns_from_switch_body(&sw.body, returns); + } + Statement::Try(t) => { + for s in t.block.statements.iter() { + collect_returns_from_stmt(s, returns); + } + for catch in t.catch_clauses.iter() { + for s in catch.block.statements.iter() { + collect_returns_from_stmt(s, returns); + } + } + if let Some(ref finally) = t.finally_clause { + for s in finally.block.statements.iter() { + collect_returns_from_stmt(s, returns); + } + } + } + Statement::Block(block) => { + for s in block.statements.iter() { + collect_returns_from_stmt(s, returns); + } + } + Statement::Declare(declare) => { + use mago_syntax::ast::declare::DeclareBody; + match &declare.body { + DeclareBody::Statement(inner) => { + collect_returns_from_stmt(inner, returns); + } + DeclareBody::ColonDelimited(body) => { + for s in body.statements.iter() { + collect_returns_from_stmt(s, returns); + } + } + } + } + // Do NOT recurse into closures, arrow functions, or nested + // functions — they have their own return types. + Statement::Function(_) => {} + Statement::Class(_) + | Statement::Interface(_) + | Statement::Trait(_) + | Statement::Enum(_) => {} + _ => {} + } +} + +fn collect_returns_from_if_body<'a>( + body: &mago_syntax::ast::control_flow::r#if::IfBody<'a>, + returns: &mut Vec<(Option<&'a Expression<'a>>, usize, usize)>, +) { + use mago_syntax::ast::control_flow::r#if::IfBody; + match body { + IfBody::Statement(inner) => { + collect_returns_from_stmt(inner.statement, returns); + for c in inner.else_if_clauses.iter() { + collect_returns_from_stmt(c.statement, returns); + } + if let Some(ref c) = inner.else_clause { + collect_returns_from_stmt(c.statement, returns); + } + } + IfBody::ColonDelimited(body) => { + for s in body.statements.iter() { + collect_returns_from_stmt(s, returns); + } + for c in body.else_if_clauses.iter() { + for s in c.statements.iter() { + collect_returns_from_stmt(s, returns); + } + } + if let Some(ref c) = body.else_clause { + for s in c.statements.iter() { + collect_returns_from_stmt(s, returns); + } + } + } + } +} + +fn collect_returns_from_switch_body<'a>( + body: &mago_syntax::ast::control_flow::switch::SwitchBody<'a>, + returns: &mut Vec<(Option<&'a Expression<'a>>, usize, usize)>, +) { + use mago_syntax::ast::control_flow::switch::SwitchBody; + match body { + SwitchBody::BraceDelimited(b) => { + for case in b.cases.iter() { + for s in case.statements().iter() { + collect_returns_from_stmt(s, returns); + } + } + } + SwitchBody::ColonDelimited(b) => { + for case in b.cases.iter() { + for s in case.statements().iter() { + collect_returns_from_stmt(s, returns); + } + } + } + } +} + +// ── Main diagnostic collection ────────────────────────────────────────────── + +impl Backend { + /// Collect return type mismatch diagnostics for a single file. + /// + /// Appends diagnostics to `out`. The caller is responsible for + /// publishing them via `textDocument/publishDiagnostics`. + pub fn collect_return_type_diagnostics( + &self, + uri: &str, + content: &str, + out: &mut Vec, + ) { + let file_ctx = self.file_context(uri); + + let _parse_guard = with_parse_cache(content); + + let class_loader = self.class_loader(&file_ctx); + let function_loader_cl = self.function_loader(&file_ctx); + let constant_loader_cl = self.constant_loader(); + let default_class = ClassInfo::default(); + + // Walk the AST, find return statements in method/function + // bodies, resolve their types, and pair them with the declared + // return type. + let results: Vec = + with_parsed_program(content, "return_type_diagnostics", |program, _content| { + let strict_types = has_strict_types(program); + let _ = strict_types; // used below in VarResolutionCtx + let mut resolved_returns: Vec = Vec::new(); + + for stmt in program.statements.iter() { + process_top_level_statement( + stmt, + content, + &file_ctx, + &class_loader, + &function_loader_cl, + &constant_loader_cl, + &default_class, + self, + &mut resolved_returns, + ); + } + + resolved_returns + }); + + // Emit diagnostics for incompatible returns. + let strict_types_for_check = with_parsed_program(content, "return_strict", |program, _| { + has_strict_types(program) + }); + + for ret in &results { + let range = match self.offset_range_to_lsp_range(uri, content, ret.start, ret.end) { + Some(r) => r, + None => continue, + }; + + let message = match &ret.ty { + // Bare `return;` in a void function — OK. + None if ret.declared_type.is_void() => continue, + // Bare `return;` in a non-void function — error. + None => format!( + "Function with return type {} must not return without a value", + ret.declared_type, + ), + // `return $expr;` in a void function — error. + Some(_) if ret.declared_type.is_void() => { + "Void function must not return a value".to_string() + } + // `return $expr;` with a compatible type — OK. + Some(ty) + if is_type_compatible( + ty, + &ret.declared_type, + &class_loader, + strict_types_for_check, + ) => + { + continue; + } + // `return $expr;` with an incompatible type — error. + Some(ty) => format!( + "Return type {} is incompatible with declared return type {}", + ty, ret.declared_type, + ), + }; + + out.push(make_diagnostic( + range, + DiagnosticSeverity::ERROR, + TYPE_MISMATCH_RETURN_CODE, + message, + )); + } + } +} + +#[allow(clippy::too_many_arguments)] +/// Resolve the type of a return expression and push a `ResolvedReturn`. +/// +/// For bare `return;` statements (`maybe_expr` is `None`), pushes with +/// `ty: None` — the diagnostic emission handles these specially. +/// For `return $expr;`, resolves the expression type and pushes with +/// `ty: Some(resolved_type)`. +fn resolve_return_and_push( + maybe_expr: Option<&Expression<'_>>, + start: usize, + end: usize, + declared_return: &PhpType, + current_class: &ClassInfo, + content: &str, + all_classes: &[Arc], + class_loader: &dyn Fn(&str) -> Option>, + loaders: Loaders<'_>, + backend: &Backend, + out: &mut Vec, +) { + match maybe_expr { + None => { + // Bare `return;` — push with ty: None for the emission logic. + out.push(ResolvedReturn { + ty: None, + start, + end, + declared_type: declared_return.clone(), + }); + } + Some(expr) => { + // `return $expr;` — skip void-declared functions here; + // they'll be flagged regardless of the expression type. + if declared_return.is_void() { + out.push(ResolvedReturn { + ty: Some(PhpType::untyped()), // placeholder; message ignores it + start, + end, + declared_type: declared_return.clone(), + }); + return; + } + + let var_ctx = VarResolutionCtx { + var_name: "", + top_level_scope: None, + current_class, + all_classes, + content, + cursor_offset: start as u32, + class_loader, + loaders, + resolved_class_cache: Some(&backend.resolved_class_cache), + enclosing_return_type: None, + branch_aware: true, + match_arm_narrowing: HashMap::new(), + scope_var_resolver: None, + }; + + let ty = resolve_expression_type(expr, &var_ctx).unwrap_or_else(PhpType::untyped); + + // Skip unresolved types. + if ty.is_untyped() || ty.is_empty() || matches!(&ty, PhpType::Raw(s) if s.is_empty()) { + return; + } + + // Resolve short class names to FQN. + let ty = ty.resolve_names(&|name: &str| { + if name.contains("__anonymous@") { + return name.to_string(); + } + if let Some(cls) = class_loader(name) { + cls.fqn().to_string() + } else { + name.to_string() + } + }); + + out.push(ResolvedReturn { + ty: Some(ty), + start, + end, + declared_type: declared_return.clone(), + }); + } + } +} + +#[allow(clippy::too_many_arguments)] +/// Walk a top-level statement looking for function/class declarations. +fn process_top_level_statement( + stmt: &Statement<'_>, + content: &str, + file_ctx: &crate::types::FileContext, + class_loader: &dyn Fn(&str) -> Option>, + function_loader: &dyn Fn(&str) -> Option, + constant_loader: &dyn Fn(&str) -> Option>, + default_class: &ClassInfo, + backend: &Backend, + out: &mut Vec, +) { + match stmt { + Statement::Namespace(ns) => { + for inner in ns.statements().iter() { + process_top_level_statement( + inner, + content, + file_ctx, + class_loader, + function_loader, + constant_loader, + default_class, + backend, + out, + ); + } + } + Statement::Class(class) => { + for member in class.members.iter() { + process_class_member( + member, + content, + file_ctx, + class_loader, + function_loader, + constant_loader, + default_class, + backend, + out, + ); + } + } + Statement::Interface(iface) => { + for member in iface.members.iter() { + process_class_member( + member, + content, + file_ctx, + class_loader, + function_loader, + constant_loader, + default_class, + backend, + out, + ); + } + } + Statement::Trait(trait_def) => { + for member in trait_def.members.iter() { + process_class_member( + member, + content, + file_ctx, + class_loader, + function_loader, + constant_loader, + default_class, + backend, + out, + ); + } + } + Statement::Enum(enum_def) => { + for member in enum_def.members.iter() { + process_class_member( + member, + content, + file_ctx, + class_loader, + function_loader, + constant_loader, + default_class, + backend, + out, + ); + } + } + Statement::Function(func) => { + let func_name = bytes_to_str(func.name.value); + let func_offset = func.name.span.start.offset; + + // Extract the declared return type. Prefer the AST's native + // return type hint (always available for the current file), + // then fall back to the global function index (which may + // carry a richer docblock-enriched type). + let declared_return = func + .return_type_hint + .as_ref() + .map(|rth| crate::parser::extract_hint_type(&rth.hint)) + .or_else(|| { + let fqn = file_ctx.resolve_name_at(func_name, func_offset); + backend + .global_functions() + .read() + .get(&fqn) + .and_then(|(_, fi)| fi.return_type.clone()) + .or_else(|| { + backend + .global_functions() + .read() + .get(func_name) + .and_then(|(_, fi)| fi.return_type.clone()) + }) + }); + + let declared_return = match declared_return { + Some(t) if !t.is_untyped() && !t.is_mixed() => t, + _ => return, + }; + + // Skip generators. + if body_contains_yield(&func.body.statements) { + return; + } + + // Collect return statements (both bare and with values). + let mut returns: Vec<(Option<&Expression<'_>>, usize, usize)> = Vec::new(); + collect_returns_from_body(&func.body.statements, &mut returns); + + if returns.is_empty() { + return; + } + + // Resolve types and check. + let enclosing = find_innermost_enclosing_class(&file_ctx.classes, func_offset); + let current_class = enclosing.unwrap_or(default_class); + + let loaders = Loaders { + function_loader: Some(function_loader), + constant_loader: Some(constant_loader), + }; + + for (maybe_expr, start, end) in returns { + resolve_return_and_push( + maybe_expr, + start, + end, + &declared_return, + current_class, + content, + &file_ctx.classes, + class_loader, + loaders, + backend, + out, + ); + } + } + Statement::Declare(declare) => { + use mago_syntax::ast::declare::DeclareBody; + match &declare.body { + DeclareBody::Statement(inner) => { + process_top_level_statement( + inner, + content, + file_ctx, + class_loader, + function_loader, + constant_loader, + default_class, + backend, + out, + ); + } + DeclareBody::ColonDelimited(body) => { + for s in body.statements.iter() { + process_top_level_statement( + s, + content, + file_ctx, + class_loader, + function_loader, + constant_loader, + default_class, + backend, + out, + ); + } + } + } + } + _ => {} + } +} + +#[allow(clippy::too_many_arguments)] +/// Process a class member (looking for methods with return types). +fn process_class_member( + member: &mago_syntax::ast::class_like::member::ClassLikeMember<'_>, + content: &str, + file_ctx: &crate::types::FileContext, + class_loader: &dyn Fn(&str) -> Option>, + function_loader: &dyn Fn(&str) -> Option, + constant_loader: &dyn Fn(&str) -> Option>, + _default_class: &ClassInfo, + backend: &Backend, + out: &mut Vec, +) { + use mago_syntax::ast::class_like::member::ClassLikeMember; + use mago_syntax::ast::class_like::method::MethodBody; + + let method = match member { + ClassLikeMember::Method(m) => m, + _ => return, + }; + + let body = match &method.body { + MethodBody::Concrete(block) => &block.statements, + MethodBody::Abstract(_) => return, + }; + + let method_name = bytes_to_str(method.name.value); + let method_offset = method.name.span.start.offset; + + // Find the enclosing class to look up the method's declared return type. + let enclosing = find_innermost_enclosing_class(&file_ctx.classes, method_offset); + let current_class = match enclosing { + Some(cls) => cls, + None => return, + }; + + // Look up the method's declared return type from the parsed MethodInfo. + let declared_return = current_class + .get_method(method_name) + .and_then(|mi| mi.return_type.clone()); + + let declared_return = match declared_return { + Some(t) if !t.is_untyped() && !t.is_mixed() => t, + _ => return, + }; + + // Skip generators. + if body_contains_yield(body) { + return; + } + + // Collect return statements (both bare and with values). + let mut returns: Vec<(Option<&Expression<'_>>, usize, usize)> = Vec::new(); + collect_returns_from_body(body, &mut returns); + + if returns.is_empty() { + return; + } + + // Resolve the declared return type's `self`/`static`/`parent` to + // concrete class names for accurate comparison. + let declared_return = declared_return.resolve_names(&|name: &str| { + let lower = name.to_ascii_lowercase(); + match lower.as_str() { + "self" | "static" | "$this" => current_class.fqn().to_string(), + "parent" => current_class + .parent_class + .as_ref() + .map(|p| p.to_string()) + .unwrap_or_else(|| name.to_string()), + _ => { + if name.contains("__anonymous@") { + return name.to_string(); + } + if let Some(cls) = class_loader(name) { + cls.fqn().to_string() + } else { + name.to_string() + } + } + } + }); + + let loaders = Loaders { + function_loader: Some(function_loader), + constant_loader: Some(constant_loader), + }; + + for (maybe_expr, start, end) in returns { + resolve_return_and_push( + maybe_expr, + start, + end, + &declared_return, + current_class, + content, + &file_ctx.classes, + class_loader, + loaders, + backend, + out, + ); + } +} diff --git a/src/diagnostics/type_errors.rs b/src/diagnostics/type_errors.rs index 44ad2d13..ea98fe6c 100644 --- a/src/diagnostics/type_errors.rs +++ b/src/diagnostics/type_errors.rs @@ -68,7 +68,7 @@ fn is_bare_array(ty: &PhpType) -> bool { /// Returns `true` if the argument type can be passed to the parameter /// without a type error. Conservative: returns `true` (compatible) /// when in doubt. -fn is_type_compatible( +pub(super) fn is_type_compatible( arg_type: &PhpType, param_type: &PhpType, class_loader: &dyn Fn(&str) -> Option>, @@ -776,7 +776,7 @@ fn is_refined_scalar_pair(arg: &PhpType, param: &PhpType) -> bool { /// `declare(strict_types=1)` directive. In PHP this must appear as /// the very first statement (after `) -> bool { +pub(super) fn has_strict_types(program: &Program<'_>) -> bool { for stmt in program.statements.iter() { if let Statement::Declare(declare) = stmt { for item in declare.items.iter() { @@ -1433,7 +1433,7 @@ impl Backend { /// /// Appends diagnostics to `out`. The caller is responsible for /// publishing them via `textDocument/publishDiagnostics`. - pub fn collect_type_error_diagnostics( + pub fn collect_argument_type_diagnostics( &self, uri: &str, content: &str, diff --git a/tests/integration/diagnostics_property_type_errors.rs b/tests/integration/diagnostics_property_type_errors.rs new file mode 100644 index 00000000..ab32d78c --- /dev/null +++ b/tests/integration/diagnostics_property_type_errors.rs @@ -0,0 +1,2203 @@ +use crate::common::create_test_backend; +use tower_lsp::lsp_types::*; + +// ─── Helpers ──────────────────────────────────────────────────────────────── + +fn collect(php: &str) -> Vec { + let backend = create_test_backend(); + let uri = "file:///test.php"; + backend.update_ast(uri, php); + let mut out = Vec::new(); + backend.collect_property_type_diagnostics(uri, php, &mut out); + out +} + +fn has_property_error(diags: &[Diagnostic]) -> bool { + diags.iter().any(|d| { + d.code.as_ref().is_some_and( + |c| matches!(c, NumberOrString::String(s) if s == "type_mismatch_property"), + ) + }) +} + +fn property_error_messages(diags: &[Diagnostic]) -> Vec { + diags + .iter() + .filter(|d| { + d.code.as_ref().is_some_and( + |c| matches!(c, NumberOrString::String(s) if s == "type_mismatch_property"), + ) + }) + .map(|d| d.message.clone()) + .collect() +} + +// ─── Basic: assign wrong type to property ─────────────────────────────────── + +#[test] +fn flags_string_assigned_to_int_property() { + let php = r#"count = "hello"; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected property type error for string assigned to int, got: {diags:?}" + ); + let msgs = property_error_messages(&diags); + assert!( + msgs.iter().any(|m| m.contains("count")), + "Expected message mentioning property name, got: {msgs:?}" + ); +} + +// ─── Correct assignment — no diagnostic ──────────────────────────────────── + +#[test] +fn no_diagnostic_for_correct_property_assignment() { + let php = r#"name = "hello"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct property assignment, got: {diags:?}" + ); +} + +// ─── Assign null to non-nullable property ────────────────────────────────── + +#[test] +fn flags_null_assigned_to_non_nullable_property() { + let php = r#"name = null; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected property type error for null assigned to string, got: {diags:?}" + ); +} + +// ─── Assign null to nullable property — OK ───────────────────────────────── + +#[test] +fn no_diagnostic_for_null_to_nullable_property() { + let php = r#"name = null; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag null assigned to ?string, got: {diags:?}" + ); +} + +// ─── Assign int to string property ───────────────────────────────────────── + +#[test] +fn flags_array_assigned_to_string_property_basic() { + let php = r#"label = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected property type error for array assigned to string, got: {diags:?}" + ); +} + +// ─── Assign bool to int property ─────────────────────────────────────────── + +#[test] +fn flags_bool_assigned_to_int_property() { + let php = r#"value = true; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected property type error for bool assigned to int, got: {diags:?}" + ); +} + +// ─── Untyped property — no diagnostic ────────────────────────────────────── + +#[test] +fn no_diagnostic_for_untyped_property() { + let php = r#"anything = 42; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag untyped property, got: {diags:?}" + ); +} + +// ─── Mixed property — no diagnostic ──────────────────────────────────────── + +#[test] +fn no_diagnostic_for_mixed_property() { + let php = r#"data = "hello"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag mixed property, got: {diags:?}" + ); +} + +// ─── Union property type — compatible ────────────────────────────────────── + +#[test] +fn no_diagnostic_for_union_property_correct() { + let php = r#"value = 42; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag int assigned to string|int, got: {diags:?}" + ); +} + +// ─── Union property type — incompatible ──────────────────────────────────── + +#[test] +fn flags_bool_assigned_to_string_or_int_property() { + let php = r#"value = true; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected property type error for bool assigned to string|int, got: {diags:?}" + ); +} + +// ─── Array assigned to string property ───────────────────────────────────── + +#[test] +fn flags_array_assigned_to_string_property() { + let php = r#"name = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected property type error for array assigned to string, got: {diags:?}" + ); +} + +// ─── Compound assignment (+=) — not flagged ──────────────────────────────── + +#[test] +fn no_diagnostic_for_compound_assignment() { + let php = r#"count += 1; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag compound assignment (+=), got: {diags:?}" + ); +} + +// ─── Assignment inside if/else ───────────────────────────────────────────── + +#[test] +fn flags_wrong_assignment_in_if_branch() { + let php = r#"count = "wrong"; + } else { + $this->count = 42; + } + } +} +"#; + let diags = collect(php); + let msgs = property_error_messages(&diags); + assert_eq!( + msgs.len(), + 1, + "Expected exactly one property error (in if branch), got: {msgs:?}" + ); +} + +// ─── Assignment in constructor ───────────────────────────────────────────── + +#[test] +fn flags_wrong_assignment_in_constructor() { + let php = r#"name = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected property type error in constructor, got: {diags:?}" + ); +} + +// ─── Correct constructor assignment — no diagnostic ──────────────────────── + +#[test] +fn no_diagnostic_for_correct_constructor_assignment() { + let php = r#"name = $name; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct constructor assignment, got: {diags:?}" + ); +} + +// ─── Assignment in try/catch ─────────────────────────────────────────────── + +#[test] +fn flags_wrong_assignment_in_try() { + let php = r#"result = false; + } catch (\Exception $e) { + $this->result = "error"; + } + } +} +"#; + let diags = collect(php); + let msgs = property_error_messages(&diags); + assert_eq!( + msgs.len(), + 1, + "Expected one property error (in try block), got: {msgs:?}" + ); +} + +// ─── Multiple wrong assignments ──────────────────────────────────────────── + +#[test] +fn flags_multiple_wrong_assignments() { + let php = r#"a = "wrong"; + $this->b = []; + } +} +"#; + let diags = collect(php); + let msgs = property_error_messages(&diags); + assert_eq!(msgs.len(), 2, "Expected two property errors, got: {msgs:?}"); +} + +// ─── Assignment inside foreach ───────────────────────────────────────────── + +#[test] +fn flags_wrong_assignment_in_foreach() { + let php = r#"total = "wrong"; + } + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected property type error in foreach, got: {diags:?}" + ); +} + +// ─── Assignment inside while ─────────────────────────────────────────────── + +#[test] +fn flags_wrong_assignment_in_while() { + let php = r#"running = "yes"; + } + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected property type error in while loop, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: type juggling (non-strict mode) +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_int_assigned_to_string_property_non_strict() { + // PHP coerces int to string in non-strict mode. + let php = r#"text = 42; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag int assigned to string property (type juggling), got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_float_assigned_to_string_property_non_strict() { + let php = r#"value = 3.14; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag float assigned to string property (type juggling), got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_int_assigned_to_float_property() { + // int is always widened to float in PHP. + let php = r#"value = 42; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag int assigned to float property (widening), got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: strict_types interactions +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn strict_types_flags_int_assigned_to_string_property() { + let php = r#"name = 42; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for int assigned to string property under strict_types=1, got: {diags:?}" + ); +} + +#[test] +fn strict_types_still_allows_null_for_nullable_property() { + let php = r#"name = null; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "strict_types should not affect nullable null assignment, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: class hierarchy (subclass / interface) +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_subclass_assigned_to_parent_property() { + let php = r#"animal = new Cat(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag subclass Cat assigned to Animal property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_interface_implementor_assigned_to_interface_property() { + let php = r#"logger = new FileLogger(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag interface implementor assigned to interface property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_deep_inheritance_assigned_to_base_property() { + let php = r#"item = new Leaf(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag deep subclass assigned to base-typed property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_object_property_with_class_instance() { + let php = r#"item = new Foo(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag class instance assigned to object property, got: {diags:?}" + ); +} + +#[test] +fn flags_wrong_class_assigned_to_typed_property() { + let php = r#"pet = new Cat(); + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for Cat assigned to Dog property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: Stringable objects assigned to string property +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_stringable_assigned_to_string_property() { + let php = r#"content = new HtmlString(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag Stringable object assigned to string property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: static property assignments +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_static_property_self() { + let php = r#"email = "test@example.com"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag string assigned to ?string property, got: {diags:?}" + ); +} + +#[test] +fn flags_array_assigned_to_nullable_string_property() { + let php = r#"email = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for array assigned to ?string property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: union property types +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_string_assigned_to_union_string_int() { + let php = r#"value = "hello"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag string assigned to string|int property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_null_assigned_to_union_with_null() { + let php = r#"value = null; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag null assigned to string|int|null property, got: {diags:?}" + ); +} + +#[test] +fn flags_bool_assigned_to_union_string_int_null() { + let php = r#"value = true; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for bool assigned to string|int|null property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: iterable / callable / array property types +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_array_assigned_to_iterable_property() { + let php = r#"items = [1, 2, 3]; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag array assigned to iterable property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_array_literal_assigned_to_array_property() { + let php = r#"items = [1, 2, 3]; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag array literal assigned to array property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: trait properties +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_trait_property_assignment() { + let php = r#"name = "hello"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct assignment in trait method, got: {diags:?}" + ); +} + +#[test] +fn flags_wrong_trait_property_assignment() { + let php = r#"name = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for array assigned to string trait property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: constructor promoted properties +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_promoted_property_assignment() { + let php = r#"name = $newName; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct assignment to promoted property, got: {diags:?}" + ); +} + +#[test] +fn flags_wrong_promoted_property_assignment() { + let php = r#"age = "not a number"; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for string assigned to promoted int property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: readonly properties +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_readonly_property_assignment() { + let php = r#"dsn = $dsn; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct assignment to readonly property, got: {diags:?}" + ); +} + +#[test] +fn flags_wrong_readonly_property_assignment() { + let php = r#"dsn = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for array assigned to readonly string property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: bool / true / false property types +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_true_assigned_to_bool_property() { + let php = r#"active = true; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag true assigned to bool property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_false_assigned_to_bool_property() { + let php = r#"active = false; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag false assigned to bool property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: namespaced classes +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_namespaced_property_assignment() { + let php = r#"name = "Alice"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct assignment in namespaced class, got: {diags:?}" + ); +} + +#[test] +fn flags_wrong_namespaced_property_assignment() { + let php = r#"name = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for array assigned to string property in namespaced class, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: multiple classes in same file +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_across_multiple_classes_correct() { + let php = r#"name = "foo"; + } +} + +class Bar { + public int $count; + + public function set(): void { + $this->count = 42; + } +} + +class Baz { + public bool $flag; + + public function set(): void { + $this->flag = true; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag any correct assignments across multiple classes, got: {diags:?}" + ); +} + +#[test] +fn only_wrong_class_flagged_among_multiple_property() { + let php = r#"name = "good"; + } +} + +class Bad { + public int $count; + + public function set(): void { + $this->count = "not a number"; + } +} + +class AlsoGood { + public bool $flag; + + public function set(): void { + $this->flag = true; + } +} +"#; + let diags = collect(php); + let msgs = property_error_messages(&diags); + assert_eq!( + msgs.len(), + 1, + "Expected exactly one property error (in Bad class), got: {msgs:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: multiple properties, only wrong ones flagged +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn flags_only_wrong_properties_in_same_method() { + let php = r#"title = "hello"; + $this->priority = "wrong"; + $this->active = false; + } +} +"#; + let diags = collect(php); + let msgs = property_error_messages(&diags); + assert_eq!( + msgs.len(), + 1, + "Expected exactly one property error (priority), got: {msgs:?}" + ); + assert!( + msgs[0].contains("priority"), + "Expected error to mention 'priority', got: {msgs:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: assignment in switch +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_assignment_in_switch() { + let php = r#"result = "one"; + break; + case 2: + $this->result = "two"; + break; + default: + $this->result = "unknown"; + } + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct assignments in switch, got: {diags:?}" + ); +} + +#[test] +fn flags_wrong_assignment_in_switch_case() { + let php = r#"result = "one"; + break; + default: + $this->result = []; + } + } +} +"#; + let diags = collect(php); + let msgs = property_error_messages(&diags); + assert_eq!( + msgs.len(), + 1, + "Expected one property error (default case), got: {msgs:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: assignment in deeply nested control flow +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_deeply_nested_assignment() { + let php = r#" 0) { + if ($b > 0) { + $this->status = "both positive"; + } else { + $this->status = "a positive"; + } + } else { + $this->status = "a non-positive"; + } + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct assignments in deeply nested if, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: assignment in for/do-while +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_assignment_in_for() { + let php = r#"result = ""; + for ($i = 0; $i < 10; $i++) { + $this->result = "built"; + } + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct assignment in for loop, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_correct_assignment_in_do_while() { + let php = r#"attempts = 0; + do { + $this->attempts = 1; + } while (false); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct assignment in do-while, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: string concat / arithmetic expressions +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_string_concat_assigned_to_string_property() { + let php = r#"message = "Hello, " . $name; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag string concat assigned to string property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: assignment in declare block +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_assignment_in_declare_block() { + let php = r#"name = "hello"; + } + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct assignment inside declare block, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: method return value assigned to property +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_method_return_assigned_to_matching_property() { + let php = r#"text = $h->getText(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag method return matching property type, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: self/static typed properties +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_self_assigned_to_self_property() { + let php = r#"next = $other; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag self assigned to ?self property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_new_self_assigned_to_self_property() { + let php = r#"head = new self(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag new self() assigned to ?self property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_null_assigned_to_nullable_self_property() { + let php = r#"next = null; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag null assigned to ?self property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Genuine errors: various real mismatches that SHOULD be flagged +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn flags_object_assigned_to_int_property() { + let php = r#"count = new Foo(); + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for object assigned to int property, got: {diags:?}" + ); +} + +#[test] +fn flags_null_to_non_nullable_int_property() { + let php = r#"value = null; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for null assigned to int property, got: {diags:?}" + ); +} + +#[test] +fn flags_bool_assigned_to_string_property() { + let php = r#"text = false; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for bool assigned to string property, got: {diags:?}" + ); +} + +#[test] +fn flags_string_assigned_to_bool_property() { + let php = r#"on = "yes"; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for string assigned to bool property, got: {diags:?}" + ); +} + +#[test] +fn flags_array_assigned_to_int_property() { + let php = r#"total = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for array assigned to int property, got: {diags:?}" + ); +} + +#[test] +fn flags_array_assigned_to_bool_property() { + let php = r#"enabled = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for array assigned to bool property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Edge case: compound assignment operators should NOT be flagged +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_concat_assign() { + let php = r#"buffer .= "more"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag .= compound assignment, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_minus_assign() { + let php = r#"count -= 1; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag -= compound assignment, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_multiply_assign() { + let php = r#"value *= 2; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag *= compound assignment, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_null_coalescing_assign() { + let php = r#"name ??= "default"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag ??= compound assignment, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Edge case: dynamic property name — should be skipped +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_dynamic_property_name() { + let php = r#"$prop = "anything"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag dynamic property name assignment, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Edge case: non-$this property access — should be skipped +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_non_this_property_access() { + // We only check $this->prop and self::$prop, not $other->prop + let php = r#"x = "wrong but we skip non-$this"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag assignment to non-$this property (not checked), got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: nullable class-typed properties +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_class_assigned_to_nullable_class_property() { + let php = r#"currentUser = new User(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag User assigned to ?User property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_null_assigned_to_nullable_class_property() { + let php = r#"currentUser = null; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag null assigned to ?User property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_subclass_assigned_to_nullable_parent_property() { + let php = r#"parked = new Car(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag subclass Car assigned to ?Vehicle property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: complex union property types with classes +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_class_in_multi_class_union_property() { + let php = r#"result = new Pending(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag Pending assigned to Success|Failure|Pending property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_false_assigned_to_string_or_false_property() { + let php = r#"value = false; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag false assigned to string|false property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_string_assigned_to_string_or_false_property() { + let php = r#"value = "cached"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag string assigned to string|false property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: intersection typed properties +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_implementing_class_assigned_to_intersection_property() { + let php = r#"collection = new SmartCollection(); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag class implementing both interfaces for intersection property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: property assigned from typed parameter +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_typed_param_assigned_to_matching_property() { + let php = r#"dsn = $dsn; + $this->timeout = $timeout; + $this->debug = $debug; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag typed params assigned to matching properties, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: property assigned from cast expressions +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_int_cast_assigned_to_int_property() { + let php = r#"value = (int) $s; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag (int) cast assigned to int property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_string_cast_assigned_to_string_property() { + let php = r#"output = (string) $v; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag (string) cast assigned to string property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_array_cast_assigned_to_array_property() { + let php = r#"data = (array) $o; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag (array) cast assigned to array property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: real-world patterns — builder, entity, DTO +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_complex_entity_class() { + let php = r#"paid = true; + $this->status = "paid"; + } + + public function addNote(string $note): void { + $this->notes = $note; + } + + public function clearNote(): void { + $this->notes = null; + } + + public function setTotal(float $amount): void { + $this->total = $amount; + } + + public function setItems(array $items): void { + $this->items = $items; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag any correct assignments in complex entity, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_dto_with_promoted_properties() { + let php = r#"phone = $phone; + } + + public function withAge(int $age): void { + $this->age = $age; + } + + public function deactivate(): void { + $this->active = false; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag any correct assignments in DTO with promoted props, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: assignment from ternary / null coalescing +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_ternary_assigned_to_string_property() { + let php = r#"mode = $flag ? "verbose" : "quiet"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag ternary returning strings assigned to string property, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_null_coalescing_assigned_to_string_property() { + let php = r#"name = $input ?? "default"; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag null coalescing assigned to string property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: private/protected properties +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_private_property_assignment() { + let php = r#"secret = "hidden"; + $this->guarded = 42; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag correct private/protected property assignments, got: {diags:?}" + ); +} + +#[test] +fn flags_wrong_private_property_assignment() { + let php = r#"secret = []; + } +} +"#; + let diags = collect(php); + assert!( + has_property_error(&diags), + "Expected error for array assigned to private string property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: multiple assignment targets in same statement line +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn flags_only_mismatched_in_sequential_assignments() { + let php = r#"a = "ok"; + $this->b = []; + $this->c = true; + $this->d = "wrong"; + } +} +"#; + let diags = collect(php); + let msgs = property_error_messages(&diags); + // $b gets array (wrong), $d gets string (wrong in strict; but float + // may or may not be juggled from string — depends on strict mode). + // At minimum $b should be flagged. + assert!( + msgs.iter().any(|m| m.contains("$b") || m.contains("b")), + "Expected at least $b to be flagged, got: {msgs:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: exception hierarchy property +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_exception_subclass_assigned_to_exception_property() { + let php = r#"lastError = new ValidationException("bad"); + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag deep exception subclass assigned to RuntimeException property, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: enum property assignment +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_backed_enum_value_in_string_context() { + // Backed enum ->value is a string, assigning to string property + // is valid but we don't necessarily track ->value type. Ensure + // we at least don't false-positive here. + let php = r#"primary = Color::Red->value; + } +} +"#; + let diags = collect(php); + assert!( + !has_property_error(&diags), + "Should not flag backed enum ->value assigned to string property, got: {diags:?}" + ); +} diff --git a/tests/integration/diagnostics_return_type_errors.rs b/tests/integration/diagnostics_return_type_errors.rs new file mode 100644 index 00000000..5dfebefd --- /dev/null +++ b/tests/integration/diagnostics_return_type_errors.rs @@ -0,0 +1,2730 @@ +use crate::common::create_test_backend; +use tower_lsp::lsp_types::*; + +// ─── Helpers ──────────────────────────────────────────────────────────────── + +fn collect(php: &str) -> Vec { + let backend = create_test_backend(); + let uri = "file:///test.php"; + backend.update_ast(uri, php); + let mut out = Vec::new(); + backend.collect_return_type_diagnostics(uri, php, &mut out); + out +} + +fn has_return_error(diags: &[Diagnostic]) -> bool { + diags.iter().any(|d| { + d.code + .as_ref() + .is_some_and(|c| matches!(c, NumberOrString::String(s) if s == "type_mismatch_return")) + }) +} + +fn return_error_messages(diags: &[Diagnostic]) -> Vec { + diags + .iter() + .filter(|d| { + d.code.as_ref().is_some_and( + |c| matches!(c, NumberOrString::String(s) if s == "type_mismatch_return"), + ) + }) + .map(|d| d.message.clone()) + .collect() +} + +// ─── Basic: return wrong type from function ───────────────────────────────── + +#[test] +fn flags_array_returned_from_string_function_basic() { + let php = r#" $x * 2; + return "done"; +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Arrow function return should not affect outer function, got: {diags:?}" + ); +} + +#[test] +fn nested_closure_with_array_map_does_not_leak() { + let php = r#" 0) { + if ($b > 0) { + if ($c > 0) { + return "deep"; + } else { + return "c-neg"; + } + } else { + return "b-neg"; + } + } else { + return "a-neg"; + } +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag correct returns in deeply nested if, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_correct_return_in_do_while() { + let php = r#" 100) { + return "big"; + } elseif ($x > 50) { + return "medium"; + } elseif ($x > 0) { + return []; + } else { + return "negative"; + } +} +"#; + let diags = collect(php); + let msgs = return_error_messages(&diags); + assert_eq!( + msgs.len(), + 1, + "Expected exactly one return error (the array branch), got: {msgs:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: declare(strict_types=1) interactions +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn strict_types_flags_int_returned_from_string() { + let php = r#"name = 'default'; + return; + } + $this->name = $name; + } +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag bare return in constructor, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// False-positive tests: returning typed parameters directly +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_returning_typed_parameter() { + let php = r#" 1000) { return "huge"; } + if ($n > 100) { return "large"; } + if ($n > 10) { return "medium"; } + if ($n > 0) { return "small"; } + if ($n === 0) { return "zero"; } + return "negative"; +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag any of the many correct string returns, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Edge case: function with no return statement and non-void type +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_no_return_statement() { + // Functions that throw or loop forever might have no return. + // We only check explicit return statements, not missing returns. + let php = r#" */ + public function getCounts(): array { + return ['a' => 1, 'b' => 2]; + } +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag array literal from method with @return array, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_phpdoc_collection_return() { + let php = r#" */ + public function getUsers(): Collection { + return new Collection(); + } +} + +class User {} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag Collection returned from Collection-typed method, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: generic / typed array return types +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_array_literal_from_typed_array_return() { + // The function returns array but the PHPDoc says int[]. + // An empty array literal should be fine. + let php = r#" */ +function get_names(): array { + return []; +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag empty array from list return type, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_array_from_generic_array_return() { + let php = r#" */ +function get_config(): array { + return ['key' => 'value']; +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag array literal from array, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: nullable union with class types +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_class_returned_from_nullable_class() { + let php = r#" 'Alice', 'age' => 30]; +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag matching array shape return, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: real-world patterns with generics and inheritance +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_builder_pattern_fluent_returns() { + let php = r#"r * $this->r; } +} + +class Square extends Shape { + public function __construct(private float $s) {} + public function area(): float { return $this->s * $this->s; } +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag factory methods returning subclasses of self, got: {diags:?}" + ); +} + +#[test] +fn no_diagnostic_for_repository_pattern_nullable_return() { + let php = r#" 'Active', + self::Inactive => 'Inactive', + self::Pending => 'Pending', + }; + } +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag match expression returning strings from string method, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: method with multiple nullable/union return paths +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_complex_conditional_nullable_returns() { + let php = r#"helper->getName(); + } +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag method call return matching declared type, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: returning from private/protected methods +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_correct_private_method_return() { + let php = r#" $s !== ''; + return []; + } +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag method with internal closures returning correct type, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: PHP 8.1+ enum backed values +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_backed_enum_method_correct_return() { + let php = r#" 'red', + self::Clubs, self::Spades => 'black', + }; + } + + public function isRed(): bool { + return $this === self::Hearts || $this === self::Diamonds; + } +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag correct returns in backed enum methods, got: {diags:?}" + ); +} + +// ═══════════════════════════════════════════════════════════════════════════ +// Advanced: never return type (should have no returns at all) +// ═══════════════════════════════════════════════════════════════════════════ + +#[test] +fn no_diagnostic_for_never_function_that_throws() { + let php = r#" $id, 'total' => 100]; + } + + public function getStatus(int $id): string|int { + if ($id <= 0) { + return "unknown"; + } + return $id; + } + + public function process(int $id): bool { + if ($id <= 0) { + return false; + } + return true; + } + + public function getTotal(int $id): float { + return 99.99; + } + + public function cancel(int $id): void { + return; + } +} +"#; + let diags = collect(php); + assert!( + !has_return_error(&diags), + "Should not flag any returns in complex service class, got: {diags:?}" + ); +} diff --git a/tests/integration/diagnostics_type_errors.rs b/tests/integration/diagnostics_type_errors.rs index 728511c0..0dec13a7 100644 --- a/tests/integration/diagnostics_type_errors.rs +++ b/tests/integration/diagnostics_type_errors.rs @@ -11,7 +11,7 @@ fn collect(php: &str) -> Vec { let uri = "file:///test.php"; backend.update_ast(uri, php); let mut out = Vec::new(); - backend.collect_type_error_diagnostics(uri, php, &mut out); + backend.collect_argument_type_diagnostics(uri, php, &mut out); out } @@ -20,7 +20,7 @@ fn collect_with_stubs(php: &str) -> Vec { let uri = "file:///test.php"; backend.update_ast(uri, php); let mut out = Vec::new(); - backend.collect_type_error_diagnostics(uri, php, &mut out); + backend.collect_argument_type_diagnostics(uri, php, &mut out); out } @@ -29,7 +29,7 @@ fn collect_with_full_stubs(php: &str) -> Vec { let uri = "file:///test.php"; backend.update_ast(uri, php); let mut out = Vec::new(); - backend.collect_type_error_diagnostics(uri, php, &mut out); + backend.collect_argument_type_diagnostics(uri, php, &mut out); out } @@ -3839,7 +3839,7 @@ class TestCase { let uri = format!("file://{}/src/TestCase.php", dir.path().display()); let content = files[3].1; let mut diags = Vec::new(); - backend.collect_type_error_diagnostics(&uri, content, &mut diags); + backend.collect_argument_type_diagnostics(&uri, content, &mut diags); let msgs = type_error_messages(&diags); // The argument to ClassNode::__construct is the return value of // getNodeForCallingTestCase which returns ASTNode. The diagnostic @@ -3939,7 +3939,7 @@ class MyException extends NativeException {} backend.update_ast("file:///test.php", php); let mut out = Vec::new(); - backend.collect_type_error_diagnostics("file:///test.php", php, &mut out); + backend.collect_argument_type_diagnostics("file:///test.php", php, &mut out); let msgs = type_error_messages(&out); assert!( msgs.is_empty(), diff --git a/tests/integration/main.rs b/tests/integration/main.rs index d5a3c245..55985411 100644 --- a/tests/integration/main.rs +++ b/tests/integration/main.rs @@ -94,6 +94,8 @@ mod diag_forward_walk_closures; mod diag_forward_walk_complex_expr; mod diag_timing; mod diagnostics_deprecated; +mod diagnostics_property_type_errors; +mod diagnostics_return_type_errors; mod diagnostics_type_errors; mod diagnostics_undefined_variables; mod diagnostics_unknown_members;