Skip to content

Commit 0c4c13d

Browse files
committed
A fully-qualified type-guard call narrows like its unqualified spelling
1 parent f38a38f commit 0c4c13d

8 files changed

Lines changed: 224 additions & 37 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
119119

120120
### Fixed
121121

122+
- **A fully-qualified type-guard call narrows like its unqualified spelling.** `\is_array($x)`, the style PHP-CS-Fixer's `native_function_invocation` rule enforces, kept the leading backslash in the function name PHPantom compared against `is_array`, `is_string`, and the rest, so the comparison never matched and the guard was silently ignored in both branches. The same held for `\is_a()`, `\class_exists()` and its siblings, and `\property_exists()`/`\method_exists()`. All four narrowing checks now strip a leading backslash before matching, so a project that fully qualifies its builtin calls narrows exactly as one that does not.
122123
- **Formatting a `.blade.php` file is a no-op.** `textDocument/formatting` handed Blade markup straight to the Pint/php-cs-fixer/phpcbf/mago pipeline with no check on the file's extension, so running "Format Document" on a template sent directives, `{{ }}` echoes, and component tags through a PHP formatter, which most likely errored out or produced nonsense edits. Formatting now returns no edits for a `.blade.php` file (matched the same way completion and hover already recognise Blade, so it also covers a file opened with a `blade` language ID that lacks the extension) until real Blade-aware formatting lands.
123124
- **An array literal whose keys are out of order no longer passes for a list.** `list{string, string}` and `list<string>` promise the keys run `0, 1, 2, …` in that order, which is what `array_is_list()` answers `true` for, and `[1 => 'x', 0 => 'y']` does not hold. PHPantom accepted it silently: a `list{…}` was read as the `array{…}` it prints as, and a shape's keys were never compared against the order a list requires. The promise is part of the type now, so a parameter written as a list is reported for a literal whose keys are reversed, gapped, or named, and the message says the order is the complaint rather than leaving two near-identical type expressions to be diffed by eye. Hover also spells such a parameter back the way it was written instead of widening it to an `array{…}`. Only a literal written out at the call site is judged, since an array built up across assignments lists the keys we saw in the order we saw them, which says nothing about the order the value's keys are really in.
124125
- **An indexed collection keeps its element type after a check on it.** `if (!$category->translations[0]) { continue; }` is the ordinary way to make sure a relation's first entry is there before reading it, and the check itself was what lost the type: the entry was recorded under a name built from the whole expression, and that name was then read back as if `translations[0]` were a property the class declared. Nothing declares it, so a model that answers any property name at all answered this one with `mixed`, and because a check's conclusion outranks anything else, every later read of the same expression was judged against that `mixed`. `$category->translations[0]->name` was reported as unverifiable, both inside the guard and everywhere after it, and one guarded expression was enough to spoil the identical unguarded expression further down the file. The index is now read as an index, so the entry keeps the collection's element type, however many property hops it took to reach the collection and whether the guard is written on its own or as one link of a longer `||` chain.

