Skip to content

Commit bff2a82

Browse files
committed
An argument written as an array element, a global constant, or a simple
operator expression is no longer read as nothing
1 parent 0c4c13d commit bff2a82

7 files changed

Lines changed: 244 additions & 66 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+
- **An argument written as an array element, a global constant, or a simple operator expression is no longer read as nothing.** `str_replace()`, `preg_replace()`, and any `@template` binding all decide part of their answer from the argument's own source text, and several ordinary ways of writing that argument left them with nothing to go on: `str_replace('a', 'b', $data['message'])` where `$data` is `array{message: string}`, `preg_replace('/-.*/', '', PHP_VERSION)`, and `preg_replace('/\s+/', ' ', $body ?: '')` all kept the full undecided union of both replace branches, even though assigning the same expression to a variable first resolved it correctly. An array-shape or generic element now reads its own value type instead of only the class-backed results the general resolver reported; a bare identifier that names a global constant is read through the same constant lookup hover already uses; and concatenation (always `string`) and the elvis operator (the union of both sides) are read directly rather than being mistaken for a bare variable name.
122123
- **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.
123124
- **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.
124125
- **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.

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ unlikely to move the needle for most users.
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)** | | |
100100
| 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 |
101-
| 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 |
102101
| | **[Code Actions](todo/actions.md)** | | |
103102
| A40 | [Generate method from call](todo/actions.md#a40-generate-method-from-call) | Medium-High | Medium |
104103
| A41 | [Create class from non-existing name](todo/actions.md#a41-create-class-from-non-existing-name) | Medium | Medium |

docs/todo/bugs.md

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -66,48 +66,3 @@ exact site before fixing.
6666
this exact `UploadedFile|array<UploadedFile>(|null)` shape, all following a
6767
correct `instanceof` guard in the source; one more instance in
6868
`projects/vytrvalec-server` uses a different class.
69-
70-
### B124. An argument's type is read from its source text, and several ordinary spellings read as nothing
71-
72-
**Impact: Medium · Effort: Medium**
73-
74-
```php
75-
/** @param array{message: string} $data */
76-
function report(array $data, string $body): void {
77-
$out = str_replace('a', 'b', $data['message']); // array<array-key, string>|string
78-
$subject = $data['message'];
79-
$sameThing = str_replace('a', 'b', $subject); // string — correct
80-
81-
$version = preg_replace('/-.*/', '', PHP_VERSION); // array<array-key, string>|string
82-
$trimmed = preg_replace('/\s+/', ' ', $body ?: ''); // array<array-key, string>|string
83-
}
84-
```
85-
86-
`Backend::resolve_arg_text_to_type` is the shared "what type is this
87-
argument" helper that conditional return types and `@template` binding both
88-
consult, and it works from the argument's *source text*. It answers for
89-
literals, casts, variables, property chains, calls, `::class` and static
90-
access, but several ordinary spellings resolve to nothing:
91-
92-
- an array element (`$data['message']`, `$rows[0]`): the general expression
93-
path reports only class-backed results, so an element holding a scalar
94-
comes back empty, and the raw-type fallback skips any text containing `[`
95-
outright;
96-
- a global constant (`PHP_VERSION`, `PHP_EOL`): there is no constant branch,
97-
and `ResolutionCtx` does not carry the constant loader that
98-
`VarResolutionCtx` has, so one would have nothing to consult;
99-
- an operator expression (`$body ?: ''`, `$a . $b`, `$n + 1`): nothing reads
100-
the operator, even where it alone decides the type.
101-
102-
Assigning the same expression to a variable first resolves fine, so the
103-
answer depends on how the call was spelled. The visible effects are a
104-
conditional return type that stays undecided (and therefore returns the
105-
union of both branches, as `str_replace` does above) and a template
106-
parameter that stays unbound.
107-
108-
**Fix:** each spelling needs its own small branch in the text resolver,
109-
answering with what the same expression resolves to when it is assigned to a
110-
variable first: array access via `SubjectExpr::ArrayAccess` plus the
111-
array-shape key lookup the forward walker already has, a global constant via
112-
the constant loader (which `ResolutionCtx` has to start carrying), and the
113-
operators whose result type is fixed regardless of their operands.

examples/demo.php

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1152,6 +1152,20 @@ public function demo(): void
11521152
$swappedAll = str_replace('a', 'b', ['banana', 'apple']);
11531153
strtoupper($swappedAll[0]); // array subject → array<array-key, string>
11541154

1155+
// The subject is read the same way spelled other ways too: an
1156+
// array-shape element, a global constant, and an elvis expression
1157+
// all rule out the array branch instead of leaving it undecided.
1158+
/** @var array{message: string} $shapeData */
1159+
$shapeData = ['message' => 'banana'];
1160+
$fromShape = str_replace('a', 'b', $shapeData['message']);
1161+
strtoupper($fromShape); // array-shape element subject → string
1162+
$fromConst = str_replace('.', '-', PHP_VERSION);
1163+
strtoupper($fromConst); // global constant subject → string
1164+
/** @var ?string $maybeBody */
1165+
$maybeBody = null;
1166+
$fromElvis = str_replace('a', 'b', $maybeBody ?: 'banana');
1167+
strtoupper($fromElvis); // elvis-operator subject → string
1168+
11551169
// `preg_replace()` keeps its `null` error branch for a string
11561170
// subject, where PCRE really can fail, and drops it for an array.
11571171
$masked = preg_replace('/\d/', '*', 'a1b2') ?? '';
@@ -8309,6 +8323,15 @@ function runDemoAssertions(): void
83098323
// ── Subject-keyed conditional return type (the replace family) ───────
83108324
assert(str_replace('a', 'b', 'banana') === 'bbnbnb', 'a string subject replaces into a string');
83118325
assert(str_replace('a', 'b', ['banana', 'apple']) === ['bbnbnb', 'bpple'], 'an array subject replaces into an array');
8326+
8327+
// ── Other ordinary subject spellings (the replace family) ────────────
8328+
/** @var array{message: string} $shapeData */
8329+
$shapeData = ['message' => 'banana'];
8330+
assert(str_replace('a', 'b', $shapeData['message']) === 'bbnbnb', 'an array-shape element subject replaces into a string');
8331+
assert(is_string(str_replace('.', '-', PHP_VERSION)), 'a global constant subject replaces into a string');
8332+
$maybeBody = null;
8333+
assert(str_replace('a', 'b', $maybeBody ?: 'banana') === 'bbnbnb', 'an elvis-operator subject replaces into a string');
8334+
83128335
assert(preg_replace('/\d/', '*', 'a1b2') === 'a*b*', 'a string subject is replaced into a string');
83138336
assert(preg_replace('/\d/', '*', ['a1', 'b2']) === ['a*', 'b*'], 'an array subject is replaced into an array');
83148337
assert(substr_replace('banana', 'x', 0, 1) === 'xanana', 'a string subject is spliced into a string');

src/type_engine/call_resolution/return_types.rs

Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1959,6 +1959,141 @@ pub(super) fn resolve_cast_type(text: &str) -> Option<PhpType> {
19591959
Some(PhpType::named(atom(name)))
19601960
}
19611961

1962+
/// The type an operator expression produces, for the operators whose
1963+
/// answer can be read from the source text without a full parse.
1964+
///
1965+
/// Concatenation (`$a . $b`) always yields `string`, whatever its operands
1966+
/// are. The elvis operator (`$body ?: ''`) yields the union of both sides —
1967+
/// resolved recursively through [`Backend::resolve_arg_text_to_type`], the
1968+
/// same way assigning the expression to a variable first would resolve
1969+
/// through the AST-based `resolve_conditional_chain`.
1970+
///
1971+
/// A full three-part ternary (`$a ? $b : $c`) and arithmetic operators
1972+
/// (`+`, `-`, `*`, …) are deliberately left unanswered here: arithmetic's
1973+
/// result depends on whether its operands are int or float (and `+` alone
1974+
/// can mean array union), which the source text can't decide without
1975+
/// resolving both operands' concrete types.
1976+
pub(super) fn resolve_operator_type(text: &str, ctx: &ResolutionCtx<'_>) -> Option<PhpType> {
1977+
if contains_top_level_concat(text) {
1978+
return Some(PhpType::named(atom("string")));
1979+
}
1980+
if let Some((left, right)) = split_top_level_elvis(text) {
1981+
let left_ty = Backend::resolve_arg_text_to_type(left, ctx);
1982+
let right_ty = Backend::resolve_arg_text_to_type(right, ctx);
1983+
return match (left_ty, right_ty) {
1984+
(Some(l), Some(r)) if l == r => Some(l),
1985+
(Some(l), Some(r)) => Some(PhpType::union(vec![l, r])),
1986+
(Some(l), None) => Some(l),
1987+
(None, Some(r)) => Some(r),
1988+
(None, None) => None,
1989+
};
1990+
}
1991+
None
1992+
}
1993+
1994+
/// Whether `text` contains a concatenation (`.`) outside quotes, parens,
1995+
/// brackets, and `->`/`?->` chain links.
1996+
///
1997+
/// A pure numeric literal (`3.14`) is already answered by
1998+
/// [`resolve_literal_type`] before this runs, so any `.` reaching this scan
1999+
/// belongs to a genuine concatenation rather than a decimal point.
2000+
fn contains_top_level_concat(text: &str) -> bool {
2001+
let bytes = text.as_bytes();
2002+
let mut depth: u32 = 0;
2003+
let mut quote: Option<u8> = None;
2004+
let mut i = 0;
2005+
while i < bytes.len() {
2006+
let b = bytes[i];
2007+
if let Some(q) = quote {
2008+
if b == b'\\' {
2009+
i += 2;
2010+
continue;
2011+
}
2012+
if b == q {
2013+
quote = None;
2014+
}
2015+
i += 1;
2016+
continue;
2017+
}
2018+
match b {
2019+
b'\'' | b'"' => {
2020+
quote = Some(b);
2021+
i += 1;
2022+
}
2023+
b'(' | b'[' | b'{' => {
2024+
depth += 1;
2025+
i += 1;
2026+
}
2027+
b')' | b']' | b'}' => {
2028+
depth = depth.saturating_sub(1);
2029+
i += 1;
2030+
}
2031+
b'?' if bytes[i..].starts_with(b"?->") => i += 3,
2032+
b'-' if bytes[i..].starts_with(b"->") => i += 2,
2033+
b'.' if depth == 0 => {
2034+
// `...` (spread/variadic) is not concatenation.
2035+
if bytes[i..].starts_with(b"...") {
2036+
i += 3;
2037+
} else {
2038+
return true;
2039+
}
2040+
}
2041+
_ => i += 1,
2042+
}
2043+
}
2044+
false
2045+
}
2046+
2047+
/// Split `text` at a top-level elvis operator (`?:`), respecting quotes,
2048+
/// parens/brackets, and the nullsafe `?->` operator (which is not this).
2049+
///
2050+
/// Returns the trimmed left and right operand texts, or `None` when no
2051+
/// top-level `?:` is found — including a full ternary (`$a ? $b : $c`),
2052+
/// which is left to the caller's other paths.
2053+
fn split_top_level_elvis(text: &str) -> Option<(&str, &str)> {
2054+
let bytes = text.as_bytes();
2055+
let mut depth: u32 = 0;
2056+
let mut quote: Option<u8> = None;
2057+
let mut i = 0;
2058+
while i < bytes.len() {
2059+
let b = bytes[i];
2060+
if let Some(q) = quote {
2061+
if b == b'\\' {
2062+
i += 2;
2063+
continue;
2064+
}
2065+
if b == q {
2066+
quote = None;
2067+
}
2068+
i += 1;
2069+
continue;
2070+
}
2071+
match b {
2072+
b'\'' | b'"' => {
2073+
quote = Some(b);
2074+
i += 1;
2075+
}
2076+
b'(' | b'[' | b'{' => {
2077+
depth += 1;
2078+
i += 1;
2079+
}
2080+
b')' | b']' | b'}' => {
2081+
depth = depth.saturating_sub(1);
2082+
i += 1;
2083+
}
2084+
b'?' if depth == 0 && !bytes[i..].starts_with(b"?->") => {
2085+
let after = text[i + 1..].trim_start();
2086+
if let Some(rest) = after.strip_prefix(':') {
2087+
return Some((text[..i].trim_end(), rest.trim_start()));
2088+
}
2089+
i += 1;
2090+
}
2091+
_ => i += 1,
2092+
}
2093+
}
2094+
None
2095+
}
2096+
19622097
/// Whether `operand` is one expression rather than several joined by an
19632098
/// operator.
19642099
///

src/type_engine/call_resolution/template_subs.rs

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ use crate::type_engine::resolver::{Loaders, ResolutionCtx};
1616

1717
use super::return_types::{
1818
resolve_call_return_hint, resolve_cast_type, resolve_chain_declared_return,
19-
resolve_expression_to_type, resolve_literal_type, resolve_static_access_type,
19+
resolve_expression_to_type, resolve_literal_type, resolve_operator_type,
20+
resolve_static_access_type,
2021
};
2122

2223
impl Backend {
@@ -734,6 +735,34 @@ impl Backend {
734735
return class_named.map(|n| PhpType::class_string(Some(n)));
735736
}
736737

738+
// Global constant access: `PHP_VERSION`, `PHP_EOL`, etc.
739+
//
740+
// A bare identifier that isn't a keyword, a `::class`/enum/const
741+
// access (handled above and below), or any other special form is a
742+
// global constant reference. Consult the constant loader (derived
743+
// from the attached `Backend`, the same source `VarResolutionCtx`
744+
// draws from) and infer the type from its value, mirroring the
745+
// `Expression::ConstantAccess` branch the AST-based RHS resolver
746+
// already has for a plain `$x = PHP_EOL;` assignment.
747+
if !trimmed.is_empty()
748+
&& !trimmed.starts_with('$')
749+
&& !trimmed.contains("::")
750+
&& !trimmed.contains("->")
751+
&& !trimmed.contains('(')
752+
&& !trimmed.contains('[')
753+
&& trimmed
754+
.chars()
755+
.all(|c| c.is_alphanumeric() || c == '_' || c == '\\')
756+
&& !is_self_or_static(trimmed)
757+
&& !trimmed.eq_ignore_ascii_case("parent")
758+
&& let Some(backend) = ctx.backend
759+
&& let Some(Some(value)) = backend.constant_loader()(trimmed)
760+
&& let Some(ty) =
761+
crate::type_engine::variable::rhs_resolution::infer_type_from_constant_value(&value)
762+
{
763+
return Some(ty);
764+
}
765+
737766
// When the expression contains a `->` chain (e.g.
738767
// `Country::DK->value`, `new Decimal($x)->toFixed(2)`),
739768
// skip the static-access and new-expression shortcuts —
@@ -787,6 +816,15 @@ impl Backend {
787816
return Some(ty);
788817
}
789818

819+
// Operators whose result is decided from their operands rather
820+
// than from source-text shape alone (`$a . $b`, `$body ?: ''`).
821+
// Checked before the general fallback because `SubjectExpr::parse`
822+
// has no notion of these operators and would otherwise misread the
823+
// whole expression as a single bare variable or class name.
824+
if let Some(ty) = resolve_operator_type(trimmed, ctx) {
825+
return Some(ty);
826+
}
827+
790828
// General expression fallback: parse the argument text as a
791829
// SubjectExpr and try to resolve it to a type. This handles
792830
// $var, $var->prop, $this->prop, $var->method(), method

0 commit comments

Comments
 (0)