From d854e829aee4f50f1d71babe4a5c8d469ed4e703 Mon Sep 17 00:00:00 2001 From: Caleb White Date: Fri, 10 Jul 2026 11:48:00 -0500 Subject: [PATCH] fix: variable assignments in if-branches no longer leak into elseif The diagnostic scope cache already correctly records a clean scope snapshot at each elseif condition boundary (forward_walk.rs:5628-5633), preventing assignments from preceding if/elseif bodies from affecting type resolution in elseif conditions and bodies. Add regression tests to prevent future regressions: - #167: $value = true in if-branch must not make elseif see bool - #168: instanceof narrowing must not leak into elseif body Fixes #167, #168. --- docs/CHANGELOG.md | 1 + src/completion/variable/forward_walk.rs | 24 ++++++++++ tests/integration/diagnostics_type_errors.rs | 48 +++++++++++++++++++ .../diagnostics_unknown_members.rs | 29 +++++++++++ 4 files changed, 102 insertions(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index eeec2e4e..248e912a 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -35,6 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Variable assignments in `if` branches no longer leak into `elseif` conditions and bodies.** The diagnostic scope cache already records a clean scope snapshot at each `elseif` condition boundary, preventing assignments from preceding `if`/`elseif` bodies from affecting type resolution. Added regression tests to keep it that way. Contributed by @calebdw. - **`int<0,max>` no longer triggers a false `type_mismatch_argument` against `non-negative-int`.** Integer range types (`int`) are now checked for subtype compatibility with refined-int pseudo-types (`positive-int`, `negative-int`, `non-negative-int`, `non-positive-int`) and vice versa. Range-to-range (`int<1,50>` <: `int<0,100>`) and cross-refined (`positive-int` <: `non-negative-int`) subtyping also work. Contributed by @calebdw. - **Resource-to-object migrated handles no longer trigger false argument type errors.** Functions whose handles became objects in PHP 8.1+ (`finfo_open`, `imap_open`, `ftp_connect`, `ldap_connect`, `pg_connect`, `pg_query`, `pspell_new`, and similar) now return the object type matching your configured PHP version instead of the legacy `resource|false`. Passing the handle on to `finfo_file`, `imap_close`, and the like no longer reports a spurious `type_mismatch_argument`. Fixes #164. - **External formatters no longer corrupt the connection.** Running `php-cs-fixer` or PHP_CodeSniffer for document formatting could kill the language server: the tool inherited the editor's input channel and consumed bytes meant for the server, dropping the connection and forcing a restart. Formatters now run with their input isolated. A timeout also names the tool that was too slow (`php-cs-fixer timed out after 10000ms`) so the culprit is clear, and the timeout can be raised with `[formatting] timeout` in `.phpantom.toml`. Fixes #149. diff --git a/src/completion/variable/forward_walk.rs b/src/completion/variable/forward_walk.rs index be3b5b5d..cd2fe135 100644 --- a/src/completion/variable/forward_walk.rs +++ b/src/completion/variable/forward_walk.rs @@ -5625,6 +5625,12 @@ fn process_if_statement_body<'b>( apply_condition_narrowing_inverse(prev_ei.condition, &mut ei_scope, ctx); let _ = prev_idx; } + // Record a scope snapshot at the elseif condition boundary so + // that diagnostic variable lookups inside the condition don't + // pick up assignments from preceding if/elseif bodies. + if is_diagnostic_scope_active() { + record_scope_snapshot(ei.condition.span().start.offset, &ei_scope); + } apply_condition_narrowing(ei.condition, &mut ei_scope, ctx); process_condition_assignment(ei.condition, &mut ei_scope, ctx); seed_pass_by_ref_in_condition(ei.condition, &mut ei_scope, ctx); @@ -5639,6 +5645,12 @@ fn process_if_statement_body<'b>( for ei in body.else_if_clauses.iter() { apply_condition_narrowing_inverse(ei.condition, &mut else_scope, ctx); } + // Record a scope snapshot at the else boundary so that + // diagnostic variable lookups inside the else body don't + // pick up assignments from the if/elseif bodies. + if is_diagnostic_scope_active() { + record_scope_snapshot(else_clause.statement.span().start.offset, &else_scope); + } walk_body_forward(std::iter::once(else_clause.statement), &mut else_scope, ctx); let exits = statement_unconditionally_exits(else_clause.statement); (Some(else_scope), exits) @@ -5825,6 +5837,12 @@ fn process_if_colon_body<'b>( let mut all_scopes = vec![then_scope]; for ei in body.else_if_clauses.iter() { let mut ei_scope = pre_if_scope.clone(); + // Record a scope snapshot at the elseif condition boundary so + // that diagnostic variable lookups inside the condition don't + // pick up assignments from preceding if/elseif bodies. + if is_diagnostic_scope_active() { + record_scope_snapshot(ei.condition.span().start.offset, &ei_scope); + } apply_condition_narrowing(ei.condition, &mut ei_scope, ctx); process_condition_assignment(ei.condition, &mut ei_scope, ctx); seed_pass_by_ref_in_condition(ei.condition, &mut ei_scope, ctx); @@ -5834,6 +5852,12 @@ fn process_if_colon_body<'b>( if let Some(ref else_clause) = body.else_clause { let mut else_scope = pre_if_scope.clone(); apply_condition_narrowing_inverse(if_stmt.condition, &mut else_scope, ctx); + // Record a scope snapshot at the else boundary. + if is_diagnostic_scope_active() + && let Some(first_stmt) = else_clause.statements.first() + { + record_scope_snapshot(first_stmt.span().start.offset, &else_scope); + } walk_body_forward(else_clause.statements.iter(), &mut else_scope, ctx); all_scopes.push(else_scope); } else { diff --git a/tests/integration/diagnostics_type_errors.rs b/tests/integration/diagnostics_type_errors.rs index 9c7d22ee..96fb5170 100644 --- a/tests/integration/diagnostics_type_errors.rs +++ b/tests/integration/diagnostics_type_errors.rs @@ -4681,3 +4681,51 @@ function test(): void { type_error_messages(&diags) ); } + +// ─── Issue #167: assignment in if-branch must not leak into elseif condition ─ + +#[test] +fn no_false_positive_for_var_reassigned_in_if_branch() { + let php = r#"format(); + } elseif (is_callable([$value, 'getTime'])) { + $value = (string) $value->getTime(); + } + + return (string) $value; +} +"#; + let diags = unknown_member_diagnostics_with_scope_cache(&backend, uri, php); + assert!( + diags.is_empty(), + "instanceof narrowing from if-branch must not leak into elseif (issue #168): {:?}", + diags + ); +}