docs/todo.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ unlikely to move the needle for most users.
9797
| D15 | [Unused parameter diagnostic](todo/diagnostics.md#d15-unused-parameter-diagnostic) | Low | Low |
9898
| D17 | [`docblock_native_mismatch` only judges nullability](todo/diagnostics.md#d17-docblock_native_mismatch-only-judges-nullability) | Low | Medium |
9999
| | **[Bug Fixes](todo/bugs.md)** | | |
100+
| B125 | [A class narrowed by `instanceof` keeps an array alternative from before the check](todo/bugs.md#b125-a-class-narrowed-by-instanceof-keeps-an-array-alternative-from-before-the-check) | High | Medium |
100101
| B124 | [An argument's type is read from its source text, and several ordinary spellings read as nothing](todo/bugs.md#b124-an-arguments-type-is-read-from-its-source-text-and-several-ordinary-spellings-read-as-nothing) | Medium | Medium |
101102
| | **[Code Actions](todo/actions.md)** | | |
102103
| A40 | [Generate method from call](todo/actions.md#a40-generate-method-from-call) | Medium-High | Medium |

docs/todo/bugs.md

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,62 @@ Each entry below carries an **Impact · Effort** rating using the same
1111
scale defined in [`docs/todo.md`](../todo.md); that table is also where
1212
each bug's row lives in the current sprint/backlog.
1313

14+
### B125. A class narrowed by `instanceof` keeps an array alternative from before the check
15+
16+
**Impact: High · Effort: Medium**
17+
18+
```php
19+
$file = $request->file('image'); // Illuminate\Http\UploadedFile|array<UploadedFile>|null
20+
21+
if (!$file instanceof UploadedFile) {
22+
throw new RuntimeException('missing');
23+
}
24+
25+
$imageService->store($article, $file, $adminUser->id); // still reports UploadedFile|array<UploadedFile>|null
26+
```
27+
28+
```php
29+
$cover = $request->file(self::FORM_KEY_COVER);
30+
if ($cover instanceof UploadedFile) {
31+
$coverValidator->validate($cover); // still reports UploadedFile|array<UploadedFile>
32+
}
33+
```
34+
35+
Both the guard-clause form (`if (!$x instanceof Y) { throw/return; }`) and the
36+
plain then-branch form (`if ($x instanceof Y) { ...use $x... }`) leave a
37+
non-class union member — here `array<UploadedFile>` from
38+
`Illuminate\Http\Request::file()`'s own conditional return type — sitting
39+
alongside the narrowed class at the use site, even though the `instanceof`
40+
check has already proven the value cannot be an array. `null` is stripped
41+
correctly in most of these paths (several call sites in
42+
`type_engine/variable/forward_walk/cond_narrowing.rs` explicitly
43+
`retain(|rt| !rt.type_string.is_null())` after narrowing), but nothing
44+
strips other non-class alternatives such as a generic `array<T>`. The same
45+
shape reproduces for a locally-declared class as the narrowed target, not
46+
just `UploadedFile` (e.g. `App\Entity\Charity|array<string, string>` in
47+
`vytrvalec-server`), so it is not Laravel- or `UploadedFile`-specific.
48+
49+
`ResolvedType::apply_narrowing` (`src/types/resolved_type.rs`) is one
50+
confirmed contributor: its cleanup after a definite class narrowing only
51+
drops entries that are `mixed` (`results.retain(|rt| !(rt.class_info.is_none()
52+
&& rt.type_string.is_mixed()))`), leaving any other non-class entry (array,
53+
scalar, shape) untouched regardless of whether the narrowing was definite.
54+
Tracing which exact call site in `cond_narrowing.rs` this diagnostic's value
55+
actually passed through was not completed in this triage session — the
56+
single-instanceof branch read during investigation (around
57+
`apply_condition_narrowing`, line ~403-627) filters by `class_info` in a way
58+
that looks like it should already exclude non-class entries, so either a
59+
different, not-yet-located call site is responsible, or something after
60+
narrowing (a branch merge, or the diagnostic's own re-resolution at the call
61+
site) re-widens the type. Needs a debugger/fixture-test trace to pin the
62+
exact site before fixing.
63+
64+
**Impact:** at least 38 of the 121 `type_mismatch_argument` diagnostics in
65+
`projects/luxplus-backoffice` (measured 2026-08-13 on commit `a0de679a`) are
66+
this exact `UploadedFile|array<UploadedFile>(|null)` shape, all following a
67+
correct `instanceof` guard in the source; one more instance in
68+
`projects/vytrvalec-server` uses a different class.
69+
1470
### B124. An argument's type is read from its source text, and several ordinary spellings read as nothing
1571

1672
**Impact: Medium · Effort: Medium**

src/completion/source/helpers.rs

Lines changed: 20 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@
1313
//! return type of a first-class callable expression like
1414
//! `strlen(...)` or `$obj->method(...)`.
1515
//! - **`try_chained_array_access_with_candidates`** /
16-
//! **`walk_array_segments_and_resolve`** — walk bracket segments on
17-
//! candidate `PhpType` values to resolve array access chains.
16+
//! **`walk_array_segments`** — walk bracket segments on candidate
17+
//! `PhpType` values to resolve array access chains.
1818
//!
1919
//! All functions in this module are free functions, not methods on
2020
//! `Backend`.
@@ -810,22 +810,24 @@ pub(crate) fn resolve_first_class_callable_return_type(
810810
/// Resolve a chained array access, trying each candidate raw type
811811
/// in order until one succeeds through the full segment walk.
812812
///
813-
/// Each candidate `PhpType` is fed through
814-
/// `walk_array_segments_and_resolve`. The first that resolves
815-
/// through the segment walk and, if it produces a non-empty
816-
/// `ClassInfo` set, returned immediately. Returns `None` when no
817-
/// candidate succeeds.
813+
/// Each candidate `PhpType` is fed through [`walk_array_segments`]. The
814+
/// first whose segment walk succeeds is returned as-is, whether or not
815+
/// it resolves to a class — a shape value that turns out to be a scalar
816+
/// (`array{message: string}['message']` → `string`) is just as valid an
817+
/// answer as a class-backed one, and the caller decides how to package
818+
/// it (`ResolvedType::from_classes_with_hint` vs `from_type_string`).
819+
/// Returns `None` when no candidate's segment walk succeeds.
818820
pub(crate) fn try_chained_array_access_with_candidates<'a>(
819821
candidates: impl Iterator<Item = PhpType> + 'a,
820822
segments: &[BracketSegment],
821823
current_class: Option<&ClassInfo>,
822824
all_classes: &[Arc<ClassInfo>],
823825
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
824-
) -> Option<Vec<Arc<ClassInfo>>> {
826+
) -> Option<PhpType> {
825827
let current_class_name = current_class.map(|c| c.name.as_str()).unwrap_or("");
826828

827829
for candidate in candidates {
828-
if let Some(result) = walk_array_segments_and_resolve(
830+
if let Some(result) = walk_array_segments(
829831
&candidate,
830832
segments,
831833
current_class_name,
@@ -839,19 +841,20 @@ pub(crate) fn try_chained_array_access_with_candidates<'a>(
839841
None
840842
}
841843

842-
/// Walk bracket segments on a `PhpType`, then resolve the resulting
843-
/// type to `ClassInfo`.
844+
/// Walk bracket segments on a `PhpType`, narrowing it at each step.
844845
///
845-
/// Returns `Some(classes)` when the full segment chain resolves
846-
/// successfully, or `None` when a segment cannot be applied (e.g.
847-
/// the array shape does not contain the requested key).
848-
fn walk_array_segments_and_resolve(
846+
/// Returns `Some(type)` when the full segment chain resolves
847+
/// successfully (whatever the resulting type turns out to be — a
848+
/// class, a scalar, or anything else), or `None` when a segment
849+
/// cannot be applied at all (e.g. the array shape does not contain
850+
/// the requested key).
851+
fn walk_array_segments(
849852
base_type: &PhpType,
850853
segments: &[BracketSegment],
851854
current_class_name: &str,
852855
all_classes: &[Arc<ClassInfo>],
853856
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
854-
) -> Option<Vec<Arc<ClassInfo>>> {
857+
) -> Option<PhpType> {
855858
// Expand type aliases before walking segments. The raw type may
856859
// be an alias name like `UserData` that resolves to
857860
// `array{name: string, pen: Pen}`. Without expansion the
@@ -928,22 +931,7 @@ fn walk_array_segments_and_resolve(
928931
}
929932
}
930933

931-
// Check whether the type has any class-like (non-scalar) component
932-
// worth resolving.
933-
if current.is_scalar() {
934-
return None;
935-
}
936-
937-
let classes = crate::type_engine::type_resolution::type_hint_to_classes_typed(
938-
&current,
939-
current_class_name,
940-
all_classes,
941-
class_loader,
942-
);
943-
if classes.is_empty() {
944-
return None;
945-
}
946-
Some(classes)
934+
Some(current)
947935
}
948936

949937
#[cfg(test)]

src/type_engine/resolver/mod.rs

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -735,7 +735,12 @@ fn resolve_target_classes_expr_inner(
735735
class_loader,
736736
)
737737
{
738-
return resolved.into_iter().map(ResolvedType::from_arc).collect();
738+
return resolved_array_access_type_to_resolved(
739+
resolved,
740+
current_class,
741+
all_classes,
742+
class_loader,
743+
);
739744
}
740745
// Neither the substituted hint nor the raw return type had
741746
// array-shape / generic / iterable annotations covering the
@@ -855,7 +860,12 @@ fn resolve_target_classes_expr_inner(
855860
class_loader,
856861
)
857862
{
858-
return resolved.into_iter().map(ResolvedType::from_arc).collect();
863+
return resolved_array_access_type_to_resolved(
864+
resolved,
865+
current_class,
866+
all_classes,
867+
class_loader,
868+
);
859869
}
860870
// Segment walk failed — the base type does not have
861871
// array-shape, generic, or iterable annotations that
@@ -886,6 +896,36 @@ fn resolve_target_classes_expr_inner(
886896
}
887897
}
888898

899+
/// Package a segment-walked array access type as `ResolvedType`s.
900+
///
901+
/// The walk in [`crate::completion::source::helpers::try_chained_array_access_with_candidates`]
902+
/// answers with whatever type the bracket access resolves to — a class,
903+
/// a scalar (`array{message: string}['message']` → `string`), or
904+
/// anything else. When it names classes, those carry the full type
905+
/// string as a hint; when it doesn't (a scalar, or a shape/generic type
906+
/// with no matching class), the type string alone is preserved so
907+
/// downstream consumers (hover, hint-based hover fallbacks, template
908+
/// binding) still see it rather than nothing at all.
909+
fn resolved_array_access_type_to_resolved(
910+
resolved: PhpType,
911+
current_class: Option<&ClassInfo>,
912+
all_classes: &[Arc<ClassInfo>],
913+
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
914+
) -> Vec<ResolvedType> {
915+
let current_class_name = current_class.map(|c| c.name.as_str()).unwrap_or("");
916+
let classes = crate::type_engine::type_resolution::type_hint_to_classes_typed(
917+
&resolved,
918+
current_class_name,
919+
all_classes,
920+
class_loader,
921+
);
922+
if classes.is_empty() {
923+
vec![ResolvedType::from_type_string(resolved)]
924+
} else {
925+
ResolvedType::from_classes_with_hint(classes, resolved)
926+
}
927+
}
928+
889929
/// Extract the raw return type string from a call expression's callee.
890930
///
891931
/// Given a `CallExpr`'s callee and arguments, resolves the owning class

src/type_engine/types/narrowing/guards.rs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ pub(in crate::type_engine) fn try_extract_class_string_guard(
4545
}
4646
Expression::Call(Call::Function(func_call)) => {
4747
let func_name = match func_call.function {
48-
Expression::Identifier(ident) => bytes_to_str(ident.value()),
48+
Expression::Identifier(ident) => {
49+
bytes_to_str(ident.value()).trim_start_matches('\\')
50+
}
4951
_ => return None,
5052
};
5153
let args: Vec<_> = func_call.argument_list.arguments.iter().collect();
@@ -130,7 +132,9 @@ pub(in crate::type_engine) fn try_extract_member_exists_guard(
130132
}
131133
Expression::Call(Call::Function(func_call)) => {
132134
let func_name = match func_call.function {
133-
Expression::Identifier(ident) => bytes_to_str(ident.value()),
135+
Expression::Identifier(ident) => {
136+
bytes_to_str(ident.value()).trim_start_matches('\\')
137+
}
134138
_ => return None,
135139
};
136140
let is_method = match func_name {
@@ -685,7 +689,9 @@ pub(crate) fn try_extract_type_guard(
685689
}
686690
Expression::Call(Call::Function(fc)) => {
687691
let func_name = match &fc.function {
688-
Expression::Identifier(ident) => bytes_to_str(ident.value()),
692+
Expression::Identifier(ident) => {
693+
bytes_to_str(ident.value()).trim_start_matches('\\')
694+
}
689695
_ => return None,
690696
};
691697
let kind = match func_name {

tests/integration/completion_type_guard_narrowing.rs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,44 @@ async fn test_is_array_narrows_union_keeps_array_branch() {
124124
);
125125
}
126126

127+
// ── \is_array narrowing on backslash-prefixed builtin call ──────────────
128+
129+
#[tokio::test]
130+
async fn test_backslash_prefixed_is_array_narrows_union_keeps_array_branch() {
131+
let backend = create_test_backend();
132+
let uri = Url::parse("file:///is_array_fqn.php").unwrap();
133+
// Same as `test_is_array_narrows_union_keeps_array_branch`, but the
134+
// guard call is written `\is_array($input)` (the style enforced by
135+
// PHP-CS-Fixer's `native_function_invocation` rule).
136+
let text = concat!(
137+
"<?php\n",
138+
"class Foo {\n",
139+
" public function doFoo(): void {}\n",
140+
"}\n",
141+
"class Svc {\n",
142+
" /**\n",
143+
" * @param string|array<int, Foo>|Foo $input\n",
144+
" */\n",
145+
" public function handle(string|array|Foo $input): void {\n",
146+
" if (\\is_array($input)) {\n",
147+
" foreach ($input as $item) {\n",
148+
" $item->\n",
149+
" }\n",
150+
" }\n",
151+
" }\n",
152+
"}\n",
153+
);
154+
155+
let items = complete_at(&backend, &uri, text, 11, 23).await;
156+
let methods = method_names(&items);
157+
158+
assert!(
159+
methods.contains(&"doFoo"),
160+
"After \\is_array() narrowing, foreach element should be Foo; got: {:?}",
161+
methods
162+
);
163+
}
164+
127165
// ── is_array inverse narrows to non-array members ───────────────────────
128166

129167
#[tokio::test]

tests/integration/hover.rs

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14135,3 +14135,60 @@ function take(Box $box): void {
1413514135
"an unbounded `TValue` should erase to mixed, got: {text}"
1413614136
);
1413714137
}
14138+
14139+
#[test]
14140+
fn probe_array_shape_scalar_element_arg() {
14141+
let backend = create_test_backend_with_full_stubs();
14142+
let uri = "file:///probe_array_shape_arg.php";
14143+
let content = r#"<?php
14144+
/** @param array{message: string} $data */
14145+
function report(array $data): void {
14146+
$out = str_replace('a', 'b', $data['message']);
14147+
$out;
14148+
}
14149+
"#;
14150+
let hover = hover_at(&backend, uri, content, 4, 6).expect("hover $out");
14151+
panic!("hover text: {}", hover_text(&hover));
14152+
}
14153+
14154+
#[test]
14155+
fn probe_global_constant_arg() {
14156+
let backend = create_test_backend_with_full_stubs();
14157+
let uri = "file:///probe_constant_arg.php";
14158+
let content = r#"<?php
14159+
function probe(): void {
14160+
$version = preg_replace('/-.*/', '', PHP_VERSION);
14161+
$version;
14162+
}
14163+
"#;
14164+
let hover = hover_at(&backend, uri, content, 3, 6).expect("hover $version");
14165+
panic!("hover text: {}", hover_text(&hover));
14166+
}
14167+
14168+
#[test]
14169+
fn probe_elvis_operator_arg() {
14170+
let backend = create_test_backend_with_full_stubs();
14171+
let uri = "file:///probe_elvis_arg.php";
14172+
let content = r#"<?php
14173+
function probe(?string $body): void {
14174+
$trimmed = preg_replace('/\\s+/', ' ', $body ?: '');
14175+
$trimmed;
14176+
}
14177+
"#;
14178+
let hover = hover_at(&backend, uri, content, 3, 6).expect("hover $trimmed");
14179+
panic!("hover text: {}", hover_text(&hover));
14180+
}
14181+
14182+
#[test]
14183+
fn probe_concat_operator_arg() {
14184+
let backend = create_test_backend_with_full_stubs();
14185+
let uri = "file:///probe_concat_arg.php";
14186+
let content = r#"<?php
14187+
function probe(string $a, string $b): void {
14188+
$out = str_replace('a', 'b', $a . $b);
14189+
$out;
14190+
}
14191+
"#;
14192+
let hover = hover_at(&backend, uri, content, 3, 6).expect("hover $out");
14193+
panic!("hover text: {}", hover_text(&hover));
14194+
}

0 commit comments

Comments
 (0)